Skip to content

Commit 02dba43

Browse files
roopa-prabhudavem330
authored andcommitted
bridge: fix setlink/dellink notifications
problems with bridge getlink/setlink notifications today: - bridge setlink generates two notifications to userspace - one from the bridge driver - one from rtnetlink.c (rtnl_bridge_notify) - dellink generates one notification from rtnetlink.c. Which means bridge setlink and dellink notifications are not consistent - Looking at the code it appears, If both BRIDGE_FLAGS_MASTER and BRIDGE_FLAGS_SELF were set, the size calculation in rtnl_bridge_notify can be wrong. Example: if you set both BRIDGE_FLAGS_MASTER and BRIDGE_FLAGS_SELF in a setlink request to rocker dev, rtnl_bridge_notify will allocate skb for one set of bridge attributes, but, both the bridge driver and rocker dev will try to add attributes resulting in twice the number of attributes being added to the skb. (rocker dev calls ndo_dflt_bridge_getlink) There are multiple options: 1) Generate one notification including all attributes from master and self: But, I don't think it will work, because both master and self may use the same attributes/policy. Cannot pack the same set of attributes in a single notification from both master and slave (duplicate attributes). 2) Generate one notification from master and the other notification from self (This seems to be ideal): For master: the master driver will send notification (bridge in this example) For self: the self driver will send notification (rocker in the above example. It can use helpers from rtnetlink.c to do so. Like the ndo_dflt_bridge_getlink api). This patch implements 2) (leaving the 'rtnl_bridge_notify' around to be used with 'self'). v1->v2 : - rtnl_bridge_notify is now called only for self, so, remove 'BRIDGE_FLAGS_SELF' check and cleanup a few things - rtnl_bridge_dellink used to always send a RTM_NEWLINK msg earlier. So, I have changed the notification from br_dellink to go as RTM_NEWLINK Signed-off-by: Roopa Prabhu <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 1e7d06b commit 02dba43

File tree

2 files changed

+26
-24
lines changed

2 files changed

+26
-24
lines changed

net/bridge/br_netlink.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,11 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
569569

570570
err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
571571
afspec, RTM_DELLINK);
572+
if (err == 0)
573+
/* Send RTM_NEWLINK because userspace
574+
* expects RTM_NEWLINK for vlan dels
575+
*/
576+
br_ifinfo_notify(RTM_NEWLINK, p);
572577

573578
return err;
574579
}

net/core/rtnetlink.c

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,32 +2880,24 @@ static inline size_t bridge_nlmsg_size(void)
28802880
+ nla_total_size(sizeof(u16)); /* IFLA_BRIDGE_MODE */
28812881
}
28822882

2883-
static int rtnl_bridge_notify(struct net_device *dev, u16 flags)
2883+
static int rtnl_bridge_notify(struct net_device *dev)
28842884
{
28852885
struct net *net = dev_net(dev);
2886-
struct net_device *br_dev = netdev_master_upper_dev_get(dev);
28872886
struct sk_buff *skb;
28882887
int err = -EOPNOTSUPP;
28892888

2889+
if (!dev->netdev_ops->ndo_bridge_getlink)
2890+
return 0;
2891+
28902892
skb = nlmsg_new(bridge_nlmsg_size(), GFP_ATOMIC);
28912893
if (!skb) {
28922894
err = -ENOMEM;
28932895
goto errout;
28942896
}
28952897

2896-
if ((!flags || (flags & BRIDGE_FLAGS_MASTER)) &&
2897-
br_dev && br_dev->netdev_ops->ndo_bridge_getlink) {
2898-
err = br_dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
2899-
if (err < 0)
2900-
goto errout;
2901-
}
2902-
2903-
if ((flags & BRIDGE_FLAGS_SELF) &&
2904-
dev->netdev_ops->ndo_bridge_getlink) {
2905-
err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
2906-
if (err < 0)
2907-
goto errout;
2908-
}
2898+
err = dev->netdev_ops->ndo_bridge_getlink(skb, 0, 0, dev, 0);
2899+
if (err < 0)
2900+
goto errout;
29092901

29102902
rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC);
29112903
return 0;
@@ -2975,16 +2967,18 @@ static int rtnl_bridge_setlink(struct sk_buff *skb, struct nlmsghdr *nlh)
29752967
err = -EOPNOTSUPP;
29762968
else
29772969
err = dev->netdev_ops->ndo_bridge_setlink(dev, nlh);
2978-
2979-
if (!err)
2970+
if (!err) {
29802971
flags &= ~BRIDGE_FLAGS_SELF;
2972+
2973+
/* Generate event to notify upper layer of bridge
2974+
* change
2975+
*/
2976+
err = rtnl_bridge_notify(dev);
2977+
}
29812978
}
29822979

29832980
if (have_flags)
29842981
memcpy(nla_data(attr), &flags, sizeof(flags));
2985-
/* Generate event to notify upper layer of bridge change */
2986-
if (!err)
2987-
err = rtnl_bridge_notify(dev, oflags);
29882982
out:
29892983
return err;
29902984
}
@@ -3049,15 +3043,18 @@ static int rtnl_bridge_dellink(struct sk_buff *skb, struct nlmsghdr *nlh)
30493043
else
30503044
err = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
30513045

3052-
if (!err)
3046+
if (!err) {
30533047
flags &= ~BRIDGE_FLAGS_SELF;
3048+
3049+
/* Generate event to notify upper layer of bridge
3050+
* change
3051+
*/
3052+
err = rtnl_bridge_notify(dev);
3053+
}
30543054
}
30553055

30563056
if (have_flags)
30573057
memcpy(nla_data(attr), &flags, sizeof(flags));
3058-
/* Generate event to notify upper layer of bridge change */
3059-
if (!err)
3060-
err = rtnl_bridge_notify(dev, oflags);
30613058
out:
30623059
return err;
30633060
}

0 commit comments

Comments
 (0)