Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
❦ 21 septembre 2017 08:09 -0700, Roopa Prabhu : >>> The one concern is that ports added or removed through ioctl should >>> cause same events as doing the same thing via netlink. Some users use >>> brctl (ioctl) and others use newer bridge (netlink) API. >> >> I'll make a third iteration to have the same notifications when using >> ioctl() with details in the commit message. >> -- > > as long as the ioctl path for bridge port removal is generating a: > RTM_DELLINK with AF_BRIDGE and > RTM_NEWLINK with AF_UNSPEC with 'master' removed > > we should be good. These are the most important ones. > > are there other specific bridge-events missing ?. you might want to > run `bridge monitor link` also. and un-slaving of a port also triggers > fdb events which are visible under `bridge monitor fdb` With the patch, bridge monitor link generates the same event with ioctl() than with netlink (like for ip monitor link, netlink generates one extra duplicate event when enslaving). For bridge monitor fdb, there is a difference. With ioctl(), I don't get the event for VLAN1: Deleted ca:18:06:bc:f6:11 dev dummy1 vlan 1 master bridge0 permanent I suppose this is an expected difference due to the inability to manage VLAN-aware bridges with ioctl(). -- Use the fundamental control flow constructs. - The Elements of Programming Style (Kernighan & Plauger)
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
On Thu, Sep 21, 2017 at 3:04 AM, Vincent Bernat wrote: > ❦ 20 septembre 2017 16:21 -0700, Stephen Hemminger > : > >> The one concern is that ports added or removed through ioctl should >> cause same events as doing the same thing via netlink. Some users use >> brctl (ioctl) and others use newer bridge (netlink) API. > > I'll make a third iteration to have the same notifications when using > ioctl() with details in the commit message. > -- as long as the ioctl path for bridge port removal is generating a: RTM_DELLINK with AF_BRIDGE and RTM_NEWLINK with AF_UNSPEC with 'master' removed we should be good. These are the most important ones. are there other specific bridge-events missing ?. you might want to run `bridge monitor link` also. and un-slaving of a port also triggers fdb events which are visible under `bridge monitor fdb`
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
❦ 20 septembre 2017 16:21 -0700, Stephen Hemminger : > The one concern is that ports added or removed through ioctl should > cause same events as doing the same thing via netlink. Some users use > brctl (ioctl) and others use newer bridge (netlink) API. I'll make a third iteration to have the same notifications when using ioctl() with details in the commit message. -- When in doubt, tell the truth. -- Mark Twain
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
On Wed, 20 Sep 2017 15:57:16 -0600 David Ahern wrote: > On 9/20/17 3:09 PM, David Miller wrote: > > From: Vincent Bernat > > Date: Sat, 16 Sep 2017 16:18:33 +0200 > > > > David, I am CC:'ing you because you've done work in this area over the > > past year. I'm applying this patch, it's been sitting since the 16th > > and likes entirely correct to me. But if you have objections just let > > me know. > > > >> Currently, when an interface is released from a bridge via > >> ioctl(), we get a RTM_DELLINK event through netlink: > >> > >> Deleted 2: dummy0: mtu 1500 master bridge0 > >> state UNKNOWN > >> link/ether 6e:23:c2:54:3a:b3 > >> > >> Userspace has to interpret that as a removal from the bridge, not as a > >> complete removal of the interface. When an bridged interface is > >> completely removed, we get two events: > >> > >> Deleted 2: dummy0: mtu 1500 master bridge0 state DOWN > >> link/ether 6e:23:c2:54:3a:b3 > >> Deleted 2: dummy0: mtu 1500 qdisc noop state DOWN group > >> default > >> link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff > >> > >> In constrast, when an interface is released from a bond, we get a > >> RTM_NEWLINK with only the new characteristics (no master): > >> > >> 3: dummy1: mtu 1500 qdisc noqueue > >> master bond0 state UNKNOWN group default > >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > >> 3: dummy1: mtu 1500 qdisc noqueue state > >> UNKNOWN group default > >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > >> 4: bond0: mtu 1500 qdisc noqueue > >> state UP group default > >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > >> 3: dummy1: mtu 1500 qdisc noqueue state DOWN group > >> default > >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > >> 3: dummy1: mtu 1500 qdisc noqueue state DOWN group > >> default > >> link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff > >> 4: bond0: mtu 1500 qdisc noqueue > >> state UP group default > >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > >> > >> Userland may be confused by the fact we say a link is deleted while > >> its characteristics are only modified. A first solution would have > >> been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK > >> event. However, maybe some piece of userland is relying on this > >> RTM_DELLINK to detect when a bridged interface is released. Instead, > >> we also emit a RTM_NEWLINK event once the interface is > >> released (without master info). > >> > >> Deleted 2: dummy0: mtu 1500 master bridge0 > >> state UNKNOWN > >> link/ether 8a:bb:e7:94:b1:f8 > >> 2: dummy0: mtu 1500 qdisc noqueue state > >> UNKNOWN group default > >> link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff > >> > >> This is done only when using ioctl(). When using Netlink, such an > >> event is already automatically emitted in do_setlink(). > > The DELLINK is for AF_BRIDGE family (ifi_family). Adding family to > print_linkinfo and running the monitor I get: > > > [LINK]family 0: 35: dummy1: mtu 1500 qdisc > noqueue master br0 state UNKNOWN group default > link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff > > [LINK]family 7: 35: dummy1: mtu 1500 > master br0 state UNKNOWN > link/ether d6:c3:73:86:3c:73 > > [LINK]Deleted family 7: 35: dummy1: mtu > 1500 master br0 state UNKNOWN > link/ether d6:c3:73:86:3c:73 > > [LINK]family 0: 35: dummy1: mtu 1500 qdisc > noqueue state UNKNOWN group default > link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff > > And that seems correct. So I think the RTM_NEWLINK added by this patch > should not be needed. Agreed, thanks for tracing this. The one concern is that ports added or removed through ioctl should cause same events as doing the same thing via netlink. Some users use brctl (ioctl) and others use newer bridge (netlink) API.
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat Date: Thu, 21 Sep 2017 00:12:53 +0200 > ❦ 20 septembre 2017 15:57 -0600, David Ahern : > >> The DELLINK is for AF_BRIDGE family (ifi_family). Adding family to >> print_linkinfo and running the monitor I get: >> >> >> [LINK]family 0: 35: dummy1: mtu 1500 qdisc >> noqueue master br0 state UNKNOWN group default >> link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff >> >> [LINK]family 7: 35: dummy1: mtu 1500 >> master br0 state UNKNOWN >> link/ether d6:c3:73:86:3c:73 >> >> [LINK]Deleted family 7: 35: dummy1: mtu >> 1500 master br0 state UNKNOWN >> link/ether d6:c3:73:86:3c:73 > > I didn't know about the family. We can drop the patch. Ok, I've reverted. Thanks.
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
❦ 20 septembre 2017 15:57 -0600, David Ahern : > The DELLINK is for AF_BRIDGE family (ifi_family). Adding family to > print_linkinfo and running the monitor I get: > > > [LINK]family 0: 35: dummy1: mtu 1500 qdisc > noqueue master br0 state UNKNOWN group default > link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff > > [LINK]family 7: 35: dummy1: mtu 1500 > master br0 state UNKNOWN > link/ether d6:c3:73:86:3c:73 > > [LINK]Deleted family 7: 35: dummy1: mtu > 1500 master br0 state UNKNOWN > link/ether d6:c3:73:86:3c:73 I didn't know about the family. We can drop the patch. -- One of the most striking differences between a cat and a lie is that a cat has only nine lives. -- Mark Twain, "Pudd'nhead Wilson's Calendar"
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
On 9/20/17 3:09 PM, David Miller wrote: > From: Vincent Bernat > Date: Sat, 16 Sep 2017 16:18:33 +0200 > > David, I am CC:'ing you because you've done work in this area over the > past year. I'm applying this patch, it's been sitting since the 16th > and likes entirely correct to me. But if you have objections just let > me know. > >> Currently, when an interface is released from a bridge via >> ioctl(), we get a RTM_DELLINK event through netlink: >> >> Deleted 2: dummy0: mtu 1500 master bridge0 >> state UNKNOWN >> link/ether 6e:23:c2:54:3a:b3 >> >> Userspace has to interpret that as a removal from the bridge, not as a >> complete removal of the interface. When an bridged interface is >> completely removed, we get two events: >> >> Deleted 2: dummy0: mtu 1500 master bridge0 state DOWN >> link/ether 6e:23:c2:54:3a:b3 >> Deleted 2: dummy0: mtu 1500 qdisc noop state DOWN group >> default >> link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff >> >> In constrast, when an interface is released from a bond, we get a >> RTM_NEWLINK with only the new characteristics (no master): >> >> 3: dummy1: mtu 1500 qdisc noqueue master >> bond0 state UNKNOWN group default >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff >> 3: dummy1: mtu 1500 qdisc noqueue state >> UNKNOWN group default >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff >> 4: bond0: mtu 1500 qdisc noqueue >> state UP group default >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff >> 3: dummy1: mtu 1500 qdisc noqueue state DOWN group default >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff >> 3: dummy1: mtu 1500 qdisc noqueue state DOWN group default >> link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff >> 4: bond0: mtu 1500 qdisc noqueue >> state UP group default >> link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff >> >> Userland may be confused by the fact we say a link is deleted while >> its characteristics are only modified. A first solution would have >> been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK >> event. However, maybe some piece of userland is relying on this >> RTM_DELLINK to detect when a bridged interface is released. Instead, >> we also emit a RTM_NEWLINK event once the interface is >> released (without master info). >> >> Deleted 2: dummy0: mtu 1500 master bridge0 >> state UNKNOWN >> link/ether 8a:bb:e7:94:b1:f8 >> 2: dummy0: mtu 1500 qdisc noqueue state >> UNKNOWN group default >> link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff >> >> This is done only when using ioctl(). When using Netlink, such an >> event is already automatically emitted in do_setlink(). The DELLINK is for AF_BRIDGE family (ifi_family). Adding family to print_linkinfo and running the monitor I get: [LINK]family 0: 35: dummy1: mtu 1500 qdisc noqueue master br0 state UNKNOWN group default link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff [LINK]family 7: 35: dummy1: mtu 1500 master br0 state UNKNOWN link/ether d6:c3:73:86:3c:73 [LINK]Deleted family 7: 35: dummy1: mtu 1500 master br0 state UNKNOWN link/ether d6:c3:73:86:3c:73 [LINK]family 0: 35: dummy1: mtu 1500 qdisc noqueue state UNKNOWN group default link/ether d6:c3:73:86:3c:73 brd ff:ff:ff:ff:ff:ff And that seems correct. So I think the RTM_NEWLINK added by this patch should not be needed.
Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat Date: Sat, 16 Sep 2017 16:18:33 +0200 David, I am CC:'ing you because you've done work in this area over the past year. I'm applying this patch, it's been sitting since the 16th and likes entirely correct to me. But if you have objections just let me know. > Currently, when an interface is released from a bridge via > ioctl(), we get a RTM_DELLINK event through netlink: > > Deleted 2: dummy0: mtu 1500 master bridge0 > state UNKNOWN > link/ether 6e:23:c2:54:3a:b3 > > Userspace has to interpret that as a removal from the bridge, not as a > complete removal of the interface. When an bridged interface is > completely removed, we get two events: > > Deleted 2: dummy0: mtu 1500 master bridge0 state DOWN > link/ether 6e:23:c2:54:3a:b3 > Deleted 2: dummy0: mtu 1500 qdisc noop state DOWN group > default > link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff > > In constrast, when an interface is released from a bond, we get a > RTM_NEWLINK with only the new characteristics (no master): > > 3: dummy1: mtu 1500 qdisc noqueue master > bond0 state UNKNOWN group default > link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > 3: dummy1: mtu 1500 qdisc noqueue state UNKNOWN > group default > link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > 4: bond0: mtu 1500 qdisc noqueue > state UP group default > link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > 3: dummy1: mtu 1500 qdisc noqueue state DOWN group default > link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > 3: dummy1: mtu 1500 qdisc noqueue state DOWN group default > link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff > 4: bond0: mtu 1500 qdisc noqueue > state UP group default > link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff > > Userland may be confused by the fact we say a link is deleted while > its characteristics are only modified. A first solution would have > been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK > event. However, maybe some piece of userland is relying on this > RTM_DELLINK to detect when a bridged interface is released. Instead, > we also emit a RTM_NEWLINK event once the interface is > released (without master info). > > Deleted 2: dummy0: mtu 1500 master bridge0 > state UNKNOWN > link/ether 8a:bb:e7:94:b1:f8 > 2: dummy0: mtu 1500 qdisc noqueue state UNKNOWN > group default > link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff > > This is done only when using ioctl(). When using Netlink, such an > event is already automatically emitted in do_setlink(). > > Signed-off-by: Vincent Bernat Applied, thank you.