Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6bc506b4 authored by Ido Schimmel's avatar Ido Schimmel Committed by David S. Miller
Browse files

bridge: switchdev: Add forward mark support for stacked devices 



switchdev_port_fwd_mark_set() is used to set the 'offload_fwd_mark' of
port netdevs so that packets being flooded by the device won't be
flooded twice.

It works by assigning a unique identifier (the ifindex of the first
bridge port) to bridge ports sharing the same parent ID. This prevents
packets from being flooded twice by the same switch, but will flood
packets through bridge ports belonging to a different switch.

This method is problematic when stacked devices are taken into account,
such as VLANs. In such cases, a physical port netdev can have upper
devices being members in two different bridges, thus requiring two
different 'offload_fwd_mark's to be configured on the port netdev, which
is impossible.

The main problem is that packet and netdev marking is performed at the
physical netdev level, whereas flooding occurs between bridge ports,
which are not necessarily port netdevs.

Instead, packet and netdev marking should really be done in the bridge
driver with the switch driver only telling it which packets it already
forwarded. The bridge driver will mark such packets using the mark
assigned to the ingress bridge port and will prevent the packet from
being forwarded through any bridge port sharing the same mark (i.e.
having the same parent ID).

Remove the current switchdev 'offload_fwd_mark' implementation and
instead implement the proposed method. In addition, make rocker - the
sole user of the mark - use the proposed method.

Signed-off-by: default avatarIdo Schimmel <idosch@mellanox.com>
Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 5c326ab4

Documentation/networking/switchdev.txt

+4 −9
Original line number Original line Diff line number Diff line
@@ -283,15 +283,10 @@ be sent to the port netdev for processing by the bridge driver. The 
bridge should not reflood the packet to the same ports the device flooded,

bridge should not reflood the packet to the same ports the device flooded,

otherwise there will be duplicate packets on the wire.

otherwise there will be duplicate packets on the wire.





To avoid duplicate packets, the device/driver should mark a packet as already

To avoid duplicate packets, the switch driver should mark a packet as already

forwarded using skb->offload_fwd_mark.  The same mark is set on the device

forwarded by setting the skb->offload_fwd_mark bit. The bridge driver will mark

ports in the domain using dev->offload_fwd_mark.  If the skb->offload_fwd_mark

the skb using the ingress bridge port's mark and prevent it from being forwarded

is non-zero and matches the forwarding egress port's dev->skb_mark, the kernel

through any bridge port with the same mark.

will drop the skb right before transmit on the egress port, with the

understanding that the device already forwarded the packet on same egress port.

The driver can use switchdev_port_fwd_mark_set() to set a globally unique mark

for port's dev->offload_fwd_mark, based on the port's parent ID (switch ID) and

a group ifindex.





It is possible for the switch device to not handle flooding and push the

It is possible for the switch device to not handle flooding and push the

packets up to the bridge driver for flooding.  This is not ideal as the number

packets up to the bridge driver for flooding.  This is not ideal as the number

drivers/net/ethernet/rocker/rocker_main.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -2412,7 +2412,7 @@ static int rocker_port_rx_proc(const struct rocker *rocker, 
	skb->protocol = eth_type_trans(skb, rocker_port->dev);

	skb->protocol = eth_type_trans(skb, rocker_port->dev);





	if (rx_flags & ROCKER_RX_FLAGS_FWD_OFFLOAD)

	if (rx_flags & ROCKER_RX_FLAGS_FWD_OFFLOAD)

		skb->offload_fwd_mark = rocker_port->dev->offload_fwd_mark;

		skb->offload_fwd_mark = 1;





	rocker_port->dev->stats.rx_packets++;

	rocker_port->dev->stats.rx_packets++;

	rocker_port->dev->stats.rx_bytes += skb->len;

	rocker_port->dev->stats.rx_bytes += skb->len;

drivers/net/ethernet/rocker/rocker_ofdpa.c

+0 −4
Original line number Original line Diff line number Diff line
@@ -2558,7 +2558,6 @@ static int ofdpa_port_init(struct rocker_port *rocker_port) 
	struct ofdpa_port *ofdpa_port = rocker_port->wpriv;

	struct ofdpa_port *ofdpa_port = rocker_port->wpriv;

	int err;

	int err;





	switchdev_port_fwd_mark_set(ofdpa_port->dev, NULL, false);

	rocker_port_set_learning(rocker_port,

	rocker_port_set_learning(rocker_port,

				 !!(ofdpa_port->brport_flags & BR_LEARNING));

				 !!(ofdpa_port->brport_flags & BR_LEARNING));
@@ -2817,7 +2816,6 @@ static int ofdpa_port_bridge_join(struct ofdpa_port *ofdpa_port, 
		ofdpa_port_internal_vlan_id_get(ofdpa_port, bridge->ifindex);

		ofdpa_port_internal_vlan_id_get(ofdpa_port, bridge->ifindex);





	ofdpa_port->bridge_dev = bridge;

	ofdpa_port->bridge_dev = bridge;

	switchdev_port_fwd_mark_set(ofdpa_port->dev, bridge, true);





	return ofdpa_port_vlan_add(ofdpa_port, NULL, OFDPA_UNTAGGED_VID, 0);

	return ofdpa_port_vlan_add(ofdpa_port, NULL, OFDPA_UNTAGGED_VID, 0);

}

}
@@ -2836,8 +2834,6 @@ static int ofdpa_port_bridge_leave(struct ofdpa_port *ofdpa_port) 
		ofdpa_port_internal_vlan_id_get(ofdpa_port,

		ofdpa_port_internal_vlan_id_get(ofdpa_port,

						ofdpa_port->dev->ifindex);

						ofdpa_port->dev->ifindex);





	switchdev_port_fwd_mark_set(ofdpa_port->dev, ofdpa_port->bridge_dev,

				    false);

	ofdpa_port->bridge_dev = NULL;

	ofdpa_port->bridge_dev = NULL;





	err = ofdpa_port_vlan_add(ofdpa_port, NULL, OFDPA_UNTAGGED_VID, 0);

	err = ofdpa_port_vlan_add(ofdpa_port, NULL, OFDPA_UNTAGGED_VID, 0);

include/linux/netdevice.h

+0 −5
Original line number Original line Diff line number Diff line
@@ -1562,8 +1562,6 @@ enum netdev_priv_flags { 
 *

 *

 *	@xps_maps:	XXX: need comments on this one

 *	@xps_maps:	XXX: need comments on this one

 *

 *

 *	@offload_fwd_mark:	Offload device fwding mark

 *

 *	@watchdog_timeo:	Represents the timeout that is used by

 *	@watchdog_timeo:	Represents the timeout that is used by

 *				the watchdog (see dev_watchdog())

 *				the watchdog (see dev_watchdog())

 *	@watchdog_timer:	List of timers

 *	@watchdog_timer:	List of timers
@@ -1814,9 +1812,6 @@ struct net_device { 
#ifdef CONFIG_NET_CLS_ACT

#ifdef CONFIG_NET_CLS_ACT

	struct tcf_proto __rcu  *egress_cl_list;

	struct tcf_proto __rcu  *egress_cl_list;

#endif

#endif

#ifdef CONFIG_NET_SWITCHDEV

	u32			offload_fwd_mark;

#endif





	/* These may be needed for future network-power-down code. */

	/* These may be needed for future network-power-down code. */

	struct timer_list	watchdog_timer;

	struct timer_list	watchdog_timer;

include/linux/skbuff.h

+5 −8
Original line number Original line Diff line number Diff line
@@ -612,7 +612,6 @@ static inline bool skb_mstamp_after(const struct skb_mstamp *t1, 
 *	@no_fcs:  Request NIC to treat last 4 bytes as Ethernet FCS

 *	@no_fcs:  Request NIC to treat last 4 bytes as Ethernet FCS

  *	@napi_id: id of the NAPI struct this skb came from

  *	@napi_id: id of the NAPI struct this skb came from

 *	@secmark: security marking

 *	@secmark: security marking

 *	@offload_fwd_mark: fwding offload mark

 *	@mark: Generic packet mark

 *	@mark: Generic packet mark

 *	@vlan_proto: vlan encapsulation protocol

 *	@vlan_proto: vlan encapsulation protocol

 *	@vlan_tci: vlan tag control information

 *	@vlan_tci: vlan tag control information
@@ -730,7 +729,10 @@ struct sk_buff { 
	__u8			ipvs_property:1;

	__u8			ipvs_property:1;

	__u8			inner_protocol_type:1;

	__u8			inner_protocol_type:1;

	__u8			remcsum_offload:1;

	__u8			remcsum_offload:1;

	/* 3 or 5 bit hole */

#ifdef CONFIG_NET_SWITCHDEV

	__u8			offload_fwd_mark:1;

#endif

	/* 2, 4 or 5 bit hole */





#ifdef CONFIG_NET_SCHED

#ifdef CONFIG_NET_SCHED

	__u16			tc_index;	/* traffic control index */

	__u16			tc_index;	/* traffic control index */
@@ -757,14 +759,9 @@ struct sk_buff { 
		unsigned int	sender_cpu;

		unsigned int	sender_cpu;

	};

	};

#endif

#endif

	union {

#ifdef CONFIG_NETWORK_SECMARK

#ifdef CONFIG_NETWORK_SECMARK

	__u32		secmark;

	__u32		secmark;

#endif

#endif

#ifdef CONFIG_NET_SWITCHDEV

		__u32		offload_fwd_mark;

#endif

	};





	union {

	union {

		__u32		mark;

		__u32		mark;