public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Wei Fang <wei.fang@nxp.com>
Cc: "davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next] net: enetc: add ethtool::get_channels support
Date: Thu, 23 Nov 2023 11:40:45 +0200	[thread overview]
Message-ID: <20231123094045.bjgo4aqwdydeirgs@skbuf> (raw)
In-Reply-To: <AM5PR04MB3139E96BF99E55E563EF71B888B9A@AM5PR04MB3139.eurprd04.prod.outlook.com>

On Thu, Nov 23, 2023 at 04:09:57AM +0200, Wei Fang wrote:
> > I would suggest finding a way for the user space implementation to not
> > assume that ETHTOOL_MSG_CHANNELS_GET is implemented by the driver.
> 
> [Wei] IMO, the issue you encountered is that libbpf will perform an 
> ETHTOOL_GCHANNELS operation. The issue I encountered is that "ethtool -x"
> will also perform an ETHTOOL_GCHANNELS operation. Besides, There are other
> apps that do the same operation as this, so I think it's best for fsl-enetc driver
> to support querying channels.
> Because your patch is more reasonable than mine, I think you should submit
> this patch to upstream separately first.

The crucial piece you're omitting is that for ethtool -x, the "get channels"
operation didn't use to be required, making this new requirement a
breaking change. The interface between the Linux kernel and applications
doesn't do that, which is why it's preferable to bring it up with the
ethtool maintainers and the patch author instead of fixing one driver
and leaving who knows how many still unfixed (and also the stable
kernels unfixed for enetc as well).

To be clear, I won't submit the "get_channels" enetc patch for the RFH
indirection table to keep working. I might resubmit it for other
reasons, like when I return to the AF_XDP zerocopy work.

      reply	other threads:[~2023-11-23  9:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-22 10:25 [PATCH net-next] net: enetc: add ethtool::get_channels support Wei Fang
2023-11-22 10:49 ` [EXT] " Suman Ghosh
2023-11-23  1:49   ` Wei Fang
2023-11-22 11:01 ` Vladimir Oltean
2023-11-23  2:09   ` Wei Fang
2023-11-23  9:40     ` Vladimir Oltean [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231123094045.bjgo4aqwdydeirgs@skbuf \
    --to=vladimir.oltean@nxp.com \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=wei.fang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox