On Mon, 2013-11-25 at 18:23 +0200, Jouni Malinen wrote:
> On Mon, Nov 25, 2013 at 05:08:27PM +0100, Johannes Berg wrote:
> > On Wed, 2013-11-20 at 12:28 +0200, Jouni Malinen wrote:
> >
> > > +struct cfg80211_qos_map {
> > > + bool valid;
> >
> > why do we need a validity flag? isn't NULL pointer enough?
>
> For cfg80211, NULL would be fine. I guess the mac80211 use case could be
> handled outside this structure.
mac80211 could also just kmemdup() it and only keep a pointer (RCU
protected?), that'd indeed be more efficient if nobody is using it,
which is probably still the majority of APs?
> > > + int (*set_qos_map)(struct wiphy *wiphy,
> > > + struct net_device *dev,
> > > + struct cfg80211_qos_map *qos_map);
> >
> > I wonder if this is necessary? The driver could just have a flag for
> > "support QoS mapping" and having such support requires using
> > classify8021d(). OTOH, that function needs to gain a parameter anyway,
> > so it's not really worth it. Also, it's something that could be changed
> > purely internal to the kernel, so I guess I'm not worried about it much.
>
> Hmm.. How else would the caller (that driver or mac80211 receiving this
> call..) of classify8021d() get these parameters?
We'd have to pass the wdev instead of the qos_map, and then we could
store it all in the wdev and handle it all in cfg80211. But then again
many other drivers might actually need it for firmware or whatever, so I
guess it makes sense. We can also see, this isn't really related to the
userland API.
> > > + * @NL80211_CMD_SET_QOS_MAP: Set Interworking QoS mapping for IP DSCP values.
> > > + * The QoS mapping information is included in %NL80211_ATTR_QOS_MAP. If
> > > + * that attribute is not included, QoS mapping is disabled.
> >
> > Might be useful to say that this is only valid while associated?
>
> When else can you send an IP packet? I don't have anything against
> adding that here, but it sounds pretty obvious to me.
Yes, obviously, but also not. If we say this, then we should also say
that it's cleared on disassoc/stop_ap and things like that, I guess? I
generally dislike state that can survive between associations (unless
the device is roaming itself, but in this case presumably it'd have to
handle it all by itself then) or across stop_ap/start_ap.
> > In fact, why is this even a new operation? Can this change during the
> > lifetime of the BSS? If not, then it might be worth just putting the
> > attribute into START_AP and ASSOCIATE/CONNECT etc?
>
> Yes, this can change during an association (configurable operation on
> AP; a new Action frame from AP to STA for notifying about the change).
Ok.
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
