On Thu, 2015-05-21 at 09:44 +0200, Michal Kazior wrote:
> On 20 May 2015 at 15:19, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> > On Wed, 2015-05-20 at 15:17 +0200, Johannes Berg wrote:
> >
> >> > - if (ntype != otype && netif_running(dev)) {
> >> > + if (ntype != otype) {
> >> > dev->ieee80211_ptr->use_4addr = false;
> >> > dev->ieee80211_ptr->mesh_id_up_len = 0;
> >> > wdev_lock(dev->ieee80211_ptr);
> >>
> >> I don't think that makes much sense - the code within this block really
> >> only makes sense when the interface *is* running, like disconnecting
> >> etc. Doing that when it's *not* would be entirely unexpected to the
> >> drivers, no?
>
> The netif_running() was originally introduced in f8cdddb8d61d which
> did in for entirely different purpose. Hence stripping netif_running()
> shouldn't be a problem for drivers, can it? The patch was also made
> kind of obsolete by b6a550156bc. Perhaps there are some other
> behavioral changes that I'm unaware of in stuff that gets called upon
> entering this condition down the stack..?
Perhaps it doesn't matter actually, since all the functions that are
called here will double-check things before calling the driver?
> Maybe it's just safer to move use_4addr/mesh_id_up_len clearing into a
> separate `oftype != ntype` condition. What do you think?
Or maybe we should just look at the functions we call in more detail :)
> > The real problem here might be the assignment to use_4addr *before*
> > we've actually disconnected or anything, perhaps that should be moved?
> >
> > Similarly, the mesh_id_up_len should probably be moved into the mesh
> > point switch case...
>
> The problem isn't use_4addr clearing ordering per se. The problem is
> it wasn't cleared at all on interface changes.
Ah.
> Do you want me to put the above into the commit log? Should I put a
> copy into the mac80211 patch as well?
I think just noting (more explicitly) that it was *missing* a clearing
in these cases would have been helpful.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
