Hello, On Wed, 9 Nov 2016 14:56:33 +0200, Baruch Siach wrote: > Current .ndo_set_mac_address implementation brings up the interface when > revert > to original address after failure succeeds. Fix this. > > Signed-off-by: Baruch Siach <bar...@tkos.co.il>
Indeed, this piece of code is not very smart. > diff --git a/drivers/net/ethernet/marvell/mvpp2.c > b/drivers/net/ethernet/marvell/mvpp2.c > index 60227a3452a4..e427b4706726 100644 > --- a/drivers/net/ethernet/marvell/mvpp2.c > +++ b/drivers/net/ethernet/marvell/mvpp2.c > @@ -5686,9 +5686,8 @@ static int mvpp2_set_mac_address(struct net_device > *dev, void *p) > if (!err) > return 0; > /* Reconfigure parser to accept the original MAC address */ > - err = mvpp2_prs_update_mac_da(dev, dev->dev_addr); > - if (err) > - goto error; > + mvpp2_prs_update_mac_da(dev, dev->dev_addr); > + goto error; Wouldn't it make more sense to call mvpp2_prs_update_mac_da() under the error: goto label? But if you think beyond that, it is a bit crazy that to handle the error case of mvpp2_prs_update_mac_da(), we have to call mvpp2_prs_update_mac_da(), which is exactly the same function... Perhaps it would be interesting to investigate what are the various conditions for which mvpp2_prs_update_mac_da() fails, and see if we can avoid them. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
