| Message ID | 20200122142930.19239-1-john@phrozen.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [RESEND] ath11k: add tx hw 802.11 encapusaltion offloading support | expand |
> 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 <slakkavalli@datto.com> > Signed-off-by: John Crispin <john@phrozen.org> > --- > 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
John Crispin <john@phrozen.org> 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 <slakkavalli@datto.com> > Signed-off-by: John Crispin <john@phrozen.org> 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.
> 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 >
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) {