On Wed, Jun 29, 2022 at 03:28:54PM +0300, Magnus Armholt wrote:
> Strip the IEC62439-3 PRP trailer if it is present
> to support PTP over PRP.
> The implementation is very pedantic about
> trailing bytes and will indicate bad message if the
> PRP trailer bytes are present when parsing the PTP message.

> +static int has_prp_trailer(unsigned char *ptr, int cnt)
> +{
> +     if (cnt < PRP_TRAILER_LEN)
> +             return -1;
> +     //verify suffix first since it is the best identifier

Please use C-style comments.

> +     unsigned short suffix_id = ntohs(*(unsigned short*)(ptr + (cnt - 2)));

This seems to be parsing the frame from the end. Couldn't that
randomly match a non-PRP frame, even if you consider the length check
below?

To me it would make more sense to start after the PTP message
according to the messageLength field.

> +     if (suffix_id != ETH_P_PRP)
> +             return -1;
> +
> +     // size should also be verified
> +     unsigned short lane_size_field = ntohs(*(unsigned short*)(ptr + (cnt - 
> 4)));

Declarations should be at the beginning of the function.

> +     // size is lower 12 bits
> +     unsigned short lsdu_size = (lane_size_field & 0x0FFF);
> +     if (lsdu_size == cnt)
> +             return 0;
> +
> +     return -1;
> +}
> +

-- 
Miroslav Lichvar



_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to