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 v7 08/16] quic: add path management
Date: Tue, 20 Jan 2026 10:34:32 -0500 [thread overview]
Message-ID: <CADvbK_dYbMa-nVi_8M-XS1QcVUw25t4CZdvcq_HcACx38bH86g@mail.gmail.com> (raw)
In-Reply-To: <71705484-46fc-469f-9357-07a076ee0e73@redhat.com>
On Tue, Jan 20, 2026 at 9:13 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 1/15/26 4:11 PM, Xin Long wrote:
> > @@ -0,0 +1,524 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/* QUIC kernel implementation
> > + * (C) Copyright Red Hat Corp. 2023
> > + *
> > + * This file is part of the QUIC kernel implementation
> > + *
> > + * Initialization/cleanup for QUIC protocol support.
> > + *
> > + * Written or modified by:
> > + * Xin Long <lucien.xin@gmail.com>
> > + */
> > +
> > +#include <net/udp_tunnel.h>
> > +#include <linux/quic.h>
> > +
> > +#include "common.h"
> > +#include "family.h"
> > +#include "path.h"
> > +
> > +static int (*quic_path_rcv)(struct sock *sk, struct sk_buff *skb, u8 err);
>
> It's unclear why an indirect call is needed here. At least some
> explanation is needed in the commit message, possibly you could call
> directly a static function.
>
This patchset makes the path subcomponent more independent from the core
implementation. Aside from a few shared helper functions, it doesn't
rely on code outside the subcomponent, in particular socket.c and
packet.c.
Other subcomponents, such as stream, connid, pnspace, cong,
crypto, common, and family follow the same approach. They expose
APIs to the core QUIC logic and don’t see the main process.
Will leave an explanation here.
> > +
> > +static int quic_udp_rcv(struct sock *sk, struct sk_buff *skb)
> > +{
> > + memset(skb->cb, 0, sizeof(skb->cb));
> > + QUIC_SKB_CB(skb)->seqno = -1;
> > + QUIC_SKB_CB(skb)->time = quic_ktime_get_us();
> > +
> > + skb_pull(skb, sizeof(struct udphdr));
> > + skb_dst_force(skb);
> > + quic_path_rcv(sk, skb, 0);
> > + return 0;
>
> Why not:
> return quic_path_rcv(sk, skb, 0);
> ?
I checked the udp tunnel users:
- bareudp: bareudp_udp_encap_recv()
- vxlan: vxlan_rcv()
- geneve: geneve_udp_encap_recv()
- tipc: tipc_udp_recv()
- sctp: sctp_udp_rcv()
they all are returning 0 in .encap_udp(), I think because they all
take care of the
skb freeing in their err path already.
is it safe to return error to UDP stack if the skb is already freed?
Do you know?
>
> > +static struct quic_udp_sock *quic_udp_sock_create(struct sock *sk, union quic_addr *a)
> > +{
> > + struct udp_tunnel_sock_cfg tuncfg = {};
> > + struct udp_port_cfg udp_conf = {};
> > + struct net *net = sock_net(sk);
> > + struct quic_uhash_head *head;
> > + struct quic_udp_sock *us;
> > + struct socket *sock;
> > +
> > + us = kzalloc(sizeof(*us), GFP_KERNEL);
> > + if (!us)
> > + return NULL;
> > +
> > + quic_udp_conf_init(sk, &udp_conf, a);
> > + if (udp_sock_create(net, &udp_conf, &sock)) {
> > + pr_debug("%s: failed to create udp sock\n", __func__);
> > + kfree(us);
> > + return NULL;
> > + }
> > +
> > + tuncfg.encap_type = 1;
> > + tuncfg.encap_rcv = quic_udp_rcv;
> > + tuncfg.encap_err_lookup = quic_udp_err;
> > + setup_udp_tunnel_sock(net, sock, &tuncfg);
> > +
> > + refcount_set(&us->refcnt, 1);
> > + us->sk = sock->sk;
> > + memcpy(&us->addr, a, sizeof(*a));
> > + us->bind_ifindex = sk->sk_bound_dev_if;
> > +
> > + head = quic_udp_sock_head(net, ntohs(a->v4.sin_port));
> > + hlist_add_head(&us->node, &head->head);
> > + INIT_WORK(&us->work, quic_udp_sock_put_work);
> > +
> > + return us;
> > +}
> > +
> > +static bool quic_udp_sock_get(struct quic_udp_sock *us)
> > +{
> > + return refcount_inc_not_zero(&us->refcnt);
> > +}
> > +
> > +static void quic_udp_sock_put(struct quic_udp_sock *us)
> > +{
> > + if (refcount_dec_and_test(&us->refcnt))
> > + queue_work(quic_wq, &us->work);
>
> Why using a workqueue here? AFAICS all the caller are in process
> context. Is that to break a possible deadlock due to nested mutex?
> Likely a comment on the refcount/locking scheme would help.
>
quic_udp_sock_put() will also be called in the RX path via
quic_path_unbind() for the connection migration (after changing
to a new path.), which is in the patchset-2.
I will leave a comment for an explanation here.
Thanks.
next prev parent reply other threads:[~2026-01-20 15:34 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-15 15:11 [PATCH net-next v7 00/16] net: introduce QUIC infrastructure and core subcomponents Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 01/16] net: define IPPROTO_QUIC and SOL_QUIC constants Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 02/16] net: build socket infrastructure for QUIC protocol Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 03/16] quic: provide common utilities and data structures Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 04/16] quic: provide family ops for address and protocol Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 05/16] quic: provide quic.h header files for kernel and userspace Xin Long
2026-01-20 12:23 ` Paolo Abeni
2026-01-15 15:11 ` [PATCH net-next v7 06/16] quic: add stream management Xin Long
2026-01-20 12:31 ` Paolo Abeni
2026-01-20 14:58 ` Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 07/16] quic: add connection id management Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 08/16] quic: add path management Xin Long
2026-01-20 14:13 ` Paolo Abeni
2026-01-20 15:34 ` Xin Long [this message]
2026-01-20 21:06 ` Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 09/16] quic: add congestion control Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 10/16] quic: add packet number space Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 11/16] quic: add crypto key derivation and installation Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 12/16] quic: add crypto packet encryption and decryption Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 13/16] quic: add timer management Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 14/16] quic: add frame encoder and decoder base Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 15/16] quic: add packet builder base Xin Long
2026-01-20 14:31 ` Paolo Abeni
2026-01-20 16:07 ` Xin Long
2026-01-15 15:11 ` [PATCH net-next v7 16/16] quic: add packet parser base Xin Long
2026-01-16 16:20 ` Stefan Metzmacher
2026-01-16 19:55 ` 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_dYbMa-nVi_8M-XS1QcVUw25t4CZdvcq_HcACx38bH86g@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