| Message ID | 890cd515345f7c1ed6fba4bf0e43c53b34ccefaa.1731094323.git.dxu@dxuuu.xyz (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Michal Kubecek |
| Headers | show |
| Series | [ethtool-next] rxclass: Make output for RSS context action explicit | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Nov 8, 2024, at 11:32 AM, Daniel Xu wrote: > Currently, if the action for an ntuple rule is to redirect to an RSS > context, the RSS context is printed as an attribute. At the same time, > a wrong action is printed. For example: > > # ethtool -X eth0 hfunc toeplitz context new start 24 equal 8 > New RSS context is 1 > > # ethtool -N eth0 flow-type ip6 dst-ip $IP6 context 1 > Added rule with ID 0 > > # ethtool -n eth0 rule 0 > Filter: 0 > Rule Type: Raw IPv6 > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > Dest IP addr: <redacted> mask: :: > Traffic Class: 0x0 mask: 0xff > Protocol: 0 mask: 0xff > L4 bytes: 0x0 mask: 0xffffffff > RSS Context ID: 1 > Action: Direct to queue 0 > > This is wrong and misleading. Fix by treating RSS context as a explicit > action. The new output looks like this: > > # ./ethtool -n eth0 rule 0 > Filter: 0 > Rule Type: Raw IPv6 > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > Dest IP addr: <redacted> mask: :: > Traffic Class: 0x0 mask: 0xff > Protocol: 0 mask: 0xff > L4 bytes: 0x0 mask: 0xffffffff > Action: Direct to RSS context id 1 > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > --- > rxclass.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/rxclass.c b/rxclass.c > index f17e3a5..80d6419 100644 > --- a/rxclass.c > +++ b/rxclass.c > @@ -248,13 +248,12 @@ static void rxclass_print_nfc_rule(struct > ethtool_rx_flow_spec *fsp, > > rxclass_print_nfc_spec_ext(fsp); > > - if (fsp->flow_type & FLOW_RSS) > - fprintf(stdout, "\tRSS Context ID: %u\n", rss_context); > - > if (fsp->ring_cookie == RX_CLS_FLOW_DISC) { > fprintf(stdout, "\tAction: Drop\n"); > } else if (fsp->ring_cookie == RX_CLS_FLOW_WAKE) { > fprintf(stdout, "\tAction: Wake-on-LAN\n"); > + } else if (fsp->flow_type & FLOW_RSS) > + fprintf(stdout, "\tRSS context id %u\n", rss_context); Gah, missed { at end of line while transcribing patches between different machines... I can resend if y'all want. > } else { > u64 vf = ethtool_get_flow_spec_ring_vf(fsp->ring_cookie); > u64 queue = ethtool_get_flow_spec_ring(fsp->ring_cookie); > -- > 2.46.0
On 08/11/2024 19:32, Daniel Xu wrote: > Currently, if the action for an ntuple rule is to redirect to an RSS > context, the RSS context is printed as an attribute. At the same time, > a wrong action is printed. For example: > > # ethtool -X eth0 hfunc toeplitz context new start 24 equal 8 > New RSS context is 1 > > # ethtool -N eth0 flow-type ip6 dst-ip $IP6 context 1 > Added rule with ID 0 > > # ethtool -n eth0 rule 0 > Filter: 0 > Rule Type: Raw IPv6 > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > Dest IP addr: <redacted> mask: :: > Traffic Class: 0x0 mask: 0xff > Protocol: 0 mask: 0xff > L4 bytes: 0x0 mask: 0xffffffff > RSS Context ID: 1 > Action: Direct to queue 0 > > This is wrong and misleading. Fix by treating RSS context as a explicit > action. The new output looks like this: > > # ./ethtool -n eth0 rule 0 > Filter: 0 > Rule Type: Raw IPv6 > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > Dest IP addr: <redacted> mask: :: > Traffic Class: 0x0 mask: 0xff > Protocol: 0 mask: 0xff > L4 bytes: 0x0 mask: 0xffffffff > Action: Direct to RSS context id 1 > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> I believe this patch is incorrect. My understanding is that on packet reception, the integer returned from the RSS indirection table is *added* to the queue number from the ntuple rule, so that for instance the same indirection table can be used for one rule distributing packets over queues 0-3 and for another rule distributing a different subset of packets over queues 4-7. I'm not sure if this behaviour is documented anywhere, and different NICs may have different interpretations, but this is how sfc ef10 behaves. -Ed
On Fri, Nov 08, 2024 at 07:56:41PM +0000, Edward Cree wrote: > On 08/11/2024 19:32, Daniel Xu wrote: > > Currently, if the action for an ntuple rule is to redirect to an RSS > > context, the RSS context is printed as an attribute. At the same time, > > a wrong action is printed. For example: > > > > # ethtool -X eth0 hfunc toeplitz context new start 24 equal 8 > > New RSS context is 1 > > > > # ethtool -N eth0 flow-type ip6 dst-ip $IP6 context 1 > > Added rule with ID 0 > > > > # ethtool -n eth0 rule 0 > > Filter: 0 > > Rule Type: Raw IPv6 > > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > > Dest IP addr: <redacted> mask: :: > > Traffic Class: 0x0 mask: 0xff > > Protocol: 0 mask: 0xff > > L4 bytes: 0x0 mask: 0xffffffff > > RSS Context ID: 1 > > Action: Direct to queue 0 > > > > This is wrong and misleading. Fix by treating RSS context as a explicit > > action. The new output looks like this: > > > > # ./ethtool -n eth0 rule 0 > > Filter: 0 > > Rule Type: Raw IPv6 > > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > > Dest IP addr: <redacted> mask: :: > > Traffic Class: 0x0 mask: 0xff > > Protocol: 0 mask: 0xff > > L4 bytes: 0x0 mask: 0xffffffff > > Action: Direct to RSS context id 1 > > > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > > I believe this patch is incorrect. My understanding is that on > packet reception, the integer returned from the RSS indirection > table is *added* to the queue number from the ntuple rule, so > that for instance the same indirection table can be used for one > rule distributing packets over queues 0-3 and for another rule > distributing a different subset of packets over queues 4-7. > I'm not sure if this behaviour is documented anywhere, and > different NICs may have different interpretations, but this is > how sfc ef10 behaves. I just wanted to chime in and say that my understanding has always been more aligned with Daniel's and I had also found the ethtool output confusing when directing flows that match a rule to a custom context. If Daniel's patch is wrong (I don't know enough to say if it is or not), would it be possible to have some alternate ethtool output that's less confusing? Or for this specific output to be outlined in the documentation somewhere?
On Fri, Nov 08, 2024 at 12:34:49PM -0800, Joe Damato wrote: > On Fri, Nov 08, 2024 at 07:56:41PM +0000, Edward Cree wrote: > > On 08/11/2024 19:32, Daniel Xu wrote: > > > Currently, if the action for an ntuple rule is to redirect to an RSS > > > context, the RSS context is printed as an attribute. At the same time, > > > a wrong action is printed. For example: > > > > > > # ethtool -X eth0 hfunc toeplitz context new start 24 equal 8 > > > New RSS context is 1 > > > > > > # ethtool -N eth0 flow-type ip6 dst-ip $IP6 context 1 > > > Added rule with ID 0 > > > > > > # ethtool -n eth0 rule 0 > > > Filter: 0 > > > Rule Type: Raw IPv6 > > > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > > > Dest IP addr: <redacted> mask: :: > > > Traffic Class: 0x0 mask: 0xff > > > Protocol: 0 mask: 0xff > > > L4 bytes: 0x0 mask: 0xffffffff > > > RSS Context ID: 1 > > > Action: Direct to queue 0 > > > > > > This is wrong and misleading. Fix by treating RSS context as a explicit > > > action. The new output looks like this: > > > > > > # ./ethtool -n eth0 rule 0 > > > Filter: 0 > > > Rule Type: Raw IPv6 > > > Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff > > > Dest IP addr: <redacted> mask: :: > > > Traffic Class: 0x0 mask: 0xff > > > Protocol: 0 mask: 0xff > > > L4 bytes: 0x0 mask: 0xffffffff > > > Action: Direct to RSS context id 1 > > > > > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> > > > > I believe this patch is incorrect. My understanding is that on > > packet reception, the integer returned from the RSS indirection > > table is *added* to the queue number from the ntuple rule, so > > that for instance the same indirection table can be used for one > > rule distributing packets over queues 0-3 and for another rule > > distributing a different subset of packets over queues 4-7. > > I'm not sure if this behaviour is documented anywhere, and > > different NICs may have different interpretations, but this is > > how sfc ef10 behaves. > > I just wanted to chime in and say that my understanding has always > been more aligned with Daniel's and I had also found the ethtool > output confusing when directing flows that match a rule to a custom > context. > > If Daniel's patch is wrong (I don't know enough to say if it is or > not), would it be possible to have some alternate ethtool output > that's less confusing? Or for this specific output to be outlined in > the documentation somewhere? Sorry for the quick follow up, but I just tested this and I do think there is an issue with ethtool's output. Here's an example from my system where I create 18 queues and a custom RSS context to send flows to queues 16 and 17 only: $ ethtool --version ethtool version 6.7 $ sudo ethtool -L eth2 combined 18 $ sudo ethtool -X eth2 weight 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 1 context new New RSS context is 1 $ sudo ethtool -U eth2 flow-type tcp4 dst-port 11211 context 1 Added rule with ID 1023 $ sudo ethtool -n eth2 rule 1023 Filter: 1023 Rule Type: TCP over IPv4 Src IP addr: 0.0.0.0 mask: 255.255.255.255 Dest IP addr: 0.0.0.0 mask: 255.255.255.255 TOS: 0x0 mask: 0xff Src port: 0 mask: 0xffff Dest port: 11211 mask: 0x0 RSS Context ID: 1 Action: Direct to queue 0 I don't understand why this would say "Direct to queue 0" when the weights specifically disallow this for context 1.
On 08/11/2024 20:34, Joe Damato wrote: > On Fri, Nov 08, 2024 at 07:56:41PM +0000, Edward Cree wrote: >> I believe this patch is incorrect. My understanding is that on >> packet reception, the integer returned from the RSS indirection >> table is *added* to the queue number from the ntuple rule, so >> that for instance the same indirection table can be used for one >> rule distributing packets over queues 0-3 and for another rule >> distributing a different subset of packets over queues 4-7. >> I'm not sure if this behaviour is documented anywhere, and >> different NICs may have different interpretations, but this is >> how sfc ef10 behaves. I've looked up the history, and my original commit[1] adding RSS + ntuple specified this addition behaviour in both the patch description and the ethtool uapi header comments. The kerneldoc comment for struct ethtool_rxnfc still has this language: * 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. The ethtool man page, however, does not document this. > I just wanted to chime in and say that my understanding has always > been more aligned with Daniel's and I had also found the ethtool > output confusing when directing flows that match a rule to a custom > context. > > If Daniel's patch is wrong (I don't know enough to say if it is or > not), would it be possible to have some alternate ethtool output > that's less confusing? Or for this specific output to be outlined in > the documentation somewhere? I think sensible output would be to keep Daniel's "Action: Direct to RSS context id %u", but also print something like "Queue base offset: %u" with the ring index that was previously printed as the Action. If the base offset is zero its output could possibly be suppressed. And we should update the ethtool man page to describe the adding behaviour, and audit device drivers to ensure that any that don't support it reject RSS filters with nonzero ring_cookie, as specified in [1]. Does this sound reasonable? -ed [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=84a1d9c4820080bebcbd413a845076dcb62f45fa
Hi Ed, Joe, Thanks for looking at this so quickly. On Fri, Nov 08, 2024 at 09:13:50PM GMT, Edward Cree wrote: > On 08/11/2024 20:34, Joe Damato wrote: > > On Fri, Nov 08, 2024 at 07:56:41PM +0000, Edward Cree wrote: > >> I believe this patch is incorrect. My understanding is that on > >> packet reception, the integer returned from the RSS indirection > >> table is *added* to the queue number from the ntuple rule, so > >> that for instance the same indirection table can be used for one > >> rule distributing packets over queues 0-3 and for another rule > >> distributing a different subset of packets over queues 4-7. > >> I'm not sure if this behaviour is documented anywhere, and > >> different NICs may have different interpretations, but this is > >> how sfc ef10 behaves. > > I've looked up the history, and my original commit[1] adding RSS + > ntuple specified this addition behaviour in both the patch > description and the ethtool uapi header comments. The kerneldoc > comment for struct ethtool_rxnfc still has this language: > * 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. > The ethtool man page, however, does not document this. FWIW, bnxt on 6.9-ish is probably non-compliant (assuming this is the correct usage of the API): # ethtool -N eth0 flow-type ip6 dst-ip ::1 context 1 queue 5 Added rule with ID 0 # ethtool -n eth0 32 RX rings available Total 1 rules Filter: 0 Rule Type: Raw IPv6 Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff Dest IP addr: ::1 mask: :: Traffic Class: 0x0 mask: 0xff Protocol: 0 mask: 0xff L4 bytes: 0x0 mask: 0xffffffff RSS Context ID: 1 Action: Direct to queue 0 Note the "Direct to queue 0" even though I specified queue 5. I'm running a new batch of tests on 6.11 next week so I'll update here if it magically got fixed. > > > I just wanted to chime in and say that my understanding has always > > been more aligned with Daniel's and I had also found the ethtool > > output confusing when directing flows that match a rule to a custom > > context. > > > > If Daniel's patch is wrong (I don't know enough to say if it is or > > not), would it be possible to have some alternate ethtool output > > that's less confusing? Or for this specific output to be outlined in > > the documentation somewhere? > > I think sensible output would be to keep Daniel's "Action: Direct to > RSS context id %u", but also print something like "Queue base offset: > %u" with the ring index that was previously printed as the Action. > If the base offset is zero its output could possibly be suppressed. > And we should update the ethtool man page to describe the adding > behaviour, and audit device drivers to ensure that any that don't > support it reject RSS filters with nonzero ring_cookie, as specified > in [1]. > Does this sound reasonable? That sounds good to me. I'll send out a v2 for the ethtool changes. I'm probably not qualified enough to do an audit. But since I've been poking around the bxnt driver the past week I'll give it a look and see if I can convince myself of anything. Thanks, Daniel
On Fri, 8 Nov 2024 21:13:50 +0000 Edward Cree wrote: > I think sensible output would be to keep Daniel's "Action: Direct to > RSS context id %u", but also print something like "Queue base offset: > %u" with the ring index that was previously printed as the Action. > If the base offset is zero its output could possibly be suppressed. > And we should update the ethtool man page to describe the adding > behaviour, and audit device drivers to ensure that any that don't > support it reject RSS filters with nonzero ring_cookie, as specified > in [1]. > Does this sound reasonable? I'd suggest we merge Daniel's patch (almost) as is, and you can (re)establish the behavior sfc wants but you owe us: - fixes for helpers used in "is the queue in use" checks like ethtool_get_max_rss_ctx_channel() - "opt in" flag for drivers which actually support this rather than silently ignoring ring_cookie if rss ctx is set - selftest :(
On 09/11/2024 17:42, Jakub Kicinski wrote: > I'd suggest we merge Daniel's patch (almost) as is, and you can > (re)establish the behavior sfc wants but you owe us: > - fixes for helpers used in "is the queue in use" checks like > ethtool_get_max_rss_ctx_channel() > - "opt in" flag for drivers which actually support this rather > than silently ignoring ring_cookie if rss ctx is set > - selftest Sure, I'll get to work on those. But I don't think Daniel's patch should be merged; the old output is confusing or misleading, but the new output is incorrect (when run against a current kernel and sfc, or a future fixed kernel and any driver that opts in to allow nonzero ring_cookie). If ring_cookie is nonzero then it *must* be printed, unless ethtool has some way to *know* it's ignored. Regressions are worse than existing bugs, after all.
On Mon, 11 Nov 2024 10:47:47 +0000 Edward Cree wrote: > On 09/11/2024 17:42, Jakub Kicinski wrote: > > I'd suggest we merge Daniel's patch (almost) as is, and you can > > (re)establish the behavior sfc wants but you owe us: > > - fixes for helpers used in "is the queue in use" checks like > > ethtool_get_max_rss_ctx_channel() > > - "opt in" flag for drivers which actually support this rather > > than silently ignoring ring_cookie if rss ctx is set > > - selftest > > Sure, I'll get to work on those. > But I don't think Daniel's patch should be merged; the old output > is confusing or misleading, but the new output is incorrect (when > run against a current kernel and sfc, or a future fixed kernel and > any driver that opts in to allow nonzero ring_cookie). > If ring_cookie is nonzero then it *must* be printed, unless ethtool > has some way to *know* it's ignored. Regressions are worse than > existing bugs, after all. If you promise sending patches for the kernel side validation soon - printing when non-zero makes sense :)
On 09/11/2024 17:42, Jakub Kicinski wrote: > - fixes for helpers used in "is the queue in use" checks like > ethtool_get_max_rss_ctx_channel() If there's an RSS context that names a queue, but no rxnfc filters currently target that context, should the queue be considered "in use" or not? (Currently it is.) I'm trying to figure out how much of ethtool_get_max_rss_ctx_channel can be subsumed by the logic I'll need to add to ethtool_get_max_rxnfc_channel; if we don't count unused contexts as 'using' their queues then ethtool_get_max_rss_ctx_channel() can almost entirely disappear.
On Tue, 12 Nov 2024 09:24:07 +0000 Edward Cree wrote: > On 09/11/2024 17:42, Jakub Kicinski wrote: > > - fixes for helpers used in "is the queue in use" checks like > > ethtool_get_max_rss_ctx_channel() > > If there's an RSS context that names a queue, but no rxnfc filters > currently target that context, should the queue be considered "in > use" or not? (Currently it is.) > I'm trying to figure out how much of ethtool_get_max_rss_ctx_channel > can be subsumed by the logic I'll need to add to > ethtool_get_max_rxnfc_channel; if we don't count unused contexts as > 'using' their queues then ethtool_get_max_rss_ctx_channel() can > almost entirely disappear. Hm, interesting idea... Practically speaking I think it introduces complexity and I'm not sure anyone will actually benefit (IOW why would anyone want to keep / create context for inactive queues?). Drivers may not expect to have contexts pointing to disabled queues. My gut feeling is that we should just leave a comment for posterity somewhere in the code but continue to validate both based on rules and based on "direct" context membership.
On 12/11/2024 15:24, Jakub Kicinski wrote: > Hm, interesting idea... > Practically speaking I think it introduces complexity and I'm not sure > anyone will actually benefit (IOW why would anyone want to keep / > create context for inactive queues?). Conceivably to save re-configuring them next time they increase the queues again? But I suppose anyone doing that kind of complicated demand-flexible tuning will be using some kind of userland software that can automate that. Anyway I don't have a dog in this fight as sfc doesn't support ethtool set-channels. (Which will make it difficult for me to test this; had I better extend netdevsim to support RSS & rxnfc?) > My gut feeling is that we should just leave a comment for posterity > somewhere in the code but continue to validate both based on rules > and based on "direct" context membership. Will do.
On Wed, 13 Nov 2024 03:30:05 +0000 Edward Cree wrote: > On 12/11/2024 15:24, Jakub Kicinski wrote: > > Hm, interesting idea... > > Practically speaking I think it introduces complexity and I'm not sure > > anyone will actually benefit (IOW why would anyone want to keep / > > create context for inactive queues?). > > Conceivably to save re-configuring them next time they increase the > queues again? But I suppose anyone doing that kind of complicated > demand-flexible tuning will be using some kind of userland software > that can automate that. > Anyway I don't have a dog in this fight as sfc doesn't support ethtool > set-channels. (Which will make it difficult for me to test this; had > I better extend netdevsim to support RSS & rxnfc?) Good question on the netdevsim. Adding the callbacks seems fine. But making it actually do RSS and nfc on packets to make the HW tests pass would be more of a lift. So I think you'd have to add a separate test under drivers/net/netdevsim for this. Is that your thinking?
On Wed, 13 Nov 2024 16:46:35 -0800 Jakub Kicinski wrote: > On Wed, 13 Nov 2024 03:30:05 +0000 Edward Cree wrote: > > On 12/11/2024 15:24, Jakub Kicinski wrote: > > > Hm, interesting idea... > > > Practically speaking I think it introduces complexity and I'm not sure > > > anyone will actually benefit (IOW why would anyone want to keep / > > > create context for inactive queues?). > > > > Conceivably to save re-configuring them next time they increase the > > queues again? But I suppose anyone doing that kind of complicated > > demand-flexible tuning will be using some kind of userland software > > that can automate that. > > Anyway I don't have a dog in this fight as sfc doesn't support ethtool > > set-channels. (Which will make it difficult for me to test this; had > > I better extend netdevsim to support RSS & rxnfc?) > > Good question on the netdevsim. Adding the callbacks seems fine. > But making it actually do RSS and nfc on packets to make the HW tests > pass would be more of a lift. So I think you'd have to add a separate > test under drivers/net/netdevsim for this. Is that your thinking? s/your/you're/
On 14/11/2024 00:46, Jakub Kicinski wrote: > Good question on the netdevsim. Adding the callbacks seems fine. > But making it actually do RSS and nfc on packets to make the HW tests > pass would be more of a lift. So I think you'd have to add a separate > test under drivers/net/netdevsim for this. Is that your thinking? Actually I was just planning to test it manually since the HW tests running on other devices should cover regression testing going forward, but I guess I could give the relevant tests from rss_ctx.py an option to skip _send_traffic_check(), and call them from the new netdevsim test. test_rss_queue_reconfigure() covers this bit of code; some of the others like test_rss_context_dump() might also be worth running on netdevsim.
diff --git a/rxclass.c b/rxclass.c index f17e3a5..80d6419 100644 --- a/rxclass.c +++ b/rxclass.c @@ -248,13 +248,12 @@ static void rxclass_print_nfc_rule(struct ethtool_rx_flow_spec *fsp, rxclass_print_nfc_spec_ext(fsp); - if (fsp->flow_type & FLOW_RSS) - fprintf(stdout, "\tRSS Context ID: %u\n", rss_context); - if (fsp->ring_cookie == RX_CLS_FLOW_DISC) { fprintf(stdout, "\tAction: Drop\n"); } else if (fsp->ring_cookie == RX_CLS_FLOW_WAKE) { fprintf(stdout, "\tAction: Wake-on-LAN\n"); + } else if (fsp->flow_type & FLOW_RSS) + fprintf(stdout, "\tRSS context id %u\n", rss_context); } else { u64 vf = ethtool_get_flow_spec_ring_vf(fsp->ring_cookie); u64 queue = ethtool_get_flow_spec_ring(fsp->ring_cookie);
Currently, if the action for an ntuple rule is to redirect to an RSS context, the RSS context is printed as an attribute. At the same time, a wrong action is printed. For example: # ethtool -X eth0 hfunc toeplitz context new start 24 equal 8 New RSS context is 1 # ethtool -N eth0 flow-type ip6 dst-ip $IP6 context 1 Added rule with ID 0 # ethtool -n eth0 rule 0 Filter: 0 Rule Type: Raw IPv6 Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff Dest IP addr: <redacted> mask: :: Traffic Class: 0x0 mask: 0xff Protocol: 0 mask: 0xff L4 bytes: 0x0 mask: 0xffffffff RSS Context ID: 1 Action: Direct to queue 0 This is wrong and misleading. Fix by treating RSS context as a explicit action. The new output looks like this: # ./ethtool -n eth0 rule 0 Filter: 0 Rule Type: Raw IPv6 Src IP addr: :: mask: ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff Dest IP addr: <redacted> mask: :: Traffic Class: 0x0 mask: 0xff Protocol: 0 mask: 0xff L4 bytes: 0x0 mask: 0xffffffff Action: Direct to RSS context id 1 Signed-off-by: Daniel Xu <dxu@dxuuu.xyz> --- rxclass.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)