From: Gurumoorthi Gnanasambandhan <[email protected]>
This provides an alternative mechanism for AP VLAN support where a
single netdev is used with VLAN tagged frames instead of separate
netdevs for each VLAN without tagged frames from the WLAN driver.
By setting NL80211_EXT_FEATURE_VLAN_OFFLOAD flag the driver indicates
support for a single netdev with VLAN tagged frames. Separate
VLAN-specific netdevs are added using vcongig similar to Ethernet.
NL80211_CMD_NEW_KEY and NL80211_CMD_SET_STATION will optionally specify
vlan_id using NL80211_ATTR_VLAN_ID.
Signed-off-by: Gurumoorthi Gnanasambandhan <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
include/net/cfg80211.h | 4 ++++
include/uapi/linux/nl80211.h | 20 ++++++++++++++++++++
net/wireless/nl80211.c | 7 +++++++
3 files changed, 31 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 35ec1f0a2bf9..3355f56ca0b8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -509,6 +509,7 @@ struct vif_params {
* with the get_key() callback, must be in little endian,
* length given by @seq_len.
* @seq_len: length of @seq.
+ * @vlan_id: vlan_id for VLAN group key (if nonzero)
* @mode: key install mode (RX_TX, NO_TX or SET_TX)
*/
struct key_params {
@@ -516,6 +517,7 @@ struct key_params {
const u8 *seq;
int key_len;
int seq_len;
+ u16 vlan_id;
u32 cipher;
enum nl80211_key_mode mode;
};
@@ -1046,6 +1048,7 @@ struct sta_txpwr {
* (bitmask of BIT(%NL80211_STA_FLAG_...))
* @listen_interval: listen interval or -1 for no change
* @aid: AID or zero for no change
+ * @vlan_id: VLAN ID for station (if nonzero)
* @peer_aid: mesh peer AID or zero for no change
* @plink_action: plink action to take
* @plink_state: set the peer link state for a station
@@ -1081,6 +1084,7 @@ struct station_parameters {
u32 sta_modify_mask;
int listen_interval;
u16 aid;
+ u16 vlan_id;
u16 peer_aid;
u8 supported_rates_len;
u8 plink_action;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 822851d369ab..0af5740648e8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -243,6 +243,17 @@
* %NL80211_ATTR_SAE_PASSWORD.
*/
+/**
+ * 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.
+ * %NL80211_CMD_NEW_KEY and %NL80211_CMD_SET_STATION will optionally specify
+ * vlan_id using NL80211_ATTR_VLAN_ID.
+ */
+
/**
* enum nl80211_commands - supported nl80211 commands
*
@@ -2361,6 +2372,8 @@ enum nl80211_commands {
* @NL80211_ATTR_HE_OBSS_PD: nested attribute for OBSS Packet Detection
* functionality.
*
+ * @NL80211_ATTR_VLAN_ID: VLAN ID for the station and VLAN group key (u16).
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2820,6 +2833,8 @@ enum nl80211_attrs {
NL80211_ATTR_HE_OBSS_PD,
+ NL80211_ATTR_VLAN_ID,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -5452,6 +5467,10 @@ enum nl80211_feature_flags {
* @NL80211_EXT_FEATURE_SAE_OFFLOAD: Device wants to do SAE authentication in
* station mode (SAE password is passed as part of the connect command).
*
+ * @NL80211_EXT_FEATURE_VLAN_OFFLOAD: The driver supports a single netdev
+ * with VLAN tagged frames and separate VLAN-specific netdevs added using
+ * vconfig similarly to the Ethernet case.
+ *
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
@@ -5497,6 +5516,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_EXT_KEY_ID,
NL80211_EXT_FEATURE_STA_TX_PWR,
NL80211_EXT_FEATURE_SAE_OFFLOAD,
+ NL80211_EXT_FEATURE_VLAN_OFFLOAD,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 1a107f29016b..860aec59003a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -584,6 +584,7 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
.len = SAE_PASSWORD_MAX_LEN },
[NL80211_ATTR_TWT_RESPONDER] = { .type = NLA_FLAG },
[NL80211_ATTR_HE_OBSS_PD] = NLA_POLICY_NESTED(he_obss_pd_policy),
+ [NL80211_ATTR_VLAN_ID] = { .type = NLA_U16 },
};
/* 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]);
+
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]);
+
if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
params.listen_interval =
nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
--
2.20.1
On 2019-08-15 06:38, Jouni Malinen wrote:
> From: Gurumoorthi Gnanasambandhan <[email protected]>
> [...]
> By setting NL80211_EXT_FEATURE_VLAN_OFFLOAD flag the driver indicates
> support for a single netdev with VLAN tagged frames. Separate
> VLAN-specific netdevs are added using vcongig similar to Ethernet.
nit: s/vcongig/vconfig/
On Thu, 2019-08-15 at 16:38 +0300, Jouni Malinen wrote:
> From: Gurumoorthi Gnanasambandhan <[email protected]>
>
> This provides an alternative mechanism for AP VLAN support where a
> single netdev is used with VLAN tagged frames instead of separate
> netdevs for each VLAN without tagged frames from the WLAN driver.
>
> By setting NL80211_EXT_FEATURE_VLAN_OFFLOAD flag the driver indicates
> support for a single netdev with VLAN tagged frames. Separate
> VLAN-specific netdevs are added using vcongig similar to Ethernet.
> NL80211_CMD_NEW_KEY and NL80211_CMD_SET_STATION will optionally specify
> vlan_id using NL80211_ATTR_VLAN_ID.
Without really looking at the specifics, it might be relatively simple
to support this in mac80211?
johannes
On Mon, Aug 19, 2019 at 12:16:03PM +0200, Johannes Berg wrote:
> On Thu, 2019-08-15 at 16:38 +0300, Jouni Malinen wrote:
> > From: Gurumoorthi Gnanasambandhan <[email protected]>
> >
> > This provides an alternative mechanism for AP VLAN support where a
> > single netdev is used with VLAN tagged frames instead of separate
> > netdevs for each VLAN without tagged frames from the WLAN driver.
> >
> > By setting NL80211_EXT_FEATURE_VLAN_OFFLOAD flag the driver indicates
> > support for a single netdev with VLAN tagged frames. Separate
> > VLAN-specific netdevs are added using vcongig similar to Ethernet.
> > NL80211_CMD_NEW_KEY and NL80211_CMD_SET_STATION will optionally specify
> > vlan_id using NL80211_ATTR_VLAN_ID.
>
> Without really looking at the specifics, it might be relatively simple
> to support this in mac80211?
Yes, that is something that I was thinking about when going through
this.. I don't remember why exactly mac80211 ended up with its current
design for per-AP_VLAN netdevs without tags, but it could indeed be
quite convenient to have this alternative approach available. I guess
both of them have some benefits, so this would likely be left with two
different mechanisms left to maintain, but the needed implementation in
mac80211 for this would seem to be pretty minimal (also without looking
at the exact details..).
--
Jouni Malinen PGP id EFC895FA
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
On Tue, 2019-08-20 at 23:50 +0300, Jouni Malinen wrote:
> > Without really looking at the specifics, it might be relatively simple
> > to support this in mac80211?
>
> Yes, that is something that I was thinking about when going through
> this.. I don't remember why exactly mac80211 ended up with its current
> design for per-AP_VLAN netdevs without tags, but it could indeed be
> quite convenient to have this alternative approach available.
Even just for testing it :-)
> I guess
> both of them have some benefits, so this would likely be left with two
> different mechanisms left to maintain,
Yes, unless we find a way to attach nl80211 to 802.1q interfaces so we
can pretend to do the netdevs ... not really worth it.
We could encapsulate the backward compatibility on top of this feature,
theoretically, by always asking the drivers to use this feature and then
implementing the AP_VLAN interfaces in cfg80211 entirely, but again,
probably not worth it since it's only mac80211, not a bunch of drivers.
> but the needed implementation in
> mac80211 for this would seem to be pretty minimal (also without looking
> at the exact details..).
Yeah, on RX the station lookup would just have to find the VLAN ID and
use __vlan_hwaccel_put_tag(); on TX ... not sure, probably
skb_vlan_tag_present()?
johannes