On Wed, Feb 26, 2014 at 10:46:20PM -0500, Vlad Yasevich wrote:
> On 02/26/2014 10:57 AM, Michael S. Tsirkin wrote:
> > On Wed, Feb 26, 2014 at 10:18:25AM -0500, Vlad Yasevich wrote:
> >> Configuration where all ports are set to non-flooding is a slight
> >> special case. In this config, the user would have to manage all fdbs
> >> for all ports. In this special case, since we'll know all addresses,
> >> we can turn off promisc on all ports and program all ports with the
> >> same set of addresses.
> >>
> >> Signed-off-by: Vlad Yasevich <vyasevic@xxxxxxxxxx>
> >
> > Here the logic is different be we could sync them,
> > just make this a per-port flag and
> > scan all ports instead of relying on flood_port.
> > This will be slower in the single flooding port
> > case but worst case remains the same since
> > this patch scans all ports.
> >
> > What do you think?
>
> I had it this way before and didn't really like it. The transitions
> between the two configs needs to be detected and appropriate
> actions taken. So it'll still be special coded.
>
> I'll see what I can do to simplify this.
>
> -vlad
Not really.
Basically if you have per-port flags p->promisc and p->flood
then the logic becomes:
int nr_flood = 0;
for each port (br, p) {
nr_flood += p->flood;
}
for each port (br, p) {
int promisc = nr_flood - p->flood;
if (p->promisc != promisc) {
dev_set_promisc(p, promisc);
dev_set_allmulti(p, !promisc);
}
}
Just re-run this whenever user tweaks any flood flags.
Replace p->flood with p->flood && p->learn if you
need to disable the optimization when learning
is on.
> >> ---
> >> include/linux/netdevice.h | 3 +++
> >> net/bridge/br_fdb.c | 22 +++++++++++++++++-----
> >> net/bridge/br_if.c | 40 ++++++++++++++++++++++++++++++++++------
> >> net/bridge/br_private.h | 2 +-
> >> net/core/dev_addr_lists.c | 7 ++++---
> >> 5 files changed, 59 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >> index e29cce1..79e97ee 100644
> >> --- a/include/linux/netdevice.h
> >> +++ b/include/linux/netdevice.h
> >> @@ -2889,6 +2889,9 @@ int __hw_addr_del(struct netdev_hw_addr_list *list,
> >> 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);
> >> +int __hw_addr_sync_multiple(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,
> >> struct netdev_hw_addr_list *from_list, int addr_len);
> >> void __hw_addr_init(struct netdev_hw_addr_list *list);
> >> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> >> index ef95e81..26ea4fe 100644
> >> --- a/net/bridge/br_fdb.c
> >> +++ b/net/bridge/br_fdb.c
> >> @@ -911,19 +911,31 @@ void br_fdb_addrs_sync(struct net_bridge *br)
> >> 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);
> >> + err = __hw_addr_sync_multiple(&p->dev->uc, &br->conf_addrs,
> >> + p->dev->addr_len);
> >> if (!err)
> >> __dev_set_rx_mode(p->dev);
> >> netif_addr_unlock(p->dev);
> >> + } else if (br->n_flood_ports == 0) {
> >> + list_for_each_entry(p, &br->port_list, list) {
> >> + /* skip over ports being deleted. */
> >> + if (!br_port_exists(p->dev))
> >> + continue;
> >>
> >> + netif_addr_lock_nested(p->dev);
> >> + err = __hw_addr_sync_multiple(&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)
> >> +void br_fdb_addrs_unsync(struct net_bridge *br, struct net_bridge_port *p)
> >> {
> >> - struct net_bridge_port *p = br->c_flood_port;
> >> -
> >> if (!p)
> >> return;
> >>
> >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >> index 55e4e28..4ba62ef 100644
> >> --- a/net/bridge/br_if.c
> >> +++ b/net/bridge/br_if.c
> >> @@ -148,6 +148,8 @@ static void del_nbp(struct net_bridge_port *p)
> >>
> >> if (p->flags & BR_FLOOD)
> >> br_del_flood_port(p, br);
> >> + else
> >> + br_fdb_addrs_unsync(br, p);
> >>
> >> nbp_vlan_flush(p);
> >> br_fdb_delete_by_port(br, p, 1);
> >> @@ -530,8 +532,26 @@ static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br)
> >> * only have 1 flooding port cache if for future use.
> >> */
> >> br->n_flood_ports++;
> >> - if (br->n_flood_ports == 1)
> >> + if (br->n_flood_ports == 1) {
> >> + struct net_bridge_port *port;
> >> +
> >> + /* We are transitioning from 0 flood ports to 1. Remove
> >> + * the addresses from all the non-flood ports and turn on
> >> + * promisc on those ports.
> >> + */
> >> + list_for_each_entry(port, &br->port_list, list) {
> >> + /* skip the current port we are changing */
> >> + if (port == p)
> >> + continue;
> >> +
> >> + if (!(port->flags & BR_FLOOD)) {
> >> + br_port_set_promisc(port);
> >> + br_fdb_addrs_unsync(br, port);
> >> + }
> >> + }
> >> +
> >> br->c_flood_port = p;
> >> + }
> >>
> >> br_fdb_addrs_sync(br);
> >> br_manage_promisc(br);
> >> @@ -547,12 +567,20 @@ 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) {
> >> - br_fdb_addrs_unsync(br);
> >> - br->c_flood_port = NULL;
> >> - }
> >> + if (br->n_flood_ports == 0) {
> >> + /* We just dropped to 0 flood ports. If we
> >> + * are deleting this port, unsync addresses
> >> + * from it.
> >> + */
> >> + if (!br_port_exists(p->dev))
> >> + br_fdb_addrs_unsync(br, p);
> >>
> >> - if (br->n_flood_ports == 1) {
> >> + br->c_flood_port = NULL;
> >> + br_fdb_addrs_sync(br);
> >> + } else if (br->n_flood_ports == 1) {
> >> + /* We just dropped to 1 flood port. Find this one flood
> >> + * port and sync to it.
> >> + */
> >> list_for_each_entry(port, &p->br->port_list, list) {
> >> if (br_port_exists(port->dev) &&
> >> (port->flags & BR_FLOOD)) {
> >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >> index 87dcc09..13840de 100644
> >> --- a/net/bridge/br_private.h
> >> +++ b/net/bridge/br_private.h
> >> @@ -400,7 +400,7 @@ int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
> >> 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);
> >> +void br_fdb_addrs_unsync(struct net_bridge *br, struct net_bridge_port *p);
> >>
> >> /* br_forward.c */
> >> void br_deliver(const struct net_bridge_port *to, struct sk_buff *skb);
> >> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> >> index 3de44a3..24da78f 100644
> >> --- a/net/core/dev_addr_lists.c
> >> +++ b/net/core/dev_addr_lists.c
> >> @@ -171,9 +171,9 @@ static void __hw_addr_unsync_one(struct netdev_hw_addr_list *to_list,
> >> __hw_addr_del_entry(from_list, ha, false, false);
> >> }
> >>
> >> -static int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> >> - struct netdev_hw_addr_list *from_list,
> >> - int addr_len)
> >> +int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> >> + struct netdev_hw_addr_list *from_list,
> >> + int addr_len)
> >> {
> >> int err = 0;
> >> struct netdev_hw_addr *ha, *tmp;
> >> @@ -189,6 +189,7 @@ static int __hw_addr_sync_multiple(struct netdev_hw_addr_list *to_list,
> >> }
> >> return err;
> >> }
> >> +EXPORT_SYMBOL(__hw_addr_sync_multiple);
> >>
> >> /* This function only works where there is a strict 1-1 relationship
> >> * between source and destionation of they synch. If you ever need to
> >> --
> >> 1.8.5.3
