| Message ID | 20250225071348.509432-1-gal@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethtool: Don't check if RSS context exists in case of context 0 | expand |
On Tue, Feb 25, 2025 at 09:13:48AM +0200, Gal Pressman wrote: > Context 0 (default context) always exists, there is no need to check > whether it exists or not when adding a flow steering rule. > > The existing check fails when creating a flow steering rule for context > 0 as it is not stored in the rss_ctx xarray. > > For example: > $ ethtool --config-ntuple eth2 flow-type tcp4 dst-ip 194.237.147.23 dst-port 19983 context 0 loc 618 > rmgr: Cannot insert RX class rule: Invalid argument > Cannot insert classification rule > > Fixes: de7f7582dff2 ("net: ethtool: prevent flow steering to RSS contexts which don't exist") > Cc: Jakub Kicinski <kuba@kernel.org> > Reviewed-by: Tariq Toukan <tariqt@nvidia.com> > Signed-off-by: Gal Pressman <gal@nvidia.com> > --- > net/ethtool/ioctl.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > I was initially confused why tools/testing/selftests/drivers/net/hw/rss_ctx.py didn't catch this bug, but if I'm reading the tests there correctly it doesn't test this case AFAICT. Do you think it's worth adding a test for this case as a separate patch to net-next ? That said, since context 0 isn't tracked in rss_ctx, the fix makes sense to me: Reviewed-by: Joe Damato <jdamato@fastly.com>
On Tue, 25 Feb 2025 09:13:48 +0200 Gal Pressman wrote: > Context 0 (default context) always exists, there is no need to check > whether it exists or not when adding a flow steering rule. > > The existing check fails when creating a flow steering rule for context > 0 as it is not stored in the rss_ctx xarray. But what is the use case for redirecting to context 0?
On 26/02/2025 3:01, Jakub Kicinski wrote: > On Tue, 25 Feb 2025 09:13:48 +0200 Gal Pressman wrote: >> Context 0 (default context) always exists, there is no need to check >> whether it exists or not when adding a flow steering rule. >> >> The existing check fails when creating a flow steering rule for context >> 0 as it is not stored in the rss_ctx xarray. > > But what is the use case for redirecting to context 0? I can think of something like redirecting all TCP traffic to context 1, and then a specific TCP 5-tuple to the default context. Anyway, it used to work.
diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 98b7dcea207a..0be7a622dddf 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -998,7 +998,8 @@ static noinline_for_stack int ethtool_set_rxnfc(struct net_device *dev, ethtool_get_flow_spec_ring(info.fs.ring_cookie)) return -EINVAL; - if (!xa_load(&dev->ethtool->rss_ctx, info.rss_context)) + if (info.rss_context && + !xa_load(&dev->ethtool->rss_ctx, info.rss_context)) return -EINVAL; }
Context 0 (default context) always exists, there is no need to check whether it exists or not when adding a flow steering rule. The existing check fails when creating a flow steering rule for context 0 as it is not stored in the rss_ctx xarray. For example: $ ethtool --config-ntuple eth2 flow-type tcp4 dst-ip 194.237.147.23 dst-port 19983 context 0 loc 618 rmgr: Cannot insert RX class rule: Invalid argument Cannot insert classification rule Fixes: de7f7582dff2 ("net: ethtool: prevent flow steering to RSS contexts which don't exist") Cc: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Tariq Toukan <tariqt@nvidia.com> Signed-off-by: Gal Pressman <gal@nvidia.com> --- net/ethtool/ioctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)