Re: [PATCH] ath10k: add dynamic vlan support
On Mon, 2018-04-23 at 21:18 +0200, Sebastian Gottschall wrote: > this patch makes no sense at some points. AP_VLAN must be enabled always > (it is enabled by mac80211 by default, but is now disabled in very > latest git version for drivers which announce sw_crypto support) > if its disabled wds ap / wds sta operation will not work anymore since > mac80211 uses AP_VLAN for the local wds sta interfaces You'd do you well to learn the correct terminology used in Linux if you try to communicate with us... What you say there makes no sense, WDS is a separate mode. Maybe you mean 4-addr mode? johannes
Re: [PATCH] ath10k: add dynamic vlan support
On Fri, 2018-05-04 at 12:20 +0530, Manikanta Pubbisetty wrote:
> Johannes,
>
> It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
> crypto controlled devices") has broken 4-addr operation on crypto
> controlled devices as reported by sebastian.
> The commit was mainly focused in addressing the problem in supporting
> VLANs on crypto controlled devices but since 4-addr mode is also
> dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
Ok.
Do you know why it actually broke it? I mean, we should've turned off
the strict requirement for sw crypto control only for the GTK, and that
shouldn't matter so much?
> I have couple of ideas on how to address the problem,
>
> 1) Add a new hw_flag and based on the hardware flag, allow/disallow the
> creation of AP_VLAN interface.
>
> + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
> + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
> + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
> + * operations in the BSS.
Based on the name and initial description, this sounds equivalent to
just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
would need some renaming.
> 2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
> crypto controlled devices, let the driver decide whether to return '1'
> or some error code based on their support for transmitting sw encrypted
> frames. I am little skeptical with this approach as drivers are totally
> unaware of AP_VLAN interfaces.
No, that won't work.
I'm unsure how 4-addr VLAN can work with ath10k either way?
Maybe it just doesn't normally need a GTK, so nothing broke before, but
your other patch changed things to remove VLAN and then of course it's
no longer available?
But then I don't understand the complaint that
So maybe the solution should be to add a separate flag for whether or
not 4-addr VLAN is supported?
johannes
Re: [PATCH] ath10k: add dynamic vlan support
Am 18.05.2018 um 11:53 schrieb Johannes Berg: On Mon, 2018-04-23 at 21:18 +0200, Sebastian Gottschall wrote: this patch makes no sense at some points. AP_VLAN must be enabled always (it is enabled by mac80211 by default, but is now disabled in very latest git version for drivers which announce sw_crypto support) if its disabled wds ap / wds sta operation will not work anymore since mac80211 uses AP_VLAN for the local wds sta interfaces You'd do you well to learn the correct terminology used in Linux if you try to communicate with us... What you say there makes no sense, WDS is a separate mode. Maybe you mean 4-addr mode? yes 4-addr mode which is common known as WDS mode. (terminology used by all sorts of vendors) Sebastian johannes
Re: [PATCH] ath10k: add dynamic vlan support
Am 18.05.2018 um 11:54 schrieb Johannes Berg:
On Fri, 2018-05-04 at 12:20 +0530, Manikanta Pubbisetty wrote:
Johannes,
It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
crypto controlled devices") has broken 4-addr operation on crypto
controlled devices as reported by sebastian.
The commit was mainly focused in addressing the problem in supporting
VLANs on crypto controlled devices but since 4-addr mode is also
dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
Ok.
Do you know why it actually broke it? I mean, we should've turned off
the strict requirement for sw crypto control only for the GTK, and that
shouldn't matter so much?
I have couple of ideas on how to address the problem,
1) Add a new hw_flag and based on the hardware flag, allow/disallow the
creation of AP_VLAN interface.
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
Based on the name and initial description, this sounds equivalent to
just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
would need some renaming.
2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
crypto controlled devices, let the driver decide whether to return '1'
or some error code based on their support for transmitting sw encrypted
frames. I am little skeptical with this approach as drivers are totally
unaware of AP_VLAN interfaces.
No, that won't work.
I'm unsure how 4-addr VLAN can work with ath10k either way?
Maybe it just doesn't normally need a GTK, so nothing broke before, but
your other patch changed things to remove VLAN and then of course it's
no longer available?
But then I don't understand the complaint that
So maybe the solution should be to add a separate flag for whether or
not 4-addr VLAN is supported?
johannes
let me explain. the vlan mode is used to create local interfaces in
4addr mode
like wlan0.sta0, wlan0.sta1 per peer. this is required to put these
peers into the local linux bridge since the local ap interface cannot
handle the bridging capabilities like correct forwarding, stp or even
filtering. this is a long term behaviour since the beginning of ath9k.
so the ap_vlan feature is used to pass the frames per peer in a
indepenened way. you may ask felix fietkau, since he developed it
originally in madwifi
and later in mac80211 / ath9k. so ap_vlan capability is a requiredment
for all 4addr capable wireless drivers.
example of a 4addr capable ap in ath10k with 2 connected 4addr stations
root@apreithalle:~# brctl show
bridge name bridge id STP enabled interfaces
br0 8000.dcef09e4ce07 no ath0
ath0.1
ath0.2
ath0.sta1
ath0.sta4
ath1
ath1.2
eth0
eth1
Re: [PATCH] ath10k: add dynamic vlan support
Do you know why it actually broke it? I mean, we should've turned off
the strict requirement for sw crypto control only for the GTK, and that
shouldn't matter so much?
With the change db3bdcb9c3ff, AP/VLAN support is advertised by the
driver conditionally; the primary reason for doing this is to support
VLAN operations on sw crypto controlled devices.
AP also creates AP/VLAN devices for supporting 4-addr clients and since
the driver now advertises AP/VLAN support conditionally, the 4-addr
operation which has no relation to the VLANs(Per VLAN GTKs) was broken
on some ath10k devices.
I have couple of ideas on how to address the problem,
1) Add a new hw_flag and based on the hardware flag, allow/disallow the
creation of AP_VLAN interface.
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
Based on the name and initial description, this sounds equivalent to
just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
would need some renaming.
I can rename it to something which is very specific to VLAN support on
sw crypto controlled devices if that is okay.
2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
crypto controlled devices, let the driver decide whether to return '1'
or some error code based on their support for transmitting sw encrypted
frames. I am little skeptical with this approach as drivers are totally
unaware of AP_VLAN interfaces.
No, that won't work.
I'm unsure how 4-addr VLAN can work with ath10k either way?
Maybe it just doesn't normally need a GTK, so nothing broke before, but
your other patch changed things to remove VLAN and then of course it's
no longer available?
But then I don't understand the complaint that
So maybe the solution should be to add a separate flag for whether or
not 4-addr VLAN is supported?
IIUC, the combination of 4-addr and VLAN doesn't work even without this
change db3bdcb9c3ff (" mac80211: allow AP_VLAN operation on crypto
controlled devices ").
AP/VLAN devices are used separately to either support 4-addr operation
or VLAN (separating the clients into VLAN groups each having unique GTK),
I believe both are mutually exclusive.
One reason why the combination of 4-addr+VLAN doesn't work could be that
a single AP/VLAN interface can cater to several wireless clients but to
support 4-addr operation every client device should have an exclusive
AP/VLAN interface.
It is possible where few clients in a specific VLAN group can use 4-addr
mode and few might not, if this is the case then AP has to create
individual AP/VLAN device for each 4-addr client and since these clients
belong to specific VLAN group, all of these clients should be tied
to AP/VLAN device created for VLAN operation(Created for maintaining
unique GTKs). I am not sure if the current stack supports this complex
combination, I could not make it work in my testing though.
Re: [PATCH] ath10k: add dynamic vlan support
On Mon, 2018-05-21 at 12:12 +0530, Manikanta Pubbisetty wrote: > > Do you know why it actually broke it? I mean, we should've turned off > > the strict requirement for sw crypto control only for the GTK, and that > > shouldn't matter so much? > > With the change db3bdcb9c3ff, AP/VLAN support is advertised by the > driver conditionally; the primary reason for doing this is to support > VLAN operations on sw crypto controlled devices. Right, or, well *not* supporting it. > AP also creates AP/VLAN devices for supporting 4-addr clients and since > the driver now advertises AP/VLAN support conditionally, the 4-addr > operation which has no relation to the VLANs(Per VLAN GTKs) was broken > on some ath10k devices. Right. Like I said, splitting those two capabilities somehow would be best. > > > + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting > > > + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL > > > + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN > > > + * operations in the BSS. > > > > Based on the name and initial description, this sounds equivalent to > > just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but > > would need some renaming. > > I can rename it to something which is very specific to VLAN support on > sw crypto controlled devices if that is okay. I don't think that makes sense. If we split the capability of AP_VLAN and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all these things. Yes, this is slightly awkward for userspace, and perhaps with the interface combination checks, but I'd like you to look at that. johannes
Re: [PATCH] ath10k: add dynamic vlan support
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN + * operations in the BSS. Based on the name and initial description, this sounds equivalent to just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but would need some renaming. I can rename it to something which is very specific to VLAN support on sw crypto controlled devices if that is okay. I don't think that makes sense. If we split the capability of AP_VLAN and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all these things. Yes, this is slightly awkward for userspace, and perhaps with the interface combination checks, but I'd like you to look at that. Makes sense, I had this thought to split the VLAN and 4-addr functionality, but since we need to fiddle with userspace, I refrained. Anyways, I will check all the possibilities of splitting these functionalities. Thanks, Manikanta
Re: [PATCH] ath10k: add dynamic vlan support
On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote: > > > > > + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of > > > > > transmitting > > > > > + * frames encrypted in software, only valid when > > > > > SW_CRYPTO_CONTROL > > > > > + * is enabled. Based on this flag, mac80211 can allow/disallow > > > > > VLAN > > > > > + * operations in the BSS. > > > > > > > > Based on the name and initial description, this sounds equivalent to > > > > just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but > > > > would need some renaming. > > > > > > I can rename it to something which is very specific to VLAN support on > > > sw crypto controlled devices if that is okay. > > > > I don't think that makes sense. If we split the capability of AP_VLAN > > and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all > > these things. Yes, this is slightly awkward for userspace, and perhaps > > with the interface combination checks, but I'd like you to look at that. > > > > Makes sense, I had this thought to split the VLAN and 4-addr > functionality, but since we need to fiddle with userspace, I refrained. > Anyways, I will check all the possibilities of splitting these > functionalities. I'm not sure we really have to - it's likely that wpa_s/hostapd don't check the capabilities? johannes
Re: [PATCH] ath10k: add dynamic vlan support
On 5/23/2018 4:09 PM, Johannes Berg wrote: On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote: + * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting + * frames encrypted in software, only valid when SW_CRYPTO_CONTROL + * is enabled. Based on this flag, mac80211 can allow/disallow VLAN + * operations in the BSS. Based on the name and initial description, this sounds equivalent to just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but would need some renaming. I can rename it to something which is very specific to VLAN support on sw crypto controlled devices if that is okay. I don't think that makes sense. If we split the capability of AP_VLAN and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all these things. Yes, this is slightly awkward for userspace, and perhaps with the interface combination checks, but I'd like you to look at that. Makes sense, I had this thought to split the VLAN and 4-addr functionality, but since we need to fiddle with userspace, I refrained. Anyways, I will check all the possibilities of splitting these functionalities. I'm not sure we really have to - it's likely that wpa_s/hostapd don't check the capabilities? johannes IIUC, hostapd will just invoke a NL command to create the AP/VLAN interface, the capabilities are checked mostly in cfg80211. If we are planning to split the functionality, possibly something like having two separate IF-TYPES(one for VLAN and one for 4-addr) instead of one(which is the case now), we should probably change the relevant areas in hostapd as well, not really sure of the complexity of the work involved in userspace though.
Re: [PATCH] ath10k: add dynamic vlan support
Am 23.05.2018 um 12:39 schrieb Johannes Berg:
On Wed, 2018-05-23 at 16:09 +0530, Manikanta Pubbisetty wrote:
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
Based on the name and initial description, this sounds equivalent to
just turning off SW_CRYPTO_CONTROL. I think that's not the intent, but
would need some renaming.
I can rename it to something which is very specific to VLAN support on
sw crypto controlled devices if that is okay.
I don't think that makes sense. If we split the capability of AP_VLAN
and AP_VLAN_FOR_4ADDR at the "root", then we don't need to play with all
these things. Yes, this is slightly awkward for userspace, and perhaps
with the interface combination checks, but I'd like you to look at that.
Makes sense, I had this thought to split the VLAN and 4-addr
functionality, but since we need to fiddle with userspace, I refrained.
Anyways, I will check all the possibilities of splitting these
functionalities.
I'm not sure we really have to - it's likely that wpa_s/hostapd don't
check the capabilities?
mmh not sure. it might not check the capabilitiy, but it sets the
interface type to IFTYPE_AP_VLAN
for wds sta interfaces. that might collide
see the following snippet taken from hostapd code
static int i802_set_wds_sta(void *priv, const u8 *addr, int aid, int val,
const char *bridge_ifname, char *ifname_wds)
{
struct i802_bss *bss = priv;
struct wpa_driver_nl80211_data *drv = bss->drv;
char name[IFNAMSIZ + 1];
os_snprintf(name, sizeof(name), "%s.sta%d", bss->ifname, aid);
if (ifname_wds)
os_strlcpy(ifname_wds, name, IFNAMSIZ + 1);
wpa_printf(MSG_DEBUG, "nl80211: Set WDS STA addr=" MACSTR
" aid=%d val=%d name=%s", MAC2STR(addr), aid, val,
name);
if (val) {
if (!if_nametoindex(name)) {
if (nl80211_create_iface(drv, name,
NL80211_IFTYPE_AP_VLAN,
bss->addr, 1, NULL,
NULL, 0) <
0)
return -1;
if (bridge_ifname &&
linux_br_add_if(drv->global->ioctl_sock,
bridge_ifname, name) < 0)
return -1;
}
if (linux_set_iface_flags(drv->global->ioctl_sock,
name, 1)) {
wpa_printf(MSG_ERROR, "nl80211: Failed to set
WDS STA "
"interface %s up", name);
}
return i802_set_sta_vlan(priv, addr, name, 0);
} else {
if (bridge_ifname)
linux_br_del_if(drv->global->ioctl_sock, bridge_ifname,
name);
i802_set_sta_vlan(priv, addr, bss->ifname, 0);
nl80211_remove_iface(drv, if_nametoindex(name));
return 0;
}
}
johannes
Re: [PATCH] ath10k: add dynamic vlan support
this patch makes no sense at some points. AP_VLAN must be enabled always
(it is enabled by mac80211 by default, but is now disabled in very
latest git version for drivers which announce sw_crypto support)
if its disabled wds ap / wds sta operation will not work anymore since
mac80211 uses AP_VLAN for the local wds sta interfaces
Sebastian
Am 20.04.2018 um 15:57 schrieb Manikanta Pubbisetty:
Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software encrypted packets.
In ath10k, sending sw encrypted frames is allowed only when we insmod
the driver with cryptmode param set to 1, this configuration disables
hardware crypto and enables RAW mode implicitly. Since, enabling raw
mode has performance impact, this cannot be considered as an ideal
solution for supporting VLANs in the driver.
As an alternative take, in this approach, cryptographic keys for
unicast traffic(per peer PTKs) and keys for non-vlan group traffic
will be configured in hardware, allowing hardware encryption for unicast
and non-vlan group traffic. Only vlan group traffic will be encrypted in
software and pushed to the target with encap mode set to RAW in the TX
descriptors.
Not all firmwares can support this type of key configuration(having few
keys installed in hardware and few only in software); for this purpose a
new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
advertise this support.
Also, adding the logic required to send sw encrypted frames in raw mode.
Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
Signed-off-by: Manikanta Pubbisetty
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 26 --
drivers/net/wireless/ath/ath10k/wmi.h | 21 +
3 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index e4ac8f2..105438d 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -122,6 +122,7 @@ enum ath10k_skb_flags {
ATH10K_SKB_F_DELIVER_CAB = BIT(2),
ATH10K_SKB_F_MGMT = BIT(3),
ATH10K_SKB_F_QOS = BIT(4),
+ ATH10K_SKB_F_RAW_TX = BIT(5),
};
struct ath10k_skb_cb {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index fc3320f..694c0aa 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3362,6 +3362,7 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
struct sk_buff *skb)
{
const struct ieee80211_hdr *hdr = (void *)skb->data;
+ const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
__le16 fc = hdr->frame_control;
if (!vif || vif->type == NL80211_IFTYPE_MONITOR)
@@ -3403,7 +3404,8 @@ ath10k_mac_tx_h_get_txmode(struct ath10k *ar,
if (ieee80211_is_data_present(fc) && sta && sta->tdls)
return ATH10K_HW_TXRX_ETHERNET;
- if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags))
+ if (test_bit(ATH10K_FLAG_RAW_MODE, &ar->dev_flags) ||
+ skb_cb->flags & ATH10K_SKB_F_RAW_TX)
return ATH10K_HW_TXRX_RAW;
return ATH10K_HW_TXRX_NATIVE_WIFI;
@@ -3513,6 +3515,9 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
{
struct ieee80211_hdr *hdr = (void *)skb->data;
struct ath10k_skb_cb *cb = ATH10K_SKB_CB(skb);
+ const struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ bool is_data = ieee80211_is_data(hdr->frame_control) ||
+ ieee80211_is_data_qos(hdr->frame_control);
cb->flags = 0;
if (!ath10k_tx_h_use_hwcrypto(vif, skb))
@@ -3524,6 +3529,16 @@ static void ath10k_mac_tx_h_fill_cb(struct ath10k *ar,
if (ieee80211_is_data_qos(hdr->frame_control))
cb->flags |= ATH10K_SKB_F_QOS;
+ /* Data frames encrypted in software will be posted to firmware
+* with tx encap mode set to RAW. One such case would be the
+* multicast traffic generated for a VLAN group.
+*/
+ if (is_data && ieee80211_has_protected(hdr->frame_control) &&
+ !info->control.hw_key) {
+ cb->flags |= ATH10K_SKB_F_NO_HWCRYPT;
+ cb->flags |= ATH10K_SKB_F_RAW_TX;
+ }
+
cb->vif = vif;
cb->txq = txq;
}
@@ -3632,6 +3647,7 @@ static int ath10k_mac_tx(struct ath10k *ar,
{
struct ieee80211_hw *hw = ar->hw;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ const struct ath10k_skb_cb *skb_cb = ATH10K_SKB_CB(skb);
int ret;
/* We should disable CCK RATE due to P2P */
@@ -3649,7 +3665,8 @@ static int ath10k_mac_tx(struct ath10k *ar,
ath10k_tx_h_8023(skb);
break;
case ATH10K_HW_TXRX_RAW:
- if (!test
Re: [PATCH] ath10k: add dynamic vlan support
Manikanta Pubbisetty writes: > Mutlicast/broadcast traffic destined for a particular vlan group will > always be encrypted in software. To enable dynamic VLANs, it requires > driver support for sending software encrypted packets. > > In ath10k, sending sw encrypted frames is allowed only when we insmod > the driver with cryptmode param set to 1, this configuration disables > hardware crypto and enables RAW mode implicitly. Since, enabling raw > mode has performance impact, this cannot be considered as an ideal > solution for supporting VLANs in the driver. > > As an alternative take, in this approach, cryptographic keys for > unicast traffic(per peer PTKs) and keys for non-vlan group traffic > will be configured in hardware, allowing hardware encryption for unicast > and non-vlan group traffic. Only vlan group traffic will be encrypted in > software and pushed to the target with encap mode set to RAW in the TX > descriptors. > > Not all firmwares can support this type of key configuration(having few > keys installed in hardware and few only in software); for this purpose a > new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to > advertise this support. > > Also, adding the logic required to send sw encrypted frames in raw mode. > > Tested this change on QCA9984(firmware version 10.4-3.5.3-00057). > > Signed-off-by: Manikanta Pubbisetty Your name in patchwork is wrong and hence my script uses the wrong name. Please fix it by registering to patchwork[1] where it's possible to change your name during registration, but only one time. If that doesn't work then send a request to helpd...@kernel.org and the admins can fix it. [1] https://patchwork.kernel.org/register/ -- Kalle Valo
Re: [PATCH] ath10k: add dynamic vlan support
consider my comment regarding vlan_ap.
this patch will break wds ap / wds sta support with latest mac80211 (see
also my post on the wireless mailing list about the breaking patch in
mac80211)
so AP_VLAN must be masked always for all chipsets. otherwise wds breaks
and this is not just a guess. i tested it yesterday using this patch and
found
the cause of the issue
the following lines
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT, ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
must be just
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
everthing else will cause a regression
Am 24.04.2018 um 10:09 schrieb Kalle Valo:
Manikanta Pubbisetty writes:
Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software encrypted packets.
In ath10k, sending sw encrypted frames is allowed only when we insmod
the driver with cryptmode param set to 1, this configuration disables
hardware crypto and enables RAW mode implicitly. Since, enabling raw
mode has performance impact, this cannot be considered as an ideal
solution for supporting VLANs in the driver.
As an alternative take, in this approach, cryptographic keys for
unicast traffic(per peer PTKs) and keys for non-vlan group traffic
will be configured in hardware, allowing hardware encryption for unicast
and non-vlan group traffic. Only vlan group traffic will be encrypted in
software and pushed to the target with encap mode set to RAW in the TX
descriptors.
Not all firmwares can support this type of key configuration(having few
keys installed in hardware and few only in software); for this purpose a
new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is introduced to
advertise this support.
Also, adding the logic required to send sw encrypted frames in raw mode.
Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
Signed-off-by: Manikanta Pubbisetty
Your name in patchwork is wrong and hence my script uses the wrong
name. Please fix it by registering to patchwork[1] where it's possible
to change your name during registration, but only one time. If that
doesn't work then send a request to helpd...@kernel.org and the admins
can fix it.
[1] https://patchwork.kernel.org/register/
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565
Re: [PATCH] ath10k: add dynamic vlan support
Yes Sebastian, Your point is valid. This would break the 4-addr
operation on other ath10k devices which does not support the new WMI
service.
I have another approach to solve this problem, I will come with a small
patch and see what johannes has to say on that approach.
Manikanta
On 4/24/2018 2:39 PM, Sebastian Gottschall wrote:
consider my comment regarding vlan_ap.
this patch will break wds ap / wds sta support with latest mac80211
(see also my post on the wireless mailing list about the breaking
patch in mac80211)
so AP_VLAN must be masked always for all chipsets. otherwise wds
breaks and this is not just a guess. i tested it yesterday using this
patch and found
the cause of the issue
the following lines
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
must be just
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
everthing else will cause a regression
Am 24.04.2018 um 10:09 schrieb Kalle Valo:
Manikanta Pubbisetty writes:
Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software encrypted packets.
In ath10k, sending sw encrypted frames is allowed only when we insmod
the driver with cryptmode param set to 1, this configuration disables
hardware crypto and enables RAW mode implicitly. Since, enabling raw
mode has performance impact, this cannot be considered as an ideal
solution for supporting VLANs in the driver.
As an alternative take, in this approach, cryptographic keys for
unicast traffic(per peer PTKs) and keys for non-vlan group traffic
will be configured in hardware, allowing hardware encryption for
unicast
and non-vlan group traffic. Only vlan group traffic will be
encrypted in
software and pushed to the target with encap mode set to RAW in the TX
descriptors.
Not all firmwares can support this type of key configuration(having few
keys installed in hardware and few only in software); for this
purpose a
new WMI service flag "WMI_SERVICE_PER_PACKET_SW_ENCRYPT" is
introduced to
advertise this support.
Also, adding the logic required to send sw encrypted frames in raw
mode.
Tested this change on QCA9984(firmware version 10.4-3.5.3-00057).
Signed-off-by: Manikanta Pubbisetty
Your name in patchwork is wrong and hence my script uses the wrong
name. Please fix it by registering to patchwork[1] where it's possible
to change your name during registration, but only one time. If that
doesn't work then send a request to helpd...@kernel.org and the admins
can fix it.
[1] https://patchwork.kernel.org/register/
Re: [PATCH] ath10k: add dynamic vlan support
Sebastian Gottschall writes: > consider my comment regarding vlan_ap. I am considering comment, I just go one issue at a time and haven't had a time to look at your comment. But PLEASE do not top post, it's very annoying and creates a mess in patchwork. https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#do_not_top_post_and_edit_your_quotes -- Kalle Valo
Re: [PATCH] ath10k: add dynamic vlan support
Am 24.04.2018 um 11:52 schrieb Kalle Valo: Sebastian Gottschall writes: consider my comment regarding vlan_ap. I am considering comment, I just go one issue at a time and haven't had a time to look at your comment. But PLEASE do not top post, it's very annoying and creates a mess in patchwork. https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#do_not_top_post_and_edit_your_quotes i almost do top posts for a single reason. you have to scroll down a long time sometimes to get the essential information. i dont know why most people in my country prefer top posting. i will try to remember and change it in future -- Mit freundlichen Grüssen / Regards Sebastian Gottschall / CTO NewMedia-NET GmbH - DD-WRT Firmensitz: Stubenwaldallee 21a, 64625 Bensheim Registergericht: Amtsgericht Darmstadt, HRB 25473 Geschäftsführer: Peter Steinhäuser, Christian Scheele http://www.dd-wrt.com email: s.gottsch...@dd-wrt.com Tel.: +496251-582650 / Fax: +496251-5826565
Re: [PATCH] ath10k: add dynamic vlan support
Johannes,
It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on
crypto controlled devices") has broken 4-addr operation on crypto
controlled devices as reported by sebastian.
The commit was mainly focused in addressing the problem in supporting
VLANs on crypto controlled devices but since 4-addr mode is also
dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
I have couple of ideas on how to address the problem,
1) Add a new hw_flag and based on the hardware flag, allow/disallow the
creation of AP_VLAN interface.
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d2279b2..301d9c38 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2084,6 +2084,11 @@ struct ieee80211_txq {
* @IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP: The driver (or firmware)
doesn't
* support QoS NDP for AP probing - that's most likely a driver
bug.
*
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing
arrays
*/
enum ieee80211_hw_flags {
@@ -2129,6 +2134,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_TDLS_BUFFER_STA,
IEEE80211_HW_DEAUTH_NEED_MGD_TX_PREP,
IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP,
+ IEEE80211_HW_SUPPORTS_SW_ENCRYPT,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 555e389..c825d27 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1736,6 +1736,11 @@ int ieee80211_if_add(struct ieee80211_local
*local, const char *name,
ASSERT_RTNL();
+ if ((type == NL80211_IFTYPE_AP_VLAN) && !params->use_4addr &&
+ ieee80211_hw_check(local->hw, SW_CRYPTO_CONTROL) &&
+ !ieee80211_hw_check(local->hw, SUPPORTS_SW_ENCRYPT))
+ return -EOPNOTSUPP;
+
if (type == NL80211_IFTYPE_P2P_DEVICE || type ==
NL80211_IFTYPE_NAN) {
struct wireless_dev *wdev;
2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
crypto controlled devices, let the driver decide whether to return '1'
or some error code based on their support for transmitting sw encrypted
frames. I am little skeptical with this approach as drivers are totally
unaware of AP_VLAN interfaces.
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index ee0d0cc..0ff5597 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -167,7 +167,8 @@ static int ieee80211_key_enable_hw_accel(struct
ieee80211_key *key)
* The driver doesn't know anything about VLAN
interfaces.
* Hence, don't send GTKs for VLAN interfaces to the
driver.
*/
- if (!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE))
+ if ((!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE) &&
+ !ieee80211_hw_check(&key->local->hw,
SW_CRYPTO_CONTROL)))
goto out_unsupported;
}
Please let me know which is the better way to deal the problem.
Thanks,
Manikanta
On 2018-04-24 14:39, Sebastian Gottschall wrote:
consider my comment regarding vlan_ap.
this patch will break wds ap / wds sta support with latest mac80211
(see also my post on the wireless mailing list about the breaking
patch in mac80211)
so AP_VLAN must be masked always for all chipsets. otherwise wds
breaks and this is not just a guess. i tested it yesterday using this
patch and found
the cause of the issue
the following lines
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |=
BIT(NL80211_IFTYPE_AP_VLAN);
+ }
must be just
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |=
BIT(NL80211_IFTYPE_AP_VLAN);
everthing else will cause a regression
Am 24.04.2018 um 10:09 schrieb Kalle Valo:
Manikanta Pubbisetty writes:
Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software encrypted packets.
In ath10k, sending sw encrypted frames is allowed only when we insmod
the driver with cryptmode param set to 1, this configuration disables
hardware crypto and enables RAW mode implicitly. Since, enabling raw
mode has performance impact, this cannot be considered as an ideal
solution for supporting VLANs in the driver.
As an alternative take, in this approach, cryptographic keys for
unicast traffic(per peer PTKs) and keys for non-vlan group traffic
will be configured in hardware, allowing hardware encryption for
unicast
and non-vlan group traffic. Only vlan gr
Re: [PATCH] ath10k: add dynamic vlan support
did someone notice that GTK rekeying is broken in 4addr mode for 10.4.
firmwares since a long time.
i have a test with 2 9984 devices. one is 4addr ap, the second 4addr
sta. it disconnects on rekeying and reauthenticates. (reproducable with
99x0 as well)
this does not occur on 10.2 based chipsets like 988x. it seems to be a
internal firmware problem since the beginning. i wrote already a long
time ago about it,
but no solution was provided. maybe its also finally time to take care
about this issue (cause unknown)
Sebastian
Am 04.05.2018 um 08:50 schrieb Manikanta Pubbisetty:
Johannes,
It seems like commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation
on crypto controlled devices") has broken 4-addr operation on crypto
controlled devices as reported by sebastian.
The commit was mainly focused in addressing the problem in supporting
VLANs on crypto controlled devices but since 4-addr mode is also
dependent on AP_VLAN interface, this commit breaks the 4-addr AP mode.
I have couple of ideas on how to address the problem,
1) Add a new hw_flag and based on the hardware flag, allow/disallow
the creation of AP_VLAN interface.
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d2279b2..301d9c38 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2084,6 +2084,11 @@ struct ieee80211_txq {
* @IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP: The driver (or firmware)
doesn't
* support QoS NDP for AP probing - that's most likely a driver bug.
*
+ * @IEEE80211_HW_SUPPORTS_SW_ENCRYPT: Device is capable of transmitting
+ * frames encrypted in software, only valid when SW_CRYPTO_CONTROL
+ * is enabled. Based on this flag, mac80211 can allow/disallow VLAN
+ * operations in the BSS.
+ *
* @NUM_IEEE80211_HW_FLAGS: number of hardware flags, used for sizing
arrays
*/
enum ieee80211_hw_flags {
@@ -2129,6 +2134,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_TDLS_BUFFER_STA,
IEEE80211_HW_DEAUTH_NEED_MGD_TX_PREP,
IEEE80211_HW_DOESNT_SUPPORT_QOS_NDP,
+ IEEE80211_HW_SUPPORTS_SW_ENCRYPT,
/* keep last, obviously */
NUM_IEEE80211_HW_FLAGS
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 555e389..c825d27 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1736,6 +1736,11 @@ int ieee80211_if_add(struct ieee80211_local
*local, const char *name,
ASSERT_RTNL();
+ if ((type == NL80211_IFTYPE_AP_VLAN) && !params->use_4addr &&
+ ieee80211_hw_check(local->hw, SW_CRYPTO_CONTROL) &&
+ !ieee80211_hw_check(local->hw, SUPPORTS_SW_ENCRYPT))
+ return -EOPNOTSUPP;
+
if (type == NL80211_IFTYPE_P2P_DEVICE || type ==
NL80211_IFTYPE_NAN) {
struct wireless_dev *wdev;
2) Allow mac80211 to call set_key for GTKs on AP_VLAN interfaces for
crypto controlled devices, let the driver decide whether to return '1'
or some error code based on their support for transmitting sw
encrypted frames. I am little skeptical with this approach as drivers
are totally unaware of AP_VLAN interfaces.
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index ee0d0cc..0ff5597 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -167,7 +167,8 @@ static int ieee80211_key_enable_hw_accel(struct
ieee80211_key *key)
* The driver doesn't know anything about VLAN
interfaces.
* Hence, don't send GTKs for VLAN interfaces to the
driver.
*/
- if (!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE))
+ if ((!(key->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE) &&
+ !ieee80211_hw_check(&key->local->hw,
SW_CRYPTO_CONTROL)))
goto out_unsupported;
}
Please let me know which is the better way to deal the problem.
Thanks,
Manikanta
On 2018-04-24 14:39, Sebastian Gottschall wrote:
consider my comment regarding vlan_ap.
this patch will break wds ap / wds sta support with latest mac80211
(see also my post on the wireless mailing list about the breaking
patch in mac80211)
so AP_VLAN must be masked always for all chipsets. otherwise wds
breaks and this is not just a guess. i tested it yesterday using this
patch and found
the cause of the issue
the following lines
+ if (test_bit(WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
ar->wmi.svc_map)) {
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ }
must be just
+ ar->hw->wiphy->interface_modes |= BIT(NL80211_IFTYPE_AP_VLAN);
+ ar->hw->wiphy->software_iftypes |= BIT(NL80211_IFTYPE_AP_VLAN);
everthing else will cause a regression
Am 24.04.2018 um 10:09 schrieb Kalle Valo:
Manikanta Pubbisetty writes:
Mutlicast/broadcast traffic destined for a particular vlan group will
always be encrypted in software. To enable dynamic VLANs, it requires
driver support for sending software en
