[PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Vladimir Oltean]

Several drivers regressed when ethtool --show-rxfh was converted from
ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
implement ETHTOOL_GRXRINGS do not necessarily implement ETHTOOL_GCHANNELS
or its netlink equivalent.

According to the man page, "A channel is an IRQ and the set of queues
that can trigger that IRQ.", which is different from the definition of
a queue/ring. So we shouldn't be attempting to query the # of rings for
the ioctl variant, but the # of channels for the netlink variant anyway.

Reimplement the args->num_rings retrieval as in do_grxfh(), aka using
the ETHTOOL_GRXRINGS ioctl.

Link: https://lore.kernel.org/netdev/20240711114535.pfrlbih3ehajnpvh@skbuf/
Fixes: ffab99c1f382 ("netlink: add netlink handler for get rss (-x)")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 ethtool.c     |  2 +-
 internal.h    |  1 +
 netlink/rss.c | 55 +++++++++++++++++++--------------------------------
 3 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 98690df05992..89c0edefe8eb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6369,7 +6369,7 @@ static int do_perqueue(struct cmd_context *ctx)
 	return 0;
 }
 
-static int ioctl_init(struct cmd_context *ctx, bool no_dev)
+int ioctl_init(struct cmd_context *ctx, bool no_dev)
 {
 	if (no_dev) {
 		ctx->fd = -1;
diff --git a/internal.h b/internal.h
index 3923719c39d5..1bd5515fc9f7 100644
--- a/internal.h
+++ b/internal.h
@@ -293,6 +293,7 @@ int test_fclose(FILE *fh);
 #endif
 #endif
 
+int ioctl_init(struct cmd_context *ctx, bool no_dev);
 int send_ioctl(struct cmd_context *ctx, void *cmd);
 
 void dump_hex(FILE *f, const u8 *data, int len, int offset);
diff --git a/netlink/rss.c b/netlink/rss.c
index 4ad6065ef698..d0045b1f0f8f 100644
--- a/netlink/rss.c
+++ b/netlink/rss.c
@@ -54,29 +54,29 @@ void dump_json_rss_info(struct cmd_context *ctx, u32 *indir_table,
 	close_json_object();
 }
 
-int get_channels_cb(const struct nlmsghdr *nlhdr, void *data)
+/* There is no netlink equivalent for ETHTOOL_GRXRINGS. */
+static int get_num_rings(struct cb_args *args)
 {
-	const struct nlattr *tb[ETHTOOL_A_CHANNELS_MAX + 1] = {};
-	DECLARE_ATTR_TB_INFO(tb);
-	struct cb_args *args = data;
 	struct nl_context *nlctx = args->nlctx;
-	bool silent;
-	int err_ret;
+	struct cmd_context *ctx = nlctx->ctx;
+	struct ethtool_rxnfc ring_count = {
+		.cmd = ETHTOOL_GRXRINGS,
+	};
 	int ret;
 
-	silent = nlctx->is_dump || nlctx->is_monitor;
-	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
-	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
-	if (ret < 0)
-		return err_ret;
-	nlctx->devname = get_dev_name(tb[ETHTOOL_A_CHANNELS_HEADER]);
-	if (!dev_ok(nlctx))
-		return err_ret;
-	if (tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT])
-		args->num_rings = mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_COMBINED_COUNT]);
-	if (tb[ETHTOOL_A_CHANNELS_RX_COUNT])
-		args->num_rings += mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]);
-	return MNL_CB_OK;
+	ret = ioctl_init(ctx, false);
+	if (ret)
+		return ret;
+
+	ret = send_ioctl(ctx, &ring_count);
+	if (ret) {
+		perror("Cannot get RX ring count");
+		return ret;
+	}
+
+	args->num_rings = (u32)ring_count.data;
+
+	return 0;
 }
 
 int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
@@ -131,22 +131,7 @@ int rss_reply_cb(const struct nlmsghdr *nlhdr, void *data)
 	if (ret < 0)
 		return silent ? MNL_CB_OK : MNL_CB_ERROR;
 
