Message ID | 20120209032206.32468.92296.stgit@jf-dev1-dcblab |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Wed, 08 Feb 2012 19:22:06 -0800 John Fastabend <john.r.fastabend@intel.com> wrote: > Propagate software FDB table into hardware uc, mc lists when > the NETIF_F_HW_FDB is set. > > This resolves the case below where an embedded switch is used > in hardware to do inter-VF or VF-PF switching. This patch > pushes the FDB entry (specifically the MAC address) into the > embedded switch with dev_add_uc and dev_add_mc so the switch > "learns" about the software bridge. > > > veth0 veth2 > | | > ------------ > | bridge0 | <---- software bridging > ------------ > / > / > ethx.y ethx > VF PF > \ \ <---- propagate FDB entries to HW > \ \ > -------------------- > | Embedded Bridge | <---- hardware offloaded switching > -------------------- > > This is only an RFC couple more changes are needed. > > (1) Optimize HW FDB set/del to only walk list if an FDB offloaded > device is attached. Or decide it doesn't matter from unlikely() > path. > > (2) Is it good enough to just call dev_uc_{add|del} or > dev_mc_{add|del}? Or do some devices really need a new netdev > callback to do this operation correctly. I think it should be > good enough as is. > > (3) wrapped list walk in rcu_read_lock() just in case maybe every > case is already inside rcu_read_lock()/unlock(). > > Also this is in response to this thread regarding the macvlan and > exposing rx filters posting now to see if folks think this is the > right idea and if it will resolve at least the bridge case. > > http://lists.openwall.net/netdev/2011/11/08/135 > > Signed-off-by: John Fastabend <john.r.fastabend@intel.com> > --- > > include/linux/netdev_features.h | 2 ++ > net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h > index 77f5202..5936fae 100644 Rather than yet another device feature, I would rather use netlink_notifier callback. The notifier is more general and generic without messing with internals of bridge. -- 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 2/8/2012 8:36 PM, Stephen Hemminger wrote: > On Wed, 08 Feb 2012 19:22:06 -0800 > John Fastabend <john.r.fastabend@intel.com> wrote: > >> Propagate software FDB table into hardware uc, mc lists when >> the NETIF_F_HW_FDB is set. >> >> This resolves the case below where an embedded switch is used >> in hardware to do inter-VF or VF-PF switching. This patch >> pushes the FDB entry (specifically the MAC address) into the >> embedded switch with dev_add_uc and dev_add_mc so the switch >> "learns" about the software bridge. >> >> >> veth0 veth2 >> | | >> ------------ >> | bridge0 | <---- software bridging >> ------------ >> / >> / >> ethx.y ethx >> VF PF >> \ \ <---- propagate FDB entries to HW >> \ \ >> -------------------- >> | Embedded Bridge | <---- hardware offloaded switching >> -------------------- >> >> This is only an RFC couple more changes are needed. >> >> (1) Optimize HW FDB set/del to only walk list if an FDB offloaded >> device is attached. Or decide it doesn't matter from unlikely() >> path. >> >> (2) Is it good enough to just call dev_uc_{add|del} or >> dev_mc_{add|del}? Or do some devices really need a new netdev >> callback to do this operation correctly. I think it should be >> good enough as is. >> >> (3) wrapped list walk in rcu_read_lock() just in case maybe every >> case is already inside rcu_read_lock()/unlock(). >> >> Also this is in response to this thread regarding the macvlan and >> exposing rx filters posting now to see if folks think this is the >> right idea and if it will resolve at least the bridge case. >> >> http://lists.openwall.net/netdev/2011/11/08/135 >> >> Signed-off-by: John Fastabend <john.r.fastabend@intel.com> >> --- >> >> include/linux/netdev_features.h | 2 ++ >> net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++++ >> 2 files changed, 36 insertions(+), 0 deletions(-) >> >> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h >> index 77f5202..5936fae 100644 > > Rather than yet another device feature, I would rather use netlink_notifier > callback. The notifier is more general and generic without messing with internals > of bridge. > But the device features makes it easy for user space to learn that the device supports this sort of offload. Now if all SR-IOV devices support this then it doesn't matter but I thought there were SR-IOV devices that didn't do any switching? I'll dig through the SR-IOV drivers to check there are not too many of them. By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() probably in rtnl_notify()? Then drivers could register with the notifier chain with atomic_notifier_chain_register() and receive the events correctly. Or did I miss some notifier chain that already exists? Thanks, John -- 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, 09 Feb 2012 09:36:47 -0800 John Fastabend <john.r.fastabend@intel.com> wrote: > But the device features makes it easy for user space to learn that the device > supports this sort of offload. Now if all SR-IOV devices support this then it > doesn't matter but I thought there were SR-IOV devices that didn't do any > switching? I'll dig through the SR-IOV drivers to check there are not too > many of them. If user space needs to know then the OS is not designed properly. The purpose of the network device is to abstract all those details, and more and more of them are bleeding through. This makes writing management applications harder and makes things dependent on features that may or may not be present. The best design is when the change is invisible. > By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() > probably in rtnl_notify()? Then drivers could register with the notifier chain with > atomic_notifier_chain_register() and receive the events correctly. Or did I miss > some notifier chain that already exists? Yes. that is what I mean. The callbacks you need may or may not already be present. -- 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 2/9/2012 9:40 AM, Stephen Hemminger wrote: > On Thu, 09 Feb 2012 09:36:47 -0800 > John Fastabend <john.r.fastabend@intel.com> wrote: > >> But the device features makes it easy for user space to learn that the device >> supports this sort of offload. Now if all SR-IOV devices support this then it >> doesn't matter but I thought there were SR-IOV devices that didn't do any >> switching? I'll dig through the SR-IOV drivers to check there are not too >> many of them. > > If user space needs to know then the OS is not designed properly. > The purpose of the network device is to abstract all those details, and more and more > of them are bleeding through. This makes writing management applications harder and makes > things dependent on features that may or may not be present. The best design is when > the change is invisible. > Agreed. >> By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() >> probably in rtnl_notify()? Then drivers could register with the notifier chain with >> atomic_notifier_chain_register() and receive the events correctly. Or did I miss >> some notifier chain that already exists? > > Yes. that is what I mean. The callbacks you need may or may not already be present. OK thanks I'll put together an update here shortly. -- 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, 2012-02-08 at 19:22 -0800, John Fastabend wrote: > Propagate software FDB table into hardware uc, mc lists when > the NETIF_F_HW_FDB is set. > > This resolves the case below where an embedded switch is used > in hardware to do inter-VF or VF-PF switching. This patch > pushes the FDB entry (specifically the MAC address) into the > embedded switch with dev_add_uc and dev_add_mc so the switch > "learns" about the software bridge. > > > veth0 veth2 > | | > ------------ > | bridge0 | <---- software bridging > ------------ > / > / > ethx.y ethx > VF PF > \ \ <---- propagate FDB entries to HW > \ \ > -------------------- > | Embedded Bridge | <---- hardware offloaded switching > -------------------- > This scenario works now as adding an interface to a bridge puts it in promiscuous mode. So adding a PF to a software bridge should not be a problem as it supports promiscuous mode. But adding a VF will not work. Are you trying to avoid the requirement of having to put the interface in promiscuous mode when adding to a bridge? Thanks Sridhar > This is only an RFC couple more changes are needed. > > (1) Optimize HW FDB set/del to only walk list if an FDB offloaded > device is attached. Or decide it doesn't matter from unlikely() > path. > > (2) Is it good enough to just call dev_uc_{add|del} or > dev_mc_{add|del}? Or do some devices really need a new netdev > callback to do this operation correctly. I think it should be > good enough as is. > > (3) wrapped list walk in rcu_read_lock() just in case maybe every > case is already inside rcu_read_lock()/unlock(). > > Also this is in response to this thread regarding the macvlan and > exposing rx filters posting now to see if folks think this is the > right idea and if it will resolve at least the bridge case. > > http://lists.openwall.net/netdev/2011/11/08/135 > > Signed-off-by: John Fastabend <john.r.fastabend@intel.com> > --- > > include/linux/netdev_features.h | 2 ++ > net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+), 0 deletions(-) > > diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h > index 77f5202..5936fae 100644 > --- a/include/linux/netdev_features.h > +++ b/include/linux/netdev_features.h > @@ -55,6 +55,8 @@ enum { > NETIF_F_NOCACHE_COPY_BIT, /* Use no-cache copyfromuser */ > NETIF_F_LOOPBACK_BIT, /* Enable loopback */ > > + NETIF_F_HW_FDB, /* Hardware supports switching */ > + > /* > * Add your fresh new feature above and remember to update > * netdev_features_strings[] in net/core/ethtool.c and maybe > diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c > index 5ba0c84..4cc545b 100644 > --- a/net/bridge/br_fdb.c > +++ b/net/bridge/br_fdb.c > @@ -81,9 +81,26 @@ static void fdb_rcu_free(struct rcu_head *head) > kmem_cache_free(br_fdb_cache, ent); > } > > +static void fdb_hw_delete(struct net_bridge *br, > + struct net_bridge_fdb_entry *fdb) > +{ > + struct net_bridge_port *op; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(op, &br->port_list, list) { > + struct net_device *dev = op->dev; > + > + if ((dev->features & NETIF_F_HW_FDB) && > + dev != fdb->dst->dev) > + dev_uc_del(dev, fdb->addr.addr); > + } > + rcu_read_unlock(); > +} > + > static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) > { > hlist_del_rcu(&f->hlist); > + fdb_hw_delete(br, f); > fdb_notify(br, f, RTM_DELNEIGH); > call_rcu(&f->rcu, fdb_rcu_free); > } > @@ -350,6 +367,22 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head, > return NULL; > } > > +static void fdb_hw_create(struct net_bridge *br, > + struct net_bridge_fdb_entry *fdb) > +{ > + struct net_bridge_port *op; > + > + rcu_read_lock(); > + list_for_each_entry_rcu(op, &br->port_list, list) { > + struct net_device *dev = op->dev; > + > + if ((dev->features & NETIF_F_HW_FDB) && > + dev != fdb->dst->dev) > + dev_uc_add(dev, fdb->addr.addr); > + } > + rcu_read_unlock(); > +} > + > static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, > struct net_bridge_port *source, > const unsigned char *addr) > @@ -363,6 +396,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, > fdb->is_local = 0; > fdb->is_static = 0; > fdb->updated = fdb->used = jiffies; > + fdb_hw_create(source->br, fdb); > hlist_add_head_rcu(&fdb->hlist, head); > } > return fdb; > -- 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 2/9/2012 10:14 AM, Sridhar Samudrala wrote: > On Wed, 2012-02-08 at 19:22 -0800, John Fastabend wrote: >> Propagate software FDB table into hardware uc, mc lists when >> the NETIF_F_HW_FDB is set. >> >> This resolves the case below where an embedded switch is used >> in hardware to do inter-VF or VF-PF switching. This patch >> pushes the FDB entry (specifically the MAC address) into the >> embedded switch with dev_add_uc and dev_add_mc so the switch >> "learns" about the software bridge. >> >> >> veth0 veth2 >> | | >> ------------ >> | bridge0 | <---- software bridging >> ------------ >> / >> / >> ethx.y ethx >> VF PF >> \ \ <---- propagate FDB entries to HW >> \ \ >> -------------------- >> | Embedded Bridge | <---- hardware offloaded switching >> -------------------- >> > > This scenario works now as adding an interface to a bridge puts it in > promiscuous mode. So adding a PF to a software bridge should not be > a problem as it supports promiscuous mode. But adding a VF will not > work. It shouldn't work because the embedded bridge will lookup the address in its FDB and when it doesn't match any unicast filters it will forward the packet onto the wire. Because the veth0 and veth2 above never get inserted into the embedded brdige's FDB the packets will _never_ get routed there. That said the current 'ixgbe' driver is doing something broken in that it is always setting the unicast hash table and mirroring bits to 1. So if you think this is working your seeing a bug where packets are being sent onto the wire AND upto the PF. Packets with destination addresses matching veth1 should not end up on the wire and vice versa. This is specific to ixgbe and is not the case for other SR-IOV devices. This causes some issues (a) has some very real performance implications, (b) at this point you have some strange behavior from my point of view. The embedded bridge is not a learning bridge nor is it acting like an 802.1Q VEB or VEPA. > > Are you trying to avoid the requirement of having to put the interface > in promiscuous mode when adding to a bridge? > I think the bridge being in promiscuous mode is correct. Hope that helps sorry its a bit long winded. John -- 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, 2012-02-09 at 09:52 -0800, John Fastabend wrote: > >> By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() > >> probably in rtnl_notify()? Then drivers could register with the notifier chain with > >> atomic_notifier_chain_register() and receive the events correctly. Or did I miss > >> some notifier chain that already exists? > > > > Yes. that is what I mean. The callbacks you need may or may not already be present. I'll go one step further. This stuff shouldnt be in the kernel at all. The disadvantage is you need a user space app to update the hardware. i.e, the same mechanism should be usable for either a switch embedded in a NIC or a standalone hardware switch (with/out the s/ware bridge presence) cheers, jamal -- 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, 2012-02-09 at 12:30 -0800, John Fastabend wrote: > On 2/9/2012 10:14 AM, Sridhar Samudrala wrote: > > On Wed, 2012-02-08 at 19:22 -0800, John Fastabend wrote: > >> Propagate software FDB table into hardware uc, mc lists when > >> the NETIF_F_HW_FDB is set. > >> > >> This resolves the case below where an embedded switch is used > >> in hardware to do inter-VF or VF-PF switching. This patch > >> pushes the FDB entry (specifically the MAC address) into the > >> embedded switch with dev_add_uc and dev_add_mc so the switch > >> "learns" about the software bridge. > >> > >> > >> veth0 veth2 > >> | | > >> ------------ > >> | bridge0 | <---- software bridging > >> ------------ > >> / > >> / > >> ethx.y ethx > >> VF PF > >> \ \ <---- propagate FDB entries to HW > >> \ \ > >> -------------------- > >> | Embedded Bridge | <---- hardware offloaded switching > >> -------------------- > >> > > > > This scenario works now as adding an interface to a bridge puts it in > > promiscuous mode. So adding a PF to a software bridge should not be > > a problem as it supports promiscuous mode. But adding a VF will not > > work. > > It shouldn't work because the embedded bridge will lookup the address > in its FDB and when it doesn't match any unicast filters it will forward > the packet onto the wire. Because the veth0 and veth2 above never get > inserted into the embedded brdige's FDB the packets will _never_ get > routed there. > > That said the current 'ixgbe' driver is doing something broken in that > it is always setting the unicast hash table and mirroring bits to 1. So > if you think this is working your seeing a bug where packets are being > sent onto the wire AND upto the PF. Packets with destination addresses > matching veth1 should not end up on the wire and vice versa. This is > specific to ixgbe and is not the case for other SR-IOV devices. OK. Is this behavior going to be fixed. > > This causes some issues (a) has some very real performance implications, > (b) at this point you have some strange behavior from my point of view. > The embedded bridge is not a learning bridge nor is it acting like an > 802.1Q VEB or VEPA. > > > > > Are you trying to avoid the requirement of having to put the interface > > in promiscuous mode when adding to a bridge? > > > > I think the bridge being in promiscuous mode is correct. The interface that is added to the bridge is put in promiscuous mode, not the bridge itself. In this example, i assumed that setting promiscuous on PF is putting the embedded bridge in learning mode. Thanks Sridhar > > Hope that helps sorry its a bit long winded. > John > > > -- 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 2/9/2012 4:39 PM, Sridhar Samudrala wrote: > On Thu, 2012-02-09 at 12:30 -0800, John Fastabend wrote: >> On 2/9/2012 10:14 AM, Sridhar Samudrala wrote: >>> On Wed, 2012-02-08 at 19:22 -0800, John Fastabend wrote: >>>> Propagate software FDB table into hardware uc, mc lists when >>>> the NETIF_F_HW_FDB is set. >>>> >>>> This resolves the case below where an embedded switch is used >>>> in hardware to do inter-VF or VF-PF switching. This patch >>>> pushes the FDB entry (specifically the MAC address) into the >>>> embedded switch with dev_add_uc and dev_add_mc so the switch >>>> "learns" about the software bridge. >>>> >>>> >>>> veth0 veth2 >>>> | | >>>> ------------ >>>> | bridge0 | <---- software bridging >>>> ------------ >>>> / >>>> / >>>> ethx.y ethx >>>> VF PF >>>> \ \ <---- propagate FDB entries to HW >>>> \ \ >>>> -------------------- >>>> | Embedded Bridge | <---- hardware offloaded switching >>>> -------------------- >>>> >>> >>> This scenario works now as adding an interface to a bridge puts it in >>> promiscuous mode. So adding a PF to a software bridge should not be >>> a problem as it supports promiscuous mode. But adding a VF will not >>> work. >> >> It shouldn't work because the embedded bridge will lookup the address >> in its FDB and when it doesn't match any unicast filters it will forward >> the packet onto the wire. Because the veth0 and veth2 above never get >> inserted into the embedded brdige's FDB the packets will _never_ get >> routed there. >> >> That said the current 'ixgbe' driver is doing something broken in that >> it is always setting the unicast hash table and mirroring bits to 1. So >> if you think this is working your seeing a bug where packets are being >> sent onto the wire AND upto the PF. Packets with destination addresses >> matching veth1 should not end up on the wire and vice versa. This is >> specific to ixgbe and is not the case for other SR-IOV devices. > > OK. Is this behavior going to be fixed. > Only after we have a mechanism to either configure the NIC FDB directly or have it stay in sync with the SW switch. Flooding traffic seems better than being unable to send traffic to the virtual device altogether. This behavior is driver specific some devices just fail outright. I'm thinking over Jamal's comment now. >> >> This causes some issues (a) has some very real performance implications, >> (b) at this point you have some strange behavior from my point of view. >> The embedded bridge is not a learning bridge nor is it acting like an >> 802.1Q VEB or VEPA. >> >>> >>> Are you trying to avoid the requirement of having to put the interface >>> in promiscuous mode when adding to a bridge? >>> >> >> I think the bridge being in promiscuous mode is correct. > > The interface that is added to the bridge is put in promiscuous mode, > not the bridge itself. In this example, i assumed that setting > promiscuous on PF is putting the embedded bridge in learning mode. Yes I misspoke I mean the PF. The embedded bridge in this case does not support learning. Also I'm not aware of any SR-IOV NICs that do support learning. > > Thanks > Sridhar > >> >> Hope that helps sorry its a bit long winded. >> John >> >> >> > > > -- > 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 -- 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 2/9/2012 1:11 PM, jamal wrote: > On Thu, 2012-02-09 at 09:52 -0800, John Fastabend wrote: > >>>> By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() >>>> probably in rtnl_notify()? Then drivers could register with the notifier chain with >>>> atomic_notifier_chain_register() and receive the events correctly. Or did I miss >>>> some notifier chain that already exists? >>> >>> Yes. that is what I mean. The callbacks you need may or may not already be present. > > I'll go one step further. > This stuff shouldnt be in the kernel at all. > The disadvantage is you need a user space app to update the hardware. > i.e, the same mechanism should be usable for either a switch embedded > in a NIC or a standalone hardware switch (with/out the s/ware bridge > presence) > > cheers, > jamal > Hi Jamal, The user space app in this case would listen for FDB updates to the SW bridge and then mirror them at the embedded NIC. In this case it seems easier to just add a notifier chain and let the kernel keep these in sync. Otherwise we need a daemon in user space to replicate these. On the other hand if you could make the same RTM_NEWNEIGH, RTM_DELNEIGH, and RTM_GETNEIGH work for the bridge, embedded bridge, and macvlan you would have one common interface to drive these. But the bridge already has this protocol/msgtype so that would require either some demux or new protocol/msgtype pairs to be created. Let me think on it. I'm tempted by the simplicity of adding notifier hooks though. .John -- 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 2/9/2012 6:14 PM, John Fastabend wrote: > On 2/9/2012 1:11 PM, jamal wrote: >> On Thu, 2012-02-09 at 09:52 -0800, John Fastabend wrote: >> >>>>> By netlink_notifier do you mean adding a notifier_block and using atomic_notifier_call_chain() >>>>> probably in rtnl_notify()? Then drivers could register with the notifier chain with >>>>> atomic_notifier_chain_register() and receive the events correctly. Or did I miss >>>>> some notifier chain that already exists? >>>> >>>> Yes. that is what I mean. The callbacks you need may or may not already be present. >> >> I'll go one step further. >> This stuff shouldnt be in the kernel at all. >> The disadvantage is you need a user space app to update the hardware. >> i.e, the same mechanism should be usable for either a switch embedded >> in a NIC or a standalone hardware switch (with/out the s/ware bridge >> presence) >> >> cheers, >> jamal >> > > Hi Jamal, > > The user space app in this case would listen for FDB updates to the SW > bridge and then mirror them at the embedded NIC. In this case it seems > easier to just add a notifier chain and let the kernel keep these in > sync. Otherwise we need a daemon in user space to replicate these. > > On the other hand if you could make the same RTM_NEWNEIGH, RTM_DELNEIGH, > and RTM_GETNEIGH work for the bridge, embedded bridge, and macvlan you > would have one common interface to drive these. But the bridge already > has this protocol/msgtype so that would require either some demux or > new protocol/msgtype pairs to be created. > > Let me think on it. I'm tempted by the simplicity of adding notifier > hooks though. > > .John > Actually because the bridge is adding/removing fdb entries dynamically maybe its best this gets done in kernel. Here's the example case, ---------- --------- | ethx.y | <---- E | veth0 | <--- A ---------- --------- | | | | | | | -------------- | | SW Bridge | <--- B | -------------- | | | | | --------- | | eth0 | <--- C | --------- | | ----------------------------------- | embedded switch | <--- D ----------------------------------- | | G With the flow by letters above hope this is not too difficult to follow. (A) veth0 a virtual device transmits packet destined for ethx.y (B) SW bridge receives frames and updates FDB flooding to C (C) eth0 the PF in this case sends the frame to the HW backed by the embedded bridge (D) The HW embedded switch has a static entry for ethx.y and forwards the frame to the VF or if its a broadcast frame also floods it to the wire and ethx.y (E) ethx.y receives the frame and generates a response to the dest mac of veth0 Now here is the potential issue, (G) The frame transmitted from ethx.y with the destination address of veth0 but the embedded switch is not a learning switch. If the FDB update is done in user space its possible (likely?) that the FDB entry for veth0 has not been added to the embedded switch yet. Now we either have to flood the frame which is not horrible but not ideal or worse if the embedded switch does not support flooding send it to the wire and veth0 never receives it. If the SW bridge pushes the FDB update down into the embedded switch the address is for sure in the embedded switches forwarding tables and the switching works as expected. So to handle this case correctly its probably best IMHO to use a notifier hook. Having a RTM_GETNEIGH for the embedded switch implemented though would be nice for dumping the FDB of the embedded switch and SET/DEL could be used to configure the FDB when its not being driven by the SW switch. Of course we should try to be minimalists here. .John -- 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 2/9/12 9:36 AM, "John Fastabend" <john.r.fastabend@intel.com> wrote: > On 2/8/2012 8:36 PM, Stephen Hemminger wrote: >> On Wed, 08 Feb 2012 19:22:06 -0800 >> John Fastabend <john.r.fastabend@intel.com> wrote: >> >>> Propagate software FDB table into hardware uc, mc lists when >>> the NETIF_F_HW_FDB is set. >>> >>> This resolves the case below where an embedded switch is used >>> in hardware to do inter-VF or VF-PF switching. This patch >>> pushes the FDB entry (specifically the MAC address) into the >>> embedded switch with dev_add_uc and dev_add_mc so the switch >>> "learns" about the software bridge. >>> >>> >>> veth0 veth2 >>> | | >>> ------------ >>> | bridge0 | <---- software bridging >>> ------------ >>> / >>> / >>> ethx.y ethx >>> VF PF >>> \ \ <---- propagate FDB entries to HW >>> \ \ >>> -------------------- >>> | Embedded Bridge | <---- hardware offloaded switching >>> -------------------- >>> >>> This is only an RFC couple more changes are needed. >>> >>> (1) Optimize HW FDB set/del to only walk list if an FDB offloaded >>> device is attached. Or decide it doesn't matter from unlikely() >>> path. >>> >>> (2) Is it good enough to just call dev_uc_{add|del} or >>> dev_mc_{add|del}? Or do some devices really need a new netdev >>> callback to do this operation correctly. I think it should be >>> good enough as is. >>> >>> (3) wrapped list walk in rcu_read_lock() just in case maybe every >>> case is already inside rcu_read_lock()/unlock(). >>> >>> Also this is in response to this thread regarding the macvlan and >>> exposing rx filters posting now to see if folks think this is the >>> right idea and if it will resolve at least the bridge case. >>> >>> http://lists.openwall.net/netdev/2011/11/08/135 >>> >>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com> >>> --- >>> >>> include/linux/netdev_features.h | 2 ++ >>> net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++++ >>> 2 files changed, 36 insertions(+), 0 deletions(-) >>> >>> diff --git a/include/linux/netdev_features.h >>> b/include/linux/netdev_features.h >>> index 77f5202..5936fae 100644 >> >> Rather than yet another device feature, I would rather use netlink_notifier >> callback. The notifier is more general and generic without messing with >> internals >> of bridge. >> > > But the device features makes it easy for user space to learn that the device > supports this sort of offload. Now if all SR-IOV devices support this then it > doesn't matter but I thought there were SR-IOV devices that didn't do any > switching? I'll dig through the SR-IOV drivers to check there are not too > many of them. Correct. Our 802.1Qbh sriov device (enic) does not do local switching. > > By netlink_notifier do you mean adding a notifier_block and using > atomic_notifier_call_chain() > probably in rtnl_notify()? Then drivers could register with the notifier chain > with > atomic_notifier_chain_register() and receive the events correctly. Or did I > miss > some notifier chain that already exists? > > Thanks, > John > -- 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
Hi John, I went backwards to summarize at the top after going through your email. TL;DR version 0.1: you provide a good use case where it makes sense to do things in the kernel. IMO, you could make the same arguement if your embedded switch could do ACLs, IPv4 forwarding etc. And the kernel bloats. I am always bigoted to move all policy control to user space instead of bloating in the kernel. On Thu, 2012-02-09 at 20:14 -0800, John Fastabend wrote: > > > > Hi Jamal, > > > > The user space app in this case would listen for FDB updates to the SW > > bridge and then mirror them at the embedded NIC. In this case it seems > > easier to just add a notifier chain and let the kernel keep these in > > sync. Otherwise we need a daemon in user space to replicate these. > > A user space daemon if you need to ensure synchronization. Thats what i meant when i said there was a "disadvantage" over the simple case when the goal is always to synchronize. > > On the other hand if you could make the same RTM_NEWNEIGH, RTM_DELNEIGH, > > and RTM_GETNEIGH work for the bridge, embedded bridge, and macvlan you > > would have one common interface to drive these. But the bridge already > > has this protocol/msgtype so that would require either some demux or > > new protocol/msgtype pairs to be created. > > The bridge is very netlink friendly these days. Given the rest of the network stack (*NEIGH* you mention above) talks netlink to user space it should be workable. > > Let me think on it. I'm tempted by the simplicity of adding notifier > > hooks though. If something is missing bridge-side it may need to be added (as Per Stephen's comment) - i just took it one further indicating those notifiers need to also netlink-speak > Actually because the bridge is adding/removing fdb entries dynamically > maybe its best this gets done in kernel. Here's the example case, [..] > > With the flow by letters above hope this is not too difficult to follow. > (A) veth0 a virtual device transmits packet destined for ethx.y > (B) SW bridge receives frames and updates FDB flooding to C > (C) eth0 the PF in this case sends the frame to the HW backed by the > embedded bridge Following so far. Can you have more than one PF per embedded switch? Or is the intent here purely to do VMs/VF separation? > (D) The HW embedded switch has a static entry for ethx.y and forwards > the frame to the VF or if its a broadcast frame also floods it to > the wire and ethx.y nod. > (E) ethx.y receives the frame and generates a response to the dest mac of > veth0 nod. Since you said in #D the entries in the switch are static, I am assuming at this point neither ethx.y nor veth0 exist in the embedded FDB. > Now here is the potential issue, > > (G) The frame transmitted from ethx.y with the destination address of > veth0 but the embedded switch is not a learning switch. If the FDB > update is done in user space its possible (likely?) that the FDB > entry for veth0 has not been added to the embedded switch yet. Ok, got it - so the catch here is the switch is not capable of learning. I think this depends on where learning is done. Your intent is to use the S/W bridge as something that does the learning for you i.e in the kernel. This makes the s/w bridge part of MUST-have-for-this-to-run. And that maybe the case for your use case. What if I dont wanna run the S/W bridge at all? Ive been making a point that with a simple knob(Stephen doesn like to add such a knob), the SW bridge could defer learning to user space. [This way you can add a lot of richness e.g on ACLs such as restricting what MAC addresses etc are allowed to talk to which ones etc.]. But if bypass the s/w bridge all together and learn in user space or have a static config in which i populate the embedded switch, i dont see the issue. > Now > we either have to flood the frame which is not horrible but not > ideal or worse if the embedded switch does not support flooding send > it to the wire and veth0 never receives it. If it is a switch it has to flood, no? Otherwise it sounds broken. > If the SW bridge pushes > the FDB update down into the embedded switch the address is for > sure in the embedded switches forwarding tables and the switching > works as expected. Yes, there is a small gap between the s/w bridge learning and the synchronization happening to the embedded nic switch. That gap gets larger if you defer learning to user space. But like you said earlier, during that gap packets are flooded - and do you care if the synchronization doesnt happen immediately? > So to handle this case correctly its probably best IMHO to use a notifier > hook. Having a RTM_GETNEIGH for the embedded switch implemented though > would be nice for dumping the FDB of the embedded switch and SET/DEL > could be used to configure the FDB when its not being driven by the SW > switch. Of course we should try to be minimalists here. Do you need to have a different *NEIGH* than what we already have really? The problem with putting policies in the kernel is you are gonna keep adding more. Bloat user space instead. cheers, jamal -- 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 Fri, 10 Feb 2012 10:18:31 -0500 jamal <hadi@cyberus.ca> wrote: > Hi John, > > I went backwards to summarize at the top after going through your email. > > TL;DR version 0.1: > you provide a good use case where it makes sense to do things in the > kernel. IMO, you could make the same arguement if your embedded switch > could do ACLs, IPv4 forwarding etc. And the kernel bloats. > I am always bigoted to move all policy control to user space instead of > bloating in the kernel. > > > On Thu, 2012-02-09 at 20:14 -0800, John Fastabend wrote: > > > > > > > Hi Jamal, > > > > > > The user space app in this case would listen for FDB updates to the SW > > > bridge and then mirror them at the embedded NIC. In this case it seems > > > easier to just add a notifier chain and let the kernel keep these in > > > sync. Otherwise we need a daemon in user space to replicate these. > > > > > A user space daemon if you need to ensure synchronization. Thats what i > meant when i said there was a "disadvantage" over the simple case when > the goal is always to synchronize. > > > > On the other hand if you could make the same RTM_NEWNEIGH, RTM_DELNEIGH, > > > and RTM_GETNEIGH work for the bridge, embedded bridge, and macvlan you > > > would have one common interface to drive these. But the bridge already > > > has this protocol/msgtype so that would require either some demux or > > > new protocol/msgtype pairs to be created. > > > > > The bridge is very netlink friendly these days. Given the rest of the > network stack (*NEIGH* you mention above) talks netlink to user space > it should be workable. > > > > Let me think on it. I'm tempted by the simplicity of adding notifier > > > hooks though. > > If something is missing bridge-side it may need to be added (as Per > Stephen's comment) - i just took it one further indicating those > notifiers need to also netlink-speak > > > > Actually because the bridge is adding/removing fdb entries dynamically > > maybe its best this gets done in kernel. Here's the example case, > > [..] > > > > > With the flow by letters above hope this is not too difficult to follow. > > > (A) veth0 a virtual device transmits packet destined for ethx.y > > (B) SW bridge receives frames and updates FDB flooding to C > > (C) eth0 the PF in this case sends the frame to the HW backed by the > > embedded bridge > > Following so far. > Can you have more than one PF per embedded switch? Or is the intent here > purely to do VMs/VF separation? > > > (D) The HW embedded switch has a static entry for ethx.y and forwards > > the frame to the VF or if its a broadcast frame also floods it to > > the wire and ethx.y > > nod. > > > (E) ethx.y receives the frame and generates a response to the dest mac of > > veth0 > > nod. > Since you said in #D the entries in the switch are static, I am assuming > at this point neither ethx.y nor veth0 exist in the embedded FDB. > > > Now here is the potential issue, > > > > (G) The frame transmitted from ethx.y with the destination address of > > veth0 but the embedded switch is not a learning switch. If the FDB > > update is done in user space its possible (likely?) that the FDB > > entry for veth0 has not been added to the embedded switch yet. > > Ok, got it - so the catch here is the switch is not capable of learning. > I think this depends on where learning is done. Your intent is to > use the S/W bridge as something that does the learning for you i.e in > the kernel. This makes the s/w bridge part of MUST-have-for-this-to-run. > And that maybe the case for your use case. > > What if I dont wanna run the S/W bridge at all? > Ive been making a point that with a simple knob(Stephen doesn like to > add such a knob), the SW bridge could defer learning to user space. > [This way you can add a lot of richness e.g on ACLs such as restricting > what MAC addresses etc are allowed to talk to which ones etc.]. > But if bypass the s/w bridge all together and learn in user space > or have a static config in which i populate the embedded switch, i dont > see the issue. > > > Now > > we either have to flood the frame which is not horrible but not > > ideal or worse if the embedded switch does not support flooding send > > it to the wire and veth0 never receives it. > > If it is a switch it has to flood, no? Otherwise it sounds broken. > > > If the SW bridge pushes > > the FDB update down into the embedded switch the address is for > > sure in the embedded switches forwarding tables and the switching > > works as expected. > > Yes, there is a small gap between the s/w bridge learning and the > synchronization happening to the embedded nic switch. That gap gets > larger if you defer learning to user space. But like you said earlier, > during that gap packets are flooded - and do you care if the > synchronization doesnt happen immediately? > > > So to handle this case correctly its probably best IMHO to use a notifier > > hook. Having a RTM_GETNEIGH for the embedded switch implemented though > > would be nice for dumping the FDB of the embedded switch and SET/DEL > > could be used to configure the FDB when its not being driven by the SW > > switch. Of course we should try to be minimalists here. > > Do you need to have a different *NEIGH* than what we already have > really? > > The problem with putting policies in the kernel is you are gonna keep > adding more. Bloat user space instead. Some related discussion points: * the bridge needs to support control from both userspace (MSTP, TRILL, ...) and kernel space (offload etc) * the bridge forwarding database is simpler and different than the existing neighbor table, don't remember the details but last time I checked it using neighbor table in bridge would be putting square peg in round hole. -- 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 Fri, 2012-02-10 at 08:39 -0800, Stephen Hemminger wrote: > Some related discussion points: > * the bridge needs to support control from both userspace (MSTP, TRILL, ...) > and kernel space (offload etc) I think all are pretty much covered if you let some controler (I prefer user space) ADD/DEL/GET/Event on the fdb TRILL really is outside the scope of this; from an encap/decap it probably needs to be YAND (Yet another netdev) and from a control side of things you need to just provide the above netlink ops(ADD, etC) on the fdb and let the controller worry about things (Actually you _may_ need to have learning done outside of the kernel for TRILL) > * the bridge forwarding database is simpler and different than the existing > neighbor table, don't remember the details but last time I checked it > using neighbor table in bridge would be putting square peg in round hole. Agreed. cheers, jamal -- 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 2/10/2012 7:18 AM, jamal wrote: > Hi John, > > I went backwards to summarize at the top after going through your email. > > TL;DR version 0.1: > you provide a good use case where it makes sense to do things in the > kernel. IMO, you could make the same arguement if your embedded switch > could do ACLs, IPv4 forwarding etc. And the kernel bloats. > I am always bigoted to move all policy control to user space instead of > bloating in the kernel. > > > On Thu, 2012-02-09 at 20:14 -0800, John Fastabend wrote: > >>> >>> Hi Jamal, >>> >>> The user space app in this case would listen for FDB updates to the SW >>> bridge and then mirror them at the embedded NIC. In this case it seems >>> easier to just add a notifier chain and let the kernel keep these in >>> sync. Otherwise we need a daemon in user space to replicate these. >>> > > A user space daemon if you need to ensure synchronization. Thats what i > meant when i said there was a "disadvantage" over the simple case when > the goal is always to synchronize. > >>> On the other hand if you could make the same RTM_NEWNEIGH, RTM_DELNEIGH, >>> and RTM_GETNEIGH work for the bridge, embedded bridge, and macvlan you >>> would have one common interface to drive these. But the bridge already >>> has this protocol/msgtype so that would require either some demux or >>> new protocol/msgtype pairs to be created. >>> > > The bridge is very netlink friendly these days. Given the rest of the > network stack (*NEIGH* you mention above) talks netlink to user space > it should be workable. > >>> Let me think on it. I'm tempted by the simplicity of adding notifier >>> hooks though. > > If something is missing bridge-side it may need to be added (as Per > Stephen's comment) - i just took it one further indicating those > notifiers need to also netlink-speak > Sure. > >> Actually because the bridge is adding/removing fdb entries dynamically >> maybe its best this gets done in kernel. Here's the example case, > > [..] > >> >> With the flow by letters above hope this is not too difficult to follow. > >> (A) veth0 a virtual device transmits packet destined for ethx.y >> (B) SW bridge receives frames and updates FDB flooding to C >> (C) eth0 the PF in this case sends the frame to the HW backed by the >> embedded bridge > > Following so far. > Can you have more than one PF per embedded switch? Or is the intent here > purely to do VMs/VF separation? > The use case here is multiple VFs but the same solution should work with multiple PFs as well. FDB controls should be independent of how the ports are exposed VFs, PFs, VMDQ/queue pairs, macvlan, etc. >> (D) The HW embedded switch has a static entry for ethx.y and forwards >> the frame to the VF or if its a broadcast frame also floods it to >> the wire and ethx.y > > nod. > >> (E) ethx.y receives the frame and generates a response to the dest mac of >> veth0 > > nod. > Since you said in #D the entries in the switch are static, I am assuming > at this point neither ethx.y nor veth0 exist in the embedded FDB. > >> Now here is the potential issue, >> >> (G) The frame transmitted from ethx.y with the destination address of >> veth0 but the embedded switch is not a learning switch. If the FDB >> update is done in user space its possible (likely?) that the FDB >> entry for veth0 has not been added to the embedded switch yet. > > Ok, got it - so the catch here is the switch is not capable of learning. > I think this depends on where learning is done. Your intent is to > use the S/W bridge as something that does the learning for you i.e in > the kernel. This makes the s/w bridge part of MUST-have-for-this-to-run. > And that maybe the case for your use case. > This is _my_ use case today. > What if I dont wanna run the S/W bridge at all? > Ive been making a point that with a simple knob(Stephen doesn like to > add such a knob), the SW bridge could defer learning to user space. > [This way you can add a lot of richness e.g on ACLs such as restricting > what MAC addresses etc are allowed to talk to which ones etc.]. > But if bypass the s/w bridge all together and learn in user space > or have a static config in which i populate the embedded switch, i dont > see the issue. With events and ADD/DEL/GET FDB controls we can solve both cases. This also solves Roopa's case with macvlan where he wants to add additional addresses to macvlan ports. > >> Now >> we either have to flood the frame which is not horrible but not >> ideal or worse if the embedded switch does not support flooding send >> it to the wire and veth0 never receives it. > > If it is a switch it has to flood, no? Otherwise it sounds broken. > Yes it should flood here, unless its acting as a 802.1Qbg VEB or VEPA. >> If the SW bridge pushes >> the FDB update down into the embedded switch the address is for >> sure in the embedded switches forwarding tables and the switching >> works as expected. > > Yes, there is a small gap between the s/w bridge learning and the > synchronization happening to the embedded nic switch. That gap gets > larger if you defer learning to user space. But like you said earlier, > during that gap packets are flooded - and do you care if the > synchronization doesnt happen immediately? > Maybe not. But the kernel already has the needed signals with one extra hook we can save running a daemon in user space. Maybe that's not a great argument to add kernel code though. >> So to handle this case correctly its probably best IMHO to use a notifier >> hook. Having a RTM_GETNEIGH for the embedded switch implemented though >> would be nice for dumping the FDB of the embedded switch and SET/DEL >> could be used to configure the FDB when its not being driven by the SW >> switch. Of course we should try to be minimalists here. > > Do you need to have a different *NEIGH* than what we already have > really? > The PF_BRIDGE:RTM_GETNEIGH,RTM_NEWNEIGH,RTM_DELNEIGH are registered in the br_netlink_init() path. Adding notifier hooks here might be possible but I'm wondering if its better to add new message types or tear apart the existing bridging events. I'll play with the code some today and see what works out better. As Stephen noted the PF_UNSPEC:RTM_XXX events in the neighbor code are not really for bridging. > The problem with putting policies in the kernel is you are gonna keep > adding more. Bloat user space instead. > Agree policy is best left for user space. Thanks, John > cheers, > jamal > > -- 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 Mon, 2012-02-13 at 07:13 -0800, John Fastabend wrote: > The use case here is multiple VFs but the same solution should work with > multiple PFs as well. FDB controls should be independent of how the ports > are exposed VFs, PFs, VMDQ/queue pairs, macvlan, etc. Makes sense. > With events and ADD/DEL/GET FDB controls we can solve both cases. This also > solves Roopa's case with macvlan where he wants to add additional addresses > to macvlan ports. Not familiar with that issue - I'll prowl the list. > Yes it should flood here, unless its acting as a 802.1Qbg VEB or VEPA. Ok. So there is a toggle somewhere which controls how flooding should happen. > > Maybe not. But the kernel already has the needed signals with one extra > hook we can save running a daemon in user space. Maybe that's not a great > argument to add kernel code though. You make a reasonable arguement to have it in the kernel but i think we win more if we separate the control. So while i empathize, I am hoping that youd go with the path that is hard to travel ;-> > The PF_BRIDGE:RTM_GETNEIGH,RTM_NEWNEIGH,RTM_DELNEIGH are registered in the > br_netlink_init() path. Hrm - hadnt paid attention to that before. Nasty. The bridge seems to be hard-coding policy on station movement, no? This is a good example of the qualms i have on adding things to the kernel;-> I may not want to auto update a MAC address moving ports as part of some policy i have. I can go and add YAK (Yet Another Knob) - but where is the line drawn? cheers, jamal -- 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 2/14/2012 5:18 AM, jamal wrote: > On Mon, 2012-02-13 at 07:13 -0800, John Fastabend wrote: > >> The use case here is multiple VFs but the same solution should work with >> multiple PFs as well. FDB controls should be independent of how the ports >> are exposed VFs, PFs, VMDQ/queue pairs, macvlan, etc. > > Makes sense. > >> With events and ADD/DEL/GET FDB controls we can solve both cases. This also >> solves Roopa's case with macvlan where she wants to add additional addresses >> to macvlan ports. > > Not familiar with that issue - I'll prowl the list. Roopa was likely on the right track here, http://patchwork.ozlabs.org/patch/123064/ But I think the proper syntax is to use the existing PF_BRIDGE:RTM_XXX netlink messages. And if possible drive this without extending ndo_ops. An ideal user space interaction IMHO would look like, [root@jf-dev1-dcblab iproute2]# ./br/br fdb add 52:e5:62:7b:57:88 dev veth10 [root@jf-dev1-dcblab iproute2]# ./br/br fdb port mac addr flags veth2 36:a6:35:9b:96:c4 local veth4 aa:54:b0:7b:42:ef local veth0 2a:e8:5c:95:6c:1b local veth6 6e:26:d5:43:a3:36 local veth0 f2:c1:39:76:6a:fb veth8 4e:35:16:af:87:13 local veth10 52:e5:62:7b:57:88 static veth10 aa:a9:35:21:15:c4 local [root@jf-dev1-dcblab iproute2]# ./br/br fdb add dev eth3 to 52:e5:62:7b:57:88 RTNETLINK answers: Invalid argument Using Stephen's br tool. First command adds FDB entry to SW bridge and if the same tool could be used to add entries to embedded bridge I think that would be the best case. So no RTNETLINK error on the second cmd. Then embedded FDB entries could be dumped this way also so I get a complete view of my FDB setup across multiple sw bridges and embedded bridges. I don't think br is part of iproute2 yet I just pulled it out of some RFC but it works reasonably well and is intuitive enough. > >> Yes it should flood here, unless its acting as a 802.1Qbg VEB or VEPA. > > Ok. So there is a toggle somewhere which controls how flooding should > happen. > Yes. The hardware has a bit to support this which is currently not exposed to user space. That's a case where we have 'yet another knob' that needs a clean solution. This causes real bugs today when users try to use the macvlan devices in VEPA mode on top of SR-IOV. By the way these modes are all part of the 802.1Qbg spec which people actually want to use with Linux so a good clean solution is probably needed. >> >> Maybe not. But the kernel already has the needed signals with one extra >> hook we can save running a daemon in user space. Maybe that's not a great >> argument to add kernel code though. > > You make a reasonable arguement to have it in the kernel but i think we > win more if we separate the control. So while i empathize, I am hoping > that youd go with the path that is hard to travel ;-> > >> The PF_BRIDGE:RTM_GETNEIGH,RTM_NEWNEIGH,RTM_DELNEIGH are registered in the >> br_netlink_init() path. > > Hrm - hadnt paid attention to that before. Nasty. > The bridge seems to be hard-coding policy on station movement, no? > This is a good example of the qualms i have on adding things to the > kernel;-> > I may not want to auto update a MAC address moving ports as part of > some policy i have. I can go and add YAK (Yet Another Knob) - but where > is the line drawn? > I have no problem with drawing the line here and trying to implement something over PF_BRIDGE:RTM_xxx nlmsgs. I'll work with Roopa and see if we can come up with something in the next couple days. w.r.t. VEPA/VEB and flooding behavior we could probably have a bit to indicate if the port is a flooding port or not. Then users could build any sort of forwarding table they wanted OR we could just drive it through a notifier (ndo_ops?) in the macvlan path which does VEPA today. OK I'll try to write some actual code now that can be critiqued. > cheers, > jamal > > -- 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 Tue, 14 Feb 2012 10:57:04 -0800 John Fastabend <john.r.fastabend@intel.com> wrote: > On 2/14/2012 5:18 AM, jamal wrote: > > On Mon, 2012-02-13 at 07:13 -0800, John Fastabend wrote: > > > >> The use case here is multiple VFs but the same solution should work with > >> multiple PFs as well. FDB controls should be independent of how the ports > >> are exposed VFs, PFs, VMDQ/queue pairs, macvlan, etc. > > > > Makes sense. > > > >> With events and ADD/DEL/GET FDB controls we can solve both cases. This also > >> solves Roopa's case with macvlan where she wants to add additional addresses > >> to macvlan ports. > > > > Not familiar with that issue - I'll prowl the list. > > Roopa was likely on the right track here, > > http://patchwork.ozlabs.org/patch/123064/ > > But I think the proper syntax is to use the existing PF_BRIDGE:RTM_XXX > netlink messages. And if possible drive this without extending ndo_ops. > > An ideal user space interaction IMHO would look like, > > [root@jf-dev1-dcblab iproute2]# ./br/br fdb add 52:e5:62:7b:57:88 dev veth10 > [root@jf-dev1-dcblab iproute2]# ./br/br fdb > port mac addr flags > veth2 36:a6:35:9b:96:c4 local > veth4 aa:54:b0:7b:42:ef local > veth0 2a:e8:5c:95:6c:1b local > veth6 6e:26:d5:43:a3:36 local > veth0 f2:c1:39:76:6a:fb > veth8 4e:35:16:af:87:13 local > veth10 52:e5:62:7b:57:88 static > veth10 aa:a9:35:21:15:c4 local > [root@jf-dev1-dcblab iproute2]# ./br/br fdb add dev eth3 to 52:e5:62:7b:57:88 > RTNETLINK answers: Invalid argument I am going to put bridge (nameclash with br) tool into iproute2 (soon). -- 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 2/14/2012 11:05 AM, Stephen Hemminger wrote: > On Tue, 14 Feb 2012 10:57:04 -0800 > John Fastabend <john.r.fastabend@intel.com> wrote: > >> On 2/14/2012 5:18 AM, jamal wrote: >>> On Mon, 2012-02-13 at 07:13 -0800, John Fastabend wrote: >>> >>>> The use case here is multiple VFs but the same solution should work with >>>> multiple PFs as well. FDB controls should be independent of how the ports >>>> are exposed VFs, PFs, VMDQ/queue pairs, macvlan, etc. >>> >>> Makes sense. >>> >>>> With events and ADD/DEL/GET FDB controls we can solve both cases. This also >>>> solves Roopa's case with macvlan where she wants to add additional addresses >>>> to macvlan ports. >>> >>> Not familiar with that issue - I'll prowl the list. >> >> Roopa was likely on the right track here, >> >> http://patchwork.ozlabs.org/patch/123064/ >> >> But I think the proper syntax is to use the existing PF_BRIDGE:RTM_XXX >> netlink messages. And if possible drive this without extending ndo_ops. >> >> An ideal user space interaction IMHO would look like, >> >> [root@jf-dev1-dcblab iproute2]# ./br/br fdb add 52:e5:62:7b:57:88 dev veth10 >> [root@jf-dev1-dcblab iproute2]# ./br/br fdb >> port mac addr flags >> veth2 36:a6:35:9b:96:c4 local >> veth4 aa:54:b0:7b:42:ef local >> veth0 2a:e8:5c:95:6c:1b local >> veth6 6e:26:d5:43:a3:36 local >> veth0 f2:c1:39:76:6a:fb >> veth8 4e:35:16:af:87:13 local >> veth10 52:e5:62:7b:57:88 static >> veth10 aa:a9:35:21:15:c4 local >> [root@jf-dev1-dcblab iproute2]# ./br/br fdb add dev eth3 to 52:e5:62:7b:57:88 >> RTNETLINK answers: Invalid argument > > I am going to put bridge (nameclash with br) tool into iproute2 (soon). I've been using it on my dev box for awhile now and it works well for me. -- 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 Tue, 2012-02-14 at 10:57 -0800, John Fastabend wrote: > Roopa was likely on the right track here, > > http://patchwork.ozlabs.org/patch/123064/ Doesnt seem related to the bridging stuff - the modeling looks reasonable however. > But I think the proper syntax is to use the existing PF_BRIDGE:RTM_XXX > netlink messages. And if possible drive this without extending ndo_ops. > > An ideal user space interaction IMHO would look like, > > [root@jf-dev1-dcblab iproute2]# ./br/br fdb add 52:e5:62:7b:57:88 dev veth10 > [root@jf-dev1-dcblab iproute2]# ./br/br fdb > port mac addr flags > veth2 36:a6:35:9b:96:c4 local > veth4 aa:54:b0:7b:42:ef local > veth0 2a:e8:5c:95:6c:1b local > veth6 6e:26:d5:43:a3:36 local > veth0 f2:c1:39:76:6a:fb > veth8 4e:35:16:af:87:13 local > veth10 52:e5:62:7b:57:88 static > veth10 aa:a9:35:21:15:c4 local Looks nice, where is the targeted bridge(eg br0) in that syntax? > Using Stephen's br tool. First command adds FDB entry to SW bridge and > if the same tool could be used to add entries to embedded bridge I think > that would be the best case. That would be nice (although adds dependency on the presence of the s/ware bridge). Would be nicer to have either a knob in the kernel to say "synchronize with h/w bridge foo" which can be turned off. > So no RTNETLINK error on the second cmd. Then > embedded FDB entries could be dumped this way also so I get a complete view > of my FDB setup across multiple sw bridges and embedded bridges. So if you had multiple h/ware bridges - which one is tied to br0? > Yes. The hardware has a bit to support this which is currently not exposed > to user space. That's a case where we have 'yet another knob' that needs > a clean solution. This causes real bugs today when users try to use the > macvlan devices in VEPA mode on top of SR-IOV. By the way these modes are > all part of the 802.1Qbg spec which people actually want to use with Linux > so a good clean solution is probably needed. I think the knobs to "flood" and "learn" are important. The hardware seems to have the "flood" but not the "learn/discover". I think the s/ware bridge needs to have both. At the moment - as pointed out in that *NEIGH* notification, s/w bridge assumes a policy that could be considered a security flaw in some circles - just because you are my neighbor does not mean i trust you to come into my house; i may trust you partially and allow you only to come through the front door. Even in Canada with a default policy of not locking your door we sometimes lock our doors ;-> > I have no problem with drawing the line here and trying to implement something > over PF_BRIDGE:RTM_xxx nlmsgs. My comment/concern was in regard to the bridge built-in policy of reading from the neighbor updates (refer to above comments) cheers, jamal -- 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 2/15/2012 6:10 AM, Jamal Hadi Salim wrote: > On Tue, 2012-02-14 at 10:57 -0800, John Fastabend wrote: > >> Roopa was likely on the right track here, >> >> http://patchwork.ozlabs.org/patch/123064/ > > Doesnt seem related to the bridging stuff - the modeling looks > reasonable however. > The operations are really the same ADD/DEL/GET additional MAC addresses to a port, in this case a macvlan type port. The difference is the macvlan port type drops any packet with an address not in the FDB where the bridge type floods these. >> But I think the proper syntax is to use the existing PF_BRIDGE:RTM_XXX >> netlink messages. And if possible drive this without extending ndo_ops. >> >> An ideal user space interaction IMHO would look like, >> >> [root@jf-dev1-dcblab iproute2]# ./br/br fdb add 52:e5:62:7b:57:88 dev veth10 >> [root@jf-dev1-dcblab iproute2]# ./br/br fdb >> port mac addr flags >> veth2 36:a6:35:9b:96:c4 local >> veth4 aa:54:b0:7b:42:ef local >> veth0 2a:e8:5c:95:6c:1b local >> veth6 6e:26:d5:43:a3:36 local >> veth0 f2:c1:39:76:6a:fb >> veth8 4e:35:16:af:87:13 local >> veth10 52:e5:62:7b:57:88 static >> veth10 aa:a9:35:21:15:c4 local > > Looks nice, where is the targeted bridge(eg br0) in that syntax? [root@jf-dev1-dcblab src]# br fdb help Usage: br fdb { add | del | replace } ADDR dev DEV br fdb {show} [ dev DEV ] In my example I just dumped all bridge devices, #br fdb show dev bridge0 > >> Using Stephen's br tool. First command adds FDB entry to SW bridge and >> if the same tool could be used to add entries to embedded bridge I think >> that would be the best case. > > That would be nice (although adds dependency on the presence of the > s/ware bridge). Would be nicer to have either a knob in the kernel to > say "synchronize with h/w bridge foo" which can be turned off. > Seems we need both a synchronize and a { add | del | replace } option. >> So no RTNETLINK error on the second cmd. Then >> embedded FDB entries could be dumped this way also so I get a complete view >> of my FDB setup across multiple sw bridges and embedded bridges. > > So if you had multiple h/ware bridges - which one is tied to br0? > Not sure I follow but does the additional dev parameter above answer this? > >> Yes. The hardware has a bit to support this which is currently not exposed >> to user space. That's a case where we have 'yet another knob' that needs >> a clean solution. This causes real bugs today when users try to use the >> macvlan devices in VEPA mode on top of SR-IOV. By the way these modes are >> all part of the 802.1Qbg spec which people actually want to use with Linux >> so a good clean solution is probably needed. > > > I think the knobs to "flood" and "learn" are important. The hardware > seems to have the "flood" but not the "learn/discover". I think the > s/ware bridge needs to have both. At the moment - as pointed out in that > *NEIGH* notification, s/w bridge assumes a policy that could be > considered a security flaw in some circles - just because you are my > neighbor does not mean i trust you to come into my house; i may trust > you partially and allow you only to come through the front door. Even in > Canada with a default policy of not locking your door we sometimes lock > our doors ;-> > > >> I have no problem with drawing the line here and trying to implement something >> over PF_BRIDGE:RTM_xxx nlmsgs. > > > My comment/concern was in regard to the bridge built-in policy of > reading from the neighbor updates (refer to above comments) > So I think what your saying is a per port bit to disable learning... hmm but if you start tweaking it too much it looks less and less like a 802.1D bridge and more like something you would want to build with tc or openvswitch or tc+bridge or tc+macvlan. .John > cheers, > jamal > > -- 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
[I'm just catching up with this after getting my own driver changes into shape.] On Fri, 2012-02-10 at 10:18 -0500, jamal wrote: > Hi John, > > I went backwards to summarize at the top after going through your email. > > TL;DR version 0.1: > you provide a good use case where it makes sense to do things in the > kernel. IMO, you could make the same arguement if your embedded switch > could do ACLs, IPv4 forwarding etc. And the kernel bloats. > I am always bigoted to move all policy control to user space instead of > bloating in the kernel. [...] > > Now here is the potential issue, > > > > (G) The frame transmitted from ethx.y with the destination address of > > veth0 but the embedded switch is not a learning switch. If the FDB > > update is done in user space its possible (likely?) that the FDB > > entry for veth0 has not been added to the embedded switch yet. > > Ok, got it - so the catch here is the switch is not capable of learning. > I think this depends on where learning is done. Your intent is to > use the S/W bridge as something that does the learning for you i.e in > the kernel. This makes the s/w bridge part of MUST-have-for-this-to-run. > And that maybe the case for your use case. [...] Well, in addition, there are SR-IOV network adapters that don't have any bridge. For these, the software bridge is necessary to handle multicast, broadcast and forwarding between local ports, not only to do learning. Solarflare's implementation of accelerated guest networking (which Shradha and I are gradually sending upstream) builds on libvirt's existing support for software bridges and assigns VFs to guests as a means to offload some of the forwarding. If and when we implement a hardware bridge, we would probably still want to keep the software bridge as a fallback. If a guest is dependent on a VF that's connected to a hardware bridge, it becomes impossible or at least very disruptive to migrate it to another host that doesn't have a compatible VF available. Ben.
Hello, Please find my comments inline. Regards, Shradha Shah On 02/16/2012 03:58 AM, Ben Hutchings wrote: > [I'm just catching up with this after getting my own driver changes into > shape.] > > On Fri, 2012-02-10 at 10:18 -0500, jamal wrote: >> Hi John, >> >> I went backwards to summarize at the top after going through your email. >> >> TL;DR version 0.1: >> you provide a good use case where it makes sense to do things in the >> kernel. IMO, you could make the same arguement if your embedded switch >> could do ACLs, IPv4 forwarding etc. And the kernel bloats. >> I am always bigoted to move all policy control to user space instead of >> bloating in the kernel. > [...] >>> Now here is the potential issue, >>> >>> (G) The frame transmitted from ethx.y with the destination address of >>> veth0 but the embedded switch is not a learning switch. If the FDB >>> update is done in user space its possible (likely?) that the FDB >>> entry for veth0 has not been added to the embedded switch yet. >> >> Ok, got it - so the catch here is the switch is not capable of learning. >> I think this depends on where learning is done. Your intent is to >> use the S/W bridge as something that does the learning for you i.e in >> the kernel. This makes the s/w bridge part of MUST-have-for-this-to-run. >> And that maybe the case for your use case. > [...] > > Well, in addition, there are SR-IOV network adapters that don't have any > bridge. For these, the software bridge is necessary to handle > multicast, broadcast and forwarding between local ports, not only to do > learning. > > Solarflare's implementation of accelerated guest networking (which > Shradha and I are gradually sending upstream) builds on libvirt's > existing support for software bridges and assigns VFs to guests as a > means to offload some of the forwarding. I am also trying to work with bridging using macvtap. Libvirt supports macvtap in four modes; vepa, bridge, private and passthrough mode. Macvtap used in the bridge mode will work similar to a software bridge and will improve performance. > > If and when we implement a hardware bridge, we would probably still want > to keep the software bridge as a fallback. If a guest is dependent on a > VF that's connected to a hardware bridge, it becomes impossible or at > least very disruptive to migrate it to another host that doesn't have a > compatible VF available. > > Ben. > -- 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, 2012-02-15 at 17:26 -0800, John Fastabend wrote: > On 2/15/2012 6:10 AM, Jamal Hadi Salim wrote: > > On Tue, 2012-02-14 at 10:57 -0800, John Fastabend wrote: > > > >> Roopa was likely on the right track here, > >> > >> http://patchwork.ozlabs.org/patch/123064/ > > > > Doesnt seem related to the bridging stuff - the modeling looks > > reasonable however. > > > > The operations are really the same ADD/DEL/GET additional MAC > addresses to a port, in this case a macvlan type port. The > difference is the macvlan port type drops any packet with an > address not in the FDB where the bridge type floods these. Ok. [the vlan piece really should have been an integrated part of bridging; in the early days this was the case] > [root@jf-dev1-dcblab src]# br fdb help > Usage: br fdb { add | del | replace } ADDR dev DEV > br fdb {show} [ dev DEV ] > > In my example I just dumped all bridge devices, > Ok, makes sense. > Seems we need both a synchronize and a { add | del | replace } option. I am conflicted on this. Not sure if that is a command line thing or something built into a user space daemon. It may be useful to have the command line variant but i feel having a daemon take care of things helps in faster synchronization. I think user space is a good spot to add such functionality (as opposed to the kernel). That way user space can work with h/ware switching such as yours as well as a standalone switching chips (from sillicon vendors like Marvel etc). IMO, the average user doesnt need to be aware of such low level stuff; so the default should be for the user not to be responsible for configuration of synchronization. IOW, I want to just run well understood user interface tools things like ifconfig, ip link etc, the new br tool and not even need to be aware that we are offloading. So as long as s/w br0 is mapping to the bridge on ixgb-0 i dont need to know ixgb0 h/w bridge exists. One last comment: With synchronization there are other challenges when the entry in the hardware conflicts with the entry in software when you intend the behavior to be the same. This is not such a big deal with bridging but becomes more apparent when you start offloading ACLs etc. > So I think what your saying is a per port bit to disable learning... > hmm but if you start tweaking it too much it looks less and less like a > 802.1D bridge and more like something you would want to build with tc or > openvswitch or tc+bridge or tc+macvlan. These are pretty commodity features in most silicon switching chips ive come across. You have a knob to control learning and another to control flooding. cheers, jamal -- 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, 2012-02-16 at 03:58 +0000, Ben Hutchings wrote: > > Well, in addition, there are SR-IOV network adapters that don't have any > bridge. For these, the software bridge is necessary to handle > multicast, broadcast and forwarding between local ports, not only to do > learning. For the scenario where there is no h/w bridge - the s/ware bridge should be usable. There's no way working around that. My contention is only with the case where there is a h/w bridge and there being two FDB tables; one in hardware and another in s/w. And both the h/w and s/w bridges doing flooding and learning. It is desirable to have options to use one or other or both with some synchronization. > Solarflare's implementation of accelerated guest networking (which > Shradha and I are gradually sending upstream) builds on libvirt's > existing support for software bridges and assigns VFs to guests as a > means to offload some of the forwarding. > If and when we implement a hardware bridge, we would probably still want > to keep the software bridge as a fallback. If a guest is dependent on a > VF that's connected to a hardware bridge, it becomes impossible or at > least very disruptive to migrate it to another host that doesn't have a > compatible VF available. In the scheme i described to John in last email, libvirt needs not be aware of existence of hardware offloading (and migration should be transparent of whether h/w bridge exists or not)... cheers, jamal -- 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 2/17/2012 6:28 AM, jamal wrote: > On Wed, 2012-02-15 at 17:26 -0800, John Fastabend wrote: >> On 2/15/2012 6:10 AM, Jamal Hadi Salim wrote: >>> On Tue, 2012-02-14 at 10:57 -0800, John Fastabend wrote: >>> >>>> Roopa was likely on the right track here, >>>> >>>> http://patchwork.ozlabs.org/patch/123064/ >>> >>> Doesnt seem related to the bridging stuff - the modeling looks >>> reasonable however. >>> >> >> The operations are really the same ADD/DEL/GET additional MAC >> addresses to a port, in this case a macvlan type port. The >> difference is the macvlan port type drops any packet with an >> address not in the FDB where the bridge type floods these. > > Ok. > [the vlan piece really should have been an integrated part of bridging; > in the early days this was the case] > > >> [root@jf-dev1-dcblab src]# br fdb help >> Usage: br fdb { add | del | replace } ADDR dev DEV >> br fdb {show} [ dev DEV ] >> >> In my example I just dumped all bridge devices, >> > > Ok, makes sense. > > >> Seems we need both a synchronize and a { add | del | replace } option. > > I am conflicted on this. > Not sure if that is a command line thing or something built into a user > space daemon. It may be useful to have the command line variant but i > feel having a daemon take care of things helps in faster > synchronization. > I think user space is a good spot to add such functionality (as opposed > to the kernel). That way user space can work with h/ware switching such > as yours as well as a standalone switching chips (from sillicon vendors > like Marvel etc). > IMO, the average user doesnt need to be aware of such low level stuff; > so the default should be for the user not to be responsible for > configuration of synchronization. IOW, I want to just run well > understood user interface tools things like ifconfig, ip link etc, the > new br tool and not even need to be aware that we are offloading. > So as long as s/w br0 is mapping to the bridge on ixgb-0 i dont need > to know ixgb0 h/w bridge exists. > Yes I agree that is the goal. > One last comment: > With synchronization there are other challenges when the entry in the > hardware conflicts with the entry in software when you intend the > behavior to be the same. This is not such a big deal with bridging but > becomes more apparent when you start offloading ACLs etc. > OK and these sorts of conflicts certainly don't need to be resolved by kernel code. So I think this is a reasonable reason to drive the synchronization into a user space daemon. > >> So I think what your saying is a per port bit to disable learning... >> hmm but if you start tweaking it too much it looks less and less like a >> 802.1D bridge and more like something you would want to build with tc or >> openvswitch or tc+bridge or tc+macvlan. > > These are pretty commodity features in most silicon switching chips ive > come across. You have a knob to control learning and another to control > flooding. > All right this looks like a follow up patch to me. First build the interface to configure the HW FDB. Then a second series to add a flooding knob which works for both embedded switches and software switches. > cheers, > jamal > -- 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 Fri, 2012-02-17 at 09:10 -0800, John Fastabend wrote: > Yes I agree that is the goal. > > > One last comment: > > With synchronization there are other challenges when the entry in the > > hardware conflicts with the entry in software when you intend the > > behavior to be the same. This is not such a big deal with bridging but > > becomes more apparent when you start offloading ACLs etc. > > > > OK and these sorts of conflicts certainly don't need to be resolved > by kernel code. So I think this is a reasonable reason to drive the > synchronization into a user space daemon. Yep. Thanks for listening John. Waiting to see them patches. cheers, jamal -- 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 2/18/2012 4:41 AM, jamal wrote: > On Fri, 2012-02-17 at 09:10 -0800, John Fastabend wrote: > >> Yes I agree that is the goal. >> >>> One last comment: >>> With synchronization there are other challenges when the entry in the >>> hardware conflicts with the entry in software when you intend the >>> behavior to be the same. This is not such a big deal with bridging but >>> becomes more apparent when you start offloading ACLs etc. >>> >> >> OK and these sorts of conflicts certainly don't need to be resolved >> by kernel code. So I think this is a reasonable reason to drive the >> synchronization into a user space daemon. > > > Yep. > Thanks for listening John. Waiting to see them patches. > > cheers, > jamal > > > +Lennert OK back to this. The last piece is where to put these messages... we could take PF_ROUTE:RTM_*NEIGH PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded switch. PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded switch. PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table The neighbor code is using the PF_UNSPEC protocol type so we won't collide with these unless someone was using PF_ROUTE and relying on falling back to PF_UNSPEC however I couldn't find any programs that did this iproute2 certainly doesn't. And the bridge pieces are using PF_BRIDGE so no collision there. I briefly thought about trying to pull the PF_BRIDGE protocol out and use this for both types but I think its better to leave the bridge code alone and there is also the issue of disambiguating a msg at a port which has both an embedded switch and has SW bridge for a master. Also if there are embedded switches with learning capabilities they might want to trigger events to user space. In this case having a protocol type makes user space a bit easier to manage. I've added Lennert so maybe he can comment I think the Marvell chipsets might support something along these lines. The SR-IOV chipsets I'm aware of _today_ don't do learning. Learning makes the event model more plausible. The other mechanism would be to embed some more attributes into the PF_UNSPEC:RTM_XXXLINK msg however I'm thinking that if we want to support learning and triggering events then we likely also don't want to send these events to every app with RTNLGRP_LINK set. Plus there is already a proliferation of LINK attributes and dumping the FDB out of this seems a bit much but could be done with some bitmasks. Although the current ext_filter_mask u32 doesn't seem to be sufficient for events to trigger this. so much for a short note... Thanks .John -- 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 2/28/2012 8:40 PM, John Fastabend wrote: > On 2/18/2012 4:41 AM, jamal wrote: >> On Fri, 2012-02-17 at 09:10 -0800, John Fastabend wrote: >> >>> Yes I agree that is the goal. >>> >>>> One last comment: >>>> With synchronization there are other challenges when the entry in the >>>> hardware conflicts with the entry in software when you intend the >>>> behavior to be the same. This is not such a big deal with bridging but >>>> becomes more apparent when you start offloading ACLs etc. >>>> >>> >>> OK and these sorts of conflicts certainly don't need to be resolved >>> by kernel code. So I think this is a reasonable reason to drive the >>> synchronization into a user space daemon. >> >> >> Yep. >> Thanks for listening John. Waiting to see them patches. >> >> cheers, >> jamal >> >> >> > > +Lennert > > OK back to this. The last piece is where to put these messages... > we could take PF_ROUTE:RTM_*NEIGH > > PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded > switch. > PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded > switch. > PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table > > The neighbor code is using the PF_UNSPEC protocol type so we won't > collide with these unless someone was using PF_ROUTE and relying on > falling back to PF_UNSPEC however I couldn't find any programs that > did this iproute2 certainly doesn't. And the bridge pieces are using > PF_BRIDGE so no collision there. > > I briefly thought about trying to pull the PF_BRIDGE protocol out > and use this for both types but I think its better to leave the > bridge code alone and there is also the issue of disambiguating a msg > at a port which has both an embedded switch and has SW bridge for a > master. Maybe I gave up too quickly here I could use a bit in the ndm_flags to specify embedded or sw bridge. But would require having the bridge module loaded. > > Also if there are embedded switches with learning capabilities they > might want to trigger events to user space. In this case having > a protocol type makes user space a bit easier to manage. I've > added Lennert so maybe he can comment I think the Marvell chipsets > might support something along these lines. The SR-IOV chipsets I'm > aware of _today_ don't do learning. Learning makes the event model > more plausible. > Just checked looks like the DSA infrastructure has commands to enable STP so guess it is doing learning. > The other mechanism would be to embed some more attributes into the > PF_UNSPEC:RTM_XXXLINK msg however I'm thinking that if we want to > support learning and triggering events then we likely also don't > want to send these events to every app with RTNLGRP_LINK set. > > Plus there is already a proliferation of LINK attributes and dumping > the FDB out of this seems a bit much but could be done with some > bitmasks. Although the current ext_filter_mask u32 doesn't seem to > be sufficient for events to trigger this. > > so much for a short note... > > Thanks > .John > > > > > -- > 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 -- 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 Tue, 2012-02-28 at 20:40 -0800, John Fastabend wrote: > OK back to this. The last piece is where to put these messages... > we could take PF_ROUTE:RTM_*NEIGH > > PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded > switch. > PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded > switch. > PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table > Why RTM_*NEIGH? RTM tends to map to Route/L3 and NEIGH tends to map to ndisc or ARP both tied to IP address resolution. While both ARP/Ndisc may play a role in the user space app populating the FDB, i dont think they are necessary players. Learning could be via a table entry miss and packet redirect to user space. So my suggestion is to use FDB_*ENTRY for names > The neighbor code is using the PF_UNSPEC protocol type so we won't > collide with these unless someone was using PF_ROUTE and relying on > falling back to PF_UNSPEC however I couldn't find any programs that > did this iproute2 certainly doesn't. And the bridge pieces are using > PF_BRIDGE so no collision there. They have to be different calls from the calls that talk to the s/ware bridge. In my opinion, as controversial as this may sound, you need to be flexible enough that some vendor can replace these calls with proprietary calls which are more efficient for their hardware. So a "plugin" to replace these calls in the user space code would be a good idea. Alternatively, you could make that something they do at the driver level i.e from user space to kernel it is "hardware, please addthistotheFDBtable()" call and the implementation of that could be proprietary to the specific hardware. [..] > Also if there are embedded switches with learning capabilities they > might want to trigger events to user space. In this case having > a protocol type makes user space a bit easier to manage. I've > added Lennert so maybe he can comment I think the Marvell chipsets > might support something along these lines. The SR-IOV chipsets I'm > aware of _today_ don't do learning. Learning makes the event model > more plausible. The other events to consider is aging of hardware entries. > The other mechanism would be to embed some more attributes into the > PF_UNSPEC:RTM_XXXLINK msg however I'm thinking that if we want to > support learning and triggering events then we likely also don't > want to send these events to every app with RTNLGRP_LINK set. I think this needs to be a different event message. FDB_TABLEMISS? FDB_EXCEPTION? > Plus there is already a proliferation of LINK attributes and dumping > the FDB out of this seems a bit much but could be done with some > bitmasks. Although the current ext_filter_mask u32 doesn't seem to > be sufficient for events to trigger this. Dumping the FDB table should be something along the lines of FDB_GET with the dump flag. It shouldnt tie to the LINK side of things. cheers, jamal -- 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 Tue, 2012-02-28 at 21:14 -0800, John Fastabend wrote: > Just checked looks like the DSA infrastructure has commands to enable > STP so guess it is doing learning. IIRC, Lennert built some of this stuff tied to the kernel. cheers, jamal -- 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 2/29/2012 5:56 AM, Jamal Hadi Salim wrote: > On Tue, 2012-02-28 at 20:40 -0800, John Fastabend wrote: > >> OK back to this. The last piece is where to put these messages... >> we could take PF_ROUTE:RTM_*NEIGH >> >> PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded >> switch. >> PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded >> switch. >> PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table >> > > Why RTM_*NEIGH? RTM tends to map to Route/L3 and NEIGH tends to map > to ndisc or ARP both tied to IP address resolution. While both ARP/Ndisc > may play a role in the user space app populating the FDB, i dont think > they are necessary players. > Learning could be via a table entry miss and packet redirect to user > space. > So my suggestion is to use FDB_*ENTRY for names > Well I think NETLINK_ROUTE is the most correct type to use in this case. Per netlink.h its for routing and device hooks. #define NETLINK_ROUTE 0 /* Routing/device hook */ And NETLINK_ROUTE msg_types use the RTM_* prefix. The _*NEIGH postfix were merely a copy from the SW BRIDGE code paths. How about, PF_BRIDGE:RTM_FDB_NEWENTRY PF_BRIDGE:RTM_FDB_DELENTRY PF_BRIDGE:RTM_FDB_GETENTRY And a new group RTNLGRP_FDB. Also using NETLINK_ROUTE gives the correct rtnl locking semantics for free. >> The neighbor code is using the PF_UNSPEC protocol type so we won't >> collide with these unless someone was using PF_ROUTE and relying on >> falling back to PF_UNSPEC however I couldn't find any programs that >> did this iproute2 certainly doesn't. And the bridge pieces are using >> PF_BRIDGE so no collision there. > > They have to be different calls from the calls that talk to the s/ware > bridge. In my opinion, as controversial as this may sound, you need to > be flexible enough that some vendor can replace these calls with > proprietary calls which are more efficient for their hardware. So a > "plugin" to replace these calls in the user space code would be a > good idea. Alternatively, you could make that something they do at > the driver level i.e from user space to kernel it is "hardware, please > addthistotheFDBtable()" call and the implementation of that could be > proprietary to the specific hardware. > Agreed. I think adding some ndo_ops for bridging offloads here would work. For example the DSA infrastructure and/or macvlan devices might need this. Along the lines of extending this RFC, [RFC] hardware bridging support for DSA switches http://patchwork.ozlabs.org/patch/16578/ .John -- 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, 29 Feb 2012 09:25:56 -0800 John Fastabend <john.r.fastabend@intel.com> wrote: > On 2/29/2012 5:56 AM, Jamal Hadi Salim wrote: > > On Tue, 2012-02-28 at 20:40 -0800, John Fastabend wrote: > > > >> OK back to this. The last piece is where to put these messages... > >> we could take PF_ROUTE:RTM_*NEIGH > >> > >> PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded > >> switch. > >> PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded > >> switch. > >> PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table > >> > > > > Why RTM_*NEIGH? RTM tends to map to Route/L3 and NEIGH tends to map > > to ndisc or ARP both tied to IP address resolution. While both ARP/Ndisc > > may play a role in the user space app populating the FDB, i dont think > > they are necessary players. > > Learning could be via a table entry miss and packet redirect to user > > space. > > So my suggestion is to use FDB_*ENTRY for names > > > > Well I think NETLINK_ROUTE is the most correct type to use in this > case. Per netlink.h its for routing and device hooks. > > #define NETLINK_ROUTE 0 /* Routing/device hook */ > > And NETLINK_ROUTE msg_types use the RTM_* prefix. The _*NEIGH postfix > were merely a copy from the SW BRIDGE code paths. How about, > > PF_BRIDGE:RTM_FDB_NEWENTRY > PF_BRIDGE:RTM_FDB_DELENTRY > PF_BRIDGE:RTM_FDB_GETENTRY > > And a new group RTNLGRP_FDB. Also using NETLINK_ROUTE gives the correct > rtnl locking semantics for free. > > >> The neighbor code is using the PF_UNSPEC protocol type so we won't > >> collide with these unless someone was using PF_ROUTE and relying on > >> falling back to PF_UNSPEC however I couldn't find any programs that > >> did this iproute2 certainly doesn't. And the bridge pieces are using > >> PF_BRIDGE so no collision there. > > > > They have to be different calls from the calls that talk to the s/ware > > bridge. In my opinion, as controversial as this may sound, you need to > > be flexible enough that some vendor can replace these calls with > > proprietary calls which are more efficient for their hardware. So a > > "plugin" to replace these calls in the user space code would be a > > good idea. Alternatively, you could make that something they do at > > the driver level i.e from user space to kernel it is "hardware, please > > addthistotheFDBtable()" call and the implementation of that could be > > proprietary to the specific hardware. > > > > Agreed. I think adding some ndo_ops for bridging offloads here would > work. For example the DSA infrastructure and/or macvlan devices might > need this. Along the lines of extending this RFC, > > [RFC] hardware bridging support for DSA switches > http://patchwork.ozlabs.org/patch/16578/ I want to see a unified API so that user space control applications (RSTP, TRILL?) can use one set of netlink calls for both software bridge and hardware offloaded bridges. Does this proposal meet that requirement? -- 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 2/29/2012 9:52 AM, Stephen Hemminger wrote: > On Wed, 29 Feb 2012 09:25:56 -0800 > John Fastabend <john.r.fastabend@intel.com> wrote: > >> On 2/29/2012 5:56 AM, Jamal Hadi Salim wrote: >>> On Tue, 2012-02-28 at 20:40 -0800, John Fastabend wrote: >>> >>>> OK back to this. The last piece is where to put these messages... >>>> we could take PF_ROUTE:RTM_*NEIGH >>>> >>>> PF_ROUTE:RTM_NEWNEIGH - Add a new FDB entry to an offloaded >>>> switch. >>>> PF_ROUTE:RTM_DELNEIGH - Delete a FDB entry from an offlaoded >>>> switch. >>>> PF_ROUTE:RTM_GETNEIGH - Dumps the embedded FDB table >>>> >>> >>> Why RTM_*NEIGH? RTM tends to map to Route/L3 and NEIGH tends to map >>> to ndisc or ARP both tied to IP address resolution. While both ARP/Ndisc >>> may play a role in the user space app populating the FDB, i dont think >>> they are necessary players. >>> Learning could be via a table entry miss and packet redirect to user >>> space. >>> So my suggestion is to use FDB_*ENTRY for names >>> >> >> Well I think NETLINK_ROUTE is the most correct type to use in this >> case. Per netlink.h its for routing and device hooks. >> >> #define NETLINK_ROUTE 0 /* Routing/device hook */ >> >> And NETLINK_ROUTE msg_types use the RTM_* prefix. The _*NEIGH postfix >> were merely a copy from the SW BRIDGE code paths. How about, >> >> PF_BRIDGE:RTM_FDB_NEWENTRY >> PF_BRIDGE:RTM_FDB_DELENTRY >> PF_BRIDGE:RTM_FDB_GETENTRY >> >> And a new group RTNLGRP_FDB. Also using NETLINK_ROUTE gives the correct >> rtnl locking semantics for free. >> >>>> The neighbor code is using the PF_UNSPEC protocol type so we won't >>>> collide with these unless someone was using PF_ROUTE and relying on >>>> falling back to PF_UNSPEC however I couldn't find any programs that >>>> did this iproute2 certainly doesn't. And the bridge pieces are using >>>> PF_BRIDGE so no collision there. >>> >>> They have to be different calls from the calls that talk to the s/ware >>> bridge. In my opinion, as controversial as this may sound, you need to >>> be flexible enough that some vendor can replace these calls with >>> proprietary calls which are more efficient for their hardware. So a >>> "plugin" to replace these calls in the user space code would be a >>> good idea. Alternatively, you could make that something they do at >>> the driver level i.e from user space to kernel it is "hardware, please >>> addthistotheFDBtable()" call and the implementation of that could be >>> proprietary to the specific hardware. >>> >> >> Agreed. I think adding some ndo_ops for bridging offloads here would >> work. For example the DSA infrastructure and/or macvlan devices might >> need this. Along the lines of extending this RFC, >> >> [RFC] hardware bridging support for DSA switches >> http://patchwork.ozlabs.org/patch/16578/ > > I want to see a unified API so that user space control applications (RSTP, TRILL?) > can use one set of netlink calls for both software bridge and hardware offloaded > bridges. Does this proposal meet that requirement? > With the patches I sent out last night the same netlink calls are used for both SW and HW with a flag set in ndm_flags to indicate it is a hardware entry. The flag is needed when a port has offload support and is also a slave of a SW bridge. Another option would be to apply the command to both hardware and software tables. This might be good enough and user space would not have to make distinctions between HW and SW bridges. Also helps with my original use case where I want the SW and HW bridge FDBs to be in sync. In response to Jamal's comment I proposed changing the type to RTM_FDB_XXXENTRY but the message contents are the same in both cases. Jamal, so why do "They have to be different calls"? I'm not so sure anymore... moving to RTM_FDB_XXXENTRY saved some refactoring in the bridge module but that is just cosmetic. Thanks, John -- 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, 2012-02-29 at 09:25 -0800, John Fastabend wrote: > Well I think NETLINK_ROUTE is the most correct type to use in this > case. Per netlink.h its for routing and device hooks. > > #define NETLINK_ROUTE 0 /* Routing/device hook */ > > And NETLINK_ROUTE msg_types use the RTM_* prefix. The _*NEIGH postfix > were merely a copy from the SW BRIDGE code paths. How about, > > PF_BRIDGE:RTM_FDB_NEWENTRY > PF_BRIDGE:RTM_FDB_DELENTRY > PF_BRIDGE:RTM_FDB_GETENTRY OK, I guess ;-> > And a new group RTNLGRP_FDB. Nod. > Also using NETLINK_ROUTE gives the correct > rtnl locking semantics for free. makes sense. > Agreed. I think adding some ndo_ops for bridging offloads here would > work. For example the DSA infrastructure and/or macvlan devices might > need this. Along the lines of extending this RFC, > > [RFC] hardware bridging support for DSA switches > http://patchwork.ozlabs.org/patch/16578/ Certainly - thats one approach that is reasonable. Where is Lennert? ;-> I changed his email address to one that i am familiar with. cheers, jamal -- 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, 2012-02-29 at 10:19 -0800, John Fastabend wrote: > > > > I want to see a unified API so that user space control applications (RSTP, TRILL?) > > can use one set of netlink calls for both software bridge and hardware offloaded > > bridges. Does this proposal meet that requirement? > > I dont see any issues with those requirements being met. > Jamal, so why do "They have to be different calls"? I'm not so sure anymore... > moving to RTM_FDB_XXXENTRY saved some refactoring in the bridge module but that > is just cosmetic. I may not want to use the s/ware bridge i.e I may want to use h/ware bridge. I may want to use both. So there are 3 variations there. You need at least 1.5 bits to represent them if you are going to use the same interface. There may be features in either h/ware but not in s/ware and vice-versa. A single interface with flags which say this applies to hware:sware:both would be good, but it may be harder to achieve - thats why i suggested they be different. cheers, jamal -- 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 29, 2012 at 09:25:56AM -0800, John Fastabend wrote: > Agreed. I think adding some ndo_ops for bridging offloads here would > work. For example the DSA infrastructure and/or macvlan devices might > need this. Along the lines of extending this RFC, > > [RFC] hardware bridging support for DSA switches > http://patchwork.ozlabs.org/patch/16578/ > > > .John One place where this might not work well would be macvtap which is not a network device so it doesn't have its own address, instead it inherits one from macvlan.
On 3/1/2012 6:14 AM, Michael S. Tsirkin wrote: > On Wed, Feb 29, 2012 at 09:25:56AM -0800, John Fastabend wrote: >> Agreed. I think adding some ndo_ops for bridging offloads here would >> work. For example the DSA infrastructure and/or macvlan devices might >> need this. Along the lines of extending this RFC, >> >> [RFC] hardware bridging support for DSA switches >> http://patchwork.ozlabs.org/patch/16578/ >> >> >> .John > > One place where this might not work well would be > macvtap which is not a network device so it doesn't have > its own address, instead it inherits one from macvlan. > But is macvtap really doing any forwarding or implementing any RX filters? Took a quick scan and it looks like the forwarding logic is all in the macvlan code paths. In this case I suspect if we enable macvlan then any device built on top of it would work. .John -- 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 3/1/2012 5:36 AM, Jamal Hadi Salim wrote: > On Wed, 2012-02-29 at 10:19 -0800, John Fastabend wrote: > >>> >>> I want to see a unified API so that user space control applications (RSTP, TRILL?) >>> can use one set of netlink calls for both software bridge and hardware offloaded >>> bridges. Does this proposal meet that requirement? >>> > > I dont see any issues with those requirements being met. > >> Jamal, so why do "They have to be different calls"? I'm not so sure anymore... >> moving to RTM_FDB_XXXENTRY saved some refactoring in the bridge module but that >> is just cosmetic. > > I may not want to use the s/ware bridge i.e I may want to use h/ware > bridge. I may want to use both. So there are 3 variations there. You > need at least 1.5 bits to represent them if you are going to use the > same interface. There may be features in either h/ware but not in > s/ware and vice-versa. > A single interface with flags which say this applies to hware:sware:both > would be good, but it may be harder to achieve - thats why i suggested > they be different. > > cheers, > jamal > Hmm so I think what I'll do is this... both: ndm_flags = 0 sw : ndm_flags = NTF_SW_FDB hw : ndm_flags = NTF_HW_FDB Then current tools will work with embedded bridges and software bridges with the interesting case being when a port supporting an offloaded FDB is attached to a SW bridge. Doing both in this case seems to be a reasonable default to me. The tricky bit will be pulling the message handlers out of the ./net/bridge code so that we don't have to always load the bridge module to add entries to a macvlan for example. I need to look at a few other things today but I'll code up a patch for this tomorrow. .John -- 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, 2012-03-01 at 14:17 -0800, John Fastabend wrote: > Hmm so I think what I'll do is this... > > both: ndm_flags = 0 > sw : ndm_flags = NTF_SW_FDB > hw : ndm_flags = NTF_HW_FDB > > Then current tools will work with embedded bridges and software > bridges > with the interesting case being when a port supporting an offloaded > FDB is attached to a SW bridge. Doing both in this case seems to be a > reasonable default to me. Looks good, although it seems like no backward compat is broken, it feels like the default should be whats goin on today i.e s/ware only. IOW, I would make that the 0. cheers, jamal -- 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 Tue, Feb 28, 2012 at 08:40:06PM -0800, John Fastabend wrote: > Also if there are embedded switches with learning capabilities they > might want to trigger events to user space. In this case having > a protocol type makes user space a bit easier to manage. I've > added Lennert so maybe he can comment I think the Marvell chipsets > might support something along these lines. The SR-IOV chipsets I'm > aware of _today_ don't do learning. Learning makes the event model > more plausible. net/dsa currently configures any switch chips in the system to do auto-learning. However, I would much prefer to disable that, and have the switch chip just pass up packets for new source addresses, have Linux do the learning, and then mirror the Linux software FDB into the hardware instead -- that avoids having to manually flush the hardware FDB on certain STP state transitions or having to configure the hardware to use a shorter address learning timeout when we're in the middle of an STP topology change, which are problems we are running into in practice. Just curious -- while your patches allow propagating FDB entries into the hardware, do you also have hooks to tell the hardware which ports are to share address databases? For net/dsa, we currently have: http://patchwork.ozlabs.org/patch/16578/ While I think this is conceptually sound, the implementation is hacky, and I wonder how you've solved it for your setup, and if DSA can piggy-back off that. -- 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, Mar 01, 2012 at 08:36:20AM -0500, Jamal Hadi Salim wrote: > > > I want to see a unified API so that user space control applications (RSTP, TRILL?) > > > can use one set of netlink calls for both software bridge and hardware offloaded > > > bridges. Does this proposal meet that requirement? > > > > > I dont see any issues with those requirements being met. > > > Jamal, so why do "They have to be different calls"? I'm not so sure anymore... > > moving to RTM_FDB_XXXENTRY saved some refactoring in the bridge module but that > > is just cosmetic. > > I may not want to use the s/ware bridge i.e I may want to use h/ware > bridge. I may want to use both. This is a rather common case in embedded wireless routers/access points, where you want to have the 4 LAN ports bridged together with the wlan0 interface. In this scenario, the bridging between the LAN ports is typically done in hardware, and the bridging between the LAN ports and wlan0 in software, but here you have to be careful when you send the packet from the switch chip up the stack to be forwarded to the wlan0 interface to not re-send it to the hardware switch chip ports other than the one that the packet came from. net/dsa currently solves this by not having the hardware handle broadcast packets at all, which circumvents the problem, but for multicast traffic you would still like to be able to do at least the forwarding that can be done in hardware in hardware. (Unicast doesn't have this problem as long as the kernel and the switch chip agree on their view of the FDB.) -- 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 3/5/2012 8:53 AM, Lennert Buytenhek wrote: > On Tue, Feb 28, 2012 at 08:40:06PM -0800, John Fastabend wrote: > >> Also if there are embedded switches with learning capabilities they >> might want to trigger events to user space. In this case having >> a protocol type makes user space a bit easier to manage. I've >> added Lennert so maybe he can comment I think the Marvell chipsets >> might support something along these lines. The SR-IOV chipsets I'm >> aware of _today_ don't do learning. Learning makes the event model >> more plausible. > > net/dsa currently configures any switch chips in the system to do > auto-learning. However, I would much prefer to disable that, and have > the switch chip just pass up packets for new source addresses, have > Linux do the learning, and then mirror the Linux software FDB into > the hardware instead -- that avoids having to manually flush the > hardware FDB on certain STP state transitions or having to configure > the hardware to use a shorter address learning timeout when we're in > the middle of an STP topology change, which are problems we are > running into in practice. > Great. And the plan is we should be able to use the same daemon with minimal changes (currently a flag) to control both sw and hw bridges. > Just curious -- while your patches allow propagating FDB entries > into the hardware, do you also have hooks to tell the hardware which > ports are to share address databases? > Not in the current patches. I don't have hardware right now that can instantiate multiple bridges. When I get some I was hoping to do something similar to this patch and use netlink commands to create/delete bridges and add/remove ports to them. This would be modifying the existing commands to work for both software and hardware bridges. By a bridge instantiation I mean a shared address database in this case. > For net/dsa, we currently have: > > http://patchwork.ozlabs.org/patch/16578/ > > While I think this is conceptually sound, the implementation is hacky, > and I wonder how you've solved it for your setup, and if DSA can > piggy-back off that. Yep anything we come up with should work in both cases. -- 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 Mon, 2012-03-05 at 17:53 +0100, Lennert Buytenhek wrote: > net/dsa currently configures any switch chips in the system to do > auto-learning. So we clearly need the (user configurable) knob to turn on/off learning. I think it should also be upto the admin to decide whether the learning happens in the kernel or user space. > However, I would much prefer to disable that, and have > the switch chip just pass up packets for new source addresses, have > Linux do the learning, and then mirror the Linux software FDB into > the hardware instead -- that avoids having to manually flush the > hardware FDB on certain STP state transitions or having to configure > the hardware to use a shorter address learning timeout when we're in > the middle of an STP topology change, which are problems we are > running into in practice. So in the scenario you are describing then it seems the h/ware has no stp state toggles, correct? In other ASICs i have seen, there is influence from stp state on behavior. > Just curious -- while your patches allow propagating FDB entries > into the hardware, do you also have hooks to tell the hardware which > ports are to share address databases? I think those are missing in this discussion and makes a lot of sense to be part of the interface. > net/dsa currently solves this by not having the hardware handle > broadcast packets at all, which circumvents the problem, but for > multicast traffic you would still like to be able to do at least the > forwarding that can be done in hardware in hardware. (Unicast doesn't > have this problem as long as the kernel and the switch chip agree on > their view of the FDB.) Of course this could represent an interesting opportunity for a DOS. Even at 4 port switch at 100Mbs, hitting 500Kpps to the CPU (I am thinking these tiny switches end up in some tiny MIPS/ARM cpu) could be devastating. How do you deal with that? cheers, jamal -- 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 Tue, Mar 06, 2012 at 08:42:26AM -0500, jamal wrote: > > net/dsa currently configures any switch chips in the system to do > > auto-learning. > > So we clearly need the (user configurable) knob to turn on/off learning. Why so? (I think the switch chips should just never do learning at all..) > I think it should also be upto the admin to decide whether the learning > happens in the kernel or user space. I can't see any point in doing it in userspace. What would be the advantage of that? And based on what would the admin make the decision? > > However, I would much prefer to disable that, and have > > the switch chip just pass up packets for new source addresses, have > > Linux do the learning, and then mirror the Linux software FDB into > > the hardware instead -- that avoids having to manually flush the > > hardware FDB on certain STP state transitions or having to configure > > the hardware to use a shorter address learning timeout when we're in > > the middle of an STP topology change, which are problems we are > > running into in practice. > > So in the scenario you are describing then it seems the h/ware has > no stp state toggles, correct? In other ASICs i have seen, there is > influence from stp state on behavior. It does, there is an STP state field per port in the switch chip, which controls whether learning takes place on this port (in Learning and Forwarding states) and whether packets are forwarded (in the Forwarding state). But e.g. it doesn't automatically flush this port's FDB entries if you move a port from Forwarding to Listening -- the STP state field only controls direct learning and forwarding for received packets. And when you receive a BPDU with the topology change notification bit set, the switch won't automatically shorten the FDB entry timeout for you until the topology change is over, either. > > Just curious -- while your patches allow propagating FDB entries > > into the hardware, do you also have hooks to tell the hardware which > > ports are to share address databases? > > I think those are missing in this discussion and makes a lot of > sense to be part of the interface. Keep in mind that these chips also do VLAN tagging in hardware, and so a scenario like: # brctl addbr br123 # brctl addif br123 lan1.123 # brctl addif br123 lan2.123 is also one that can be handled in hardware (which the current patchwork patch doesn't handle yet). > > net/dsa currently solves this by not having the hardware handle > > broadcast packets at all, which circumvents the problem, but for > > multicast traffic you would still like to be able to do at least the > > forwarding that can be done in hardware in hardware. (Unicast doesn't > > have this problem as long as the kernel and the switch chip agree on > > their view of the FDB.) > > Of course this could represent an interesting opportunity for a DOS. > Even at 4 port switch at 100Mbs, hitting 500Kpps to the CPU (I am > thinking these tiny switches end up in some tiny MIPS/ARM cpu) could > be devastating. How do you deal with that? You can let the switch rate limit the number of packets passed up to the CPU. 500 kp/s broadcast traffic seems somewhat excessive in any case, and I'm not sure if this deserves handling apart from QoSing those streams to manageable levels. thanks, Lennert -- 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 Mon, Mar 05, 2012 at 07:45:22PM -0800, John Fastabend wrote: > >> Also if there are embedded switches with learning capabilities they > >> might want to trigger events to user space. In this case having > >> a protocol type makes user space a bit easier to manage. I've > >> added Lennert so maybe he can comment I think the Marvell chipsets > >> might support something along these lines. The SR-IOV chipsets I'm > >> aware of _today_ don't do learning. Learning makes the event model > >> more plausible. > > > > net/dsa currently configures any switch chips in the system to do > > auto-learning. However, I would much prefer to disable that, and have > > the switch chip just pass up packets for new source addresses, have > > Linux do the learning, and then mirror the Linux software FDB into > > the hardware instead -- that avoids having to manually flush the > > hardware FDB on certain STP state transitions or having to configure > > the hardware to use a shorter address learning timeout when we're in > > the middle of an STP topology change, which are problems we are > > running into in practice. > > Great. And the plan is we should be able to use the same daemon with > minimal changes (currently a flag) to control both sw and hw bridges. Why should userspace care at all whether there is a hw bridge present? > > Just curious -- while your patches allow propagating FDB entries > > into the hardware, do you also have hooks to tell the hardware which > > ports are to share address databases? > > Not in the current patches. I don't have hardware right now > that can instantiate multiple bridges. When I get some I was hoping > to do something similar to this patch and use netlink commands > to create/delete bridges and add/remove ports to them. This would > be modifying the existing commands to work for both software and > hardware bridges. In the DSA h/w bridging patch, a hardware address database is only allocated on addif to an existing bridge, not on addbr. For one, at addbr time, you have no idea yet whether there will be any switch chip ports in the bridge port group for this bridge. Also, the h/w has some restrictions on the assignment of address database identifiers (e.g. if you want to bridge between lan1.123 and lan2.123, you have to use address database identifier '123'). > By a bridge instantiation I mean a shared address database in this case. Makes sense. (I'd add STP port states to this.) -- 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 Tue, 2012-03-06 at 15:09 +0100, Lennert Buytenhek wrote: > Why so? (I think the switch chips should just never do learning at > all..) I agree that learning in software gives you more flexibility; however, I am for providing interface flexibility as well - switches have learning features. I think i should be able to use them when it makes sense to. > > I think it should also be upto the admin to decide whether the learning > > happens in the kernel or user space. > > I can't see any point in doing it in userspace. What would be the > advantage of that? And based on what would the admin make the decision? > If i wanted to do some funky access control based on some new MAC address showing up - best place to do it is user space. > It does, there is an STP state field per port in the switch chip, > which controls whether learning takes place on this port (in > Learning and Forwarding states) and whether packets are forwarded > (in the Forwarding state). ok, makes sense. > But e.g. it doesn't automatically flush this port's FDB entries if > you move a port from Forwarding to Listening -- the STP state field > only controls direct learning and forwarding for received packets. > > And when you receive a BPDU with the topology change notification > bit set, the switch won't automatically shorten the FDB entry > timeout for you until the topology change is over, either. I have to go back and look at some manuals i have - but iirc, the ones ive played with behaved similarly. As long as we provide knobs to set/unset those different attributes, I think the handling of all that should be from software (likely some daemon in user space); then it shouldnt matter whether we are working with STP BPDUs or TRILL or thenewprotocolTM etc. > Keep in mind that these chips also do VLAN tagging in hardware, and > so a scenario like: > > # brctl addbr br123 > # brctl addif br123 lan1.123 > # brctl addif br123 lan2.123 > > is also one that can be handled in hardware (which the current > patchwork patch doesn't handle yet). > We would need to work with offloading VLANs, no? Do the current VLAN offloads used for NICs suffice for switching chips as well? i.e typically most chips have a table associated with some port in which the Vlan is partof or is the lookup key. > You can let the switch rate limit the number of packets passed up to > the CPU. 500 kp/s broadcast traffic seems somewhat excessive in any > case, and I'm not sure if this deserves handling apart from QoSing > those streams to manageable levels. Yes, that would provide a solution. I havent seen anything where you can rate limit the learning(SA lookup failure). cheers, jamal -- 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, Mar 07, 2012 at 09:11:40AM -0500, Jamal Hadi Salim wrote: > > Why so? (I think the switch chips should just never do learning at > > all..) > > I agree that learning in software gives you more flexibility; > however, I am for providing interface flexibility as well - switches > have learning features. I think i should be able to use them when it > makes sense to. Since it can lead to problems (address database mismatches, doesn't correctly handle STP transitions or topology changes automatically), I think it should be avoided whenever possible. I don't see any advantages of hardware based learning over software based learning anyway ('flexibility' doesn't seem like a very good argument). > > > I think it should also be upto the admin to decide whether the > > > learning happens in the kernel or user space. > > > > I can't see any point in doing it in userspace. What would be the > > advantage of that? And based on what would the admin make the decision? > > If i wanted to do some funky access control based on some new MAC > address showing up - best place to do it is user space. Alright, that sounds fair. > > Keep in mind that these chips also do VLAN tagging in hardware, and > > so a scenario like: > > > > # brctl addbr br123 > > # brctl addif br123 lan1.123 > > # brctl addif br123 lan2.123 > > > > is also one that can be handled in hardware (which the current > > patchwork patch doesn't handle yet). > > We would need to work with offloading VLANs, no? Yes. > Do the current VLAN offloads used for NICs suffice for switching > chips as well? i.e typically most chips have a table associated > with some port in which the Vlan is partof or is the lookup key. It should be doable along the lines of the current DSA patch -- add a VLAN ID argument to the interface add/remove callbacks, and when a VLAN virtual interface is added to the bridge, call the relevant callbacks with the parent interface + VLAN ID instead. (This doesn't work for stacked VLANs, but the current net/dsa supported chips don't handle those anyway.) -- 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 Mon, 2012-03-12 at 09:48 +0100, Lennert Buytenhek wrote: > Since it can lead to problems (address database mismatches, doesn't > correctly handle STP transitions or topology changes automatically), > I think it should be avoided whenever possible. I don't see any > advantages of hardware based learning over software based learning > anyway ('flexibility' doesn't seem like a very good argument). Indeed address mismatches may happen if you have two databases. You have two choices then: Do learning in user space or be able to tolerate some transient inconsistency (if you have some software that lazily looks at the database). But there is a case where the database sits only in hardware. In such a case, you cant have mismatches. I think the STP problem can be handled by user space regardless of whether address mismatch happens or not. > It should be doable along the lines of the current DSA patch -- > add a VLAN ID argument to the interface add/remove callbacks, and > when a VLAN virtual interface is added to the bridge, call the > relevant callbacks with the parent interface + VLAN ID instead. > (This doesn't work for stacked VLANs, but the current net/dsa > supported chips don't handle those anyway.) Sounds like a good start - we could have a different interface for stacked variants. I think you should push in the patch. cheers, jamal -- 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/include/linux/netdev_features.h b/include/linux/netdev_features.h index 77f5202..5936fae 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h @@ -55,6 +55,8 @@ enum { NETIF_F_NOCACHE_COPY_BIT, /* Use no-cache copyfromuser */ NETIF_F_LOOPBACK_BIT, /* Enable loopback */ + NETIF_F_HW_FDB, /* Hardware supports switching */ + /* * Add your fresh new feature above and remember to update * netdev_features_strings[] in net/core/ethtool.c and maybe diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c index 5ba0c84..4cc545b 100644 --- a/net/bridge/br_fdb.c +++ b/net/bridge/br_fdb.c @@ -81,9 +81,26 @@ static void fdb_rcu_free(struct rcu_head *head) kmem_cache_free(br_fdb_cache, ent); } +static void fdb_hw_delete(struct net_bridge *br, + struct net_bridge_fdb_entry *fdb) +{ + struct net_bridge_port *op; + + rcu_read_lock(); + list_for_each_entry_rcu(op, &br->port_list, list) { + struct net_device *dev = op->dev; + + if ((dev->features & NETIF_F_HW_FDB) && + dev != fdb->dst->dev) + dev_uc_del(dev, fdb->addr.addr); + } + rcu_read_unlock(); +} + static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f) { hlist_del_rcu(&f->hlist); + fdb_hw_delete(br, f); fdb_notify(br, f, RTM_DELNEIGH); call_rcu(&f->rcu, fdb_rcu_free); } @@ -350,6 +367,22 @@ static struct net_bridge_fdb_entry *fdb_find_rcu(struct hlist_head *head, return NULL; } +static void fdb_hw_create(struct net_bridge *br, + struct net_bridge_fdb_entry *fdb) +{ + struct net_bridge_port *op; + + rcu_read_lock(); + list_for_each_entry_rcu(op, &br->port_list, list) { + struct net_device *dev = op->dev; + + if ((dev->features & NETIF_F_HW_FDB) && + dev != fdb->dst->dev) + dev_uc_add(dev, fdb->addr.addr); + } + rcu_read_unlock(); +} + static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, struct net_bridge_port *source, const unsigned char *addr) @@ -363,6 +396,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head, fdb->is_local = 0; fdb->is_static = 0; fdb->updated = fdb->used = jiffies; + fdb_hw_create(source->br, fdb); hlist_add_head_rcu(&fdb->hlist, head); } return fdb;
Propagate software FDB table into hardware uc, mc lists when the NETIF_F_HW_FDB is set. This resolves the case below where an embedded switch is used in hardware to do inter-VF or VF-PF switching. This patch pushes the FDB entry (specifically the MAC address) into the embedded switch with dev_add_uc and dev_add_mc so the switch "learns" about the software bridge. veth0 veth2 | | ------------ | bridge0 | <---- software bridging ------------ / / ethx.y ethx VF PF \ \ <---- propagate FDB entries to HW \ \ -------------------- | Embedded Bridge | <---- hardware offloaded switching -------------------- This is only an RFC couple more changes are needed. (1) Optimize HW FDB set/del to only walk list if an FDB offloaded device is attached. Or decide it doesn't matter from unlikely() path. (2) Is it good enough to just call dev_uc_{add|del} or dev_mc_{add|del}? Or do some devices really need a new netdev callback to do this operation correctly. I think it should be good enough as is. (3) wrapped list walk in rcu_read_lock() just in case maybe every case is already inside rcu_read_lock()/unlock(). Also this is in response to this thread regarding the macvlan and exposing rx filters posting now to see if folks think this is the right idea and if it will resolve at least the bridge case. http://lists.openwall.net/netdev/2011/11/08/135 Signed-off-by: John Fastabend <john.r.fastabend@intel.com> --- include/linux/netdev_features.h | 2 ++ net/bridge/br_fdb.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 0 deletions(-) -- 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