| Message ID | 1566500850-6247-2-git-send-email-horatiu.vultur@microchip.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add NETIF_F_HW_BRIDGE feature | expand |
> +/* Determin if the SW bridge can be offloaded to HW. Return true if all > + * the interfaces of the bridge have the feature NETIF_F_HW_SWITCHDEV set > + * and have the same netdev_ops. > + */ Hi Horatiu Why do you need these restrictions. The HW bridge should be able to learn that a destination MAC address can be reached via the SW bridge. The software bridge can then forward it out the correct interface. Or are you saying your hardware cannot learn from frames which come from the CPU? Andrew
The 08/22/2019 22:08, Andrew Lunn wrote: > External E-Mail > > > > +/* Determin if the SW bridge can be offloaded to HW. Return true if all > > + * the interfaces of the bridge have the feature NETIF_F_HW_SWITCHDEV set > > + * and have the same netdev_ops. > > + */ > > Hi Horatiu > > Why do you need these restrictions. The HW bridge should be able to > learn that a destination MAC address can be reached via the SW > bridge. The software bridge can then forward it out the correct > interface. > > Or are you saying your hardware cannot learn from frames which come > from the CPU? > > Andrew > Hi Andrew, I do not believe that our HW can learn from frames which comes from the CPU, at least not in the way they are injected today. But in case of Ocelot (and the next chip we are working on), we have other issues in mixing with foreign interfaces which is why we have the check in ocelot_netdevice_dev_check. More important, as we responded to Nikolay, we properly introduced this restriction for the wrong reasons. In SW bridge I will remove all these restrictions and only set ports in promisc mode only if NETIF_F_HW_BRIDGE is not set. Then in the network driver I can see if a foreign interface is added to the bridge, and when that happens I can set the port in promisc mode. Then the frames will be flooded to the SW bridge which eventually will send to the foreign interface.
On 8/23/19 5:39 AM, Horatiu Vultur wrote: > The 08/22/2019 22:08, Andrew Lunn wrote: >> External E-Mail >> >> >>> +/* Determin if the SW bridge can be offloaded to HW. Return true if all >>> + * the interfaces of the bridge have the feature NETIF_F_HW_SWITCHDEV set >>> + * and have the same netdev_ops. >>> + */ >> >> Hi Horatiu >> >> Why do you need these restrictions. The HW bridge should be able to >> learn that a destination MAC address can be reached via the SW >> bridge. The software bridge can then forward it out the correct >> interface. >> >> Or are you saying your hardware cannot learn from frames which come >> from the CPU? >> >> Andrew >> > Hi Andrew, > > I do not believe that our HW can learn from frames which comes from the > CPU, at least not in the way they are injected today. But in case of Ocelot > (and the next chip we are working on), we have other issues in mixing with > foreign interfaces which is why we have the check in > ocelot_netdevice_dev_check. > > More important, as we responded to Nikolay, we properly introduced this > restriction for the wrong reasons. > > In SW bridge I will remove all these restrictions and only set ports in > promisc mode only if NETIF_F_HW_BRIDGE is not set. > Then in the network driver I can see if a foreign interface is added to > the bridge, and when that happens I can set the port in promisc mode. > Then the frames will be flooded to the SW bridge which eventually will > send to the foreign interface. Is that really necessary? Is not the skb->fwd_offload_mark as well as the phys_switch_id supposed to tell that information to the bridge already?
The 08/23/2019 16:30, Florian Fainelli wrote: > External E-Mail > > > On 8/23/19 5:39 AM, Horatiu Vultur wrote: > > The 08/22/2019 22:08, Andrew Lunn wrote: > >> External E-Mail > >> > >> > >>> +/* Determin if the SW bridge can be offloaded to HW. Return true if all > >>> + * the interfaces of the bridge have the feature NETIF_F_HW_SWITCHDEV set > >>> + * and have the same netdev_ops. > >>> + */ > >> > >> Hi Horatiu > >> > >> Why do you need these restrictions. The HW bridge should be able to > >> learn that a destination MAC address can be reached via the SW > >> bridge. The software bridge can then forward it out the correct > >> interface. > >> > >> Or are you saying your hardware cannot learn from frames which come > >> from the CPU? > >> > >> Andrew > >> > > Hi Andrew, > > > > I do not believe that our HW can learn from frames which comes from the > > CPU, at least not in the way they are injected today. But in case of Ocelot > > (and the next chip we are working on), we have other issues in mixing with > > foreign interfaces which is why we have the check in > > ocelot_netdevice_dev_check. > > > > More important, as we responded to Nikolay, we properly introduced this > > restriction for the wrong reasons. > > > > In SW bridge I will remove all these restrictions and only set ports in > > promisc mode only if NETIF_F_HW_BRIDGE is not set. > > Then in the network driver I can see if a foreign interface is added to > > the bridge, and when that happens I can set the port in promisc mode. > > Then the frames will be flooded to the SW bridge which eventually will > > send to the foreign interface. > > Is that really necessary? From what I see, it seems to be necessary. > Is not the skb->fwd_offload_mark as well as > the phys_switch_id supposed to tell that information to the bridge already? Yes, the bridge is using the fwd_offload_mark to know that it should or should not forward the frame. But in case that the network driver knows that the SW bridge will not do anything with the frame, then there is no point to send the frame to SW bridge just to use CPU cycles for dropping the frame. > -- > Florian
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h index 4b19c54..34274de 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h @@ -78,6 +78,8 @@ enum { NETIF_F_HW_TLS_TX_BIT, /* Hardware TLS TX offload */ NETIF_F_HW_TLS_RX_BIT, /* Hardware TLS RX offload */ + NETIF_F_HW_BRIDGE_BIT, /* Bridge offload support */ + NETIF_F_GRO_HW_BIT, /* Hardware Generic receive offload */ NETIF_F_HW_TLS_RECORD_BIT, /* Offload TLS record */ @@ -150,6 +152,7 @@ enum { #define NETIF_F_GSO_UDP_L4 __NETIF_F(GSO_UDP_L4) #define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX) #define NETIF_F_HW_TLS_RX __NETIF_F(HW_TLS_RX) +#define NETIF_F_HW_BRIDGE __NETIF_F(HW_BRIDGE) /* Finds the next feature with the highest number of the range of start till 0. */ diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 4fe30b1..975a34c 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -127,6 +127,31 @@ static void br_port_clear_promisc(struct net_bridge_port *p) p->flags &= ~BR_PROMISC; } +/* Determin if the SW bridge can be offloaded to HW. Return true if all + * the interfaces of the bridge have the feature NETIF_F_HW_SWITCHDEV set + * and have the same netdev_ops. + */ +static int br_hw_offload(struct net_bridge *br) +{ + const struct net_device_ops *ops = NULL; + struct net_bridge_port *p; + + list_for_each_entry(p, &br->port_list, list) { + if (!(p->dev->hw_features & NETIF_F_HW_BRIDGE)) + return 0; + + if (!ops) { + ops = p->dev->netdev_ops; + continue; + } + + if (ops != p->dev->netdev_ops) + return 0; + } + + return 1; +} + /* When a port is added or removed or when certain port flags * change, this function is called to automatically manage * promiscuity setting of all the bridge ports. We are always called @@ -134,6 +159,7 @@ static void br_port_clear_promisc(struct net_bridge_port *p) */ void br_manage_promisc(struct net_bridge *br) { + bool hw_offload = br_hw_offload(br); struct net_bridge_port *p; bool set_all = false; @@ -161,7 +187,8 @@ void br_manage_promisc(struct net_bridge *br) (br->auto_cnt == 1 && br_auto_port(p))) br_port_clear_promisc(p); else - br_port_set_promisc(p); + if (!hw_offload) + br_port_set_promisc(p); } } } diff --git a/net/core/ethtool.c b/net/core/ethtool.c index 6288e69..4e224fe 100644 --- a/net/core/ethtool.c +++ b/net/core/ethtool.c @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record", [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload", [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload", + [NETIF_F_HW_BRIDGE_BIT] = "hw-bridge-offload", }; static const char
This patch adds a netdev feature to configure the HW as a switch. The purpose of this flag is to show that the hardware can do switching of the frames. Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- include/linux/netdev_features.h | 3 +++ net/bridge/br_if.c | 29 ++++++++++++++++++++++++++++- net/core/ethtool.c | 1 + 3 files changed, 32 insertions(+), 1 deletion(-)