-	nlctx->devname = get_dev_name(tb[ETHTOOL_A_RSS_HEADER]);
-	if (!dev_ok(nlctx))
-		return MNL_CB_OK;
-
-	/* Fetch ring count info into args->num_rings */
-	ret = nlsock_prep_get_request(nlctx->ethnl2_socket,
-				      ETHTOOL_MSG_CHANNELS_GET,
-				      ETHTOOL_A_CHANNELS_HEADER, 0);
-	if (ret < 0)
-		return MNL_CB_ERROR;
-
-	ret = nlsock_sendmsg(nlctx->ethnl2_socket, NULL);
-	if (ret < 0)
-		return MNL_CB_ERROR;
-
-	ret = nlsock_process_reply(nlctx->ethnl2_socket, get_channels_cb, args);
+	ret = get_num_rings(args);
 	if (ret < 0)
 		return MNL_CB_ERROR;
 
-- 
2.34.1

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Jakub Kicinski]

On Fri, 13 Sep 2024 12:38:28 +0300 Vladimir Oltean wrote:
> Several drivers regressed when ethtool --show-rxfh was converted from
> ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
> ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
> ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
> implement ETHTOOL_GRXRINGS do not necessarily implement ETHTOOL_GCHANNELS
> or its netlink equivalent.
> 
> According to the man page, "A channel is an IRQ and the set of queues
> that can trigger that IRQ.", which is different from the definition of
> a queue/ring. So we shouldn't be attempting to query the # of rings for
> the ioctl variant, but the # of channels for the netlink variant anyway.
> 
> Reimplement the args->num_rings retrieval as in do_grxfh(), aka using
> the ETHTOOL_GRXRINGS ioctl.

Acked-by: Jakub Kicinski <kuba@kernel.org>

Thanks for fixing this! Not sure why Sudheer didn't.

RE: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Mogilappagari, Sudheer]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Friday, September 13, 2024 2:38 AM
> To: netdev@vger.kernel.org
> Cc: Michal Kubecek <mkubecek@suse.cz>; Jakub Kicinski
> <kuba@kernel.org>; Mogilappagari, Sudheer
> <sudheer.mogilappagari@intel.com>
> Subject: [PATCH ethtool] netlink: rss: retrieve ring count using
> ETHTOOL_GRXRINGS ioctl
> 
> Several drivers regressed when ethtool --show-rxfh was converted from
> ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
> ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
> ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
> implement ETHTOOL_GRXRINGS do not necessarily implement
> ETHTOOL_GCHANNELS or its netlink equivalent.
> 
> According to the man page, "A channel is an IRQ and the set of queues
> that can trigger that IRQ.", which is different from the definition of
> a queue/ring. So we shouldn't be attempting to query the # of rings for
> the ioctl variant, but the # of channels for the netlink variant
> anyway.
> 
> Reimplement the args->num_rings retrieval as in do_grxfh(), aka using
> the ETHTOOL_GRXRINGS ioctl.
> 


> -	if (tb[ETHTOOL_A_CHANNELS_RX_COUNT])
> -		args->num_rings +=
> mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]);
> -	return MNL_CB_OK;
> +	ret = ioctl_init(ctx, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = send_ioctl(ctx, &ring_count);
> +	if (ret) {
> +		perror("Cannot get RX ring count");
> +		return ret;
> +	}
> +
> +	args->num_rings = (u32)ring_count.data;
> +
> +	return 0;
>  }
> 

Hi Vladimir, my understanding is ioctls are not used
in ethtool netlink path. Can we use ETHTOOL_MSG_RINGS_GET 
(tb[ETHTOOL_A_RINGS_RX] member) instead ? 

Couldn't work on this since I was on sabbatical till this week.

RE: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Mogilappagari, Sudheer]

> -----Original Message-----
> From: Mogilappagari, Sudheer
> Sent: Friday, September 13, 2024 3:43 PM
> To: Vladimir Oltean <vladimir.oltean@nxp.com>; netdev@vger.kernel.org
> Cc: Michal Kubecek <mkubecek@suse.cz>; Jakub Kicinski <kuba@kernel.org>
> Subject: RE: [PATCH ethtool] netlink: rss: retrieve ring count using
> ETHTOOL_GRXRINGS ioctl
> 
> 
> 
> > -----Original Message-----
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > Sent: Friday, September 13, 2024 2:38 AM
> > To: netdev@vger.kernel.org
> > Cc: Michal Kubecek <mkubecek@suse.cz>; Jakub Kicinski
> > <kuba@kernel.org>; Mogilappagari, Sudheer
> > <sudheer.mogilappagari@intel.com>
> > Subject: [PATCH ethtool] netlink: rss: retrieve ring count using
> > ETHTOOL_GRXRINGS ioctl
> >
> > Several drivers regressed when ethtool --show-rxfh was converted from
> > ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
> > ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
> > ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
> > implement ETHTOOL_GRXRINGS do not necessarily implement
> > ETHTOOL_GCHANNELS or its netlink equivalent.
> >
> > According to the man page, "A channel is an IRQ and the set of queues
> > that can trigger that IRQ.", which is different from the definition
> of
> > a queue/ring. So we shouldn't be attempting to query the # of rings
> > for the ioctl variant, but the # of channels for the netlink variant
> > anyway.
> >
> > Reimplement the args->num_rings retrieval as in do_grxfh(), aka using
> > the ETHTOOL_GRXRINGS ioctl.
> >
> 
> 
> > -	if (tb[ETHTOOL_A_CHANNELS_RX_COUNT])
> > -		args->num_rings +=
> > mnl_attr_get_u32(tb[ETHTOOL_A_CHANNELS_RX_COUNT]);
> > -	return MNL_CB_OK;
> > +	ret = ioctl_init(ctx, false);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = send_ioctl(ctx, &ring_count);
> > +	if (ret) {
> > +		perror("Cannot get RX ring count");
> > +		return ret;
> > +	}
> > +
> > +	args->num_rings = (u32)ring_count.data;
> > +
> > +	return 0;
> >  }
> >
> 
> Hi Vladimir, my understanding is ioctls are not used in ethtool netlink
> path. Can we use ETHTOOL_MSG_RINGS_GET (tb[ETHTOOL_A_RINGS_RX] member)
> instead ?
> 
> Couldn't work on this since I was on sabbatical till this week.

I see Jakub ack'ed this. Please ignore my comment above. 

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Vladimir Oltean]

Hi Sudheer,

On Fri, Sep 13, 2024 at 10:43:19PM +0000, Mogilappagari, Sudheer wrote:
> Hi Vladimir, my understanding is ioctls are not used
> in ethtool netlink path. Can we use ETHTOOL_MSG_RINGS_GET 
> (tb[ETHTOOL_A_RINGS_RX] member) instead ? 

You mean this?

  ``ETHTOOL_A_RINGS_RX``                u32     size of RX ring

RE: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Mogilappagari, Sudheer]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: Friday, September 13, 2024 3:49 PM
> To: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
> Cc: netdev@vger.kernel.org; Michal Kubecek <mkubecek@suse.cz>; Jakub
> Kicinski <kuba@kernel.org>
> Subject: Re: [PATCH ethtool] netlink: rss: retrieve ring count using
> ETHTOOL_GRXRINGS ioctl
> 
> Hi Sudheer,
> 
> On Fri, Sep 13, 2024 at 10:43:19PM +0000, Mogilappagari, Sudheer wrote:
> > Hi Vladimir, my understanding is ioctls are not used in ethtool
> > netlink path. Can we use ETHTOOL_MSG_RINGS_GET
> (tb[ETHTOOL_A_RINGS_RX]
> > member) instead ?
> 
> You mean this?
> 
>   ``ETHTOOL_A_RINGS_RX``                u32     size of RX ring

Yes. I had meant ETHTOOL_A_RINGS_RX but I see it is ring size and not ring
count. There seems to be no netlink message that fetches ringcount info. 

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Jacob Keller]

On 9/13/2024 4:07 PM, Mogilappagari, Sudheer wrote:
> 
> 
>> -----Original Message-----
>> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>> Sent: Friday, September 13, 2024 3:49 PM
>> To: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
>> Cc: netdev@vger.kernel.org; Michal Kubecek <mkubecek@suse.cz>; Jakub
>> Kicinski <kuba@kernel.org>
>> Subject: Re: [PATCH ethtool] netlink: rss: retrieve ring count using
>> ETHTOOL_GRXRINGS ioctl
>>
>> Hi Sudheer,
>>
>> On Fri, Sep 13, 2024 at 10:43:19PM +0000, Mogilappagari, Sudheer wrote:
>>> Hi Vladimir, my understanding is ioctls are not used in ethtool
>>> netlink path. Can we use ETHTOOL_MSG_RINGS_GET
>> (tb[ETHTOOL_A_RINGS_RX]
>>> member) instead ?
>>
>> You mean this?
>>
>>   ``ETHTOOL_A_RINGS_RX``                u32     size of RX ring
> 
> Yes. I had meant ETHTOOL_A_RINGS_RX but I see it is ring size and not ring
> count. There seems to be no netlink message that fetches ringcount info. 
> 

We could extend the netlink interface to add this over netlink instead
of ioctl, but that hasn't been done yet.

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Vladimir Oltean]

