On 25/11/2024 16:20, Ahmed Zaki wrote:
On 2024-11-25 7:10 a.m., Gal Pressman wrote:
On 25/11/2024 15:21, Edward Cree wrote:
On 25/11/2024 07:11, Gal Pressman wrote:
On 13/11/2024 14:13, edward.cree@amd.com wrote:
Ethtool ntuple filters with FLOW_RSS were originally defined as adding the base queue ID (ring_cookie) to the value from the indirection table, so that the same table could distribute over more than one set of queues when used by different filters.
TBH, I'm not sure I understand the difference? Perhaps you can share an example?
Something like this:
ethtool -X $intf context new equal 2 # creates context ID 1, table filled with 0s and 1s ethtool -N $intf <match fields...> context 1 # filter distributes traffic to queues 0 and 1 ethtool -N $intf <match fields...> context 1 action 2 # filter distributes traffic to queues 2 and 3
See the selftest in patch 4 for a concrete example of this. Some NICs were apparently sending the traffic from both filters to queues 0 and 1, and ignoring the 'action 2' on the second filter.
Thanks, I did not know it works that way, is it actually documented anywhere?
@@ -992,6 +992,11 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, if (rc) return rc; + /* Nonzero ring with RSS only makes sense if NIC adds them together */ + if (info.flow_type & FLOW_RSS && !ops->cap_rss_rxnfc_adds && + ethtool_get_flow_spec_ring(info.fs.ring_cookie)) + return -EINVAL;
I believe this check shouldn't happen when we do ETHTOOL_SRXCLSRLDEL as flow_type is garbage, WDYT?
Agreed; this check should only apply to ETHTOOL_SRXCLSRLINS. Do you want to send the fix or shall I?
I will do it.
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:
- Enable symmetric xor
- 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.
- Request hash on src only
- Enable symmetric xor
= Success :(.
I've been thinking of improving the situation, but that requires iterating over all flow types on symmetric xor enablement and that feels quite bad..
and delete/disable filters? may be just a warning to the user that some filters are not symmetric.
I think the right thing to do in that case is fail the symmetric xor enablement, but do we really want to query the driver for all flow types and check if an asymmetric filter exists?