Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)

[Johannes Berg]

Hi,

> > To clarify, I think what you - conceptually - want is the following
> > topology:
> > 
> > +--- eth0.1  ---  br.1  ---  wlan0.1
> > |
> > eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
> > |
> > +--- eth0.3  ---  br.3  ---  wlan0.3
[...]
> That's right. In fact, hostapd is able to create this kind of network
> bridge infrastructure automatically when it is built
> with CONFIG_FULL_DYNAMIC_VLAN option enabled.

Cool, I was unaware of the exact functionality of this build-time
option. :)

> > Now, you seem to want to compress this to
> > 
> >   +---  wlan0.1
> >   |
> > eth0  ---  br  ---+---  wlan0.2
> >   |
> >   +---  wlan0.3

[...]

> Exactly. And yes, the only purpose of this 'non-conventional' mode
> was to have 802.1Q acceleration on the ethernet port.

Right. You can still have acceleration in the top picture by placing
the feature into the Ethernet hardware, so that tagging/untagging
doesn't have to touch the packet data but just touches (skb) metadata.
But obviously that's something that happens on the other side and you
don't have control over it.

Anyway, I'm happy we cleared up what was going on and also that you
decided to leave it for now and work with the regular Linux topology
model.

Thanks,
johannes

Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support

[Sergey Matyukevich]

Hi Johannes and all,

> > 1. Passing dynamic VLAN tag to firmware
> 
> There's no such thing as a "VLAN tag" with AP_VLAN interfaces. You want
> 802.1Q, which can be done on top of them if needed, but it's an
> unrelated concept.
> 
> > Currently there is no established way to pass VLAN tag assigned to
> > STA by Radius server to wireless driver. It is assumed that hostapd
> > is able to setup all the bridging and tagging on its own.
> 
> I don't even know if this is true? Does hostapd in fact set up 802.1Q
> tagging in any way? Can you say how to configure this?

Yes, hostapd supports this feature when it is built with option
CONFIG_FULL_DYNAMIC_VLAN enabled. There is a set of hostapd configuration
options and additional files that make it possible to specify 'WAN'
interface, naming conventions for those dynamic bridges, and more.

However I agree that using 802.1Q is only one of possible ways to isolate
stations attached to different AP_VLAN intefaces.

> > 2. Packet routing to/from AP/VLAN interfaces
> > Firmware operates with tagged packets after dynamic VLAN mode is
> > configured. In particular, packets destined to STAs should be
> > properly tagged before they can be passed to firmware. Packets
> > received from STAs are properly tagged by firmware and then
> > passed to wireless driver. As a result, packet routing to AP/VLAN
> > interfaces is straightforward: it is enough to check VLAN tags.
> 
> Ok, so here the whole tagging comes - a bit - into play. Technically
> you could, however, do the following:
> 
>  * assign "fake" tags as I suggested above
>  * pack/unpack the 802.1Q header from the firmware (or put it into
>metadata) in the driver and just tx/rx untagged packets into the
>right interface
>  * if the AP_VLAN has a real 802.1Q on top, then it's re-packed again
>by the ethernet driver when the data goes out

Yes, this is approach with 'fake' tags is how it can be done provided
we do not want to use any hacks with naming conventions for AP_VLAN
interfaces enforced by hostapd.

In fact, the 802.1Q approach has been chosen since in our case it is
possible to make use of 802.1Q acceleration in firmware. So we could
save some cycles on host CPU, tagging/untagging packets on the card.
But again, I agree that using 802.1Q is only one of the possible ways
to isolate stations attached to different AP_VLAN. Thus there is no
point to complicate/drop other usecases only because we are able
to support 802.1Q case better due to acceleration in firmware.

> > Normally hostapd expects untagged packets from AP/VLAN interfaces.
> 
> hostapd doesn't really expect anything there, does it?

In fact, hostapd does not expect 802.1Q tagged packets. IIRC when
CONFIG_FULL_DYNAMIC_VLAN enabled, then hostapd creates bridges
and VLAN interfaces dynamically, connecting them to 'WAN' network
interface specified by vlan_tagged_interface configuration option.

