On 19/10/17 18:54, Nikolay Aleksandrov wrote: > Hi, > Before this set the bridge would generate a notification on vlan add or del > even if they didn't actually do any changes, which confuses listeners and > is generally not preferred. We could also lose notifications on actual > changes if one adds a range of vlans and there's an error in the middle. > The problem with just breaking and returning an error is that we could > break existing user-space scripts which rely on the vlan delete to clear > all existing entries in the specified range and ignore the non-existing > errors (typically used to clear the current vlan config). > So in order to make the notifications more accurate while keeping backwards > compatibility we add a boolean that tracks if anything actually changed > during the config calls. > > The vlan add is more difficult to fix because it always returns 0 even if > nothing changed, so we use EEXIST to signal that and in order not to break > overlapping vlan range add or script return expectations we clear it on > return, the functions used by vlan add are not expected to return EEXIST. > > Thanks, > Nik > > Nikolay Aleksandrov (2): > bridge: netlink: make setlink/dellink notifications more accurate > bridge: vlan: return EEXIST on add if nothing changed > > net/bridge/br_netlink.c | 49 ++++++++++++++++++++++++++---------------- > net/bridge/br_netlink_tunnel.c | 14 +++++++----- > net/bridge/br_private_tunnel.h | 3 ++- > net/bridge/br_vlan.c | 38 +++++++++++++++++++++----------- > 4 files changed, 68 insertions(+), 36 deletions(-) > Self-NAK Dave, please ignore this set and apologies for the noise. I'd actually prefer to send "changed" down to vlan_add and vlan_del to be set there instead of overloading EEXIST like that. It will be safer. I'll wait for some comments about the rest of the change and will send a v2. Thanks, Nik