Hello Jake,

On Thu, Sep 19, 2024 at 10:58:48AM -0700, Jacob Keller wrote:
> We could extend the netlink interface to add this over netlink instead
> of ioctl, but that hasn't been done yet.

Let me understand how this will play out. So you're proposing that we
add a new netlink attribute in ETHTOOL_MSG_RSS_GET_REPLY, with Fixes:
7112a04664bf ("ethtool: add netlink based get rss support"), and this
gets backported to all stable kernels which include this commit?

And then also 'fix' ethtool to parse this new netlink attribute instead
of ETHTOOL_A_CHANNELS_RX_COUNT? Thus introducing another breakage: when
the new ethtool program runs with the old (aka unpatched stable) kernel?

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

On Thu, Oct 03, 2024 at 12:18:10PM +0300, Vladimir Oltean wrote:
> Hello Jake,
> 
> On Thu, Sep 19, 2024 at 10:58:48AM -0700, Jacob Keller wrote:
> > We could extend the netlink interface to add this over netlink instead
> > of ioctl, but that hasn't been done yet.
> 
> Let me understand how this will play out. So you're proposing that we
> add a new netlink attribute in ETHTOOL_MSG_RSS_GET_REPLY, with Fixes:
> 7112a04664bf ("ethtool: add netlink based get rss support"), and this
> gets backported to all stable kernels which include this commit?
> 
> And then also 'fix' ethtool to parse this new netlink attribute instead
> of ETHTOOL_A_CHANNELS_RX_COUNT? Thus introducing another breakage: when
> the new ethtool program runs with the old (aka unpatched stable) kernel?

I'm afraid we will have to keep the unfortunate ioctl fallback for quite
long. The only other option would be to only use netlink for RSS against
kernel which provides full information and use only ioctl against those
which don't.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Vladimir Oltean]

On Thu, Oct 03, 2024 at 01:09:47PM +0200, Michal Kubecek wrote:
> I'm afraid we will have to keep the unfortunate ioctl fallback for quite
> long. The only other option would be to only use netlink for RSS against
> kernel which provides full information and use only ioctl against those
> which don't.
> 
> Michal

So, then, is there anything blocking this patch?

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Thu, Oct 03, 2024 at 04:49:16PM +0300, Vladimir Oltean wrote:
> On Thu, Oct 03, 2024 at 01:09:47PM +0200, Michal Kubecek wrote:
> > I'm afraid we will have to keep the unfortunate ioctl fallback for quite
> > long. The only other option would be to only use netlink for RSS against
> > kernel which provides full information and use only ioctl against those
> > which don't.
> > 
> > Michal
> 
> So, then, is there anything blocking this patch?

I'm still not fully convinced that this mix of netlink and ioctl is
actually better than fully reverting to ioctl until we can get all
information via netlink.

Either way, I'm going to handle this before the end of this week so that
ethtool 6.11 can be released. At the moment I'm in favor of your patch,
however unhappy I'm about it.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 484 bytes --]

RE: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Keller, Jacob E]

> -----Original Message-----
> From: Michal Kubecek <mkubecek@suse.cz>
> Sent: Thursday, October 3, 2024 4:20 PM
> To: Vladimir Oltean <vladimir.oltean@nxp.com>
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; Mogilappagari, Sudheer
> <sudheer.mogilappagari@intel.com>; netdev@vger.kernel.org; Jakub Kicinski
> <kuba@kernel.org>
> Subject: Re: [PATCH ethtool] netlink: rss: retrieve ring count using
> ETHTOOL_GRXRINGS ioctl
> 
> On Thu, Oct 03, 2024 at 04:49:16PM +0300, Vladimir Oltean wrote:
> > On Thu, Oct 03, 2024 at 01:09:47PM +0200, Michal Kubecek wrote:
> > > I'm afraid we will have to keep the unfortunate ioctl fallback for quite
> > > long. The only other option would be to only use netlink for RSS against
> > > kernel which provides full information and use only ioctl against those
> > > which don't.
> > >
> > > Michal
> >
> > So, then, is there anything blocking this patch?
> 
> I'm still not fully convinced that this mix of netlink and ioctl is
> actually better than fully reverting to ioctl until we can get all
> information via netlink.
> 
> Either way, I'm going to handle this before the end of this week so that
> ethtool 6.11 can be released. At the moment I'm in favor of your patch,
> however unhappy I'm about it.
> 
> Michal

