Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

[DCS Patch 1/5] port dynamic channel selection from rdkb bwl #637

Merged
merged 4 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/beerocks/bwl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ if(BWL_TYPE STREQUAL "DWPAL")
)

find_package(dwpal REQUIRED)
list(APPEND BWL_LIBS dwpal)
list(APPEND BWL_LIBS dwpal nl-3 nl-genl-3)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's better to create a FindNL cmake file and use the standard cmake find_package().
Here's an example: https://github.com/nasa/channel-emulator/blob/master/cmake/Modules/FindLibNL.cmake

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example: https://github.com/nasa/channel-emulator/blob/master/cmake/Modules/FindLibNL.cmake

Careful with that! The project is GPL-2.0 so presumably that file is GPL-2.0 as well. As such it's OK if we include GPL-2.0 files that are used during the build (it just means that if you distribute the source code, you need to give the recipient the right to redistribute that particular file further). But it's making our license more complicated. If you do take over that file, it should get a copyright header including authorship information of the original (which is difficult to retrieve since they probably also copied it from somewhere else).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was just an example. We should write our own cmake find file for libnl..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As agreed, and since its already exists for other libraries, lets do this in a separate PR as part #665.
Keeping the comment open for that.


# safec library can come from either libsafec or slibc
find_package(safec QUIET)
Expand Down
6 changes: 6 additions & 0 deletions common/beerocks/bwl/dummy/base_wlan_hal_dummy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ bool base_wlan_hal_dummy::dummy_send_cmd(const std::string &cmd) { return false;

bool base_wlan_hal_dummy::dummy_send_cmd(const std::string &cmd, char **reply) { return false; }

bool base_wlan_hal_dummy::process_nl_events()
{
LOG(ERROR) << __func__ << "not implemented";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we agree to remove the __func__ from all logs?
#665

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

first time i hear about it. why?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is redundant - see @vitalybu 's comments

return false;
}

bool base_wlan_hal_dummy::refresh_radio_info()
{
if (get_iface_name() == "wlan2") {
Expand Down
1 change: 1 addition & 0 deletions common/beerocks/bwl/dummy/base_wlan_hal_dummy.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class base_wlan_hal_dummy : public virtual base_wlan_hal,
virtual bool refresh_radio_info() override;
virtual bool refresh_vaps_info(int id) override;
virtual bool process_ext_events() override;
virtual bool process_nl_events() override;
virtual std::string get_radio_mac() override;

// Protected methods
Expand Down
6 changes: 6 additions & 0 deletions common/beerocks/bwl/dwpal/ap_wlan_hal_dwpal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,12 @@ bool ap_wlan_hal_dwpal::process_dwpal_event(char *buffer, int bufLen, const std:
return true;
}

bool ap_wlan_hal_dwpal::process_dwpal_nl_event(struct nl_msg *msg)
{
LOG(ERROR) << __func__ << "isn't implemented by this derived and shouldn't be called";
return false;
}

} // namespace dwpal

std::shared_ptr<ap_wlan_hal> ap_wlan_hal_create(std::string iface_name, hal_conf_t hal_conf,
Expand Down
1 change: 1 addition & 0 deletions common/beerocks/bwl/dwpal/ap_wlan_hal_dwpal.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class ap_wlan_hal_dwpal : public base_wlan_hal_dwpal, public ap_wlan_hal {
// Protected methods:
protected:
virtual bool process_dwpal_event(char *buffer, int bufLen, const std::string &opcode) override;
virtual bool process_dwpal_nl_event(struct nl_msg *msg) override;

// Overload for AP events
bool event_queue_push(ap_wlan_hal::Event event, std::shared_ptr<void> data = {})
Expand Down
185 changes: 185 additions & 0 deletions common/beerocks/bwl/dwpal/base_wlan_hal_dwpal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,20 @@ bool base_wlan_hal_dwpal::fsm_setup()
attached = true;
}

//Attach NL interface
if (!m_dwpal_nl_ctx) {
//Attach nl interface
if (dwpal_driver_nl_attach(&m_dwpal_nl_ctx) == DWPAL_FAILURE) {
LOG(ERROR)
<< "dwpal_driver_nl_attach returned ERROR for: "
<< m_radio_info.iface_name << "disbling netlink for this platform";
m_dwpal_nl_ctx = nullptr;
} else {
LOG(DEBUG)
<< "dwpal_driver_nl_attach() success for: " << m_radio_info.iface_name;
}
}

if (attached) {
if (get_type() != HALType::Station) {
transition.change_destination(dwpal_fsm_state::GetRadioInfo);
Expand Down Expand Up @@ -292,6 +306,22 @@ bool base_wlan_hal_dwpal::fsm_setup()
// Success
LOG(DEBUG)
<< "Open and attach an event interface to wpa_supplicant/hostapd - SUCCESS!";

// Get the nl event interface file descriptor
if (!m_dwpal_nl_ctx) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dwpal netlink context is created in the Init state. On failure, we transition the FSM to the Detach state.
In what scenario do we get to the Attach state without a valid context?
I think this if statement is redundant.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure it this test is mandatory.
I added it since we do the same with the VAPs .

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, this should be removed as part of #665

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is a live example why it should stay.

netgear-rax40-1_logs:
DEBUG 17:21:12:420 <1991208208>
base_wlan_hal_dwpal.cpp[148] --> dwpal_driver_nl_attach() success for radio =wlan0
...
DEBUG 17:21:13:264 <1991208208>
base_wlan_hal_dwpal.cpp[394] --> DWPAL FSM wlan0 Attach State: Attach
INFO 17:21:13:264 <1991208208>
ap_manager_thread.cpp[343] --> waiting to attach to wlan0
DEBUG 17:21:13:665 <1991208208>
base_wlan_hal_dwpal.cpp[306] --> Open and attach an event interface to wpa_supplicant/hostapd - SUCCESS!
ERROR 17:21:13:665 <1991208208>
base_wlan_hal_dwpal.cpp[322] --> Can't get event fd since dwpal ctx is NULL!
INFO 17:21:13:666 <1991208208>
beerocks_state_machine.h[53] --> 'Attach': Overriding destination for 'Attach' from 'Operational' to 'Detach'
DEBUG 17:21:13:666 <1991208208>
base_wlan_hal_dwpal.cpp[348] --> dwpal_fsm_state::Detach

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. Remember that RAX40 has a much older driver version at the moment so the attach will fail
We mustn't fail the FSM on that! Just log an warning and proceed without NL. Similar to what we do when we lack permissions to open raw socket - we simply don't open it.

if (dwpal_driver_nl_fd_get(m_dwpal_nl_ctx, &m_fd_nl_events, &m_fd_nl_cmd_get)) {
LOG(ERROR) << "getting nl fd failed for: " << m_radio_info.iface_name
<< "disbling netlink for this platform";
m_dwpal_nl_ctx = nullptr;
} else {
LOG(DEBUG) << "Attach event interface to nl - SUCCESS! for: "
<< m_radio_info.iface_name << " nl_fd = " << m_fd_nl_events
<< " fdcmdget_nl = " << m_fd_nl_cmd_get;
}
} else {
LOG(ERROR) << "NL ctx is NULL! netlink is disabled for this platform";
}

return true;
})

Expand Down Expand Up @@ -331,6 +361,17 @@ bool base_wlan_hal_dwpal::fsm_setup()

m_fd_ext_events = -1;

// detach nl socket from main vap
if (m_dwpal_nl_ctx) {
LOG(DEBUG) << "detaching nl interface";
if (dwpal_driver_nl_detach(&m_dwpal_nl_ctx) == DWPAL_FAILURE) {
LOG(ERROR) << "dwpal_driver_nl_detach() failed for radio ="
<< m_radio_info.iface_name;
}
m_fd_nl_events = -1;
m_fd_nl_cmd_get = -1;
}

return success;
})

Expand Down Expand Up @@ -507,6 +548,150 @@ bool base_wlan_hal_dwpal::attach_ctrl_interface(int vap_id)
return true;
}

bool base_wlan_hal_dwpal::process_nl_events()
{
if (!m_dwpal_nl_ctx) {
LOG(ERROR) << "Invalid Netlink socket used for nl events (m_dwpal_nl_ctx == nullptr)";
return false;
}

// check if there is nothing to proccess
if (m_fd_nl_events <= 0) {
LOG(ERROR) << __func__ << "nothing to proccess fd= " << m_fd_nl_events;
return false;
}

// Passing a lambda with capture is not supported for standard C function
// pointers. As a workaround, we create a static (but thread local) wrapper
// function that calls the capturing lambda function.
static __thread std::function<DWPAL_Ret(struct nl_msg * msg)> nl_handler_cb_wrapper;
nl_handler_cb_wrapper = [&](struct nl_msg *msg) -> DWPAL_Ret {
if (!process_dwpal_nl_event(msg)) {
LOG(ERROR) << "User's netlink handler function failed!";
return DWPAL_FAILURE;
vitalybu marked this conversation as resolved.
Show resolved Hide resolved
}
return DWPAL_SUCCESS;
};
auto nl_handler_cb = [](struct nl_msg *msg) -> DWPAL_Ret { return nl_handler_cb_wrapper(msg); };

//parsing will be done in callback function
if (dwpal_driver_nl_msg_get(m_dwpal_nl_ctx, DWPAL_NL_UNSOLICITED_EVENT, NULL, nl_handler_cb) ==
DWPAL_FAILURE) {
LOG(ERROR) << " dwpal_driver_nl_msg_get failed,"
<< " ctx=" << m_dwpal_nl_ctx;
return false;
}

return true;
}

bool base_wlan_hal_dwpal::dwpal_nl_cmd_set(const std::string &ifname, unsigned int nl_cmd,
unsigned char *vendor_data, size_t vendor_data_size)
{
if (vendor_data == nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!vendor_data) { ... }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOG(ERROR) << __func__ << "vendor_data is NULL ==> Abort!";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove __func__ and __FUNCTION__ from all log messages

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return false;
}

if (dwpal_driver_nl_cmd_send(m_dwpal_nl_ctx, DWPAL_NL_UNSOLICITED_EVENT, (char *)ifname.c_str(),
NL80211_CMD_VENDOR, DWPAL_NETDEV_ID,
(enum ltq_nl80211_vendor_subcmds)nl_cmd, vendor_data,
vendor_data_size) != DWPAL_SUCCESS) {
LOG(ERROR) << __func__ << "ERROR for cmd = " << nl_cmd;
return false;
}

return true;
}

size_t base_wlan_hal_dwpal::dwpal_nl_cmd_get(const std::string &ifname, unsigned int nl_cmd,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function should return ssize_t (signed size_t).
Error value should be <= 0 and any positive value is the length of the received message.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsigned char *out_buffer,
const size_t max_buffer_size)
{
size_t data_size = 0;

if (out_buffer == nullptr) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!out_buffer) { ... }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOG(ERROR) << __func__ << "out_buffer is invalid ==> Abort!";
return data_size;
}

/* Handle a command which invokes an event with the output data */
if (dwpal_driver_nl_cmd_send(
m_dwpal_nl_ctx, DWPAL_NL_SOLICITED_EVENT, (char *)ifname.c_str(), NL80211_CMD_VENDOR,
DWPAL_NETDEV_ID, (enum ltq_nl80211_vendor_subcmds)nl_cmd, NULL, 0) == DWPAL_FAILURE) {
LOG(ERROR) << __func__ << "ERROR for cmd = " << nl_cmd;
return data_size;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either initialize data_size to -1, or simply return -1 here to indicate a failure.
Same goes to all failures...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

// Passing a lambda with capture is not supported for standard C function
// pointers. As a workaround, we create a static (but thread local) wrapper
// function that calls the capturing lambda function.
static __thread std::function<DWPAL_Ret(char *ifname, int event, int subevent, size_t len,
unsigned char *data)>
nl_handler_cb_wrapper;
nl_handler_cb_wrapper = [&](char *ifname, int event, int subevent, size_t len,
unsigned char *data) -> DWPAL_Ret {
if (!len || !data) {
LOG(ERROR) << "len=0 and/or data is NULL ==> Abort!";
return DWPAL_FAILURE;
}
if (event == NL80211_CMD_VENDOR) {
if (len >= max_buffer_size) {
LOG(ERROR) << "NL size exceeds out_buffer size ==> Abort!";
return DWPAL_FAILURE;
}

// copy result from nl data buffer to local buffer
std::copy_n(data, len, out_buffer);

// update data size
data_size = len;
} else {
LOG(ERROR) << "not handling non vendor event = " << event;
return DWPAL_FAILURE;
}
return DWPAL_SUCCESS;
};
auto nl_handler_cb = [](char *ifname, int event, int subevent, size_t len,
unsigned char *data) -> DWPAL_Ret {
return nl_handler_cb_wrapper(ifname, event, subevent, len, data);
};

//parsing will be done in callback func
if (dwpal_driver_nl_msg_get(m_dwpal_nl_ctx, DWPAL_NL_SOLICITED_EVENT, nl_handler_cb, NULL) ==
DWPAL_FAILURE) {
LOG(ERROR) << " dwpal_driver_nl_msg_get failed,"
<< " ctx=" << m_dwpal_nl_ctx;
return data_size;
}

return data_size;
}

bool base_wlan_hal_dwpal::dwpal_nl_cmd_scan_dump()
{
// Passing a lambda with capture is not supported for standard C function
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment repeates in every function, maybe it would be better to move it to the Doxygen.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// pointers. As a workaround, we create a static (but thread local) wrapper
// function that calls the capturing lambda function.
static __thread std::function<DWPAL_Ret(struct nl_msg * msg)> nl_handler_cb_wrapper;
nl_handler_cb_wrapper = [&](struct nl_msg *msg) -> DWPAL_Ret {
if (!process_dwpal_nl_event(msg)) {
LOG(ERROR) << "User's netlink handler function failed!";
return DWPAL_FAILURE;
}
return DWPAL_SUCCESS;
};
auto nl_handler_cb = [](struct nl_msg *msg) -> DWPAL_Ret { return nl_handler_cb_wrapper(msg); };

if (dwpal_driver_nl_scan_dump(m_dwpal_nl_ctx, (char *)m_radio_info.iface_name.c_str(),
nl_handler_cb) != DWPAL_SUCCESS) {
LOG(ERROR) << "dwpal_driver_nl_scan_dump Failed to request the nl scan dump";
return false;
}

return true;
}

bool base_wlan_hal_dwpal::refresh_radio_info()
{
char *reply = nullptr;
Expand Down
29 changes: 29 additions & 0 deletions common/beerocks/bwl/dwpal/base_wlan_hal_dwpal.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class base_wlan_hal_dwpal : public virtual base_wlan_hal,
virtual bool refresh_radio_info() override;
virtual bool refresh_vaps_info(int id) override;
virtual bool process_ext_events() override;
virtual bool process_nl_events() override;
virtual std::string get_radio_mac() override;

// Protected methods
Expand All @@ -53,6 +54,7 @@ class base_wlan_hal_dwpal : public virtual base_wlan_hal,

// Process dwpal event
virtual bool process_dwpal_event(char *buffer, int bufLen, const std::string &opcode) = 0;
virtual bool process_dwpal_nl_event(struct nl_msg *msg) = 0;

bool set(const std::string &param, const std::string &value,
int vap_id = beerocks::IFACE_RADIO_ID);
Expand All @@ -62,6 +64,30 @@ class base_wlan_hal_dwpal : public virtual base_wlan_hal,
bool dwpal_send_cmd(const std::string &cmd, int vap_id = beerocks::IFACE_RADIO_ID);
bool attach_ctrl_interface(int vap_id);

/**
* @brief handle get data cmd from netlink
* @param ifname radio interface name
* @param nl_cmd netlink get command number
* @param out_buffer pointer to data buffer for the result
* @param max_buffer_size buffer size
* @return size of returned data (success if > 0)
*/
size_t dwpal_nl_cmd_get(const std::string &ifname, unsigned int nl_cmd,
unsigned char *out_buffer, const size_t max_buffer_size);
/**
* @brief handle set vendor data cmd to netlink
* @param ifname radio interface name
* @param nl_cmd netlink set command number
* @param vendor_data pointer to vendor data buffer
* @param vendor_data_size size of data
* @return true on success
* @return false on failure
*/
bool dwpal_nl_cmd_set(const std::string &ifname, unsigned int nl_cmd,
unsigned char *vendor_data, size_t vendor_data_size);
bool dwpal_nl_cmd_scan_dump();
void *get_dwpal_nl_ctx() const { return (m_dwpal_nl_ctx); }

// Private data-members:
private:
const uint32_t AP_ENABLED_TIMEOUT_SEC = 15;
Expand All @@ -75,6 +101,9 @@ class base_wlan_hal_dwpal : public virtual base_wlan_hal,
std::chrono::steady_clock::time_point m_state_timeout;

void *m_dwpal_ctx[beerocks::IFACE_TOTAL_VAPS] = {nullptr};
void *m_dwpal_nl_ctx = nullptr;

int m_fd_nl_cmd_get = -1;

char m_wpa_ctrl_buffer[HOSTAPD_TO_DWPAL_MSG_LENGTH];
size_t m_wpa_ctrl_buffer_size = HOSTAPD_TO_DWPAL_MSG_LENGTH;
Expand Down
Loading