Re: [PATCH v6 1/6] mac80211: introduce switch_vif_chanctx op

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2014-05-23 at 08:09 +0200, Michal Kazior wrote:

> >> +#define IEEE80211_MAX_NUM_SWITCH_VIFS 8
> >
> > :-)
> >
> > That seems artificial though - why not dynamically allocate?
> 
> I tend to avoid dynamic allocations whenever I can. I can make it
> dynamic if you want.

I'm just thinking that Ben Greear will invariably complain about this
limit ;-)

> I think I've had the same problem when I was trying to make a
> single-call multi-vif csa tracing. Is using dynamic_array for this
> really doable? I haven't found any code in the kernel using
> __dynamic_array for anything but simple scalars and buffers.

You could either use multiple arrays, or I guess you could even pack a
struct into the array, no?

> >> +     if (WARN_ON(!use_chanctx && ops->switch_vif_chanctx))
> >> +             return NULL;
> >
> > I don't think this makes sense - we perfectly handle the case right now
> > by disconnecting (and not advertising switch to userspace, I guess? if
> > not we should)
> >
> > Requiring drivers to implement this just makes things more difficult,
> > and the channel switch isn't really mandatory spec-wise.
> 
> This is supposed to prevent non-chanctx drivers from accidentally
> implementing switch_vif_chanctx. It doesn't require chanctx drivers to
> implement switch_vif_chanctx.

Err, yeah, I misread the code - sorry.

> >> +             __field(u32, old_control_freq)
> >
> > I believe there's a macro for a chandef?
> 
> Yes there is, but it assumes you have a `control_freq`. Since there
> are old_ and new_ prefixes in my tracing I can't really use that
> define, can I? I'd have to modify the define (and all callsites) to
> take an argument with a prefix and concatenate symbols with ##.

Oh, you're right, somehow I thought we'd done something like that
already.

Probably not worth it then.

johannes