On 11/03/2015 12:36 PM, Jarod Wilson wrote:...
With moving netdev_sync_lower_features() after the .ndo_set_features
calls, I neglected to verify that devices added *after* a flag had been
disabled on an upper device were properly added with that flag
disabled as
well. This currently happens, because we exit __netdev_update_features()
when we see dev->features == features for the upper dev. We can retain
the
optimization of leaving without calling .ndo_set_features with a bit of
tweaking and a goto here.
Changing err to ret was somewhat arbitrary and makes the patch look more
involved, but seems to better fit the altered use.
+ if (!ret) {
+ dev->features = features;
+ ret = 1;
+ }
+
I would take the "ret = 1;" out of the if statement and let it stay here
by itself. Technically anything that traversed this path was returning 1
previously so we probably want to retain that behavior.
+sync_lower:
/* some features must be disabled on lower devices when disabled
* on an upper device (think: bonding master or bridge)
*/
netdev_for_each_lower_dev(dev, lower, iter)
netdev_sync_lower_features(dev, lower, features);
- if (!err)
- dev->features = features;
You could just alter the if statement here to check for a non-zero ret
value since you should have it as either 0 or 1. It shouldn't have any
other values.
That way you will have disabled the feature on the lower devices before
advertising that it has been disabled on the upper device.