mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kuen-Han Tsai <khtsai@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Felipe Balbi <balbi@ti.com>,
	 Kyungmin Park <kyungmin.park@samsung.com>
Cc: David Heidelberg <david@ixit.cz>,
	Ernest Van Hoecke <ernest.vanhoecke@toradex.com>,
	 Jon Hunter <jonathanh@nvidia.com>,
	LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>,
	 linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Kuen-Han Tsai <khtsai@google.com>
Subject: [PATCH v2 6/7] Revert "usb: gadget: u_ether: add gether_opts for config caching"
Date: Mon, 09 Mar 2026 20:04:51 +0800	[thread overview]
Message-ID: <20260309-f-ncm-revert-v2-6-ea2afbc7d9b2@google.com> (raw)
In-Reply-To: <20260309-f-ncm-revert-v2-0-ea2afbc7d9b2@google.com>

This reverts commit e065c6a7e46c2ee9c677fdbf50035323d2de1215.

This commit is being reverted as part of a series-wide revert.

By deferring the net_device allocation to the bind() phase, a single
function instance will spawn multiple network devices if it is symlinked
to multiple USB configurations.

This causes regressions for userspace tools (like the postmarketOS DHCP
daemon) that rely on reading the interface name (e.g., "usb0") from
configfs. Currently, configfs returns the template "usb%d", causing the
userspace network setup to fail.

Crucially, because this patch breaks the 1:1 mapping between the
function instance and the network device, this naming issue cannot
simply be patched. Configfs only exposes a single 'ifname' attribute per
instance, making it impossible to accurately report the actual interface
name when multiple underlying network devices can exist for that single
instance.

All configurations tied to the same function instance are meant to share
a single network device. Revert this change to restore the 1:1 mapping
by allocating the network device at the instance level (alloc_inst).

Reported-by: David Heidelberg <david@ixit.cz>
Closes: https://lore.kernel.org/linux-usb/70b558ea-a12e-4170-9b8e-c951131249af@ixit.cz/
Fixes: 56a512a9b410 ("usb: gadget: f_ncm: align net_device lifecycle with bind/unbind")
Signed-off-by: Kuen-Han Tsai <khtsai@google.com>
---
 drivers/usb/gadget/function/u_ether.c          |  30 -----
 drivers/usb/gadget/function/u_ether.h          |  28 ----
 drivers/usb/gadget/function/u_ether_configfs.h | 176 -------------------------
 3 files changed, 234 deletions(-)

diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 15685b2f8887..c47965d850d4 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -1040,36 +1040,6 @@ int gether_set_ifname(struct net_device *net, const char *name, int len)
 }
 EXPORT_SYMBOL_GPL(gether_set_ifname);
 
