Re: [PATCH net 1/2] bonding: reject frames with insufficient headroom in bond_header_create

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Aug 27, 2026 at 05:37:09PM -0400, Willem de Bruijn wrote:
> Qihang wrote:
> > Hi Willem,
> > 
> > I'll resend this as v2 with a cover letter.
> > 
> > On scope: I grepped every header_ops->create.  Five devices delegate to a
> > lower device (bond, team, macvlan, ipvlan, 6lowpan), but only bond and
> > team re-select that lower device under RCU -- the rest bind it at netdev
> > creation.  So bond and team are the full scope.
> > 
> > Re Fixes: I used 950803f because it introduced bond_header_create (where
> > this check lives); before it the type-confusion BUG masked this race.
> > Happy to point at 1284cd3a2b74 instead if you prefer.
> > 
> > On the fix location, I'd rather get your read before writing more.  The
> > options I see:
> > 
> >   1. per-wrapper headroom reject in bond/team only -- small, safe,
> >      backport-friendly, but not generic (the next stacked device needs
> >      its own).
> > 
> >   2. central check in dev_hard_header on dev->hard_header_len -- one
> >      place, but I think it's unsafe for bond: bond_dev->hard_header_len
> >      only follows the first slave (bond_setup_by_slave),
> >      bond_change_active_slave flips curr_active_slave without touching
> >      it, and same-type slaves can have different hard_header_len (two GRE
> >      tunnels).  So it can under-reject.  Would need bond to maintain
> >      hard_header_len differently first.
> > 
> >   3. resolve the effective {dev, ops, hard_header_len} as one RCU triple
> >      via a generic callback, so the caller never cares which subordinate
> >      the master picks.  Truly generic and race-free, but it's a new ndo --
> >      net-next material, awkward for stable backport.
> > 
> >   4. make header_ops->create() take an explicit headroom contract and
> >      fail cleanly instead of pushing blind -- cleanest long-term, but
> >      touches every create() in the tree.  Too heavy for now.
> > 
> > My v1 is option 1 (per-wrapper).  Want one of these, a combination,
> > or something else entirely?
> 
> Thanks for the analysis.
> 
> I agree that 3 nd 4 are too invasive for the scope of the bug.
> 
> It's a bit odd that bond and team just take the hard_header_len of the
> first slave. And that bond_create_header just passes to

Actually, bond will use the max hard_header_len of all slaves, see

netdev_compute_master_upper_features()


> curr_active_slave, while various bond modes like LAG will have
> multiple concurrently active slaves.
> 
> That indicates that the intent is for all slaves to have the same
> header length and header_ops->create callback.

And here seem you want to all saves also sync the header length?

I'm not sure if we should/could do this in the same function
netdev_compute_master_upper_features().

Thanks
Hangbin