On Tue, Jan 15, 2019 at 12:50:33AM +0200, Lauri Tirkkonen wrote:
> On Mon, Jan 14 2019 16:41:13 +0200, Lauri Tirkkonen wrote:
> > > Indeed, my diff was bad as well. Thanks for spotting these issues.
> > > I hadn't run this diff yet cause I was still building a new snapshot
> > > to test it. Could you also test this new version please?
> >
> > I'm not currently physically near my AP, but the diff looks good and I
> > can test it later today.
>
> I've had a chance to test your diff, and it seems to work well, but I
> have one behavior change I can't explain (haven't looked at packet
> captures at this point): without this diff applied, or with my own
> previous diff applied, I can run iperf3 -s on my Android phone and
> connect to it with UDP iperf3 -c from a wired host connected to the AP,
> and measure packet loss. With this diff applied I'm unable to connect to
> the phone's iperf server, and in fact am getting huge packet loss even
> pinging the phone (and it seems that the times that it responds to pings
> are correlated with times the phone was actually transmitting, eg. when
> I hit refresh in the phone browser).
>
> Maybe it could be a good thing, perhaps related to aggressive power
> management on the phone, but I don't know what it is with your diff that
> could cause this behavior change. Maybe I need to do some packet
> captures later.
Turns out we need to allow "no data" frames to pass a bit further into
ieee80211_input() because they carry sequence numbers and because they
are used to toggle power-saving state via IEEE80211_FC1_PWR_MGT in the
frame header.
I've checked what Linux mac80211 does -- it ignores "no data" frames
for A-MPDU reordering like we already did (good!), but it drops them
right before decryption. So your original diff was actually correct,
apart from the memory leak ;-)
Does this diff work? You should now be able to see 'no data' frames
on your AP with 'tcpdump -n -i athn0 -y IEEE802_11_RADIO' as well.
Index: ieee80211.h
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211.h,v
retrieving revision 1.60
diff -u -p -r1.60 ieee80211.h
--- ieee80211.h 2 Jul 2017 14:48:19 -0000 1.60
+++ ieee80211.h 14 Jan 2019 13:29:35 -0000
@@ -140,13 +140,13 @@ struct ieee80211_htframe_addr4 { /* 11n
#define IEEE80211_FC0_SUBTYPE_CF_END_ACK 0xf0
/* for TYPE_DATA (bit combination) */
#define IEEE80211_FC0_SUBTYPE_DATA 0x00
-#define IEEE80211_FC0_SUBTYPE_CF_ACK 0x10
-#define IEEE80211_FC0_SUBTYPE_CF_POLL 0x20
-#define IEEE80211_FC0_SUBTYPE_CF_ACPL 0x30
+#define IEEE80211_FC0_SUBTYPE_DATA_CF_ACK 0x10
+#define IEEE80211_FC0_SUBTYPE_DATA_CF_POLL 0x20
+#define IEEE80211_FC0_SUBTYPE_DATA_CF_ACKPOLL 0x30
#define IEEE80211_FC0_SUBTYPE_NODATA 0x40
-#define IEEE80211_FC0_SUBTYPE_CFACK 0x50
-#define IEEE80211_FC0_SUBTYPE_CFPOLL 0x60
-#define IEEE80211_FC0_SUBTYPE_CF_ACK_CF_ACK 0x70
+#define IEEE80211_FC0_SUBTYPE_NODATA_CF_ACK 0x50
+#define IEEE80211_FC0_SUBTYPE_NODATA_CF_POLL 0x60
+#define IEEE80211_FC0_SUBTYPE_NODATA_CF_ACKPOLL 0x70
#define IEEE80211_FC0_SUBTYPE_QOS 0x80
#define IEEE80211_FC1_DIR_MASK 0x03
Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.202
diff -u -p -r1.202 ieee80211_input.c
--- ieee80211_input.c 7 Aug 2018 18:13:14 -0000 1.202
+++ ieee80211_input.c 15 Jan 2019 09:29:21 -0000
@@ -405,6 +405,13 @@ ieee80211_input(struct ifnet *ifp, struc
goto out;
}
+ /* Do not process "no data" frames any further. */
+ if (subtype & IEEE80211_FC0_SUBTYPE_NODATA) {
+ if (ic->ic_rawbpf)
+ bpf_mtap(ic->ic_rawbpf, m, BPF_DIRECTION_IN);
+ goto out;
+ }
+
if ((ic->ic_flags & IEEE80211_F_WEPON) ||
((ic->ic_flags & IEEE80211_F_RSNON) &&
(ni->ni_flags & IEEE80211_NODE_RXPROT))) {
