netdev - Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device driver for config selection

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: <20251020175921.37f35e5a.michal.pecio@gmail.com>
Date: Mon, 20 Oct 2025 17:59:21 +0200
From: Michal Pecio <michal.pecio@...il.com>
To: Oliver Neukum <oneukum@...e.com>
Cc: Alan Stern <stern@...land.harvard.edu>, yicongsrfy@....com,
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, linux-usb@...r.kernel.org, netdev@...r.kernel.org,
 oliver@...kum.org, pabeni@...hat.com
Subject: Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device
 driver for config selection

On Mon, 20 Oct 2025 11:59:06 +0200, Oliver Neukum wrote:
> On 18.10.25 17:21, Michal Pecio wrote:
> 
> > index e85105939af8..1d2c5ebc81ab 100644
> > --- a/include/linux/usb.h
> > +++ b/include/linux/usb.h
> > @@ -1202,6 +1202,8 @@ extern ssize_t usb_show_dynids(struct usb_dynids *dynids, char *buf);
> >    * @post_reset: Called by usb_reset_device() after the device
> >    *	has been reset
> >    * @shutdown: Called at shut-down time to quiesce the device.
> > + * @preferred: Check if this driver is preferred over generic class drivers
> > + *	applicable to the device. May probe device with control transfers.
> >    * @id_table: USB drivers use ID table to support hotplugging.
> >    *	Export this with MODULE_DEVICE_TABLE(usb,...).  This must be set
> >    *	or your driver's probe function will never get called.
> > @@ -1255,6 +1257,8 @@ struct usb_driver {
> >   
> >   	void (*shutdown)(struct usb_interface *intf);
> >   
> > +	bool (*preferred)(struct usb_device *udev);  
> 
> I am sorry, but this is a bit clunky. If you really want to
> introduce such a method, why not just return the preferred
> configuration?

Because I wanted to introduce exactly such a method, rather than one
which returns the configuration ;)

The point was to pull configuration selection *out* of those drivers.
They already do it, and it makes them copy-paste the same trivial loop
which iterates through configs until it finds the vendor interface.

The idea is to have a maximally simple check for a known-good vendor
interface driver before making unfounded assumptions like:

/* From the remaining configs, choose the first one whose
 * first interface is for a non-vendor-specific class.
 * Reason: Linux is more likely to have a class driver
 * than a vendor-specific driver. */

Unfortunately, that's only half the battle. The other half is forcing
configuration reevaluation when such a driver is loaded. I hoped it
would be trivial, but so far it costs me a new bus_for_each_device()
and a whole nontrivial function, while cfgselectors have it for free.

I got my PoC up to feature parity with r8152-cfgselector and it adds
about as much code as it removes (uless more cfgselectors are added).
And of course it's dead weight for those with USB but not USBNET.

 drivers/net/usb/r8152.c    | 69 ++++++++-------------------------------------------------------------
 drivers/usb/core/driver.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/core/generic.c | 26 ++++++++++++++++----------
 include/linux/usb.h        | 11 ++++++-----
 4 files changed, 78 insertions(+), 76 deletions(-)

So not sure if it's worth pursuing.

Regards,
Michal