prplfoundation / prplMesh

This repository moved to https://gitlab.com/prpl-foundation/prplmesh/prplMesh
Other
65 stars 32 forks source link

[TASK] TLVF: Correctly create and parse Vendor Extension subelements #667

Closed tomereli closed 4 years ago

tomereli commented 4 years ago

Description

We currently have multiple attributes and classes all with the same attribute type - vendor extension attribute - sWscAttrVersion2, sWscAttrVendorExtMultiAp, cWscVendorExtWfa, and cWscAttrVersion2. All are basically wrong since they do not support the generic vendor extension attribute which does not necessarily have to be WFA vendor extension.

On TX - In one (M1) we create a WFA vendor extension that contains a single version2 subelement. In other cases (M2 config data), we create a WFA vendor extension that contains a single MultiAP subelement containing the bss_type.

On RX - parsing M1 or M2 can fail if there are other subelements and the one we are looking for is not the first. Parsing a vendor-specific vendor extension will surely fail for obvious reasons.

This has been discussed in #661 and one optional solution was proposed in https://github.com/prplfoundation/prplMesh/pull/661#discussion_r366092254 which is to create a single vendor extension class which will have unknown length list of uint8_t for all of the vs_data, this includes the 3 bytes vendor id and all the subelements, which can then be parsed manually. For easy create, we can keep the tlvf structures for version2 and MultiAP vendor extension and simply copy it to the unknown length list as raw data with default values.

Another option is to create and parse the vendor extension vs_data as a AttrList, similar to how we parse M1, M2, and config data.

In any case, the following should be the yaml file for the cAttrVendorExt class:

cWscVendorExt:
  _type: class
  _is_tlv_class: True
  type:
    _type: eWscAttributes
    _value: ATTR_VENDOR_EXTENSION
  length:
    _type: uint16_t
    _length_var: True
  vs_data:
    _type: uint8_t
    _length: [ ]

Testing

tomereli commented 4 years ago

TLVF: Correctly create and parse vendor extension attributes

rmelotte commented 4 years ago

We previously thought some of the certified device of the WFA testbed were configuring prplMesh incorrectly, but after investigating for the 4.10.4 flow, it turns out it's prplMesh issue.

Here is the (decrypted) content of the M2s sent by the Qualcomm controller:

10 45 00 0e 4d 75 6c 74 69 2d 41 50 2d 32 34 47 
2d 31 10 03 00 02 00 20 10 0f 00 02 00 08 10 27 
00 09 6d 61 70 72 6f 63 6b 73 31 10 20 00 06 10 
0c 6b a1 bd 6c 10 49 00 09 00 37 2a 00 01 20 06 
01 20 10 1e 00 08 b4 c1 30 9f 64 00 4e 2a 

10 45 00 0e 4d 75 6c 74 69 2d 41 50 2d 32 34 47 
2d 32 10 03 00 02 00 20 10 0f 00 02 00 08 10 27 
00 09 6d 61 70 72 6f 63 6b 73 32 10 20 00 06 10 
0c 6b a1 bd 6c 10 49 00 09 00 37 2a 00 01 20 06 
01 40 10 1e 00 08 5e c0 2d 91 5a 5e 71 03  

Here is the (decrypted) content of the M2s sent by the Mediatek controller:

10 45 00 0e 4d 75 6c 74 69 2d 41 50 2d 32 34 47 
2d 31 10 03 00 02 00 20 10 0f 00 02 00 08 10 27 
00 09 6d 61 70 72 6f 63 6b 73 31 10 20 00 06 10 
0c 6b a1 bd 6c 10 49 00 09 00 37 2a 06 01 20 00 
01 20 10 1e 00 08 8c d9 8e 92 ac 55 ed 26 

10 45 00 0e 4d 75 6c 74 69 2d 41 50 2d 32 34 47 
2d 32 10 03 00 02 00 20 10 0f 00 02 00 08 10 27 
00 09 6d 61 70 72 6f 63 6b 73 32 10 20 00 06 10 
0c 6b a1 bd 6c 10 49 00 09 00 37 2a 06 01 40 00 
01 20 10 1e 00 08 43 47 3b 97 96 45 77 ba 

From the prplMesh logs, we initially thought the Qualcomm device was configuring prplMesh with 2 fronthauls, because of the following sequence that is present for the 2 vaps: 00 37 2a 00 01 20 (the 0x20 value for a bss_type corresponds to a fronthaul).

However, after a closer inspection, it turns out both the Qualcomm and the Mediatek M2s are correct. The difference between the 2 is that the Qualcomm device sends a "Version2" subelement first (indicated by the 00 byte after the 00 37 2a vendor ID), and then only the multiAP subelement (whose subelement ID is 06).

The Mediatek device however sends the multiAP subelement first, and only after the Version2 subelement.

The current parsing has at least the following issues:

Because of this, in the case of the Qualcomm device, we take the Version2 value field (0x20) for a bss_type (and 0x20 turns out to be a valid value for a bss_type).

We swapped the certified controller for a similar reason in 4.10.3 (https://git.prpl.dev/prplmesh/wfa-certification/easymesh_cert/-/commit/1e295f738574bfdebc492ca70a5267ef09ff637d) we have to check if the issue is the same and if it is revert back to the controller used in the test plan.