Re: [PATCH v4 0/4] mac80211/cfg80211: CSA fixes/cleanups

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2014-05-07 at 09:04 +0200, Michal Kazior wrote:

> > However, there's one more thing - one caller of ieee80211_csa_finalize()
> > doesn't check the return value now, I think it probably should. Mind
> > taking a look at that? It should *probably* be the same as the other
> > caller (and not propagate the error), I'd say, so maybe that should be
> > rolled into the function?
> 
> Right - the return value of ieee80211_csa_finalize() in
> __ieee80211_channel_switch() should be checked against. I think it
> should be like:
> 
>  err = ieee80211_csa_finalize(sdata);
>  if (err) {
>   sdata_info(sdata, "failed to finalize immediate CSA, disconnecting\n");
>   cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev, GFP_KERNEL);
> 
>   /* don't propagate the error here - stop the iface instead since
> there's no way to revert CS now */
>   err = 0;
>  }
> 
> Should I re-post the patch(set?), post a follow up, or are you going
> to update the patch yourself?

Can you post a separate patch on top? I already have it in my tree, but
I can squash it I guess if I really want to.

> I'm not sure what you mean by rolling "that" into the function. You
> want to put the stop_iface() in csa_finalize()? I think it's going to
> look a bit ugly that way.

Well, the only other caller of it right now does the same thing:

       err = ieee80211_csa_finalize(sdata);
       if (err) {
               sdata_info(sdata, "failed to finalize CSA, disconnecting
\n");
               cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev,
GFP_KERNEL);
               goto unlock;
       }

so it seems it would make sense to have that as common code, maybe in
the ieee80211_csa_finalize() function or maybe by changing
ieee80211_csa_finalize() to be __ieee80211_csa_finalize() and making a
new ieee80211_csa_finalize() that does this step?

In fact, then we could get rid of the int return value again, so maybe
better post a new version of this particular patch after all.

johannes