[PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Sebastian Andrzej Siewior]

HSR forwards all packets it received on slave port 1 to slave port 2 and
one of the two copies to the user (master) interface.
In terms of PTP this is not good because the latency introduced by
forwarding makes the timestamp in the PTP packet inaccurate.
The PTP packets should not forwarded like regular packets.

In order to work with PTP over HSR the following has been done:
- PTP packets which are received are dropped within the HSR stack. That
  means they are not forwarded or injected into the master port. If the
  user requires them, then they need to be obtained directly from the
  SLAVE interface.

- Sending packets. If the ethernet type of the packet is ETH_P_1588 then
  the stack assumes a header of type struct hsr_inline_header. The size
  of this header is as ethhdr. As a safeguard, the header contains a
  magic field which matches the position of h_source and it needs to
  match HSR_INLINE_HDR.
  Once this is verified, the header contains the port on which this
  packet needs to be sent and if system's HSR header should be added.
  This information is used with the HSR stack and also attached to skb
  as a skb-extension. The packet is then pulled passed the custom header
  so the remaining stack will see the actual data.
  The skb-extension is needed so that an ethernet driver, with
  HSR-offload capabilities, knows that it must not add HSR-header (unless
  requested) and send the packet on both ports.

The originally submitted skb is freed and only the (altered) clone is
submitted to the slave interface for sending. Therefore on cloning, the
socket and tx_flags/ tskey are copied so that the PTP timestamp is
forwarded to the submitting socket.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
I am trying to extend linuxptp to support PTP over a HSR network.

This is the kernel side of the changes. In short PTP over HSR sends its
packets to a multicast address and every node needs to forward the PTP
packet (SYNC and FOLLOW-UP for instance) within the HSR ring.
In order to achieve this, the HSR stack must not duplicate and forward
the PTP packets as it would do with other packets. The delay caused by
the duplication and forwarding adds overhead which in turn makes the
timing information within the PTP packet inaccurate.

My current approach is to open the slave devices (eth0/ eth1) from
userland in order to receive the PTP packets. Sending happens from the
hsr0 device. The actual packet can have an optional inline header
prepended of type struct hsr_inline_header. The size of the header is
equivalent to ethhdr. The header has a type (h_proto) at the same
position as ethhdr and expects it to be ETH_P_1588 as this extra meta
information is only relevant for PTP packets. It makes no sense to send
PTP packets via the HSR interface because it gets duplicated and the
timestamp information is lost so this should not break anything. As an
additional safe guard there is a magic value at h_source position. The
value has '0xaf' at the most significant byte which makes the address a
locally administered multicast address.

The header passes two information from userland: On which slave port
the packet has to be sent and does the HSR stack need to prepend a
header or not.
This information is then added to a skb-extension, the header is skipped
so that the remaining stack sees the actual data and then send it as
requested.

The PRP packets are sent directly via the SLAVE interface. The standard
mandates not add a PRP trailer (PRP, redundancy control trailer) to PTP
packets. There not really a reason to use hsr interface.

HSR hardware offloading is optional. If the skb has a HSR skb-extension
then based on the provided information it decides if it is required to
add a HSR-header or send it as-is.
If the extension is not present and the ethernet type is PTP then it
assumes a PRP network and the packet is sent as-is on the requested
port.

This has been tested in a pure software environment and in an HW-assisted
environment where the HW is able to duplicate and duplicate packets
but does not do it for PTP packets.
It has not been tested within an environment where the HW is able to
forward the PTP packet and correctly update the timing information.
---
v2…v3: https://patch.msgid.link/20260309-hsr_ptp-v2-0-798262aad3a4@linutronix.de
- Remove af_packet changes entirely.
- Add an internal header to pass additional information for HSR-PTP
  packets.
- Remove PRP, userland will use slave devices directly.
- Drop all received PTP packets. Userland needs to use the slave device
  for RX.

v1…v2: https://patch.msgid.link/20260204-hsr_ptp-v1-0-b421c69a77da@linutronix.de
- Added PRP support
- skb extention is used instead of extending struct skb_shared_info
- in af_packet
  - packet_sendmsg_spkt() is no longer extended
  - jump labels are used to avoid the overhead if there no socket that
    is using this HSR extension.
---
 include/linux/if_hsr.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/skbuff.h |  3 ++
 net/core/skbuff.c      |  4 +++
 net/hsr/Kconfig        |  1 +
 net/hsr/hsr_device.c   | 51 +++++++++++++++++++++++------
 net/hsr/hsr_forward.c  | 74 +++++++++++++++++++++++++++++++++++++-----
 net/hsr/hsr_framereg.h |  1 +
 net/hsr/hsr_slave.c    | 33 +++++++++++++++----
 8 files changed, 229 insertions(+), 25 deletions(-)

diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
index f4cf2dd36d193..220f6e5d7b24c 100644
--- a/include/linux/if_hsr.h
+++ b/include/linux/if_hsr.h
@@ -3,6 +3,7 @@
 #define _LINUX_IF_HSR_H_
 
 #include <linux/types.h>
+#include <linux/skbuff.h>
 
 struct net_device;
 
@@ -22,6 +23,21 @@ enum hsr_port_type {
 	HSR_PT_PORTS,	/* This must be the last item in the enum */
 };
 
