Re: [net-next,v9,15/15] quic: add packet parser base
From: Xin Long <lucien.xin@gmail.com>
Date: 2026-02-05 19:03:11
Also in:
linux-cifs
On Thu, Feb 5, 2026 at 6:55 AM Simon Horman [off-list ref] wrote:
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- quic: add packet parser base This patch uses 'quic_packet' to handle packing of QUIC packets on the receive (RX) path. It introduces mechanisms to parse the ALPN from client Initial packets to determine the correct listener socket, routes received packets accordingly, and adds support for ICMP-based MTU updates by locating the relevant socket and updating the stored PMTU.quoted
diff --git a/net/quic/packet.c b/net/quic/packet.c index cf64a6aaa795..16a7db6ccd87 100644 --- a/net/quic/packet.c +++ b/net/quic/packet.c[ ... ]quoted
@@ -14,6 +14,637 @@ #define QUIC_HLEN 1 +/* Handle ICMP Toobig packet and update QUIC socket path MTU. */ +static int quic_packet_rcv_err(struct sock *sk, struct sk_buff *skb) +{ + union quic_addr daddr, saddr; + 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, sk, NULL);Are the address arguments swapped in quic_packet_rcv_err()? Looking at other call sites in this file, quic_get_msg_addrs is consistently called with &daddr first, then &saddr: Line 476 in quic_packet_get_listen_sock(): quic_get_msg_addrs(skb, &daddr, &saddr); Line 513 in quic_packet_get_sock(): quic_get_msg_addrs(skb, &daddr, &saddr); Line 536 in quic_packet_get_sock(): quic_get_msg_addrs(skb, &daddr, &saddr); But here quic_packet_rcv_err() passes &saddr first, then &daddr. Looking at the implementation of quic_v4_get_msg_addrs() in net/quic/family.c, the function extracts addresses from the packet as: sa->v4.sin_port = uh->source; sa->v4.sin_addr.s_addr = ip_hdr(skb)->saddr; da->v4.sin_port = uh->dest; da->v4.sin_addr.s_addr = ip_hdr(skb)->daddr; So sa gets the packet's source address (remote), and da gets the packet's destination address (local). Then quic_sock_lookup() in net/quic/socket.c expects: if (quic_cmp_sk_addr(tmp, quic_path_saddr(paths, 0), sa) && quic_cmp_sk_addr(tmp, quic_path_daddr(paths, 0), da) && This compares the socket's local address with sa and the socket's remote address with da, meaning sa should be the local (packet dest) address and da should be the remote (packet source) address. With the swapped arguments, quic_packet_rcv_err() would pass the remote address where the local is expected and vice versa, causing the socket lookup to fail. This would prevent ICMP Packet Too Big messages from being processed correctly, breaking Path MTU Discovery.
The code is correct, I may add a comment for the explanation:
/* ICMP embeds the original outgoing QUIC packet, so
saddr/daddr are reversed when
* parsed. Only address-based socket lookup is possible in this case.
*/
Thanks.