| Message ID | 20240325173155.671807-1-jtornosm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: usb: ax88179_178a: avoid the interface always configured as random address | expand |
On Mon, Mar 25, 2024 at 06:31:50PM +0100, Jose Ignacio Tornos Martinez wrote: > After the commit d2689b6a86b9 ("net: usb: ax88179_178a: avoid two > consecutive device resets"), reset is not executed from bind operation and > mac address is not read from the device registers or the devicetree at that > moment. Since the check to configure if the assigned mac address is random > or not for the interface, happens after the bind operation from > usbnet_probe, the interface keeps configured as random address, although the > address is correctly read and set during open operation (the only reset > now). > > In order to keep only one reset for the device and to avoid the interface > always configured as random address, after reset, configure correctly the > suitable field from the driver, if the mac address is read successfully from > the device registers or the devicetree. Thanks Jose, The above makes sense to me and I agree with your fix and corresponding Fixes tag. > In addition, if mac address can not be read from the driver, a random > address is configured again, so it is not necessary to call > eth_hw_addr_random from here. Indeed, in this situtatuon, when reset was > also executed from bind, this was invalidating the check to configure if the > assigned mac address for the interface was random or not. I also agree with your analysis here. However it does seem to be a separate problem. And perhaps warrants a separate patch. I am also wondering if this is more of a clean-up than a fix: does it cause a bug that is observable by users? > cc: stable@vger.kernel.org # 6.6+ > Fixes: d2689b6a86b9 ("net: usb: ax88179_178a: avoid two consecutive device resets") > Reported-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> > --- > drivers/net/usb/ax88179_178a.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c > index 88e084534853..d2324cc02461 100644 > --- a/drivers/net/usb/ax88179_178a.c > +++ b/drivers/net/usb/ax88179_178a.c > @@ -1273,10 +1273,9 @@ static void ax88179_get_mac_addr(struct usbnet *dev) > > if (is_valid_ether_addr(mac)) { > eth_hw_addr_set(dev->net, mac); > - } else { > + dev->net->addr_assign_type = NET_ADDR_PERM; > + } else > netdev_info(dev->net, "invalid MAC address, using random\n"); > - eth_hw_addr_random(dev->net); > - } nit: AFAIK, if one arm of a conditional has curly-brackets, then all should. So there is no need to drop them here. > > ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, ETH_ALEN, > dev->net->dev_addr); > -- > 2.44.0 > >
Hello Simon, >> In addition, if mac address can not be read from the driver, a random >> address is configured again, so it is not necessary to call >> eth_hw_addr_random from here. Indeed, in this situtatuon, when reset was >> also executed from bind, this was invalidating the check to configure if the >> assigned mac address for the interface was random or not. > > I also agree with your analysis here. However it does seem to be a separate > problem. And perhaps warrants a separate patch. I am also wondering > if this is more of a clean-up than a fix: does it cause a bug > that is observable by users? You are right, really it is a separate improvement or simplification. Right now, it is not affecting the users and it is not producing any problem, just a second random address is generated if there is any problem, and this is not necessary, because there is a random address generated previously. When the extra reset was done during binding operation, as we were modifying the pregenerated random address, the check in usbnet_probe was useless. Ok, I will split the patch in two to be considered separately, the first with the important fix and the second with the commented improvement or clean-up. > nit: AFAIK, if one arm of a conditional has curly-brackets, then all should. > So there is no need to drop them here. I didn't know the related criteria, I will do as you say. Thank you Best regards José Ignacio
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 88e084534853..d2324cc02461 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1273,10 +1273,9 @@ static void ax88179_get_mac_addr(struct usbnet *dev) if (is_valid_ether_addr(mac)) { eth_hw_addr_set(dev->net, mac); - } else { + dev->net->addr_assign_type = NET_ADDR_PERM; + } else netdev_info(dev->net, "invalid MAC address, using random\n"); - eth_hw_addr_random(dev->net); - } ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, ETH_ALEN, dev->net->dev_addr);
After the commit d2689b6a86b9 ("net: usb: ax88179_178a: avoid two consecutive device resets"), reset is not executed from bind operation and mac address is not read from the device registers or the devicetree at that moment. Since the check to configure if the assigned mac address is random or not for the interface, happens after the bind operation from usbnet_probe, the interface keeps configured as random address, although the address is correctly read and set during open operation (the only reset now). In order to keep only one reset for the device and to avoid the interface always configured as random address, after reset, configure correctly the suitable field from the driver, if the mac address is read successfully from the device registers or the devicetree. In addition, if mac address can not be read from the driver, a random address is configured again, so it is not necessary to call eth_hw_addr_random from here. Indeed, in this situtatuon, when reset was also executed from bind, this was invalidating the check to configure if the assigned mac address for the interface was random or not. cc: stable@vger.kernel.org # 6.6+ Fixes: d2689b6a86b9 ("net: usb: ax88179_178a: avoid two consecutive device resets") Reported-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> --- drivers/net/usb/ax88179_178a.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)