+struct hsr_ptp_ext {
+	u8	port;
+	u8	header;
+};
+
+#define HSR_INLINE_HDR	0xaf485352
+struct hsr_inline_header {
+	uint8_t tx_port;
+	uint8_t hsr_hdr;
+	uint8_t __pad0[4];
+	uint32_t magic;
+	uint8_t __pad1[2];
+	uint16_t eth_type;
+} __packed;
+
 /* HSR Tag.
  * As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
  * path, LSDU_size, sequence Nr }. But we let eth_header() create { h_dest,
@@ -45,6 +61,60 @@ struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 				     enum hsr_port_type pt);
 int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev,
 		      enum hsr_port_type *type);
+
+static inline bool hsr_skb_has_header(struct sk_buff *skb)
+{
+	struct hsr_ptp_ext *ptp_ext;
+
+	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
+	if (!ptp_ext)
+		return false;
+	return ptp_ext->header;
+}
+
+static inline unsigned int hsr_skb_has_port(struct sk_buff *skb)
+{
+	struct hsr_ptp_ext *ptp_ext;
+
+	if (!skb)
+		return 0;
+
+	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
+	if (!ptp_ext)
+		return 0;
+	return ptp_ext->port;
+}
+
+static inline bool hsr_skb_get_header_port(struct sk_buff *skb, bool *header,
+					   enum hsr_port_type *port_type)
+{
+	struct hsr_ptp_ext *ptp_ext;
+
+	*port_type = HSR_PT_NONE;
+	*header = false;
+
+	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
+	if (!ptp_ext)
+		return false;
+
+	*port_type = ptp_ext->port;
+	*header = ptp_ext->header;
+	return true;
+}
+
+static inline bool hsr_skb_add_header_port(struct sk_buff *skb, bool header,
+					   enum hsr_port_type port)
+{
+	struct hsr_ptp_ext *ptp_ext;
+
+	ptp_ext = skb_ext_add(skb, SKB_EXT_HSR);
+	if (!ptp_ext)
+		return false;
+	ptp_ext->port = port;
+	ptp_ext->header = header;
+	return true;
+}
+
 #else
 static inline bool is_hsr_master(struct net_device *dev)
 {
@@ -68,6 +138,23 @@ static inline int hsr_get_port_type(struct net_device *hsr_dev,
 {
 	return -EINVAL;
 }
+
+static inline bool hsr_skb_has_header(struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline unsigned int hsr_skb_has_port(struct sk_buff *skb)
+{
+	return 0;
+}
+
+static inline bool hsr_skb_get_header_port(struct sk_buff *skb, bool *header,
+					   enum hsr_port_type *port_type)
+{
+	return false;
+}
+
 #endif /* CONFIG_HSR */
 
 #endif /*_LINUX_IF_HSR_H_*/
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2bcf78a4de7b9..17918eecaf6cf 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -5030,6 +5030,9 @@ enum skb_ext_id {
 #endif
 #if IS_ENABLED(CONFIG_CAN)
 	SKB_EXT_CAN,
+#endif
+#if IS_ENABLED(CONFIG_HSR)
+	SKB_EXT_HSR,
 #endif
 	SKB_EXT_NUM, /* must be last */
 };
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7dad68e3b5186..fa0780c4b7d1b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -60,6 +60,7 @@
 #include <linux/errqueue.h>
 #include <linux/prefetch.h>
 #include <linux/bitfield.h>
+#include <linux/if_hsr.h>
 #include <linux/if_vlan.h>
 #include <linux/mpls.h>
 #include <linux/kcov.h>
@@ -5116,6 +5117,9 @@ static const u8 skb_ext_type_len[] = {
 #if IS_ENABLED(CONFIG_CAN)
 	[SKB_EXT_CAN] = SKB_EXT_CHUNKSIZEOF(struct can_skb_ext),
 #endif
+#if IS_ENABLED(CONFIG_HSR)
+	[SKB_EXT_HSR] = SKB_EXT_CHUNKSIZEOF(struct hsr_ptp_ext),
+#endif
 };
 
 static __always_inline __no_profile unsigned int skb_ext_total_length(void)
diff --git a/net/hsr/Kconfig b/net/hsr/Kconfig
index fcacdf4f0ffc3..973f8b03aeb11 100644
--- a/net/hsr/Kconfig
+++ b/net/hsr/Kconfig
@@ -5,6 +5,7 @@
 
 config HSR
 	tristate "High-availability Seamless Redundancy (HSR & PRP)"
