Discussion:
[NET/USB] ax88179_set_mac_addr: return only error or zero
Ian Morgan
2014-10-15 13:49:19 UTC
Permalink
(It's about a USB NIC, so not sure if this is suitable to the netdev
mailing list or the linux-usb mailing list, so you both get it.)

I had an issue with bonding one of these USB3 Gigabit NIC devices
because set_mac_addr was returning a positive value (6), and the
bonding driver thinks that any non-zero value is an error.

My proposed fix was to (I thought correctly) check for negative return
codes in bond_enslave() instead of any non-zero value.

I was told by the bonding driver maintainers that my proposed fix was
incorrect, and that the device driver should return only zero on
success, as other drivers do this and that assumption has been made.
I'm really not sure why the more vigilant check in the bonding driver
was "wrong" though. We all know the rule about assumptions ;-) But I
defer to their superior knowledge of kernel interfaces.

Anyhow, it would seem that the following patch is the "more
acceptable" solution, and is in line with what other drivers do:


--- linux-3.17/drivers/net/usb/ax88179_178a.c.orig 2014-10-05
15:23:04.000000000 -0400
+++ linux-3.17/drivers/net/usb/ax88179_178a.c 2014-10-15
09:07:31.217810217 -0400
@@ -937,6 +937,7 @@ static int ax88179_set_mac_addr(struct n
{
struct usbnet *dev = netdev_priv(net);
struct sockaddr *addr = p;
+ int ret;

if (netif_running(net))
return -EBUSY;
@@ -946,8 +947,12 @@ static int ax88179_set_mac_addr(struct n
memcpy(net->dev_addr, addr->sa_data, ETH_ALEN);

/* Set the MAC address */
- return ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
+ ret = ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
ETH_ALEN, net->dev_addr);
+ if (ret < 0)
+ return ret;
+
+ return 0;
}

static const struct net_device_ops ax88179_netdev_ops = {


(non-tab-mangled patch file attached)

Your feedback is duly appreciated. Please CC me, as I am not
subscribed to these mailing lists.

--
Ian Morgan

Loading...