Re: [PATCH net-next v15 11/22] ovpn: implement TCP transport
flat view
From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2024-12-16 13:59:13
Also in:
linux-kselftest, lkml
2024-12-11, 22:15:15 +0100, Antonio Quartulli wrote:
quoted hunk ↗ jump to hunk
@@ -42,6 +56,31 @@ struct ovpn_peer { struct in6_addr ipv6; } vpn_addrs; struct ovpn_socket *sock; + + /* state of the TCP reading. Needed to keep track of how much of a + * single packet has already been read from the stream and how much is + * missing + */
nit: not so accurate since the switch to strp, can probably be dropped since @tcp has a kdoc entry
quoted hunk ↗ jump to hunk
+ struct { + struct strparser strp; + struct work_struct tx_work; + struct sk_buff_head user_queue; + struct sk_buff_head out_queue; + bool tx_in_progress; + + struct { + struct sk_buff *skb; + int offset; + int len; + } out_msg; + + struct { + void (*sk_data_ready)(struct sock *sk); + void (*sk_write_space)(struct sock *sk); + struct proto *prot; + const struct proto_ops *ops; + } sk_cb; + } tcp;
[...]
quoted hunk ↗ jump to hunk
+static void ovpn_tcp_send_sock_skb(struct ovpn_peer *peer, struct sk_buff *skb) +{ + if (peer->tcp.out_msg.skb) + ovpn_tcp_send_sock(peer); + + if (peer->tcp.out_msg.skb) { + dev_core_stats_rx_dropped_inc(peer->ovpn->dev);
tx_dropped?
quoted hunk ↗ jump to hunk
+ kfree_skb(skb); + return; + } + + peer->tcp.out_msg.skb = skb; + peer->tcp.out_msg.len = skb->len; + peer->tcp.out_msg.offset = 0; + ovpn_tcp_send_sock(peer); +} + +void ovpn_tcp_send_skb(struct ovpn_peer *peer, struct sk_buff *skb) +{ + u16 len = skb->len; + + *(__be16 *)__skb_push(skb, sizeof(u16)) = htons(len); + + bh_lock_sock(peer->sock->sock->sk); + if (sock_owned_by_user(peer->sock->sock->sk)) { + if (skb_queue_len(&peer->tcp.out_queue) >= + READ_ONCE(net_hotdata.max_backlog)) { + dev_core_stats_rx_dropped_inc(peer->ovpn->dev);
tx_dropped?
quoted hunk ↗ jump to hunk
+ kfree_skb(skb); + goto unlock; + } + __skb_queue_tail(&peer->tcp.out_queue, skb); + } else { + ovpn_tcp_send_sock_skb(peer, skb); + } +unlock: + bh_unlock_sock(peer->sock->sock->sk); +}
[...]
quoted hunk ↗ jump to hunk
+static void ovpn_tcp_close(struct sock *sk, long timeout) +{ + struct ovpn_socket *sock; + + rcu_read_lock();
[can't sleep until unlock]
quoted hunk ↗ jump to hunk
+ sock = rcu_dereference_sk_user_data(sk); + + strp_stop(&sock->peer->tcp.strp); + + tcp_close(sk, timeout);
void tcp_close(struct sock *sk, long timeout)
{
lock_sock(sk);
but this can sleep.
Is there anything that prevents delaying tcp_close until after
ovpn_peer_del and rcu_read_unlock?
quoted hunk ↗ jump to hunk
+ ovpn_peer_del(sock->peer, OVPN_DEL_PEER_REASON_TRANSPORT_ERROR); + rcu_read_unlock(); +}
[...]
quoted hunk ↗ jump to hunk
+void __init ovpn_tcp_init(void) +{ + ovpn_tcp_build_protos(&ovpn_tcp_prot, &ovpn_tcp_ops, &tcp_prot, + &inet_stream_ops); + +#if IS_ENABLED(CONFIG_IPV6) + ovpn_tcp_build_protos(&ovpn_tcp6_prot, &ovpn_tcp6_ops, &tcpv6_prot, + &inet6_stream_ops);
I don't think that works for CONFIG_OVPN=y and CONFIG_IPV6=m. You can either go back to the ugly thing espintcp and tls do, or use the traditional Kconfig hack: depends on IPV6 || !IPV6 (you can find it sprinkled in various places of drivers/net/Kconfig and net/) -- Sabrina