From: Simon Horman <horms@kernel.org>
To: Alice Mikityanska <alice@isovalent.com>
Cc: alice.kernel@fastmail.im, daniel@iogearbox.net,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, lucien.xin@gmail.com,
willemdebruijn.kernel@gmail.com, willemb@google.com,
dsahern@kernel.org, razor@blackwall.org, shuah@kernel.org,
stfomichev@gmail.com, andrew+netdev@lunn.ch, fw@strlen.de,
netdev@vger.kernel.org
Subject: Re: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide
Date: Fri, 17 Jul 2026 13:40:46 +0100 [thread overview]
Message-ID: <20260717124046.GO95246@horms.kernel.org> (raw)
In-Reply-To: <CAD0BsJWRPNsQrBT06QqCtB5atqjBQz4y8QWvGg-v2WZDqRiPRA@mail.gmail.com>
On Thu, Jul 16, 2026 at 06:18:59PM +0300, Alice Mikityanska wrote:
> On Thu, 16 Jul 2026 at 17:47, Simon Horman <horms@kernel.org> 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.
> > Full review at: https://sashiko.dev
> > ---
> > --- Patch [1]: [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide ---
> > commit 113d28557bcedccc492f330fafdaaf1377eb2e79
> > Author: Alice Mikityanska <alice@isovalent.com>
> >
> > net: Use helpers to get/set UDP len tree-wide
> >
> > This commit introduces helpers for setting and getting the UDP header length
> > tree-wide. It prepares the codebase to support BIG TCP for UDP tunnels by
> > explicitly handling cases where UDP length might be 0 for packets larger
> > than 65535 bytes.
> >
> > > diff --git a/net/6lowpan/nhc_udp.c b/net/6lowpan/nhc_udp.c
> > > index 0a506c77283dd..ed4227e6db744 100644
> > > --- a/net/6lowpan/nhc_udp.c
> > > +++ b/net/6lowpan/nhc_udp.c
> > > @@ -88,16 +88,16 @@ static int udp_uncompress(struct sk_buff *skb, size_t needed)
> > > switch (lowpan_dev(skb->dev)->lltype) {
> > > case LOWPAN_LLTYPE_IEEE802154:
> > > if (lowpan_802154_cb(skb)->d_size)
> > > - uh.len = htons(lowpan_802154_cb(skb)->d_size -
> > > - sizeof(struct ipv6hdr));
> > > + udp_set_len_short(&uh, lowpan_802154_cb(skb)->d_size -
> > > + sizeof(struct ipv6hdr));
> >
> > [Severity: High]
> > If a crafted packet specifies a datagram size smaller than
> > sizeof(struct ipv6hdr), won't this subtraction underflow?
>
> First, I'd like to emphasize that I'm merely converting direct uh.len
> access to the new helper. If there is an existing bug with underflow,
> my new helper will expose it with a WARN, which is good. But should
> such bugs exist before this refactoring, I don't think that fixing all
> of them is in scope of this series.
Yes, point taken.
...
> > [Severity: High]
> > If gro_max_size is configured larger than 65535 (which is supported and used
> > for BIG TCP), can't UDP GRO aggregate packets such that newlen > 65535?
>
> This is the first patch that just converted direct access to helpers.
> Patch 4 "udp: Support gro_ipv4_max_size > 65536" explicitly enables
> len > 64k and replaces this line with udp_set_len. This comment on
> patch 1 is irrelevant because it assumes that functionality added in a
> later patch should work in a prior patch.
Agreed, the AI-generated review seems wide of the mark here.
...
> > [Severity: High]
> > When transmitting a TCP BIG TCP packet (which can be > 64K) over a UDP
> > tunnel like VXLAN, skb->len will exceed 65535.
> >
> > Does using udp_set_len_short() here incorrectly trigger the warning for
> > valid large GSO payloads?
>
> Same here. This is replaced by udp_set_len, which supports len > 64k,
> in the patch that actually enables len > 64k.
>
> > > [ ... ]
> > > diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
> > > index 9adb5775487fa..dcff7fb16ff64 100644
> > > --- a/net/ipv6/ip6_udp_tunnel.c
> > > +++ b/net/ipv6/ip6_udp_tunnel.c
> > > @@ -93,7 +93,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
> > > uh->dest = dst_port;
> > > uh->source = src_port;
> > >
> > > - uh->len = htons(skb->len);
> > > + udp_set_len_short(uh, skb->len);
> >
> > [Severity: High]
> > This appears to have the same issue as IPv4 tunnels. If a large GSO payload
> > is sent through an IPv6 UDP tunnel, won't skb->len exceed 65535 and trigger
> > the warning?
>
> Same here.
>
> I'm sorry, but I highly doubt that these AI review comments were validated.
Clearly my validation was insufficient.
Sorry about that.
next prev parent reply other threads:[~2026-07-17 12:40 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20260715061259epcas5p378f743489493da324fc9092ed2dfe147@epcas5p3.samsung.com>
2026-07-10 13:42 ` [PATCH net-next v9 0/9] BIG TCP for UDP tunnels Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 1/9] net: Use helpers to get/set UDP len tree-wide Alice Mikityanska
2026-07-16 14:46 ` Simon Horman
2026-07-16 15:18 ` Alice Mikityanska
2026-07-17 12:40 ` Simon Horman [this message]
2026-07-10 13:42 ` [PATCH net-next v9 2/9] net: Enable BIG TCP with partial GSO Alice Mikityanska
2026-07-16 14:47 ` Simon Horman
2026-07-16 16:28 ` Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 3/9] udp: Support BIG TCP GSO packets where they can occur Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 4/9] udp: Support gro_ipv4_max_size > 65536 Alice Mikityanska
2026-07-16 14:47 ` Simon Horman
2026-07-16 17:04 ` Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 5/9] udp: Validate UDP length in udp_gro_receive Alice Mikityanska
2026-07-16 14:48 ` Simon Horman
2026-07-16 18:32 ` Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 6/9] udp: Set length in UDP header to 0 for big GSO packets Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 7/9] vxlan: Enable BIG TCP packets Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 8/9] geneve: " Alice Mikityanska
2026-07-16 14:48 ` Simon Horman
2026-07-16 18:45 ` Alice Mikityanska
2026-07-10 13:42 ` [PATCH net-next v9 9/9] selftests: net: Add a test for BIG TCP in UDP tunnels Alice Mikityanska
2026-07-11 9:47 ` [PATCH net-next v9 0/9] BIG TCP for " Nikolay Aleksandrov
2026-07-15 6:11 ` zebang.li
[not found] ` <(raw)>
2026-07-15 9:06 ` Alice Mikityanska
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=20260717124046.GO95246@horms.kernel.org \
--to=horms@kernel.org \
--cc=alice.kernel@fastmail.im \
--cc=alice@isovalent.com \
--cc=andrew+netdev@lunn.ch \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=kuba@kernel.org \
--cc=lucien.xin@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
--cc=shuah@kernel.org \
--cc=stfomichev@gmail.com \
--cc=willemb@google.com \
--cc=willemdebruijn.kernel@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