On 2024-11-25 12:01 p.m., Ahmed Zaki wrote:
On 2024-11-25 7:42 a.m., Edward Cree wrote:
On 25/11/2024 14:20, Ahmed Zaki wrote:
On 2024-11-25 7:10 a.m., Gal Pressman wrote:
On 25/11/2024 15:21, Edward Cree wrote:
Also, the check below it, dealing with sym-xor, looks like it's only
relevant to ETHTOOL_SRXFH, since info.data is garbage for other
commands.
Ahmed, is my understanding correct there?
Speaking of the below check, the sanity check depends on the order of
operations, for example:
1. Enable symmetric xor
2. Request hash on src only
= Error as expected, however:
Correct. The check below is to make sure that no ntuple that does not
cover symmetric fields is added if symm-xor is enabled.
But symm-xor is about hashing, and is only relevant to traffic being
directed by RSS. The user should still be allowed to, and the NIC
should be able to handle, setting an ntuple filter (SRXCLSRLINS)
that is asymmetric, to override the symmetric hashing for selected
traffic.
I agree, and in its first version, the sym-xor series was setting sym-
xor per ntuple, not per netdev. So the NIC can support different RSS
functions for different filters. Unfortunately, this was then changed to
be per-netdev during review. At that point, these checks were added in
nxfc path.
symm-xor should only constrain RXFH settings. And in fact even if
you wanted to block asymm ntuple filters, the current code does not
do that, since the info.data fields it looks at aren't populated for
ntuple filters (whose filter fields are defined by info.fs).
So the xfrm check should be under `if (info.cmd == ETHTOOL_SRXFH)`.
If it is not, then it is a bug. I will try to test later this week and
send a fix if needed.
Sorry, I misunderstood your original question. The check was actually
intended for "rx-flow-hash":
# ethtool -X eth0 xfrm symmetric-xor
# ethtool -N eth0 rx-flow-hash udp4 sdf
Cannot change RX network flow hashing options: Invalid argument
and the NICs that support symm-xor do not use RSS on ntuple filters. So
you are right, we should:
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -997,7 +997,7 @@ static noinline_for_stack int
ethtool_set_rxnfc(struct net_device *dev,
ethtool_get_flow_spec_ring(info.fs.ring_cookie))
return -EINVAL;
- if (ops->get_rxfh) {
+ if (info.cmd == ETHTOOL_SRXFH && ops->get_rxfh) {
struct ethtool_rxfh_param rxfh = {};
rc = ops->get_rxfh(dev, &rxfh);
Let me know if you want me to send this.
Thanks.
