[PATCH net] net: stmmac: selftests: Pass the IP proto mask in the TC selftest

[PATCH net] net: stmmac: selftests: Pass the IP proto mask in the TC selftest

[Maxime Chevallier]

The stmmac TC filtering rules have recently gained sanity checks to make
sure the passed keys and their respective masks are aligned with the HW
filtering abilities.

The stmmac selftests failed to pass the mask in the match data for L4
filtering tests, and are now failing consistently with -EINVAL :

$ ethtool -t eth1
[...]
23. L4 DA TCP Filtering          -22
24. L4 SA TCP Filtering          -22
25. L4 DA UDP Filtering          -22
26. L4 SA UDP Filtering          -22

Let's pass the ip_proto mask in the l4 filtering tests match data. Found
on imx8mp, which now have passing L4 tests :

$ ethtool -t eth1
[...]
23. L4 DA TCP Filtering          0
24. L4 SA TCP Filtering          0
25. L4 DA UDP Filtering          0
26. L4 SA UDP Filtering          0

Fixes: 5536d7c84363 ("net: stmmac: fix l3l4 filter rejecting unsupported offload requests")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 1df26c217f9a..e9be4d7cbb82 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -1489,6 +1489,8 @@ static int __stmmac_test_l4filt(struct stmmac_priv *priv, u32 dst, u32 src,
 	keys.bkey.ip_proto = udp ? IPPROTO_UDP : IPPROTO_TCP;
 	keys.key.src = htons(src);
 	keys.key.dst = htons(dst);
+	/* Match the full IP proto field */
+	masks.bmask.ip_proto = 0xff;
 	masks.mask.src = src_mask;
 	masks.mask.dst = dst_mask;
 
-- 
2.55.0

Re: [PATCH net] net: stmmac: selftests: Pass the IP proto mask in the TC selftest

[Andrew Lunn]

On Sat, Aug 22, 2026 at 05:32:43PM +0200, Maxime Chevallier wrote:
> The stmmac TC filtering rules have recently gained sanity checks to make
> sure the passed keys and their respective masks are aligned with the HW
> filtering abilities.
> 
> The stmmac selftests failed to pass the mask in the match data for L4
> filtering tests, and are now failing consistently with -EINVAL :
> 
> $ ethtool -t eth1
> [...]
> 23. L4 DA TCP Filtering          -22
> 24. L4 SA TCP Filtering          -22
> 25. L4 DA UDP Filtering          -22
> 26. L4 SA UDP Filtering          -22
> 
> Let's pass the ip_proto mask in the l4 filtering tests match data. Found
> on imx8mp, which now have passing L4 tests :
> 
> $ ethtool -t eth1
> [...]
> 23. L4 DA TCP Filtering          0
> 24. L4 SA TCP Filtering          0
> 25. L4 DA UDP Filtering          0
> 26. L4 SA UDP Filtering          0
> 
> Fixes: 5536d7c84363 ("net: stmmac: fix l3l4 filter rejecting unsupported offload requests")
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net] net: stmmac: selftests: Pass the IP proto mask in the TC selftest

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: stmmac: selftests: Pass the IP proto mask in the TC selftest

This adds an ip_proto mask to the match data used by the L4 filtering
selftests in __stmmac_test_l4filt(), so that the new mask sanity checks in
tc_add_basic_flow() accept the rule.  The commit message reports tests 23-26
going from -22 to 0 on imx8mp.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index 1df26c217f9a..e9be4d7cbb82 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> @@ -1489,6 +1489,8 @@ static int __stmmac_test_l4filt(struct stmmac_priv *priv, u32 dst, u32 src,
>  	keys.bkey.ip_proto = udp ? IPPROTO_UDP : IPPROTO_TCP;
>  	keys.key.src = htons(src);
>  	keys.key.dst = htons(dst);
> +	/* Match the full IP proto field */
> +	masks.bmask.ip_proto = 0xff;
>  	masks.mask.src = src_mask;
>  	masks.mask.dst = dst_mask;
>  

[Severity: Medium]

Is the n_proto half of the basic key still left uninitialized here?  In
__stmmac_test_l4filt() both structs are declared without an initializer:

	struct {
		struct flow_dissector_key_basic bkey;
		struct flow_dissector_key_ports key;
	} __aligned(BITS_PER_LONG / 8) keys;
	struct {
		struct flow_dissector_key_basic bmask;
		struct flow_dissector_key_ports mask;
	} __aligned(BITS_PER_LONG / 8) masks;

The function then only assigns keys.bkey.ip_proto, keys.key.src,
keys.key.dst, and now masks.bmask.ip_proto, masks.mask.src, masks.mask.dst.
Since dissector->offset[FLOW_DISSECTOR_KEY_BASIC] is 0,
flow_rule_match_basic() hands tc_add_basic_flow() pointers straight at
keys.bkey / masks.bmask, so match.mask->n_proto and match.key->n_proto are
whatever was on the stack.

tc_add_basic_flow() in stmmac_tc.c makes decisions on exactly those fields:

	if ((match.mask->n_proto && match.mask->n_proto != ETHER_TYPE_FULL_MASK) ||
	    (match.mask->ip_proto && match.mask->ip_proto != IP_PROTO_FULL_MASK)) {
		NL_SET_ERR_MSG_MOD(cls->common.extack,
				   "only full protocol mask is supported");
		return -EOPNOTSUPP;
	}

	/* Network proto is present in the key and is not IPv4 */
	if (match.mask->n_proto && match.key->n_proto != cpu_to_be16(ETH_P_IP)) {
		NL_SET_ERR_MSG_MOD(cls->common.extack,
				   "only IPv4 network protocol is supported");
		return -EOPNOTSUPP;
	}

So if the stack slot for masks.bmask.n_proto happens to be non-zero and not
0xffff, does the rule get rejected with -EOPNOTSUPP again?  With
CONFIG_INIT_STACK_ALL_PATTERN the slot holds the 0xAA pattern, which is
neither 0 nor ETHER_TYPE_FULL_MASK.

From there tc_add_flow_cls() falls back:

	ret = tc_add_flow(priv, cls);
	if (!ret)
		return ret;

	ret = tc_add_ethtype_flow(priv, cls);
	if (!ret)
		return ret;

	return tc_add_vlan_flow(priv, cls);

and with cls->classid == 0 those fallbacks return -EINVAL, which is the same
-22 that tests 23-26 report in the commit message.  Would zero-initializing
both structs ("= { }", as attr already does in the same function), and
setting keys.bkey.n_proto = htons(ETH_P_IP) together with
masks.bmask.n_proto = ETHER_TYPE_FULL_MASK given the injected frames are
IPv4, make the result independent of the stack contents?
-- 
This is an AI-generated review.

Re: [PATCH net] net: stmmac: selftests: Pass the IP proto mask in the TC selftest

[Maxime Chevallier]

Hi,

On 8/25/26 09:51, Paolo Abeni wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/

> Would zero-initializing
> both structs ("= { }", as attr already does in the same function), and
> setting keys.bkey.n_proto = htons(ETH_P_IP) together with
> masks.bmask.n_proto = ETHER_TYPE_FULL_MASK given the injected frames are
> IPv4, make the result independent of the stack contents?

Is surely doesn't hurt to explicitely initialize these fields, will do in
V2.

Maxime