I have no objection to your patch, I think its correct to do now. My suggestion was that we can improve the netlink interface for the future, and I believe we can make ethtool continue to use the existing ioctl interface on older kernels, but use the netlink interface once its available.

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

On Fri, Oct 04, 2024 at 07:19:24PM +0000, Keller, Jacob E wrote:
> I have no objection to your patch, I think its correct to do now. My
> suggestion was that we can improve the netlink interface for the
> future, and I believe we can make ethtool continue to use the existing
> ioctl interface on older kernels, but use the netlink interface once
> its available.

This is not the problem. It's what we have been doing since netlink
interface was introduced and it's what we are going to be doing for
quite long.

What I'm unhappy about is the mix of netlink and ioctl where we use
netlink request for RSS but also an ioctl request to get the ring count.
I can't help wondering if it wouldn't make more sense to fallback to
ioctl fully unless we can retrieve full information via netlink.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Jacob Keller]

On 10/5/2024 12:46 AM, Michal Kubecek wrote:
> On Fri, Oct 04, 2024 at 07:19:24PM +0000, Keller, Jacob E wrote:
>> I have no objection to your patch, I think its correct to do now. My
>> suggestion was that we can improve the netlink interface for the
>> future, and I believe we can make ethtool continue to use the existing
>> ioctl interface on older kernels, but use the netlink interface once
>> its available.
> 
> This is not the problem. It's what we have been doing since netlink
> interface was introduced and it's what we are going to be doing for
> quite long.
> 
> What I'm unhappy about is the mix of netlink and ioctl where we use
> netlink request for RSS but also an ioctl request to get the ring count.
> I can't help wondering if it wouldn't make more sense to fallback to
> ioctl fully unless we can retrieve full information via netlink.
> 
> Michal

That seems reasonable to me

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

On Fri, Sep 13, 2024 at 12:38:28PM +0300, Vladimir Oltean wrote:
> Several drivers regressed when ethtool --show-rxfh was converted from
> ioctl to netlink. This is because ETHTOOL_GRXRINGS was converted to
> ETHTOOL_MSG_CHANNELS_GET, which is semantically equivalent to
> ETHTOOL_GCHANNELS but different from ETHTOOL_GRXRINGS. Drivers which
> implement ETHTOOL_GRXRINGS do not necessarily implement ETHTOOL_GCHANNELS
> or its netlink equivalent.
> 
> According to the man page, "A channel is an IRQ and the set of queues
> that can trigger that IRQ.", which is different from the definition of
> a queue/ring. So we shouldn't be attempting to query the # of rings for
> the ioctl variant, but the # of channels for the netlink variant anyway.

I have asked this multiple times but I never got a direct answer: is
this only a formal terminology problem or is there an actual difference
between the two? In particular: is someone aware of an example of
a driver and device where these two counts differ? Or is there a reason
to expect such device either exists or will exist in the future?

(Actually, I seem to remember that the first time I asked, the general
consensus was that combined + rx is indeed the value we need here -
which is what current code does. But it has been few years so I would
have to look it up to be sure.)

If there is no real difference, it would be really unfortunate to have
the same count presented under two different names via two different
interfaces (->get_rxnfc() and ->get_channels() in ethtool_ops) with most
drivers providing both but some only one and some only the other. Which
seems to be the current state and the long term solution should be
cleaning this up.

If there is an actual difference, the long term solution would be
providing the necessary information as part of the
reply to ETHTOOL_MSG_RSS_GET requests - and the sooner we add it, the
better.

Short term, I'm afraid that however ugly this hack is, it's either it or
disabling the netlink handler until we can get the information reliably
from kernel.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool] netlink: rss: retrieve ring count using ETHTOOL_GRXRINGS ioctl

[Jakub Kicinski]

On Mon, 16 Sep 2024 21:38:14 +0200 Michal Kubecek wrote:
> I have asked this multiple times but I never got a direct answer: is
> this only a formal terminology problem or is there an actual difference
> between the two? In particular: is someone aware of an example of
> a driver and device where these two counts differ? Or is there a reason
> to expect such device either exists or will exist in the future?
> 
> (Actually, I seem to remember that the first time I asked, the general
> consensus was that combined + rx is indeed the value we need here -
> which is what current code does. But it has been few years so I would
> have to look it up to be sure.)

The change in perspective comes from the potential for dynamic queue
allocation in the future. We don't have the exact details but it's
possible that we'd choose to treat "channels" as NAPI instances rather
than queues. And more queues can be dynamically opened by the
applications and attached to the existing channels.