Re: [PATCH RFC net-next v2 2/2] af_packet: Add port specific handling for HSR

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-03-16 16:12:41 [-0400], Willem de Bruijn wrote:
> > The suggested changes to af_packet, that remains a hard no?
> 
> Yes. Packet sockets are largely superseded by XDP and AF_XDP when it
> comes to new functionality. And were never open for such protocol
> specific logic.

ptp4l uses AF_PACKET and while I am not against AF_XDP I am not sure if
it helps. I don't have any protocol stack so extending SOL_PACKET was
the only option left.

> Protocol independent extensions, such as reading skb->mark as part of
> extended auxdata, could be up for debate. But again, we already have
> XDP and AF_XDP which allow passing arbitrary metadata to/from
> userspace. That is preferable over adding new structs to the ABI.

Is this struct xsk_tx_metadata and something similar for RX? If so, I
would require HSR fields for both.
My understanding is that you bind XDP to device+queue. I would require
four sockets (generic+event for both slaves) so this does not work,
right? Also I would need to extend HSR stack so support XDP in a similar
fashion as network driver do?

> If you only want to attach to a single (hsr0) device, I still think
> passing the data inline might work. Either by overriding existing
> fields such as a MAC addr (a hack for sure) or something in the HSR
> header (it as the direction? or by inserting a custom header (or
> trailer) akin to virtio_net_hdr for tun/tap. But custom to your
> workload.

But here you have PACKET_VNET_HDR_SZ to configure virtio_net_hdr. This
is kind of what I ask for with PACKET_HSR_INFO + PACKET_HSR_BIND_PORT
(and I added static branches so nobody has to suffer with CONFIG_HSR
enabled but no HSR enabled socket)..

The diff below would be what means to bypass AF_PACKET entirely and use
eth0/ eth1 to send via SLAVE_A/ B and hsr0 with SO_MARK to send with
system's HSR header but only on one of the two ports.
With HSR-offloading enabled we need to attach the SO_MARK hints also on
eth0/ eth1 devices, too. Otherwise the offloading part will send the
packet on both ports and attach a header (as designed).

Now I have now 8 sockets in userland instead of 4.
I added icssg to show the offloading case:

diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index 79df641b4fb97..a2e50cae01686 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -909,13 +909,17 @@ enum netdev_tx icssg_ndo_start_xmit(struct sk_buff *skb, struct net_device *ndev
 	 */
 	dst_tag_id = emac->port_id | (q_idx << 8);
 
-	if (prueth->is_hsr_offload_mode &&
-	    (ndev->features & NETIF_F_HW_HSR_DUP))
-		dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
+	if (prueth->is_hsr_offload_mode) {
+		bool has_header;
+		bool has_port;
 
-	if (prueth->is_hsr_offload_mode &&
-	    (ndev->features & NETIF_F_HW_HSR_TAG_INS))
-		epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
+		hsr_skb_get_header_port_mark(skb, &has_header, &has_port);
+		if (ndev->features & NETIF_F_HW_HSR_DUP && !has_port)
+			dst_tag_id = PRUETH_UNDIRECTED_PKT_DST_TAG;
+
+		if (ndev->features & NETIF_F_HW_HSR_TAG_INS && !has_header)
+			epib[1] |= PRUETH_UNDIRECTED_PKT_TAG_INS;
+	}
 
 	cppi5_desc_set_tags_ids(&first_desc->hdr, 0, dst_tag_id);
 	k3_udma_glue_tx_dma_to_cppi5_addr(tx_chn->tx_chn, &buf_dma);
diff --git a/include/linux/if_hsr.h b/include/linux/if_hsr.h
index d7941fd880329..814b30f65f102 100644
--- a/include/linux/if_hsr.h
+++ b/include/linux/if_hsr.h
@@ -22,6 +22,11 @@ enum hsr_port_type {
 	HSR_PT_PORTS,	/* This must be the last item in the enum */
 };
 
+struct hsr_ptp_cb {
+	u8	ptp;
+	u8	port;
+};
+
 /* 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,
@@ -43,6 +48,57 @@ extern bool is_hsr_master(struct net_device *dev);
 extern int hsr_get_version(struct net_device *dev, enum hsr_version *ver);
 struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 				     enum hsr_port_type pt);
+
+static inline unsigned int hsr_skb_get_TX_port(struct sk_buff *skb)
+{
+	struct hsr_ptp_cb *cb;
+
+	if (!skb)
+		return 0;
+
+	cb = (struct hsr_ptp_cb*)skb->cb;
+	if (!cb->ptp)
+		return 0;
+	return cb->port;
+}
+
+static inline void hsr_skb_set_TX_port(struct sk_buff *skb,
+				       enum hsr_port_type port)
+{
+	struct hsr_ptp_cb *cb;
+
+	cb = (struct hsr_ptp_cb*)skb->cb;
+	cb->ptp = 1;
+	cb->port = port;
+}
+
+static inline void hsr_skb_non_ptp(struct sk_buff *skb)
+{
+	struct hsr_ptp_cb *cb;
+
+	cb = (struct hsr_ptp_cb*)skb->cb;
+	cb->ptp = 0;
+}
+
+static inline void hsr_skb_get_header_port_mark(struct sk_buff *skb, bool *has_header,
+						bool *has_port)
+{
+	u32 mark = skb->mark;
+	u32 val;
+
+	val = mark & 0b11;
+	if (val == 1 || val == 2)
+		*has_port = true;
+	else
+		*has_port = false;
+
+	val = mark & 0b100;
+	if (val)
+		*has_header = true;
+	else
+		*has_header = false;
+}
+
 #else
 static inline bool is_hsr_master(struct net_device *dev)
 {
@@ -59,6 +115,13 @@ static inline struct net_device *hsr_get_port_ndev(struct net_device *ndev,
 {
 	return ERR_PTR(-EINVAL);
 }
+
+static inline void hsr_skb_get_header_port_mark(struct sk_buff *skb, bool *has_header,
+						bool *has_port)
+{
+	*has_port = false;
+	*has_header = false;
+}
 #endif /* CONFIG_HSR */
 
 #endif /*_LINUX_IF_HSR_H_*/
diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 2c43776b7c4fb..e01d3a33ac941 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -229,6 +229,14 @@ static netdev_tx_t hsr_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 	master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
 	if (master) {
 		skb->dev = master->dev;
+
+		if (skb->mark == 1)
+			hsr_skb_set_TX_port(skb, HSR_PT_SLAVE_A);
+		else if (skb->mark == 2)
+			hsr_skb_set_TX_port(skb, HSR_PT_SLAVE_B);
+		else
+			hsr_skb_non_ptp(skb);
+
 		skb_reset_mac_header(skb);
 		skb_reset_mac_len(skb);
 		spin_lock_bh(&hsr->seqnr_lock);
@@ -355,6 +363,7 @@ static void send_hsr_supervision_frame(struct hsr_port *port,
 		return;
 	}
 
+	hsr_skb_non_ptp(skb);
 	hsr_forward_skb(skb, port);
 	spin_unlock_bh(&hsr->seqnr_lock);
 	return;
@@ -396,6 +405,7 @@ static void send_prp_supervision_frame(struct hsr_port *master,
 		return;
 	}
 
+	hsr_skb_non_ptp(skb);
 	hsr_forward_skb(skb, master);
 	spin_unlock_bh(&hsr->seqnr_lock);
 }
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index c67c0d35921de..d0d6a8342bb33 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -12,6 +12,7 @@
 #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"
 
@@ -333,7 +334,10 @@ struct sk_buff *hsr_create_tagged_frame(struct hsr_frame_info *frame,
 		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);
+		skb = skb_clone(frame->skb_std, GFP_ATOMIC);
+		if (hsr_skb_get_TX_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 */
@@ -355,6 +359,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_get_TX_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
 	 */
@@ -377,12 +390,23 @@ struct sk_buff *prp_create_tagged_frame(struct hsr_frame_info *frame,
 		}
 		return skb_clone(frame->skb_prp, 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_get_TX_port(skb))
+			skb_set_owner_w(skb, frame->skb_std->sk);
+		return skb;
 	}
 
 	skb = skb_copy_expand(frame->skb_std, skb_headroom(frame->skb_std),
 			      skb_tailroom(frame->skb_std) + HSR_HLEN,
 			      GFP_ATOMIC);
+	if (hsr_skb_get_TX_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);
+	}
 	return prp_fill_rct(skb, frame, port);
 }
 
