On 2020-12-16 22:03, Johannes Berg wrote: > On Wed, 2020-12-16 at 21:43 +0100, Felix Fietkau wrote: >> >> + offload = assign && >> + (sdata->vif.offload_flags & IEEE80211_OFFLOAD_DECAP_ENABLED); >> + >> + if (offload) >> + set_offload = !test_and_set_sta_flag(sta, WLAN_STA_DECAP_OFFLOAD); >> + else >> + set_offload = test_and_clear_sta_flag(sta, WLAN_STA_DECAP_OFFLOAD); >> + >> + if (set_offload) >> + drv_sta_set_decap_offload(local, sdata, &sta->sta, assign); > > Some of these lines look a bit long? Just a tiny bit over 80 characters. Wasn't the 80 characters line limit removed a while back? I don't think line wrapping would make things more readable here. >> - skb = ieee80211_rx_monitor(local, skb, rate); >> + if (!(status->flag & RX_FLAG_8023)) >> + skb = ieee80211_rx_monitor(local, skb, rate); > > Is that worth the check? You basically disable it anyway if monitor > interfaces are there. There could be a race. The driver or hw might have queued up some 802.3 frames after offload was disabled. > Not sure that's really the right thing to do ... we often want monitor > interfaces (with no flags set) for debug? > > Or maybe we should add some tracing then (instead). Tracing probably makes more sense. I'm not sure pcap or radiotap can deal with a mix of 802.3 and 802.11 packets. Leaving offload enabled and silently dropping 802.3 packets seems like a bad idea to me as well. - Felix
