mbox series

[v3,0/3] mac80211/ath11k: HE mesh support

Message ID 20190612193510.29489-1-sven@narfation.org (mailing list archive)
Headers show
Series mac80211/ath11k: HE mesh support | expand

Message

Sven Eckelmann June 12, 2019, 7:35 p.m. UTC
Hi,

Some features of 802.11ax without central organizing (AP) STA can also be
used in mesh mode. The main goal is to get HE mesh working with ath11k.
For persons without ath11k compatible hw, hwsim can be used in the as basis
for further development of these features.


* v3

  - force ath11k PHY mode for meshpoint vif to HE mode to avoid hang of
    firmware when HE (or VHT on 2.4GHz) device tries to connect

* v2:

  - add of ath11k patch

* v1:

  - initial RFC


Kind regards,
        Sven

Sven Eckelmann (3):
  mac80211_hwsim: Register support for HE meshpoint
  mac80211: implement HE support for mesh
  ath11k: register HE mesh capabilities

 drivers/net/wireless/ath/ath11k/mac.c |  63 ++++++
 drivers/net/wireless/mac80211_hwsim.c | 283 +++++++++++++++++---------
 include/net/cfg80211.h                |  19 ++
 net/mac80211/ieee80211_i.h            |   2 +
 net/mac80211/mesh.c                   |  61 ++++++
 net/mac80211/mesh.h                   |   4 +
 net/mac80211/mesh_plink.c             |  11 +-
 net/mac80211/util.c                   |  52 +++++
 8 files changed, 400 insertions(+), 95 deletions(-)

Comments

Johannes Berg June 14, 2019, 2:10 p.m. UTC | #1
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
Sven Eckelmann July 3, 2019, 9:23 a.m. UTC | #2
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/
Johannes Berg July 12, 2019, 7:58 a.m. UTC | #3
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
Sven Eckelmann July 12, 2019, 9:36 a.m. UTC | #4
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
Johannes Berg July 12, 2019, 9:42 a.m. UTC | #5
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
Sven Eckelmann July 12, 2019, 10:38 a.m. UTC | #6
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