Message ID | 20210718214434.3938850-8-vladimir.oltean@nxp.com |
---|---|
State | Superseded |
Headers | show |
Series | Allow forwarding for the software bridge data path to be offloaded to capable devices | expand |
On 7/18/2021 2:44 PM, Vladimir Oltean wrote: > From: Tobias Waldekranz <tobias@waldekranz.com> > > Before this change, four related - but distinct - concepts where named > offload_fwd_mark: > > - skb->offload_fwd_mark: Set by the switchdev driver if the underlying > hardware has already forwarded this frame to the other ports in the > same hardware domain. > > - nbp->offload_fwd_mark: An idetifier used to group ports that share > the same hardware forwarding domain. > > - br->offload_fwd_mark: Counter used to make sure that unique IDs are > used in cases where a bridge contains ports from multiple hardware > domains. > > - skb->cb->offload_fwd_mark: The hardware domain on which the frame > ingressed and was forwarded. > > Introduce the term "hardware forwarding domain" ("hwdom") in the > bridge to denote a set of ports with the following property: > > If an skb with skb->offload_fwd_mark set, is received on a port > belonging to hwdom N, that frame has already been forwarded to all > other ports in hwdom N. > > By decoupling the name from "offload_fwd_mark", we can extend the > term's definition in the future - e.g. to add constraints that > describe expected egress behavior - without overloading the meaning of > "offload_fwd_mark". > > - nbp->offload_fwd_mark thus becomes nbp->hwdom. > > - br->offload_fwd_mark becomes br->last_hwdom. > > - skb->cb->offload_fwd_mark becomes skb->cb->src_hwdom. The slight > change in naming here mandates a slight change in behavior of the > nbp_switchdev_frame_mark() function. Previously, it only set this > value in skb->cb for packets with skb->offload_fwd_mark true (ones > which were forwarded in hardware). Whereas now we always track the > incoming hwdom for all packets coming from a switchdev (even for the > packets which weren't forwarded in hardware, such as STP BPDUs, IGMP > reports etc). As all uses of skb->cb->offload_fwd_mark were already > gated behind checks of skb->offload_fwd_mark, this will not introduce > any functional change, but it paves the way for future changes where > the ingressing hwdom must be known for frames coming from a switchdev > regardless of whether they were forwarded in hardware or not > (basically, if the skb comes from a switchdev, skb->cb->src_hwdom now > always tracks which one). > > A typical example where this is relevant: the switchdev has a fixed > configuration to trap STP BPDUs, but STP is not running on the bridge > and the group_fwd_mask allows them to be forwarded. Say we have this > setup: > > br0 > / | \ > / | \ > swp0 swp1 swp2 > > A BPDU comes in on swp0 and is trapped to the CPU; the driver does not > set skb->offload_fwd_mark. The bridge determines that the frame should > be forwarded to swp{1,2}. It is imperative that forward offloading is > _not_ allowed in this case, as the source hwdom is already "poisoned". > > Recording the source hwdom allows this case to be handled properly. > > v2->v3: added code comments > v3->v4: none > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c index 6e4a32354a13..838a277e3cf7 100644 --- a/net/bridge/br_if.c +++ b/net/bridge/br_if.c @@ -643,7 +643,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev, if (err) goto err5; - err = nbp_switchdev_mark_set(p); + err = nbp_switchdev_hwdom_set(p); if (err) goto err6; diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 2b48b204205e..54e29a8576a1 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -329,7 +329,10 @@ struct net_bridge_port { struct netpoll *np; #endif #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* Identifier used to group ports that share the same switchdev + * hardware domain. + */ + int hwdom; #endif u16 group_fwd_mask; u16 backup_redirected_cnt; @@ -476,7 +479,10 @@ struct net_bridge { u32 auto_cnt; #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* Counter used to make sure that hardware domains get unique + * identifiers in case a bridge spans multiple switchdev instances. + */ + int last_hwdom; #endif struct hlist_head fdb_list; @@ -506,7 +512,12 @@ struct br_input_skb_cb { #endif #ifdef CONFIG_NET_SWITCHDEV - int offload_fwd_mark; + /* The switchdev hardware domain from which this packet was received. + * If skb->offload_fwd_mark was set, then this packet was already + * forwarded by hardware to the other ports in the source hardware + * domain, otherwise it wasn't. + */ + int src_hwdom; #endif }; @@ -1645,7 +1656,7 @@ static inline void br_sysfs_delbr(struct net_device *dev) { return; } /* br_switchdev.c */ #ifdef CONFIG_NET_SWITCHDEV -int nbp_switchdev_mark_set(struct net_bridge_port *p); +int nbp_switchdev_hwdom_set(struct net_bridge_port *p); void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb); bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, @@ -1665,7 +1676,7 @@ static inline void br_switchdev_frame_unmark(struct sk_buff *skb) skb->offload_fwd_mark = 0; } #else -static inline int nbp_switchdev_mark_set(struct net_bridge_port *p) +static inline int nbp_switchdev_hwdom_set(struct net_bridge_port *p) { return 0; } diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c index d3adee0f91f9..833fd30482c2 100644 --- a/net/bridge/br_switchdev.c +++ b/net/bridge/br_switchdev.c @@ -8,20 +8,20 @@ #include "br_private.h" -static int br_switchdev_mark_get(struct net_bridge *br, struct net_device *dev) +static int br_switchdev_hwdom_get(struct net_bridge *br, struct net_device *dev) { struct net_bridge_port *p; /* dev is yet to be added to the port list. */ list_for_each_entry(p, &br->port_list, list) { if (netdev_port_same_parent_id(dev, p->dev)) - return p->offload_fwd_mark; + return p->hwdom; } - return ++br->offload_fwd_mark; + return ++br->last_hwdom; } -int nbp_switchdev_mark_set(struct net_bridge_port *p) +int nbp_switchdev_hwdom_set(struct net_bridge_port *p) { struct netdev_phys_item_id ppid = { }; int err; @@ -35,7 +35,7 @@ int nbp_switchdev_mark_set(struct net_bridge_port *p) return err; } - p->offload_fwd_mark = br_switchdev_mark_get(p->br, p->dev); + p->hwdom = br_switchdev_hwdom_get(p->br, p->dev); return 0; } @@ -43,15 +43,15 @@ int nbp_switchdev_mark_set(struct net_bridge_port *p) void nbp_switchdev_frame_mark(const struct net_bridge_port *p, struct sk_buff *skb) { - if (skb->offload_fwd_mark && !WARN_ON_ONCE(!p->offload_fwd_mark)) - BR_INPUT_SKB_CB(skb)->offload_fwd_mark = p->offload_fwd_mark; + if (p->hwdom) + BR_INPUT_SKB_CB(skb)->src_hwdom = p->hwdom; } bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p, const struct sk_buff *skb) { return !skb->offload_fwd_mark || - BR_INPUT_SKB_CB(skb)->offload_fwd_mark != p->offload_fwd_mark; + BR_INPUT_SKB_CB(skb)->src_hwdom != p->hwdom; } /* Flags that can be offloaded to hardware */