@@ -508,10 +532,13 @@ 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;
 	struct hsr_port *port;
 	struct sk_buff *skb;
 	bool sent = false;
 
+	req_tx_port = hsr_skb_get_TX_port(frame->skb_std);
+
 	hsr_for_each_port(frame->port_rcv->hsr, port) {
 		struct hsr_priv *hsr = port->hsr;
 		/* Don't send frame back the way it came */
@@ -532,6 +559,16 @@ static void hsr_forward_do(struct hsr_frame_info *frame)
 		if ((port->dev->features & NETIF_F_HW_HSR_DUP) && sent)
 			continue;
 
+		/* RX PTP packets have the received port recorded */
+		if (frame->skb_hsr)
+			skb = frame->skb_hsr;
+		else if (frame->skb_prp)
+			skb = frame->skb_prp;
+
+		/* PTP TX packets have an outgoing port specified */
+		if (req_tx_port != HSR_PT_NONE && req_tx_port != port->type)
+			continue;
+
 		/* Don't send frame over port where it has been sent before.
 		 * Also for SAN, this shouldn't be done.
 		 */
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index 464f683e016db..f4879d1ee8ab6 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
@@ -70,15 +69,36 @@ static rx_handler_result_t hsr_handle_frame(struct sk_buff **pskb)
 	/* Only the frames received over the interlink port will assign a
 	 * sequence number and require synchronisation vs other sender.
 	 */
+	hsr_skb_non_ptp(skb);
 	if (port->type == HSR_PT_INTERLINK) {
 		spin_lock_bh(&hsr->seqnr_lock);
 		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/ PRP
+		 * as-is. The latency introduced by forwarding renders
+		 * the time information useless.
+		 */
+		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 {
+			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:

Sebastian