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 for Android: free password hash cracker in your pocket

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

Date: Tue, 11 Jul 2023 15:13:36 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Junfeng Guo <junfeng.guo@...el.com>
CC: <netdev@...r.kernel.org>, <jeroendb@...gle.com>,
	<pkaligineedi@...gle.com>, <shailend@...gle.com>, <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

Can you say which region belongs to which NIC? Nope.
If you really want to make this more "user friendly", you should make it
possible for users to distinguish different NICs in your system. The
easiest way:

	err = pci_request_regions(pdev, pci_name(pdev));

But you're not limited to this. Just make it unique.

(as a net-next commit obv)

>  	if (err)
>  		goto abort_with_enabled;
>  
[...]

Thanks,
Olek