public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] e1000e: ethtool: add get_channels support
@ 2026-05-04 15:48 Jon Kohler
  2026-05-04 17:41 ` Joe Damato
  2026-05-04 23:49 ` Jakub Kicinski
  0 siblings, 2 replies; 7+ messages in thread
From: Jon Kohler @ 2026-05-04 15:48 UTC (permalink / raw)
  To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, intel-wired-lan,
	netdev, linux-kernel
  Cc: Jon Kohler

e1000e hardware supports a single RX/TX queue pair, add basic support
for ethtool -l (i.e. get_channels), so that callers indeed see a single
queue.

Signed-off-by: Jon Kohler <jon@nutanix.com>
---
 drivers/net/ethernet/intel/e1000e/ethtool.c | 26 +++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index dbed30943ef4..3a2c1a77ee74 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -2353,6 +2353,31 @@ static int e1000e_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 	return 0;
 }
 
+/**
+ * e1000e_get_channels - Report number of network channels
+ * @netdev: network interface device structure
+ * @ch: ethtool channels structure
+ *
+ * e1000e hardware supports a single RX/TX queue pair. When MSI-X is
+ * in use, an additional vector is dedicated to link/status ("other")
+ * events.
+ **/
+static void e1000e_get_channels(struct net_device *netdev,
+				struct ethtool_channels *ch)
+{
+	struct e1000_adapter *adapter = netdev_priv(netdev);
+
+	/* single combined RX/TX channel */
+	ch->max_combined = 1;
+	ch->combined_count = 1;
+
+	/* report "other" vector when MSI-X is in use */
+	if (adapter->msix_entries) {
+		ch->max_other = 1;
+		ch->other_count = 1;
+	}
+}
+
 static const struct ethtool_ops e1000_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS,
 	.get_drvinfo		= e1000_get_drvinfo,
@@ -2386,6 +2411,7 @@ static const struct ethtool_ops e1000_ethtool_ops = {
 	.set_link_ksettings	= e1000_set_link_ksettings,
 	.get_priv_flags		= e1000e_get_priv_flags,
 	.set_priv_flags		= e1000e_set_priv_flags,
+	.get_channels		= e1000e_get_channels,
 };
 
 void e1000e_set_ethtool_ops(struct net_device *netdev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-04 15:48 [PATCH net-next] e1000e: ethtool: add get_channels support Jon Kohler
@ 2026-05-04 17:41 ` Joe Damato
  2026-05-04 23:49 ` Jakub Kicinski
  1 sibling, 0 replies; 7+ messages in thread
From: Joe Damato @ 2026-05-04 17:41 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, intel-wired-lan,
	netdev, linux-kernel

On Mon, May 04, 2026 at 08:48:23AM -0700, Jon Kohler wrote:
> e1000e hardware supports a single RX/TX queue pair, add basic support
> for ethtool -l (i.e. get_channels), so that callers indeed see a single
> queue.
> 
> Signed-off-by: Jon Kohler <jon@nutanix.com>
> ---
>  drivers/net/ethernet/intel/e1000e/ethtool.c | 26 +++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Joe Damato <joe@dama.to>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-04 15:48 [PATCH net-next] e1000e: ethtool: add get_channels support Jon Kohler
  2026-05-04 17:41 ` Joe Damato
@ 2026-05-04 23:49 ` Jakub Kicinski
  2026-05-05  0:59   ` Jon Kohler
  1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-05-04 23:49 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, intel-wired-lan, netdev, linux-kernel

On Mon,  4 May 2026 08:48:23 -0700 Jon Kohler wrote:
> e1000e hardware supports a single RX/TX queue pair, add basic support
> for ethtool -l (i.e. get_channels), so that callers indeed see a single
> queue.

Why? Isn't EOPNOTSUP from ethtool -l implicitly saying that there's
only one queue?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-04 23:49 ` Jakub Kicinski
@ 2026-05-05  0:59   ` Jon Kohler
  2026-05-05  1:06     ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Jon Kohler @ 2026-05-05  0:59 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org



> On May 4, 2026, at 7:49 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Mon,  4 May 2026 08:48:23 -0700 Jon Kohler wrote:
>> e1000e hardware supports a single RX/TX queue pair, add basic support
>> for ethtool -l (i.e. get_channels), so that callers indeed see a single
>> queue.
> 
> Why? Isn't EOPNOTSUP from ethtool -l implicitly saying that there's
> only one queue?

Perhaps, but I’m not sure that is a guarantee. A good relevant example
is when I added get_channels support to enic, which supports all sorts
of channels, so I don’t think EOPNOTSUP can be 100% consider reliable
in that case. Meaning, if it just so happens that the original author(s)
didn't put in get_channels, that doesn’t necessarily mean there is only
one queue.

And in this case, there is an "other" queue as as well too, as far as
I can tell, so the output is at least semi-interesting.

Jon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-05  0:59   ` Jon Kohler
@ 2026-05-05  1:06     ` Jakub Kicinski
  2026-05-05  1:12       ` Jon Kohler
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-05-05  1:06 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org

On Tue, 5 May 2026 00:59:40 +0000 Jon Kohler wrote:
> > On May 4, 2026, at 7:49 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> >> e1000e hardware supports a single RX/TX queue pair, add basic support
> >> for ethtool -l (i.e. get_channels), so that callers indeed see a single
> >> queue.  
> > 
> > Why? Isn't EOPNOTSUP from ethtool -l implicitly saying that there's
> > only one queue?  
> 
> Perhaps, but I’m not sure that is a guarantee. A good relevant example
> is when I added get_channels support to enic, which supports all sorts
> of channels, so I don’t think EOPNOTSUP can be 100% consider reliable
> in that case. Meaning, if it just so happens that the original author(s)
> didn't put in get_channels, that doesn’t necessarily mean there is only
> one queue.
> 
> And in this case, there is an "other" queue as as well too, as far as
> I can tell, so the output is at least semi-interesting.

Sorry I wasn't clear enough - if you have an actual, real life use case
why you need queue count of 1 to be explicitly reported - please explain
it and put it in the commit message.

If you don't - please don't send patches for the sake of it.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-05  1:06     ` Jakub Kicinski
@ 2026-05-05  1:12       ` Jon Kohler
  2026-05-05  1:26         ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Jon Kohler @ 2026-05-05  1:12 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org



> On May 4, 2026, at 9:06 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> 
> On Tue, 5 May 2026 00:59:40 +0000 Jon Kohler wrote:
>>> On May 4, 2026, at 7:49 PM, Jakub Kicinski <kuba@kernel.org> wrote:
>>>> e1000e hardware supports a single RX/TX queue pair, add basic support
>>>> for ethtool -l (i.e. get_channels), so that callers indeed see a single
>>>> queue.  
>>> 
>>> Why? Isn't EOPNOTSUP from ethtool -l implicitly saying that there's
>>> only one queue?  
>> 
>> Perhaps, but I’m not sure that is a guarantee. A good relevant example
>> is when I added get_channels support to enic, which supports all sorts
>> of channels, so I don’t think EOPNOTSUP can be 100% consider reliable
>> in that case. Meaning, if it just so happens that the original author(s)
>> didn't put in get_channels, that doesn’t necessarily mean there is only
>> one queue.
>> 
>> And in this case, there is an "other" queue as as well too, as far as
>> I can tell, so the output is at least semi-interesting.
> 
> Sorry I wasn't clear enough - if you have an actual, real life use case
> why you need queue count of 1 to be explicitly reported - please explain
> it and put it in the commit message.
> 
> If you don't - please don't send patches for the sake of it.

Ah, ok, sorry I misread your message, this isn’t a patch for the sake of
a patch. Long story short, we’ve got a user space part of our control plane
that reads in the output of ethtool -l as part of some broader queue
management code. On systems with an e1000e device present, this specific
component goes into a crash loop as it expects all NIC(s) to at least
give it some sort of output.

That crash loop is easy enough to fix to ignore unsupported outputs;
however, my thought here is a simply defense in depth fixup, especially
since the kernel patch is quite trivial.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next] e1000e: ethtool: add get_channels support
  2026-05-05  1:12       ` Jon Kohler
@ 2026-05-05  1:26         ` Jakub Kicinski
  0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2026-05-05  1:26 UTC (permalink / raw)
  To: Jon Kohler
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org

