On Wed, May 6, 2020 at 11:46 AM Cong Wang <[email protected]> wrote: > > On Tue, May 5, 2020 at 3:42 PM Jay Vosburgh <[email protected]> > wrote: > > > > Cong Wang <[email protected]> wrote: > > > > >syzbot managed to trigger a recursive NETDEV_FEAT_CHANGE event > > >between bonding master and slave. I managed to find a reproducer > > >for this: > > > > > > ip li set bond0 up > > > ifenslave bond0 eth0 > > > brctl addbr br0 > > > ethtool -K eth0 lro off > > > brctl addif br0 bond0 > > > ip li set br0 up > > > > Presumably this is tied to the LRO feature being special in > > netdev_sync_lower_features (via NETIF_F_UPPER_DISABLES), but why doesn't > > LRO become disabled and stop the recursion once the test > > > > if (!(features & feature) && (lower->features & feature)) { > > > > no longer evalutes to true (in theory)? > > Good point! > > Actually the LRO feature fails to disable: > > [ 62.559537] netdevice: bond0: failed to disable 0x0000000000008000 on eth0! > ... > [ 78.312003] netdevice: eth0: failed to disable LRO! > > It seems we should only skip netdev_update_features() for such case, > like below. Note __netdev_update_features() intentionally returns -1 > for this failure, so I am afraid we just have to live with it.
Oops, I meant netdev_features_change() of course.
