Send standard NL80211 commands using DWPAL

[ghost]

From the description of the problem by @mariomaz :

In order to obtain AP VT/VHT/HE capabilities using Netlink and DWPAL we should be sending command NL80211_CMD_GET_WIPHY (as in ap_wlan_hal_nl80211::read_supported_channels).

However, this is something we cannot do as method base_wlan_hal_dwpal::dwpal_nl_cmd_get can be used to send NL80211_CMD_VENDOR command only. This method is internally calling dwpal_driver_nl_cmd_send(), using NL80211_CMD_VENDOR and given sub-command. We should then create another but similar method to call dwpal_driver_nl_cmd_send() with NL80211_CMD_GET_WIPHY instead. However, dwpal_driver_nl_cmd_send() implementation in prplWrt only supports NL80211_CMD_VENDOR command:

       if (nl80211Command != NL80211_CMD_VENDOR /*0x67*/)
       {
                console_printf("%s; non supported command (0x%x); currently we support ONLY NL80211_CMD_VENDOR (0x67) ==> Abort!\n", __FUNCTION__, (unsigned int)nl80211Command);
                return DWPAL_FAILURE;
       }

[tomereli]

Answer is that it is OK to add implementation in DWPAL. Once it is there, we should port it back to Intel so we can get it in the next prplWrt upgrade.

[tomereli]

@mariomaz please implement what you need to support this in DWPAL, create a PR to our gitlab fork which updates dwpal_6x-uci-1.0.0.1.tar.bz2 in https://git.prpl.dev/prplmesh/iwlwav/blob/iwlwav-19.07/dl/dwpal_6x-uci-1.0.0.1.tar.bz2 and https://git.prpl.dev/prplmesh/iwlwav/blob/iwlwav-19.07/dwpal-6x-uci/Makefile.
Then also create a patch and send it in response to the mail from Gur Sannikov, He will port it to Intel.

[kantera800]

@mariomaz you are using an older dwpal version than INTEL's repositories so apparently you would need to implement it.

dwpal_driver_nl_msg_get() already supports non-vendor commands.

I checked the code and you can use NL80211_CMD_GET_SCAN implementation as reference (see dwpal_driver_nl_scan_dump).

you have 2 options:

1. you can propriety implement it , use NL80211_CMD_GET_SCAN implementation dwpal_driver_nl_scan_dump() as reference.
   I would create a non-vendor new callback and then read the response from process_nl_events() .
   interface.driver.nlNonVendorEventCallback = nlEventCallback;

2. you can use dwpal_nl_cmd_get() but it will require changing dwpal_driver_nl_cmd_send()
   to support the new non-vendor commands (NL80211_CMD_GET_WIPHY and NL80211_CMD_GET_STATION).

[mariomaz]

This is a summary of the work done so far regarding this issue.

After reviewing the code of DWPAL and exploring different alternatives to include support for standard NL80211 commands into DWPAL, there exist reasonable doubts about the suitability of this approach. To overcome drawbacks found, another solution is proposed that does not involve the modification of DWPAL.

The final goal is to obtain capabilities and metrics from a WiFi interface. To accomplish this task, NL80211_CMD_GET_WIPHY and NL80211_CMD_GET_STATION commands must be used. Provided solution should be easily extendable to send other standard NL80211 commands in the future.

First of all we tried to send these commands using function base_wlan_hal_dwpal::dwpal_nl_cmd_get() in BWL, which in turn calls dwpal_driver_nl_cmd_send() in DWPAL. However, currently DWPAL supports NL80211_CMD_VENDOR only and thus it had to be extended to support standard commands too.

Two options were suggested by Alex Kanter to add support for standard commands to DWPAL:

1. Create new functions using dwpal_driver_nl_scan_dump() as a reference implementation (this function is sending NL80211_CMD_GET_SCAN command).
2. Modify existing dwpal_driver_nl_cmd_send() to support standard commands too.

Drawbacks of option 1:

• Most of the code in reference function has been borrowed from dwpal_driver_nl_cmd_send. If we follow the same approach for every new command to be added, we will end up with lots of duplicated code.
• Running reference function through dwpal_cli causes a segmentation fault. After restarting dwpal_cli, keyboard was not working inside dwpal_cli and a reboot was required.
• Thread listenerThread is overwriting the nlNonVendorEventCallback field previously set by dwpal_driver_nl_scan_dump() in main thread. Besides the fact the value is being overwritten, both threads should not be writing to the same variable (context) without a mutex.
• In dwpal_cli.cpp, oneShotCommandVal (-c command line argument) can be either a NL80211_CMD_* command or a subcommand of NL80211_CMD_VENDOR command (commands and subcommands are mixed). As a side effect, subcommands with a value matching that of a supported command cannot be executed.

Drawbacks of option 2:

• Despite of its name, dwpal_driver_nl_cmd_send() design can be used to send vendor commands only. Another limitation is that such commands cannot return any output data. Extend this function to suit all our needs would require a very big refactoring: it must support vendor and standard commands, that may or may not require parameters, that may or may not return output data (in one or a variable amount of reply messages), that may or may not require special flags to run. Even if such refactoring was done, parsing results must be done differently for each command and current design does not allow it.
• The same happens with base_wlan_hal_dwpal::dwpal_nl_cmd_get and base_wlan_hal_dwpal::dwpal_nl_cmd_set methods: they are valid for vendor commands only, despite of their names, and would also require a refactoring.

Drawbacks of options 1 and 2:

• Executing some commands result in two or more reply messages (e.g.: NL80211_CMD_GET_SCAN returns two NL80211_CMD_NEW_SCAN_RESULTS). Since dwpal_command_get_ended_socket_wait() waits for and returns the first reply message only, result will be incomplete for these commands.
• Response timeout is fixed to 1s in dwpal_command_get_ended_socket_wait. There may be some commands which require more than 1s to execute.

Even if there were no drawbacks, if support for standard commands was added to DWPAL, then such support would be available to the DWPAL version of BWL only (i.e.: RAX40). Such support should then be added to the NL80211 version of BWL too, ending up with duplicated code. Support for every new command from now on should be added twice. And worst of all: DWPAL is out of the prplMesh GIT repository.

Because we need support for standard NL80211 commands, that support can be added out of DWPAL so it will be available to all devices. Moreover, we already have one implementation to start with and it is C++ (see base_wlan_hal_nl80211::send_nl80211_msg). We could move it to a common place like BWL and use it from wherever is needed. We could create a netlink_socket base class and a nl80211_socket derived class, so we could support other netlink commands too (apart from nl80211).

Note: while reviewing DWPAL code, some other bugs and nitpicks where found:

• dwpal_cli does not show errors unless console_printf is modified to call printf
• malloc + memset could be replaced with calloc
• This code

stringLength = strnlen_s(optarg, DWPAL_GENERAL_STRING_LENGTH);
strncpy_s(interfaceType, sizeof(interfaceType), optarg, stringLength);
interfaceType[stringLength] = '\0';

should be replaced with

stringLength = strnlen_s(optarg, DWPAL_GENERAL_STRING_LENGTH - 1);
strncpy_s(interfaceType, sizeof(interfaceType), optarg, stringLength);

Such code pattern is repeated in several places in dwpal_cli.

• Parameters to interfaceIndexGet() do not match with those in logged message (occurs several times)

if ((idx = interfaceIndexGet("Driver", "ALL")) == -1)
{
  console_printf("%s; interfaceIndexGet (radioName= 'wlan0', serviceName= 'Both')

• Macro COMMAND_ENDED_SOCKET is defined twice, both in dwpal_ext.c and dwpal_ext.h
• This may be an error, but I’m not sure:
  - In dwpal_cli, flag isDwpalExtenderMode is set to true either when DWPAL_EXT_DRIVER_NL_IF_ATTACH or DWPAL_EXT_DRIVER_NL_IF_DETACH are executed. Shouldn't it be set to false in the second case?
  - isDwpalExtenderMode is set to true also when DWPAL_EXT_HOSTAP_IF_ATTACH is executed but not set when DWPAL_EXT_HOSTAP_IF_DETACH is executed. Shouldn't it be set in the second case?
  - If isDwpalExtenderMode is set to true either when DWPAL_EXT_DRIVER_NL_IF_ATTACH or DWPAL_EXT_HOSTAP_IF_ATTACH are executed because listenerThread is started, shouldn't such initialization be moved to listenerThreadStart?
• A segmentation fault occurs in dwpal_cli if DWPAL_INIT is issued and then and a mobile phone is connected to the “prplMesh” SSID.
• Using a mutex plus a condition variable to communicate different threads of the same process should be more efficient than using Unix sockets (socket dwpal_command_get_ended is used to tell the main thread that resposne has been received and a buffer of up to 8Kb is copied through the socket)

[mariomaz]

It has finally been decided that we should not implement support for the generic nl80211 commands in DWPAL, so this issue is being closed.

[mariomaz]

See "Feature/add support for nl80211 standard commands to BWL #900"