> > Meanwhile firmware performs tagging using h/w acceleration. That
> > is why it makes sense to avoid untagging packets in driver if
> > they are supposed to by tagged again on host. To enable this
> > behavior a new module parameter 'dyn_vlan_tagged' has been
> > introduced:
> >
> > - dyn_vlan_tagged = 0 (default)
> > In this case untagged packets are sent to and expected from AP/VLAN
> > interfaces.
> > Driver tags/untags packets going to/from firmware. This behaviour is
> > expected
> > by hostapd which is able to create bridges and VLAN interfaces
> > automatically
> > when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
> >
> > - dyn_vlan_tagged = 1
> > In this case tagged packets are sent to and expected from AP/VLAN
> > interfaces.
> > Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled.
> > Setup of
> > networking topology on host is left up to the implementers.
> 
> This is all very hacky, I really don't think we can accept that.

Fair enough. I didn't like it that much anyway :) I will rework the whole
patch using opaque tags for 802.1Q headers from the firmware, leaving AP_VLAN
interface management to hostapd or any other userspace tool.

Regards,
Sergey

Re: VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)

[Sergey Matyukevich]

Hi Johannes and all,

> > In a way this feature seems mis-designed - you never have 802.1Q tags
> > over the air, but you're inserting them on RX and stripping them on
> > TX, probably in order to make bridging to ethernet easier and not
> > have to have 802.1Q acceleration on the ethernet port, or - well - in
> > order to have an ability to do this with an ethernet card that only
> > has a single CPU port.
> 
> Ok this isn't really right either - it's only for saving the 802.1Q
> acceleration on the Ethernet port, really - and saving the extra
> bridges.
> 
> To clarify, I think what you - conceptually - want is the following
> topology:
> 
> +--- eth0.1  ---  br.1  ---  wlan0.1
> |
> eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
> |
> +--- eth0.3  ---  br.3  ---  wlan0.3
> 
> where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
> and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
> type interfaces that isolate the clients against each other as far as
> wifi is concerned.
> 
> Is this correct? As far as I understand, that's the baseline topology
> that you're trying to achieve, expressed in terms of Linux networking.

That's right. In fact, hostapd is able to create this kind of network
bridge infrastructure automatically when it is built
with CONFIG_FULL_DYNAMIC_VLAN option enabled.

> Now, you seem to want to compress this to
> 
>   +---  wlan0.1
>   |
> eth0  ---  br  ---+---  wlan0.2
>   |
>   +---  wlan0.3
> 
> and have the 802.1Q tag insertion/removal that's normally configured to
> happen in eth0.N already be handled in wlan0.N.
> 
> Also correct?

Exactly. And yes, the only purpose of this 'non-conventional' mode was
to have 802.1Q acceleration on the ethernet port.

> 
> 
> We clearly don't have APIs for this, and I don't think it makes sense
> in the Linux space - the bridge and wlan0.N suddenly have tagged
> traffic rather than untagged, and the VLAN tagging is completely hidden
> from the management view.
> 
> johannes

VLAN/bridge "compression" in wifi (was: Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support)

[Johannes Berg]

+netdev

On Tue, 2017-09-05 at 15:45 +0200, Johannes Berg wrote:
> 
> In a way this feature seems mis-designed - you never have 802.1Q tags
> over the air, but you're inserting them on RX and stripping them on
> TX, probably in order to make bridging to ethernet easier and not
> have to have 802.1Q acceleration on the ethernet port, or - well - in
> order to have an ability to do this with an ethernet card that only
> has a single CPU port.

Ok this isn't really right either - it's only for saving the 802.1Q
acceleration on the Ethernet port, really - and saving the extra
bridges.

To clarify, I think what you - conceptually - want is the following
topology:

+--- eth0.1  ---  br.1  ---  wlan0.1
|
eth0 ---+--- eth0.2  ---  br.2  ---  wlan0.2
| 
+--- eth0.3  ---  br.3  ---  wlan0.3

where eth0.N is just "ip link add link eth0 name eth0.N type vlan id N"
and br.N is obviously a bridge for each, and the wlan0.N are AP_VLAN
type interfaces that isolate the clients against each other as far as
wifi is concerned.

Is this correct? As far as I understand, that's the baseline topology
that you're trying to achieve, expressed in terms of Linux networking.

Now, you seem to want to compress this to

  +---  wlan0.1
  |
eth0  ---  br  ---+---  wlan0.2
  |
  +---  wlan0.3

and have the 802.1Q tag insertion/removal that's normally configured to
happen in eth0.N already be handled in wlan0.N.

Also correct?


We clearly don't have APIs for this, and I don't think it makes sense
in the Linux space - the bridge and wlan0.N suddenly have tagged
traffic rather than untagged, and the VLAN tagging is completely hidden
from the management view.

johannes

Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support

[Johannes Berg]

