Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support

[Maxime Chevallier]

Hi Luke,

On 7/3/26 08:41, Luke Howard wrote:
> mv88e6xxx switches can support embedding PTP timestamps directly
> in the frame, either as a trailer or at a configurable offset
> (typically the reserved bytes in the PTP header).
> 
> Add support for this on the 88E6341 and 88E6352 switches, being
> those on which I was able to verify this. Other switch chips may
> also work. The arrival timestamp offsets are relative to the PTP
> common header and will work for both L2 and L3 PTP packets; the
> respective headers are skipped by the number of bytes set in the
> ETJump and IPJump registers, which are initialized to sensible
> defaults and are VLAN tag-aware.
> 
> (Note: the 6352 datasheet incorrectly states that ETJump and
> IPJump are initialized to zero. They are initialized to 12 and
> 2 respectively; this is corrected in the 6341 data sheet.)
> 
> Signed-off-by: Luke Howard <lukeh@xxxxxxxx>

[...]

> +static bool parse_embedded_ts(unsigned int arr_ts_mode,
> +			      struct sk_buff *skb, u64 *ns)
> +{
> +	struct ptp_header *hdr;
> +
> +	*ns = 0;
> +
> +	/* APPEND means the switch appended the time stamp as a 4-byte trailer
> +	 * (not all switches support this). Any other non-zero value is the byte
> +	 * offset past the start of the PTP common header at which the switch
> +	 * overwrote the time stamp in place (e.g. the reserved header bytes).
> +	 */
> +	if (arr_ts_mode == MV88E6XXX_PTP_ARR_TS_MODE_APPEND && skb->len >= 4) {
> +		*ns = (u64)get_unaligned_be32(&skb->data[skb->len - 4]);

One thing is that you need to make sure you're dealing with a linear skb
when accessing the trailer, which can be done by calling skb_linearize(skb).

Then you should use skb_tail_pointer(skb) to get to the trailer.

You can take as examples the DSA taggers that pull the tag from the tail, such as
the tag_trailer [1], tag_ksz, tag_rtl8_4, etc. 

And finally you may have to use the chksum variants of the skb_trim such as
pskb_trim_rcsum(), like done in the above-mentionned taggers.

[1] : https://elixir.bootlin.com/linux/v7.1.2/source/net/dsa/tag_trailer.c#L28

Thanks,

Maxime