Message ID | 1398341787.4171.16.camel@ubuntu-vm-makita |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, Apr 24, 2014 at 2:16 PM, Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote: > When bridge device is created with IFLA_ADDRESS, we are not calling > br_stp_change_bridge_id(), which leads to incorrect local fdb > management and bridge id calculation, and prevents us from receiving > frames on the bridge device. > > Reported-by: Tom Gundersen <teg@jklm.no> Thanks. That looks correct to me (not able to test at the moment though). Would this be appropriate for stable if it goes in? Cheers, Tom > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> > --- > net/bridge/br_netlink.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c > index e74b6d53..a8b664e 100644 > --- a/net/bridge/br_netlink.c > +++ b/net/bridge/br_netlink.c > @@ -445,6 +445,25 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[]) > return 0; > } > > +static int br_dev_newlink(struct net *src_net, struct net_device *dev, > + struct nlattr *tb[], struct nlattr *data[]) > +{ > + int err; > + struct net_bridge *br = netdev_priv(dev); > + > + if (tb[IFLA_ADDRESS]) { > + spin_lock_bh(&br->lock); > + br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS])); > + spin_unlock_bh(&br->lock); > + } > + > + err = register_netdevice(dev); > + if (err) > + return err; > + > + return 0; > +} > + > static size_t br_get_link_af_size(const struct net_device *dev) > { > struct net_port_vlans *pv; > @@ -473,6 +492,7 @@ struct rtnl_link_ops br_link_ops __read_mostly = { > .priv_size = sizeof(struct net_bridge), > .setup = br_dev_setup, > .validate = br_validate, > + .newlink = br_dev_newlink, > .dellink = br_dev_delete, > }; > > -- > 1.8.1.2 > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 24 Apr 2014 21:16:27 +0900 Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote: > +static int br_dev_newlink(struct net *src_net, struct net_device *dev, > + struct nlattr *tb[], struct nlattr *data[]) > +{ > + int err; > + struct net_bridge *br = netdev_priv(dev); > + > + if (tb[IFLA_ADDRESS]) { > + spin_lock_bh(&br->lock); > + br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS])); > + spin_unlock_bh(&br->lock); > + } > + > + err = register_netdevice(dev); > + if (err) > + return err; > + > + return 0; > +} Looks good. Why not just do simpler tail call?? return register_netdevice(dev); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2014-04-24 at 11:38 -0700, Stephen Hemminger wrote: > On Thu, 24 Apr 2014 21:16:27 +0900 > Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote: > > > +static int br_dev_newlink(struct net *src_net, struct net_device *dev, > > + struct nlattr *tb[], struct nlattr *data[]) > > +{ > > + int err; > > + struct net_bridge *br = netdev_priv(dev); > > + > > + if (tb[IFLA_ADDRESS]) { > > + spin_lock_bh(&br->lock); > > + br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS])); > > + spin_unlock_bh(&br->lock); > > + } > > + > > + err = register_netdevice(dev); > > + if (err) > > + return err; > > + > > + return 0; > > +} > > Looks good. > > Why not just do simpler tail call?? > return register_netdevice(dev); Yes, It's simpler. I mimicked team_newlink()'s style, so we can also make it simpler later :) I'll send v2. Thanks, Toshiaki Makita -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2014-04-24 at 18:04 +0200, Tom Gundersen wrote: > On Thu, Apr 24, 2014 at 2:16 PM, Toshiaki Makita > <makita.toshiaki@lab.ntt.co.jp> wrote: > > When bridge device is created with IFLA_ADDRESS, we are not calling > > br_stp_change_bridge_id(), which leads to incorrect local fdb > > management and bridge id calculation, and prevents us from receiving > > frames on the bridge device. > > > > Reported-by: Tom Gundersen <teg@jklm.no> > > Thanks. That looks correct to me (not able to test at the moment > though). Would this be appropriate for stable if it goes in? This can apply 3.14 as is. For 3.12 or earlier tree, this needs modification to insert the fdb entry (or backport fdb fix patchset (684bd2e1)). And for 3.2, another patch needs to be backported before this because br_fdb_change_mac_address() doesn't exist. Thanks, Toshiaki Makita -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index e74b6d53..a8b664e 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -445,6 +445,25 @@ static int br_validate(struct nlattr *tb[], struct nlattr *data[]) return 0; } +static int br_dev_newlink(struct net *src_net, struct net_device *dev, + struct nlattr *tb[], struct nlattr *data[]) +{ + int err; + struct net_bridge *br = netdev_priv(dev); + + if (tb[IFLA_ADDRESS]) { + spin_lock_bh(&br->lock); + br_stp_change_bridge_id(br, nla_data(tb[IFLA_ADDRESS])); + spin_unlock_bh(&br->lock); + } + + err = register_netdevice(dev); + if (err) + return err; + + return 0; +} + static size_t br_get_link_af_size(const struct net_device *dev) { struct net_port_vlans *pv; @@ -473,6 +492,7 @@ struct rtnl_link_ops br_link_ops __read_mostly = { .priv_size = sizeof(struct net_bridge), .setup = br_dev_setup, .validate = br_validate, + .newlink = br_dev_newlink, .dellink = br_dev_delete, };
When bridge device is created with IFLA_ADDRESS, we are not calling br_stp_change_bridge_id(), which leads to incorrect local fdb management and bridge id calculation, and prevents us from receiving frames on the bridge device. Reported-by: Tom Gundersen <teg@jklm.no> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> --- net/bridge/br_netlink.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)