> -----Original Message-----
> From: Nikolay Aleksandrov <[email protected]>
> Sent: Monday, 20 July 2026 12:26
> To: Danielle Ratson <[email protected]>; [email protected]
> Cc: [email protected]; Ido Schimmel <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Petr Machata
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH net-next 4/5] bridge: Linearize skb once the ND message
> type is validated
> 
> On 19/07/2026 16:34, Danielle Ratson wrote:
> > br_nd_send() parses ND options from ns->opt[] and therefore needs the
> > skb to be linear. Commit a01aee7cafc5 ("bridge: br_nd_send: linearize
> > skb before parsing ND options") ensured that by linearizing inside
> > br_nd_send() itself.
> >
> > Move the linearization up into br_is_nd_neigh_msg(), right after
> > ndisc_check_ns_na() has validated the message as an NS/NA. This makes
> > a linear buffer a property of every recognized ND message, so that
> > this and any future ND message handling operate on a linear skb and
> > cannot reintroduce that class of bug by forgetting to linearize.
> >
> > Since the skb is now linear by the time br_nd_send() runs, drop the
> > linearization there and derive ns from the transport header set by
> > ndisc_check_ns_na(), instead of recomputing it from the network header.
> >
> > If linearization fails under memory pressure, br_is_nd_neigh_msg()
> > returns NULL and the packet falls back to normal forwarding rather
> > than being suppressed.
> >
> > Reviewed-by: Petr Machata <[email protected]>
> > Signed-off-by: Danielle Ratson <[email protected]>
> > ---
> >   net/bridge/br_arp_nd_proxy.c | 8 +++++---
> >   1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bridge/br_arp_nd_proxy.c
> > b/net/bridge/br_arp_nd_proxy.c index 445c930ed59b..46779d9fad61
> 100644
> > --- a/net/bridge/br_arp_nd_proxy.c
> > +++ b/net/bridge/br_arp_nd_proxy.c
> > @@ -240,6 +240,9 @@ struct nd_msg *br_is_nd_neigh_msg(struct sk_buff
> *skb)
> >     if (ndisc_check_ns_na(skb))
> >             return NULL;
> >
> > +   if (skb_linearize(skb))
> > +           return NULL;
> > +
> >     return (struct nd_msg *)skb_transport_header(skb);
> >   }
> 
> This one is a bit weird - I wouldn't expect the check and validation to also
> linearize the skb. I'd rename this helper to show that it also linearizes the 
> skb.
> Other than that the patch looks good.

I agree it's not obvious from the name. But, I think it might be better to add 
a short comment than rename.
The name captures the intent, while linearizing is just prep for the option 
parsing in br_nd_send().
This also follows ipv6_mc_check_mld(), which is also a "check" that sets the 
transport header and documents its skb side effects in text rather than the 
name.

No other function here has a kdoc, so I'd keep it a plain comment:

/* Validate skb as an NS/NA and linearize it for br_nd_send()'s ND
  * option parsing; returns the nd_msg, or NULL on failure.
  */
  
Would that work, or do you feel strongly about the rename?

> 
> >
> > @@ -259,7 +262,7 @@ static void br_nd_send(struct net_bridge *br, struct
> net_bridge_port *p,
> >     bool dad;
> >     u16 pvid;
> >
> > -   if (!dev || skb_linearize(request))
> > +   if (!dev)
> >             return;
> >
> >     len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) + @@ -276,8
> > +279,7 @@ static void br_nd_send(struct net_bridge *br, struct
> net_bridge_port *p,
> >     skb_set_mac_header(reply, 0);
> >
> >     daddr = eth_hdr(request)->h_source;
> > -   ns = (struct nd_msg *)(skb_network_header(request) +
> > -                          sizeof(struct ipv6hdr));
> > +   ns = (struct nd_msg *)skb_transport_header(request);
> >
> >     /* Do we need option processing ? */
> >     ns_olen = request->len - (skb_network_offset(request) +

Reply via email to