[PATCH] ath10k: add dynamic vlan support

[PATCH] ath10k: add dynamic vlan support

[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 <mpubbise@codeaurora.org>
---
 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_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)) {
 			WARN_ON_ONCE(1);
 			ieee80211_free_txskb(hw, skb);
 			return -ENOTSUPP;
@@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
 		goto err_dfs_detector_exit;
 	}
 
+	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);
+	}
+
 	if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
 		ret = regulatory_hint(ar->hw->wiphy,
 				      ar->ath_common.regulatory.alpha2);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 3cc129d..e359b6af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -202,6 +202,10 @@ enum wmi_service {
 	WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_SERVICE_TPC_STATS_FINAL,
 	WMI_SERVICE_RESET_CHIP,
+	WMI_SERVICE_CFR_CAPTURE_SUPPORT,
+	WMI_SERVICE_TX_DATA_ACK_RSSI,
+	WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
 
 	/* keep last */
 	WMI_SERVICE_MAX,
@@ -349,6 +353,10 @@ enum wmi_10_4_service {
 	WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
 	WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_10_4_SERVICE_TPC_STATS_FINAL,
+	WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+	WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+	WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
 };
 
 static inline char *wmi_service_name(int service_id)
@@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
 	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
 	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
+	SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
+	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
+	SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
+	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
+
 	default:
 		return NULL;
 	}
@@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
 	       WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
 	SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
 	       WMI_SERVICE_TPC_STATS_FINAL, len);
+	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+	       WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
+	SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+	       WMI_SERVICE_TX_DATA_ACK_RSSI, len);
+	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	       WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
+	SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
+	       WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
 }
 
 #undef SVCMAP
-- 
2.7.4

[PATCH] ath10k: add dynamic vlan support

[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 <mpubbise@codeaurora.org>
---
 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_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)) {
 			WARN_ON_ONCE(1);
 			ieee80211_free_txskb(hw, skb);
 			return -ENOTSUPP;
@@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
 		goto err_dfs_detector_exit;
 	}
 
+	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);
+	}
+
 	if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
 		ret = regulatory_hint(ar->hw->wiphy,
 				      ar->ath_common.regulatory.alpha2);
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 3cc129d..e359b6af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -202,6 +202,10 @@ enum wmi_service {
 	WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_SERVICE_TPC_STATS_FINAL,
 	WMI_SERVICE_RESET_CHIP,
+	WMI_SERVICE_CFR_CAPTURE_SUPPORT,
+	WMI_SERVICE_TX_DATA_ACK_RSSI,
+	WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
 
 	/* keep last */
 	WMI_SERVICE_MAX,
@@ -349,6 +353,10 @@ enum wmi_10_4_service {
 	WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
 	WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
 	WMI_10_4_SERVICE_TPC_STATS_FINAL,
+	WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+	WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+	WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
 };
 
 static inline char *wmi_service_name(int service_id)
@@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
 	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
 	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
 	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
+	SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
+	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
+	SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
+	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
+
 	default:
 		return NULL;
 	}
@@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
 	       WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
 	SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
 	       WMI_SERVICE_TPC_STATS_FINAL, len);
+	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
+	       WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
+	SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
+	       WMI_SERVICE_TX_DATA_ACK_RSSI, len);
+	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
+	       WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
+	SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
+	       WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
 }
 
 #undef SVCMAP
-- 
2.7.4


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org>
> ---
>   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_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)) {
>   			WARN_ON_ONCE(1);
>   			ieee80211_free_txskb(hw, skb);
>   			return -ENOTSUPP;
> @@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
>   		goto err_dfs_detector_exit;
>   	}
>   
> +	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);
> +	}
> +
>   	if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
>   		ret = regulatory_hint(ar->hw->wiphy,
>   				      ar->ath_common.regulatory.alpha2);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> index 3cc129d..e359b6af 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -202,6 +202,10 @@ enum wmi_service {
>   	WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
>   	WMI_SERVICE_TPC_STATS_FINAL,
>   	WMI_SERVICE_RESET_CHIP,
> +	WMI_SERVICE_CFR_CAPTURE_SUPPORT,
> +	WMI_SERVICE_TX_DATA_ACK_RSSI,
> +	WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
>   
>   	/* keep last */
>   	WMI_SERVICE_MAX,
> @@ -349,6 +353,10 @@ enum wmi_10_4_service {
>   	WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
>   	WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
>   	WMI_10_4_SERVICE_TPC_STATS_FINAL,
> +	WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> +	WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> +	WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
>   };
>   
>   static inline char *wmi_service_name(int service_id)
> @@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
>   	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
>   	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
>   	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
> +	SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
> +	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
> +	SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
> +	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
> +
>   	default:
>   		return NULL;
>   	}
> @@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
>   	       WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
>   	SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
>   	       WMI_SERVICE_TPC_STATS_FINAL, len);
> +	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> +	       WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
> +	SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> +	       WMI_SERVICE_TX_DATA_ACK_RSSI, len);
> +	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	       WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
> +	SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
> +	       WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
>   }
>   
>   #undef SVCMAP


-- 
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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org>
> ---
>   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_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)) {
>   			WARN_ON_ONCE(1);
>   			ieee80211_free_txskb(hw, skb);
>   			return -ENOTSUPP;
> @@ -8455,6 +8472,11 @@ int ath10k_mac_register(struct ath10k *ar)
>   		goto err_dfs_detector_exit;
>   	}
>   
> +	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);
> +	}
> +
>   	if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
>   		ret = regulatory_hint(ar->hw->wiphy,
>   				      ar->ath_common.regulatory.alpha2);
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
> index 3cc129d..e359b6af 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi.h
> @@ -202,6 +202,10 @@ enum wmi_service {
>   	WMI_SERVICE_HOST_DFS_CHECK_SUPPORT,
>   	WMI_SERVICE_TPC_STATS_FINAL,
>   	WMI_SERVICE_RESET_CHIP,
> +	WMI_SERVICE_CFR_CAPTURE_SUPPORT,
> +	WMI_SERVICE_TX_DATA_ACK_RSSI,
> +	WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	WMI_SERVICE_PER_PACKET_SW_ENCRYPT,
>   
>   	/* keep last */
>   	WMI_SERVICE_MAX,
> @@ -349,6 +353,10 @@ enum wmi_10_4_service {
>   	WMI_10_4_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS,
>   	WMI_10_4_SERVICE_HOST_DFS_CHECK_SUPPORT,
>   	WMI_10_4_SERVICE_TPC_STATS_FINAL,
> +	WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> +	WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> +	WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
>   };
>   
>   static inline char *wmi_service_name(int service_id)
> @@ -461,6 +469,11 @@ static inline char *wmi_service_name(int service_id)
>   	SVCSTR(WMI_SERVICE_HTT_MGMT_TX_COMP_VALID_FLAGS);
>   	SVCSTR(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT);
>   	SVCSTR(WMI_SERVICE_TPC_STATS_FINAL);
> +	SVCSTR(WMI_SERVICE_CFR_CAPTURE_SUPPORT);
> +	SVCSTR(WMI_SERVICE_TX_DATA_ACK_RSSI);
> +	SVCSTR(WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1);
> +	SVCSTR(WMI_SERVICE_PER_PACKET_SW_ENCRYPT);
> +
>   	default:
>   		return NULL;
>   	}
> @@ -769,6 +782,14 @@ static inline void wmi_10_4_svc_map(const __le32 *in, unsigned long *out,
>   	       WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, len);
>   	SVCMAP(WMI_10_4_SERVICE_TPC_STATS_FINAL,
>   	       WMI_SERVICE_TPC_STATS_FINAL, len);
> +	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_SUPPORT,
> +	       WMI_SERVICE_CFR_CAPTURE_SUPPORT, len);
> +	SVCMAP(WMI_10_4_SERVICE_TX_DATA_ACK_RSSI,
> +	       WMI_SERVICE_TX_DATA_ACK_RSSI, len);
> +	SVCMAP(WMI_10_4_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1,
> +	       WMI_SERVICE_CFR_CAPTURE_IND_MSG_TYPE_1, len);
> +	SVCMAP(WMI_10_4_SERVICE_PER_PACKET_SW_ENCRYPT,
> +	       WMI_SERVICE_PER_PACKET_SW_ENCRYPT, len);
>   }
>   
>   #undef SVCMAP


-- 
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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Kalle Valo]

Manikanta Pubbisetty <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>

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 helpdesk@kernel.org and the admins
can fix it.

[1] https://patchwork.kernel.org/register/

-- 
Kalle Valo

Re: [PATCH] ath10k: add dynamic vlan support

[Kalle Valo]

Manikanta Pubbisetty <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>

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 helpdesk@kernel.org and the admins
can fix it.

[1] https://patchwork.kernel.org/register/

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
> 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 helpdesk@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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
> 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 helpdesk@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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>> 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 helpdesk@kernel.org and the admins
>> can fix it.
>>
>> [1] https://patchwork.kernel.org/register/
>>
>

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>> 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 helpdesk@kernel.org and the admins
>> can fix it.
>>
>> [1] https://patchwork.kernel.org/register/
>>
>


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Kalle Valo]

Sebastian Gottschall <s.gottschall@dd-wrt.com> 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

[Kalle Valo]

Sebastian Gottschall <s.gottschall@dd-wrt.com> 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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

Am 24.04.2018 um 11:52 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> 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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

Am 24.04.2018 um 11:52 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> 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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>> 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 helpdesk@kernel.org and the admins
>> can fix it.
>> 
>> [1] https://patchwork.kernel.org/register/
>> 

Re: [PATCH] ath10k: add dynamic vlan support

[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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>> 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 helpdesk@kernel.org and the admins
>> can fix it.
>> 
>> [1] https://patchwork.kernel.org/register/
>> 

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>>> 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 helpdesk@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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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 <mpubbise@codeaurora.org> 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 <mpubbise@codeaurora.org>
>>> 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 helpdesk@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.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[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?

johannes

Re: [PATCH] ath10k: add dynamic vlan support

[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?

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[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

Re: [PATCH] ath10k: add dynamic vlan support

[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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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

[Sebastian Gottschall]

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
>


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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

[Sebastian Gottschall]

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

>


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

> 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

[Manikanta Pubbisetty]

> 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.


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

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

[Johannes Berg]

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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

>>>> + * @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

[Manikanta Pubbisetty]

>>>> + * @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

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[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?

johannes

Re: [PATCH] ath10k: add dynamic vlan support

[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?

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

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

[Manikanta Pubbisetty]

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.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Sebastian Gottschall]

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

[Sebastian Gottschall]

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
>


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

On Thu, 2018-05-24 at 06:41 +0200, Sebastian Gottschall wrote:
> 
> 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

That was my point - if it doesn't check the capability then we can
remove it without impacting backward compatibility, and add it back in
another bit saying "AP-VLAN for 4-addr only".

johannes

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

On Thu, 2018-05-24 at 06:41 +0200, Sebastian Gottschall wrote:
> 
> 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

That was my point - if it doesn't check the capability then we can
remove it without impacting backward compatibility, and add it back in
another bit saying "AP-VLAN for 4-addr only".

johannes


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 5/23/2018 3:20 PM, Johannes Berg wrote:
> 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.

Hi Johannes,

I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
4-addr handling from AP/VLAN to this new iftype. But this approach 
breaks the backward compatibility with older userspace applications. 
Since I am completely moving the 4-addr handling to the new type, older 
applications which do not understand this new type will simply fail and 
4-addr mode will be completely broken.

Currently, whenever a 4-addr client attempts a connection, hostapd just 
creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN 
iface; there are no other checks. I had no other option other than going 
with a new iftype for 4-addr handling.

Is there a way we can maintain backward compatibility with this 
approach? Retaining the 4-addr handling in AP/VLAN and duplicating the 
same functionality to the new iftype seems will work but I am not sure 
if this is the right approach.

Thanks,
Manikanta

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 5/23/2018 3:20 PM, Johannes Berg wrote:
> 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.

Hi Johannes,

I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
4-addr handling from AP/VLAN to this new iftype. But this approach 
breaks the backward compatibility with older userspace applications. 
Since I am completely moving the 4-addr handling to the new type, older 
applications which do not understand this new type will simply fail and 
4-addr mode will be completely broken.

Currently, whenever a 4-addr client attempts a connection, hostapd just 
creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN 
iface; there are no other checks. I had no other option other than going 
with a new iftype for 4-addr handling.

Is there a way we can maintain backward compatibility with this 
approach? Retaining the 4-addr handling in AP/VLAN and duplicating the 
same functionality to the new iftype seems will work but I am not sure 
if this is the right approach.

Thanks,
Manikanta



_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:

> > 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.

> I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
> 4-addr handling from AP/VLAN to this new iftype. But this approach 
> breaks the backward compatibility with older userspace applications. 

Yeah ...

I'm confused and no longer sure what I was thinking, nor even what we're
trying to achieve here...

> Since I am completely moving the 4-addr handling to the new type, older 
> applications which do not understand this new type will simply fail and 
> 4-addr mode will be completely broken.
> 
> Currently, whenever a 4-addr client attempts a connection, hostapd just 
> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN 
> iface; there are no other checks. I had no other option other than going 
> with a new iftype for 4-addr handling.
> 
> Is there a way we can maintain backward compatibility with this 
> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the 
> same functionality to the new iftype seems will work but I am not sure 
> if this is the right approach.

I think we have to keep the 4-addr handling in AP_VLAN iftype either
way, to not break existing hostapd. We could introduce a separate
AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
VLAN, but that also seems awkward.

Since hostapd doesn't currently check anything...

Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't we
get what we want? 4-addr AP_VLAN interfaces would no longer be permitted
to be created?

johannes

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:

> > 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.

> I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
> 4-addr handling from AP/VLAN to this new iftype. But this approach 
> breaks the backward compatibility with older userspace applications. 

Yeah ...

I'm confused and no longer sure what I was thinking, nor even what we're
trying to achieve here...

> Since I am completely moving the 4-addr handling to the new type, older 
> applications which do not understand this new type will simply fail and 
> 4-addr mode will be completely broken.
> 
> Currently, whenever a 4-addr client attempts a connection, hostapd just 
> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN 
> iface; there are no other checks. I had no other option other than going 
> with a new iftype for 4-addr handling.
> 
> Is there a way we can maintain backward compatibility with this 
> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the 
> same functionality to the new iftype seems will work but I am not sure 
> if this is the right approach.

I think we have to keep the 4-addr handling in AP_VLAN iftype either
way, to not break existing hostapd. We could introduce a separate
AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
VLAN, but that also seems awkward.

Since hostapd doesn't currently check anything...

Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't we
get what we want? 4-addr AP_VLAN interfaces would no longer be permitted
to be created?

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 2018-08-16 13:57, Johannes Berg wrote:
> On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
> 
>> > 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.
> 
>> I was working on splitting the 4-addr functionality from AP/VLAN 
>> iftype;
>> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
>> 4-addr handling from AP/VLAN to this new iftype. But this approach
>> breaks the backward compatibility with older userspace applications.
> 
> Yeah ...
> 
> I'm confused and no longer sure what I was thinking, nor even what 
> we're
> trying to achieve here...
> 

I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN 
operation on crypto controlled devices ") for supporting VLAN 
functionality on ath10k devices; this commit has broken 4 addr support 
on ath10k devices as I was advertising the AP/VLAN support 
conditionally. Since 4 addr operation is tied to AP/VLAN support, with 
this change, only the chips which support VLAN functionality can support 
4 addr operation but other ath10k chips don't.

 From what I can understand from our previous discussions, we had to 
separate the 4addr support from the AP/VLAN iftype but doing so could 
lead to backward compatibility issues. I have the code which separates 
the 4addr functionality from AP/VLAN but the bigger problem is the 
backward compatibility.

I am hoping that now I have set the context correctly :)

>> Since I am completely moving the 4-addr handling to the new type, 
>> older
>> applications which do not understand this new type will simply fail 
>> and
>> 4-addr mode will be completely broken.
>> 
>> Currently, whenever a 4-addr client attempts a connection, hostapd 
>> just
>> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN
>> iface; there are no other checks. I had no other option other than 
>> going
>> with a new iftype for 4-addr handling.
>> 
>> Is there a way we can maintain backward compatibility with this
>> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the
>> same functionality to the new iftype seems will work but I am not sure
>> if this is the right approach.
> 
> I think we have to keep the 4-addr handling in AP_VLAN iftype either
> way, to not break existing hostapd. We could introduce a separate
> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> VLAN, but that also seems awkward.
> 
> Since hostapd doesn't currently check anything...
> 
> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't 
> we
> get what we want? 4-addr AP_VLAN interfaces would no longer be 
> permitted
> to be created?

As I explained above, the agenda is to provide the driver (in this case, 
ath10k) a better control for advertising the device capabilities; only 
few ath10k chips can support VLAN functionality but all of them can 
support 4 addr operation.

Manikanta

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 2018-08-16 13:57, Johannes Berg wrote:
> On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
> 
>> > 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.
> 
>> I was working on splitting the 4-addr functionality from AP/VLAN 
>> iftype;
>> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the
>> 4-addr handling from AP/VLAN to this new iftype. But this approach
>> breaks the backward compatibility with older userspace applications.
> 
> Yeah ...
> 
> I'm confused and no longer sure what I was thinking, nor even what 
> we're
> trying to achieve here...
> 

I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN 
operation on crypto controlled devices ") for supporting VLAN 
functionality on ath10k devices; this commit has broken 4 addr support 
on ath10k devices as I was advertising the AP/VLAN support 
conditionally. Since 4 addr operation is tied to AP/VLAN support, with 
this change, only the chips which support VLAN functionality can support 
4 addr operation but other ath10k chips don't.

 From what I can understand from our previous discussions, we had to 
separate the 4addr support from the AP/VLAN iftype but doing so could 
lead to backward compatibility issues. I have the code which separates 
the 4addr functionality from AP/VLAN but the bigger problem is the 
backward compatibility.

I am hoping that now I have set the context correctly :)

>> Since I am completely moving the 4-addr handling to the new type, 
>> older
>> applications which do not understand this new type will simply fail 
>> and
>> 4-addr mode will be completely broken.
>> 
>> Currently, whenever a 4-addr client attempts a connection, hostapd 
>> just
>> creates a AP/VLAN interface and moves the 4-addr client to the AP/VLAN
>> iface; there are no other checks. I had no other option other than 
>> going
>> with a new iftype for 4-addr handling.
>> 
>> Is there a way we can maintain backward compatibility with this
>> approach? Retaining the 4-addr handling in AP/VLAN and duplicating the
>> same functionality to the new iftype seems will work but I am not sure
>> if this is the right approach.
> 
> I think we have to keep the 4-addr handling in AP_VLAN iftype either
> way, to not break existing hostapd. We could introduce a separate
> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> VLAN, but that also seems awkward.
> 
> Since hostapd doesn't currently check anything...
> 
> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't 
> we
> get what we want? 4-addr AP_VLAN interfaces would no longer be 
> permitted
> to be created?

As I explained above, the agenda is to provide the driver (in this case, 
ath10k) a better control for advertising the device capabilities; only 
few ath10k chips can support VLAN functionality but all of them can 
support 4 addr operation.

Manikanta

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Kalle Valo]

Manikanta Pubbisetty <mpubbise@codeaurora.org> writes:

> On 8/16/2018 1:57 PM, Johannes Berg wrote:
>
>     On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
>
>             
>             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.
>
>     
>     
>         I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
> 4-addr handling from AP/VLAN to this new iftype. But this approach 
> breaks the backward compatibility with older userspace applications. 
>
>     
> Yeah ...
>
> I'm confused and no longer sure what I was thinking, nor even what we're
> trying to achieve here...
>
> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
> operation on crypto controlled devices ") for supporting VLAN
> functionality on ath10k devices; this commit has broken 4 addr support
> on ath10k devices as I was advertising the AP/VLAN support
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
> this change, only the chips which support VLAN functionality can
> support 4 addr operation but other ath10k chips don't.

Manikanta, please set up your Thunderbird so that it uses the standard
'>' quotation style. This mail is impossible to read as I don't know
which part is written by you and which part by Johannes.

-- 
Kalle Valo

Re: [PATCH] ath10k: add dynamic vlan support

[Kalle Valo]

Manikanta Pubbisetty <mpubbise@codeaurora.org> writes:

> On 8/16/2018 1:57 PM, Johannes Berg wrote:
>
>     On Tue, 2018-08-14 at 18:23 +0530, Manikanta Pubbisetty wrote:
>
>             
>             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.
>
>     
>     
>         I was working on splitting the 4-addr functionality from AP/VLAN iftype; 
> I have introduced a new iftype NL80211_IFTYPE_AP_4ADDR and moved the 
> 4-addr handling from AP/VLAN to this new iftype. But this approach 
> breaks the backward compatibility with older userspace applications. 
>
>     
> Yeah ...
>
> I'm confused and no longer sure what I was thinking, nor even what we're
> trying to achieve here...
>
> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
> operation on crypto controlled devices ") for supporting VLAN
> functionality on ath10k devices; this commit has broken 4 addr support
> on ath10k devices as I was advertising the AP/VLAN support
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
> this change, only the chips which support VLAN functionality can
> support 4 addr operation but other ath10k chips don't.

Manikanta, please set up your Thunderbird so that it uses the standard
'>' quotation style. This mail is impossible to read as I don't know
which part is written by you and which part by Johannes.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

Hi,

Sorry ... this got delayed again.

> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN 
> operation on crypto controlled devices ") for supporting VLAN 
> functionality on ath10k devices; this commit has broken 4 addr support 
> on ath10k devices as I was advertising the AP/VLAN support 
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with 
> this change, only the chips which support VLAN functionality can support 
> 4 addr operation but other ath10k chips don't.

Right.

>  From what I can understand from our previous discussions, we had to 
> separate the 4addr support from the AP/VLAN iftype but doing so could 
> lead to backward compatibility issues. I have the code which separates 
> the 4addr functionality from AP/VLAN but the bigger problem is the 
> backward compatibility.

Ok.

> I am hoping that now I have set the context correctly :)

Thanks!

> > I think we have to keep the 4-addr handling in AP_VLAN iftype either
> > way, to not break existing hostapd. We could introduce a separate
> > AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> > VLAN, but that also seems awkward.
> > 
> > Since hostapd doesn't currently check anything...
> > 
> > Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't 
> > we
> > get what we want? 4-addr AP_VLAN interfaces would no longer be 
> > permitted
> > to be created?
> 
> As I explained above, the agenda is to provide the driver (in this case, 
> ath10k) a better control for advertising the device capabilities; only 
> few ath10k chips can support VLAN functionality but all of them can 
> support 4 addr operation.

So the problematic part isn't actually the *4-addr* (fake) VLAN, the
problematic part is the actual AP_VLAN - I suppose because that uses a
separate GTK.


So I guess the only, mostly backward compatible way to really separate
the two would be to

1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
   I guess the STATION always should've been since there's nothing that
   indicates support for it today in the API

along with one of:

2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
    only supported for 4-addr

2b) Allow creating an AP_VLAN interface in 4-addr mode in 
    nl80211_new_interface() even when AP_VLAN is not in the supported
    interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
    (or rather the new extended feature flag after doing 1). Update the
    language in the documentation somewhere indicating this.


Hostapd clearly deals with both 2a and 2b since it never bothers to
check the combinations. As a result, I prefer 2b rather than 2a since it
doesn't add any weird combinations to the API that would be impossible.

johannes

Re: [PATCH] ath10k: add dynamic vlan support

[Johannes Berg]

Hi,

Sorry ... this got delayed again.

> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN 
> operation on crypto controlled devices ") for supporting VLAN 
> functionality on ath10k devices; this commit has broken 4 addr support 
> on ath10k devices as I was advertising the AP/VLAN support 
> conditionally. Since 4 addr operation is tied to AP/VLAN support, with 
> this change, only the chips which support VLAN functionality can support 
> 4 addr operation but other ath10k chips don't.

Right.

>  From what I can understand from our previous discussions, we had to 
> separate the 4addr support from the AP/VLAN iftype but doing so could 
> lead to backward compatibility issues. I have the code which separates 
> the 4addr functionality from AP/VLAN but the bigger problem is the 
> backward compatibility.

Ok.

> I am hoping that now I have set the context correctly :)

Thanks!

> > I think we have to keep the 4-addr handling in AP_VLAN iftype either
> > way, to not break existing hostapd. We could introduce a separate
> > AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
> > VLAN, but that also seems awkward.
> > 
> > Since hostapd doesn't currently check anything...
> > 
> > Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't 
> > we
> > get what we want? 4-addr AP_VLAN interfaces would no longer be 
> > permitted
> > to be created?
> 
> As I explained above, the agenda is to provide the driver (in this case, 
> ath10k) a better control for advertising the device capabilities; only 
> few ath10k chips can support VLAN functionality but all of them can 
> support 4 addr operation.

So the problematic part isn't actually the *4-addr* (fake) VLAN, the
problematic part is the actual AP_VLAN - I suppose because that uses a
separate GTK.


So I guess the only, mostly backward compatible way to really separate
the two would be to

1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
   I guess the STATION always should've been since there's nothing that
   indicates support for it today in the API

along with one of:

2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
    only supported for 4-addr

2b) Allow creating an AP_VLAN interface in 4-addr mode in 
    nl80211_new_interface() even when AP_VLAN is not in the supported
    interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
    (or rather the new extended feature flag after doing 1). Update the
    language in the documentation somewhere indicating this.


Hostapd clearly deals with both 2a and 2b since it never bothers to
check the combinations. As a result, I prefer 2b rather than 2a since it
doesn't add any weird combinations to the API that would be impossible.

johannes

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 9/3/2018 4:09 PM, Johannes Berg wrote:

> Hi,
>
> Sorry ... this got delayed again.
>
>> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
>> operation on crypto controlled devices ") for supporting VLAN
>> functionality on ath10k devices; this commit has broken 4 addr support
>> on ath10k devices as I was advertising the AP/VLAN support
>> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
>> this change, only the chips which support VLAN functionality can support
>> 4 addr operation but other ath10k chips don't.
> Right.
>
>>   From what I can understand from our previous discussions, we had to
>> separate the 4addr support from the AP/VLAN iftype but doing so could
>> lead to backward compatibility issues. I have the code which separates
>> the 4addr functionality from AP/VLAN but the bigger problem is the
>> backward compatibility.
> Ok.
>
>> I am hoping that now I have set the context correctly :)
> Thanks!
>
>>> I think we have to keep the 4-addr handling in AP_VLAN iftype either
>>> way, to not break existing hostapd. We could introduce a separate
>>> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
>>> VLAN, but that also seems awkward.
>>>
>>> Since hostapd doesn't currently check anything...
>>>
>>> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
>>> we
>>> get what we want? 4-addr AP_VLAN interfaces would no longer be
>>> permitted
>>> to be created?
>> As I explained above, the agenda is to provide the driver (in this case,
>> ath10k) a better control for advertising the device capabilities; only
>> few ath10k chips can support VLAN functionality but all of them can
>> support 4 addr operation.
> So the problematic part isn't actually the *4-addr* (fake) VLAN, the
> problematic part is the actual AP_VLAN - I suppose because that uses a
> separate GTK.
>
>
> So I guess the only, mostly backward compatible way to really separate
> the two would be to
>
> 1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
>     I guess the STATION always should've been since there's nothing that
>     indicates support for it today in the API
>
> along with one of:
>
> 2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
>      only supported for 4-addr
>
> 2b) Allow creating an AP_VLAN interface in 4-addr mode in
>      nl80211_new_interface() even when AP_VLAN is not in the supported
>      interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
>      (or rather the new extended feature flag after doing 1). Update the
>      language in the documentation somewhere indicating this.
>
>
> Hostapd clearly deals with both 2a and 2b since it never bothers to
> check the combinations. As a result, I prefer 2b rather than 2a since it
> doesn't add any weird combinations to the API that would be impossible.

Sure Johannes!!

Thanks,
Manikanta

Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty]

On 9/3/2018 4:09 PM, Johannes Berg wrote:

> Hi,
>
> Sorry ... this got delayed again.
>
>> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
>> operation on crypto controlled devices ") for supporting VLAN
>> functionality on ath10k devices; this commit has broken 4 addr support
>> on ath10k devices as I was advertising the AP/VLAN support
>> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
>> this change, only the chips which support VLAN functionality can support
>> 4 addr operation but other ath10k chips don't.
> Right.
>
>>   From what I can understand from our previous discussions, we had to
>> separate the 4addr support from the AP/VLAN iftype but doing so could
>> lead to backward compatibility issues. I have the code which separates
>> the 4addr functionality from AP/VLAN but the bigger problem is the
>> backward compatibility.
> Ok.
>
>> I am hoping that now I have set the context correctly :)
> Thanks!
>
>>> I think we have to keep the 4-addr handling in AP_VLAN iftype either
>>> way, to not break existing hostapd. We could introduce a separate
>>> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
>>> VLAN, but that also seems awkward.
>>>
>>> Since hostapd doesn't currently check anything...
>>>
>>> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
>>> we
>>> get what we want? 4-addr AP_VLAN interfaces would no longer be
>>> permitted
>>> to be created?
>> As I explained above, the agenda is to provide the driver (in this case,
>> ath10k) a better control for advertising the device capabilities; only
>> few ath10k chips can support VLAN functionality but all of them can
>> support 4 addr operation.
> So the problematic part isn't actually the *4-addr* (fake) VLAN, the
> problematic part is the actual AP_VLAN - I suppose because that uses a
> separate GTK.
>
>
> So I guess the only, mostly backward compatible way to really separate
> the two would be to
>
> 1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
>     I guess the STATION always should've been since there's nothing that
>     indicates support for it today in the API
>
> along with one of:
>
> 2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
>      only supported for 4-addr
>
> 2b) Allow creating an AP_VLAN interface in 4-addr mode in
>      nl80211_new_interface() even when AP_VLAN is not in the supported
>      interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
>      (or rather the new extended feature flag after doing 1). Update the
>      language in the documentation somewhere indicating this.
>
>
> Hostapd clearly deals with both 2a and 2b since it never bothers to
> check the combinations. As a result, I prefer 2b rather than 2a since it
> doesn't add any weird combinations to the API that would be impossible.

Sure Johannes!!

Thanks,
Manikanta

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k