Re: [PATCH net-next v8 0/8] hsr: Add additional info to send/ receive skbs

[Daniel Machon <daniel.machon@microchip.com>]

Sebastian,

> 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 has an 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.
> The header is skipped so that the remaining stack sees the actual data
> and can 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 is not really a reason to use hsr interface.
> 
> HSR hardware offloading is optional. The driver needs to know if the
> operating mode is HSR or PRP. In PRP mode it needs to check the ether
> type and for ETH_P_1588 it must not perform any offloading.
> In HSR mode, for ether-type ETH_P_1588 there must be no offloading. If
> the ether-type is ETH_P_HSR there must be no offloading if the
> encapsulated protocol is ETH_P_1588.


Do any drivers need to be updated as part of this series?

A number of drivers offload HSR duplication using NETIF_F_HW_HSR_DUP. For a
directed PTP frame the HSR layer only sends it to the requested tx_port, but
that is undone further down the stack (e.g. dsa_xmit_port_mask() adds the HSR
partner port back, and icssg sends it as undirected), so e.g. a SYNC frame sent
to port B would also leave on port A. Or am I missing something?

> 
> 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.

We support PTP over HSR downstream on lan969x and lan9645x (with a different
uAPI). Maybe I can be of assistance testing this. Let me know.

> 
> ---
> v7…v8: https://patch.msgid.link/20260928-hsr_ptp-v7-0-d55d304d9a7e@linutronix.de
> - Use __u8 instead of uint8_t, add header file for in hsr_ptp.h
> - Add header file to if_hsr.h for struct ethhdr
> - Remove the inline keyword from hsr_ptp_test.c
> 
> v6…v7: https://patch.msgid.link/20260923-hsr_ptp-v6-0-6ea07b3fb8a8@linutronix.de
> - Change the logic in hsr_create_tagged_frame():
>   - Don't update ->csum_start (the cloning takes care of this)
>   - pskb_may_pull() does not need to include HSR_HLEN (it only copies
>     'movelen')
>   - Set network header to 'movelen' which takes VLAN into account
> - Drop also PTP packets which were received on HSR_PT_INTERLINK (not
>   just HSR A/B).
> - Move include HSR header to include/uapi/linux/hsr_ptp.h, so it is also
>   available in userland.
> - selftests:
>   - Add hsr_ptp.sh alphabetically ordered to the Makefile
>   - Use "$0" in hsr_ptp.sh
>   - Drop unnused arguments in open_socket()
> 
> v5…v6: https://lore.kernel.org/r/20260527-hsr_ptp-v5-0-158a7633eac0@linutronix.de
> - hsr_ptp_test: Let it wait up to 100ms for a packet. Otherwise it will
>   complain if the recevied packet is not already in socket.
> - Use TEST_GEN_PROGS instead TEST_GEN_FILES, the test can not run
>   without additional arguments.
> - Make the inline header mandatory for sending ETH_P_1588 packets via
>   the HSR device.
> - Use kfree_skb() instead kfree() for the skb.
> - Rephrase the commit message saying that skb_clone() shares
>   skb_shared_info and does not copy it.
> 
> v4…v5: https://lore.kernel.org/r/20260508-hsr_ptp-v4-1-aa19aa7c6a71@linutronix.de
> - Split the patch into smaller pieces
> - Added a test for the added inline header (which signals the port while
>   sending packets).
> - Replaced __pskb_copy() with skb_clone() + skb_cow_head() in
>   hsr_create_tagged_frame() to preserve timestamp request.
> 
> v3…v4: https://lore.kernel.org/r/20260429-hsr_ptp-v3-1-afbf8f200f48@linutronix.de
> - Removed skb extention. The information within HSR is passed via
>   struct hsr_frame_info. Driver with HSR-offloading capabilities need to
>   know the HSR mode (HSR or PRP) and parse the skb to decide what needs
>   to be done (whether to send on both ports and if adding a header is
>   needed).
> 
> 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.
> 
> ---
> Sebastian Andrzej Siewior (8):
>       hsr: Add header_ops::parse_protocol
>       hsr: Use skb_clone() while adding the HSR header
>       hsr: Add a magic header for sending PTP packets
>       hsr: Drop received PTP packets
>       hsr: Use the port and header information in hsr_forward_skb()
>       hsr: Assign a socket for cloned skbs
>       hsr: Move struct hsr_ethhdr to a global header
>       selftests: hsr: Add test for the inline PTP header on HSR
> 
>  MAINTAINERS                                    |   1 +
>  include/linux/if_hsr.h                         |   7 +
>  include/uapi/linux/hsr_ptp.h                   |  20 +
>  net/hsr/hsr_device.c                           |  67 +++-
>  net/hsr/hsr_forward.c                          |  78 +++-
>  net/hsr/hsr_forward.h                          |   3 +-
>  net/hsr/hsr_framereg.h                         |   2 +
>  net/hsr/hsr_main.h                             |   5 -
>  net/hsr/hsr_slave.c                            |  34 +-
>  tools/include/uapi/linux/hsr_ptp.h             |  20 +
>  tools/testing/selftests/net/hsr/.gitignore     |   1 +
>  tools/testing/selftests/net/hsr/Makefile       |   5 +
>  tools/testing/selftests/net/hsr/hsr_ptp.sh     | 109 ++++++
>  tools/testing/selftests/net/hsr/hsr_ptp_test.c | 482 +++++++++++++++++++++++++
>  14 files changed, 789 insertions(+), 45 deletions(-)
> ---
> base-commit: 5a956dde5526a634dca7ccad27c051ebcc306089
> change-id: 20260204-hsr_ptp-1f6380f1d35f
> 
> Best regards,
> --
> Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 

/Daniel