Re: [PATCH net-next v2 5/5] net: dsa: motorcomm: Add TC prio support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new comment in yt921x_acl_rule_ext_parse_flow()
  (drivers/net/dsa/motorcomm/tc.c) says "map 1...U16_MAX to (ACL_ORD_NUM…

--

net: dsa: motorcomm: Add TC prio support

This commit maps TC flower priorities onto the hardware ACL order field.
TC priorities that are multiples of (U16_MAX + 1) / YT921X_ACL_ORD_NUM are
scaled down. 65535 maps to order 0, and every other value is rejected.

> diff --git a/drivers/net/dsa/motorcomm/tc.c b/drivers/net/dsa/motorcomm/tc.c
> index 95ed0f245b1fa..c5581e85208f3 100644
> --- a/drivers/net/dsa/motorcomm/tc.c
> +++ b/drivers/net/dsa/motorcomm/tc.c

[ ... ]

> @@ -1042,6 +1045,20 @@ yt921x_acl_rule_ext_parse_flow(struct yt921x_acl_rule_ext *ruleext, int port,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/* TC prio can't be 0 & TC lowest first -> HW highest first
> +	 * map 1...U16_MAX to (ACL_ORD_NUM - 1)...0
> +	 */

[Severity: Low]
This isn't a bug, but the comment doesn't match what the code below
does.

With scale = 65536 / 512 = 128, the only values accepted are:

  prio == k * 128 (k = 1..511)  ->  ord = 512 - k
  prio == 65535                 ->  ord = 0

The other values in 1...U16_MAX are rejected with -EOPNOTSUPP. That
includes prio 1, which is where the comment says the range starts.
Ord 511 comes from prio 128, not prio 1.

Could the comment describe the discrete mapping instead, in line with
the extack message "must be a multiple of %u or 65535"?

> +	if (prio == U16_MAX) {
> +		ord = 0;
> +	} else if (!(prio % scale)) {
> +		ord = YT921X_ACL_ORD_NUM - prio / scale;
> +	} else {
> +		NL_SET_ERR_MSG_FMT_MOD(extack,
> +				       "Invalid priority %u, must be a multiple of %u or 65535",
> +				       prio, scale);
> +		return -EOPNOTSUPP;
> +	}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001204851.2576101-1-mmyangfl%40gmail.com