Hi Sergey, all,

On Tue, 2017-06-20 at 22:55 +0300, Sergey Matyukevich wrote:
> This patch implements AP_VLAN interface type support enabling
> the use of dynamic VLAN mode in hostapd.

My first thought here is that this is completely wrong.

AP_VLAN interfaces have no relation to 802.1Q, but it seems that you're
trying to use them as such.

I'll comment a bit below, but I'd like to ask you to actually explain
what you're trying to achieve, because then I think I can comment
better on what you need to be doing.

> Implementation notes.
> 
> 1. Passing dynamic VLAN tag to firmware

There's no such thing as a "VLAN tag" with AP_VLAN interfaces. You want
802.1Q, which can be done on top of them if needed, but it's an
unrelated concept.

> Currently there is no established way to pass VLAN tag assigned to
> STA by Radius server to wireless driver. It is assumed that hostapd
> is able to setup all the bridging and tagging on its own. 

I don't even know if this is true? Does hostapd in fact set up 802.1Q
tagging in any way? Can you say how to configure this?

This might actually be your problem - what you *want* is doing 802.1Q
tagging which hostapd doesn't support, and so you're actually doing
client isolation which hostapd *does* support, instead?

As far as I can tell hostapd only has the ability to put different
types of clients into different AP_VLANs to achieve *over the air*
isolation. It has no isolation over the backend (yet) - that's
typically set up by putting the AP_VLAN interfaces into different
bridges or whatever.


> However qtnf firmware needs to know the assigned dynamic VLAN tags in
> order to perform various internal tasks including group key
> management, filtering, acceleration, and more.

This doesn't make any sense - what does it need the *tag* for? I can
understand it needing *a* tag, but not *the* tag that the RADIUS server
gave it?

IOW - I can understand it needing an identifier of the VLAN or
something like that, but then why can't you just number the AP_VLAN
interfaces 1,2,3,4,...?

> Current implementation makes use of the following workaround.
[snip]

This really shouldn't be done.

> 2. Packet routing to/from AP/VLAN interfaces
> Firmware operates with tagged packets after dynamic VLAN mode is
> configured. In particular, packets destined to STAs should be
> properly tagged before they can be passed to firmware. Packets
> received from STAs are properly tagged by firmware and then
> passed to wireless driver. As a result, packet routing to AP/VLAN
> interfaces is straightforward: it is enough to check VLAN tags.

Ok, so here the whole tagging comes - a bit - into play. Technically
you could, however, do the following:

 * assign "fake" tags as I suggested above
 * pack/unpack the 802.1Q header from the firmware (or put it into
   metadata) in the driver and just tx/rx untagged packets into the
   right interface
 * if the AP_VLAN has a real 802.1Q on top, then it's re-packed again
   by the ethernet driver when the data goes out

> Normally hostapd expects untagged packets from AP/VLAN interfaces.

hostapd doesn't really expect anything there, does it?

> Meanwhile firmware performs tagging using h/w acceleration. That
> is why it makes sense to avoid untagging packets in driver if
> they are supposed to by tagged again on host. To enable this
> behavior a new module parameter 'dyn_vlan_tagged' has been
> introduced:
> 
> - dyn_vlan_tagged = 0 (default)
> In this case untagged packets are sent to and expected from AP/VLAN
> interfaces.
> Driver tags/untags packets going to/from firmware. This behaviour is
> expected
> by hostapd which is able to create bridges and VLAN interfaces
> automatically
> when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
> 
> - dyn_vlan_tagged = 1
> In this case tagged packets are sent to and expected from AP/VLAN
> interfaces.
> Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled.
> Setup of
> networking topology on host is left up to the implementers.

This is all very hacky, I really don't think we can accept that.

Firewalling and similar things will likely not work correctly if
there's an 802.1Q packet coming in that they don't expect, for example.
We don't want to go around all the Linux infrastructure for something
like this.

In a way this feature seems mis-designed - you never have 802.1Q tags
over the air, but you're inserting them on RX and stripping them on TX,
probably in order to make bridging to ethernet easier and not have to
have 802.1Q acceleration on the ethernet port, or - well - in order to
have an ability to do this with an ethernet card that only has a single
CPU port.


In a way this is more like a switch or bridge feature, but I don't
think we have any abstraction for this in Linux. I think this is
something we should discuss over on netdev and perhaps in person at the
wireless workshop/netdev conference?

johannes

Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support

[Sergey Matyukevich]

