netdev - RE: [PATCH net-next] net: enetc: add ethtool::get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 23 Nov 2023 02:09:57 +0000
From: Wei Fang <wei.fang@....com>
To: Vladimir Oltean <vladimir.oltean@....com>
CC: "davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, Claudiu Manoil
	<claudiu.manoil@....com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next] net: enetc: add ethtool::get_channels support

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@....com>
> Sent: 2023年11月22日 19:01
> To: Wei Fang <wei.fang@....com>
> Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; Claudiu Manoil <claudiu.manoil@....com>;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH net-next] net: enetc: add ethtool::get_channels support
> 
> Hi Wei,
> 
> On Wed, Nov 22, 2023 at 06:25:40PM +0800, Wei Fang wrote:
> > Since ETHTOOL_MSG_RSS_GET netlink message [1] has been applied to
> > ethtool tree, there is a netlink error when using "ethtool -x eno0"
> > command to get RSS information from fsl-enetc driver, and the user
> > cannot get the information, the error logs are as follows:
> >
> > root@...028ardb:~# ./ethtool -x eno0
> > netlink error: Operation not supported
> >
> > The rationale is that ethtool will issue a ETHTOOL_MSG_CHANNELS_GET
> > netlink message to get the number of Rx ring. However, the fsl-enetc
> > driver doesn't support ethtool::get_channels, so it directly returns
> > -EOPNOTSUPP error.
> >
> > [1]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/ethtool.git/c
> > ommit/?id=ffab99c1f3820e21d65686e030dcf2c4fd0a8bd0
> >
> > Signed-off-by: Wei Fang <wei.fang@....com>
> > ---
> 
> I think we have 2 problems on our hands.
> 
> 1. enetc is not the only driver that doesn't report
> ETHTOOL_MSG_CHANNELS_GET.
>    So it is a general issue for Sudheer Mogilappagari's implementation
>    of "ethtool -x" using netlink. The ioctl-based implementation used to
>    look at ETHTOOL_GRXRINGS which was handled in the kernel through
>    ethtool_ops :: get_rxnfc.
> 
> 2. I used to have a different implementation (and interpretation) of
>    ETHTOOL_MSG_CHANNELS_GET for enetc anyway, which associated
> channels
>    not with rings, but with interrupt vectors (making the reported
>    channels combined):
> 
> https://patchwork.kernel.org/project/netdevbpf/patch/20230206100837.451
> 300-6-vladimir.oltean@....com/
> 
[Wei] Sorry, I didn't notice you patch before, I think you patch is more better than
mine after I read the "channel" interpretation in napi.rst doc.

> 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.