All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH net-next 0/3] add HSR offloading support for DSA switches
@ 2021-01-22 15:59 George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 1/3] net: hsr: generate supervision frame without HSR tag George McCollister
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: George McCollister @ 2021-01-22 15:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jonathan Corbet, Murali Karicheri, netdev, George McCollister

Add support for offloading HSR/PRP (IEC 62439-3) tag insertion, tag
removal, forwarding and duplication on DSA switches.
This series adds offloading to the xrs700x DSA driver.

Please let me know if you see any issues or anything that should be
discussed before submitting it as a non-RFC patch series. Since
many may not be familiar with HSR/PRP let me know if I omitted anything
in the commit messages which might have seemed obvious to me.

George McCollister (3):
  net: hsr: generate supervision frame without HSR tag
  net: hsr: add DSA offloading support
  net: dsa: xrs700x: add HSR offloading support

 Documentation/networking/netdev-features.rst |  20 +++++
 drivers/net/dsa/xrs700x/xrs700x.c            | 106 +++++++++++++++++++++++++++
 drivers/net/dsa/xrs700x/xrs700x_reg.h        |   5 ++
 include/linux/if_hsr.h                       |  22 ++++++
 include/linux/netdev_features.h              |   9 +++
 include/linux/netdevice.h                    |  13 ++++
 include/net/dsa.h                            |  13 ++++
 net/dsa/dsa_priv.h                           |  11 +++
 net/dsa/port.c                               |  34 +++++++++
 net/dsa/slave.c                              |  13 ++++
 net/dsa/switch.c                             |  24 ++++++
 net/dsa/tag_xrs700x.c                        |   7 +-
 net/ethtool/common.c                         |   4 +
 net/hsr/hsr_device.c                         |  44 ++---------
 net/hsr/hsr_forward.c                        |  37 ++++++++--
 net/hsr/hsr_forward.h                        |   1 +
 net/hsr/hsr_main.c                           |  14 ++++
 net/hsr/hsr_main.h                           |   8 +-
 net/hsr/hsr_slave.c                          |  13 +++-
 19 files changed, 343 insertions(+), 55 deletions(-)
 create mode 100644 include/linux/if_hsr.h

-- 
2.11.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH net-next 1/3] net: hsr: generate supervision frame without HSR tag
  2021-01-22 15:59 [RFC PATCH net-next 0/3] add HSR offloading support for DSA switches George McCollister