> Sergey Matyukevich  writes:
> 
> > This patch implements AP_VLAN interface type support enabling
> > the use of dynamic VLAN mode in hostapd.
> >
> > Implementation notes.

[...]

> > Signed-off-by: Sergey Matyukevich 
> 
> This looks somewhat controversial to me but I'm not very familiar with
> VLANs. Do other wireless drivers do something similar or how is this
> usually implemented? Or not at all?
> 
> --
> Kalle Valo

Well, IIUC this feature is not supported by other full-mac drivers at the
moment, so no good example to follow. I don't think it is too controversial.
Probably the restrictions on hostapd AP_VLAN interface names can be
frowned upon. But at the moment this information is not passed by hostapd
to kernel. It needs some additional work both in hostapd and cfg80211
to get this info directly in the driver.

Anyways, other patches in this series are more straighforward. So I will split
submission in v2: AP_VLAN support will be posted separately as RFC in order to
some feedback regarding the overall approach.

Regards,
Sergey

Re: [PATCH 3/8] qtnfmac: implement AP_VLAN iftype support

[Kalle Valo]

Sergey Matyukevich  writes:

> This patch implements AP_VLAN interface type support enabling
> the use of dynamic VLAN mode in hostapd.
>
> Implementation notes.
>
> 1. Passing dynamic VLAN tag to firmware
> Currently there is no established way to pass VLAN tag assigned to STA
> by Radius server to wireless driver. It is assumed that hostapd is able
> to setup all the bridging and tagging on its own. However qtnf firmware
> needs to know the assigned dynamic VLAN tags in order to perform various
> internal tasks including group key management, filtering, acceleration,
> and more.
>
> Current implementation makes use of the following workaround.
> Driver obtains dynamic VLAN tags assigned by Radius server
> from AP/VLAN interface names:
> - for primary interfaces: wlanX.Z
>   where X and Z are macid and vlan tag respectively
> - for MBSS virtual interfaces: wlanX_Y.Z
>   where X, Y, Z are macid, vifid, and vlan tag respectively
>
> Such a naming convention can be configured using
> hostapd vlan_file configuration file.
>
> 2. Packet routing to/from AP/VLAN interfaces
> Firmware operates with tagged packets after dynamic VLAN mode is
> configured. In particular, packets destined to STAs should be
> properly tagged before they can be passed to firmware. Packets
> received from STAs are properly tagged by firmware and then
> passed to wireless driver. As a result, packet routing to AP/VLAN
> interfaces is straightforward: it is enough to check VLAN tags.
>
> Normally hostapd expects untagged packets from AP/VLAN interfaces.
> Meanwhile firmware performs tagging using h/w acceleration. That
> is why it makes sense to avoid untagging packets in driver if
> they are supposed to by tagged again on host. To enable this
> behavior a new module parameter 'dyn_vlan_tagged' has been
> introduced:
>
> - dyn_vlan_tagged = 0 (default)
> In this case untagged packets are sent to and expected from AP/VLAN 
> interfaces.
> Driver tags/untags packets going to/from firmware. This behaviour is expected
> by hostapd which is able to create bridges and VLAN interfaces automatically
> when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.
>
> - dyn_vlan_tagged = 1
> In this case tagged packets are sent to and expected from AP/VLAN interfaces.
> Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled. Setup of
> networking topology on host is left up to the implementers.
>
> Signed-off-by: Sergey Matyukevich 

This looks somewhat controversial to me but I'm not very familiar with
VLANs. Do other wireless drivers do something similar or how is this
usually implemented? Or not at all?

-- 
Kalle Valo

[PATCH 3/8] qtnfmac: implement AP_VLAN iftype support

[Sergey Matyukevich]

This patch implements AP_VLAN interface type support enabling
the use of dynamic VLAN mode in hostapd.

Implementation notes.

1. Passing dynamic VLAN tag to firmware
Currently there is no established way to pass VLAN tag assigned to STA
by Radius server to wireless driver. It is assumed that hostapd is able
to setup all the bridging and tagging on its own. However qtnf firmware
needs to know the assigned dynamic VLAN tags in order to perform various
internal tasks including group key management, filtering, acceleration,
and more.

Current implementation makes use of the following workaround.
Driver obtains dynamic VLAN tags assigned by Radius server
from AP/VLAN interface names:
- for primary interfaces: wlanX.Z
  where X and Z are macid and vlan tag respectively
- for MBSS virtual interfaces: wlanX_Y.Z
  where X, Y, Z are macid, vifid, and vlan tag respectively

Such a naming convention can be configured using
hostapd vlan_file configuration file.

2. Packet routing to/from AP/VLAN interfaces
Firmware operates with tagged packets after dynamic VLAN mode is
configured. In particular, packets destined to STAs should be
properly tagged before they can be passed to firmware. Packets
received from STAs are properly tagged by firmware and then
passed to wireless driver. As a result, packet routing to AP/VLAN
interfaces is straightforward: it is enough to check VLAN tags.

Normally hostapd expects untagged packets from AP/VLAN interfaces.
Meanwhile firmware performs tagging using h/w acceleration. That
is why it makes sense to avoid untagging packets in driver if
they are supposed to by tagged again on host. To enable this
behavior a new module parameter 'dyn_vlan_tagged' has been
introduced:

- dyn_vlan_tagged = 0 (default)
In this case untagged packets are sent to and expected from AP/VLAN interfaces.
Driver tags/untags packets going to/from firmware. This behaviour is expected
by hostapd which is able to create bridges and VLAN interfaces automatically
when hostapd is built with CONFIG_FULL_DYNAMIC_VLAN option enabled.

- dyn_vlan_tagged = 1
In this case tagged packets are sent to and expected from AP/VLAN interfaces.
Hostapd build option CONFIG_FULL_DYNAMIC_VLAN should be disabled. Setup of
networking topology on host is left up to the implementers.

Signed-off-by: Sergey Matyukevich 
---
 drivers/net/wireless/quantenna/qtnfmac/bus.h   |   1 +
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c  | 247 ++---
 drivers/net/wireless/quantenna/qtnfmac/commands.c  |  28 ++-
 drivers/net/wireless/quantenna/qtnfmac/core.c  |  78 ++-
 drivers/net/wireless/quantenna/qtnfmac/core.h  |  25 ++-
 .../net/wireless/quantenna/qtnfmac/pearl/pcie.c|   5 +
 drivers/net/wireless/quantenna/qtnfmac/qlink.h |  10 +-
 .../net/wireless/quantenna/qtnfmac/qlink_util.c|   3 +
 drivers/net/wireless/quantenna/qtnfmac/util.c  |  74 --
 drivers/net/wireless/quantenna/qtnfmac/util.h  |  34 ++-
 10 files changed, 428 insertions(+), 77 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/bus.h 
b/drivers/net/wireless/quantenna/qtnfmac/bus.h
index dda05003d522..819ba3ba0f05 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/bus.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/bus.h
@@ -52,6 +52,7 @@ struct qtnf_bus {
enum qtnf_fw_state fw_state;
u32 chip;
u32 chiprev;
+   u8 dyn_vlan_tagged;
const struct qtnf_bus_ops *bus_ops;
struct qtnf_wmac *mac[QTNF_MAX_MAC];
struct qtnf_qlink_transport trans;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c 
b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 825a6334fbfe..e222e8d038d3 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -77,6 +77,35 @@ qtnf_mgmt_stypes[NUM_NL80211_IFTYPES] = {
},
 };
 
+static int qtnf_vlan_vif_exists(struct qtnf_vif *vif, u16 vlanid)
+{
+   struct qtnf_vif *vlan_vif;
+
+   vlan_vif = qtnf_vlan_list_lookup(>vlan_list, vlanid);
+   if (vlan_vif)
+   return 1;
+
+   return 0;
+}
+
+static struct qtnf_vif *qtnf_add_vlan_vif(struct qtnf_vif *vif, u16 vlanid)
+{
+   struct qtnf_vif *vlan_vif;
+
+   vlan_vif = qtnf_vlan_list_add(>vlan_list, vlanid);
+   if (vlan_vif) {
+   vlan_vif->u.vlan.parent = vif;
+   vlan_vif->u.vlan.vlanid = vlanid;
+   }
+
+   return vlan_vif;
+}
+
+static int qtnf_del_vlan_vif(struct qtnf_vif *vif, u16 vlanid)
+{
+   return qtnf_vlan_list_del(>vlan_list, vlanid);
+}
+
 static int
 qtnf_change_virtual_intf(struct wiphy *wiphy,
 struct net_device *dev,
@@ -92,7 +121,15 @@ qtnf_change_virtual_intf(struct wiphy *wiphy,
else
mac_addr = NULL;
 
-   qtnf_scan_done(vif->mac, true);
+   switch (type) {
+   case NL80211_IFTYPE_STATION:
+   case NL80211_IFTYPE_AP:
+