Hi,
> > > + if (live)
> > > + drv_remove_interface(local, sdata);
> > > ret = eth_mac_addr(dev, sa);
> > > + if (live)
> > > + ret = drv_add_interface(local, sdata);
> > >
> > > if (ret == 0)
> > > memcpy(sdata->vif.addr, sa->sa_data, ETH_ALEN);
> > >
> >
> > I still don't like the (lack of) error checking here. As far as I know,
> > eth_mac_addr() can very happily fail if the passed address is invalid,
> > so we really shouldn't overwrite the ret value by drv_add_interface().
>
> Ah yes, that was an oversight. I assume we do want to add_interface
> even if eth_mac_addr fails though.
Right.
> So my only question is about the
> return from drv_add_interface(). Is this unlikely to fail? If so would
> just a WARN_ON be sufficient and return the value from eth_mac_addr()?
Hm, yeah, I guess it really ought to not fail here.
> So something like:
>
> if (live)
> drv_remove_interface(local, sdata);
> ret = eth_mac_addr(dev, sa);
> if (ret == 0)
> memcpy(sdata->vif.addr, sa->sa_data, ETH_ALEN);
>
> if (live)
> WARN_ON(drv_add_interface(local, sdata));
>
> return ret;
Seems reasonable. We could do something like
err = drv_add_interface(...);
if (err) {
dev_close(...);
ret = ret ?: err;
}
or something, but not sure that's worth it, it really shouldn't fail at
this point.
But I guess we could leave setting NL80211_EXT_FEATURE_AUTH_TX_RANDOM_TA
to the driver if we think it'd be less risky that way?
johannes