@ 2021-01-22 15:59 ` George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 3/3] net: dsa: xrs700x: add HSR " George McCollister
  2 siblings, 0 replies; 12+ messages in thread
From: George McCollister @ 2021-01-22 15:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jonathan Corbet, Murali Karicheri, netdev, George McCollister

Generate supervision frame without HSR/PRP tag and rely on existing
code which inserts it later.
This will allow HSR/PRP tag insertions to be offloaded in the future.

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 net/hsr/hsr_device.c  | 32 ++++----------------------------
 net/hsr/hsr_forward.c | 10 +++++++---
 2 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index ab953a1a0d6c..161b8da6a21d 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -242,8 +242,7 @@ static struct sk_buff *hsr_init_skb(struct hsr_port *master, u16 proto)
 	 * being, for PRP it is a trailer and for HSR it is a
 	 * header
 	 */
-	skb = dev_alloc_skb(sizeof(struct hsr_tag) +
-			    sizeof(struct hsr_sup_tag) +
+	skb = dev_alloc_skb(sizeof(struct hsr_sup_tag) +
 			    sizeof(struct hsr_sup_payload) + hlen + tlen);
 
 	if (!skb)
@@ -275,12 +274,10 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
 {
 	struct hsr_priv *hsr = master->hsr;
 	__u8 type = HSR_TLV_LIFE_CHECK;
-	struct hsr_tag *hsr_tag = NULL;
 	struct hsr_sup_payload *hsr_sp;
 	struct hsr_sup_tag *hsr_stag;
 	unsigned long irqflags;
 	struct sk_buff *skb;
-	u16 proto;
 
 	*interval = msecs_to_jiffies(HSR_LIFE_CHECK_INTERVAL);
 	if (hsr->announce_count < 3 && hsr->prot_version == 0) {
@@ -289,23 +286,12 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
 		hsr->announce_count++;
 	}
 
-	if (!hsr->prot_version)
-		proto = ETH_P_PRP;
-	else
-		proto = ETH_P_HSR;
-
-	skb = hsr_init_skb(master, proto);
+	skb = hsr_init_skb(master, ETH_P_PRP);
 	if (!skb) {
 		WARN_ONCE(1, "HSR: Could not send supervision frame\n");
 		return;
 	}
 
-	if (hsr->prot_version > 0) {
-		hsr_tag = skb_put(skb, sizeof(struct hsr_tag));
-		hsr_tag->encap_proto = htons(ETH_P_PRP);
-		set_hsr_tag_LSDU_size(hsr_tag, HSR_V1_SUP_LSDUSIZE);
-	}
-
 	hsr_stag = skb_put(skb, sizeof(struct hsr_sup_tag));
 	set_hsr_stag_path(hsr_stag, (hsr->prot_version ? 0x0 : 0xf));
 	set_hsr_stag_HSR_ver(hsr_stag, hsr->prot_version);
@@ -315,8 +301,6 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
 	if (hsr->prot_version > 0) {
 		hsr_stag->sequence_nr = htons(hsr->sup_sequence_nr);
 		hsr->sup_sequence_nr++;
-		hsr_tag->sequence_nr = htons(hsr->sequence_nr);
-		hsr->sequence_nr++;
 	} else {
 		hsr_stag->sequence_nr = htons(hsr->sequence_nr);
 		hsr->sequence_nr++;
@@ -332,7 +316,7 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
 	hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
 	ether_addr_copy(hsr_sp->macaddress_A, master->dev->dev_addr);
 
-	if (skb_put_padto(skb, ETH_ZLEN + HSR_HLEN))
+	if (skb_put_padto(skb, ETH_ZLEN))
 		return;
 
 	hsr_forward_skb(skb, master);
@@ -348,8 +332,6 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 	struct hsr_sup_tag *hsr_stag;
 	unsigned long irqflags;
 	struct sk_buff *skb;
-	struct prp_rct *rct;
-	u8 *tail;
 
 	skb = hsr_init_skb(master, ETH_P_PRP);
 	if (!skb) {
@@ -373,17 +355,11 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 	hsr_sp = skb_put(skb, sizeof(struct hsr_sup_payload));
 	ether_addr_copy(hsr_sp->macaddress_A, master->dev->dev_addr);
 
-	if (skb_put_padto(skb, ETH_ZLEN + HSR_HLEN)) {
+	if (skb_put_padto(skb, ETH_ZLEN)) {
 		spin_unlock_irqrestore(&master->hsr->seqnr_lock, irqflags);
 		return;
 	}
 
-	tail = skb_tail_pointer(skb) - HSR_HLEN;
-	rct = (struct prp_rct *)tail;
-	rct->PRP_suffix = htons(ETH_P_PRP);
-	set_prp_LSDU_size(rct, HSR_V1_SUP_LSDUSIZE);
-	rct->sequence_nr = htons(hsr->sequence_nr);
-	hsr->sequence_nr++;
 	spin_unlock_irqrestore(&master->hsr->seqnr_lock, irqflags);
 
 	hsr_forward_skb(skb, master);
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index cadfccd7876e..a5566b2245a0 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -454,8 +454,10 @@ static void handle_std_frame(struct sk_buff *skb,
 void hsr_fill_frame_info(__be16 proto, struct sk_buff *skb,
 			 struct hsr_frame_info *frame)
 {
-	if (proto == htons(ETH_P_PRP) ||
-	    proto == htons(ETH_P_HSR)) {
+	struct hsr_port *port = frame->port_rcv;
+
+	if (port->type != HSR_PT_MASTER &&
+	    (proto == htons(ETH_P_PRP) || proto == htons(ETH_P_HSR))) {
 		/* HSR tagged frame :- Data or Supervision */
 		frame->skb_std = NULL;
 		frame->skb_prp = NULL;
@@ -473,8 +475,10 @@ void prp_fill_frame_info(__be16 proto, struct sk_buff *skb,
 {
 	/* Supervision frame */
 	struct prp_rct *rct = skb_get_PRP_rct(skb);
+	struct hsr_port *port = frame->port_rcv;
 
-	if (rct &&
+	if (port->type != HSR_PT_MASTER &&
+	    rct &&
 	    prp_check_lsdu_size(skb, rct, frame->is_supervision)) {
 		frame->skb_hsr = NULL;
 		frame->skb_std = NULL;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 15:59 [RFC PATCH net-next 0/3] add HSR offloading support for DSA switches George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 1/3] net: hsr: generate supervision frame without HSR tag George McCollister
@ 2021-01-22 15:59 ` George McCollister
  2021-01-22 17:56   ` Florian Fainelli
  2021-01-24 17:29   ` Vladimir Oltean
  2021-01-22 15:59 ` [RFC PATCH net-next 3/3] net: dsa: xrs700x: add HSR " George McCollister
  2 siblings, 2 replies; 12+ messages in thread
From: George McCollister @ 2021-01-22 15:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jonathan Corbet, Murali Karicheri, netdev, George McCollister

Add support for offloading of HSR/PRP (IEC 62439-3) tag insertion
tag removal, duplicate generation and forwarding on DSA switches.

Use a new netdev_priv_flag IFF_HSR to indicate that a device is an HSR
device so DSA can tell them apart from other devices in
dsa_slave_changeupper.

Add DSA_NOTIFIER_HSR_JOIN and DSA_NOTIFIER_HSR_LEAVE which trigger calls
to .port_hsr_join and .port_hsr_leave in the DSA driver for the switch.

The DSA switch driver should then set netdev feature flags for the
HSR/PRP operation that it offloads.
    NETIF_F_HW_HSR_TAG_INS
    NETIF_F_HW_HSR_TAG_RM
    NETIF_F_HW_HSR_FWD
    NETIF_F_HW_HSR_DUP

For HSR, insertion involves the switch adding a 6 byte HSR header after
the 14 byte Ethernet header. For PRP it adds a 6 byte trailer.

Tag removal involves automatically stripping the HSR/PRP header/trailer
in the switch. This is possible when the switch also preforms auto
deduplication using the HSR/PRP header/trailer (making it no longer
required).

Forwarding involves automatically forwarding between redundant ports in
an HSR. This is crucial because delay is accumulated as a frame passes
through each node in the ring.

Duplication involves the switch automatically sending a single frame
from the CPU port to both redundant ports. This is required because the
inserted HSR/PRP header/trailer must contain the same sequence number
on the frames sent out both redundant ports.

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 Documentation/networking/netdev-features.rst | 20 ++++++++++++++++
 include/linux/if_hsr.h                       | 22 ++++++++++++++++++
 include/linux/netdev_features.h              |  9 ++++++++
 include/linux/netdevice.h                    | 13 +++++++++++
 include/net/dsa.h                            | 13 +++++++++++
 net/dsa/dsa_priv.h                           | 11 +++++++++
 net/dsa/port.c                               | 34 ++++++++++++++++++++++++++++
 net/dsa/slave.c                              | 13 +++++++++++
 net/dsa/switch.c                             | 24 ++++++++++++++++++++
 net/ethtool/common.c                         |  4 ++++
 net/hsr/hsr_device.c                         | 12 +++-------
 net/hsr/hsr_forward.c                        | 27 +++++++++++++++++++---
 net/hsr/hsr_forward.h                        |  1 +
 net/hsr/hsr_main.c                           | 14 ++++++++++++
 net/hsr/hsr_main.h                           |  8 +------
 net/hsr/hsr_slave.c                          | 13 +++++++----
 16 files changed, 215 insertions(+), 23 deletions(-)
 create mode 100644 include/linux/if_hsr.h

diff --git a/Documentation/networking/netdev-features.rst b/Documentation/networking/netdev-features.rst
index a2d7d7160e39..4eab45405031 100644
--- a/Documentation/networking/netdev-features.rst
+++ b/Documentation/networking/netdev-features.rst
@@ -182,3 +182,23 @@ stricter than Hardware LRO.  A packet stream merged by Hardware GRO must
 be re-segmentable by GSO or TSO back to the exact original packet stream.
 Hardware GRO is dependent on RXCSUM since every packet successfully merged
 by hardware must also have the checksum verified by hardware.
+
+* hsr-tag-ins-offload
+
+This should be set for devices which insert an HSR (highspeed ring) tag
+automatically when in HSR mode.
+
+* hsr-tag-rm-offload
+
+This should be set for devices which remove HSR (highspeed ring) tags
+automatically when in HSR mode.
+
+* hsr-fwd-offload
+
+This should be set for devices which forward HSR (highspeed ring) frames from
+one port to another in hardware.
+
+* hsr-dup-offload
+
+This should be set for devices which duplicate outgoing HSR (highspeed ring)
+frames in hardware.
diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
new file mode 100644
index 000000000000..eec9079efab0
--- /dev/null
+++ b/include/linux/if_hsr.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_IF_HSR_H_
+#define _LINUX_IF_HSR_H_
+
+/* used to differentiate various protocols */
+enum hsr_version {
+	HSR_V0 = 0,
+	HSR_V1,
+	PRP_V1,
+};
+
+#if IS_ENABLED(CONFIG_HSR)
+extern int hsr_get_version(struct net_device *dev, enum hsr_version *ver);
+#else
+static inline int hsr_get_version(struct net_device *dev,
+				  enum hsr_version *ver)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_HSR */
+
+#endif /*_LINUX_IF_HSR_H_*/
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 934de56644e7..13b3fa7668bb 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -85,6 +85,11 @@ enum {
 
 	NETIF_F_HW_MACSEC_BIT,		/* Offload MACsec operations */
 
+	NETIF_F_HW_HSR_TAG_INS_BIT,	/* Offload HSR tag insertion */
+	NETIF_F_HW_HSR_TAG_RM_BIT,	/* Offload HSR tag removal */
+	NETIF_F_HW_HSR_FWD_BIT,		/* Offload HSR forwarding */
+	NETIF_F_HW_HSR_DUP_BIT,		/* Offload HSR duplication */
+
 	/*
 	 * Add your fresh new feature above and remember to update
 	 * netdev_features_strings[] in net/core/ethtool.c and maybe
@@ -157,6 +162,10 @@ enum {
 #define NETIF_F_GRO_FRAGLIST	__NETIF_F(GRO_FRAGLIST)
 #define NETIF_F_GSO_FRAGLIST	__NETIF_F(GSO_FRAGLIST)
 #define NETIF_F_HW_MACSEC	__NETIF_F(HW_MACSEC)
+#define NETIF_F_HW_HSR_TAG_INS	__NETIF_F(HW_HSR_TAG_INS)
+#define NETIF_F_HW_HSR_TAG_RM	__NETIF_F(HW_HSR_TAG_RM)
+#define NETIF_F_HW_HSR_FWD	__NETIF_F(HW_HSR_FWD)
+#define NETIF_F_HW_HSR_DUP	__NETIF_F(HW_HSR_DUP)
 
 /* Finds the next feature with the highest number of the range of start till 0.
  */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5b949076ed23..4a1533aff3cf 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1523,6 +1523,7 @@ struct net_device_ops {
  * @IFF_FAILOVER_SLAVE: device is lower dev of a failover master device
  * @IFF_L3MDEV_RX_HANDLER: only invoke the rx handler of L3 master device
  * @IFF_LIVE_RENAME_OK: rename is allowed while device is up and running
+ * @IFF_HSR: device is an hsr device
  */
 enum netdev_priv_flags {
 	IFF_802_1Q_VLAN			= 1<<0,
@@ -1556,6 +1557,7 @@ enum netdev_priv_flags {
 	IFF_FAILOVER_SLAVE		= 1<<28,
 	IFF_L3MDEV_RX_HANDLER		= 1<<29,
 	IFF_LIVE_RENAME_OK		= 1<<30,
+	IFF_HSR				= 1<<31,
 };
 
 #define IFF_802_1Q_VLAN			IFF_802_1Q_VLAN
@@ -1588,6 +1590,7 @@ enum netdev_priv_flags {
 #define IFF_FAILOVER_SLAVE		IFF_FAILOVER_SLAVE
 #define IFF_L3MDEV_RX_HANDLER		IFF_L3MDEV_RX_HANDLER
 #define IFF_LIVE_RENAME_OK		IFF_LIVE_RENAME_OK
+#define IFF_HSR				IFF_HSR
 
 /**
  *	struct net_device - The DEVICE structure.
@@ -4996,6 +4999,16 @@ static inline bool netif_is_failover_slave(const struct net_device *dev)
 	return dev->priv_flags & IFF_FAILOVER_SLAVE;
 }
 
+static inline bool netif_is_hsr_master(const struct net_device *dev)
+{
+	return dev->flags & IFF_MASTER && dev->priv_flags & IFF_HSR;
+}
+
+static inline bool netif_is_hsr_slave(const struct net_device *dev)
+{
+	return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_HSR;
+}
+
 /* This device needs to keep skb dst for qdisc enqueue or ndo_start_xmit() */
 static inline void netif_keep_dst(struct net_device *dev)
 {
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2f5435d3d1db..584e2b5c02e0 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -167,6 +167,10 @@ struct dsa_switch_tree {
 	list_for_each_entry((_dp), &(_dst)->ports, list)	\
 		if ((_dp)->lag_dev == (_lag))
 
+#define dsa_hsr_foreach_port(_dp, _ds, _hsr)			\
+	list_for_each_entry((_dp), &(_ds)->dst->ports, list)	\
+		if ((_dp)->ds == (_ds) && (_dp)->hsr_dev == (_hsr))
+
 static inline struct net_device *dsa_lag_dev(struct dsa_switch_tree *dst,
 					     unsigned int id)
 {
@@ -257,6 +261,7 @@ struct dsa_port {
 	struct phylink_config	pl_config;
 	struct net_device	*lag_dev;
 	bool			lag_tx_enabled;
+	struct net_device	*hsr_dev;
 
 	struct list_head list;
 
@@ -753,6 +758,14 @@ struct dsa_switch_ops {
 				 struct netdev_lag_upper_info *info);
 	int	(*port_lag_leave)(struct dsa_switch *ds, int port,
 				  struct net_device *lag);
+
+	/*
+	 * HSR integration
+	 */
+	int	(*port_hsr_join)(struct dsa_switch *ds, int port,
+				 struct net_device *hsr);
+	void	(*port_hsr_leave)(struct dsa_switch *ds, int port,
+				  struct net_device *hsr);
 };
 
 #define DSA_DEVLINK_PARAM_DRIVER(_id, _name, _type, _cmodes)		\
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2ce46bb87703..9006f62f69cd 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -20,6 +20,8 @@ enum {
 	DSA_NOTIFIER_BRIDGE_LEAVE,
 	DSA_NOTIFIER_FDB_ADD,
 	DSA_NOTIFIER_FDB_DEL,
+	DSA_NOTIFIER_HSR_JOIN,
+	DSA_NOTIFIER_HSR_LEAVE,
 	DSA_NOTIFIER_LAG_CHANGE,
 	DSA_NOTIFIER_LAG_JOIN,
 	DSA_NOTIFIER_LAG_LEAVE,
@@ -94,6 +96,13 @@ struct dsa_switchdev_event_work {
 	u16 vid;
 };
 
+/* DSA_NOTIFIER_HSR_* */
+struct dsa_notifier_hsr_info {
+	struct net_device *hsr;
+	int sw_index;
+	int port;
+};
+
 struct dsa_slave_priv {
 	/* Copy of CPU port xmit for faster access in slave transmit hot path */
 	struct sk_buff *	(*xmit)(struct sk_buff *skb,
@@ -174,6 +183,8 @@ int dsa_port_vlan_del(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan);
 int dsa_port_link_register_of(struct dsa_port *dp);
 void dsa_port_link_unregister_of(struct dsa_port *dp);
+int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
+void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
 extern const struct phylink_mac_ops dsa_port_phylink_mac_ops;
 
 static inline bool dsa_port_offloads_netdev(struct dsa_port *dp,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index f5b0f72ee7cd..09738af4d32e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -870,3 +870,37 @@ int dsa_port_get_phy_sset_count(struct dsa_port *dp)
 	return ret;
 }
 EXPORT_SYMBOL_GPL(dsa_port_get_phy_sset_count);
+
+int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
+{
+	struct dsa_notifier_hsr_info info = {
+		.sw_index = dp->ds->index,
+		.port = dp->index,
+		.hsr = hsr,
+	};
+	int err;
+
+	dp->hsr_dev = hsr;
+
+	err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_JOIN, &info);
+	if (err)
+		dp->hsr_dev = NULL;
+
+	return err;
+}
+
+void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr)
+{
+	struct dsa_notifier_hsr_info info = {
+		.sw_index = dp->ds->index,
+		.port = dp->index,
+		.hsr = hsr,
+	};
+	int err;
+
+	dp->hsr_dev = NULL;
+
+	err = dsa_port_notify(dp, DSA_NOTIFIER_HSR_LEAVE, &info);
+	if (err)
+		pr_err("DSA: failed to notify DSA_NOTIFIER_HSR_LEAVE\n");
+}
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f2fb433f3828..fc7e3ff11c5c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1924,6 +1924,19 @@ static int dsa_slave_changeupper(struct net_device *dev,
 			dsa_port_lag_leave(dp, info->upper_dev);
 			err = NOTIFY_OK;
 		}
+	} else if (netif_is_hsr_master(info->upper_dev)) {
+		if (info->linking) {
+			err = dsa_port_hsr_join(dp, info->upper_dev);
+			if (err == -EOPNOTSUPP) {
+				NL_SET_ERR_MSG_MOD(info->info.extack,
+						   "Offloading not supported");
+				err = 0;
+			}
+			err = notifier_from_errno(err);
+		} else {
+			dsa_port_hsr_leave(dp, info->upper_dev);
+			err = NOTIFY_OK;
+		}
 	}
 
 	return err;
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index cc0b25f3adea..c1e5083f2cfc 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -166,6 +166,24 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
 	return ds->ops->port_fdb_del(ds, port, info->addr, info->vid);
 }
 
+static int dsa_switch_hsr_join(struct dsa_switch *ds,
+			       struct dsa_notifier_hsr_info *info)
+{
+	if (ds->index == info->sw_index && ds->ops->port_hsr_join)
+		return ds->ops->port_hsr_join(ds, info->port, info->hsr);
+
+	return 0;
+}
+
+static int dsa_switch_hsr_leave(struct dsa_switch *ds,
+				struct dsa_notifier_hsr_info *info)
+{
+	if (ds->index == info->sw_index && ds->ops->port_hsr_leave)
+		ds->ops->port_hsr_leave(ds, info->port, info->hsr);
+
+	return 0;
+}
+
 static int dsa_switch_lag_change(struct dsa_switch *ds,
 				 struct dsa_notifier_lag_info *info)
 {
@@ -319,6 +337,12 @@ static int dsa_switch_event(struct notifier_block *nb,
 	case DSA_NOTIFIER_FDB_DEL:
 		err = dsa_switch_fdb_del(ds, info);
 		break;
+	case DSA_NOTIFIER_HSR_JOIN:
+		err = dsa_switch_hsr_join(ds, info);
+		break;
+	case DSA_NOTIFIER_HSR_LEAVE:
+		err = dsa_switch_hsr_leave(ds, info);
+		break;
 	case DSA_NOTIFIER_LAG_CHANGE:
 		err = dsa_switch_lag_change(ds, info);
 		break;
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 24036e3055a1..934e98e495a6 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -68,6 +68,10 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
 	[NETIF_F_HW_TLS_RX_BIT] =	 "tls-hw-rx-offload",
 	[NETIF_F_GRO_FRAGLIST_BIT] =	 "rx-gro-list",
 	[NETIF_F_HW_MACSEC_BIT] =	 "macsec-hw-offload",
+	[NETIF_F_HW_HSR_TAG_INS_BIT] =	 "hsr-tag-ins-offload",
+	[NETIF_F_HW_HSR_TAG_RM_BIT] =	 "hsr-tag-rm-offload",
+	[NETIF_F_HW_HSR_FWD_BIT] =	 "hsr-fwd-offload",
+	[NETIF_F_HW_HSR_DUP_BIT] =	 "hsr-dup-offload",
 };
 
 const char
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 161b8da6a21d..d9b033e9b18c 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -418,6 +418,7 @@ static struct hsr_proto_ops hsr_ops = {
 	.send_sv_frame = send_hsr_supervision_frame,
 	.create_tagged_frame = hsr_create_tagged_frame,
 	.get_untagged_frame = hsr_get_untagged_frame,
+	.drop_frame = hsr_drop_frame,
 	.fill_frame_info = hsr_fill_frame_info,
 	.invalid_dan_ingress_frame = hsr_invalid_dan_ingress_frame,
 };
@@ -521,15 +522,8 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
 
 	hsr->prot_version = protocol_version;
 
-	/* FIXME: should I modify the value of these?
-	 *
-	 * - hsr_dev->flags - i.e.
-	 *			IFF_MASTER/SLAVE?
-	 * - hsr_dev->priv_flags - i.e.
-	 *			IFF_EBRIDGE?
-	 *			IFF_TX_SKB_SHARING?
-	 *			IFF_HSR_MASTER/SLAVE?
-	 */
+	hsr_dev->flags |= IFF_MASTER;
+	hsr_dev->priv_flags |= IFF_HSR;
 
 	/* Make sure the 1st call to netif_carrier_on() gets through */
 	netif_carrier_off(hsr_dev);
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index a5566b2245a0..9c79d602c4e0 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -247,6 +247,8 @@ struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 		/* set the lane id properly */
 		hsr_set_path_id(hsr_ethhdr, port);
 		return skb_clone(frame->skb_hsr, GFP_ATOMIC);
+	} else if (port->dev->features & NETIF_F_HW_HSR_TAG_INS) {
+		return skb_clone(frame->skb_std, GFP_ATOMIC);
 	}
 
 	/* Create the new skb with enough headroom to fit the HSR tag */
@@ -341,6 +343,14 @@ bool prp_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
 		 port->type ==  HSR_PT_SLAVE_A));
 }
 
+bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
+{
+	if (port->dev->features & NETIF_F_HW_HSR_FWD)
+		return prp_drop_frame(frame, port);
+
+	return false;
+}
+
 /* Forward the frame through all devices except:
  * - Back through the receiving device
  * - If it's a HSR frame: through a device where it has passed before
@@ -357,6 +367,7 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 {
 	struct hsr_port *port;
 	struct sk_buff *skb;
+	bool sent = false;
 
 	hsr_for_each_port(frame->port_rcv->hsr, port) {
 		struct hsr_priv *hsr = port->hsr;
@@ -372,6 +383,12 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 		if (port->type != HSR_PT_MASTER && frame->is_local_exclusive)
 			continue;
 
+		/* If hardware duplicate generation is enabled, only send out
+		 * one port.
+		 */
+		if ((port->dev->features & NETIF_F_HW_HSR_DUP) && sent)
+			continue;
+
 		/* Don't send frame over port where it has been sent before.
 		 * Also fro SAN, this shouldn't be done.
 		 */
@@ -403,10 +420,12 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 		}
 
 		skb->dev = port->dev;
-		if (port->type == HSR_PT_MASTER)
+		if (port->type == HSR_PT_MASTER) {
 			hsr_deliver_master(skb, port->dev, frame->node_src);
-		else
-			hsr_xmit(skb, port, frame);
+		} else {
+			if (!hsr_xmit(skb, port, frame))
+				sent = true;
+		}
 	}
 }
 
@@ -457,6 +476,7 @@ void hsr_fill_frame_info(__be16 proto, struct sk_buff *skb,
 	struct hsr_port *port = frame->port_rcv;
 
 	if (port->type != HSR_PT_MASTER &&
+	    !(port->dev->features & NETIF_F_HW_HSR_TAG_RM) &&
 	    (proto == htons(ETH_P_PRP) || proto == htons(ETH_P_HSR))) {
 		/* HSR tagged frame :- Data or Supervision */
 		frame->skb_std = NULL;
@@ -478,6 +498,7 @@ void prp_fill_frame_info(__be16 proto, struct sk_buff *skb,
 	struct hsr_port *port = frame->port_rcv;
 
 	if (port->type != HSR_PT_MASTER &&
+	    !(port->dev->features & NETIF_F_HW_HSR_TAG_RM) &&
 	    rct &&
 	    prp_check_lsdu_size(skb, rct, frame->is_supervision)) {
 		frame->skb_hsr = NULL;
diff --git a/net/hsr/hsr_forward.h b/net/hsr/hsr_forward.h
index 618140d484ad..b6acaafa83fc 100644
--- a/net/hsr/hsr_forward.h
+++ b/net/hsr/hsr_forward.h
@@ -23,6 +23,7 @@ struct sk_buff *hsr_get_untagged_frame(struct hsr_frame_info *frame,
 struct sk_buff *prp_get_untagged_frame(struct hsr_frame_info *frame,
 				       struct hsr_port *port);
 bool prp_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port);
+bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port);
 void prp_fill_frame_info(__be16 proto, struct sk_buff *skb,
 			 struct hsr_frame_info *frame);
 void hsr_fill_frame_info(__be16 proto, struct sk_buff *skb,
diff --git a/net/hsr/hsr_main.c b/net/hsr/hsr_main.c
index 2fd1976e5b1c..0e7b5b18b5e3 100644
--- a/net/hsr/hsr_main.c
+++ b/net/hsr/hsr_main.c
@@ -131,6 +131,20 @@ struct hsr_port *hsr_port_get_hsr(struct hsr_priv *hsr, enum hsr_port_type pt)
 	return NULL;
 }
 
+int hsr_get_version(struct net_device *dev, enum hsr_version *ver)
+{
+	struct hsr_priv *hsr;
+
+	if (!(dev->priv_flags & IFF_HSR))
+		return -EINVAL;
+
+	hsr = netdev_priv(dev);
+	*ver = hsr->prot_version;
+
+	return 0;
+}
+EXPORT_SYMBOL(hsr_get_version);
+
 static struct notifier_block hsr_nb = {
 	.notifier_call = hsr_netdev_notify,	/* Slave event notifications */
 };
diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
index 7dc92ce5a134..7369b2febe0f 100644
--- a/net/hsr/hsr_main.h
+++ b/net/hsr/hsr_main.h
@@ -13,6 +13,7 @@
 #include <linux/netdevice.h>
 #include <linux/list.h>
 #include <linux/if_vlan.h>
+#include <linux/if_hsr.h>
 
 /* Time constants as specified in the HSR specification (IEC-62439-3 2010)
  * Table 8.
@@ -171,13 +172,6 @@ struct hsr_port {
 	enum hsr_port_type	type;
 };
 
-/* used by driver internally to differentiate various protocols */
-enum hsr_version {
-	HSR_V0 = 0,
-	HSR_V1,
-	PRP_V1,
-};
-
 struct hsr_frame_info;
 struct hsr_node;
 
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index 36d5fcf09c61..59f8d2b68376 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -48,12 +48,14 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 		goto finish_consume;
 	}
 
-	/* For HSR, only tagged frames are expected, but for PRP
-	 * there could be non tagged frames as well from Single
-	 * attached nodes (SANs).
+	/* For HSR, only tagged frames are expected (unless the device offloads
+	 * HSR tag removal), but for PRP there could be non tagged frames as
+	 * well from Single attached nodes (SANs).
 	 */
 	protocol = eth_hdr(skb)->h_proto;
-	if (hsr->proto_ops->invalid_dan_ingress_frame &&
+
+	if (!(port->dev->features & NETIF_F_HW_HSR_TAG_RM) &&
+	    hsr->proto_ops->invalid_dan_ingress_frame &&
 	    hsr->proto_ops->invalid_dan_ingress_frame(protocol))
 		goto finish_pass;
 
@@ -137,6 +139,9 @@ static int hsr_portdev_setup(struct hsr_priv *hsr, struct net_device *dev,
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
 	hsr_dev = master->dev;
 
+	dev->flags |= IFF_SLAVE;
+	dev->priv_flags |= IFF_HSR;
+
 	res = netdev_upper_dev_link(dev, hsr_dev, extack);
 	if (res)
 		goto fail_upper_dev_link;
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC PATCH net-next 3/3] net: dsa: xrs700x: add HSR offloading support
  2021-01-22 15:59 [RFC PATCH net-next 0/3] add HSR offloading support for DSA switches George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 1/3] net: hsr: generate supervision frame without HSR tag George McCollister
  2021-01-22 15:59 ` [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support George McCollister
@ 2021-01-22 15:59 ` George McCollister
  2 siblings, 0 replies; 12+ messages in thread
From: George McCollister @ 2021-01-22 15:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	Jonathan Corbet, Murali Karicheri, netdev, George McCollister

Add offloading for HSR/PRP (IEC 62439-3) tag insertion, tag removal
forwarding and duplication supported by the xrs7000 series switches.

Only HSR v1 and PRP v1 are supported by the xrs7000 series switches (HSR
v0 is not).

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 drivers/net/dsa/xrs700x/xrs700x.c     | 106 ++++++++++++++++++++++++++++++++++
 drivers/net/dsa/xrs700x/xrs700x_reg.h |   5 ++
 net/dsa/tag_xrs700x.c                 |   7 ++-
 3 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/xrs700x/xrs700x.c b/drivers/net/dsa/xrs700x/xrs700x.c
index 259f5e657c46..566ce9330903 100644
--- a/drivers/net/dsa/xrs700x/xrs700x.c
+++ b/drivers/net/dsa/xrs700x/xrs700x.c
@@ -7,6 +7,8 @@
 #include <net/dsa.h>
 #include <linux/if_bridge.h>
 #include <linux/of_device.h>
+#include <linux/netdev_features.h>
+#include <linux/if_hsr.h>
 #include "xrs700x.h"
 #include "xrs700x_reg.h"
 
@@ -496,6 +498,108 @@ static void xrs700x_bridge_leave(struct dsa_switch *ds, int port,
 	xrs700x_bridge_common(ds, port, bridge, false);
 }
 
+static int xrs700x_hsr_join(struct dsa_switch *ds, int port,
+			    struct net_device *hsr)
+{
+	unsigned int val = XRS_HSR_CFG_HSR_PRP;
+	struct dsa_port *partner = NULL, *dp;
+	struct xrs700x *priv = ds->priv;
+	struct net_device *slave;
+	enum hsr_version ver;
+	int ret;
+
+	ret = hsr_get_version(hsr, &ver);
+	if (ret)
+		return ret;
+
+	if (ver == HSR_V1)
+		val |= XRS_HSR_CFG_HSR;
+	else if (ver == PRP_V1)
+		val |= XRS_HSR_CFG_PRP;
+	else
+		return -EOPNOTSUPP;
+
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		partner = dp;
+	}
+
+	/* We can't enable redundancy on the switch until both
+	 * redundant ports have signed up.
+	 */
+	if (!partner)
+		return 0;
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_DISABLED);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
+
+	regmap_write(priv->regmap, XRS_HSR_CFG(partner->index),
+		     val | XRS_HSR_CFG_LANID_A);
+	regmap_write(priv->regmap, XRS_HSR_CFG(port),
+		     val | XRS_HSR_CFG_LANID_B);
+
+	/* Clear bits for both redundant ports (HSR only) and the CPU port to
+	 * enable forwarding.
+	 */
+	val = GENMASK(ds->num_ports - 1, 0);
+	if (ver == HSR_V1) {
+		val &= ~BIT(partner->index);
+		val &= ~BIT(port);
+	}
+	val &= ~BIT(dsa_upstream_port(ds, port));
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_FORWARDING);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
+
+	slave = dsa_to_port(ds, port)->slave;
+
+	slave->features |= NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
+			   NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP;
+
+	return 0;
+}
+
+static void xrs700x_hsr_leave(struct dsa_switch *ds, int port,
+			      struct net_device *hsr)
+{
+	struct dsa_port *partner = NULL, *dp;
+	struct xrs700x *priv = ds->priv;
+	struct net_device *slave;
+	unsigned int val;
+
+	dsa_hsr_foreach_port(dp, ds, hsr) {
+		partner = dp;
+	}
+
+	if (!partner)
+		return;
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_DISABLED);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_DISABLED);
+
+	regmap_write(priv->regmap, XRS_HSR_CFG(partner->index), 0);
+	regmap_write(priv->regmap, XRS_HSR_CFG(port), 0);
+
+	/* Clear bit for the CPU port to enable forwarding. */
+	val = GENMASK(ds->num_ports - 1, 0);
+	val &= ~BIT(dsa_upstream_port(ds, port));
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
+	regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
+
+	regmap_fields_write(priv->ps_forward, partner->index,
+			    XRS_PORT_FORWARDING);
+	regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
+
+	slave = dsa_to_port(ds, port)->slave;
+
+	slave->features &= ~(NETIF_F_HW_HSR_TAG_INS | NETIF_F_HW_HSR_TAG_RM |
+			   NETIF_F_HW_HSR_FWD | NETIF_F_HW_HSR_DUP);
+}
+
 static const struct dsa_switch_ops xrs700x_ops = {
 	.get_tag_protocol	= xrs700x_get_tag_protocol,
 	.setup			= xrs700x_setup,
@@ -509,6 +613,8 @@ static const struct dsa_switch_ops xrs700x_ops = {
 	.get_stats64		= xrs700x_get_stats64,
 	.port_bridge_join	= xrs700x_bridge_join,
 	.port_bridge_leave	= xrs700x_bridge_leave,
+	.port_hsr_join		= xrs700x_hsr_join,
+	.port_hsr_leave		= xrs700x_hsr_leave,
 };
 
 static int xrs700x_detect(struct xrs700x *priv)
diff --git a/drivers/net/dsa/xrs700x/xrs700x_reg.h b/drivers/net/dsa/xrs700x/xrs700x_reg.h
index a135d4d92b6d..470d00e07f15 100644
--- a/drivers/net/dsa/xrs700x/xrs700x_reg.h
+++ b/drivers/net/dsa/xrs700x/xrs700x_reg.h
@@ -49,6 +49,11 @@
 
 /* Port Configuration Registers - HSR/PRP */
 #define XRS_HSR_CFG(x)			(XRS_PORT_HSR_BASE(x) + 0x0)
+#define XRS_HSR_CFG_HSR_PRP		BIT(0)
+#define XRS_HSR_CFG_HSR			0
+#define XRS_HSR_CFG_PRP			BIT(8)
+#define XRS_HSR_CFG_LANID_A		0
+#define XRS_HSR_CFG_LANID_B		BIT(10)
 
 /* Port Configuration Registers - PTP */
 #define XRS_PTP_RX_SYNC_DELAY_NS_LO(x)	(XRS_PORT_PTP_BASE(x) + 0x2)
diff --git a/net/dsa/tag_xrs700x.c b/net/dsa/tag_xrs700x.c
index db0ed1a5fcb7..858cdf9d2913 100644
--- a/net/dsa/tag_xrs700x.c
+++ b/net/dsa/tag_xrs700x.c
@@ -11,12 +11,17 @@
 
 static struct sk_buff *xrs700x_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct dsa_port *dp = dsa_slave_to_port(dev);
+	struct dsa_port *partner, *dp = dsa_slave_to_port(dev);
 	u8 *trailer;
 
 	trailer = skb_put(skb, 1);
 	trailer[0] = BIT(dp->index);
 
+	if (dp->hsr_dev)
+		dsa_hsr_foreach_port(partner, dp->ds, dp->hsr_dev)
+			if (partner != dp)
+				trailer[0] |= BIT(partner->index);
+
 	return skb;
 }
 
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 15:59 ` [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support George McCollister
@ 2021-01-22 17:56   ` Florian Fainelli
  2021-01-22 18:00     ` Florian Fainelli
  2021-01-22 18:50     ` George McCollister
  2021-01-24 17:29   ` Vladimir Oltean
  1 sibling, 2 replies; 12+ messages in thread
From: Florian Fainelli @ 2021-01-22 17:56 UTC (permalink / raw)
  To: George McCollister, Jakub Kicinski
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Jonathan Corbet,
	Murali Karicheri, netdev



On 1/22/2021 7:59 AM, George McCollister wrote:
> Add support for offloading of HSR/PRP (IEC 62439-3) tag insertion
> tag removal, duplicate generation and forwarding on DSA switches.
> 
> Use a new netdev_priv_flag IFF_HSR to indicate that a device is an HSR
> device so DSA can tell them apart from other devices in
> dsa_slave_changeupper.
> 
> Add DSA_NOTIFIER_HSR_JOIN and DSA_NOTIFIER_HSR_LEAVE which trigger calls
> to .port_hsr_join and .port_hsr_leave in the DSA driver for the switch.
> 
> The DSA switch driver should then set netdev feature flags for the
> HSR/PRP operation that it offloads.
>     NETIF_F_HW_HSR_TAG_INS
>     NETIF_F_HW_HSR_TAG_RM
>     NETIF_F_HW_HSR_FWD
>     NETIF_F_HW_HSR_DUP
> 
> For HSR, insertion involves the switch adding a 6 byte HSR header after
> the 14 byte Ethernet header. For PRP it adds a 6 byte trailer.
> 
> Tag removal involves automatically stripping the HSR/PRP header/trailer
> in the switch. This is possible when the switch also preforms auto
> deduplication using the HSR/PRP header/trailer (making it no longer
> required).
> 
> Forwarding involves automatically forwarding between redundant ports in
> an HSR. This is crucial because delay is accumulated as a frame passes
> through each node in the ring.
> 
> Duplication involves the switch automatically sending a single frame
> from the CPU port to both redundant ports. This is required because the
> inserted HSR/PRP header/trailer must contain the same sequence number
> on the frames sent out both redundant ports.
> 
> Signed-off-by: George McCollister <george.mccollister@gmail.com>

This is just a high level overview for now, but I would split this into two:

- a patch that adds HSR offload to the existing HSR stack and introduces
the new netdev_features_t bits to support HSR offload, more on that below

- a patch that does the plumbing between HSR and within the DSA framework

> ---
>  Documentation/networking/netdev-features.rst | 20 ++++++++++++++++
>  include/linux/if_hsr.h                       | 22 ++++++++++++++++++
>  include/linux/netdev_features.h              |  9 ++++++++
>  include/linux/netdevice.h                    | 13 +++++++++++
>  include/net/dsa.h                            | 13 +++++++++++
>  net/dsa/dsa_priv.h                           | 11 +++++++++
>  net/dsa/port.c                               | 34 ++++++++++++++++++++++++++++
>  net/dsa/slave.c                              | 13 +++++++++++
>  net/dsa/switch.c                             | 24 ++++++++++++++++++++
>  net/ethtool/common.c                         |  4 ++++
>  net/hsr/hsr_device.c                         | 12 +++-------
>  net/hsr/hsr_forward.c                        | 27 +++++++++++++++++++---
>  net/hsr/hsr_forward.h                        |  1 +
>  net/hsr/hsr_main.c                           | 14 ++++++++++++
>  net/hsr/hsr_main.h                           |  8 +------
>  net/hsr/hsr_slave.c                          | 13 +++++++----
>  16 files changed, 215 insertions(+), 23 deletions(-)
>  create mode 100644 include/linux/if_hsr.h
> 
> diff --git a/Documentation/networking/netdev-features.rst b/Documentation/networking/netdev-features.rst
> index a2d7d7160e39..4eab45405031 100644
> --- a/Documentation/networking/netdev-features.rst
> +++ b/Documentation/networking/netdev-features.rst
> @@ -182,3 +182,23 @@ stricter than Hardware LRO.  A packet stream merged by Hardware GRO must
>  be re-segmentable by GSO or TSO back to the exact original packet stream.
>  Hardware GRO is dependent on RXCSUM since every packet successfully merged
>  by hardware must also have the checksum verified by hardware.
> +
> +* hsr-tag-ins-offload
> +
> +This should be set for devices which insert an HSR (highspeed ring) tag
> +automatically when in HSR mode.
> +
> +* hsr-tag-rm-offload
> +
> +This should be set for devices which remove HSR (highspeed ring) tags
> +automatically when in HSR mode.
> +
> +* hsr-fwd-offload
> +
> +This should be set for devices which forward HSR (highspeed ring) frames from
> +one port to another in hardware.
> +
> +* hsr-dup-offload
> +
> +This should be set for devices which duplicate outgoing HSR (highspeed ring)
> +frames in hardware.

Do you think we can start with a hsr-hw-offload feature and create new
bits to described how challenged a device may be with HSR offload? Is it
 reasonable assumption that functional hardware should be able to
offload all of these functions or none of them?

It may be a good idea to know what the platform that Murali is working
on or has worked on is capable of doing, too.

[snip]

>  
> +static inline bool netif_is_hsr_master(const struct net_device *dev)
> +{
> +	return dev->flags & IFF_MASTER && dev->priv_flags & IFF_HSR;
> +}
> +
> +static inline bool netif_is_hsr_slave(const struct net_device *dev)
> +{
> +	return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_HSR;
> +}

I believe the kernel community is now trying to eliminate the use of the
terms master and slave, can you replace these with some HSR specific
naming maybe?
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 17:56   ` Florian Fainelli
@ 2021-01-22 18:00     ` Florian Fainelli
  2021-01-22 18:48       ` Nishanth Menon
  2021-01-22 18:50     ` George McCollister
  1 sibling, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2021-01-22 18:00 UTC (permalink / raw)
  To: George McCollister, Jakub Kicinski, grygorii.strashko,
	Kishon Vijay Abraham I, Nishanth Menon
  Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, Jonathan Corbet, netdev



On 1/22/2021 9:56 AM, Florian Fainelli wrote:
> 
> 
> On 1/22/2021 7:59 AM, George McCollister wrote:
>> Add support for offloading of HSR/PRP (IEC 62439-3) tag insertion
>> tag removal, duplicate generation and forwarding on DSA switches.
>>
>> Use a new netdev_priv_flag IFF_HSR to indicate that a device is an HSR
>> device so DSA can tell them apart from other devices in
>> dsa_slave_changeupper.
>>
>> Add DSA_NOTIFIER_HSR_JOIN and DSA_NOTIFIER_HSR_LEAVE which trigger calls
>> to .port_hsr_join and .port_hsr_leave in the DSA driver for the switch.
>>
>> The DSA switch driver should then set netdev feature flags for the
>> HSR/PRP operation that it offloads.
>>     NETIF_F_HW_HSR_TAG_INS
>>     NETIF_F_HW_HSR_TAG_RM
>>     NETIF_F_HW_HSR_FWD
>>     NETIF_F_HW_HSR_DUP
>>
>> For HSR, insertion involves the switch adding a 6 byte HSR header after
>> the 14 byte Ethernet header. For PRP it adds a 6 byte trailer.
>>
>> Tag removal involves automatically stripping the HSR/PRP header/trailer
>> in the switch. This is possible when the switch also preforms auto
>> deduplication using the HSR/PRP header/trailer (making it no longer
>> required).
>>
>> Forwarding involves automatically forwarding between redundant ports in
>> an HSR. This is crucial because delay is accumulated as a frame passes
>> through each node in the ring.
>>
>> Duplication involves the switch automatically sending a single frame
>> from the CPU port to both redundant ports. This is required because the
>> inserted HSR/PRP header/trailer must contain the same sequence number
>> on the frames sent out both redundant ports.
>>
>> Signed-off-by: George McCollister <george.mccollister@gmail.com>
> 
> This is just a high level overview for now, but I would split this into two:
> 
> - a patch that adds HSR offload to the existing HSR stack and introduces
> the new netdev_features_t bits to support HSR offload, more on that below
> 
> - a patch that does the plumbing between HSR and within the DSA framework
> 
>> ---
>>  Documentation/networking/netdev-features.rst | 20 ++++++++++++++++
>>  include/linux/if_hsr.h                       | 22 ++++++++++++++++++
>>  include/linux/netdev_features.h              |  9 ++++++++
>>  include/linux/netdevice.h                    | 13 +++++++++++
>>  include/net/dsa.h                            | 13 +++++++++++
>>  net/dsa/dsa_priv.h                           | 11 +++++++++
>>  net/dsa/port.c                               | 34 ++++++++++++++++++++++++++++
>>  net/dsa/slave.c                              | 13 +++++++++++
>>  net/dsa/switch.c                             | 24 ++++++++++++++++++++
>>  net/ethtool/common.c                         |  4 ++++
>>  net/hsr/hsr_device.c                         | 12 +++-------
>>  net/hsr/hsr_forward.c                        | 27 +++++++++++++++++++---
>>  net/hsr/hsr_forward.h                        |  1 +
>>  net/hsr/hsr_main.c                           | 14 ++++++++++++
>>  net/hsr/hsr_main.h                           |  8 +------
>>  net/hsr/hsr_slave.c                          | 13 +++++++----
>>  16 files changed, 215 insertions(+), 23 deletions(-)
>>  create mode 100644 include/linux/if_hsr.h
>>
>> diff --git a/Documentation/networking/netdev-features.rst b/Documentation/networking/netdev-features.rst
>> index a2d7d7160e39..4eab45405031 100644
>> --- a/Documentation/networking/netdev-features.rst
>> +++ b/Documentation/networking/netdev-features.rst
>> @@ -182,3 +182,23 @@ stricter than Hardware LRO.  A packet stream merged by Hardware GRO must
>>  be re-segmentable by GSO or TSO back to the exact original packet stream.
>>  Hardware GRO is dependent on RXCSUM since every packet successfully merged
>>  by hardware must also have the checksum verified by hardware.
>> +
>> +* hsr-tag-ins-offload
>> +
>> +This should be set for devices which insert an HSR (highspeed ring) tag
>> +automatically when in HSR mode.
>> +
>> +* hsr-tag-rm-offload
>> +
>> +This should be set for devices which remove HSR (highspeed ring) tags
>> +automatically when in HSR mode.
>> +
>> +* hsr-fwd-offload
>> +
>> +This should be set for devices which forward HSR (highspeed ring) frames from
>> +one port to another in hardware.
>> +
>> +* hsr-dup-offload
>> +
>> +This should be set for devices which duplicate outgoing HSR (highspeed ring)
>> +frames in hardware.
> 
> Do you think we can start with a hsr-hw-offload feature and create new
> bits to described how challenged a device may be with HSR offload? Is it
>  reasonable assumption that functional hardware should be able to
> offload all of these functions or none of them?
> 
> It may be a good idea to know what the platform that Murali is working
> on or has worked on is capable of doing, too.

Murali's email address is bouncing, adding Grygorii, Kishon and
Nishanth, they may know.
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 18:00     ` Florian Fainelli
@ 2021-01-22 18:48       ` Nishanth Menon
  2021-01-24  8:43         ` Vladimir Oltean
  0 siblings, 1 reply; 12+ messages in thread
From: Nishanth Menon @ 2021-01-22 18:48 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: George McCollister, Jakub Kicinski, grygorii.strashko,
	Kishon Vijay Abraham I, Andrew Lunn, Vivien Didelot,
	Vladimir Oltean, Jonathan Corbet, netdev, Sekhar Nori,
	Lokesh Vutla

On 10:00-20210122, Florian Fainelli wrote:
[...]

> >> +
> >> +This should be set for devices which duplicate outgoing HSR (highspeed ring)
> >> +frames in hardware.
> > 
> > Do you think we can start with a hsr-hw-offload feature and create new
> > bits to described how challenged a device may be with HSR offload? Is it
> >  reasonable assumption that functional hardware should be able to
> > offload all of these functions or none of them?
> > 
> > It may be a good idea to know what the platform that Murali is working
> > on or has worked on is capable of doing, too.
> 
> Murali's email address is bouncing, adding Grygorii, Kishon and
> Nishanth, they may know.

Gee, thanks for looping us in.. Yup, unfortunately, Murali is no longer
with TI. I have bounced the email over to right folks, whom I hope will
be able to help add more color..

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 17:56   ` Florian Fainelli
  2021-01-22 18:00     ` Florian Fainelli
@ 2021-01-22 18:50     ` George McCollister
  1 sibling, 0 replies; 12+ messages in thread
From: George McCollister @ 2021-01-22 18:50 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jakub Kicinski, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
	Jonathan Corbet, Murali Karicheri, netdev

On Fri, Jan 22, 2021 at 11:56 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 1/22/2021 7:59 AM, George McCollister wrote:
> > Add support for offloading of HSR/PRP (IEC 62439-3) tag insertion
> > tag removal, duplicate generation and forwarding on DSA switches.
> >
> > Use a new netdev_priv_flag IFF_HSR to indicate that a device is an HSR
> > device so DSA can tell them apart from other devices in
> > dsa_slave_changeupper.
> >
> > Add DSA_NOTIFIER_HSR_JOIN and DSA_NOTIFIER_HSR_LEAVE which trigger calls
> > to .port_hsr_join and .port_hsr_leave in the DSA driver for the switch.
> >
> > The DSA switch driver should then set netdev feature flags for the
> > HSR/PRP operation that it offloads.
> >     NETIF_F_HW_HSR_TAG_INS
> >     NETIF_F_HW_HSR_TAG_RM
> >     NETIF_F_HW_HSR_FWD
> >     NETIF_F_HW_HSR_DUP
> >
> > For HSR, insertion involves the switch adding a 6 byte HSR header after
> > the 14 byte Ethernet header. For PRP it adds a 6 byte trailer.
> >
> > Tag removal involves automatically stripping the HSR/PRP header/trailer
> > in the switch. This is possible when the switch also preforms auto
> > deduplication using the HSR/PRP header/trailer (making it no longer
> > required).
> >
> > Forwarding involves automatically forwarding between redundant ports in
> > an HSR. This is crucial because delay is accumulated as a frame passes
> > through each node in the ring.
> >
> > Duplication involves the switch automatically sending a single frame
> > from the CPU port to both redundant ports. This is required because the
> > inserted HSR/PRP header/trailer must contain the same sequence number
> > on the frames sent out both redundant ports.
> >
> > Signed-off-by: George McCollister <george.mccollister@gmail.com>
>
> This is just a high level overview for now, but I would split this into two:
>
> - a patch that adds HSR offload to the existing HSR stack and introduces
> the new netdev_features_t bits to support HSR offload, more on that below
>
> - a patch that does the plumbing between HSR and within the DSA framework

Okay. I think I could just move everything that touches DSA into a
second commit. I kind of pondered this to begin but settled on it
being unnecessary.

>
> > ---
>
> Do you think we can start with a hsr-hw-offload feature and create new
> bits to described how challenged a device may be with HSR offload? Is it
>  reasonable assumption that functional hardware should be able to
> offload all of these functions or none of them?

I'm fine with either way. I see the ksz9477 supports HSR (not sure
which version) and it looks like it doesn't do header insertion or
removal.

I expect the insertion and removal would always come as a pair but I
suppose someone could always do something weird and just do one.
Forwarding and duplication seem like a given for anything claiming
HSR/PRP hardware support.

Knowing this do you think I should go ahead and just change it to
hsr-hw-offload and if someone adds ksz9477 support or whatever later
they can add a new bit?

>
> It may be a good idea to know what the platform that Murali is working
> on or has worked on is capable of doing, too.

Yeah, I copied him but it bounced as did yours.

>
> [snip]
>
> >
> > +static inline bool netif_is_hsr_master(const struct net_device *dev)
> > +{
> > +     return dev->flags & IFF_MASTER && dev->priv_flags & IFF_HSR;
> > +}
> > +
> > +static inline bool netif_is_hsr_slave(const struct net_device *dev)
> > +{
> > +     return dev->flags & IFF_SLAVE && dev->priv_flags & IFF_HSR;
> > +}
>
> I believe the kernel community is now trying to eliminate the use of the
> terms master and slave, can you replace these with some HSR specific
> naming maybe?

Understood however these both already exist and are referenced in ~40
places. Are you saying I should create different macros that wrap them
or change everywhere they are used?

> --
> Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 18:48       ` Nishanth Menon
@ 2021-01-24  8:43         ` Vladimir Oltean
  2021-01-25 13:56           ` George McCollister
  0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-01-24  8:43 UTC (permalink / raw)
  To: Nishanth Menon
  Cc: Florian Fainelli, George McCollister, Jakub Kicinski,
	grygorii.strashko, Kishon Vijay Abraham I, Andrew Lunn,
	Vivien Didelot, Jonathan Corbet, netdev, Sekhar Nori,
	Lokesh Vutla

Hi George,

On Fri, Jan 22, 2021 at 12:48:11PM -0600, Nishanth Menon wrote:
> On 10:00-20210122, Florian Fainelli wrote:
> [...]
>
> > >> +
> > >> +This should be set for devices which duplicate outgoing HSR (highspeed ring)
> > >> +frames in hardware.
> > >
> > > Do you think we can start with a hsr-hw-offload feature and create new
> > > bits to described how challenged a device may be with HSR offload? Is it
> > >  reasonable assumption that functional hardware should be able to
> > > offload all of these functions or none of them?
> > >
> > > It may be a good idea to know what the platform that Murali is working
> > > on or has worked on is capable of doing, too.
> >
> > Murali's email address is bouncing, adding Grygorii, Kishon and
> > Nishanth, they may know.
>
> Gee, thanks for looping us in.. Yup, unfortunately, Murali is no longer
> with TI. I have bounced the email over to right folks, whom I hope will
> be able to help add more color..

I would like to give HSR a spin to get a better idea of what you're doing, but
it's kinda hard when this happens out of the box, with none of your changes
already:

[ 1385.000453] hsr0: hw csum failure
[ 1385.004105] skb len=333 headroom=78 headlen=333 tailroom=293
[ 1385.004105] mac=(64,14) net=(78,20) trans=98
[ 1385.004105] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
[ 1385.004105] csum(0x14a00 ip_summed=2 complete_sw=0 valid=0 level=0)
[ 1385.004105] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=1 iif=16
[ 1385.032520] dev name=hsr0 feat=0x0x0000000000007400
[ 1385.037496] skb headroom: 00000000: 44 00 00 02 18 00 00 06 ef 00 00 00 b4 da ff ff
[ 1385.045257] skb headroom: 00000010: ff ff ff ff 00 04 9f 05 de 0a 81 00 08 00 08 00
[ 1385.053013] skb headroom: 00000020: 45 00 01 4d e1 e0 00 00 40 11 97 c0 00 00 00 00
[ 1385.060767] skb headroom: 00000030: ff ff ff ff 00 44 00 43 01 39 f3 8a 01 01 06 00
[ 1385.068521] skb headroom: 00000040: ff ff ff ff ff ff 00 04 9f 05 f4 ab 08 00
[ 1385.075753] skb linear:   00000000: 45 00 01 4d f4 b2 00 00 40 11 84 ee 00 00 00 00
[ 1385.083508] skb linear:   00000010: ff ff ff ff 00 44 00 43 01 39 e7 0e 01 01 06 00
[ 1385.091262] skb linear:   00000020: 90 5c 56 3c 00 7c 00 00 00 00 00 00 00 00 00 00
[ 1385.099116] skb linear:   00000030: 00 00 00 00 00 00 00 00 00 04 9f 05 f4 ab 00 00
[ 1385.106874] skb linear:   00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.114628] skb linear:   00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.122380] skb linear:   00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.130132] skb linear:   00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.137883] skb linear:   00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.145635] skb linear:   00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.153386] skb linear:   000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.161139] skb linear:   000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.168890] skb linear:   000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.176641] skb linear:   000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.184392] skb linear:   000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.192144] skb linear:   000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.199896] skb linear:   00000100: 00 00 00 00 00 00 00 00 63 82 53 63 35 01 01 3d
[ 1385.207730] skb linear:   00000110: 13 ff 9f 05 f4 ab 00 01 00 01 26 da 82 ac 00 04
[ 1385.215566] skb linear:   00000120: 9f 05 de 0a 50 00 74 01 01 39 02 05 ba 0c 0a 4c
[ 1385.223323] skb linear:   00000130: 53 31 30 32 38 41 52 44 42 91 01 01 37 0e 01 79
[ 1385.231075] skb linear:   00000140: 21 03 06 0c 0f 1a 1c 33 36 3a 3b 77 ff
[ 1385.238042] skb tailroom: 00000000: 32 39 30 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.245795] skb tailroom: 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.253547] skb tailroom: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.261298] skb tailroom: 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.269050] skb tailroom: 00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.276801] skb tailroom: 00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.284553] skb tailroom: 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.292304] skb tailroom: 00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.300110] skb tailroom: 00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.307864] skb tailroom: 00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.315615] skb tailroom: 000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.323367] skb tailroom: 000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.331119] skb tailroom: 000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.338871] skb tailroom: 000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.346622] skb tailroom: 000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.354373] skb tailroom: 000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.362124] skb tailroom: 00000100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.369875] skb tailroom: 00000110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1385.377626] skb tailroom: 00000120: 00 00 00 00 00
[ 1385.382504] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-rc4+ #410
[ 1385.389117] Hardware name: LS1028A RDB Board (DT)
[ 1385.393885] Call trace:
[ 1385.396380]  dump_backtrace+0x0/0x1f0
[ 1385.400130]  show_stack+0x24/0x80
[ 1385.403516]  dump_stack+0xf8/0x168
[ 1385.406990]  netdev_rx_csum_fault.part.0+0x54/0x64
[ 1385.411861]  netdev_rx_csum_fault+0x48/0x50
[ 1385.416119]  __skb_checksum_complete+0x110/0x120
[ 1385.420811]  nf_ip_checksum+0x88/0x160
[ 1385.424636]  nf_checksum+0x58/0x70
[ 1385.428110]  nf_conntrack_udp_packet+0x194/0x2a0
[ 1385.432807]  nf_conntrack_in+0x148/0x7d0
[ 1385.436800]  ipv4_conntrack_in+0x24/0x30
[ 1385.440797]  nf_hook_slow+0x58/0x100
[ 1385.444443]  ip_rcv+0x13c/0x210
[ 1385.447654]  __netif_receive_skb_one_core+0x60/0x90
[ 1385.452603]  __netif_receive_skb+0x20/0x70
[ 1385.456764]  process_backlog+0x138/0x2e4
[ 1385.460753]  net_rx_action+0x12c/0x424
[ 1385.464566]  __do_softirq+0x1f4/0x630
[ 1385.468291]  __irq_exit_rcu+0x194/0x1c0
[ 1385.472199]  irq_exit+0x1c/0x4c
[ 1385.475407]  __handle_domain_irq+0x8c/0xec
[ 1385.479574]  gic_handle_irq+0xcc/0x14c
[ 1385.483386]  el1_irq+0xb4/0x180
[ 1385.486589]  cpuidle_enter_state+0xdc/0x31c
[ 1385.490844]  cpuidle_enter+0x44/0x5c
[ 1385.494486]  do_idle+0x240/0x2d0
[ 1385.497775]  cpu_startup_entry+0x30/0x8c
[ 1385.501762]  rest_init+0x1b8/0x28c
[ 1385.505226]  arch_call_rest_init+0x1c/0x28
[ 1385.509397]  start_kernel+0x580/0x5b8

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-22 15:59 ` [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support George McCollister
  2021-01-22 17:56   ` Florian Fainelli
@ 2021-01-24 17:29   ` Vladimir Oltean
  2021-01-25 14:33     ` George McCollister
  1 sibling, 1 reply; 12+ messages in thread
From: Vladimir Oltean @ 2021-01-24 17:29 UTC (permalink / raw)
  To: George McCollister
  Cc: Jakub Kicinski, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jonathan Corbet, Nishanth Menon, Grygorii Strashko,
	Kishon Vijay Abraham I, Sekhar Nori, Lokesh Vutla, netdev

On Fri, Jan 22, 2021 at 09:59:47AM -0600, George McCollister wrote:
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index f2fb433f3828..fc7e3ff11c5c 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1924,6 +1924,19 @@ static int dsa_slave_changeupper(struct net_device *dev,
>  			dsa_port_lag_leave(dp, info->upper_dev);
>  			err = NOTIFY_OK;
>  		}
> +	} else if (netif_is_hsr_master(info->upper_dev)) {
> +		if (info->linking) {
> +			err = dsa_port_hsr_join(dp, info->upper_dev);
> +			if (err == -EOPNOTSUPP) {
> +				NL_SET_ERR_MSG_MOD(info->info.extack,
> +						   "Offloading not supported");
> +				err = 0;
> +			}
> +			err = notifier_from_errno(err);
> +		} else {
> +			dsa_port_hsr_leave(dp, info->upper_dev);
> +			err = NOTIFY_OK;
> +		}
>  	}

How is the RedBox use case handled with the Linux hsr driver (i.e. a
HSR-unaware SAN endpoint attached to a HSR ring)? I would expect
something like this:

                   +---------+
                   |         |
                   |   SAN   |
                   |         |
                   +---------+
                        |
                        |
                        |
 +-----------------+---------+------------------+
 |                 |         |                  |
 |  Your           |   swp0  |                  |
 |  board          |         |                  |
 |                 +---------+                  |
 |                    |   ^                     |
 |                    |   |                     |
 |                    |   | br0                 |
 |                    |   |                     |
 |                    v   |                     |
 |       +-----------------------------+        |
 |       |                             |        |
 |       |             hsr0            |        |
 |       |                             |        |
 |       +---------+---------+---------+        |
 |       |         |         |         |        |
 |       |   swp1  |  DAN-H  |  swp2   |        |
 |       |         |         |         |        |
 +-------+---------+---------+---------+--------+
            |   ^               |   ^
    to/from |   |               |   | to/from
     ring   |   |               |   |  ring
            v   |               v   |

Therefore, aren't you interested in offloading this setup as well?
I.e. the case where the hsr0 interface joins a bridge that also
contains other DSA switch ports. This would be similar to the LAG
offload recently added by Tobias.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-24  8:43         ` Vladimir Oltean
@ 2021-01-25 13:56           ` George McCollister
  0 siblings, 0 replies; 12+ messages in thread
From: George McCollister @ 2021-01-25 13:56 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Nishanth Menon, Florian Fainelli, Jakub Kicinski,
	grygorii.strashko, Kishon Vijay Abraham I, Andrew Lunn,
	Vivien Didelot, Jonathan Corbet, netdev, Sekhar Nori,
	Lokesh Vutla

On Sun, Jan 24, 2021 at 2:43 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
[snip]
>
> I would like to give HSR a spin to get a better idea of what you're doing, but
> it's kinda hard when this happens out of the box, with none of your changes
> already:
>
> [ 1385.000453] hsr0: hw csum failure
> [ 1385.004105] skb len=333 headroom=78 headlen=333 tailroom=293
> [ 1385.004105] mac=(64,14) net=(78,20) trans=98
> [ 1385.004105] shinfo(txflags=0 nr_frags=0 gso(size=0 type=0 segs=0))
> [ 1385.004105] csum(0x14a00 ip_summed=2 complete_sw=0 valid=0 level=0)
> [ 1385.004105] hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=1 iif=16
> [ 1385.032520] dev name=hsr0 feat=0x0x0000000000007400
> [ 1385.037496] skb headroom: 00000000: 44 00 00 02 18 00 00 06 ef 00 00 00 b4 da ff ff
> [ 1385.045257] skb headroom: 00000010: ff ff ff ff 00 04 9f 05 de 0a 81 00 08 00 08 00
> [ 1385.053013] skb headroom: 00000020: 45 00 01 4d e1 e0 00 00 40 11 97 c0 00 00 00 00
> [ 1385.060767] skb headroom: 00000030: ff ff ff ff 00 44 00 43 01 39 f3 8a 01 01 06 00
> [ 1385.068521] skb headroom: 00000040: ff ff ff ff ff ff 00 04 9f 05 f4 ab 08 00
> [ 1385.075753] skb linear:   00000000: 45 00 01 4d f4 b2 00 00 40 11 84 ee 00 00 00 00
> [ 1385.083508] skb linear:   00000010: ff ff ff ff 00 44 00 43 01 39 e7 0e 01 01 06 00
> [ 1385.091262] skb linear:   00000020: 90 5c 56 3c 00 7c 00 00 00 00 00 00 00 00 00 00
> [ 1385.099116] skb linear:   00000030: 00 00 00 00 00 00 00 00 00 04 9f 05 f4 ab 00 00
> [ 1385.106874] skb linear:   00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.114628] skb linear:   00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.122380] skb linear:   00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.130132] skb linear:   00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.137883] skb linear:   00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.145635] skb linear:   00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.153386] skb linear:   000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.161139] skb linear:   000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.168890] skb linear:   000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.176641] skb linear:   000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.184392] skb linear:   000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.192144] skb linear:   000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.199896] skb linear:   00000100: 00 00 00 00 00 00 00 00 63 82 53 63 35 01 01 3d
> [ 1385.207730] skb linear:   00000110: 13 ff 9f 05 f4 ab 00 01 00 01 26 da 82 ac 00 04
> [ 1385.215566] skb linear:   00000120: 9f 05 de 0a 50 00 74 01 01 39 02 05 ba 0c 0a 4c
> [ 1385.223323] skb linear:   00000130: 53 31 30 32 38 41 52 44 42 91 01 01 37 0e 01 79
> [ 1385.231075] skb linear:   00000140: 21 03 06 0c 0f 1a 1c 33 36 3a 3b 77 ff
> [ 1385.238042] skb tailroom: 00000000: 32 39 30 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.245795] skb tailroom: 00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.253547] skb tailroom: 00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.261298] skb tailroom: 00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.269050] skb tailroom: 00000040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.276801] skb tailroom: 00000050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.284553] skb tailroom: 00000060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.292304] skb tailroom: 00000070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.300110] skb tailroom: 00000080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.307864] skb tailroom: 00000090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.315615] skb tailroom: 000000a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.323367] skb tailroom: 000000b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.331119] skb tailroom: 000000c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.338871] skb tailroom: 000000d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.346622] skb tailroom: 000000e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.354373] skb tailroom: 000000f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.362124] skb tailroom: 00000100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.369875] skb tailroom: 00000110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1385.377626] skb tailroom: 00000120: 00 00 00 00 00
> [ 1385.382504] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-rc4+ #410
> [ 1385.389117] Hardware name: LS1028A RDB Board (DT)
> [ 1385.393885] Call trace:
> [ 1385.396380]  dump_backtrace+0x0/0x1f0
> [ 1385.400130]  show_stack+0x24/0x80
> [ 1385.403516]  dump_stack+0xf8/0x168
> [ 1385.406990]  netdev_rx_csum_fault.part.0+0x54/0x64
> [ 1385.411861]  netdev_rx_csum_fault+0x48/0x50
> [ 1385.416119]  __skb_checksum_complete+0x110/0x120
> [ 1385.420811]  nf_ip_checksum+0x88/0x160
> [ 1385.424636]  nf_checksum+0x58/0x70
> [ 1385.428110]  nf_conntrack_udp_packet+0x194/0x2a0
> [ 1385.432807]  nf_conntrack_in+0x148/0x7d0
> [ 1385.436800]  ipv4_conntrack_in+0x24/0x30
> [ 1385.440797]  nf_hook_slow+0x58/0x100
> [ 1385.444443]  ip_rcv+0x13c/0x210
> [ 1385.447654]  __netif_receive_skb_one_core+0x60/0x90
> [ 1385.452603]  __netif_receive_skb+0x20/0x70
> [ 1385.456764]  process_backlog+0x138/0x2e4
> [ 1385.460753]  net_rx_action+0x12c/0x424
> [ 1385.464566]  __do_softirq+0x1f4/0x630
> [ 1385.468291]  __irq_exit_rcu+0x194/0x1c0
> [ 1385.472199]  irq_exit+0x1c/0x4c
> [ 1385.475407]  __handle_domain_irq+0x8c/0xec
> [ 1385.479574]  gic_handle_irq+0xcc/0x14c
> [ 1385.483386]  el1_irq+0xb4/0x180
> [ 1385.486589]  cpuidle_enter_state+0xdc/0x31c
> [ 1385.490844]  cpuidle_enter+0x44/0x5c
> [ 1385.494486]  do_idle+0x240/0x2d0
> [ 1385.497775]  cpu_startup_entry+0x30/0x8c
> [ 1385.501762]  rest_init+0x1b8/0x28c
> [ 1385.505226]  arch_call_rest_init+0x1c/0x28
> [ 1385.509397]  start_kernel+0x580/0x5b8

Yikes! I never ran into this on any of the hardware I tested on.

-George

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support
  2021-01-24 17:29   ` Vladimir Oltean
@ 2021-01-25 14:33     ` George McCollister
  0 siblings, 0 replies; 12+ messages in thread
From: George McCollister @ 2021-01-25 14:33 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Jakub Kicinski, Andrew Lunn, Vivien Didelot, Florian Fainelli,
	Jonathan Corbet, Nishanth Menon, Grygorii Strashko,
	Kishon Vijay Abraham I, Sekhar Nori, Lokesh Vutla, netdev

On Sun, Jan 24, 2021 at 11:29 AM Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Fri, Jan 22, 2021 at 09:59:47AM -0600, George McCollister wrote:
> > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > index f2fb433f3828..fc7e3ff11c5c 100644
> > --- a/net/dsa/slave.c
> > +++ b/net/dsa/slave.c
> > @@ -1924,6 +1924,19 @@ static int dsa_slave_changeupper(struct net_device *dev,
> >                       dsa_port_lag_leave(dp, info->upper_dev);
> >                       err = NOTIFY_OK;
> >               }
> > +     } else if (netif_is_hsr_master(info->upper_dev)) {
> > +             if (info->linking) {
> > +                     err = dsa_port_hsr_join(dp, info->upper_dev);
> > +                     if (err == -EOPNOTSUPP) {
> > +                             NL_SET_ERR_MSG_MOD(info->info.extack,
> > +                                                "Offloading not supported");
> > +                             err = 0;
> > +                     }
> > +                     err = notifier_from_errno(err);
> > +             } else {
> > +                     dsa_port_hsr_leave(dp, info->upper_dev);
> > +                     err = NOTIFY_OK;
> > +             }
> >       }
>
> How is the RedBox use case handled with the Linux hsr driver (i.e. a
> HSR-unaware SAN endpoint attached to a HSR ring)? I would expect
> something like this:
>
>                    +---------+
>                    |         |
>                    |   SAN   |
>                    |         |
>                    +---------+
>                         |
>                         |
>                         |
>  +-----------------+---------+------------------+
>  |                 |         |                  |
>  |  Your           |   swp0  |                  |
>  |  board          |         |                  |
>  |                 +---------+                  |
>  |                    |   ^                     |
>  |                    |   |                     |
>  |                    |   | br0                 |
>  |                    |   |                     |
>  |                    v   |                     |
>  |       +-----------------------------+        |
>  |       |                             |        |
>  |       |             hsr0            |        |
>  |       |                             |        |
>  |       +---------+---------+---------+        |
>  |       |         |         |         |        |
>  |       |   swp1  |  DAN-H  |  swp2   |        |
>  |       |         |         |         |        |
>  +-------+---------+---------+---------+--------+
>             |   ^               |   ^
>     to/from |   |               |   | to/from
>      ring   |   |               |   |  ring
>             v   |               v   |
>
> Therefore, aren't you interested in offloading this setup as well?
> I.e. the case where the hsr0 interface joins a bridge that also
> contains other DSA switch ports. This would be similar to the LAG
> offload recently added by Tobias.

I'm familiar with this use case but I don't currently need to support
it on products I'm working on and in fact my hardware doesn't support
it because the fourth switch port isn't brought out on the boards.
I've also never tested it in software. I suppose when an hsr is
bridged with a non-HSR switch port it would need to figure out the hsr
redundancy interfaces are on the same switch and configure the
forwarding accordingly. It's also worth noting that on switches which
don't support automatic insertion/deletion of the HSR/PRP tag (like
the ksz9477 I think) this wouldn't be possible.

If someone has hardware that supports this and wants to work with me
to add support for it I'd certainly be open to it. Adding support for
this in the future if demand arises seems like the best plan given
what I have to work with.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-26  5:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-22 15:59 [RFC PATCH net-next 0/3] add HSR offloading support for DSA switches George McCollister
2021-01-22 15:59 ` [RFC PATCH net-next 1/3] net: hsr: generate supervision frame without HSR tag George McCollister
2021-01-22 15:59 ` [RFC PATCH net-next 2/3] net: hsr: add DSA offloading support George McCollister
2021-01-22 17:56   ` Florian Fainelli
2021-01-22 18:00     ` Florian Fainelli
2021-01-22 18:48       ` Nishanth Menon
2021-01-24  8:43         ` Vladimir Oltean
2021-01-25 13:56           ` George McCollister
2021-01-22 18:50     ` George McCollister
2021-01-24 17:29   ` Vladimir Oltean
2021-01-25 14:33     ` George McCollister
2021-01-22 15:59 ` [RFC PATCH net-next 3/3] net: dsa: xrs700x: add HSR " George McCollister

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.