netdev - Re: [PATCH net-next] net: mana: Add new device attributes for mana

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]

Message-ID: <b34bfb11-98a3-4418-b482-14f2e50745d3@lunn.ch>
Date: Tue, 16 Apr 2024 20:09:35 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Zhu Yanjun <yanjun.zhu@...ux.dev>
Cc: Jason Gunthorpe <jgg@...pe.ca>,
	Shradha Gupta <shradhagupta@...ux.microsoft.com>,
	linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Ajay Sharma <sharmaajay@...rosoft.com>,
	Leon Romanovsky <leon@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	"K. Y. Srinivasan" <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
	Long Li <longli@...rosoft.com>,
	Michael Kelley <mikelley@...rosoft.com>,
	Shradha Gupta <shradhagupta@...rosoft.com>,
	Yury Norov <yury.norov@...il.com>,
	Konstantin Taranov <kotaranov@...rosoft.com>,
	Souradeep Chakrabarti <schakrabarti@...ux.microsoft.com>
Subject: Re: [PATCH net-next] net: mana: Add new device attributes for mana

On Tue, Apr 16, 2024 at 06:27:04AM +0200, Zhu Yanjun wrote:
> 在 2024/4/15 18:13, Jason Gunthorpe 写道:
> > On Mon, Apr 15, 2024 at 02:49:49AM -0700, Shradha Gupta wrote:
> > > Add new device attributes to view multiport, msix, and adapter MTU
> > > setting for MANA device.
> > > 
> > > Signed-off-by: Shradha Gupta <shradhagupta@...ux.microsoft.com>
> > > ---
> > >   .../net/ethernet/microsoft/mana/gdma_main.c   | 74 +++++++++++++++++++
> > >   include/net/mana/gdma.h                       |  9 +++
> > >   2 files changed, 83 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > index 1332db9a08eb..6674a02cff06 100644
> > > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > > @@ -1471,6 +1471,65 @@ static bool mana_is_pf(unsigned short dev_id)
> > >   	return dev_id == MANA_PF_DEVICE_ID;
> > >   }
> > > +static ssize_t mana_attr_show(struct device *dev,
> > > +			      struct device_attribute *attr, char *buf)
> > > +{
> > > +	struct pci_dev *pdev = to_pci_dev(dev);
> > > +	struct gdma_context *gc = pci_get_drvdata(pdev);
> > > +	struct mana_context *ac = gc->mana.driver_data;
> > > +
> > > +	if (strcmp(attr->attr.name, "mport") == 0)
> > > +		return snprintf(buf, PAGE_SIZE, "%d\n", ac->num_ports);
> > > +	else if (strcmp(attr->attr.name, "adapter_mtu") == 0)
> > > +		return snprintf(buf, PAGE_SIZE, "%d\n", gc->adapter_mtu);
> > > +	else if (strcmp(attr->attr.name, "msix") == 0)
> > > +		return snprintf(buf, PAGE_SIZE, "%d\n", gc->max_num_msix);
> > > +	else
> > > +		return -EINVAL;
> > > +
> > 
> > That is not how sysfs should be implemented at all, please find a
> > good example to copy from. Every attribute should use its own function
> > with the macros to link it into an attributes group and sysfs_emit
> > should be used for printing
> 
> Not sure if this file drivers/infiniband/hw/usnic/usnic_ib_sysfs.c is a good
> example or not.

The first question should be, what are these values used for? You can
then decide on debugfs or sysfs. debugfs is easier to use, and you
avoid any ABI, which will make long term support easier.

      Andrew