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@xxxxxxxxxx>
> >
> > 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
