On Wed, 6 May 2026 08:31:01 +0000 Danielle Ratson wrote:
> > > I think this should be u8 ? neigh-vlan-suppress looks buggy too  
> > 
> > I pointed this out during internal review, but assumed I am missing 
> > something
> > since almost all the attributes use flag when they are in fact u8. We can 
> > fix  
> 
> This is in fact the reason why I also changed it myself to use flag before 
> sending.
> 
> > neigh-forward-grat to use u8 in v2 and change the rest in net. To be clear, 
> > I
> > believe the following should be converted from flag to u8:
> > 
> > mode, guard, protect, fast-leave, learning, unicast-flood, proxyarp, 
> > learning-
> > sync, proxyarp-wifi, mcast-flood, mcast-to-ucast, vlan-tunnel, bcast-flood,
> > neigh-suppress, isolated, mrp-ring-open, mrp-in-open, locked, mab, neigh-
> > vlan-suppress
> 
> So should we proceed as Ido suggested?

I think so. Tweak this patch and send a separate fix to net for the
existing attrs.

> > > flag is a type without a payload, the presence of the attr is the
> > > entire information
> > >
> > > None of the AIs seem to catch this, I think you may have over-split
> > > this submission a little bit. This patch may have been better off
> > > squashed into patch 4 ?  
> 
> It seems like the patch has enough content, but I can squash. I guess
> ill split the commit between patches 4 and 5 accordingly.

Could be my lack of familiarity with bridge but FWIW reading the
submission it felt like it was split by "area of code" rather than
logical steps. Splitting work into patches is a bit of an art..

> > Related: The AI also did not catch that the spec was missing (easy
> > to forget for rtnetlink). Do you think it's worth adding to
> > review-prompts?  

I assumed Sashiko missed this because it doesn't spent too much time on
the series as a whole right now (I was going to tweak that but looks
like Claude is having another meltdown, crazy inference latency,
patches backlogging for review..)

Are you saying that the review prompts-based test is also not catching
this even if you give it the range of commits that form the series?

Reply via email to