On Thu, 2019-08-15 at 16:38 +0300, Jouni Malinen wrote:
> +/**
> + * DOC: VLAN offload support for setting group keys and binding STAs to VLANs
> + *
> + * By setting @NL80211_EXT_FEATURE_VLAN_OFFLOAD flag drivers can indicate they
> + * support offloading VLAN functionality in a manner where the driver exposes a
> + * single netdev that used VLAN tagged frames and separate VLAN-specific netdevs
> + * can then be added using vconfig similarly to the Ethernet case.
I don't think we should be referring to vconfig these days? It's pretty
much a deprecated userspace tool, the kernel would like to think that
all of this is done over netlink with iproute2 now :-)
But even then, it should probably just reference the kernel mechanisms
(creating a VLAN netdev) than the userspace implementation thereof
(vconfig or iproute2).
Something that's not quite clear to me - I think we support some frames
on the AP interface even when VLANs are in use. Would the tagged AP/VLAN
interface actually support untagged frames, and then in what way? I
think it'd be good to specify that here.
> + * %NL80211_CMD_NEW_KEY and %NL80211_CMD_SET_STATION will optionally specify
> + * vlan_id using NL80211_ATTR_VLAN_ID.
Guess that should be %NL80211_ATTR_VLAN_ID too.
> + [NL80211_ATTR_VLAN_ID] = { .type = NLA_U16 },
That should probably have a range, VLAN IDs can only be 1-4094 since
there's only a 12-bit field and all-0/all-1 are reserved.
Saves us from having to check it in the drivers later on, since they'd
otherwise generate invalid (or confusing) frames if there's no check.
> /* policy for the key attributes */
> @@ -3865,6 +3866,9 @@ static int nl80211_new_key(struct sk_buff *skb, struct genl_info *info)
> if (info->attrs[NL80211_ATTR_MAC])
> mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
>
> + if (info->attrs[NL80211_ATTR_VLAN_ID])
> + key.p.vlan_id = nla_get_u16(info->attrs[NL80211_ATTR_VLAN_ID]);
Seems like there probably should be some kind of check on what type of
key can set a VLAN ID?
> if (key.type == -1) {
> if (mac_addr)
> key.type = NL80211_KEYTYPE_PAIRWISE;
> @@ -5647,6 +5651,9 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
> if (info->attrs[NL80211_ATTR_STA_AID])
> params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
>
> + if (info->attrs[NL80211_ATTR_VLAN_ID])
> + params.vlan_id = nla_get_u16(info->attrs[NL80211_ATTR_VLAN_ID]);
How about nl80211_new_station()? Also, should it really be allowed to
change this at will? I'm not sure we allow changing the VLAN netdev (if
used) at will?
johannes
