On Thu, Jan 14, 2021 at 12:45:50AM +0900, Lorenzo Colitti wrote:
> This patch allows the administrator to configure the interface
> name of a function using u_ether (e.g., eem, ncm, rndis).
>
> Currently, all such interfaces, regardless of function type, are
> always called usb0, usb1, etc. This makes it very cumbersome to
> use more than one such type at a time, because userspace cannnot
> easily tell the interfaces apart and apply the right
> configuration to each one. Interface renaming in userspace based
> on driver doesn't help, because the interfaces all have the same
> driver. Without this patch, doing this require hacks/workarounds
> such as setting fixed MAC addresses on the functions, and then
> renaming by MAC address, or scraping configfs after each
> interface is created to find out what it is.
>
> Setting the interface name is done by writing to the same
> "ifname" configfs attribute that reports the interface name after
> the function is bound. The write must contain an interface
> pattern such as "usb%d" (which will cause the net core to pick
> the next available interface name starting with "usb").
> This patch does not allow writing an exact interface name (as
> opposed to a pattern) because if the interface already exists at
> bind time, the bind will fail and the whole gadget will fail to
> activate. This could be allowed in a future patch.
>
> For compatibility with current userspace, when reading an ifname
> that has not currently been set, the result is still "(unnamed
> net_device)". Once a write to ifname happens, then reading ifname
> will return whatever was last written.
>
> Tested by configuring an rndis function and an ncm function on
> the same gadget, and writing "rndis%d" to ifname on the rndis
> function and "ncm%d" to ifname on the ncm function. When the
> gadget was bound, the rndis interface was rndis0 and the ncm
> interface was ncm0.
>
> Signed-off-by: Lorenzo Colitti <lorenzo@xxxxxxxxxx>
> ---
> Documentation/usb/gadget-testing.rst | 30 +++++++--------
> drivers/usb/gadget/function/u_ether.c | 37 +++++++++++++++++--
> drivers/usb/gadget/function/u_ether.h | 12 ++++++
> .../usb/gadget/function/u_ether_configfs.h | 15 +++++++-
> 4 files changed, 75 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index 2eeb3e9299e4..2085e7b24eeb 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -91,9 +91,9 @@ The ECM function provides these attributes in its function directory:
>
> and after creating the functions/ecm.<instance name> they contain default
> values: qmult is 5, dev_addr and host_addr are randomly selected.
> -Except for ifname they can be written to until the function is linked to a
> -configuration. The ifname is read-only and contains the name of the interface
> -which was assigned by the net core, e. g. usb0.
> +The ifname can be written to if the function is not bound. A write must be an
> +interface pattern such as "usb%d", which will cause the net core to choose the
> +next free usbX interface. By default, it is set to "usb%d".
>
> Testing the ECM function
> ------------------------
> @@ -131,9 +131,9 @@ The ECM subset function provides these attributes in its function directory:
>
> and after creating the functions/ecm.<instance name> they contain default
> values: qmult is 5, dev_addr and host_addr are randomly selected.
> -Except for ifname they can be written to until the function is linked to a
> -configuration. The ifname is read-only and contains the name of the interface
> -which was assigned by the net core, e. g. usb0.
> +The ifname can be written to if the function is not bound. A write must be an
> +interface pattern such as "usb%d", which will cause the net core to choose the
> +next free usbX interface. By default, it is set to "usb%d".
>
> Testing the ECM subset function
> -------------------------------
> @@ -171,9 +171,9 @@ The EEM function provides these attributes in its function directory:
>
> and after creating the functions/eem.<instance name> they contain default
> values: qmult is 5, dev_addr and host_addr are randomly selected.
> -Except for ifname they can be written to until the function is linked to a
> -configuration. The ifname is read-only and contains the name of the interface
> -which was assigned by the net core, e. g. usb0.
> +The ifname can be written to if the function is not bound. A write must be an
> +interface pattern such as "usb%d", which will cause the net core to choose the
> +next free usbX interface. By default, it is set to "usb%d".
>
> Testing the EEM function
> ------------------------
> @@ -453,9 +453,9 @@ The NCM function provides these attributes in its function directory:
>
> and after creating the functions/ncm.<instance name> they contain default
> values: qmult is 5, dev_addr and host_addr are randomly selected.
> -Except for ifname they can be written to until the function is linked to a
> -configuration. The ifname is read-only and contains the name of the interface
> -which was assigned by the net core, e. g. usb0.
> +The ifname can be written to if the function is not bound. A write must be an
> +interface pattern such as "usb%d", which will cause the net core to choose the
> +next free usbX interface. By default, it is set to "usb%d".
>
> Testing the NCM function
> ------------------------
> @@ -591,9 +591,9 @@ The RNDIS function provides these attributes in its function directory:
>
> and after creating the functions/rndis.<instance name> they contain default
> values: qmult is 5, dev_addr and host_addr are randomly selected.
> -Except for ifname they can be written to until the function is linked to a
> -configuration. The ifname is read-only and contains the name of the interface
> -which was assigned by the net core, e. g. usb0.
> +The ifname can be written to if the function is not bound. A write must be an
> +interface pattern such as "usb%d", which will cause the net core to choose the
> +next free usbX interface. By default, it is set to "usb%d".
>
> Testing the RNDIS function
> --------------------------
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index c019f2b0c0af..a4ade7983402 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -78,8 +78,9 @@ struct eth_dev {
> unsigned long todo;
> #define WORK_RX_MEMORY 0
>
> - bool zlp;
> - bool no_skb_reserve;
> + bool zlp:1;
> + bool no_skb_reserve:1;
> + bool ifname_set:1;
Not to be a pain, but "bool var:1"? What's wrong with just a bool
itself? gcc is known to have issues with setting/clearing bit fields
like this, and there is no size issues here with this device, adding one
more byte is just fine.
So just add:
bool ifname_set;
please.
thanks,
greg k-h