-void gether_setup_opts_default(struct gether_opts *opts, const char *name)
-{
-	opts->qmult = QMULT_DEFAULT;
-	snprintf(opts->name, sizeof(opts->name), "%s%%d", name);
-	eth_random_addr(opts->dev_mac);
-	opts->addr_assign_type = NET_ADDR_RANDOM;
-	eth_random_addr(opts->host_mac);
-}
-EXPORT_SYMBOL_GPL(gether_setup_opts_default);
-
-void gether_apply_opts(struct net_device *net, struct gether_opts *opts)
-{
-	struct eth_dev *dev = netdev_priv(net);
-
-	dev->qmult = opts->qmult;
-
-	if (opts->ifname_set) {
-		strscpy(net->name, opts->name, sizeof(net->name));
-		dev->ifname_set = true;
-	}
-
-	memcpy(dev->host_mac, opts->host_mac, sizeof(dev->host_mac));
-
-	if (opts->addr_assign_type == NET_ADDR_SET) {
-		memcpy(dev->dev_mac, opts->dev_mac, sizeof(dev->dev_mac));
-		net->addr_assign_type = opts->addr_assign_type;
-	}
-}
-EXPORT_SYMBOL_GPL(gether_apply_opts);
-
 void gether_suspend(struct gether *link)
 {
 	struct eth_dev *dev = link->ioport;
diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h
index 63a0240df4d7..34be220cef77 100644
--- a/drivers/usb/gadget/function/u_ether.h
+++ b/drivers/usb/gadget/function/u_ether.h
@@ -38,31 +38,6 @@
 
 struct eth_dev;
 
-/**
- * struct gether_opts - Options for Ethernet gadget function instances
- * @name: Pattern for the network interface name (e.g., "usb%d").
- *        Used to generate the net device name.
- * @qmult: Queue length multiplier for high/super speed.
- * @host_mac: The MAC address to be used by the host side.
- * @dev_mac: The MAC address to be used by the device side.
- * @ifname_set: True if the interface name pattern has been set by userspace.
- * @addr_assign_type: The method used for assigning the device MAC address
- *                    (e.g., NET_ADDR_RANDOM, NET_ADDR_SET).
- *
- * This structure caches network-related settings provided through configfs
- * before the net_device is fully instantiated. This allows for early
- * configuration while deferring net_device allocation until the function
- * is bound.
- */
-struct gether_opts {
-	char			name[IFNAMSIZ];
-	unsigned int		qmult;
-	u8			host_mac[ETH_ALEN];
-	u8			dev_mac[ETH_ALEN];
-	bool			ifname_set;
-	unsigned char		addr_assign_type;
-};
-
 /*
  * This represents the USB side of an "ethernet" link, managed by a USB
  * function which provides control and (maybe) framing.  Two functions
@@ -284,9 +259,6 @@ int gether_set_ifname(struct net_device *net, const char *name, int len);
 
 void gether_cleanup(struct eth_dev *dev);
 
-void gether_setup_opts_default(struct gether_opts *opts, const char *name);
-void gether_apply_opts(struct net_device *net, struct gether_opts *opts);
-
 void gether_suspend(struct gether *link);
 void gether_resume(struct gether *link);
 
diff --git a/drivers/usb/gadget/function/u_ether_configfs.h b/drivers/usb/gadget/function/u_ether_configfs.h
index 39d3a261496d..51f0d79e5eca 100644
--- a/drivers/usb/gadget/function/u_ether_configfs.h
+++ b/drivers/usb/gadget/function/u_ether_configfs.h
@@ -13,12 +13,6 @@
 #ifndef __U_ETHER_CONFIGFS_H
 #define __U_ETHER_CONFIGFS_H
 
-#include <linux/cleanup.h>
-#include <linux/if_ether.h>
-#include <linux/mutex.h>
-#include <linux/netdevice.h>
-#include <linux/rtnetlink.h>
-
 #define USB_ETHERNET_CONFIGFS_ITEM(_f_)					\
 	static void _f_##_attr_release(struct config_item *item)	\
 	{								\
@@ -203,174 +197,4 @@ out:									\
 									\
 	CONFIGFS_ATTR(_f_##_opts_, _n_)
 
-#define USB_ETHER_OPTS_ITEM(_f_)						\
-	static void _f_##_attr_release(struct config_item *item)		\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-										\
-		usb_put_function_instance(&opts->func_inst);			\
-	}									\
-										\
-	static struct configfs_item_operations _f_##_item_ops = {		\
-		.release	= _f_##_attr_release,				\
-	}
-
-#define USB_ETHER_OPTS_ATTR_DEV_ADDR(_f_)					\
-	static ssize_t _f_##_opts_dev_addr_show(struct config_item *item,	\
-						char *page)			\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-										\
-		guard(mutex)(&opts->lock);					\
-		return sysfs_emit(page, "%pM\n", opts->net_opts.dev_mac);	\
-	}									\
-										\
-	static ssize_t _f_##_opts_dev_addr_store(struct config_item *item,	\
-						 const char *page, size_t len)	\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-		u8 new_addr[ETH_ALEN];						\
-		const char *p = page;						\
-										\
-		guard(mutex)(&opts->lock);					\
-		if (opts->refcnt)						\
-			return -EBUSY;						\
-										\
-		for (int i = 0; i < ETH_ALEN; i++) {				\
-			unsigned char num;					\
-			if ((*p == '.') || (*p == ':'))				\
-				p++;						\
-			num = hex_to_bin(*p++) << 4;				\
-			num |= hex_to_bin(*p++);				\
-			new_addr[i] = num;					\
-		}								\
-		if (!is_valid_ether_addr(new_addr))				\
-			return -EINVAL;						\
-		memcpy(opts->net_opts.dev_mac, new_addr, ETH_ALEN);		\
-		opts->net_opts.addr_assign_type = NET_ADDR_SET;			\
-		return len;							\
-	}									\
-										\
-	CONFIGFS_ATTR(_f_##_opts_, dev_addr)
-
-#define USB_ETHER_OPTS_ATTR_HOST_ADDR(_f_)					\
-	static ssize_t _f_##_opts_host_addr_show(struct config_item *item,	\
-						 char *page)			\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-										\
-		guard(mutex)(&opts->lock);					\
-		return sysfs_emit(page, "%pM\n", opts->net_opts.host_mac);	\
-	}									\
-										\
-	static ssize_t _f_##_opts_host_addr_store(struct config_item *item,	\
-						  const char *page, size_t len)	\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-		u8 new_addr[ETH_ALEN];						\
-		const char *p = page;						\
-										\
-		guard(mutex)(&opts->lock);					\
-		if (opts->refcnt)						\
-			return -EBUSY;						\
-										\
-		for (int i = 0; i < ETH_ALEN; i++) {				\
-			unsigned char num;					\
-			if ((*p == '.') || (*p == ':'))				\
-				p++;						\
-			num = hex_to_bin(*p++) << 4;				\
-			num |= hex_to_bin(*p++);				\
-			new_addr[i] = num;					\
-		}								\
-		if (!is_valid_ether_addr(new_addr))				\
-			return -EINVAL;						\
-		memcpy(opts->net_opts.host_mac, new_addr, ETH_ALEN);		\
-		return len;							\
-	}									\
-										\
-	CONFIGFS_ATTR(_f_##_opts_, host_addr)
-
-#define USB_ETHER_OPTS_ATTR_QMULT(_f_)						\
-	static ssize_t _f_##_opts_qmult_show(struct config_item *item,		\
-					     char *page)			\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-										\
-		guard(mutex)(&opts->lock);					\
-		return sysfs_emit(page, "%u\n", opts->net_opts.qmult);		\
-	}									\
-										\
-	static ssize_t _f_##_opts_qmult_store(struct config_item *item,		\
-					      const char *page, size_t len)	\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-		u32 val;							\
-		int ret;							\
-										\
-		guard(mutex)(&opts->lock);					\
-		if (opts->refcnt)						\
-			return -EBUSY;						\
-										\
-		ret = kstrtou32(page, 0, &val);					\
-		if (ret)							\
-			return ret;						\
-										\
-		opts->net_opts.qmult = val;					\
-		return len;							\
-	}									\
-										\
-	CONFIGFS_ATTR(_f_##_opts_, qmult)
-
-#define USB_ETHER_OPTS_ATTR_IFNAME(_f_)						\
-	static ssize_t _f_##_opts_ifname_show(struct config_item *item,		\
-					      char *page)			\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-		const char *name;						\
-										\
-		guard(mutex)(&opts->lock);					\
-		rtnl_lock();							\
-		if (opts->net_opts.ifname_set)					\
-			name = opts->net_opts.name;				\
-		else if (opts->net)						\
-			name = netdev_name(opts->net);				\
-		else								\
-			name = "(inactive net_device)";				\
-		rtnl_unlock();							\
-		return sysfs_emit(page, "%s\n", name);				\
-	}									\
-										\
-	static ssize_t _f_##_opts_ifname_store(struct config_item *item,	\
-					       const char *page, size_t len)	\
-	{									\
-		struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item);		\
-		char tmp[IFNAMSIZ];						\
-		const char *p;							\
-		size_t c_len = len;						\
-										\
-		if (c_len > 0 && page[c_len - 1] == '\n')			\
-			c_len--;						\
-										\
-		if (c_len >= sizeof(tmp))					\
-			return -E2BIG;						\
-										\
-		strscpy(tmp, page, c_len + 1);					\
-		if (!dev_valid_name(tmp))					\
-			return -EINVAL;						\
-										\
-		/* Require exactly one %d */					\
-		p = strchr(tmp, '%');						\
-		if (!p || p[1] != 'd' || strchr(p + 2, '%'))			\
-			return -EINVAL;						\
-										\
-		guard(mutex)(&opts->lock);					\
-		if (opts->refcnt)						\
-			return -EBUSY;						\
-		strscpy(opts->net_opts.name, tmp, sizeof(opts->net_opts.name));	\
-		opts->net_opts.ifname_set = true;				\
-		return len;							\
-	}									\
-										\
-	CONFIGFS_ATTR(_f_##_opts_, ifname)
-
 #endif /* __U_ETHER_CONFIGFS_H */

-- 
2.53.0.473.g4a7958ca14-goog


  parent reply	other threads:[~2026-03-09 12:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 12:04 [PATCH v2 0/7] usb: gadget: Fix net_device lifecycle with device_move Kuen-Han Tsai
2026-03-09 12:04 ` [PATCH v2 1/7] Revert "usb: gadget: f_ncm: Fix atomic context locking issue" Kuen-Han Tsai
2026-03-09 12:04 ` [PATCH v2 2/7] Revert "usb: legacy: ncm: Fix NPE in gncm_bind" Kuen-Han Tsai
2026-03-09 12:04 ` [PATCH v2 3/7] Revert "usb: gadget: f_ncm: align net_device lifecycle with bind/unbind" Kuen-Han Tsai
2026-03-09 12:04 ` [PATCH v2 4/7] Revert "usb: gadget: u_ether: Add auto-cleanup helper for freeing net_device" Kuen-Han Tsai
2026-03-09 12:04 ` [PATCH v2 5/7] Revert "usb: gadget: u_ether: use <linux/hex.h> header file" Kuen-Han Tsai
2026-03-09 12:04 ` Kuen-Han Tsai [this message]
2026-03-09 12:04 ` [PATCH v2 7/7] usb: gadget: f_ncm: Fix net_device lifecycle with device_move Kuen-Han Tsai
2026-03-15  5:21   ` Val Packett
2026-03-16  6:03     ` Kuen-Han Tsai
2026-03-13 12:40 ` [PATCH v2 0/7] usb: gadget: " Luca Weiss
2026-03-16  6:17   ` Kuen-Han Tsai
2026-03-16  6:35     ` Greg Kroah-Hartman
2026-03-16  6:47       ` Kuen-Han Tsai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260309-f-ncm-revert-v2-6-ea2afbc7d9b2@google.com \
    --to=khtsai@google.com \
    --cc=Qing-wu.Li@leica-geosystems.com.cn \
    --cc=balbi@ti.com \
    --cc=david@ixit.cz \
    --cc=ernest.vanhoecke@toradex.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jonathanh@nvidia.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®