Commit f7cf972f authored by Tobias Waldekranz's avatar Tobias Waldekranz Committed by David S. Miller
Browse files

net: bridge: disambiguate offload_fwd_mark 



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->v6: none

Signed-off-by: default avatarTobias Waldekranz <tobias@waldekranz.com>
Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: default avatarGrygorii Strashko <grygorii.strashko@ti.com>
Reviewed-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 45035feb

net/bridge/br_if.c

+1 −1
Original line number Diff line number Diff line
@@ -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;

net/bridge/br_private.h

+16 −5
Original line number Diff line number Diff line
@@ -386,7 +386,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;
@@ -510,7 +513,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;
@@ -540,7 +546,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

};
@@ -1829,7 +1840,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,
@@ -1849,7 +1860,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;

}

net/bridge/br_switchdev.c

+8 −8
Original line number Diff line number Diff line
@@ -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 */