| Message ID | 1481781608-5181-4-git-send-email-vthiagar@qti.qualcomm.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
> This rx path only checks if the driver has advertised > it's support of 802.11 header encap/decap offload for > data frames. I'm not even sure I see the point in that? Other than that (and the various other offload requirements), it seems that encap/decap could be considered orthogonal. > + * Adhoc interface depends on bssid to udpate last_rx. type - update > + if (!(status->flag & RX_FLAG_MCAST)) { > + sta->rx_stats.last_rx = jiffies; > + sta->rx_stats.last_rate = > sta_stats_encode_rate(status); > + } You should probably rename that flag to something like RX_FLAG_80211_MCAST since otherwise it's confusing with the next multicast ether addr check: > + if (sdata->vif.type == NL80211_IFTYPE_STATION && > + !is_multicast_ether_addr(ehdr->h_dest)) > + ieee80211_sta_reset_conn_monitor(sdata); But this could just also use the flag, since in station mode the two are equivalent, and it'd be easier to figure out if this was "else if (station mode)"? > + memset(&rx, 0, sizeof(rx)); That seems a bit pointless? > + rx.skb = skb; > + rx.sdata = sdata; > + rx.local = local; > + rx.sta = sta; > + > + if (vif->type == NL80211_IFTYPE_AP_VLAN && sdata->bss && > + unlikely(ehdr->h_proto == sdata->control_port_protocol)) > { > + sdata = container_of(sdata->bss, struct > ieee80211_sub_if_data, > + u.ap); > + dev = sdata->dev; > + rx.sdata = sdata; > + } > + > + rx.skb->dev = dev; > + > + /* XXX: Since rx.seqno_idx is not available for decap > offloaded frames > + * rx msdu stats update at the seqno_idx in > ieee80211_deliver_skb() > + * will always be updated at index 0 and will not be very > useful. > + */ > + ieee80211_deliver_skb(&rx); Yeah, that's not nice - perhaps we can provide the TID out of band? If not, we'll have to disable those statistics *all the way*, i.e. not even report them to userspace when filling sinfo. > + return; > + > +mic_fail: > + cfg80211_michael_mic_failure(sdata->dev, sta->addr, > + (status->flag & RX_FLAG_MCAST) > ? > + NL80211_KEYTYPE_GROUP : > + NL80211_KEYTYPE_PAIRWISE, > + key ? key->conf.keyidx : -1, > + NULL, GFP_ATOMIC); Do we really want to handle that inline here? The driver probably has a different check to even set RX_FLAG_MMIC_ERROR, so we could just ask it to call cfg80211_michael_mic_failure() [or a wrapper to get sdata->dev] instead? I guess this works too though, and might be easier to understand. johannes
On Thursday 15 December 2016 03:08 PM, Johannes Berg wrote: > >> This rx path only checks if the driver has advertised >> it's support of 802.11 header encap/decap offload for >> data frames. > > I'm not even sure I see the point in that? Other than that (and the > various other offload requirements), it seems that encap/decap could be > considered orthogonal. Ok. So it is mainly for encap there needs to be some capability advertisement from driver because for decap driver would use appropriate mac80211 Rx function to indicate? > >> + * Adhoc interface depends on bssid to udpate last_rx. > > type - update > >> + if (!(status->flag & RX_FLAG_MCAST)) { >> + sta->rx_stats.last_rx = jiffies; >> + sta->rx_stats.last_rate = >> sta_stats_encode_rate(status); >> + } > > You should probably rename that flag to something like > RX_FLAG_80211_MCAST since otherwise it's confusing with the next > multicast ether addr check: Sure. > >> + if (sdata->vif.type == NL80211_IFTYPE_STATION && >> + !is_multicast_ether_addr(ehdr->h_dest)) >> + ieee80211_sta_reset_conn_monitor(sdata); > > But this could just also use the flag, since in station mode the two > are equivalent, and it'd be easier to figure out if this was "else if > (station mode)"? Ah, correct. > >> + memset(&rx, 0, sizeof(rx)); > > That seems a bit pointless? Hmmm, is it not that all member other than the ones initialized will be junk and might result in crash when accessing uninitialized pointer member like napi? > >> + rx.skb = skb; >> + rx.sdata = sdata; >> + rx.local = local; >> + rx.sta = sta; >> + >> + if (vif->type == NL80211_IFTYPE_AP_VLAN && sdata->bss && >> + unlikely(ehdr->h_proto == sdata->control_port_protocol)) >> { >> + sdata = container_of(sdata->bss, struct >> ieee80211_sub_if_data, >> + u.ap); >> + dev = sdata->dev; >> + rx.sdata = sdata; >> + } >> + >> + rx.skb->dev = dev; >> + >> + /* XXX: Since rx.seqno_idx is not available for decap >> offloaded frames >> + * rx msdu stats update at the seqno_idx in >> ieee80211_deliver_skb() >> + * will always be updated at index 0 and will not be very >> useful. >> + */ >> + ieee80211_deliver_skb(&rx); > > Yeah, that's not nice - perhaps we can provide the TID out of band? Right. This is possible with ath10k where 802.11 header is also available in Rx desc of the frame irrespective of the format of the Rx payload If > not, we'll have to disable those statistics *all the way*, i.e. not > even report them to userspace when filling sinfo. Yes, this may be the option when driver is unaware of rx packet tid. > >> + return; >> + >> +mic_fail: >> + cfg80211_michael_mic_failure(sdata->dev, sta->addr, >> + (status->flag & RX_FLAG_MCAST) >> ? >> + NL80211_KEYTYPE_GROUP : >> + NL80211_KEYTYPE_PAIRWISE, >> + key ? key->conf.keyidx : -1, >> + NULL, GFP_ATOMIC); > > Do we really want to handle that inline here? The driver probably has a > different check to even set RX_FLAG_MMIC_ERROR, so we could just ask it > to call cfg80211_michael_mic_failure() [or a wrapper to get sdata->dev] > instead? I guess this works too though, and might be easier to > understand. Yeah, driver directly reporting MIC failure will be fine. I think a wrapper may be required rather than mac80211 based driver directly calling cfg80211 function? Vasanth
> Ok. So it is mainly for encap there needs to be some capability > advertisement from driver Obviously, since mac80211 needs to pick the ndo struct to use. > because for decap driver would use appropriate mac80211 Rx function > to indicate? That should be sufficient, no? In fact, for RX, assuming that the relevant things are offloaded (and we should ensure that in the RX function(s)), the driver could be allowed to switch around as dynamically as it wants to (as long as it's not mixing in _irqsafe calls or doing other things that might cause reordering) > > > + memset(&rx, 0, sizeof(rx)); > > > > That seems a bit pointless? > > Hmmm, is it not that all member other than the ones initialized will > be junk and might result in crash when accessing uninitialized > pointer member like napi? Oh, ok. I thought it was actually all filled. You really should support NAPI though. > > Yeah, that's not nice - perhaps we can provide the TID out of band? > > Right. This is possible with ath10k where 802.11 header is also > available in Rx desc of the frame irrespective of the format of the > Rx payload We could go with that for now then. Leave a comment indicating that if it can't be provided, we need to disable statistics for it? > > > > > + return; > > > + > > > +mic_fail: > > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr, > > > + (status->flag & > > > RX_FLAG_MCAST) > > > ? > > > + NL80211_KEYTYPE_GROUP : > > > + NL80211_KEYTYPE_PAIRWISE, > > > + key ? key->conf.keyidx : > > > -1, > > > + NULL, GFP_ATOMIC); > > > > Do we really want to handle that inline here? The driver probably > > has a different check to even set RX_FLAG_MMIC_ERROR, so we could > > just ask it to call cfg80211_michael_mic_failure() [or a wrapper to > > get sdata->dev] instead? I guess this works too though, and might > > be easier to understand. > > Yeah, driver directly reporting MIC failure will be fine. I think a > wrapper may be required rather than mac80211 based driver directly > calling cfg80211 function? It would be, because the driver can't get sdata->dev (although I think there's now a hidden path to do this?) johannes
> > > > + return; > > > > + > > > > +mic_fail: > > > > + cfg80211_michael_mic_failure(sdata->dev, sta->addr, > > > > + (status->flag & > > > > RX_FLAG_MCAST) > > > > ? > > > > + NL80211_KEYTYPE_GROUP : > > > > + NL80211_KEYTYPE_PAIRWISE, > > > > + key ? key->conf.keyidx : > > > > -1, > > > > + NULL, GFP_ATOMIC); > > > > > > Do we really want to handle that inline here? The driver probably > > > has a different check to even set RX_FLAG_MMIC_ERROR, so we could > > > just ask it to call cfg80211_michael_mic_failure() [or a wrapper > > > to > > > get sdata->dev] instead? I guess this works too though, and might > > > be easier to understand. > > > > Yeah, driver directly reporting MIC failure will be fine. I think a > > wrapper may be required rather than mac80211 based driver directly > > calling cfg80211 function? > > It would be, because the driver can't get sdata->dev (although I > think there's now a hidden path to do this?) However, we can do both ways, I don't really care that much. It seems possible though that a driver would not even report the frame, but only the necessary info, in this case - so that we might need an out-of-band path for it anyway? johannes
diff --git a/include/net/mac80211.h b/include/net/mac80211.h index 225abaa..75c55e2 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1088,6 +1088,9 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info) * @RX_FLAG_ALLOW_SAME_PN: Allow the same PN as same packet before. * This is used for AMSDU subframes which can have the same PN as * the first subframe. + * @RX_FLAG_MCAST: If the receiver address (addr1) in the frame is multicast. + * This is used with the data frames by the drivers supporting 802.11 hdr + * decap offload. */ enum mac80211_rx_flags { RX_FLAG_MMIC_ERROR = BIT(0), @@ -1123,6 +1126,7 @@ enum mac80211_rx_flags { RX_FLAG_RADIOTAP_VENDOR_DATA = BIT(31), RX_FLAG_MIC_STRIPPED = BIT_ULL(32), RX_FLAG_ALLOW_SAME_PN = BIT_ULL(33), + RX_FLAG_MCAST = BIT_ULL(34), }; #define RX_FLAG_STBC_SHIFT 26 @@ -3989,6 +3993,21 @@ static inline void ieee80211_rx_ni(struct ieee80211_hw *hw, } /** + * ieee80211_rx_decap_offl - Receive frames in 802.11 decapsulated format + * + * Low level driver capable of 802.11 header decap uses this function. The frame + * will be in ethernet format. + * This function may not be called in IRQ context. Calls to this function + * for a single hardware must be synchronized against each other. + * + * @hw: the hardware this frame came in on + * @sta : the station the frame was received from, must not be %NULL + * @skb: the buffer to receive, owned by mac80211 after this call + */ +void ieee80211_rx_decap_offl(struct ieee80211_hw *hw, struct ieee80211_sta *sta, + struct sk_buff *skb); + +/** * ieee80211_sta_ps_transition - PS transition for connected sta * * When operating in AP mode with the %IEEE80211_HW_AP_LINK_PS diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 2e8a902..3cb8d6e 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -2103,13 +2103,14 @@ __ieee80211_data_to_8023(struct ieee80211_rx_data *rx, bool *port_control) return 0; } +static const u8 pae_group_addr[ETH_ALEN] __aligned(2) + = { 0x01, 0x80, 0xC2, 0x00, 0x00, 0x03 }; + /* * requires that rx->skb is a frame with ethernet header */ static bool ieee80211_frame_allowed(struct ieee80211_rx_data *rx, __le16 fc) { - static const u8 pae_group_addr[ETH_ALEN] __aligned(2) - = { 0x01, 0x80, 0xC2, 0x00, 0x00, 0x03 }; struct ethhdr *ehdr = (struct ethhdr *) rx->skb->data; /* @@ -4180,3 +4181,187 @@ void ieee80211_rx_irqsafe(struct ieee80211_hw *hw, struct sk_buff *skb) tasklet_schedule(&local->tasklet); } EXPORT_SYMBOL(ieee80211_rx_irqsafe); + +/* Receive path for decap offloaded data frames */ + +static void +ieee80211_rx_handle_decap_offl(struct ieee80211_sub_if_data *sdata, + struct sta_info *sta, struct sk_buff *skb) +{ + struct ieee80211_local *local = sdata->local; + struct ieee80211_vif *vif = &sdata->vif; + struct net_device *dev = sdata->dev; + struct ieee80211_rx_status *status; + struct ieee80211_key *key = NULL; + struct ieee80211_rx_data rx; + int i; + struct ethhdr *ehdr; + + ehdr = (struct ethhdr *)skb->data; + status = IEEE80211_SKB_RXCB(skb); + + /* TODO: Extend ieee80211_rx_decap_offl() with bssid so that Ethernet + * encap/decap can be supported in Adhoc interface type as well. + * Adhoc interface depends on bssid to udpate last_rx. + */ + if (vif->type != NL80211_IFTYPE_STATION && + vif->type != NL80211_IFTYPE_AP_VLAN && + vif->type != NL80211_IFTYPE_AP) + goto drop; + + I802_DEBUG_INC(local->dot11ReceivedFragmentCount); + + if (!(status->flag & RX_FLAG_MCAST)) { + sta->rx_stats.last_rx = jiffies; + sta->rx_stats.last_rate = sta_stats_encode_rate(status); + } + + if (sdata->vif.type == NL80211_IFTYPE_STATION && + !is_multicast_ether_addr(ehdr->h_dest)) + ieee80211_sta_reset_conn_monitor(sdata); + + sta->rx_stats.fragments++; + + u64_stats_update_begin(&sta->rx_stats.syncp); + sta->rx_stats.bytes += skb->len; + u64_stats_update_end(&sta->rx_stats.syncp); + + if (!(status->flag & RX_FLAG_NO_SIGNAL_VAL)) { + sta->rx_stats.last_signal = status->signal; + ewma_signal_add(&sta->rx_stats_avg.signal, -status->signal); + } + + if (status->chains) { + sta->rx_stats.chains = status->chains; + for (i = 0; i < ARRAY_SIZE(status->chain_signal); i++) { + int signal = status->chain_signal[i]; + + if (!(status->chains & BIT(i))) + continue; + + sta->rx_stats.chain_signal_last[i] = signal; + ewma_signal_add(&sta->rx_stats_avg.chain_signal[i], + -signal); + } + } + + if (status->flag & RX_FLAG_MCAST) { + for (i = 0; i < NUM_DEFAULT_KEYS; i++) { + key = rcu_dereference(sta->gtk[i]); + if (key) + break; + } + } else { + key = rcu_dereference(sta->ptk[sta->ptk_idx]); + } + + if (key && unlikely(key->flags & KEY_FLAG_TAINTED)) + goto drop; + + if (status->flag & RX_FLAG_MMIC_ERROR) { + if (key) + key->u.tkip.mic_failures++; + goto mic_fail; + } + + if (unlikely(!test_sta_flag(sta, WLAN_STA_AUTHORIZED))) { + if (ehdr->h_proto != sdata->control_port_protocol) + goto drop; + else if (!ether_addr_equal(ehdr->h_dest, vif->addr) && + !ether_addr_equal(ehdr->h_dest, pae_group_addr)) + goto drop; + } + + if (unlikely(ehdr->h_proto == cpu_to_be16(ETH_P_TDLS))) { + struct ieee80211_tdls_data *tf = (void *)skb->data; + + if (pskb_may_pull(skb, + offsetof(struct ieee80211_tdls_data, u)) && + tf->payload_type == WLAN_TDLS_SNAP_RFTYPE && + tf->category == WLAN_CATEGORY_TDLS && + (tf->action_code == WLAN_TDLS_CHANNEL_SWITCH_REQUEST || + tf->action_code == WLAN_TDLS_CHANNEL_SWITCH_RESPONSE)) { + skb_queue_tail(&local->skb_queue_tdls_chsw, skb); + schedule_work(&local->tdls_chsw_work); + sta->rx_stats.packets++; + return; + } + } + + memset(&rx, 0, sizeof(rx)); + rx.skb = skb; + rx.sdata = sdata; + rx.local = local; + rx.sta = sta; + + if (vif->type == NL80211_IFTYPE_AP_VLAN && sdata->bss && + unlikely(ehdr->h_proto == sdata->control_port_protocol)) { + sdata = container_of(sdata->bss, struct ieee80211_sub_if_data, + u.ap); + dev = sdata->dev; + rx.sdata = sdata; + } + + rx.skb->dev = dev; + + /* XXX: Since rx.seqno_idx is not available for decap offloaded frames + * rx msdu stats update at the seqno_idx in ieee80211_deliver_skb() + * will always be updated at index 0 and will not be very useful. + */ + ieee80211_deliver_skb(&rx); + + return; + +mic_fail: + cfg80211_michael_mic_failure(sdata->dev, sta->addr, + (status->flag & RX_FLAG_MCAST) ? + NL80211_KEYTYPE_GROUP : + NL80211_KEYTYPE_PAIRWISE, + key ? key->conf.keyidx : -1, + NULL, GFP_ATOMIC); + +drop: + sta->rx_stats.dropped++; + dev_kfree_skb(skb); +} + +/* Receive path handler that a low level driver supporting 802.11 hdr decap + * offload can call. The frame is in ethernet format and the assumption is + * all necessary operations like decryption, defrag, deaggregation, etc. + * requiring 802.11 headers are already performed in the low level driver + * or hardware. + */ +void ieee80211_rx_decap_offl(struct ieee80211_hw *hw, + struct ieee80211_sta *pubsta, struct sk_buff *skb) +{ + struct ieee80211_local *local = hw_to_local(hw); + struct sta_info *sta = container_of(pubsta, struct sta_info, sta); + + if (unlikely(local->quiescing || local->suspended)) + goto drop; + + if (unlikely(local->in_reconfig)) + goto drop; + + if (WARN_ON(!local->started)) + goto drop; + + /* TODO: Sanity checks on flags/rx_flags like done in + * ieee80211_invoke_fast_rx() to confirm if the frame + * has gone through all the functionalities which require + * 802.11 frame header. + */ + + rcu_read_lock(); + + /* TODO: Toggle Rx throughput LED */ + + ieee80211_rx_handle_decap_offl(sta->sdata, sta, skb); + + rcu_read_unlock(); + + return; +drop: + kfree_skb(skb); +} +EXPORT_SYMBOL(ieee80211_rx_decap_offl);
Implement rx path which does fewer processing on the received data frame which has already gone through 802.11 header decapsulation and other functionalities which require 802.11 header in the low level driver or hardware. Currently this rx path is restricted to AP and STA mode, but can be extended for Adhoc mode as well. This rx path only checks if the driver has advertised it's support of 802.11 header encap/decap offload for data frames. It is upto the low level driver to make sure if the frame that it passes is in ethernet format and the sta pointer is valid. Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com> --- include/net/mac80211.h | 19 +++++ net/mac80211/rx.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 206 insertions(+), 2 deletions(-)