HW bridging support using notifiers?
Subsystems (2)
Hi all,
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an action
So here's what I am up to now:
- use the NETDEV_JOIN notifier to discover when a bridge port is added
- use the NETDEV_LEAVE notifier, still need to verify this does not break netconsole as indicated in net/bridge/br_if.c
- use the NETDEV_CHANGEINFODATA notifier to notify about STP state changes
Now, this raises a bunch of questions:
- we would need a getter to return the stp state of a given network device when called with NETDEV_CHANGEINFODATA, is that acceptable? This would be the first function exported by the bridge layer to expose internal data
NB: this also raises the question of the race condition and locking within br_set_stp_state() and when the network devices notifier callback runs
- or do we need a new network device notifier accepting an opaque pointer which could provide us with the data we what, something like
this: call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info), where info would be a structure/union telling what's this data about
Let me know what you think, thanks!
[1]: http://patchwork.ozlabs.org/patch/16578/
--
Florian
Quoted 10 lines · Florian Fainelli
Hi all,
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct
net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an action
Quoted 5 lines
So here's what I am up to now:
- use the
NETDEV_JOINnotifier to discover when a bridge port is added
- use theNETDEV_LEAVEnotifier, still need to verify this does not break netconsole as indicated innet/bridge/br_if.c
I can’t find a NETDEV_LEAVE...is this new?
For this, rocker is using netdev_notifier and watching for NETDEV_CHANGEUPPER. On CHANGEUPPER, use netdev_master_upper_dev_get(dev) to get master. If master and master rtnl_link_ops->kind is “bridge”, then dev port is in bridge; otherwise port is not in bridge. Of course, master could be “openvswitch”, if the driver swings both ways.
Would this approach work for DSA?
Quoted 9 lines
- use the
NETDEV_CHANGEINFODATAnotifier to notify about STP state changesNow, this raises a bunch of questions:
- we would need a getter to return the stp state of a given network device when called with
NETDEV_CHANGEINFODATA, is that acceptable? This would be the first function exported by the bridge layer to expose internal data
Quoted 9 lines
NB: this also raises the question of the race condition and locking within
br_set_stp_state()and when the network devices notifier callback runs- or do we need a new network device notifier accepting an opaque pointer which could provide us with the data we what, something like
this:call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info), where info would be a structure/union telling what's this data about
We need STP state change notification for rocker also, so we can install/remove STP/ARP filters on port state change. Netdev_notifier would work. I was also thinking about using Jiri’s ndo_swdev_flow_install/remove to install/remove the STP filters on the port, rather than using netdev_notifier. In other words, does the HW bridge driver need to know the STP state or can it be dumb (stateless) and told when to accept STP BPDUs, or not, using swdev_flow construct:
dst_mac 01:80:c2:00:00:00 lasp 0x4242 in_port <port ifindex> actions output <br ifindex>
and later when reaching LEARNING state:
dst_mac ff:ff:ff:ff:ff:ff eth_type ARP in_port <port ifindex> actions output <br ifindex>
and finally when reaching FORWARDING state, the learned/static bridge fdbs:
dst_mac <neigh_mac> in_port <br ifindex> actions output <port ifindex>
So driver doesn’t really know what STP is; it’s just installs/removes port filter when told to, using the common ndo_swdev_flow API. The smarts stay in the bridge driver.
Quoted 5 lines
Let me know what you think, thanks!
[1]: http://patchwork.ozlabs.org/patch/16578/
--
Florian
-scott
Quoted 15 lines · Fri, Oct 03, 2014 at 07:13:35AM CEST, [email protected] wrote:
Quoted 12 lines · Florian Fainelli
Hi all,
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct
net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an actionSo here's what I am up to now:
Quoted 7 lines
Quoted 3 lines
- use the
NETDEV_JOINnotifier to discover when a bridge port is added
- use theNETDEV_LEAVEnotifier, still need to verify this does not break netconsole as indicated innet/bridge/br_if.cI can’t find a NETDEV_LEAVE...is this new?
For this, rocker is using
netdev_notifierand watching forNETDEV_CHANGEUPPER. On CHANGEUPPER, usenetdev_master_upper_dev_get(dev)to get master. If master and masterrtnl_link_ops->kindis “bridge”, then dev port is in bridge; otherwise port is not in bridge. Of course, master could be “openvswitch”, if the driver swings both ways.
I agree that NETDEV_CHANGEUPPER should be used for this.
Quoted 48 lines
Would this approach work for DSA?
Quoted 9 lines
- use the
NETDEV_CHANGEINFODATAnotifier to notify about STP state changesNow, this raises a bunch of questions:
- we would need a getter to return the stp state of a given network device when called with
NETDEV_CHANGEINFODATA, is that acceptable? This would be the first function exported by the bridge layer to expose internal dataQuoted 9 lines
NB: this also raises the question of the race condition and locking within
br_set_stp_state()and when the network devices notifier callback runs- or do we need a new network device notifier accepting an opaque pointer which could provide us with the data we what, something like
this:call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info), where info would be a structure/union telling what's this data aboutWe need STP state change notification for rocker also, so we can install/remove STP/ARP filters on port state change.
Netdev_notifierwould work. I was also thinking about using Jiri’s ndo_swdev_flow_install/remove to install/remove the STP filters on the port, rather than usingnetdev_notifier. In other words, does the HW bridge driver need to know the STP state or can it be dumb (stateless) and told when to accept STP BPDUs, or not, usingswdev_flowconstruct:
dst_mac01:80:c2:00:00:00 lasp0x4242in_port<port ifindex> actions output <br ifindex>and later when reaching LEARNING state:
dst_macff:ff:ff:ff:ff:ffeth_typeARPin_port<port ifindex> actions output <br ifindex>and finally when reaching FORWARDING state, the learned/static bridge fdbs:
dst_mac<neigh_mac>in_port<br ifindex> actions output <port ifindex>So driver doesn’t really know what STP is; it’s just installs/removes port filter when told to, using the common
ndo_swdev_flowAPI. The smarts stay in the bridge driver.Quoted 5 lines
Let me know what you think, thanks!
[1]: http://patchwork.ozlabs.org/patch/16578/
--
Florian-scott
Hi Florian et al,
Quoted 4 lines · Florian Fainelli
Hi all,
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
I've been working on support for the RTL8366S switch, and our work is directly overlapping here. I actually have something that is working at configuring port and tag based vlans on the RTL8366S. I'll try to clean up the code to post something for discussion over the next couple of days.
Quoted 4 lines
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct
net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an action
I think having ndo implementer methods for hardware switch offloads makes more sense. Such a scheme is needed in order to implement the stacking of devices that is required in order to transparently handle configuration of vlans on switch ports where the 8021q device has to pass on the vlan tag to the switch device. The ndo methods do perform an action of causing the switch to be configured to match the bridge config. Additionally, they can be used to veto changes that cannot be offloaded to hardware -- this (configurable) behaviour is desired by some users of these APIs who wish to be made aware when a particuarly configuration is not supported by the underlying hardware.
Quoted 6 lines
So here's what I am up to now:
- use the
NETDEV_JOINnotifier to discover when a bridge port is added
- use theNETDEV_LEAVEnotifier, still need to verify this does not break netconsole as indicated innet/bridge/br_if.c
- use theNETDEV_CHANGEINFODATAnotifier to notify about STP state changes
To me, notifiers are the wrong model for join and leave. Implementing stacking on top of notifiers is somewhat more complicated. Here are the ndo methods I've implemented so far which are sufficient for basic config of the RTL8366S. They're fairly similar to those in [1].
+ int (*ndo_join_bridge)(struct net_bridge *bridge,
+ struct net_device *dev, + int *switch_nr, + int *switch_port_nr, + int vlan); + int (*ndo_leave_bridge)(struct net_bridge *bridge, + struct net_device *dev, + int switch_nr, + int switch_port_nr, + int vlan);
+ int (*ndo_flood_xmit)(struct switch_info *dev,
+ struct sk_buff *skb,
+ u64 port_mask);
There are a couple of important points here. In the case of joining and leaving a bridge, the bridge needs to be provided with information it can use to identify switch ports. This is needed in order to offload the flooding of packets to multiple ports, as otherwise the Linux bridge code doesn't have any way to figure out which packets can be merged into a single transmit via the ndo_flood_xmit() method.
Quoted 6 lines
Now, this raises a bunch of questions:
- we would need a getter to return the stp state of a given network device when called with
NETDEV_CHANGEINFODATA, is that acceptable? This would be the first function exported by the bridge layer to expose internal data
I have yet to dig into STP, so I'll refrain from commenting on these parts for now.
Quoted 3 lines
NB: this also raises the question of the race condition and locking within
br_set_stp_state()and when the network devices notifier callback runs
U
Quoted 8 lines
- or do we need a new network device notifier accepting an opaque pointer which could provide us with the data we what, something like
this:call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info), where info would be a structure/union telling what's this data aboutLet me know what you think, thanks!
Thanks for the pointer to this. Cheers!
-ben
Quoted 6 lines
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 1 line
"Thought is the essence of where you are now."
Hi Benjamin,
Quoted 5 lines · Benjamin LaHaise
Hi Florian et al,
Quoted 2 lines · Florian Fainelli
Hi all,
Quoted 7 lines
Quoted 2 lines
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
I've been working on support for the RTL8366S switch, and our work is directly overlapping here. I actually have something that is working at configuring port and tag based vlans on the RTL8366S. I'll try to clean up the code to post something for discussion over the next couple of days.
Cool, please do!
Quoted 6 lines
Quoted 5 lines
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct
net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an action
Quoted 10 lines
I think having ndo implementer methods for hardware switch offloads makes more sense. Such a scheme is needed in order to implement the stacking of devices that is required in order to transparently handle configuration of vlans on switch ports where the 8021q device has to pass on the vlan tag to the switch device. The ndo methods do perform an action of causing the switch to be configured to match the bridge config. Additionally, they can be used to veto changes that cannot be offloaded to hardware -- this (configurable) behaviour is desired by some users of these APIs who wish to be made aware when a particuarly configuration is not supported by the underlying hardware.
Humm, that's a fair point, so not only would we want new NDOs, but we'd also need to specify the return values (invalid, no space etc...).
As far as bridging alone is concerned (not including VLANs for now), I don't think there are restrictions in terms of what the hardware can do, since we mostly tell it to "group" N-ports together.
For VLANs, there should be a way for the switch driver to tell whether that's supported or not.
Quoted 16 lines
Quoted 7 lines
So here's what I am up to now:
- use the
NETDEV_JOINnotifier to discover when a bridge port is added
- use theNETDEV_LEAVEnotifier, still need to verify this does not break netconsole as indicated innet/bridge/br_if.c
- use theNETDEV_CHANGEINFODATAnotifier to notify about STP state changesTo me, notifiers are the wrong model for join and leave. Implementing stacking on top of notifiers is somewhat more complicated. Here are the ndo methods I've implemented so far which are sufficient for basic config of the RTL8366S. They're fairly similar to those in [1].
(quoted diff) +3
-0+ int (*ndo_join_bridge)(struct net_bridge *bridge, + struct net_device *dev, + int *switch_nr,
Review
Quoted 10 lines
(quoted diff) +10
-0+ int *switch_port_nr, + int vlan); + int (*ndo_leave_bridge)(struct net_bridge *bridge, + struct net_device *dev, + int switch_nr, + int switch_port_nr, + int vlan); + int (*ndo_flood_xmit)(struct switch_info *dev, + struct sk_buff *skb, + u64 port_mask);
I don't think the switch_port_nr belongs here, this is something that should be resolved within the implementer of these ndo's, whether that is DSA, or Jiri's switchdev, since the net_device argument should be linked to both the switch port number, and the switch number.
Quoted 7 lines
There are a couple of important points here. In the case of joining and leaving a bridge, the bridge needs to be provided with information it can use to identify switch ports. This is needed in order to offload the flooding of packets to multiple ports, as otherwise the Linux bridge code doesn't have any way to figure out which packets can be merged into a single transmit via the
ndo_flood_xmit()method.
I am not exactly sure yet how ndo_flood_xmit() fits in the picture here, but it might be optional based on how the switch has been configured I presume?
Quoted 10 lines
Quoted 7 lines
Now, this raises a bunch of questions:
- we would need a getter to return the stp state of a given network device when called with
NETDEV_CHANGEINFODATA, is that acceptable? This would be the first function exported by the bridge layer to expose internal dataI have yet to dig into STP, so I'll refrain from commenting on these parts for now.
The idea is to use the Linux STP result and apply the results to the bridge port/switch ports members directly, since switches (at least those from Marvell and Broadcom) have that semantic built into their hardware logic.
Quoted 25 lines
Quoted 4 lines
NB: this also raises the question of the race condition and locking within
br_set_stp_state()and when the network devices notifier callback runsU
Quoted 8 lines
- or do we need a new network device notifier accepting an opaque pointer which could provide us with the data we what, something like
this:call_netdevices_notifier_data(NETDEV_CHANGEINFODATA, dev, info), where info would be a structure/union telling what's this data aboutLet me know what you think, thanks!
Thanks for the pointer to this. Cheers!
-ben
Quoted 6 lines
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 38 lines · Florian Fainelli
Hi Benjamin,
Quoted 5 lines · Benjamin LaHaise
Hi Florian et al,
Quoted 2 lines · Florian Fainelli
Hi all,
Quoted 7 lines
Quoted 2 lines
I am taking a look at adding HW bridging support to DSA, in way that's usable outside of DSA.
I've been working on support for the RTL8366S switch, and our work is directly overlapping here. I actually have something that is working at configuring port and tag based vlans on the RTL8366S. I'll try to clean up the code to post something for discussion over the next couple of days.
Cool, please do!
Quoted 6 lines
Quoted 5 lines
Lennert's approach in 2008 [1] looks conceptually good to me,as he noted, it uses a bunch of new ndo's which is not only limiting to one ndo implementer per struct
net_device, but also is mostly consuming the information from the bridge layer, while the ndo is an actionQuoted 10 lines
I think having ndo implementer methods for hardware switch offloads makes more sense. Such a scheme is needed in order to implement the stacking of devices that is required in order to transparently handle configuration of vlans on switch ports where the 8021q device has to pass on the vlan tag to the switch device. The ndo methods do perform an action of causing the switch to be configured to match the bridge config. Additionally, they can be used to veto changes that cannot be offloaded to hardware -- this (configurable) behaviour is desired by some users of these APIs who wish to be made aware when a particuarly configuration is not supported by the underlying hardware.
Humm, that's a fair point, so not only would we want new NDOs, but we'd also need to specify the return values (invalid, no space etc...).
Quoted 6 lines
As far as bridging alone is concerned (not including VLANs for now), I don't think there are restrictions in terms of what the hardware can do, since we mostly tell it to "group" N-ports together.
For VLANs, there should be a way for the switch driver to tell whether that's supported or not.
What the hardware can support varies widely. For example, the RTL8366S happens to support a total of 8 FDBs in hardware, which, given how the Linux bridge works, implies a total of at most 8 VLANs. However, it can use more VLANs if they share overlapping FDBs (which Linux doesn't support). There are also features like VLAN remapping, q-in-q support... We're going to have to do a fair amount of work to learn about all these quirks of hardware features that need to be identified and reported.
Quoted 23 lines
Quoted 23 lines
Quoted 7 lines
So here's what I am up to now:
- use the
NETDEV_JOINnotifier to discover when a bridge port is added
- use theNETDEV_LEAVEnotifier, still need to verify this does not break netconsole as indicated innet/bridge/br_if.c
- use theNETDEV_CHANGEINFODATAnotifier to notify about STP state changesTo me, notifiers are the wrong model for join and leave. Implementing stacking on top of notifiers is somewhat more complicated. Here are the ndo methods I've implemented so far which are sufficient for basic config of the RTL8366S. They're fairly similar to those in [1].
(quoted diff) +10
-0+ int (*ndo_join_bridge)(struct net_bridge *bridge, + struct net_device *dev, + int *switch_nr, + int *switch_port_nr, + int vlan); + int (*ndo_leave_bridge)(struct net_bridge *bridge, + struct net_device *dev, + int switch_nr, + int switch_port_nr, + int vlan);
Quoted 8 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ int (*ndo_flood_xmit)(struct switch_info *dev, + struct sk_buff *skb, + u64 port_mask);I don't think the
switch_port_nrbelongs here, this is something that should be resolved within the implementer of these ndo's, whether that is DSA, or Jiri's switchdev, since thenet_deviceargument should be linked to both the switch port number, and the switch number.
The switch_port_nr is absolutely required for flood offloading. (more below)
Quoted 11 lines
Quoted 7 lines
There are a couple of important points here. In the case of joining and leaving a bridge, the bridge needs to be provided with information it can use to identify switch ports. This is needed in order to offload the flooding of packets to multiple ports, as otherwise the Linux bridge code doesn't have any way to figure out which packets can be merged into a single transmit via the
ndo_flood_xmit()method.I am not exactly sure yet how
ndo_flood_xmit()fits in the picture here, but it might be optional based on how the switch has been configured I presume?
ndo_flood_xmit() is a method that sends a single packet to a bitmask of the ports attached to the switch. This is quite useful for saving bandwidth on the CPU port of a switch when sending out broadcast packets, and, more importantly, multicast packets. The bits in that bitmask correspond to the switch_port_nr reported ny ndo_join_bridge(), and I modified the Linux bridge code to group ports attached to the same switch together and use the switch_nr to identify that ports are on the same switch and collapse flooding to multiple ports into a single call of ndo_flood_xmit(). The RTL8366S has support for this feature (that's why I implemented it), and I'm pretty sure other switches do as well -- at the very least I know one of the Marvell switches I was exposed to in the past that had this capability, but I don't recall the precise details of the interface since I wasn't directly involved in the coding for that driver.
I'm sure there are other hardware features we'll have to come up with a model for. Cheers,
-ben
Signature · 1 line
"Thought is the essence of where you are now."