netdev - RE: [PATCH net] gve: unify driver name usage

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] [day] [month] [year] [list]

Message-ID: <BYAPR11MB34952904711B92E42EAF3A9CF736A@BYAPR11MB3495.namprd11.prod.outlook.com>
Date: Wed, 12 Jul 2023 17:44:55 +0000
From: "Wang, Haiyue" <haiyue.wang@...el.com>
To: "Lobakin, Aleksander" <aleksander.lobakin@...el.com>
CC: "Guo, Junfeng" <junfeng.guo@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "jeroendb@...gle.com" <jeroendb@...gle.com>,
	"pkaligineedi@...gle.com" <pkaligineedi@...gle.com>, "shailend@...gle.com"
	<shailend@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"awogbemila@...gle.com" <awogbemila@...gle.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"yangchun@...gle.com" <yangchun@...gle.com>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "csully@...gle.com" <csully@...gle.com>
Subject: RE: [PATCH net] gve: unify driver name usage

> -----Original Message-----
> From: Lobakin, Aleksander <aleksander.lobakin@...el.com>
> Sent: Thursday, July 13, 2023 00:55
> To: Wang, Haiyue <haiyue.wang@...el.com>
> Cc: Guo, Junfeng <junfeng.guo@...el.com>; netdev@...r.kernel.org; jeroendb@...gle.com;
> pkaligineedi@...gle.com; shailend@...gle.com; kuba@...nel.org; awogbemila@...gle.com;
> davem@...emloft.net; pabeni@...hat.com; yangchun@...gle.com; edumazet@...gle.com; csully@...gle.com
> Subject: Re: [PATCH net] gve: unify driver name usage
> 
> From: Wang, Haiyue <haiyue.wang@...el.com>
> Date: Tue, 11 Jul 2023 19:24:36 +0200
> 
> >> -----Original Message-----
> >> From: Lobakin, Aleksander <aleksander.lobakin@...el.com>
> >> Sent: Tuesday, July 11, 2023 21:14
> >> To: Guo, Junfeng <junfeng.guo@...el.com>
> >> Cc: netdev@...r.kernel.org; jeroendb@...gle.com; pkaligineedi@...gle.com; shailend@...gle.com; Wang,
> >> Haiyue <haiyue.wang@...el.com>; kuba@...nel.org; awogbemila@...gle.com; davem@...emloft.net;
> >> pabeni@...hat.com; yangchun@...gle.com; edumazet@...gle.com; csully@...gle.com
> >> Subject: Re: [PATCH net] gve: unify driver name usage
> >>
> >> From: Junfeng Guo <junfeng.guo@...el.com>
> >> Date: Fri,  7 Jul 2023 18:37:10 +0800
> >>
> >>> Current codebase contained the usage of two different names for this
> >>> driver (i.e., `gvnic` and `gve`), which is quite unfriendly for users
> >>> to use, especially when trying to bind or unbind the driver manually.
> >>> The corresponding kernel module is registered with the name of `gve`.
> >>> It's more reasonable to align the name of the driver with the module.
> >>
> >> [...]
> >>
> >>> @@ -2200,7 +2201,7 @@ static int gve_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>>  	if (err)
> >>>  		return err;
> >>>
> >>> -	err = pci_request_regions(pdev, "gvnic-cfg");
> >>> +	err = pci_request_regions(pdev, gve_driver_name);
> >>
> >> I won't repeat others' comments, but will comment on this.
> >> Passing just driver name with no unique identifiers makes it very
> >> confusing to read /proc/iomem et al.
> >> Imagine you have 2 NICs in your system. Then, in /proc/iomem you will have:
> >>
> >> gve 0x00001000-0x00002000
> >> gve 0x00004000-0x00005000
> >>
> >
> > Looks like, in real world, it is PCI BAR tree layers, take Intel ice as an example:
> >
> > err = pcim_iomap_regions(pdev, BIT(ICE_BAR0), dev_driver_string(dev));
> >
> > 3b4000000000-3b7fffffffff : PCI Bus 0000:b7
> >   3b7ffa000000-3b7ffe4fffff : PCI Bus 0000:b8
> >     3b7ffa000000-3b7ffbffffff : 0000:b8:00.0   <--- Different NIC, has different BDF
> >       3b7ffa000000-3b7ffbffffff : ice          <--- The region name is driver name.
> 
> I didn't say Intel drivers do that better ¯\_(ツ)_/¯
> 
> Why rely on that the kernel or something else will beautify the output
> for you or that the user won't do `grep <drvname> /proc/iomem` or
> something else? Or do that just because "look, it's done the same way in
> other drivers", which were taken into the tree years ago and sometimes
> with no detailed review?
> There are efforts[0] time to time[1] to convert precisely what you are
> doing into what I'm asking for. Do they exist by mistake or...?
> 
> (the second link also shows that even pci_name() is not enough when you
>  map several BARs of the same device, but that's not the case this time)
> 

> >>
> >> Thanks,
> >> Olek
> 
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/commit/?id=0af6e21eed2778e68139941389460e2a00d6ef8e
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/commit/?id=35bd8c07db2ce8fd2834ef866240613a4ef982e7

I see, yes, making sense to beautify the output, and is nice for 'grep ..'
to debug quickly, thanks for sharing more experience. :-)

> Thanks,
> Olek