From: Xin Long <lucien.xin@gmail.com>
To: Paolo Abeni <pabeni@redhat.com>
Cc: network dev <netdev@vger.kernel.org>,
quic@lists.linux.dev, davem@davemloft.net, kuba@kernel.org,
Eric Dumazet <edumazet@google.com>,
Simon Horman <horms@kernel.org>,
Stefan Metzmacher <metze@samba.org>,
Moritz Buhl <mbuhl@openbsd.org>,
Tyler Fanelli <tfanelli@redhat.com>,
Pengtao He <hepengtao@xiaomi.com>,
Thomas Dreibholz <dreibh@simula.no>,
linux-cifs@vger.kernel.org, Steve French <smfrench@gmail.com>,
Namjae Jeon <linkinjeon@kernel.org>,
Paulo Alcantara <pc@manguebit.com>, Tom Talpey <tom@talpey.com>,
kernel-tls-handshake@lists.linux.dev,
Chuck Lever <chuck.lever@oracle.com>,
Jeff Layton <jlayton@kernel.org>,
Steve Dickson <steved@redhat.com>,
Hannes Reinecke <hare@suse.de>,
Alexander Aring <aahringo@redhat.com>,
David Howells <dhowells@redhat.com>,
Matthieu Baerts <matttbe@kernel.org>,
John Ericson <mail@johnericson.me>,
Cong Wang <xiyou.wangcong@gmail.com>,
"D . Wythe" <alibuda@linux.alibaba.com>,
Jason Baron <jbaron@akamai.com>,
illiliti <illiliti@protonmail.com>,
Sabrina Dubroca <sd@queasysnail.net>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
Daniel Stenberg <daniel@haxx.se>,
Andy Gospodarek <andrew.gospodarek@broadcom.com>
Subject: Re: [PATCH net-next v4 15/15] quic: add packet builder and parser base
Date: Thu, 6 Nov 2025 14:24:21 -0500 [thread overview]
Message-ID: <CADvbK_evd2=Cs5EZGf3EVBiY5SvF_aOtbu6wMjj_mSgFgfBpzw@mail.gmail.com> (raw)
In-Reply-To: <6dfd2fe8-65b6-40db-b0f2-34aa0e4f3e9b@redhat.com>
On Tue, Nov 4, 2025 at 9:44 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 10/29/25 3:35 PM, Xin Long wrote:
> > +/* Process PMTU reduction event on a QUIC socket. */
> > +void quic_packet_rcv_err_pmtu(struct sock *sk)
> > +{
> > + struct quic_path_group *paths = quic_paths(sk);
> > + struct quic_packet *packet = quic_packet(sk);
> > + struct quic_config *c = quic_config(sk);
> > + u32 pathmtu, info, taglen;
> > + struct dst_entry *dst;
> > + bool reset_timer;
> > +
> > + if (!ip_sk_accept_pmtu(sk))
> > + return;
> > +
> > + info = clamp(paths->mtu_info, QUIC_PATH_MIN_PMTU, QUIC_PATH_MAX_PMTU);
> > + /* If PLPMTUD is not enabled, update MSS using the route and ICMP info. */
> > + if (!c->plpmtud_probe_interval) {
> > + if (quic_packet_route(sk) < 0)
> > + return;
> > +
> > + dst = __sk_dst_get(sk);
> > + dst->ops->update_pmtu(dst, sk, NULL, info, true);
> > + quic_packet_mss_update(sk, info - packet->hlen);
> > + return;
> > + }
> > + /* PLPMTUD is enabled: adjust to smaller PMTU, subtract headers and AEAD tag. Also
> > + * notify the QUIC path layer for possible state changes and probing.
> > + */
> > + taglen = quic_packet_taglen(packet);
> > + info = info - packet->hlen - taglen;
> > + pathmtu = quic_path_pl_toobig(paths, info, &reset_timer);
> > + if (reset_timer)
> > + quic_timer_reset(sk, QUIC_TIMER_PMTU, c->plpmtud_probe_interval);
> > + if (pathmtu)
> > + quic_packet_mss_update(sk, pathmtu + taglen);
> > +}
> > +
> > +/* Handle ICMP Toobig packet and update QUIC socket path MTU. */
> > +static int quic_packet_rcv_err(struct sk_buff *skb)
> > +{
> > + union quic_addr daddr, saddr;
> > + struct sock *sk = NULL;
> > + int ret = 0;
> > + u32 info;
> > +
> > + /* All we can do is lookup the matching QUIC socket by addresses. */
> > + quic_get_msg_addrs(skb, &saddr, &daddr);
> > + sk = quic_sock_lookup(skb, &daddr, &saddr, NULL);
> > + if (!sk)
> > + return -ENOENT;
> > +
> > + bh_lock_sock(sk);
> > + if (quic_is_listen(sk))
>
> The above looks race-prone. You should check the status only when
> holding the sk socket lock, i.e. if !sock_owned_by_user(sk)
This check can be deleted now, as quic_sock_lookup()
has only returned regular socket since I moved listen socket to
another hashtable.
>
> > + goto out;
> > +
> > + if (quic_get_mtu_info(skb, &info))
> > + goto out;
>
> This can be moved outside the lock.
>
Right.
> > +
> > + ret = 1; /* Success: update socket path MTU info. */
> > + quic_paths(sk)->mtu_info = info;
> > + if (sock_owned_by_user(sk)) {
> > + /* Socket is in use by userspace context. Defer MTU processing to later via
> > + * tasklet. Ensure the socket is not dropped before deferral.
> > + */
> > + if (!test_and_set_bit(QUIC_MTU_REDUCED_DEFERRED, &sk->sk_tsq_flags))
> > + sock_hold(sk);
> > + goto out;
> > + }
> > + /* Otherwise, process the MTU reduction now. */
> > + quic_packet_rcv_err_pmtu(sk);
> > +out:
> > + bh_unlock_sock(sk);
> > + sock_put(sk);
> > + return ret;
> > +}
> > +
> > +#define QUIC_PACKET_BACKLOG_MAX 4096
> > +
> > +/* Queue a packet for later processing when sleeping is allowed. */
> > +static int quic_packet_backlog_schedule(struct net *net, struct sk_buff *skb)
> > +{
> > + struct quic_skb_cb *cb = QUIC_SKB_CB(skb);
> > + struct quic_net *qn = quic_net(net);
> > +
> > + if (cb->backlog)
> > + return 0;
>
> The above test is present also in the only caller of this function. It
> should be removed from there.
I may delete the one from the caller, as there will be other
callers in the 2nd patchset need it to be checked in here.
>
> [...]> +/* Work function to process packets in the backlog queue. */
> > +void quic_packet_backlog_work(struct work_struct *work)
> > +{
> > + struct quic_net *qn = container_of(work, struct quic_net, work);
> > + struct sk_buff *skb;
> > + struct sock *sk;
> > +
> > + skb = skb_dequeue(&qn->backlog_list);
> > + while (skb) {
> > + sk = quic_packet_get_listen_sock(skb);
> > + if (!sk)
> > + continue;
> > +
> > + lock_sock(sk);
>
> Possibly lock_sock_fast(sk);
These are some control QUIC packets (not on the main data path) for
which we need to generate keys from header fields in order to process
them.
However, the kernel crypto API cannot install a key into a tfm in
atomic context, so we must use a workqueue to handle key generation,
installation, and then decryption.
lock_sock_fast() can not be used here, otherwise this path runs in
atomic context again.
>
> > + quic_packet_process(sk, skb);
> > + release_sock(sk);
> > + sock_put(sk);
> > +
> > + skb = skb_dequeue(&qn->backlog_list);
> > + }
> > +}
>
> [...]> +/* Create and transmit a new QUIC packet. */
> > +int quic_packet_create(struct sock *sk)
> Possibly rename the function accordingly to its actual action, i.e.
> quic_packet_create_xmit()
I will use quic_packet_create_and_xmit().
>
> [...]> @@ -291,6 +294,8 @@ static void __net_exit quic_net_exit(struct
> net *net)
> > #ifdef CONFIG_PROC_FS
> > quic_net_proc_exit(net);
> > #endif
> > + skb_queue_purge(&qn->backlog_list);
> > + cancel_work_sync(&qn->work);
>
> Likely: disable_work_sync()
>
Will update it.
> > quic_crypto_free(&qn->crypto);
> > free_percpu(qn->stat);
> > qn->stat = NULL;
>
> EPATCHISTOOBIG, very hard to process. Please split this one it at least
> 2 (i.e. rx and tx part), even if the series will grow above 15
>
Sure, I will do it.
BTW, about the MAINTAINERS entry Jakub mentioned, should I
create a new patch for it, or append it in one of these patches, like into:
[PATCH net-next 02/15] net: build socket infrastructure for QUIC protocol
Thanks.
next prev parent reply other threads:[~2025-11-06 19:24 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-29 14:35 [PATCH net-next v4 00/15] net: introduce QUIC infrastructure and core subcomponents Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 01/15] net: define IPPROTO_QUIC and SOL_QUIC constants Xin Long
2025-11-04 9:20 ` Paolo Abeni
2025-10-29 14:35 ` [PATCH net-next v4 02/15] net: build socket infrastructure for QUIC protocol Xin Long
2025-10-29 16:22 ` Stefan Metzmacher
2025-10-29 19:57 ` Xin Long
2025-10-30 11:29 ` Stefan Metzmacher
2025-10-30 14:13 ` Xin Long
2025-10-30 14:17 ` Stefan Metzmacher
2025-10-30 14:28 ` Xin Long
2025-11-04 9:38 ` Paolo Abeni
2025-11-05 22:20 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 03/15] quic: provide common utilities and data structures Xin Long
2025-11-04 9:55 ` Paolo Abeni
2025-10-29 14:35 ` [PATCH net-next v4 04/15] quic: provide family ops for address and protocol Xin Long
2025-11-04 10:27 ` Paolo Abeni
2025-11-06 1:01 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 05/15] quic: provide quic.h header files for kernel and userspace Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 06/15] quic: add stream management Xin Long
2025-11-04 11:05 ` Paolo Abeni
2025-11-06 1:27 ` Xin Long
2025-11-06 8:51 ` Paolo Abeni
2025-11-06 16:22 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 07/15] quic: add connection id management Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 08/15] quic: add path management Xin Long
2025-11-04 11:50 ` Paolo Abeni
2025-11-06 1:28 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 09/15] quic: add congestion control Xin Long
2025-11-04 12:02 ` Paolo Abeni
2025-11-06 20:24 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 10/15] quic: add packet number space Xin Long
2025-11-04 12:17 ` Paolo Abeni
2025-11-06 16:40 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 11/15] quic: add crypto key derivation and installation Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 12/15] quic: add crypto packet encryption and decryption Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 13/15] quic: add timer management Xin Long
2025-11-04 12:33 ` Paolo Abeni
2025-11-06 16:49 ` Xin Long
2025-11-13 21:23 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 14/15] quic: add frame encoder and decoder base Xin Long
2025-11-04 12:47 ` Paolo Abeni
2025-11-06 17:22 ` Xin Long
2025-11-13 21:26 ` Xin Long
2025-10-29 14:35 ` [PATCH net-next v4 15/15] quic: add packet builder and parser base Xin Long
2025-11-04 14:44 ` Paolo Abeni
2025-11-06 19:24 ` Xin Long [this message]
2025-11-04 2:41 ` [PATCH net-next v4 00/15] net: introduce QUIC infrastructure and core subcomponents Jakub Kicinski
2025-11-05 22:19 ` Xin Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CADvbK_evd2=Cs5EZGf3EVBiY5SvF_aOtbu6wMjj_mSgFgfBpzw@mail.gmail.com' \
--to=lucien.xin@gmail.com \
--cc=aahringo@redhat.com \
--cc=alibuda@linux.alibaba.com \
--cc=andrew.gospodarek@broadcom.com \
--cc=chuck.lever@oracle.com \
--cc=daniel@haxx.se \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=dreibh@simula.no \
--cc=edumazet@google.com \
--cc=hare@suse.de \
--cc=hepengtao@xiaomi.com \
--cc=horms@kernel.org \
--cc=illiliti@protonmail.com \
--cc=jbaron@akamai.com \
--cc=jlayton@kernel.org \
--cc=kernel-tls-handshake@lists.linux.dev \
--cc=kuba@kernel.org \
--cc=linkinjeon@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=mail@johnericson.me \
--cc=marcelo.leitner@gmail.com \
--cc=matttbe@kernel.org \
--cc=mbuhl@openbsd.org \
--cc=metze@samba.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pc@manguebit.com \
--cc=quic@lists.linux.dev \
--cc=sd@queasysnail.net \
--cc=smfrench@gmail.com \
--cc=steved@redhat.com \
--cc=tfanelli@redhat.com \
--cc=tom@talpey.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox