Re: [PATCH v7 net-next 8/9] net: dsa: netc: add PTP two-step timestamping support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] net: dsa: netc: incomplete IPFT_FAF_L4_CODE mask in
  netc_build_ptp_ipft_keye()
--

commit 3f2860d50cc9cac6dd4199f17906d6ee10ebd675
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: add PTP two-step timestamping support

This commit adds RX hardware timestamping via ingress port filter table
(IPFT) rules and two-step TX hardware timestamping support to the NXP NETC
DSA switch driver and tagger.

> diff --git a/drivers/net/dsa/netc/netc_ptp.c b/drivers/net/dsa/netc/netc_ptp.c
> new file mode 100644
> index 0000000000000..0bcc01ebf01de
> --- /dev/null
> +++ b/drivers/net/dsa/netc/netc_ptp.c

[ ... ]

> +static int netc_build_ptp_ipft_keye(struct ipft_keye_data *keye, int port,
> +				    enum netc_ptp_type type)
> +{
> +	u16 src_port, frm_attr_flags;
> +
> +	keye->precedence = cpu_to_le16(NETC_IPFT_PTP_PRECEDENCE);
> +	src_port = FIELD_PREP(IPFT_SRC_PORT, port);
> +	src_port |= IPFT_SRC_PORT_MASK;
> +	keye->src_port = cpu_to_le16(src_port);
> +
> +	switch (type) {
> +	case NETC_PTP_L2:
> +		keye->ethertype = htons(ETH_P_1588);
> +		keye->ethertype_mask = htons(0xffff);
> +		break;
> +	case NETC_PTP_L4_IPV4_EVENT:
> +	case NETC_PTP_L4_IPV4_GENERAL:
> +	case NETC_PTP_L4_IPV6_EVENT:
> +	case NETC_PTP_L4_IPV6_GENERAL:
> +		frm_attr_flags = IPFT_FAF_IP_HDR | FIELD_PREP(IPFT_FAF_L4_CODE,
> +				 IPFT_FAF_UDP_HDR);
> +		if (type == NETC_PTP_L4_IPV6_EVENT ||
> +		    type == NETC_PTP_L4_IPV6_GENERAL)
> +			frm_attr_flags |= IPFT_FAF_IP_VER6;
> +
> +		keye->frm_attr_flags = cpu_to_le16(frm_attr_flags);
> +
> +		/* Set IP version bit in flags_mask to match IPv4 or IPv6
> +		 * packets
> +		 */
> +		frm_attr_flags |= IPFT_FAF_IP_VER6;
> +		keye->frm_attr_flags_mask = cpu_to_le16(frm_attr_flags);

[Severity: Low]
Does reusing frm_attr_flags for keye->frm_attr_flags_mask leave the lower
bit of IPFT_FAF_L4_CODE unmasked?

In include/linux/fsl/ntmp.h, IPFT_FAF_L4_CODE is GENMASK(11, 10) and
IPFT_FAF_UDP_HDR is 2, so FIELD_PREP(IPFT_FAF_L4_CODE, IPFT_FAF_UDP_HDR)
only sets bit 11.  Without ORing IPFT_FAF_L4_CODE into frm_attr_flags_mask,
bit 10 is left wildcarded in the frame attribute flags check (matching both
IPFT_FAF_UDP_HDR and IPFT_FAF_SCTP_HDR, though keye->ip_protocol still
restricts the rule to IPPROTO_UDP).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111062753.1397582-1-wei.fang@oss.nxp.com?part=8