Re: [PATCH 7/7] mac80211: add rx decapsulation offload support

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2020-12-16 at 22:19 +0100, Felix Fietkau wrote:
> 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.

Ok, fair point, I didn't count :-)

> > 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.

Right. I've long felt that perhaps we should have tracing for this,
rather than relying on the extra monitor interface, but the monitor
interface is oh so convenient since you can directly use the result for
wireshark etc. :)

I guess I don't really care that much either way. Going with your
approach seems reasonable, but people will have to be aware that "just
add a monitor interface" is now going to affect things more than it used
to.

johannes