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
