* [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
@ 2017-09-15 19:38 Vincent Bernat
2017-09-15 21:08 ` Vincent Bernat
0 siblings, 1 reply; 18+ messages in thread
From: Vincent Bernat @ 2017-09-15 19:38 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge, netdev; +Cc: Vincent Bernat
Currently, when an interface is released from a bridge, we get a
RTM_DELLINK event through netlink:
Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
link/ether 6e:23:c2:54:3a:b3
Deleted 2: dummy0: <BROADCAST,NOARP> 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: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 8a:bb:e7:94:b1:f8
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff
Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
net/bridge/br_if.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3aef22931ab..636e0a842f8a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -288,6 +288,8 @@ static void del_nbp(struct net_bridge_port *p)
dev->priv_flags &= ~IFF_BRIDGE_PORT;
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
+
netdev_rx_handler_unregister(dev);
br_multicast_del_port(p);
--
2.14.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-15 19:38 [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge Vincent Bernat
@ 2017-09-15 21:08 ` Vincent Bernat
2017-09-16 14:18 ` [PATCH net-next v2] " Vincent Bernat
0 siblings, 1 reply; 18+ messages in thread
From: Vincent Bernat @ 2017-09-15 21:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev
❦ 15 septembre 2017 21:38 +0200, Vincent Bernat <vincent@bernat.im> :
> Currently, when an interface is released from a bridge, we get a
> RTM_DELLINK event through netlink:
>
> Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 6e:23:c2:54:3a:b3
It should be noted this only happens when using the ioctl API. When
using the netlink API, rtnetlink.c will send a RTM_NEWLINK because of
the modification.
--
When in doubt, tell the truth.
-- Mark Twain
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-15 21:08 ` Vincent Bernat
@ 2017-09-16 14:18 ` Vincent Bernat
2017-09-20 21:09 ` David Miller
0 siblings, 1 reply; 18+ messages in thread
From: Vincent Bernat @ 2017-09-16 14:18 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge, netdev; +Cc: Vincent Bernat
Currently, when an interface is released from a bridge via
ioctl(), we get a RTM_DELLINK event through netlink:
Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
link/ether 6e:23:c2:54:3a:b3
Deleted 2: dummy0: <BROADCAST,NOARP> 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: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 8a:bb:e7:94:b1:f8
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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 <vincent@bernat.im>
---
net/bridge/br_ioctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 7970f8540cbb..3148cb3a8e82 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -99,8 +99,10 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
if (isadd)
ret = br_add_if(br, dev);
- else
+ else {
ret = br_del_if(br, dev);
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
+ }
return ret;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-16 14:18 ` [PATCH net-next v2] " Vincent Bernat
@ 2017-09-20 21:09 ` David Miller
2017-09-20 21:57 ` David Ahern
0 siblings, 1 reply; 18+ messages in thread
From: David Miller @ 2017-09-20 21:09 UTC (permalink / raw)
To: vincent; +Cc: stephen, bridge, netdev, dsahern
From: Vincent Bernat <vincent@bernat.im>
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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
> link/ether 6e:23:c2:54:3a:b3
> Deleted 2: dummy0: <BROADCAST,NOARP> 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: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 8a:bb:e7:94:b1:f8
> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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 <vincent@bernat.im>
Applied, thank you.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-20 21:09 ` David Miller
@ 2017-09-20 21:57 ` David Ahern
2017-09-20 22:12 ` Vincent Bernat
2017-09-20 23:21 ` Stephen Hemminger
0 siblings, 2 replies; 18+ messages in thread
From: David Ahern @ 2017-09-20 21:57 UTC (permalink / raw)
To: David Miller, vincent; +Cc: stephen, bridge, netdev
On 9/20/17 3:09 PM, David Miller wrote:
> From: Vincent Bernat <vincent@bernat.im>
> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
>> link/ether 6e:23:c2:54:3a:b3
>> Deleted 2: dummy0: <BROADCAST,NOARP> 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: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 8a:bb:e7:94:b1:f8
>> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500
master br0 state UNKNOWN
link/ether d6:c3:73:86:3c:73
[LINK]Deleted family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu
1500 master br0 state UNKNOWN
link/ether d6:c3:73:86:3c:73
[LINK]family 0: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-20 21:57 ` David Ahern
@ 2017-09-20 22:12 ` Vincent Bernat
2017-09-20 22:41 ` David Miller
2017-09-20 23:21 ` Stephen Hemminger
1 sibling, 1 reply; 18+ messages in thread
From: Vincent Bernat @ 2017-09-20 22:12 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, stephen, bridge, netdev
❦ 20 septembre 2017 15:57 -0600, David Ahern <dsahern@gmail.com> :
> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500
> master br0 state UNKNOWN
> link/ether d6:c3:73:86:3c:73
>
> [LINK]Deleted family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> 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"
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-20 22:12 ` Vincent Bernat
@ 2017-09-20 22:41 ` David Miller
0 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2017-09-20 22:41 UTC (permalink / raw)
To: vincent; +Cc: dsahern, stephen, bridge, netdev
From: Vincent Bernat <vincent@bernat.im>
Date: Thu, 21 Sep 2017 00:12:53 +0200
> ❦ 20 septembre 2017 15:57 -0600, David Ahern <dsahern@gmail.com> :
>
>> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500
>> master br0 state UNKNOWN
>> link/ether d6:c3:73:86:3c:73
>>
>> [LINK]Deleted family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-20 21:57 ` David Ahern
2017-09-20 22:12 ` Vincent Bernat
@ 2017-09-20 23:21 ` Stephen Hemminger
2017-09-21 10:04 ` Vincent Bernat
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
1 sibling, 2 replies; 18+ messages in thread
From: Stephen Hemminger @ 2017-09-20 23:21 UTC (permalink / raw)
To: David Ahern; +Cc: David Miller, vincent, bridge, netdev
On Wed, 20 Sep 2017 15:57:16 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 9/20/17 3:09 PM, David Miller wrote:
> > From: Vincent Bernat <vincent@bernat.im>
> > 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
> >> link/ether 6e:23:c2:54:3a:b3
> >> Deleted 2: dummy0: <BROADCAST,NOARP> 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: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP> 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: <BROADCAST,NOARP> 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: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> >> link/ether 8a:bb:e7:94:b1:f8
> >> 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> 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: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500
> master br0 state UNKNOWN
> link/ether d6:c3:73:86:3c:73
>
> [LINK]Deleted family 7: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu
> 1500 master br0 state UNKNOWN
> link/ether d6:c3:73:86:3c:73
>
> [LINK]family 0: 35: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> 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.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-20 23:21 ` Stephen Hemminger
@ 2017-09-21 10:04 ` Vincent Bernat
2017-09-21 15:09 ` Roopa Prabhu
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
1 sibling, 1 reply; 18+ messages in thread
From: Vincent Bernat @ 2017-09-21 10:04 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, David Miller, bridge, netdev
❦ 20 septembre 2017 16:21 -0700, Stephen Hemminger <stephen@networkplumber.org> :
> 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
^ permalink raw reply [flat|nested] 18+ messages in thread
* [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-20 23:21 ` Stephen Hemminger
2017-09-21 10:04 ` Vincent Bernat
@ 2017-09-21 10:05 ` Vincent Bernat
2017-09-21 15:15 ` Stephen Hemminger
` (2 more replies)
1 sibling, 3 replies; 18+ messages in thread
From: Vincent Bernat @ 2017-09-21 10:05 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern, David Miller, bridge, netdev
Cc: Vincent Bernat
Currently, there is a difference in netlink events received when an
interface is modified through bridge ioctl() or through netlink. This
patch generates additional events when an interface is added to or
removed from a bridge via ioctl().
When adding then removing an interface from a bridge with netlink, we
get:
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
When using ioctl():
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 9e:da:60:ee:cf:c8
5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
Without this patch, the last netlink notification is not sent.
Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
net/bridge/br_ioctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index 7970f8540cbb..66cd98772051 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -102,6 +102,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
else
ret = br_del_if(br, dev);
+ if (!ret)
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
+
return ret;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-21 10:04 ` Vincent Bernat
@ 2017-09-21 15:09 ` Roopa Prabhu
2017-09-21 15:31 ` Vincent Bernat
0 siblings, 1 reply; 18+ messages in thread
From: Roopa Prabhu @ 2017-09-21 15:09 UTC (permalink / raw)
To: Vincent Bernat
Cc: Stephen Hemminger, David Ahern, David Miller, bridge,
netdev@vger.kernel.org
On Thu, Sep 21, 2017 at 3:04 AM, Vincent Bernat <vincent@bernat.im> wrote:
> ❦ 20 septembre 2017 16:21 -0700, Stephen Hemminger <stephen@networkplumber.org> :
>
>> 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`
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
@ 2017-09-21 15:15 ` Stephen Hemminger
2017-09-21 15:45 ` Vincent Bernat
2017-09-21 16:43 ` David Ahern
2017-09-21 22:45 ` David Miller
2 siblings, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2017-09-21 15:15 UTC (permalink / raw)
To: Vincent Bernat; +Cc: netdev, bridge, David Miller, David Ahern
On Thu, 21 Sep 2017 12:05:25 +0200
Vincent Bernat <vincent@bernat.im> wrote:
> Currently, there is a difference in netlink events received when an
> interface is modified through bridge ioctl() or through netlink. This
> patch generates additional events when an interface is added to or
> removed from a bridge via ioctl().
>
> When adding then removing an interface from a bridge with netlink, we
> get:
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> When using ioctl():
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> Without this patch, the last netlink notification is not sent.
>
> Signed-off-by: Vincent Bernat <vincent@bernat.im>
This makes sense, you should probably add a Fixes: tag to help maintainers
of long term stable kernels.
Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH net-next v2] bridge: also trigger RTM_NEWLINK when interface is released from bridge
2017-09-21 15:09 ` Roopa Prabhu
@ 2017-09-21 15:31 ` Vincent Bernat
0 siblings, 0 replies; 18+ messages in thread
From: Vincent Bernat @ 2017-09-21 15:31 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Stephen Hemminger, David Ahern, David Miller, bridge,
netdev@vger.kernel.org
❦ 21 septembre 2017 08:09 -0700, Roopa Prabhu <roopa@cumulusnetworks.com> :
>>> 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)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 15:15 ` Stephen Hemminger
@ 2017-09-21 15:45 ` Vincent Bernat
0 siblings, 0 replies; 18+ messages in thread
From: Vincent Bernat @ 2017-09-21 15:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, David Miller, bridge, netdev
❦ 21 septembre 2017 08:15 -0700, Stephen Hemminger <stephen@networkplumber.org> :
>> Currently, there is a difference in netlink events received when an
>> interface is modified through bridge ioctl() or through netlink. This
>> patch generates additional events when an interface is added to or
>> removed from a bridge via ioctl().
>>
>> When adding then removing an interface from a bridge with netlink, we
>> get:
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> When using ioctl():
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> Without this patch, the last netlink notification is not sent.
>>
>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>
> This makes sense, you should probably add a Fixes: tag to help maintainers
> of long term stable kernels.
>
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
I wouldn't know which commit would be fixed since this is not a
regression, just a behavior difference.
--
Make sure special cases are truly special.
- The Elements of Programming Style (Kernighan & Plauger)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
2017-09-21 15:15 ` Stephen Hemminger
@ 2017-09-21 16:43 ` David Ahern
2017-09-21 17:20 ` Roopa Prabhu
2017-09-21 22:45 ` David Miller
2 siblings, 1 reply; 18+ messages in thread
From: David Ahern @ 2017-09-21 16:43 UTC (permalink / raw)
To: Vincent Bernat, Stephen Hemminger, David Miller, bridge, netdev
On 9/21/17 4:05 AM, Vincent Bernat wrote:
> Currently, there is a difference in netlink events received when an
> interface is modified through bridge ioctl() or through netlink. This
> patch generates additional events when an interface is added to or
> removed from a bridge via ioctl().
>
> When adding then removing an interface from a bridge with netlink, we
> get:
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> When using ioctl():
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 9e:da:60:ee:cf:c8
> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>
> Without this patch, the last netlink notification is not sent.
>
> Signed-off-by: Vincent Bernat <vincent@bernat.im>
> ---
> net/bridge/br_ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
> index 7970f8540cbb..66cd98772051 100644
> --- a/net/bridge/br_ioctl.c
> +++ b/net/bridge/br_ioctl.c
> @@ -102,6 +102,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
> else
> ret = br_del_if(br, dev);
>
> + if (!ret)
> + rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
> +
> return ret;
> }
>
>
Agreed that this is needed for userspace to know about the master change
when done through ioctl. The bridge code is emitting a lot of what
appears to be redundant messages for both paths (netlink and ioctl).
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 16:43 ` David Ahern
@ 2017-09-21 17:20 ` Roopa Prabhu
2017-09-21 17:29 ` David Ahern
0 siblings, 1 reply; 18+ messages in thread
From: Roopa Prabhu @ 2017-09-21 17:20 UTC (permalink / raw)
To: David Ahern
Cc: Vincent Bernat, Stephen Hemminger, David Miller, bridge,
netdev@vger.kernel.org
On Thu, Sep 21, 2017 at 9:43 AM, David Ahern <dsahern@gmail.com> wrote:
> On 9/21/17 4:05 AM, Vincent Bernat wrote:
>> Currently, there is a difference in netlink events received when an
>> interface is modified through bridge ioctl() or through netlink. This
>> patch generates additional events when an interface is added to or
>> removed from a bridge via ioctl().
>>
>> When adding then removing an interface from a bridge with netlink, we
>> get:
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> When using ioctl():
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master bridge0 state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> Deleted 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
>> link/ether 9e:da:60:ee:cf:c8
>> 5: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>> link/ether 9e:da:60:ee:cf:c8 brd ff:ff:ff:ff:ff:ff
>>
>> Without this patch, the last netlink notification is not sent.
>>
>> Signed-off-by: Vincent Bernat <vincent@bernat.im>
>> ---
>> net/bridge/br_ioctl.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
>> index 7970f8540cbb..66cd98772051 100644
>> --- a/net/bridge/br_ioctl.c
>> +++ b/net/bridge/br_ioctl.c
>> @@ -102,6 +102,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd)
>> else
>> ret = br_del_if(br, dev);
>>
>> + if (!ret)
>> + rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
>> +
>> return ret;
>> }
>>
>>
>
> Agreed that this is needed for userspace to know about the master change
> when done through ioctl. The bridge code is emitting a lot of what
> appears to be redundant messages for both paths (netlink and ioctl).
>
> Reviewed-by: David Ahern <dsahern@gmail.com>
>
this patch seems fine...but ideally I would have assumed
netdev_upper_dev_unlink which
is eventually called in both paths would generate the RTN_NEWLINK
IFF_MASTER in response
to the NETDEV_CHANGEUPPER notifier. If we add it there now, it might
need some more fixes
to not generate redundant notifications for the netlink case.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 17:20 ` Roopa Prabhu
@ 2017-09-21 17:29 ` David Ahern
0 siblings, 0 replies; 18+ messages in thread
From: David Ahern @ 2017-09-21 17:29 UTC (permalink / raw)
To: Roopa Prabhu, David Ahern
Cc: netdev@vger.kernel.org, bridge, Vincent Bernat, David Miller
On 9/21/17 11:20 AM, Roopa Prabhu wrote:
> this patch seems fine...but ideally I would have assumed
> netdev_upper_dev_unlink which
> is eventually called in both paths would generate the RTN_NEWLINK
> IFF_MASTER in response
> to the NETDEV_CHANGEUPPER notifier. If we add it there now, it might
> need some more fixes
> to not generate redundant notifications for the netlink case.
Agreed and it is another pandora's box
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
2017-09-21 15:15 ` Stephen Hemminger
2017-09-21 16:43 ` David Ahern
@ 2017-09-21 22:45 ` David Miller
2 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2017-09-21 22:45 UTC (permalink / raw)
To: vincent; +Cc: stephen, dsahern, bridge, netdev
From: Vincent Bernat <vincent@bernat.im>
Date: Thu, 21 Sep 2017 12:05:25 +0200
> Currently, there is a difference in netlink events received when an
> interface is modified through bridge ioctl() or through netlink. This
> patch generates additional events when an interface is added to or
> removed from a bridge via ioctl().
...
> Signed-off-by: Vincent Bernat <vincent@bernat.im>
Applied.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-09-21 22:45 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-15 19:38 [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge Vincent Bernat
2017-09-15 21:08 ` Vincent Bernat
2017-09-16 14:18 ` [PATCH net-next v2] " Vincent Bernat
2017-09-20 21:09 ` David Miller
2017-09-20 21:57 ` David Ahern
2017-09-20 22:12 ` Vincent Bernat
2017-09-20 22:41 ` David Miller
2017-09-20 23:21 ` Stephen Hemminger
2017-09-21 10:04 ` Vincent Bernat
2017-09-21 15:09 ` Roopa Prabhu
2017-09-21 15:31 ` Vincent Bernat
2017-09-21 10:05 ` [net-next v3] bridge: trigger RTM_NEWLINK when interface is modified by bridge ioctl Vincent Bernat
2017-09-21 15:15 ` Stephen Hemminger
2017-09-21 15:45 ` Vincent Bernat
2017-09-21 16:43 ` David Ahern
2017-09-21 17:20 ` Roopa Prabhu
2017-09-21 17:29 ` David Ahern
2017-09-21 22:45 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).