| Message ID | cc3da0844083b0e301a33092a6299e4042b65221.1731499022.git.ecree.xilinx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: make RSS+RXNFC semantics more explicit | expand |
On Wed, Nov 13, 2024 at 12:13:09PM +0000, edward.cree@amd.com wrote: > > From: Edward Cree <ecree.xilinx@gmail.com> > > 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. > However, some drivers / hardware ignore the ring_cookie, and simply use > the indirection table entries as queue IDs directly. Thus, for drivers > which have not opted in by setting ethtool_ops.cap_rss_rxnfc_adds to > declare that they support the original (addition) semantics, reject in > ethtool_set_rxnfc any filter which combines FLOW_RSS and a nonzero ring. > (For a ring_cookie of zero, both behaviours are equivalent.) > Set the cap bit in sfc, as it is known to support this feature. > > Signed-off-by: Edward Cree <ecree.xilinx@gmail.com> Reviewed-by: Martin Habets <habetsm.xilinx@gmail.com> > --- > drivers/net/ethernet/sfc/ef100_ethtool.c | 1 + > drivers/net/ethernet/sfc/ethtool.c | 1 + > include/linux/ethtool.h | 4 ++++ > net/ethtool/ioctl.c | 5 +++++ > 4 files changed, 11 insertions(+) > > diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c > index 5c2551369812..6c3b74000d3b 100644 > --- a/drivers/net/ethernet/sfc/ef100_ethtool.c > +++ b/drivers/net/ethernet/sfc/ef100_ethtool.c > @@ -59,6 +59,7 @@ const struct ethtool_ops ef100_ethtool_ops = { > .get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size, > .get_rxfh_key_size = efx_ethtool_get_rxfh_key_size, > .rxfh_per_ctx_key = true, > + .cap_rss_rxnfc_adds = true, > .rxfh_priv_size = sizeof(struct efx_rss_context_priv), > .get_rxfh = efx_ethtool_get_rxfh, > .set_rxfh = efx_ethtool_set_rxfh, > diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c > index bb1930818beb..83d715544f7f 100644 > --- a/drivers/net/ethernet/sfc/ethtool.c > +++ b/drivers/net/ethernet/sfc/ethtool.c > @@ -263,6 +263,7 @@ const struct ethtool_ops efx_ethtool_ops = { > .get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size, > .get_rxfh_key_size = efx_ethtool_get_rxfh_key_size, > .rxfh_per_ctx_key = true, > + .cap_rss_rxnfc_adds = true, > .rxfh_priv_size = sizeof(struct efx_rss_context_priv), > .get_rxfh = efx_ethtool_get_rxfh, > .set_rxfh = efx_ethtool_set_rxfh, > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h > index 1199e308c8dd..299280c94d07 100644 > --- a/include/linux/ethtool.h > +++ b/include/linux/ethtool.h > @@ -734,6 +734,9 @@ struct kernel_ethtool_ts_info { > * @rxfh_per_ctx_key: device supports setting different RSS key for each > * additional context. Netlink API should report hfunc, key, and input_xfrm > * for every context, not just context 0. > + * @cap_rss_rxnfc_adds: device supports nonzero ring_cookie in filters with > + * %FLOW_RSS flag; the queue ID from the filter is added to the value from > + * the indirection table to determine the delivery queue. > * @rxfh_indir_space: max size of RSS indirection tables, if indirection table > * size as returned by @get_rxfh_indir_size may change during lifetime > * of the device. Leave as 0 if the table size is constant. > @@ -956,6 +959,7 @@ struct ethtool_ops { > u32 cap_rss_ctx_supported:1; > u32 cap_rss_sym_xor_supported:1; > u32 rxfh_per_ctx_key:1; > + u32 cap_rss_rxnfc_adds:1; > u32 rxfh_indir_space; > u16 rxfh_key_space; > u16 rxfh_priv_size; > diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c > index 7da94e26ced6..d86399bcf223 100644 > --- a/net/ethtool/ioctl.c > +++ b/net/ethtool/ioctl.c > @@ -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; > + > if (ops->get_rxfh) { > struct ethtool_rxfh_param rxfh = {}; >
Hi Edward, On 13/11/2024 14:13, edward.cree@amd.com wrote: > From: Edward Cree <ecree.xilinx@gmail.com> > > 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? > However, some drivers / hardware ignore the ring_cookie, and simply use > the indirection table entries as queue IDs directly. Thus, for drivers > which have not opted in by setting ethtool_ops.cap_rss_rxnfc_adds to > declare that they support the original (addition) semantics, reject in > ethtool_set_rxnfc any filter which combines FLOW_RSS and a nonzero ring. > (For a ring_cookie of zero, both behaviours are equivalent.) > Set the cap bit in sfc, as it is known to support this feature. > > Signed-off-by: Edward Cree <ecree.xilinx@gmail.com> > --- > diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c > index 7da94e26ced6..d86399bcf223 100644 > --- a/net/ethtool/ioctl.c > +++ b/net/ethtool/ioctl.c > @@ -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? > + > if (ops->get_rxfh) { > struct ethtool_rxfh_param rxfh = {}; > >
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. >> @@ -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? 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?
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: 1. Enable symmetric xor 2. Request hash on src only = Error as expected, however: 1. Request hash on src only 2. 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..
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: > 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. > > 1. Request hash on src only > 2. 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.
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: >> 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. > >> >> 1. Request hash on src only >> 2. 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?
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. 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)`.
On 25/11/2024 14:10, Gal Pressman wrote: > Thanks, I did not know it works that way, is it actually documented > anywhere? Yes; the struct ethtool_rxnfc kerneldoc has: * For %ETHTOOL_SRXCLSRLINS, @fs specifies the rule to add or update. * @fs.@location either specifies the location to use or is a special * location value with %RX_CLS_LOC_SPECIAL flag set. On return, * @fs.@location is the actual rule location. If @fs.@flow_type * includes the %FLOW_RSS flag, @rss_context is the RSS context ID to * use for flow spreading traffic which matches this rule. The value * from the rxfh indirection table will be added to @fs.@ring_cookie * to choose which ring to deliver to. I am also preparing a patch to add this info to the ethtool man page.
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.
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.
diff --git a/drivers/net/ethernet/sfc/ef100_ethtool.c b/drivers/net/ethernet/sfc/ef100_ethtool.c index 5c2551369812..6c3b74000d3b 100644 --- a/drivers/net/ethernet/sfc/ef100_ethtool.c +++ b/drivers/net/ethernet/sfc/ef100_ethtool.c @@ -59,6 +59,7 @@ const struct ethtool_ops ef100_ethtool_ops = { .get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size, .get_rxfh_key_size = efx_ethtool_get_rxfh_key_size, .rxfh_per_ctx_key = true, + .cap_rss_rxnfc_adds = true, .rxfh_priv_size = sizeof(struct efx_rss_context_priv), .get_rxfh = efx_ethtool_get_rxfh, .set_rxfh = efx_ethtool_set_rxfh, diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c index bb1930818beb..83d715544f7f 100644 --- a/drivers/net/ethernet/sfc/ethtool.c +++ b/drivers/net/ethernet/sfc/ethtool.c @@ -263,6 +263,7 @@ const struct ethtool_ops efx_ethtool_ops = { .get_rxfh_indir_size = efx_ethtool_get_rxfh_indir_size, .get_rxfh_key_size = efx_ethtool_get_rxfh_key_size, .rxfh_per_ctx_key = true, + .cap_rss_rxnfc_adds = true, .rxfh_priv_size = sizeof(struct efx_rss_context_priv), .get_rxfh = efx_ethtool_get_rxfh, .set_rxfh = efx_ethtool_set_rxfh, diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h index 1199e308c8dd..299280c94d07 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h @@ -734,6 +734,9 @@ struct kernel_ethtool_ts_info { * @rxfh_per_ctx_key: device supports setting different RSS key for each * additional context. Netlink API should report hfunc, key, and input_xfrm * for every context, not just context 0. + * @cap_rss_rxnfc_adds: device supports nonzero ring_cookie in filters with + * %FLOW_RSS flag; the queue ID from the filter is added to the value from + * the indirection table to determine the delivery queue. * @rxfh_indir_space: max size of RSS indirection tables, if indirection table * size as returned by @get_rxfh_indir_size may change during lifetime * of the device. Leave as 0 if the table size is constant. @@ -956,6 +959,7 @@ struct ethtool_ops { u32 cap_rss_ctx_supported:1; u32 cap_rss_sym_xor_supported:1; u32 rxfh_per_ctx_key:1; + u32 cap_rss_rxnfc_adds:1; u32 rxfh_indir_space; u16 rxfh_key_space; u16 rxfh_priv_size; diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 7da94e26ced6..d86399bcf223 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -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; + if (ops->get_rxfh) { struct ethtool_rxfh_param rxfh = {};