Message ID | 20190612193510.29489-1-sven@narfation.org (mailing list archive) |
---|---|
Headers | show |
Series | mac80211/ath11k: HE mesh support | expand |
Hi Sven, Two comments: 1) It seems to me that patches 1/2 really should be in opposite order, since you can't really claim HE mesh support in hwsim when you don't have it in mac80211? Or maybe I misunderstand? 2) This series breaks the wpas_mesh_max_peering wpa_supplicant/hwsim test, and I'm not sure why. Perhaps some length calculations are bad? johannes
Hi, Thanks for clarifying your statement regarding ie_len. I think I should have been able to guess what you meant but for some reason my brain wasn't able to understand the phrase at that time. On Friday, 14 June 2019 16:10:46 CEST Johannes Berg wrote: > Two comments: > > 1) It seems to me that patches 1/2 really should be in opposite order, > since you can't really claim HE mesh support in hwsim when you don't > have it in mac80211? > Or maybe I misunderstand? I personally didn't have an opinion regarding the patch order. It was just the order how I committed the stuff. And it was just committed in this order because I could easier amend changes to mac80211. But yes, (in retrospective) it is a lot better to have first the mac80211 change and then the driver changes for meshpoint. > 2) This series breaks the wpas_mesh_max_peering wpa_supplicant/hwsim > test, and I'm not sure why. Perhaps some length calculations are bad? I just went through all tests and saw following failed ones before the patches: failed tests: tnc_peap_soh tnc_peap_soh_errors tnc_ttls tnc_ttls_fragmentation tnc_fast ap_ft_ptk_rekey p2p_go_move_reg_change p2ps_connect_adv_go_persistent p2ps_channel_both_connected_different ap_wps_conf_5ghz ap_wps_upnp_http_proto wpas_mesh_gate_forwarding olbc proxyarp_open_ebtables p2p_device_persistent_group2 dpp_ap_config_p521_p521 wnm_bss_tm_reject and following after the patches: failed tests: tnc_peap_soh tnc_peap_soh_errors tnc_ttls tnc_ttls_fragmentation tnc_fast ap_ft_ptk_rekey ap_acs_vht discovery_group_client p2p_go_move_reg_change p2ps_stale_group_removal2 ap_wps_conf_5ghz ap_wps_upnp_http_proto radius_acct_interim_unreachable mesh_secure_ocv_mix_legacy mesh_secure_ocv_mix_ht wpas_mesh_max_peering wpas_mesh_open_ht40 wpas_mesh_gate_forwarding rrm_neighbor_rep_oom rrm_beacon_req_passive_scan_vht ap_vht160b ap_vht160_no_dfs_112_minus proxyarp_open_ebtables So as you can see, even more stuff failed which I haven't touched. And other stuff which I haven't touched now work. The interesting ones were: * mesh_secure_ocv_mix_legacy * mesh_secure_ocv_mix_ht * wpas_mesh_open_ht40 * wpas_mesh_max_peering The last one (mentioned by you) is interesting - because I can see the accepting additional peers == No for the beacons in the dump. But it is not recognized as such by the script. With new tshark: ~/tmp/wireshark/build/run/tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' 02:00:00:00:01:00 0x00000009 02:00:00:00:00:00 0x00000009 02:00:00:00:01:00 0x00000009 02:00:00:00:02:00 0x00000009 02:00:00:00:00:00 0x00000008 02:00:00:00:01:00 0x00000009 02:00:00:00:02:00 0x00000009 02:00:00:00:00:00 0x00000008 With wireshark 3.0.0: tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' 02:00:00:00:01:00 0x00000001 02:00:00:00:00:00 0x00000001 02:00:00:00:01:00 0x00000001 02:00:00:00:00:00 0x00000001 02:00:00:00:02:00 0x00000001 02:00:00:00:01:00 0x00000001 02:00:00:00:02:00 0x00000001 02:00:00:00:00:00 0x00000001 So I had to change the wireshark version (see below) which is used by hostapd. (just to document for me what I have forced it to a different version) diff --git a/tests/hwsim/tshark.py b/tests/hwsim/tshark.py index 019df781a760c657b8854acfcee94dc83e30575f..ecf83a7a97dde0e52b54e994d2dd4d46bddaa9df 100644 --- a/tests/hwsim/tshark.py +++ b/tests/hwsim/tshark.py @@ -28,7 +28,7 @@ def _run_tshark(filename, filter, display=None, wait=True): time.sleep(0.1) try: - arg = ["tshark", "-r", filename, + arg = ["/home/sven/tmp/wireshark/build/run/tshark", "-r", filename, _tshark_filter_arg, filter] if display: arg.append('-Tfields') @@ -102,7 +102,7 @@ def run_tshark(filename, filter, display=None, wait=True): wait) def run_tshark_json(filename, filter): - arg = ["tshark", "-r", filename, + arg = ["/home/sven/tmp/wireshark/build/run/tshark", "-r", filename, _tshark_filter_arg, filter] arg.append('-Tjson') arg.append('-x') The first three things looks like wpa_supplicant problems. Seems to be caused by the way HE forces VHT to be enabled and now the tests fail which try to disable VHT. There was already a patch [0] for that but it wasn't considered a good solution. But beside these three things there are various other problems in wpa_supplicant regarding HE with pending patches. So I've used wpa_supplicant 91b6eba7732354ed3dfe0aa9715dc4c0746e3336 with two additional patches [1,2] and increased the VM memory to 1024 MB. Also wireshark (tshark to be more precise) had to be updated to v3.1.0rc0-1192-gf64990438c Kind regards, Sven [0] https://patchwork.ozlabs.org/patch/1125305/ [1] https://patchwork.ozlabs.org/patch/1125314/ [2] https://patchwork.ozlabs.org/patch/1125322/
On Wed, 2019-07-03 at 11:23 +0200, Sven Eckelmann wrote: > > ~/tmp/wireshark/build/run/tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' > 02:00:00:00:01:00 0x00000009 > 02:00:00:00:00:00 0x00000009 > 02:00:00:00:01:00 0x00000009 > 02:00:00:00:02:00 0x00000009 > 02:00:00:00:00:00 0x00000008 > 02:00:00:00:01:00 0x00000009 > 02:00:00:00:02:00 0x00000009 > 02:00:00:00:00:00 0x00000008 > > With wireshark 3.0.0: > > tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' > 02:00:00:00:01:00 0x00000001 > 02:00:00:00:00:00 0x00000001 > 02:00:00:00:01:00 0x00000001 > 02:00:00:00:00:00 0x00000001 > 02:00:00:00:02:00 0x00000001 > 02:00:00:00:01:00 0x00000001 > 02:00:00:00:02:00 0x00000001 > 02:00:00:00:00:00 0x00000001 > > So I had to change the wireshark version (see below) which is used by hostapd. > (just to document for me what I have forced it to a different version) What. +hostap list. This makes no sense, is that a tshark bug in one of the versions? Maybe we should just use the JSON output, and parse that, but if tshark now has a different idea of what the "wlan.mesh.config.cap" field is, that won't help us at all? Older versions were misparsing the HE element, but that has a length so should only affect the HE element *itself*? So ultimately, what do we do here? Should we take this and sort out the tests? johannes
On Friday, 12 July 2019 09:58:50 CEST Johannes Berg wrote: > On Wed, 2019-07-03 at 11:23 +0200, Sven Eckelmann wrote: > > > > ~/tmp/wireshark/build/run/tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' > > 02:00:00:00:01:00 0x00000009 [...] > > With wireshark 3.0.0: > > > > tshark -r /tmp/hwsim-test-logs/latest/wpas_mesh_max_peering.hwsim0.pcapng -T fields -e wlan.sa -e wlan.mesh.config.cap 'wlan.fc.type_subtype == 8' > > 02:00:00:00:01:00 0x00000001 [...] > > So I had to change the wireshark version (see below) which is used by hostapd. > > (just to document for me what I have forced it to a different version) > > What. +hostap list. > > This makes no sense, is that a tshark bug in one of the versions? Yes (but more a bug in the ieee80211 dissector), see commit f3ef8575d462 ("ieee80211: fix wrong offset for mesh configuration capability bitmask") [1]. I've also attached the pcap in case there are still doubts about my statement. As you can see, it will just fail to parse the mesh peering management element correctly. It should parse the last byte of the element payload but it falls back to parsing the first byte (path selection protocol) as capabilities. See 9.4.2.98 "Mesh Configuration element" from 802.11-2016 for details. There is already a workaround for that in the hostap testcases: if all_cap_one: # It looks like tshark parser was broken at some point for # wlan.mesh.config.cap which is now (tshark 2.6.3) pointing to incorrect # field (same as wlan.mesh.config.ps_protocol). This used to work with # tshark 2.2.6. # # For now, assume the capability field ends up being the last octet of # the frame. one = [0, 0, 0] zero = [0, 0, 0] addrs = [addr0, addr1, addr2] for idx in range(3): addr = addrs[idx] out = run_tshark_json(capfile, filt + " && wlan.sa == " + addr) pkts = json.loads(out) for pkt in pkts: frame = pkt["_source"]["layers"]["frame_raw"][0] cap = int(frame[-2:], 16) if cap & 0x01: one[idx] += 1 else: zero[idx] += 1 logger.info("one: " + str(one)) logger.info("zero: " + str(zero)) But maybe you already spotted the problem - it requires that mesh configuration element is the last element. Which is not the case here - similar to 5GHz tests (where you have most likely a VHT cap/oper element after the meshconf_ie). I hope that this makes more sense now. Kind regards, Sven [1] https://github.com/wireshark/wireshark/commit/f3ef8575d4620a62f1c4609bf14961c3e78993f3
On Fri, 2019-07-12 at 11:36 +0200, Sven Eckelmann wrote: > > There is already a workaround for that in the hostap testcases: > > if all_cap_one: > # It looks like tshark parser was broken at some point for > # wlan.mesh.config.cap which is now (tshark 2.6.3) pointing to incorrect > # field (same as wlan.mesh.config.ps_protocol). This used to work with > # tshark 2.2.6. > # > # For now, assume the capability field ends up being the last octet of > # the frame. > But maybe you already spotted the problem - it requires that mesh > configuration element is the last element. Which is not the case here - > similar to 5GHz tests (where you have most likely a VHT cap/oper element > after the meshconf_ie). > > I hope that this makes more sense now. Ah, yes, it does. So I guess we need to update/fix that workaround. And I guess newer tshark (which you used) is fixed again, if I understand correctly? johannes
On Friday, 12 July 2019 11:42:51 CEST Johannes Berg wrote: > On Fri, 2019-07-12 at 11:36 +0200, Sven Eckelmann wrote: > > > > There is already a workaround for that in the hostap testcases: > > > > if all_cap_one: > > # It looks like tshark parser was broken at some point for > > # wlan.mesh.config.cap which is now (tshark 2.6.3) pointing to incorrect > > # field (same as wlan.mesh.config.ps_protocol). This used to work with > > # tshark 2.2.6. > > # > > # For now, assume the capability field ends up being the last octet of > > # the frame. > > > But maybe you already spotted the problem - it requires that mesh > > configuration element is the last element. Which is not the case here - > > similar to 5GHz tests (where you have most likely a VHT cap/oper element > > after the meshconf_ie). > > > > I hope that this makes more sense now. > > Ah, yes, it does. So I guess we need to update/fix that workaround. I will prepare a patch now for hostap after lunch. > And > I guess newer tshark (which you used) is fixed again, if I understand > correctly? Yes and no, the master branch is fixed. But unfortunately, there is no release with this fix. And the problems is there since commit 3c427376579a ("802.11: Use proto_tree_add_bitmask") and release v2.4.0rc0. But unfortunately, the workaround was added with commit 2cbaf0de223b ("tests: Work around tshark bug in wpas_mesh_max_peering") instead of bringing a fix upstream. Kind regards, Sven