+	select SKB_EXTENSIONS
 	help
 	  This enables IEC 62439 defined High-availability Seamless
 	  Redundancy (HSR) and Parallel Redundancy Protocol (PRP).
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 5555b71ab19b5..ac39b2347aa0f 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	rcu_read_lock();
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
-	if (master) {
-		skb->dev = master->dev;
-		skb_reset_mac_header(skb);
-		skb_reset_mac_len(skb);
-		spin_lock_bh(&hsr->seqnr_lock);
-		hsr_forward_skb(skb, master);
-		spin_unlock_bh(&hsr->seqnr_lock);
-	} else {
-		dev_core_stats_tx_dropped_inc(dev);
-		dev_kfree_skb_any(skb);
+	if (!master)
+		goto drop;
+
+	skb->dev = master->dev;
+	if (skb->len > ETH_HLEN * 2) {
+		struct hsr_inline_header *hsr_opt;
+
+		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
+		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
+		if (hsr_opt->eth_type == htons(ETH_P_1588) &&
+		    hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
+			enum hsr_port_type tx_port;
+			bool has_header;
+
+			has_header = hsr_opt->hsr_hdr;
+			tx_port = hsr_opt->tx_port;
+			if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
+				goto drop;
+
+			if (!hsr_skb_add_header_port(skb, has_header, tx_port))
+				goto drop;
+
+			skb_pull(skb, ETH_HLEN);
+			if (has_header)
+				skb_set_network_header(skb, ETH_HLEN + HSR_HLEN);
+			else
+				skb_set_network_header(skb, ETH_HLEN);
+		}
 	}
+
+	skb_reset_mac_header(skb);
+	skb_reset_mac_len(skb);
+	spin_lock_bh(&hsr->seqnr_lock);
+	hsr_forward_skb(skb, master);
+	spin_unlock_bh(&hsr->seqnr_lock);
+
 	rcu_read_unlock();
 
 	return NETDEV_TX_OK;
+
+drop:
+	dev_core_stats_tx_dropped_inc(dev);
+	dev_kfree_skb_any(skb);
+	rcu_read_unlock();
+	return NETDEV_TX_OK;
 }
 
 static const struct header_ops hsr_header_ops = {
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 0aca859c88cbb..3345eb26cd98e 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -12,11 +12,25 @@
 #include <linux/skbuff.h>
 #include <linux/etherdevice.h>
 #include <linux/if_vlan.h>
+#include <net/sock.h>
 #include "hsr_main.h"
 #include "hsr_framereg.h"
 
 struct hsr_node;
 
+static void hsr_parse_req_master(struct hsr_frame_info *frame,
+				 unsigned int *port,
+				 bool *header)
+{
+	*port = HSR_PT_NONE;
+	*header = false;
+
+	if (!frame->skb_std)
+		return;
+
+	hsr_skb_get_header_port(frame->skb_std, header, port);
+}
+
 /* The uses I can see for these HSR supervision frames are:
  * 1) Use the frames that are sent after node initialization ("HSR_TLV.Type =
  *    22") to reset any sequence_nr counters belonging to that node. Useful if
@@ -343,7 +357,10 @@ struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 		hsr_set_path_id(frame, 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);
+		skb = skb_clone(frame->skb_std, GFP_ATOMIC);
+		if (hsr_skb_has_port(skb))
+			skb_set_owner_w(skb, frame->skb_std->sk);
+		return skb;
 	}
 
 	/* Create the new skb with enough headroom to fit the HSR tag */
@@ -365,6 +382,15 @@ struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 	memmove(dst, src, movelen);
 	skb_reset_mac_header(skb);
 
+	if (hsr_skb_has_port(skb)) {
+		/* Packets are bound to a port and the sender may expect time
+		 * information.
+		 */
+		skb_shinfo(skb)->tx_flags = skb_shinfo(frame->skb_std)->tx_flags;
+		skb_shinfo(skb)->tskey = skb_shinfo(frame->skb_std)->tskey;
+		skb_set_owner_w(skb, frame->skb_std->sk);
+	}
+
 	/* skb_put_padto free skb on error and hsr_fill_tag returns NULL in
 	 * that case
 	 */
@@ -420,7 +446,7 @@ static void hsr_deliver_master(struct sk_buff *skb, struct net_device *dev,
 static int hsr_xmit(struct sk_buff *skb, struct hsr_port *port,
 		    struct hsr_frame_info *frame)
 {
-	if (frame->port_rcv->type == HSR_PT_MASTER) {
+	if (frame->port_rcv->type == HSR_PT_MASTER && !frame->has_foreign_header) {
 		hsr_addr_subst_dest(frame->node_src, skb, port);
 
 		/* Address substitution (IEC62439-3 pp 26, 50): replace mac
@@ -518,12 +544,17 @@ bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
  */
 static void hsr_forward_do(struct hsr_frame_info *frame)
 {
+	unsigned int req_tx_port;
+	bool req_tx_keep_header;
 	struct hsr_port *port;
-	struct sk_buff *skb;
 	bool sent = false;
 
+	hsr_parse_req_master(frame, &req_tx_port, &req_tx_keep_header);
+
 	hsr_for_each_port(frame->port_rcv->hsr, port) {
 		struct hsr_priv *hsr = port->hsr;
+		struct sk_buff *skb = NULL;
+
 		/* Don't send frame back the way it came */
 		if (port == frame->port_rcv)
 			continue;
@@ -542,6 +573,19 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 		if ((port->dev->features & NETIF_F_HW_HSR_DUP) && sent)
 			continue;
 
+		/* PTP TX packets have an outgoing port specified */
+		if (req_tx_port != HSR_PT_NONE && req_tx_port != port->type)
+			continue;
+		/* PTP TX packets may already have a HSR header which needs to
+		 * be preserved
+		 */
+		if (req_tx_keep_header) {
+			skb = skb_clone(frame->skb_std, GFP_ATOMIC);
+			if (skb)
+				skb_set_owner_w(skb, frame->skb_std->sk);
+			goto inject_into_stack;
+		}
+
 		/* Don't send frame over port where it has been sent before.
 		 * Also for SAN, this shouldn't be done.
 		 */
@@ -569,6 +613,7 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 		else
 			skb = hsr->proto_ops->get_untagged_frame(frame, port);
 
+inject_into_stack:
 		if (!skb) {
 			frame->port_rcv->dev->stats.rx_dropped++;
 			continue;
@@ -633,6 +678,13 @@ int hsr_fill_frame_info(__be16 proto, struct sk_buff *skb,
 	struct hsr_port *port = frame->port_rcv;
 	struct hsr_priv *hsr = port->hsr;
 
+	if (frame->has_foreign_header) {
+		frame->skb_std = skb;
+
+		WARN_ON_ONCE(port->type != HSR_PT_MASTER);
+		WARN_ON_ONCE(skb->mac_len < sizeof(struct hsr_ethhdr));
+		return 0;
+	}
 	/* HSRv0 supervisory frames double as a tag so treat them as tagged. */
 	if ((!hsr->prot_version && proto == htons(ETH_P_PRP)) ||
 	    proto == htons(ETH_P_HSR)) {
@@ -697,10 +749,15 @@ static int fill_frame_info(struct hsr_frame_info *frame,
 	if (port->type == HSR_PT_INTERLINK)
 		n_db = &hsr->proxy_node_db;
 
-	frame->node_src = hsr_get_node(port, n_db, skb,
-				       frame->is_supervision, port->type);
-	if (!frame->node_src)
-		return -1; /* Unknown node and !is_supervision, or no mem */
+	if (hsr_skb_has_header(skb))
+		frame->has_foreign_header = true;
+
+	if (!frame->has_foreign_header) {
+		frame->node_src = hsr_get_node(port, n_db, skb,
+					       frame->is_supervision, port->type);
+		if (!frame->node_src)
+			return -1; /* Unknown node and !is_supervision, or no mem */
+	}
 
 	ethhdr = (struct ethhdr *)skb_mac_header(skb);
 	frame->is_vlan = false;
@@ -739,7 +796,8 @@ void hsr_forward_skb(struct sk_buff *skb, struct hsr_port *port)
 	if (fill_frame_info(&frame, skb, port) < 0)
 		goto out_drop;
 
-	hsr_register_frame_in(frame.node_src, port, frame.sequence_nr);
+	if (!frame.has_foreign_header)
+		hsr_register_frame_in(frame.node_src, port, frame.sequence_nr);
 	hsr_forward_do(&frame);
 	rcu_read_unlock();
 	/* Gets called for ingress frames as well as egress from master port.
diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h
index c65ecb9257348..fc0341b158f67 100644
--- a/net/hsr/hsr_framereg.h
+++ b/net/hsr/hsr_framereg.h
@@ -27,6 +27,7 @@ struct hsr_frame_info {
 	bool is_local_dest;
 	bool is_local_exclusive;
 	bool is_from_san;
+	bool has_foreign_header;
 };
 
 void hsr_del_self_node(struct hsr_priv *hsr);
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index d9af9e65f72f0..00aa8d848e2a9 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -44,8 +44,7 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 
 	if (hsr_addr_is_self(port->hsr, eth_hdr(skb)->h_source)) {
 		/* Directly kill frames sent by ourselves */
-		kfree_skb(skb);
-		goto finish_consume;
+		goto finish_free_consume;
 	}
 
 	/* For HSR, only tagged frames are expected (unless the device offloads
@@ -64,10 +63,8 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 	skb_reset_mac_header(skb);
 	if ((!hsr->prot_version && protocol == htons(ETH_P_PRP)) ||
 	    protocol == htons(ETH_P_HSR)) {
-		if (!pskb_may_pull(skb, ETH_HLEN + HSR_HLEN)) {
-			kfree_skb(skb);
-			goto finish_consume;
-		}
+		if (!pskb_may_pull(skb, ETH_HLEN + HSR_HLEN))
+			goto finish_free_consume;
 
 		skb_set_network_header(skb, ETH_HLEN + HSR_HLEN);
 	}
@@ -81,10 +78,32 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 		hsr_forward_skb(skb, port);
 		spin_unlock_bh(&hsr->seqnr_lock);
 	} else {
+		struct hsr_ethhdr *hsr_ethhdr;
+
+		/* PTP packets are not supposed to be forwarded via HSR as-is.
+		 * The latency introduced by forwarding renders the time
+		 * information useless. Userland needs to capture the packet on
+		 * the original interface instead of hsr.
+		 */
+		if ((!hsr->prot_version && protocol == htons(ETH_P_PRP)) ||
+		    protocol == htons(ETH_P_HSR)) {
+			/* HSR */
+			hsr_ethhdr = (struct hsr_ethhdr *)skb_mac_header(skb);
+			if (hsr_ethhdr->hsr_tag.encap_proto == htons(ETH_P_1588))
+				goto finish_free_consume;
+		} else {
+			/* PRP */
+			if (protocol == htons(ETH_P_1588))
+				goto finish_free_consume;
+		}
+
 		hsr_forward_skb(skb, port);
 	}
 
-finish_consume:
+	return RX_HANDLER_CONSUMED;
+
+finish_free_consume:
+	kfree_skb(skb);
 	return RX_HANDLER_CONSUMED;
 
 finish_pass:

---
base-commit: 5e9b7d093f3f77cb0af4409559e3d139babfb443
change-id: 20260204-hsr_ptp-1f6380f1d35f

Best regards,
-- 
Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Willem de Bruijn]

Sebastian Andrzej Siewior wrote:
> HSR forwards all packets it received on slave port 1 to slave port 2 and
> one of the two copies to the user (master) interface.
> In terms of PTP this is not good because the latency introduced by
> forwarding makes the timestamp in the PTP packet inaccurate.
> The PTP packets should not forwarded like regular packets.
> 
> In order to work with PTP over HSR the following has been done:
> - PTP packets which are received are dropped within the HSR stack. That
>   means they are not forwarded or injected into the master port. If the
>   user requires them, then they need to be obtained directly from the
>   SLAVE interface.
> 
> - Sending packets. If the ethernet type of the packet is ETH_P_1588 then
>   the stack assumes a header of type struct hsr_inline_header. The size
>   of this header is as ethhdr. As a safeguard, the header contains a
>   magic field which matches the position of h_source and it needs to
>   match HSR_INLINE_HDR.
>   Once this is verified, the header contains the port on which this
>   packet needs to be sent and if system's HSR header should be added.
>   This information is used with the HSR stack and also attached to skb
>   as a skb-extension. The packet is then pulled passed the custom header
>   so the remaining stack will see the actual data.
>   The skb-extension is needed so that an ethernet driver, with
>   HSR-offload capabilities, knows that it must not add HSR-header (unless
>   requested) and send the packet on both ports.
> 
> The originally submitted skb is freed and only the (altered) clone is
> submitted to the slave interface for sending. Therefore on cloning, the
> socket and tx_flags/ tskey are copied so that the PTP timestamp is
> forwarded to the submitting socket.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Great to see a solution that keeps the added logic mostly within HSR
itself, and works for the userspace component too.


> diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
> index f4cf2dd36d193..220f6e5d7b24c 100644
> --- a/include/linux/if_hsr.h
> +++ b/include/linux/if_hsr.h
> @@ -3,6 +3,7 @@
>  #define _LINUX_IF_HSR_H_
>  
>  #include <linux/types.h>
> +#include <linux/skbuff.h>
>  
>  struct net_device;
>  
> @@ -22,6 +23,21 @@ enum hsr_port_type {
>  	HSR_PT_PORTS,	/* This must be the last item in the enum */
>  };
>  
> +struct hsr_ptp_ext {
> +	u8	port;
> +	u8	header;
> +};
> +
> +#define HSR_INLINE_HDR	0xaf485352
> +struct hsr_inline_header {
> +	uint8_t tx_port;
> +	uint8_t hsr_hdr;
> +	uint8_t __pad0[4];
> +	uint32_t magic;
> +	uint8_t __pad1[2];
> +	uint16_t eth_type;
> +} __packed;
> +

No specific need to make this header ethhdr like?

>  /* HSR Tag.
>   * As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
>   * path, LSDU_size, sequence Nr }. But we let eth_header() create { h_dest,
> @@ -45,6 +61,60 @@ struct net_device *hsr_get_port_ndev(struct net_device *ndev,
>  				     enum hsr_port_type pt);
>  int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev,
>  		      enum hsr_port_type *type);
> +
> +static inline bool hsr_skb_has_header(struct sk_buff *skb)
> +{
> +	struct hsr_ptp_ext *ptp_ext;
> +
> +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> +	if (!ptp_ext)
> +		return false;
> +	return ptp_ext->header;
> +}
> +
> +static inline unsigned int hsr_skb_has_port(struct sk_buff *skb)
> +{
> +	struct hsr_ptp_ext *ptp_ext;
> +
> +	if (!skb)
> +		return 0;
> +
> +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> +	if (!ptp_ext)
> +		return 0;
> +	return ptp_ext->port;
> +}
> +
> +static inline bool hsr_skb_get_header_port(struct sk_buff *skb, bool *header,
> +					   enum hsr_port_type *port_type)
> +{
> +	struct hsr_ptp_ext *ptp_ext;
> +
> +	*port_type = HSR_PT_NONE;
> +	*header = false;
> +
> +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> +	if (!ptp_ext)
> +		return false;
> +
> +	*port_type = ptp_ext->port;
> +	*header = ptp_ext->header;
> +	return true;
> +}
> +
> +static inline bool hsr_skb_add_header_port(struct sk_buff *skb, bool header,
> +					   enum hsr_port_type port)
> +{
> +	struct hsr_ptp_ext *ptp_ext;
> +
> +	ptp_ext = skb_ext_add(skb, SKB_EXT_HSR);
> +	if (!ptp_ext)
> +		return false;
> +	ptp_ext->port = port;
> +	ptp_ext->header = header;
> +	return true;
> +}
> +
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 5555b71ab19b5..ac39b2347aa0f 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  	rcu_read_lock();
>  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> -	if (master) {
> -		skb->dev = master->dev;
> -		skb_reset_mac_header(skb);
> -		skb_reset_mac_len(skb);
> -		spin_lock_bh(&hsr->seqnr_lock);
> -		hsr_forward_skb(skb, master);
> -		spin_unlock_bh(&hsr->seqnr_lock);
> -	} else {
> -		dev_core_stats_tx_dropped_inc(dev);
> -		dev_kfree_skb_any(skb);
> +	if (!master)
> +		goto drop;
> +
> +	skb->dev = master->dev;
> +	if (skb->len > ETH_HLEN * 2) {
> +		struct hsr_inline_header *hsr_opt;
> +
> +		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> +		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> +		if (hsr_opt->eth_type == htons(ETH_P_1588) &&
> +		    hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
> +			enum hsr_port_type tx_port;
> +			bool has_header;
> +
> +			has_header = hsr_opt->hsr_hdr;
> +			tx_port = hsr_opt->tx_port;
> +			if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
> +				goto drop;
> +
> +			if (!hsr_skb_add_header_port(skb, has_header, tx_port))
> +				goto drop;

All use of this information happens in the context of this ndo_start_xmit?

If so, no skb_ext needed. Can store the data in skb->cb[], which in
that case is assured to not be overwritten by another layer.

Among various options. skb_ext works, but is a bit heavyhanded for
passing around state within the same layer and call stack.

> +
> +			skb_pull(skb, ETH_HLEN);

Prefer sizeof(struct hsr_inline_header)

> +			if (has_header)
> +				skb_set_network_header(skb, ETH_HLEN + HSR_HLEN);
> +			else
> +				skb_set_network_header(skb, ETH_HLEN);
> +		}
>  	}
> +
> +	skb_reset_mac_header(skb);
> +	skb_reset_mac_len(skb);
> +	spin_lock_bh(&hsr->seqnr_lock);
> +	hsr_forward_skb(skb, master);
> +	spin_unlock_bh(&hsr->seqnr_lock);
> +
>  	rcu_read_unlock();
>  
>  	return NETDEV_TX_OK;
> +
> +drop:
> +	dev_core_stats_tx_dropped_inc(dev);
> +	dev_kfree_skb_any(skb);
> +	rcu_read_unlock();
> +	return NETDEV_TX_OK;
>  }
>  

Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Sebastian Andrzej Siewior]

On 2026-04-29 13:46:13 [-0400], Willem de Bruijn wrote:
> 
> Great to see a solution that keeps the added logic mostly within HSR
> itself, and works for the userspace component too.

;)

> 
> > diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
> > index f4cf2dd36d193..220f6e5d7b24c 100644
> > --- a/include/linux/if_hsr.h
> > +++ b/include/linux/if_hsr.h
> > @@ -3,6 +3,7 @@
> >  #define _LINUX_IF_HSR_H_
> >  
> >  #include <linux/types.h>
> > +#include <linux/skbuff.h>
> >  
> >  struct net_device;
> >  
> > @@ -22,6 +23,21 @@ enum hsr_port_type {
> >  	HSR_PT_PORTS,	/* This must be the last item in the enum */
> >  };
> >  
> > +struct hsr_ptp_ext {
> > +	u8	port;
> > +	u8	header;
> > +};
> > +
> > +#define HSR_INLINE_HDR	0xaf485352
> > +struct hsr_inline_header {
> > +	uint8_t tx_port;
> > +	uint8_t hsr_hdr;
> > +	uint8_t __pad0[4];
> > +	uint32_t magic;
> > +	uint8_t __pad1[2];
> > +	uint16_t eth_type;
> > +} __packed;
> > +
> 
> No specific need to make this header ethhdr like?

What do you mean? eth_type is at the same spot or do you mean it should
be named h_proto?

> >  /* HSR Tag.
> >   * As defined in IEC-62439-3:2010, the HSR tag is really { ethertype = 0x88FB,
> >   * path, LSDU_size, sequence Nr }. But we let eth_header() create { h_dest,
> > @@ -45,6 +61,60 @@ struct net_device *hsr_get_port_ndev(struct net_device *ndev,
> >  				     enum hsr_port_type pt);
> >  int hsr_get_port_type(struct net_device *hsr_dev, struct net_device *dev,
> >  		      enum hsr_port_type *type);
> > +
> > +static inline bool hsr_skb_has_header(struct sk_buff *skb)
> > +{
> > +	struct hsr_ptp_ext *ptp_ext;
> > +
> > +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> > +	if (!ptp_ext)
> > +		return false;
> > +	return ptp_ext->header;
> > +}
> > +
> > +static inline unsigned int hsr_skb_has_port(struct sk_buff *skb)
> > +{
> > +	struct hsr_ptp_ext *ptp_ext;
> > +
> > +	if (!skb)
> > +		return 0;
> > +
> > +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> > +	if (!ptp_ext)
> > +		return 0;
> > +	return ptp_ext->port;
> > +}
> > +
> > +static inline bool hsr_skb_get_header_port(struct sk_buff *skb, bool *header,
> > +					   enum hsr_port_type *port_type)
> > +{
> > +	struct hsr_ptp_ext *ptp_ext;
> > +
> > +	*port_type = HSR_PT_NONE;
> > +	*header = false;
> > +
> > +	ptp_ext = skb_ext_find(skb, SKB_EXT_HSR);
> > +	if (!ptp_ext)
> > +		return false;
> > +
> > +	*port_type = ptp_ext->port;
> > +	*header = ptp_ext->header;
> > +	return true;
> > +}
> > +
> > +static inline bool hsr_skb_add_header_port(struct sk_buff *skb, bool header,
> > +					   enum hsr_port_type port)
> > +{
> > +	struct hsr_ptp_ext *ptp_ext;
> > +
> > +	ptp_ext = skb_ext_add(skb, SKB_EXT_HSR);
> > +	if (!ptp_ext)
> > +		return false;
> > +	ptp_ext->port = port;
> > +	ptp_ext->header = header;
> > +	return true;
> > +}
> > +
> > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > index 5555b71ab19b5..ac39b2347aa0f 100644
> > --- a/net/hsr/hsr_device.c
> > +++ b/net/hsr/hsr_device.c
> > @@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> >  
> >  	rcu_read_lock();
> >  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> > -	if (master) {
> > -		skb->dev = master->dev;
> > -		skb_reset_mac_header(skb);
> > -		skb_reset_mac_len(skb);
> > -		spin_lock_bh(&hsr->seqnr_lock);
> > -		hsr_forward_skb(skb, master);
> > -		spin_unlock_bh(&hsr->seqnr_lock);
> > -	} else {
> > -		dev_core_stats_tx_dropped_inc(dev);
> > -		dev_kfree_skb_any(skb);
> > +	if (!master)
> > +		goto drop;
> > +
> > +	skb->dev = master->dev;
> > +	if (skb->len > ETH_HLEN * 2) {
> > +		struct hsr_inline_header *hsr_opt;
> > +
> > +		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> > +		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> > +		if (hsr_opt->eth_type == htons(ETH_P_1588) &&
> > +		    hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
> > +			enum hsr_port_type tx_port;
> > +			bool has_header;
> > +
> > +			has_header = hsr_opt->hsr_hdr;
> > +			tx_port = hsr_opt->tx_port;
> > +			if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
> > +				goto drop;
> > +
> > +			if (!hsr_skb_add_header_port(skb, has_header, tx_port))
> > +				goto drop;
> 
> All use of this information happens in the context of this ndo_start_xmit?

I receive it from af_packet in HSR's ndo_start_xmit, yes. Then
hsr_xmit() is forwarding it to the slave device via dev_queue_xmit().
Here the skb->cb information gets overwritten.

I need this hint in the slave eth driver in case there is hsr-offloading
available. Now that I look at it again, there a netdev-event once the
HSR-master is set/ changed. So maybe I can use this and look at the
skb frame to decide what is required. Le me see.

> If so, no skb_ext needed. Can store the data in skb->cb[], which in
> that case is assured to not be overwritten by another layer.
> 
> Among various options. skb_ext works, but is a bit heavyhanded for
> passing around state within the same layer and call stack.

Sure.

> > +
> > +			skb_pull(skb, ETH_HLEN);
> 
> Prefer sizeof(struct hsr_inline_header)

Okay.

Sebastian

Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Willem de Bruijn]

> > > +#define HSR_INLINE_HDR	0xaf485352
> > > +struct hsr_inline_header {
> > > +	uint8_t tx_port;
> > > +	uint8_t hsr_hdr;
> > > +	uint8_t __pad0[4];
> > > +	uint32_t magic;
> > > +	uint8_t __pad1[2];
> > > +	uint16_t eth_type;
> > > +} __packed;
> > > +
> > 
> > No specific need to make this header ethhdr like?
> 
> What do you mean? eth_type is at the same spot or do you mean it should
> be named h_proto?

I mean there is no reason your fake header needs to be 14B or
or otherwise resemble an Ethernet header.

That ties into my last comment to use sizeof(struct hsr_inline_header)
where working with that header, rather than ETH_HLEN. It is an
independent header.

This padding and packing is unnecessary.

> > > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > > index 5555b71ab19b5..ac39b2347aa0f 100644
> > > --- a/net/hsr/hsr_device.c
> > > +++ b/net/hsr/hsr_device.c
> > > @@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> > >  
> > >  	rcu_read_lock();
> > >  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> > > -	if (master) {
> > > -		skb->dev = master->dev;
> > > -		skb_reset_mac_header(skb);
> > > -		skb_reset_mac_len(skb);
> > > -		spin_lock_bh(&hsr->seqnr_lock);
> > > -		hsr_forward_skb(skb, master);
> > > -		spin_unlock_bh(&hsr->seqnr_lock);
> > > -	} else {
> > > -		dev_core_stats_tx_dropped_inc(dev);
> > > -		dev_kfree_skb_any(skb);
> > > +	if (!master)
> > > +		goto drop;
> > > +
> > > +	skb->dev = master->dev;
> > > +	if (skb->len > ETH_HLEN * 2) {
> > > +		struct hsr_inline_header *hsr_opt;
> > > +
> > > +		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> > > +		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> > > +		if (hsr_opt->eth_type == htons(ETH_P_1588) &&
> > > +		    hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
> > > +			enum hsr_port_type tx_port;
> > > +			bool has_header;
> > > +
> > > +			has_header = hsr_opt->hsr_hdr;
> > > +			tx_port = hsr_opt->tx_port;
> > > +			if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
> > > +				goto drop;
> > > +
> > > +			if (!hsr_skb_add_header_port(skb, has_header, tx_port))
> > > +				goto drop;
> > 
> > All use of this information happens in the context of this ndo_start_xmit?
> 
> I receive it from af_packet in HSR's ndo_start_xmit, yes. Then
> hsr_xmit() is forwarding it to the slave device via dev_queue_xmit().
> Here the skb->cb information gets overwritten.
> 
> I need this hint in the slave eth driver in case there is hsr-offloading
> available. 

This assumes that the slave device is somehow HSR aware. But standard
net_devices are not?

Isn't it HSR's ndo_start_xmit that selects which slave to forward to,
based on the information in this header?

Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Sebastian Andrzej Siewior]

On 2026-05-04 13:08:04 [-0400], Willem de Bruijn wrote:
> > > > +#define HSR_INLINE_HDR	0xaf485352
> > > > +struct hsr_inline_header {
> > > > +	uint8_t tx_port;
> > > > +	uint8_t hsr_hdr;
> > > > +	uint8_t __pad0[4];
> > > > +	uint32_t magic;
> > > > +	uint8_t __pad1[2];
> > > > +	uint16_t eth_type;
> > > > +} __packed;
> > > > +
> > > 
> > > No specific need to make this header ethhdr like?
> > 
> > What do you mean? eth_type is at the same spot or do you mean it should
> > be named h_proto?
> 
> I mean there is no reason your fake header needs to be 14B or
> or otherwise resemble an Ethernet header.
> 
> That ties into my last comment to use sizeof(struct hsr_inline_header)
> where working with that header, rather than ETH_HLEN. It is an
> independent header.

But I can't expect that the header is always there. A random ping/ arp
packet goes via the same flow. At the same time I don't want to make it
mandatory for all AF_PACKET users by checking the skb's socket.

The skb starts always with the ethernet header. To avoid collisions with
a regular packet that looks similar I use the ether-type of the ethernet
frame and it has to match PTP. It makes no sense to send a PTP packet via
HSR. Since ether-type is at the end, I can't make it smaller than 14b.
So eth-type is safeguard #1 and the second is SRC-MAC with a multicast
sender bit set which is usually not the case.

> This padding and packing is unnecessary.

The padding is needed to match the ethernet-type and restrict it to PTP
only. Then packing is needed so it is not aligned by the compiler.

> > > > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > > > index 5555b71ab19b5..ac39b2347aa0f 100644
> > > > --- a/net/hsr/hsr_device.c
> > > > +++ b/net/hsr/hsr_device.c
> > > > @@ -228,20 +228,51 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
> > > >  
> > > >  	rcu_read_lock();
> > > >  	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
> > > > -	if (master) {
> > > > -		skb->dev = master->dev;
> > > > -		skb_reset_mac_header(skb);
> > > > -		skb_reset_mac_len(skb);
> > > > -		spin_lock_bh(&hsr->seqnr_lock);
> > > > -		hsr_forward_skb(skb, master);
> > > > -		spin_unlock_bh(&hsr->seqnr_lock);
> > > > -	} else {
> > > > -		dev_core_stats_tx_dropped_inc(dev);
> > > > -		dev_kfree_skb_any(skb);
> > > > +	if (!master)
> > > > +		goto drop;
> > > > +
> > > > +	skb->dev = master->dev;
> > > > +	if (skb->len > ETH_HLEN * 2) {
> > > > +		struct hsr_inline_header *hsr_opt;
> > > > +
> > > > +		BUILD_BUG_ON(sizeof(struct hsr_inline_header) != sizeof(struct ethhdr));
> > > > +		hsr_opt = (struct hsr_inline_header *)skb_mac_header(skb);
> > > > +		if (hsr_opt->eth_type == htons(ETH_P_1588) &&
> > > > +		    hsr_opt->magic == htonl(HSR_INLINE_HDR)) {
> > > > +			enum hsr_port_type tx_port;
> > > > +			bool has_header;
> > > > +
> > > > +			has_header = hsr_opt->hsr_hdr;
> > > > +			tx_port = hsr_opt->tx_port;
> > > > +			if (tx_port != HSR_PT_SLAVE_A && tx_port != HSR_PT_SLAVE_B)
> > > > +				goto drop;
> > > > +
> > > > +			if (!hsr_skb_add_header_port(skb, has_header, tx_port))
> > > > +				goto drop;
> > > 
> > > All use of this information happens in the context of this ndo_start_xmit?
> > 
> > I receive it from af_packet in HSR's ndo_start_xmit, yes. Then
> > hsr_xmit() is forwarding it to the slave device via dev_queue_xmit().
> > Here the skb->cb information gets overwritten.
> > 
> > I need this hint in the slave eth driver in case there is hsr-offloading
> > available. 
> 
> This assumes that the slave device is somehow HSR aware. But standard
> net_devices are not?

Yes, standard devices are not hsr-aware. It is just your average
ethernet device that sends everything as-is.
But you can have devices with HSR-offload capabilities such as
NETIF_F_HW_HSR_FWD, NETIF_F_HW_HSR_DUP, NETIF_F_HW_HSR_TAG_INS,
NETIF_F_HW_HSR_TAG_RM.

So if a device provides NETIF_F_HW_HSR_TAG_INS and it is enabled
(via ethtool -K) then the HSR stack will pass regular skb and the device
will prepend the HSR-header (and maintain the HSR-sequence number and so
on) before putting it on the wire.
Also, the HSR stack will send a skb on both slave interfaces which are
regular ethernet devices. But with NETIF_F_HW_HSR_DUP enabled it will
pass it only to the first slave and expect the underlying device to
forward it also via the other slave interface.

So I need to pass skb which is can be sent by a dumb device but at the
same time a device which does offload needs to be able to know when it
should not do it.

> Isn't it HSR's ndo_start_xmit that selects which slave to forward to,
> based on the information in this header?

Yes, this is correct. I needed to verify that the slave device with
offload capabilities does not send the skb on both ports and add a
HSR header while doing so. The non-offloading devices are not an issue,
it sends packets as-is.
I managed to rework it and it works without the skb-ext. I just track
the HSR mode (HSR vs PRP) and then parse the packet for its type and
decide what needs to be done.
This works now. Once we settle on the header I can repost it.

Sebastian

Re: [PATCH RFC net-next v3] hsr: Allow to send a specific port and with HSR header

[Willem de Bruijn]

Sebastian Andrzej Siewior wrote:
> On 2026-05-04 13:08:04 [-0400], Willem de Bruijn wrote:
> > > > > +#define HSR_INLINE_HDR	0xaf485352
> > > > > +struct hsr_inline_header {
> > > > > +	uint8_t tx_port;
> > > > > +	uint8_t hsr_hdr;
> > > > > +	uint8_t __pad0[4];
> > > > > +	uint32_t magic;
> > > > > +	uint8_t __pad1[2];
> > > > > +	uint16_t eth_type;
> > > > > +} __packed;
> > > > > +
> > > > 
> > > > No specific need to make this header ethhdr like?
> > > 
> > > What do you mean? eth_type is at the same spot or do you mean it should
> > > be named h_proto?
> > 
> > I mean there is no reason your fake header needs to be 14B or
> > or otherwise resemble an Ethernet header.
> > 
> > That ties into my last comment to use sizeof(struct hsr_inline_header)
> > where working with that header, rather than ETH_HLEN. It is an
> > independent header.
> 
> But I can't expect that the header is always there. A random ping/ arp
> packet goes via the same flow. At the same time I don't want to make it
> mandatory for all AF_PACKET users by checking the skb's socket.

I thought skb->protocol is the unambiguous signal whether this custom
header is present?

> The skb starts always with the ethernet header. To avoid collisions with
> a regular packet that looks similar I use the ether-type of the ethernet
> frame and it has to match PTP. It makes no sense to send a PTP packet via
> HSR. Since ether-type is at the end, I can't make it smaller than 14b.
> So eth-type is safeguard #1 and the second is SRC-MAC with a multicast
> sender bit set which is usually not the case.

> > > > All use of this information happens in the context of this ndo_start_xmit?
> > > 
> > > I receive it from af_packet in HSR's ndo_start_xmit, yes. Then
> > > hsr_xmit() is forwarding it to the slave device via dev_queue_xmit().
> > > Here the skb->cb information gets overwritten.
> > > 
> > > I need this hint in the slave eth driver in case there is hsr-offloading
> > > available. 
> > 
> > This assumes that the slave device is somehow HSR aware. But standard
> > net_devices are not?
> 
> Yes, standard devices are not hsr-aware. It is just your average
> ethernet device that sends everything as-is.
> But you can have devices with HSR-offload capabilities such as
> NETIF_F_HW_HSR_FWD, NETIF_F_HW_HSR_DUP, NETIF_F_HW_HSR_TAG_INS,
> NETIF_F_HW_HSR_TAG_RM.
> 
> So if a device provides NETIF_F_HW_HSR_TAG_INS and it is enabled
> (via ethtool -K) then the HSR stack will pass regular skb and the device
> will prepend the HSR-header (and maintain the HSR-sequence number and so
> on) before putting it on the wire.
> Also, the HSR stack will send a skb on both slave interfaces which are
> regular ethernet devices. But with NETIF_F_HW_HSR_DUP enabled it will
> pass it only to the first slave and expect the underlying device to
> forward it also via the other slave interface.
> 
> So I need to pass skb which is can be sent by a dumb device but at the
> same time a device which does offload needs to be able to know when it
> should not do it.

Interesting. I was not aware of those offloads. 
> 
> > Isn't it HSR's ndo_start_xmit that selects which slave to forward to,
> > based on the information in this header?
> 
> Yes, this is correct. I needed to verify that the slave device with
> offload capabilities does not send the skb on both ports and add a
> HSR header while doing so. The non-offloading devices are not an issue,
> it sends packets as-is.
> I managed to rework it and it works without the skb-ext. I just track
> the HSR mode (HSR vs PRP) and then parse the packet for its type and
> decide what needs to be done.
> This works now. Once we settle on the header I can repost it.

Excellent.