[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

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

[Jakub Kicinski]

On Fri,  3 Jul 2026 15:39:36 +0800 Hanson Wang wrote:
> -	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);

nit: sashiko suggest the indent of the continuation line is off, which
seems true. Please fix this and repost

[PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[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.

Callers such as netif_set_mac_address() and bonding treat any
non-zero return value as failure when setting the MAC address.

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

Link: https://lore.kernel.org/netdev/20260703073936.462231-1-hanson.wang@ugreen.com

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

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 9201ee10a..827aab540 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -476,8 +476,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 net v2] net: usb: aqc111: fix set_mac_address return value

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[sashiko-bot]

> 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.
> 
> Callers such as netif_set_mac_address() and bonding treat any
> non-zero return value as failure when setting the MAC address.
> 
> Return 0 on success and propagate negative error codes on failure.
> 
> Link: https://lore.kernel.org/netdev/20260703073936.462231-1-hanson.wang@ugreen.com
> 
> Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016122944.1889622-1-hanson.wang@ugreen.com?part=1

[PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[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.

Callers such as netif_set_mac_address() and bonding treat any
non-zero return value as failure when setting the MAC address.

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

Link: https://lore.kernel.org/netdev/20260703073936.462231-1-hanson.wang@ugreen.com

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

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index 9201ee10a..827aab540 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -476,8 +476,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 net v2] net: usb: aqc111: fix set_mac_address return value

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[sashiko-bot]

> 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.
> 
> Callers such as netif_set_mac_address() and bonding treat any
> non-zero return value as failure when setting the MAC address.
> 
> Return 0 on success and propagate negative error codes on failure.
> 
> Link: https://lore.kernel.org/netdev/20260703073936.462231-1-hanson.wang@ugreen.com
> 
> Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016124858.1939743-1-hanson.wang@ugreen.com?part=1

Re: [PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[Simon Horman]

On Fri, Oct 16, 2026 at 08:48:58PM +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.
> 
> Callers such as netif_set_mac_address() and bonding treat any
> non-zero return value as failure when setting the MAC address.
> 
> Return 0 on success and propagate negative error codes on failure.
> 
> Link: https://lore.kernel.org/netdev/20260703073936.462231-1-hanson.wang@ugreen.com
> 
> Signed-off-by: Hanson Wang <hanson.wang@ugreen.com>

Overall this looks good to me, thanks for your contribution.

Reviewed-by: Simon Horman <horms@kernel.org>

Some points on process to keep in mind.
There is no need to repost to address these.

1. Please respond to netdev-bot+sinfo.
   It's not too late to do so!

   https://lore.kernel.org/netdev/179215519735.408196.139629932849333374@kernel.org/

2. Please post new versions to patches in new email threads,
   not as a reply to an earlier versions.

3. Please don't repost within 24h.

4. Please don't repost the same version, without either
   incrementing the version number or including repost in the subject.

   [PATCh net v2 repost] ...

5. Please do include a summary of what has changed below the scissors ('---').
   Ideally including lore.kernel.org links to earlier versions.

   v2: Fixes indentation
   v1: https://lore.kernel.org/netdev/43a0904c-de59-4b40-b186-edd130f6c0cb@lunn.ch/

6. Please don't use the Link tag to link to an earlier version.
   Instead use it if there is some extra information,
   e.g. an otherwise unrelated email thread that lead to this patch.

For more information on Netdev processes please see
https://docs.kernel.org/process/maintainer-netdev.html

Re: [PATCH net v2] net: usb: aqc111: fix set_mac_address return value

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 16 Oct 2026 20:48:58 +0800 you 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.
> 
> Callers such as netif_set_mac_address() and bonding treat any
> non-zero return value as failure when setting the MAC address.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: usb: aqc111: fix set_mac_address return value
    https://git.kernel.org/netdev/net/c/c3a1ddd5e116

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html