| Message ID | 1393427905-6811-4-git-send-email-vyasevic@redhat.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 02/26/2014 10:46 AM, Michael S. Tsirkin wrote: >> + >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >> index f072b34..e782c2e 100644 >> --- a/net/bridge/br_if.c >> +++ b/net/bridge/br_if.c >> @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) >> >> br_ifinfo_notify(RTM_DELLINK, p); >> >> - list_del_rcu(&p->list); >> + dev->priv_flags &= ~IFF_BRIDGE_PORT; >> >> if (p->flags & BR_FLOOD) >> br_del_flood_port(p, br); >> @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) >> nbp_vlan_flush(p); >> br_fdb_delete_by_port(br, p, 1); >> >> - dev->priv_flags &= ~IFF_BRIDGE_PORT; >> + list_del_rcu(&p->list); >> >> netdev_rx_handler_unregister(dev); >> > > Hmm here we are moving list_del_rcu > back to after nbp_vlan_flush. > Was the reordering in the previous patch necessary? Oops. Will fix it up. -vlad > >> @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> br->n_flood_ports++; >> if (br->n_flood_ports == 1) >> br->c_flood_port = p; >> + >> + br_fdb_addrs_sync(br); >> } >> >> static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> * set it if it is not set. >> */ >> br->n_flood_ports--; >> - if (p == br->c_flood_port) >> + if (p == br->c_flood_port) { >> + br_fdb_addrs_unsync(br); >> br->c_flood_port = NULL; >> + } >> >> if (br->n_flood_ports == 1) { >> list_for_each_entry(port, &p->br->port_list, list) { >> - if (port->flags & BR_FLOOD) { >> + if (br_port_exists(port->dev) && >> + (port->flags & BR_FLOOD)) { >> br->c_flood_port = port; >> + br_fdb_addrs_sync(br); >> break; >> } >> } >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index 26a3987..40a6927 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -296,6 +296,7 @@ struct net_bridge >> u8 vlan_enabled; >> struct net_port_vlans __rcu *vlan_info; >> #endif >> + struct netdev_hw_addr_list conf_addrs; >> }; >> >> struct br_input_skb_cb { >> @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, >> const unsigned char *addr, u16 nlh_flags); >> int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, >> struct net_device *dev, int idx); >> +void br_fdb_addrs_sync(struct net_bridge *br); >> +void br_fdb_addrs_unsync(struct net_bridge *br); >> >> /* br_forward.c */ >> void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); >> diff --git a/net/core/dev.c b/net/core/dev.c >> index 4ad1b78..eca4d476 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) >> if (ops->ndo_set_rx_mode) >> ops->ndo_set_rx_mode(dev); >> } >> +EXPORT_SYMBOL(__dev_set_rx_mode); >> >> void dev_set_rx_mode(struct net_device *dev) >> { >> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c >> index 329d579..3de44a3 100644 >> --- a/net/core/dev_addr_lists.c >> +++ b/net/core/dev_addr_lists.c >> @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, >> sync); >> } >> >> -static int __hw_addr_add(struct netdev_hw_addr_list *list, >> - const unsigned char *addr, int addr_len, >> - unsigned char addr_type) >> +int __hw_addr_add(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type) >> { >> return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, >> 0); >> } >> +EXPORT_SYMBOL(__hw_addr_add); >> >> static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, >> struct netdev_hw_addr *ha, bool global, >> @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, >> return -ENOENT; >> } >> >> -static int __hw_addr_del(struct netdev_hw_addr_list *list, >> - const unsigned char *addr, int addr_len, >> - unsigned char addr_type) >> +int __hw_addr_del(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type) >> { >> return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); >> } >> +EXPORT_SYMBOL(__hw_addr_del); >> >> static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, >> struct netdev_hw_addr *ha, >> -- >> 1.8.5.3 > > > I would split net/core/ changes out, and Cc more people > on it. > -- 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 Wed, Feb 26, 2014 at 10:18:21AM -0500, Vlad Yasevich wrote: > When a static fdb entry is created, add the mac address to the bridge > address list. This list is used to program the proper port's > address list. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > --- > include/linux/netdevice.h | 6 +++ > net/bridge/br_device.c | 2 + > net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++--- > net/bridge/br_if.c | 14 ++++-- > net/bridge/br_private.h | 3 ++ > net/core/dev.c | 1 + > net/core/dev_addr_lists.c | 14 +++--- > 7 files changed, 134 insertions(+), 16 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 440a02e..e29cce1 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev); > void unregister_netdev(struct net_device *dev); > > /* General hardware address lists handling functions */ > +int __hw_addr_add(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type); > +int __hw_addr_del(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type); > int __hw_addr_sync(struct netdev_hw_addr_list *to_list, > struct netdev_hw_addr_list *from_list, int addr_len); > void __hw_addr_unsync(struct netdev_hw_addr_list *to_list, > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index 63f0455..1521db6 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev) > u64_stats_init(&br_dev_stats->syncp); > } > > + __hw_addr_init(&br->conf_addrs); > + > return 0; > } > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index 9203d5a..ef95e81 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > static void fdb_notify(struct net_bridge *br, > const struct net_bridge_fdb_entry *, int); > > +static int br_addr_add(struct net_bridge *br, const unsigned char *addr); > +static int br_addr_del(struct net_bridge *br, const unsigned char *addr); > + > static u32 fdb_salt __read_mostly; > > int __init br_fdb_init(void) > @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head) > > static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) > { > + if (f->is_static) > + br_addr_del(br, f->addr.addr); > + > hlist_del_rcu(&f->hlist); > fdb_notify(br, f, RTM_DELNEIGH); > call_rcu(&f->rcu, fdb_rcu_free); > @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > return -ENOMEM; > > fdb->is_local = fdb->is_static = 1; > + br_addr_add(br, addr); > fdb_notify(br, fdb, RTM_NEWNEIGH); > return 0; > } > @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, > } > > if (fdb_to_nud(fdb) != state) { > - if (state & NUD_PERMANENT) > - fdb->is_local = fdb->is_static = 1; > - else if (state & NUD_NOARP) { > + if (state & NUD_PERMANENT) { > + fdb->is_local = 1; > + if (!fdb->is_static) { > + fdb->is_static = 1; > + br_addr_add(br, addr); > + } > + } else if (state & NUD_NOARP) { > + fdb->is_local = 0; > + if (!fdb->is_static) { > + fdb->is_static = 1; > + br_addr_add(br, addr); > + } > + } else { > fdb->is_local = 0; > - fdb->is_static = 1; > - } else > - fdb->is_local = fdb->is_static = 0; > + if (fdb->is_static) { > + fdb->is_static = 0; > + br_addr_del(br, addr); > + } > + } > > modified = true; > } > + > fdb->added_by_user = 1; > > fdb->used = jiffies; > @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], > out: > return err; > } > + > + most places have a single line between functions, maybe it's best to be consistent. > +/* Sync the current list to the correct flood port. */ > +void br_fdb_addrs_sync(struct net_bridge *br) > +{ > + struct net_bridge_port *p; > + int err; > + > + /* This function has to run under RTNL. > + * Program the user added addresses into the proper port. This > + * fuction follows the following algorithm: > + * a) If only 1 port is flooding: > + * - write all the addresses to this one port. > + */ > + if (br->n_flood_ports == 1) { > + p = br->c_flood_port; > + netif_addr_lock(p->dev); > + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs, > + p->dev->addr_len); > + if (!err) > + __dev_set_rx_mode(p->dev); > + netif_addr_unlock(p->dev); > + > + } > +} > + > +void br_fdb_addrs_unsync(struct net_bridge *br) > +{ > + struct net_bridge_port *p = br->c_flood_port; > + > + if (!p) > + return; > + > + netif_addr_lock(p->dev); > + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len); > + __dev_set_rx_mode(p->dev); > + netif_addr_unlock(p->dev); > +} > + > +/* When a static FDB entry is added, the mac address from the entry is > + * added to the bridge private HW address list and all required ports > + * are then updated with the new information. > + * Called under RTNL. > + */ > +static int br_addr_add(struct net_bridge *br, const unsigned char *addr) > +{ > + int err; > + > + ASSERT_RTNL(); > + > + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len, > + NETDEV_HW_ADDR_T_UNICAST); > + > + if (!err) > + br_fdb_addrs_sync(br); > + > + return err; > +} > + > +/* When a static FDB entry is deleted, the HW address from that entry is > + * also removed from the bridge private HW address list and updates all > + * the ports with needed information. > + * Called under RTNL. > + */ > +static int br_addr_del(struct net_bridge *br, const unsigned char *addr) > +{ > + int err; > + > + ASSERT_RTNL(); > + > + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len, > + NETDEV_HW_ADDR_T_UNICAST); > + if (!err) > + br_fdb_addrs_sync(br); > + > + return err; > +} > + > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index f072b34..e782c2e 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) > > br_ifinfo_notify(RTM_DELLINK, p); > > - list_del_rcu(&p->list); > + dev->priv_flags &= ~IFF_BRIDGE_PORT; > > if (p->flags & BR_FLOOD) > br_del_flood_port(p, br); > @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) > nbp_vlan_flush(p); > br_fdb_delete_by_port(br, p, 1); > > - dev->priv_flags &= ~IFF_BRIDGE_PORT; > + list_del_rcu(&p->list); > > netdev_rx_handler_unregister(dev); > Hmm here we are moving list_del_rcu back to after nbp_vlan_flush. Was the reordering in the previous patch necessary? > @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) > br->n_flood_ports++; > if (br->n_flood_ports == 1) > br->c_flood_port = p; > + > + br_fdb_addrs_sync(br); > } > > static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > * set it if it is not set. > */ > br->n_flood_ports--; > - if (p == br->c_flood_port) > + if (p == br->c_flood_port) { > + br_fdb_addrs_unsync(br); > br->c_flood_port = NULL; > + } > > if (br->n_flood_ports == 1) { > list_for_each_entry(port, &p->br->port_list, list) { > - if (port->flags & BR_FLOOD) { > + if (br_port_exists(port->dev) && > + (port->flags & BR_FLOOD)) { > br->c_flood_port = port; > + br_fdb_addrs_sync(br); > break; > } > } > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 26a3987..40a6927 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -296,6 +296,7 @@ struct net_bridge > u8 vlan_enabled; > struct net_port_vlans __rcu *vlan_info; > #endif > + struct netdev_hw_addr_list conf_addrs; > }; > > struct br_input_skb_cb { > @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, > const unsigned char *addr, u16 nlh_flags); > int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, > struct net_device *dev, int idx); > +void br_fdb_addrs_sync(struct net_bridge *br); > +void br_fdb_addrs_unsync(struct net_bridge *br); > > /* br_forward.c */ > void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); > diff --git a/net/core/dev.c b/net/core/dev.c > index 4ad1b78..eca4d476 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) > if (ops->ndo_set_rx_mode) > ops->ndo_set_rx_mode(dev); > } > +EXPORT_SYMBOL(__dev_set_rx_mode); > > void dev_set_rx_mode(struct net_device *dev) > { > diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c > index 329d579..3de44a3 100644 > --- a/net/core/dev_addr_lists.c > +++ b/net/core/dev_addr_lists.c > @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > sync); > } > > -static int __hw_addr_add(struct netdev_hw_addr_list *list, > - const unsigned char *addr, int addr_len, > - unsigned char addr_type) > +int __hw_addr_add(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type) > { > return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, > 0); > } > +EXPORT_SYMBOL(__hw_addr_add); > > static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > struct netdev_hw_addr *ha, bool global, > @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, > return -ENOENT; > } > > -static int __hw_addr_del(struct netdev_hw_addr_list *list, > - const unsigned char *addr, int addr_len, > - unsigned char addr_type) > +int __hw_addr_del(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type) > { > return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); > } > +EXPORT_SYMBOL(__hw_addr_del); > > static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, > struct netdev_hw_addr *ha, > -- > 1.8.5.3 I would split net/core/ changes out, and Cc more people on it. -- 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 Wed, Feb 26, 2014 at 10:18:21AM -0500, Vlad Yasevich wrote: > When a static fdb entry is created, add the mac address to the bridge > address list. This list is used to program the proper port's > address list. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> Hmm won't this mean learned addresses are missing there? And if so isn't this a problem when we use this list in the next patch? I guess we could limit this to configurations where learning is also disabled (not just flood) - seems a bit arbitrary but will likely cover most use-cases. > --- > include/linux/netdevice.h | 6 +++ > net/bridge/br_device.c | 2 + > net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++--- > net/bridge/br_if.c | 14 ++++-- > net/bridge/br_private.h | 3 ++ > net/core/dev.c | 1 + > net/core/dev_addr_lists.c | 14 +++--- > 7 files changed, 134 insertions(+), 16 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 440a02e..e29cce1 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev); > void unregister_netdev(struct net_device *dev); > > /* General hardware address lists handling functions */ > +int __hw_addr_add(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type); > +int __hw_addr_del(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type); > int __hw_addr_sync(struct netdev_hw_addr_list *to_list, > struct netdev_hw_addr_list *from_list, int addr_len); > void __hw_addr_unsync(struct netdev_hw_addr_list *to_list, > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index 63f0455..1521db6 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev) > u64_stats_init(&br_dev_stats->syncp); > } > > + __hw_addr_init(&br->conf_addrs); > + > return 0; > } > > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index 9203d5a..ef95e81 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > static void fdb_notify(struct net_bridge *br, > const struct net_bridge_fdb_entry *, int); > > +static int br_addr_add(struct net_bridge *br, const unsigned char *addr); > +static int br_addr_del(struct net_bridge *br, const unsigned char *addr); > + > static u32 fdb_salt __read_mostly; > > int __init br_fdb_init(void) > @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head) > > static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) > { > + if (f->is_static) > + br_addr_del(br, f->addr.addr); > + > hlist_del_rcu(&f->hlist); > fdb_notify(br, f, RTM_DELNEIGH); > call_rcu(&f->rcu, fdb_rcu_free); > @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > return -ENOMEM; > > fdb->is_local = fdb->is_static = 1; > + br_addr_add(br, addr); > fdb_notify(br, fdb, RTM_NEWNEIGH); > return 0; > } > @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, > } > > if (fdb_to_nud(fdb) != state) { > - if (state & NUD_PERMANENT) > - fdb->is_local = fdb->is_static = 1; > - else if (state & NUD_NOARP) { > + if (state & NUD_PERMANENT) { > + fdb->is_local = 1; > + if (!fdb->is_static) { > + fdb->is_static = 1; > + br_addr_add(br, addr); > + } > + } else if (state & NUD_NOARP) { > + fdb->is_local = 0; > + if (!fdb->is_static) { > + fdb->is_static = 1; > + br_addr_add(br, addr); > + } > + } else { > fdb->is_local = 0; > - fdb->is_static = 1; > - } else > - fdb->is_local = fdb->is_static = 0; > + if (fdb->is_static) { > + fdb->is_static = 0; > + br_addr_del(br, addr); > + } > + } > > modified = true; > } > + > fdb->added_by_user = 1; > > fdb->used = jiffies; > @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], > out: > return err; > } > + > + > +/* Sync the current list to the correct flood port. */ > +void br_fdb_addrs_sync(struct net_bridge *br) > +{ > + struct net_bridge_port *p; > + int err; > + > + /* This function has to run under RTNL. > + * Program the user added addresses into the proper port. This > + * fuction follows the following algorithm: > + * a) If only 1 port is flooding: > + * - write all the addresses to this one port. > + */ > + if (br->n_flood_ports == 1) { > + p = br->c_flood_port; > + netif_addr_lock(p->dev); > + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs, > + p->dev->addr_len); > + if (!err) > + __dev_set_rx_mode(p->dev); > + netif_addr_unlock(p->dev); > + > + } > +} > + > +void br_fdb_addrs_unsync(struct net_bridge *br) > +{ > + struct net_bridge_port *p = br->c_flood_port; > + > + if (!p) > + return; > + > + netif_addr_lock(p->dev); > + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len); > + __dev_set_rx_mode(p->dev); > + netif_addr_unlock(p->dev); > +} > + > +/* When a static FDB entry is added, the mac address from the entry is > + * added to the bridge private HW address list and all required ports > + * are then updated with the new information. > + * Called under RTNL. > + */ > +static int br_addr_add(struct net_bridge *br, const unsigned char *addr) > +{ > + int err; > + > + ASSERT_RTNL(); > + > + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len, > + NETDEV_HW_ADDR_T_UNICAST); > + > + if (!err) > + br_fdb_addrs_sync(br); > + > + return err; > +} > + > +/* When a static FDB entry is deleted, the HW address from that entry is > + * also removed from the bridge private HW address list and updates all > + * the ports with needed information. > + * Called under RTNL. > + */ > +static int br_addr_del(struct net_bridge *br, const unsigned char *addr) > +{ > + int err; > + > + ASSERT_RTNL(); > + > + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len, > + NETDEV_HW_ADDR_T_UNICAST); > + if (!err) > + br_fdb_addrs_sync(br); > + > + return err; > +} > + > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index f072b34..e782c2e 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) > > br_ifinfo_notify(RTM_DELLINK, p); > > - list_del_rcu(&p->list); > + dev->priv_flags &= ~IFF_BRIDGE_PORT; > > if (p->flags & BR_FLOOD) > br_del_flood_port(p, br); > @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) > nbp_vlan_flush(p); > br_fdb_delete_by_port(br, p, 1); > > - dev->priv_flags &= ~IFF_BRIDGE_PORT; > + list_del_rcu(&p->list); > > netdev_rx_handler_unregister(dev); > > @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) > br->n_flood_ports++; > if (br->n_flood_ports == 1) > br->c_flood_port = p; > + > + br_fdb_addrs_sync(br); > } > > static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > * set it if it is not set. > */ > br->n_flood_ports--; > - if (p == br->c_flood_port) > + if (p == br->c_flood_port) { > + br_fdb_addrs_unsync(br); > br->c_flood_port = NULL; > + } > > if (br->n_flood_ports == 1) { > list_for_each_entry(port, &p->br->port_list, list) { > - if (port->flags & BR_FLOOD) { > + if (br_port_exists(port->dev) && > + (port->flags & BR_FLOOD)) { > br->c_flood_port = port; > + br_fdb_addrs_sync(br); > break; > } > } > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 26a3987..40a6927 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -296,6 +296,7 @@ struct net_bridge > u8 vlan_enabled; > struct net_port_vlans __rcu *vlan_info; > #endif > + struct netdev_hw_addr_list conf_addrs; > }; > > struct br_input_skb_cb { > @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, > const unsigned char *addr, u16 nlh_flags); > int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, > struct net_device *dev, int idx); > +void br_fdb_addrs_sync(struct net_bridge *br); > +void br_fdb_addrs_unsync(struct net_bridge *br); > > /* br_forward.c */ > void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); > diff --git a/net/core/dev.c b/net/core/dev.c > index 4ad1b78..eca4d476 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) > if (ops->ndo_set_rx_mode) > ops->ndo_set_rx_mode(dev); > } > +EXPORT_SYMBOL(__dev_set_rx_mode); > > void dev_set_rx_mode(struct net_device *dev) > { > diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c > index 329d579..3de44a3 100644 > --- a/net/core/dev_addr_lists.c > +++ b/net/core/dev_addr_lists.c > @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > sync); > } > > -static int __hw_addr_add(struct netdev_hw_addr_list *list, > - const unsigned char *addr, int addr_len, > - unsigned char addr_type) > +int __hw_addr_add(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type) > { > return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, > 0); > } > +EXPORT_SYMBOL(__hw_addr_add); > > static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > struct netdev_hw_addr *ha, bool global, > @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, > return -ENOENT; > } > > -static int __hw_addr_del(struct netdev_hw_addr_list *list, > - const unsigned char *addr, int addr_len, > - unsigned char addr_type) > +int __hw_addr_del(struct netdev_hw_addr_list *list, > + const unsigned char *addr, int addr_len, > + unsigned char addr_type) > { > return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); > } > +EXPORT_SYMBOL(__hw_addr_del); > > static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, > struct netdev_hw_addr *ha, > -- > 1.8.5.3 -- 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 Wed, 26 Feb 2014 10:18:21 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote: > When a static fdb entry is created, add the mac address to the bridge > address list. This list is used to program the proper port's > address list. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> I don't like this level of bookkeeping it starts to mix layers between the bridge network interface as entity for talking to the local host, and forwarding table entries. Many times static entries are used as alternative to flooding in environments which don't trust STP. Plus, it looks like another major source of bugs. -- 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 02/26/2014 11:23 AM, Michael S. Tsirkin wrote: > On Wed, Feb 26, 2014 at 10:18:21AM -0500, Vlad Yasevich wrote: >> When a static fdb entry is created, add the mac address to the bridge >> address list. This list is used to program the proper port's >> address list. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > Hmm won't this mean learned addresses are missing there? Learning should be disabled on interfaces that disable flood. The combination of learning without flood is problematic in cases where fdbs timeout before arp entries do. In such cases, arp will first try unicasts to validate the neighbor and these would be dropped. I've been thinking about automatically turning off learning when flood is turned off. What I don't like about it is that it needs extra state to reverse the process. We could log a warning, but that requires people to read logs... > And if so isn't this a problem when we use this list > in the next patch? > > I guess we could limit this to configurations where > learning is also disabled (not just flood) - > seems a bit arbitrary but will likely cover > most use-cases. > Right. From the start we wanted manual configuration here. Doing this with learning will open us to remote mac filter exhaustion attacks. -vlad > >> --- >> include/linux/netdevice.h | 6 +++ >> net/bridge/br_device.c | 2 + >> net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++--- >> net/bridge/br_if.c | 14 ++++-- >> net/bridge/br_private.h | 3 ++ >> net/core/dev.c | 1 + >> net/core/dev_addr_lists.c | 14 +++--- >> 7 files changed, 134 insertions(+), 16 deletions(-) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index 440a02e..e29cce1 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev); >> void unregister_netdev(struct net_device *dev); >> >> /* General hardware address lists handling functions */ >> +int __hw_addr_add(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type); >> +int __hw_addr_del(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type); >> int __hw_addr_sync(struct netdev_hw_addr_list *to_list, >> struct netdev_hw_addr_list *from_list, int addr_len); >> void __hw_addr_unsync(struct netdev_hw_addr_list *to_list, >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >> index 63f0455..1521db6 100644 >> --- a/net/bridge/br_device.c >> +++ b/net/bridge/br_device.c >> @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev) >> u64_stats_init(&br_dev_stats->syncp); >> } >> >> + __hw_addr_init(&br->conf_addrs); >> + >> return 0; >> } >> >> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c >> index 9203d5a..ef95e81 100644 >> --- a/net/bridge/br_fdb.c >> +++ b/net/bridge/br_fdb.c >> @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, >> static void fdb_notify(struct net_bridge *br, >> const struct net_bridge_fdb_entry *, int); >> >> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr); >> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr); >> + >> static u32 fdb_salt __read_mostly; >> >> int __init br_fdb_init(void) >> @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head) >> >> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) >> { >> + if (f->is_static) >> + br_addr_del(br, f->addr.addr); >> + >> hlist_del_rcu(&f->hlist); >> fdb_notify(br, f, RTM_DELNEIGH); >> call_rcu(&f->rcu, fdb_rcu_free); >> @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, >> return -ENOMEM; >> >> fdb->is_local = fdb->is_static = 1; >> + br_addr_add(br, addr); >> fdb_notify(br, fdb, RTM_NEWNEIGH); >> return 0; >> } >> @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, >> } >> >> if (fdb_to_nud(fdb) != state) { >> - if (state & NUD_PERMANENT) >> - fdb->is_local = fdb->is_static = 1; >> - else if (state & NUD_NOARP) { >> + if (state & NUD_PERMANENT) { >> + fdb->is_local = 1; >> + if (!fdb->is_static) { >> + fdb->is_static = 1; >> + br_addr_add(br, addr); >> + } >> + } else if (state & NUD_NOARP) { >> + fdb->is_local = 0; >> + if (!fdb->is_static) { >> + fdb->is_static = 1; >> + br_addr_add(br, addr); >> + } >> + } else { >> fdb->is_local = 0; >> - fdb->is_static = 1; >> - } else >> - fdb->is_local = fdb->is_static = 0; >> + if (fdb->is_static) { >> + fdb->is_static = 0; >> + br_addr_del(br, addr); >> + } >> + } >> >> modified = true; >> } >> + >> fdb->added_by_user = 1; >> >> fdb->used = jiffies; >> @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], >> out: >> return err; >> } >> + >> + >> +/* Sync the current list to the correct flood port. */ >> +void br_fdb_addrs_sync(struct net_bridge *br) >> +{ >> + struct net_bridge_port *p; >> + int err; >> + >> + /* This function has to run under RTNL. >> + * Program the user added addresses into the proper port. This >> + * fuction follows the following algorithm: >> + * a) If only 1 port is flooding: >> + * - write all the addresses to this one port. >> + */ >> + if (br->n_flood_ports == 1) { >> + p = br->c_flood_port; >> + netif_addr_lock(p->dev); >> + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs, >> + p->dev->addr_len); >> + if (!err) >> + __dev_set_rx_mode(p->dev); >> + netif_addr_unlock(p->dev); >> + >> + } >> +} >> + >> +void br_fdb_addrs_unsync(struct net_bridge *br) >> +{ >> + struct net_bridge_port *p = br->c_flood_port; >> + >> + if (!p) >> + return; >> + >> + netif_addr_lock(p->dev); >> + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len); >> + __dev_set_rx_mode(p->dev); >> + netif_addr_unlock(p->dev); >> +} >> + >> +/* When a static FDB entry is added, the mac address from the entry is >> + * added to the bridge private HW address list and all required ports >> + * are then updated with the new information. >> + * Called under RTNL. >> + */ >> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr) >> +{ >> + int err; >> + >> + ASSERT_RTNL(); >> + >> + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len, >> + NETDEV_HW_ADDR_T_UNICAST); >> + >> + if (!err) >> + br_fdb_addrs_sync(br); >> + >> + return err; >> +} >> + >> +/* When a static FDB entry is deleted, the HW address from that entry is >> + * also removed from the bridge private HW address list and updates all >> + * the ports with needed information. >> + * Called under RTNL. >> + */ >> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr) >> +{ >> + int err; >> + >> + ASSERT_RTNL(); >> + >> + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len, >> + NETDEV_HW_ADDR_T_UNICAST); >> + if (!err) >> + br_fdb_addrs_sync(br); >> + >> + return err; >> +} >> + >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c >> index f072b34..e782c2e 100644 >> --- a/net/bridge/br_if.c >> +++ b/net/bridge/br_if.c >> @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) >> >> br_ifinfo_notify(RTM_DELLINK, p); >> >> - list_del_rcu(&p->list); >> + dev->priv_flags &= ~IFF_BRIDGE_PORT; >> >> if (p->flags & BR_FLOOD) >> br_del_flood_port(p, br); >> @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) >> nbp_vlan_flush(p); >> br_fdb_delete_by_port(br, p, 1); >> >> - dev->priv_flags &= ~IFF_BRIDGE_PORT; >> + list_del_rcu(&p->list); >> >> netdev_rx_handler_unregister(dev); >> >> @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> br->n_flood_ports++; >> if (br->n_flood_ports == 1) >> br->c_flood_port = p; >> + >> + br_fdb_addrs_sync(br); >> } >> >> static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) >> * set it if it is not set. >> */ >> br->n_flood_ports--; >> - if (p == br->c_flood_port) >> + if (p == br->c_flood_port) { >> + br_fdb_addrs_unsync(br); >> br->c_flood_port = NULL; >> + } >> >> if (br->n_flood_ports == 1) { >> list_for_each_entry(port, &p->br->port_list, list) { >> - if (port->flags & BR_FLOOD) { >> + if (br_port_exists(port->dev) && >> + (port->flags & BR_FLOOD)) { >> br->c_flood_port = port; >> + br_fdb_addrs_sync(br); >> break; >> } >> } >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index 26a3987..40a6927 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -296,6 +296,7 @@ struct net_bridge >> u8 vlan_enabled; >> struct net_port_vlans __rcu *vlan_info; >> #endif >> + struct netdev_hw_addr_list conf_addrs; >> }; >> >> struct br_input_skb_cb { >> @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, >> const unsigned char *addr, u16 nlh_flags); >> int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, >> struct net_device *dev, int idx); >> +void br_fdb_addrs_sync(struct net_bridge *br); >> +void br_fdb_addrs_unsync(struct net_bridge *br); >> >> /* br_forward.c */ >> void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); >> diff --git a/net/core/dev.c b/net/core/dev.c >> index 4ad1b78..eca4d476 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) >> if (ops->ndo_set_rx_mode) >> ops->ndo_set_rx_mode(dev); >> } >> +EXPORT_SYMBOL(__dev_set_rx_mode); >> >> void dev_set_rx_mode(struct net_device *dev) >> { >> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c >> index 329d579..3de44a3 100644 >> --- a/net/core/dev_addr_lists.c >> +++ b/net/core/dev_addr_lists.c >> @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, >> sync); >> } >> >> -static int __hw_addr_add(struct netdev_hw_addr_list *list, >> - const unsigned char *addr, int addr_len, >> - unsigned char addr_type) >> +int __hw_addr_add(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type) >> { >> return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, >> 0); >> } >> +EXPORT_SYMBOL(__hw_addr_add); >> >> static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, >> struct netdev_hw_addr *ha, bool global, >> @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, >> return -ENOENT; >> } >> >> -static int __hw_addr_del(struct netdev_hw_addr_list *list, >> - const unsigned char *addr, int addr_len, >> - unsigned char addr_type) >> +int __hw_addr_del(struct netdev_hw_addr_list *list, >> + const unsigned char *addr, int addr_len, >> + unsigned char addr_type) >> { >> return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); >> } >> +EXPORT_SYMBOL(__hw_addr_del); >> >> static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, >> struct netdev_hw_addr *ha, >> -- >> 1.8.5.3 -- 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 Wed, Feb 26, 2014 at 12:25:53PM -0500, Vlad Yasevich wrote: > On 02/26/2014 11:23 AM, Michael S. Tsirkin wrote: > > On Wed, Feb 26, 2014 at 10:18:21AM -0500, Vlad Yasevich wrote: > >> When a static fdb entry is created, add the mac address to the bridge > >> address list. This list is used to program the proper port's > >> address list. > >> > >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > > > Hmm won't this mean learned addresses are missing there? > > Learning should be disabled on interfaces that disable flood. > The combination of learning without flood is problematic in > cases where fdbs timeout before arp entries do. > In such cases, arp will first try unicasts to validate the neighbor > and these would be dropped. > > I've been thinking about automatically turning off learning > when flood is turned off. What I don't like about it is > that it needs extra state to reverse the process. > We could log a warning, but that requires people to read logs... For now maybe just keep promisc enabled in this configuration. > > And if so isn't this a problem when we use this list > > in the next patch? > > > > I guess we could limit this to configurations where > > learning is also disabled (not just flood) - > > seems a bit arbitrary but will likely cover > > most use-cases. > > > > Right. From the start we wanted manual configuration here. > Doing this with learning will open us to remote mac filter > exhaustion attacks. > > -vlad > > > > >> --- > >> include/linux/netdevice.h | 6 +++ > >> net/bridge/br_device.c | 2 + > >> net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++--- > >> net/bridge/br_if.c | 14 ++++-- > >> net/bridge/br_private.h | 3 ++ > >> net/core/dev.c | 1 + > >> net/core/dev_addr_lists.c | 14 +++--- > >> 7 files changed, 134 insertions(+), 16 deletions(-) > >> > >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > >> index 440a02e..e29cce1 100644 > >> --- a/include/linux/netdevice.h > >> +++ b/include/linux/netdevice.h > >> @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev); > >> void unregister_netdev(struct net_device *dev); > >> > >> /* General hardware address lists handling functions */ > >> +int __hw_addr_add(struct netdev_hw_addr_list *list, > >> + const unsigned char *addr, int addr_len, > >> + unsigned char addr_type); > >> +int __hw_addr_del(struct netdev_hw_addr_list *list, > >> + const unsigned char *addr, int addr_len, > >> + unsigned char addr_type); > >> int __hw_addr_sync(struct netdev_hw_addr_list *to_list, > >> struct netdev_hw_addr_list *from_list, int addr_len); > >> void __hw_addr_unsync(struct netdev_hw_addr_list *to_list, > >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > >> index 63f0455..1521db6 100644 > >> --- a/net/bridge/br_device.c > >> +++ b/net/bridge/br_device.c > >> @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev) > >> u64_stats_init(&br_dev_stats->syncp); > >> } > >> > >> + __hw_addr_init(&br->conf_addrs); > >> + > >> return 0; > >> } > >> > >> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > >> index 9203d5a..ef95e81 100644 > >> --- a/net/bridge/br_fdb.c > >> +++ b/net/bridge/br_fdb.c > >> @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > >> static void fdb_notify(struct net_bridge *br, > >> const struct net_bridge_fdb_entry *, int); > >> > >> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr); > >> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr); > >> + > >> static u32 fdb_salt __read_mostly; > >> > >> int __init br_fdb_init(void) > >> @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head) > >> > >> static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) > >> { > >> + if (f->is_static) > >> + br_addr_del(br, f->addr.addr); > >> + > >> hlist_del_rcu(&f->hlist); > >> fdb_notify(br, f, RTM_DELNEIGH); > >> call_rcu(&f->rcu, fdb_rcu_free); > >> @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, > >> return -ENOMEM; > >> > >> fdb->is_local = fdb->is_static = 1; > >> + br_addr_add(br, addr); > >> fdb_notify(br, fdb, RTM_NEWNEIGH); > >> return 0; > >> } > >> @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, > >> } > >> > >> if (fdb_to_nud(fdb) != state) { > >> - if (state & NUD_PERMANENT) > >> - fdb->is_local = fdb->is_static = 1; > >> - else if (state & NUD_NOARP) { > >> + if (state & NUD_PERMANENT) { > >> + fdb->is_local = 1; > >> + if (!fdb->is_static) { > >> + fdb->is_static = 1; > >> + br_addr_add(br, addr); > >> + } > >> + } else if (state & NUD_NOARP) { > >> + fdb->is_local = 0; > >> + if (!fdb->is_static) { > >> + fdb->is_static = 1; > >> + br_addr_add(br, addr); > >> + } > >> + } else { > >> fdb->is_local = 0; > >> - fdb->is_static = 1; > >> - } else > >> - fdb->is_local = fdb->is_static = 0; > >> + if (fdb->is_static) { > >> + fdb->is_static = 0; > >> + br_addr_del(br, addr); > >> + } > >> + } > >> > >> modified = true; > >> } > >> + > >> fdb->added_by_user = 1; > >> > >> fdb->used = jiffies; > >> @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], > >> out: > >> return err; > >> } > >> + > >> + > >> +/* Sync the current list to the correct flood port. */ > >> +void br_fdb_addrs_sync(struct net_bridge *br) > >> +{ > >> + struct net_bridge_port *p; > >> + int err; > >> + > >> + /* This function has to run under RTNL. > >> + * Program the user added addresses into the proper port. This > >> + * fuction follows the following algorithm: > >> + * a) If only 1 port is flooding: > >> + * - write all the addresses to this one port. > >> + */ > >> + if (br->n_flood_ports == 1) { > >> + p = br->c_flood_port; > >> + netif_addr_lock(p->dev); > >> + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs, > >> + p->dev->addr_len); > >> + if (!err) > >> + __dev_set_rx_mode(p->dev); > >> + netif_addr_unlock(p->dev); > >> + > >> + } > >> +} > >> + > >> +void br_fdb_addrs_unsync(struct net_bridge *br) > >> +{ > >> + struct net_bridge_port *p = br->c_flood_port; > >> + > >> + if (!p) > >> + return; > >> + > >> + netif_addr_lock(p->dev); > >> + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len); > >> + __dev_set_rx_mode(p->dev); > >> + netif_addr_unlock(p->dev); > >> +} > >> + > >> +/* When a static FDB entry is added, the mac address from the entry is > >> + * added to the bridge private HW address list and all required ports > >> + * are then updated with the new information. > >> + * Called under RTNL. > >> + */ > >> +static int br_addr_add(struct net_bridge *br, const unsigned char *addr) > >> +{ > >> + int err; > >> + > >> + ASSERT_RTNL(); > >> + > >> + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len, > >> + NETDEV_HW_ADDR_T_UNICAST); > >> + > >> + if (!err) > >> + br_fdb_addrs_sync(br); > >> + > >> + return err; > >> +} > >> + > >> +/* When a static FDB entry is deleted, the HW address from that entry is > >> + * also removed from the bridge private HW address list and updates all > >> + * the ports with needed information. > >> + * Called under RTNL. > >> + */ > >> +static int br_addr_del(struct net_bridge *br, const unsigned char *addr) > >> +{ > >> + int err; > >> + > >> + ASSERT_RTNL(); > >> + > >> + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len, > >> + NETDEV_HW_ADDR_T_UNICAST); > >> + if (!err) > >> + br_fdb_addrs_sync(br); > >> + > >> + return err; > >> +} > >> + > >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > >> index f072b34..e782c2e 100644 > >> --- a/net/bridge/br_if.c > >> +++ b/net/bridge/br_if.c > >> @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) > >> > >> br_ifinfo_notify(RTM_DELLINK, p); > >> > >> - list_del_rcu(&p->list); > >> + dev->priv_flags &= ~IFF_BRIDGE_PORT; > >> > >> if (p->flags & BR_FLOOD) > >> br_del_flood_port(p, br); > >> @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) > >> nbp_vlan_flush(p); > >> br_fdb_delete_by_port(br, p, 1); > >> > >> - dev->priv_flags &= ~IFF_BRIDGE_PORT; > >> + list_del_rcu(&p->list); > >> > >> netdev_rx_handler_unregister(dev); > >> > >> @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) > >> br->n_flood_ports++; > >> if (br->n_flood_ports == 1) > >> br->c_flood_port = p; > >> + > >> + br_fdb_addrs_sync(br); > >> } > >> > >> static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > >> @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) > >> * set it if it is not set. > >> */ > >> br->n_flood_ports--; > >> - if (p == br->c_flood_port) > >> + if (p == br->c_flood_port) { > >> + br_fdb_addrs_unsync(br); > >> br->c_flood_port = NULL; > >> + } > >> > >> if (br->n_flood_ports == 1) { > >> list_for_each_entry(port, &p->br->port_list, list) { > >> - if (port->flags & BR_FLOOD) { > >> + if (br_port_exists(port->dev) && > >> + (port->flags & BR_FLOOD)) { > >> br->c_flood_port = port; > >> + br_fdb_addrs_sync(br); > >> break; > >> } > >> } > >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > >> index 26a3987..40a6927 100644 > >> --- a/net/bridge/br_private.h > >> +++ b/net/bridge/br_private.h > >> @@ -296,6 +296,7 @@ struct net_bridge > >> u8 vlan_enabled; > >> struct net_port_vlans __rcu *vlan_info; > >> #endif > >> + struct netdev_hw_addr_list conf_addrs; > >> }; > >> > >> struct br_input_skb_cb { > >> @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, > >> const unsigned char *addr, u16 nlh_flags); > >> int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, > >> struct net_device *dev, int idx); > >> +void br_fdb_addrs_sync(struct net_bridge *br); > >> +void br_fdb_addrs_unsync(struct net_bridge *br); > >> > >> /* br_forward.c */ > >> void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); > >> diff --git a/net/core/dev.c b/net/core/dev.c > >> index 4ad1b78..eca4d476 100644 > >> --- a/net/core/dev.c > >> +++ b/net/core/dev.c > >> @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) > >> if (ops->ndo_set_rx_mode) > >> ops->ndo_set_rx_mode(dev); > >> } > >> +EXPORT_SYMBOL(__dev_set_rx_mode); > >> > >> void dev_set_rx_mode(struct net_device *dev) > >> { > >> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c > >> index 329d579..3de44a3 100644 > >> --- a/net/core/dev_addr_lists.c > >> +++ b/net/core/dev_addr_lists.c > >> @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > >> sync); > >> } > >> > >> -static int __hw_addr_add(struct netdev_hw_addr_list *list, > >> - const unsigned char *addr, int addr_len, > >> - unsigned char addr_type) > >> +int __hw_addr_add(struct netdev_hw_addr_list *list, > >> + const unsigned char *addr, int addr_len, > >> + unsigned char addr_type) > >> { > >> return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, > >> 0); > >> } > >> +EXPORT_SYMBOL(__hw_addr_add); > >> > >> static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > >> struct netdev_hw_addr *ha, bool global, > >> @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, > >> return -ENOENT; > >> } > >> > >> -static int __hw_addr_del(struct netdev_hw_addr_list *list, > >> - const unsigned char *addr, int addr_len, > >> - unsigned char addr_type) > >> +int __hw_addr_del(struct netdev_hw_addr_list *list, > >> + const unsigned char *addr, int addr_len, > >> + unsigned char addr_type) > >> { > >> return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); > >> } > >> +EXPORT_SYMBOL(__hw_addr_del); > >> > >> static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, > >> struct netdev_hw_addr *ha, > >> -- > >> 1.8.5.3 -- 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 02/26/2014 11:57 AM, Stephen Hemminger wrote: > On Wed, 26 Feb 2014 10:18:21 -0500 > Vlad Yasevich <vyasevic@redhat.com> wrote: > >> When a static fdb entry is created, add the mac address to the bridge >> address list. This list is used to program the proper port's >> address list. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > I don't like this level of bookkeeping it starts to mix > layers between the bridge network interface as entity for talking to the > local host, and forwarding table entries. Actually this is one of the reasons this isn't done through the br->dev->uc. Forwarding table entries are still per-port. > > Many times static entries are used as alternative to flooding in > environments which don't trust STP. Ok, and how would this be problematic? If one wants to turn off promisc in this environment, then receive filters needs to be properly programmed. > > Plus, it looks like another major source of bugs. > Any new code is a potential source of issues. Are you saying No to any new code in bridge? -vlad -- 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 Wed, Feb 26, 2014 at 12:35:08PM -0500, Vlad Yasevich wrote: > On 02/26/2014 11:57 AM, Stephen Hemminger wrote: > > On Wed, 26 Feb 2014 10:18:21 -0500 > > Vlad Yasevich <vyasevic@redhat.com> wrote: > > > >> When a static fdb entry is created, add the mac address to the bridge > >> address list. This list is used to program the proper port's > >> address list. > >> > >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > > > I don't like this level of bookkeeping it starts to mix > > layers between the bridge network interface as entity for talking to the > > local host, and forwarding table entries. > > Actually this is one of the reasons this isn't done through the > br->dev->uc. Forwarding table entries are still per-port. > > > > > Many times static entries are used as alternative to flooding in > > environments which don't trust STP. > > Ok, and how would this be problematic? If one wants to turn off > promisc in this environment, then receive filters needs to be properly > programmed. > > > > > Plus, it looks like another major source of bugs. > > > > Any new code is a potential source of issues. Are you saying > No to any new code in bridge? > > -vlad I'm guessing Stephen merely worries about multiple data structures that need to stay in sync, and asks that you revisit using private hw address list in the bridge. What's the issue with walking fdb exactly? You say 1) I tried using the fdb table itself as main repository, but this caused difficulties in synchronizing this table with the interface filters later on. I'm guessing you refer to writing addresses out to ports directly when walking the hash being impossible since this datastructure uses rcu and spinlocks? Fair enough but the entries you care about seem to only be modified under RTNL so just copy them out to a temporary list. This might be less efficient, but will be simpler I think.
On 02/27/2014 02:53 AM, Michael S. Tsirkin wrote: > On Wed, Feb 26, 2014 at 12:35:08PM -0500, Vlad Yasevich wrote: >> On 02/26/2014 11:57 AM, Stephen Hemminger wrote: >>> On Wed, 26 Feb 2014 10:18:21 -0500 >>> Vlad Yasevich <vyasevic@redhat.com> wrote: >>> >>>> When a static fdb entry is created, add the mac address to the bridge >>>> address list. This list is used to program the proper port's >>>> address list. >>>> >>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >>> >>> I don't like this level of bookkeeping it starts to mix >>> layers between the bridge network interface as entity for talking to the >>> local host, and forwarding table entries. >> >> Actually this is one of the reasons this isn't done through the >> br->dev->uc. Forwarding table entries are still per-port. >> >>> >>> Many times static entries are used as alternative to flooding in >>> environments which don't trust STP. >> >> Ok, and how would this be problematic? If one wants to turn off >> promisc in this environment, then receive filters needs to be properly >> programmed. >> >>> >>> Plus, it looks like another major source of bugs. >>> >> >> Any new code is a potential source of issues. Are you saying >> No to any new code in bridge? >> >> -vlad > > I'm guessing Stephen merely worries about > multiple data structures that need to stay in > sync, and asks that you revisit > using private hw address list in the bridge. > > What's the issue with walking fdb exactly? > You say > 1) I tried using the fdb table itself as main repository, but > this caused difficulties in synchronizing this table with > the interface filters later on. > > I'm guessing you refer to writing addresses out to ports > directly when walking the hash being impossible > since this datastructure uses rcu and spinlocks? > Fair enough but the entries you care about > seem to only be modified under RTNL so just > copy them out to a temporary list. > This might be less efficient, but will be simpler I think. > There are 2 ways to populate the the ports uc list. 1) We can use dev_uc_add() directly. The issue here is how to know if a given entry has been written to port. I've played with this is and we end completely replicating the netdev_hw_addr functionality in fdb to support the Patch7 (0 flooding ports). 2) We can use dev_uc_sync() which is what this series does. This api needs to keep track of sync counts so that things get properly deleted. For that a temporary list will not work since you'd be re-creating it every time. Now, I think I've come up with a way to remove the private address list and use bridge->dev->uc, but that requires that we implement fdb-based filtering for local addresses. The idea is to implement learning on the bridge device xmit path. This will support things like vlans on top of the bridge that change their mac, or even stack bridge configs that exist in the wild. My guess, however, is that Stephen would have an even bigger issue with this. ;) -vlad -- 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, Feb 27, 2014 at 08:08:05AM -0500, Vlad Yasevich wrote: > On 02/27/2014 02:53 AM, Michael S. Tsirkin wrote: > > On Wed, Feb 26, 2014 at 12:35:08PM -0500, Vlad Yasevich wrote: > >> On 02/26/2014 11:57 AM, Stephen Hemminger wrote: > >>> On Wed, 26 Feb 2014 10:18:21 -0500 > >>> Vlad Yasevich <vyasevic@redhat.com> wrote: > >>> > >>>> When a static fdb entry is created, add the mac address to the bridge > >>>> address list. This list is used to program the proper port's > >>>> address list. > >>>> > >>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > >>> > >>> I don't like this level of bookkeeping it starts to mix > >>> layers between the bridge network interface as entity for talking to the > >>> local host, and forwarding table entries. > >> > >> Actually this is one of the reasons this isn't done through the > >> br->dev->uc. Forwarding table entries are still per-port. > >> > >>> > >>> Many times static entries are used as alternative to flooding in > >>> environments which don't trust STP. > >> > >> Ok, and how would this be problematic? If one wants to turn off > >> promisc in this environment, then receive filters needs to be properly > >> programmed. > >> > >>> > >>> Plus, it looks like another major source of bugs. > >>> > >> > >> Any new code is a potential source of issues. Are you saying > >> No to any new code in bridge? > >> > >> -vlad > > > > I'm guessing Stephen merely worries about > > multiple data structures that need to stay in > > sync, and asks that you revisit > > using private hw address list in the bridge. > > > > What's the issue with walking fdb exactly? > > You say > > 1) I tried using the fdb table itself as main repository, but > > this caused difficulties in synchronizing this table with > > the interface filters later on. > > > > I'm guessing you refer to writing addresses out to ports > > directly when walking the hash being impossible > > since this datastructure uses rcu and spinlocks? > > Fair enough but the entries you care about > > seem to only be modified under RTNL so just > > copy them out to a temporary list. > > This might be less efficient, but will be simpler I think. > > > > There are 2 ways to populate the the ports uc list. > 1) We can use dev_uc_add() directly. The issue here is > how to know if a given entry has been written to port. > I've played with this is and we end completely replicating > the netdev_hw_addr functionality in fdb to support the Patch7 > (0 flooding ports). I think I begin to understand. But I still think dev_uc_add is the way to go here. Simply build the list twice, before and after the change. On a change, first dev_uc_add all addresses in the new list, then dev_uc_del all addresses in the old list. > 2) We can use dev_uc_sync() which is what this series does. > This api needs to keep track of sync counts so that things get > properly deleted. For that a temporary list will not work > since you'd be re-creating it every time. > > Now, I think I've come up with a way to remove the private address list > and use bridge->dev->uc, but that requires that we implement fdb-based > filtering for local addresses. > > The idea is to implement learning on the bridge device xmit path. This > will support things like vlans on top of the bridge that change their > mac, or even stack bridge configs that exist in the wild. > My guess, however, is that Stephen would have an even bigger issue with > this. ;) > > -vlad It sounds like a nifty feature without respect to whether it's the right thing to do for the non promisc feature.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 440a02e..e29cce1 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2881,6 +2881,12 @@ int register_netdev(struct net_device *dev); void unregister_netdev(struct net_device *dev); /* General hardware address lists handling functions */ +int __hw_addr_add(struct netdev_hw_addr_list *list, + const unsigned char *addr, int addr_len, + unsigned char addr_type); +int __hw_addr_del(struct netdev_hw_addr_list *list, + const unsigned char *addr, int addr_len, + unsigned char addr_type); int __hw_addr_sync(struct netdev_hw_addr_list *to_list, struct netdev_hw_addr_list *from_list, int addr_len); void __hw_addr_unsync(struct netdev_hw_addr_list *to_list, diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 63f0455..1521db6 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -100,6 +100,8 @@ static int br_dev_init(struct net_device *dev) u64_stats_init(&br_dev_stats->syncp); } + __hw_addr_init(&br->conf_addrs); + return 0; } diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 9203d5a..ef95e81 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -35,6 +35,9 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, static void fdb_notify(struct net_bridge *br, const struct net_bridge_fdb_entry *, int); +static int br_addr_add(struct net_bridge *br, const unsigned char *addr); +static int br_addr_del(struct net_bridge *br, const unsigned char *addr); + static u32 fdb_salt __read_mostly; int __init br_fdb_init(void) @@ -87,6 +90,9 @@ static void fdb_rcu_free(struct rcu_head *head) static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) { + if (f->is_static) + br_addr_del(br, f->addr.addr); + hlist_del_rcu(&f->hlist); fdb_notify(br, f, RTM_DELNEIGH); call_rcu(&f->rcu, fdb_rcu_free); @@ -466,6 +472,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source, return -ENOMEM; fdb->is_local = fdb->is_static = 1; + br_addr_add(br, addr); fdb_notify(br, fdb, RTM_NEWNEIGH); return 0; } @@ -678,16 +685,29 @@ static int fdb_add_entry(struct net_bridge_port *source, const __u8 *addr, } if (fdb_to_nud(fdb) != state) { - if (state & NUD_PERMANENT) - fdb->is_local = fdb->is_static = 1; - else if (state & NUD_NOARP) { + if (state & NUD_PERMANENT) { + fdb->is_local = 1; + if (!fdb->is_static) { + fdb->is_static = 1; + br_addr_add(br, addr); + } + } else if (state & NUD_NOARP) { + fdb->is_local = 0; + if (!fdb->is_static) { + fdb->is_static = 1; + br_addr_add(br, addr); + } + } else { fdb->is_local = 0; - fdb->is_static = 1; - } else - fdb->is_local = fdb->is_static = 0; + if (fdb->is_static) { + fdb->is_static = 0; + br_addr_del(br, addr); + } + } modified = true; } + fdb->added_by_user = 1; fdb->used = jiffies; @@ -874,3 +894,81 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], out: return err; } + + +/* Sync the current list to the correct flood port. */ +void br_fdb_addrs_sync(struct net_bridge *br) +{ + struct net_bridge_port *p; + int err; + + /* This function has to run under RTNL. + * Program the user added addresses into the proper port. This + * fuction follows the following algorithm: + * a) If only 1 port is flooding: + * - write all the addresses to this one port. + */ + if (br->n_flood_ports == 1) { + p = br->c_flood_port; + netif_addr_lock(p->dev); + err = __hw_addr_sync(&p->dev->uc, &br->conf_addrs, + p->dev->addr_len); + if (!err) + __dev_set_rx_mode(p->dev); + netif_addr_unlock(p->dev); + + } +} + +void br_fdb_addrs_unsync(struct net_bridge *br) +{ + struct net_bridge_port *p = br->c_flood_port; + + if (!p) + return; + + netif_addr_lock(p->dev); + __hw_addr_unsync(&p->dev->uc, &br->conf_addrs, p->dev->addr_len); + __dev_set_rx_mode(p->dev); + netif_addr_unlock(p->dev); +} + +/* When a static FDB entry is added, the mac address from the entry is + * added to the bridge private HW address list and all required ports + * are then updated with the new information. + * Called under RTNL. + */ +static int br_addr_add(struct net_bridge *br, const unsigned char *addr) +{ + int err; + + ASSERT_RTNL(); + + err = __hw_addr_add(&br->conf_addrs, addr, br->dev->addr_len, + NETDEV_HW_ADDR_T_UNICAST); + + if (!err) + br_fdb_addrs_sync(br); + + return err; +} + +/* When a static FDB entry is deleted, the HW address from that entry is + * also removed from the bridge private HW address list and updates all + * the ports with needed information. + * Called under RTNL. + */ +static int br_addr_del(struct net_bridge *br, const unsigned char *addr) +{ + int err; + + ASSERT_RTNL(); + + err = __hw_addr_del(&br->conf_addrs, addr, br->dev->addr_len, + NETDEV_HW_ADDR_T_UNICAST); + if (!err) + br_fdb_addrs_sync(br); + + return err; +} + diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index f072b34..e782c2e 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -144,7 +144,7 @@ static void del_nbp(struct net_bridge_port *p) br_ifinfo_notify(RTM_DELLINK, p); - list_del_rcu(&p->list); + dev->priv_flags &= ~IFF_BRIDGE_PORT; if (p->flags & BR_FLOOD) br_del_flood_port(p, br); @@ -152,7 +152,7 @@ static void del_nbp(struct net_bridge_port *p) nbp_vlan_flush(p); br_fdb_delete_by_port(br, p, 1); - dev->priv_flags &= ~IFF_BRIDGE_PORT; + list_del_rcu(&p->list); netdev_rx_handler_unregister(dev); @@ -473,6 +473,8 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br) br->n_flood_ports++; if (br->n_flood_ports == 1) br->c_flood_port = p; + + br_fdb_addrs_sync(br); } static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) @@ -485,13 +487,17 @@ static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br) * set it if it is not set. */ br->n_flood_ports--; - if (p == br->c_flood_port) + if (p == br->c_flood_port) { + br_fdb_addrs_unsync(br); br->c_flood_port = NULL; + } if (br->n_flood_ports == 1) { list_for_each_entry(port, &p->br->port_list, list) { - if (port->flags & BR_FLOOD) { + if (br_port_exists(port->dev) && + (port->flags & BR_FLOOD)) { br->c_flood_port = port; + br_fdb_addrs_sync(br); break; } } diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 26a3987..40a6927 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -296,6 +296,7 @@ struct net_bridge u8 vlan_enabled; struct net_port_vlans __rcu *vlan_info; #endif + struct netdev_hw_addr_list conf_addrs; }; struct br_input_skb_cb { @@ -397,6 +398,8 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev, const unsigned char *addr, u16 nlh_flags); int br_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, struct net_device *dev, int idx); +void br_fdb_addrs_sync(struct net_bridge *br); +void br_fdb_addrs_unsync(struct net_bridge *br); /* br_forward.c */ void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb); diff --git a/net/core/dev.c b/net/core/dev.c index 4ad1b78..eca4d476 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -5212,6 +5212,7 @@ void __dev_set_rx_mode(struct net_device *dev) if (ops->ndo_set_rx_mode) ops->ndo_set_rx_mode(dev); } +EXPORT_SYMBOL(__dev_set_rx_mode); void dev_set_rx_mode(struct net_device *dev) { diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c index 329d579..3de44a3 100644 --- a/net/core/dev_addr_lists.c +++ b/net/core/dev_addr_lists.c @@ -81,13 +81,14 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, sync); } -static int __hw_addr_add(struct netdev_hw_addr_list *list, - const unsigned char *addr, int addr_len, - unsigned char addr_type) +int __hw_addr_add(struct netdev_hw_addr_list *list, + const unsigned char *addr, int addr_len, + unsigned char addr_type) { return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, 0); } +EXPORT_SYMBOL(__hw_addr_add); static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, struct netdev_hw_addr *ha, bool global, @@ -127,12 +128,13 @@ static int __hw_addr_del_ex(struct netdev_hw_addr_list *list, return -ENOENT; } -static int __hw_addr_del(struct netdev_hw_addr_list *list, - const unsigned char *addr, int addr_len, - unsigned char addr_type) +int __hw_addr_del(struct netdev_hw_addr_list *list, + const unsigned char *addr, int addr_len, + unsigned char addr_type) { return __hw_addr_del_ex(list, addr, addr_len, addr_type, false, false); } +EXPORT_SYMBOL(__hw_addr_del); static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, struct netdev_hw_addr *ha,
When a static fdb entry is created, add the mac address to the bridge address list. This list is used to program the proper port's address list. Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> --- include/linux/netdevice.h | 6 +++ net/bridge/br_device.c | 2 + net/bridge/br_fdb.c | 110 +++++++++++++++++++++++++++++++++++++++++++--- net/bridge/br_if.c | 14 ++++-- net/bridge/br_private.h | 3 ++ net/core/dev.c | 1 + net/core/dev_addr_lists.c | 14 +++--- 7 files changed, 134 insertions(+), 16 deletions(-)