[PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding

[PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding

[Hanson Wang]

aqc111_set_mac_addr() returns the result of aqc111_write_cmd() on
success. That function wraps usb_control_msg(), which returns the
number of bytes transferred (6 for ETH_ALEN) rather than zero.

Bonding calls ndo_set_mac_address() when enslaving an interface and
treats any non-zero return value as failure.

Return 0 on success and propagate negative error codes on failure.

Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>
---
 drivers/net/usb/aqc111.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index dd53f413c38f..da5b74637ee2 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -471,8 +471,12 @@ static int aqc111_set_mac_addr(struct net_device *net, void *p)
 		return ret;
 
 	/* Set the MAC address */
-	return aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_NODE_ID, ETH_ALEN,
-				ETH_ALEN, net->dev_addr);
+	ret = aqc111_write_cmd(dev, AQ_ACCESS_MAC, SFR_NODE_ID, ETH_ALEN,
+			        ETH_ALEN, net->dev_addr);
+	if (ret < 0)
+		return ret;
+
+	return 0;
 }
 
 static int aqc111_vlan_rx_kill_vid(struct net_device *net,
-- 
2.34.1

Re: [PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding

[Andrew Lunn]

On Fri, Jul 03, 2026 at 03:39:36PM +0800, Hanson Wang wrote:
> aqc111_set_mac_addr() returns the result of aqc111_write_cmd() on
> success. That function wraps usb_control_msg(), which returns the
> number of bytes transferred (6 for ETH_ALEN) rather than zero.
> 
> Bonding calls ndo_set_mac_address() when enslaving an interface and
> treats any non-zero return value as failure.

It is not just bonding which has problems:

int netif_set_mac_address(struct net_device *dev, struct sockaddr_storage *ss,
			  struct netlink_ext_ack *extack)
{
	const struct net_device_ops *ops = dev->netdev_ops;
	int err;

	if (!ops->ndo_set_mac_address)
		return -EOPNOTSUPP;
	if (ss->ss_family != dev->type)
		return -EINVAL;
	if (!netif_device_present(dev))
		return -ENODEV;
	err = netif_pre_changeaddr_notify(dev, ss->__data, extack);
	if (err)
		return err;
	if (memcmp(dev->dev_addr, ss->__data, dev->addr_len)) {
		err = ops->ndo_set_mac_address(dev, ss);
		if (err)
			return err;

Could you take a quick look at all USB ethernet drivers, and see if
this bug exists in other drivers. If one driver has it wrong, it could
well be more have it wrong.

     Andrew

Re: [PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding

[Hanson Wang]

Hi Andrew,

Thanks for the review. You are right that this is not bonding-specific -
any caller of ndo_set_mac_address() (including netif_set_mac_address()
and "ip link set address") expects 0 on success and treats any non-zero
return value as failure.

I audited all drivers under drivers/net/usb/ that implement a custom
ndo_set_mac_address callback:

  aqc111.c        - BUG: returns usb_control_msg byte count (6) [this patch]
  ax88179_178a.c  - OK: returns 0 after ax_write_cmd()
  asix_common.c   - OK: returns 0 (async write to hardware)
  ch397.c         - OK: returns 0
  dm9601.c        - OK: returns 0 (async write)
  lan78xx.c       - OK: returns 0
  mcs7830.c       - OK: returns 0
  qmi_wwan.c      - OK: returns 0
  r8152.c/r8157.c - OK: returns 0 on success (via usb_autopm_get_interface)
  rtl8150.c       - OK: returns 0
  sr9700.c        - OK: returns 0 (async write)
  sr9800.c        - OK: returns 0 (async write)

Drivers that use eth_mac_addr as ndo_set_mac_address (smsc95xx,
smsc75xx, cdc_ncm, rndis, pegasus, usbnet default, etc.) always
return 0 from the netdev op.

As far as I can tell, aqc111 is the only driver that directly returns
the result of a USB control write without normalizing success to 0.

Please let me know if you would like any further changes to this patch.

Best regards,
Hanson Wang

Re: [PATCH 1/1] net: usb: aqc111: fix set_mac_address return value for bonding

[Andrew Lunn]

On Sat, Jul 04, 2026 at 04:42:58PM +0800, Hanson Wang wrote:
> Hi Andrew,
> 
> Thanks for the review. You are right that this is not bonding-specific -
> any caller of ndo_set_mac_address() (including netif_set_mac_address()
> and "ip link set address") expects 0 on success and treats any non-zero
> return value as failure.
> 
> I audited all drivers under drivers/net/usb/ that implement a custom
> ndo_set_mac_address callback:
> 
>   aqc111.c        - BUG: returns usb_control_msg byte count (6) [this patch]
>   ax88179_178a.c  - OK: returns 0 after ax_write_cmd()
>   asix_common.c   - OK: returns 0 (async write to hardware)
>   ch397.c         - OK: returns 0
>   dm9601.c        - OK: returns 0 (async write)
>   lan78xx.c       - OK: returns 0
>   mcs7830.c       - OK: returns 0
>   qmi_wwan.c      - OK: returns 0
>   r8152.c/r8157.c - OK: returns 0 on success (via usb_autopm_get_interface)
>   rtl8150.c       - OK: returns 0
>   sr9700.c        - OK: returns 0 (async write)
>   sr9800.c        - OK: returns 0 (async write)

Thanks for reviewing these drivers.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew