2020-01-22 14:30:08

by John Crispin

[permalink] [raw]
Subject: [RESEND] ath11k: add tx hw 802.11 encapusaltion offloading support

This patch adds support for ethernet rxtx mode to the driver. The feature
is enabled via a new module parameter. If enabled to driver will enable
the feature on a per vif basis if all other requirements were met.

Signed-off-by: Shashidhar Lakkavalli <[email protected]>
Signed-off-by: John Crispin <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.h | 5 +++
drivers/net/wireless/ath/ath11k/dp_tx.c | 16 ++++++++--
drivers/net/wireless/ath/ath11k/mac.c | 41 ++++++++++++++++++++-----
3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 25cdcf71d0c4..15676dce98b1 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -59,9 +59,14 @@ static inline enum wme_ac ath11k_tid_to_ac(u32 tid)
WME_AC_VO);
}

+enum ath11k_skb_flags {
+ ATH11K_SKB_HW_80211_ENCAP = BIT(0),
+};
+
struct ath11k_skb_cb {
dma_addr_t paddr;
u8 eid;
+ u8 flags;
struct ath11k *ar;
struct ieee80211_vif *vif;
} __packed;
diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c
index 918305dda106..3206f432f65b 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
@@ -15,7 +15,11 @@ ath11k_txq_tcl_ring_map[ATH11K_HW_MAX_QUEUES] = { 0x0, 0x1, 0x2, 0x2 };
static enum hal_tcl_encap_type
ath11k_dp_tx_get_encap_type(struct ath11k_vif *arvif, struct sk_buff *skb)
{
- /* TODO: Determine encap type based on vif_type and configuration */
+ struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
+
+ if (tx_info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)
+ return HAL_TCL_ENCAP_TYPE_ETHERNET;
+
return HAL_TCL_ENCAP_TYPE_NATIVE_WIFI;
}

@@ -39,8 +43,11 @@ static void ath11k_dp_tx_encap_nwifi(struct sk_buff *skb)
static u8 ath11k_dp_tx_get_tid(struct sk_buff *skb)
{
struct ieee80211_hdr *hdr = (void *)skb->data;
+ struct ath11k_skb_cb *cb = ATH11K_SKB_CB(skb);

- if (!ieee80211_is_data_qos(hdr->frame_control))
+ if (cb->flags & ATH11K_SKB_HW_80211_ENCAP)
+ return skb->priority % IEEE80211_QOS_CTL_TID_MASK;
+ else if (!ieee80211_is_data_qos(hdr->frame_control))
return HAL_DESC_REO_NON_QOS_TID;
else
return skb->priority & IEEE80211_QOS_CTL_TID_MASK;
@@ -87,7 +94,8 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
if (test_bit(ATH11K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags))
return -ESHUTDOWN;

- if (!ieee80211_is_data(hdr->frame_control))
+ if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) &&
+ !ieee80211_is_data(hdr->frame_control))
return -ENOTSUPP;

pool_id = skb_get_queue_mapping(skb) & (ATH11K_HW_MAX_QUEUES - 1);
@@ -148,6 +156,8 @@ int ath11k_dp_tx(struct ath11k *ar, struct ath11k_vif *arvif,
* skb_checksum_help() is needed
*/
case HAL_TCL_ENCAP_TYPE_ETHERNET:
+ /* no need to encap */
+ break;
case HAL_TCL_ENCAP_TYPE_802_3:
/* TODO: Take care of other encap modes as well */
ret = -EINVAL;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 556eef9881a7..f56adba5f838 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -33,6 +33,10 @@
.max_power = 30, \
}

+static unsigned int ath11k_ethernet_mode;
+module_param_named(ethernet_mode, ath11k_ethernet_mode, uint, 0644);
+MODULE_PARM_DESC(ethernet_mode, "Use ethernet frame datapath");
+
static const struct ieee80211_channel ath11k_2ghz_channels[] = {
CHAN2G(1, 2412, 0),
CHAN2G(2, 2417, 0),
@@ -3633,6 +3637,7 @@ static int ath11k_mac_mgmt_tx_wmi(struct ath11k *ar, struct ath11k_vif *arvif,
{
struct ath11k_base *ab = ar->ab;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_tx_info *info;
dma_addr_t paddr;
int buf_id;
int ret;
@@ -3644,11 +3649,14 @@ static int ath11k_mac_mgmt_tx_wmi(struct ath11k *ar, struct ath11k_vif *arvif,
if (buf_id < 0)
return -ENOSPC;

- if ((ieee80211_is_action(hdr->frame_control) ||
- ieee80211_is_deauth(hdr->frame_control) ||
- ieee80211_is_disassoc(hdr->frame_control)) &&
- ieee80211_has_protected(hdr->frame_control)) {
- skb_put(skb, IEEE80211_CCMP_MIC_LEN);
+ info = IEEE80211_SKB_CB(skb);
+ if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)) {
+ if ((ieee80211_is_action(hdr->frame_control) ||
+ ieee80211_is_deauth(hdr->frame_control) ||
+ ieee80211_is_disassoc(hdr->frame_control)) &&
+ ieee80211_has_protected(hdr->frame_control)) {
+ skb_put(skb, IEEE80211_CCMP_MIC_LEN);
+ }
}

paddr = dma_map_single(ab->dev, skb->data, skb->len, DMA_TO_DEVICE);
@@ -3745,6 +3753,7 @@ static void ath11k_mac_op_tx(struct ieee80211_hw *hw,
struct ieee80211_tx_control *control,
struct sk_buff *skb)
{
+ struct ath11k_skb_cb *skb_cb = ATH11K_SKB_CB((struct sk_buff *)skb);
struct ath11k *ar = hw->priv;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_vif *vif = info->control.vif;
@@ -3753,7 +3762,10 @@ static void ath11k_mac_op_tx(struct ieee80211_hw *hw,
bool is_prb_rsp;
int ret;

- if (ieee80211_is_mgmt(hdr->frame_control)) {
+ skb_cb->flags = 0;
+ if (info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) {
+ skb_cb->flags = ATH11K_SKB_HW_80211_ENCAP;
+ } else if (ieee80211_is_mgmt(hdr->frame_control)) {
is_prb_rsp = ieee80211_is_probe_resp(hdr->frame_control);
ret = ath11k_mac_mgmt_tx(ar, skb, is_prb_rsp);
if (ret) {
@@ -4028,6 +4040,7 @@ static int ath11k_mac_op_add_interface(struct ieee80211_hw *hw,
struct vdev_create_params vdev_param = {0};
struct peer_create_params peer_param;
u32 param_id, param_value;
+ int hw_encap = 0;
u16 nss;
int i;
int ret;
@@ -4119,7 +4132,21 @@ static int ath11k_mac_op_add_interface(struct ieee80211_hw *hw,
spin_unlock_bh(&ar->data_lock);

param_id = WMI_VDEV_PARAM_TX_ENCAP_TYPE;
- param_value = ATH11K_HW_TXRX_NATIVE_WIFI;
+ switch (vif->type) {
+ case NL80211_IFTYPE_STATION:
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_AP:
+ hw_encap = 1;
+ break;
+ default:
+ break;
+ }
+
+ if (ieee80211_set_hw_80211_encap(vif, ath11k_ethernet_mode && hw_encap))
+ param_value = ATH11K_HW_TXRX_ETHERNET;
+ else
+ param_value = ATH11K_HW_TXRX_NATIVE_WIFI;
+
ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id,
param_id, param_value);
if (ret) {
--
2.20.1


2020-01-23 09:48:31

by Karthikeyan periyasamy

[permalink] [raw]
Subject: Re: [RESEND] ath11k: add tx hw 802.11 encapusaltion offloading support

> This patch adds support for ethernet rxtx mode to the driver. The
> feature
> is enabled via a new module parameter. If enabled to driver will enable
> the feature on a per vif basis if all other requirements were met.
>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/core.h | 5 +++
> drivers/net/wireless/ath/ath11k/dp_tx.c | 16 ++++++++--
> drivers/net/wireless/ath/ath11k/mac.c | 41 ++++++++++++++++++++-----
> 3 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/mac.c
> b/drivers/net/wireless/ath/ath11k/mac.c
> index 556eef9881a7..f56adba5f838 100644
> --- a/drivers/net/wireless/ath/ath11k/mac.c
> +++ b/drivers/net/wireless/ath/ath11k/mac.c
> @@ -33,6 +33,10 @@
> .max_power = 30, \
> }
>
> +static unsigned int ath11k_ethernet_mode;
> +module_param_named(ethernet_mode, ath11k_ethernet_mode, uint, 0644);
> +MODULE_PARM_DESC(ethernet_mode, "Use ethernet frame datapath");
> +

Shall we have a generic module parameter instead of a dedicated module
parameter for ethernet mode?
so that we can extend this module parameter for raw mode also like below

/* 0 - Nativi wifi
* 1 - Ethernet mode
*/
static unsigned int ath11k_mode;
module_param_named(mode, ath11k_mode, uint, 0644);
MODULE_PARM_DESC(mode, "Use for rxtx frame datapath");

- Karthikeyan

2020-01-26 11:30:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [RESEND] ath11k: add tx hw 802.11 encapusaltion offloading support

John Crispin <[email protected]> wrote:

> This patch adds support for ethernet rxtx mode to the driver. The feature
> is enabled via a new module parameter. If enabled to driver will enable
> the feature on a per vif basis if all other requirements were met.
>
> Signed-off-by: Shashidhar Lakkavalli <[email protected]>
> Signed-off-by: John Crispin <[email protected]>

Depends on:

50ff477a8639 mac80211: add 802.11 encapsulation offloading support

Currently in mac80211-next.

drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_mgmt_tx_wmi':
drivers/net/wireless/ath/ath11k/mac.c:3653:30: error: 'IEEE80211_TX_CTRL_HW_80211_ENCAP' undeclared (first use in this function); did you mean 'IEEE80211_TX_CTRL_RATE_INJECT'?
if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IEEE80211_TX_CTRL_RATE_INJECT
drivers/net/wireless/ath/ath11k/mac.c:3653:30: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_tx':
drivers/net/wireless/ath/ath11k/mac.c:3766:28: error: 'IEEE80211_TX_CTRL_HW_80211_ENCAP' undeclared (first use in this function); did you mean 'IEEE80211_TX_CTRL_RATE_INJECT'?
if (info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IEEE80211_TX_CTRL_RATE_INJECT
drivers/net/wireless/ath/ath11k/mac.c: In function 'ath11k_mac_op_add_interface':
drivers/net/wireless/ath/ath11k/mac.c:4145:6: error: implicit declaration of function 'ieee80211_set_hw_80211_encap'; did you mean 'ieee80211_get_he_sta_cap'? [-Werror=implicit-function-declaration]
if (ieee80211_set_hw_80211_encap(vif, ath11k_ethernet_mode && hw_encap))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
ieee80211_get_he_sta_cap
cc1: some warnings being treated as errors
make[5]: *** [drivers/net/wireless/ath/ath11k/mac.o] Error 1
make[5]: *** Waiting for unfinished jobs....
drivers/net/wireless/ath/ath11k/dp_tx.c: In function 'ath11k_dp_tx_get_encap_type':
drivers/net/wireless/ath/ath11k/dp_tx.c:20:31: error: 'IEEE80211_TX_CTRL_HW_80211_ENCAP' undeclared (first use in this function); did you mean 'IEEE80211_TX_CTRL_RATE_INJECT'?
if (tx_info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IEEE80211_TX_CTRL_RATE_INJECT
drivers/net/wireless/ath/ath11k/dp_tx.c:20:31: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/wireless/ath/ath11k/dp_tx.c: In function 'ath11k_dp_tx':
drivers/net/wireless/ath/ath11k/dp_tx.c:97:30: error: 'IEEE80211_TX_CTRL_HW_80211_ENCAP' undeclared (first use in this function); did you mean 'IEEE80211_TX_CTRL_RATE_INJECT'?
if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) &&
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
IEEE80211_TX_CTRL_RATE_INJECT
make[5]: *** [drivers/net/wireless/ath/ath11k/dp_tx.o] Error 1
make[4]: *** [drivers/net/wireless/ath/ath11k] Error 2
make[3]: *** [drivers/net/wireless/ath] Error 2
make[2]: *** [drivers/net/wireless] Error 2
make[1]: *** [drivers/net] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2

Patch set to Awaiting Upstream.

--
https://patchwork.kernel.org/patch/11345841/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2020-01-26 13:14:24

by Justin Capella

[permalink] [raw]
Subject: Re: [RESEND] ath11k: add tx hw 802.11 encapusaltion offloading support

> ath11k_dp_tx_get_encap_type(struct ath11k_vif *arvif, struct sk_buff *skb)
> {
> - /* TODO: Determine encap type based on vif_type and configuration */
> + struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
> +
> + if (tx_info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)
> + return HAL_TCL_ENCAP_TYPE_ETHERNET;
> +
> return HAL_TCL_ENCAP_TYPE_NATIVE_WIFI;
> }

Would reserving some bits/separating encapsulation so a mask/shift
could allow for enum/switch? I second Karthikeyan's idea of the
generic module_param--- If you look at the ath10k codebase they have
separate flags for sw/hwcrypto and ethernet and it resulted in needing
to check for the mutually exclusive options

> @@ -39,8 +43,11 @@ static void ath11k_dp_tx_encap_nwifi(struct sk_buff *skb)
> + if (cb->flags & ATH11K_SKB_HW_80211_ENCAP)
> + return skb->priority % IEEE80211_QOS_CTL_TID_MASK;

Maybe use & to be consistent with other _MASKs instead of %?

> pool_id = skb_get_queue_mapping(skb) & (ATH11K_HW_MAX_QUEUES - 1);

Not part of the patch but would "min" be better here?

> case HAL_TCL_ENCAP_TYPE_802_3:\

+default? (Take care of that todo:?)


> +static unsigned int ath11k_ethernet_mode;
> +module_param_named(ethernet_mode, ath11k_ethernet_mode, uint, 0644);
> +MODULE_PARM_DESC(ethernet_mode, "Use ethernet frame datapath");

See above

> if (buf_id < 0)
> return -ENOSPC;

Again not part of your patch but, why is this not just unsigned then,
or are negatives used to invalidate? Haven't looked through the code
enough yet

> + info = IEEE80211_SKB_CB(skb);

Could this be done at at initialization?


> + if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)) {
> + if ((ieee80211_is_action(hdr->frame_control) ||
> + ieee80211_is_deauth(hdr->frame_control) ||
> + ieee80211_is_disassoc(hdr->frame_control)) &&
> + ieee80211_has_protected(hdr->frame_control)) {
> + skb_put(skb, IEEE80211_CCMP_MIC_LEN);

Maybe just skip/goto past this if offloading? Totally a style thing,
but if more encapsulation/offloading is added later it might pave the
way for cleaner code?

Totally trivial/not a real issue, but I had the thought that if it
were written in the reverse order, protected && (action || deauth ||
dissassoc), it could shortcut quicker potentially?

> @@ -3745,6 +3753,7 @@ static void ath11k_mac_op_tx(struct ieee80211_hw *hw,
> struct ieee80211_tx_control *control,
> struct sk_buff *skb)
> {
> + struct ath11k_skb_cb *skb_cb = ATH11K_SKB_CB((struct sk_buff *)skb);

I don't think this cast is needed.

> @@ -4028,6 +4040,7 @@ static int ath11k_mac_op_add_interface(struct ieee80211_hw *hw,
> + int hw_encap = 0;

Another spot where the possibility of having an enum for the
encapsulation/flags could be handy?


- if (ieee80211_is_mgmt(hdr->frame_control)) {
+ skb_cb->flags = 0;
+ if (info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) {
+ skb_cb->flags = ATH11K_SKB_HW_80211_ENCAP;
+ } else if (ieee80211_is_mgmt(hdr->frame_control)) {

Should this maybe be future proofed, something like skb_cb->flags |=
ATH11K_SKB_HW_80211_ENCAP or perhaps even masking the encapsulation
bits as to not reset all the flags ( =0)



> + switch (vif->type) {
> + case NL80211_IFTYPE_STATION:
> + case NL80211_IFTYPE_AP_VLAN:
> + case NL80211_IFTYPE_AP:
> + hw_encap = 1;
> + break;
> + default:
> + break;

No mesh?



> +static unsigned int ath11k_ethernet_mode;
> +module_param_named(ethernet_mode, ath11k_ethernet_mode, uint, 0644);
> +MODULE_PARM_DESC(ethernet_mode, "Use ethernet frame datapath");

> +
> static const struct ieee80211_channel ath11k_2ghz_channels[] = {
> CHAN2G(1, 2412, 0),
> CHAN2G(2, 2417, 0),
> @@ -3633,6 +3637,7 @@ static int ath11k_mac_mgmt_tx_wmi(struct ath11k *ar, struct ath11k_vif *arvif,
> {
> struct ath11k_base *ab = ar->ab;
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> + struct ieee80211_tx_info *info;
> dma_addr_t paddr;
> int buf_id;
> int ret;
> @@ -3644,11 +3649,14 @@ static int ath11k_mac_mgmt_tx_wmi(struct ath11k *ar, struct ath11k_vif *arvif,
> if (buf_id < 0)
> return -ENOSPC;
>
> - if ((ieee80211_is_action(hdr->frame_control) ||
> - ieee80211_is_deauth(hdr->frame_control) ||
> - ieee80211_is_disassoc(hdr->frame_control)) &&
> - ieee80211_has_protected(hdr->frame_control)) {
> - skb_put(skb, IEEE80211_CCMP_MIC_LEN);
> + info = IEEE80211_SKB_CB(skb);
> + if (!(info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP)) {
> + if ((ieee80211_is_action(hdr->frame_control) ||
> + ieee80211_is_deauth(hdr->frame_control) ||
> + ieee80211_is_disassoc(hdr->frame_control)) &&
> + ieee80211_has_protected(hdr->frame_control)) {
> + skb_put(skb, IEEE80211_CCMP_MIC_LEN);
> + }
> }
>
> paddr = dma_map_single(ab->dev, skb->data, skb->len, DMA_TO_DEVICE);
> @@ -3745,6 +3753,7 @@ static void ath11k_mac_op_tx(struct ieee80211_hw *hw,
> struct ieee80211_tx_control *control,
> struct sk_buff *skb)
> {
> + struct ath11k_skb_cb *skb_cb = ATH11K_SKB_CB((struct sk_buff *)skb);
> struct ath11k *ar = hw->priv;
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> struct ieee80211_vif *vif = info->control.vif;
> @@ -3753,7 +3762,10 @@ static void ath11k_mac_op_tx(struct ieee80211_hw *hw,
> bool is_prb_rsp;
> int ret;
>
> - if (ieee80211_is_mgmt(hdr->frame_control)) {
> + skb_cb->flags = 0;
> + if (info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) {
> + skb_cb->flags = ATH11K_SKB_HW_80211_ENCAP;
> + } else if (ieee80211_is_mgmt(hdr->frame_control)) {
> is_prb_rsp = ieee80211_is_probe_resp(hdr->frame_control);
> ret = ath11k_mac_mgmt_tx(ar, skb, is_prb_rsp);
> if (ret) {
> @@ -4028,6 +4040,7 @@ static int ath11k_mac_op_add_interface(struct ieee80211_hw *hw,
> struct vdev_create_params vdev_param = {0};
> struct peer_create_params peer_param;
> u32 param_id, param_value;
> + int hw_encap = 0;
> u16 nss;
> int i;
> int ret;
> @@ -4119,7 +4132,21 @@ static int ath11k_mac_op_add_interface(struct ieee80211_hw *hw,
> spin_unlock_bh(&ar->data_lock);
>
> param_id = WMI_VDEV_PARAM_TX_ENCAP_TYPE;
> - param_value = ATH11K_HW_TXRX_NATIVE_WIFI;
> + switch (vif->type) {
> + case NL80211_IFTYPE_STATION:
> + case NL80211_IFTYPE_AP_VLAN:
> + case NL80211_IFTYPE_AP:
> + hw_encap = 1;
> + break;
> + default:
> + break;
> + }
> +
> + if (ieee80211_set_hw_80211_encap(vif, ath11k_ethernet_mode && hw_encap))
> + param_value = ATH11K_HW_TXRX_ETHERNET;
> + else
> + param_value = ATH11K_HW_TXRX_NATIVE_WIFI;
> +
> ret = ath11k_wmi_vdev_set_param_cmd(ar, arvif->vdev_id,
> param_id, param_value);
> if (ret) {
> --
> 2.20.1
>