On Tue, 5 May 2026 01:12:29 +0000 Jon Kohler wrote:
> > On May 4, 2026, at 9:06 PM, Jakub Kicinski <kuba@kernel.org> wrote:
> > 
> > On Tue, 5 May 2026 00:59:40 +0000 Jon Kohler wrote:  
>  [...]  
>  [...]  
>  [...]  
> >> 
> >> Perhaps, but I’m not sure that is a guarantee. A good relevant example
> >> is when I added get_channels support to enic, which supports all sorts
> >> of channels, so I don’t think EOPNOTSUP can be 100% consider reliable
> >> in that case. Meaning, if it just so happens that the original author(s)
> >> didn't put in get_channels, that doesn’t necessarily mean there is only
> >> one queue.
> >> 
> >> And in this case, there is an "other" queue as as well too, as far as
> >> I can tell, so the output is at least semi-interesting.  
> > 
> > Sorry I wasn't clear enough - if you have an actual, real life use case
> > why you need queue count of 1 to be explicitly reported - please explain
> > it and put it in the commit message.
> > 
> > If you don't - please don't send patches for the sake of it.  
> 
> Ah, ok, sorry I misread your message, this isn’t a patch for the sake of
> a patch. Long story short, we’ve got a user space part of our control plane
> that reads in the output of ethtool -l as part of some broader queue
> management code. On systems with an e1000e device present, this specific
> component goes into a crash loop as it expects all NIC(s) to at least
> give it some sort of output.
> 
> That crash loop is easy enough to fix to ignore unsupported outputs;
> however, my thought here is a simply defense in depth fixup, especially
> since the kernel patch is quite trivial.

Got it, thanks for explaining.

My concern is that if we are expected to always report channel counts
we're signing up for a major whack-a-mole with the existing drivers.
Most drivers don't implement it. The networking stack does report
the number of queues the device asked for via rtnetlink:

ip -j -d li show dev $ifc | jq '.[].num_rx_queues'

but in your case I'd personally lean towards user space fix.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-05  1:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 15:48 [PATCH net-next] e1000e: ethtool: add get_channels support Jon Kohler
2026-05-04 17:41 ` Joe Damato
2026-05-04 23:49 ` Jakub Kicinski
2026-05-05  0:59   ` Jon Kohler
2026-05-05  1:06     ` Jakub Kicinski
2026-05-05  1:12       ` Jon Kohler
2026-05-05  1:26         ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox