This repository has been archived by the owner on Sep 7, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 32
Hotfix/ppm 118 wireless backhaul sta mac #1586
Open
rmelotte
wants to merge
2
commits into
master
Choose a base branch
from
hotfix/ppm-118-wireless-backhaul-sta-mac
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we get dev_get_param with the "macaddr" parameter and no SSID, we have to reply with the MAC address of the backhaul sta, and not the MAC address of the radio. From the CAPI specification: When SSID is not specified for the parameter "macaddr", it implies that device should return backhaul STA's MAC address. db: - Add a new function: get_bsta_mac_by_ruid(). Its sole purpose it to get the backhaul sta MAC address based on the ruid. agent_ucc_listener: - When the UCC requests a "macaddr" with no SSID, use get_bsta_mac_by_ruid() to find and return the backhaul sta MAC. MAP-4.2.2_BH24G:axepoint Signed-off-by: Raphaël Mélotte <[email protected]>
sniffer: use the tshark version from the environment. See commit 689a5c7ebd2d7e80f869d571bdde057927d90abd to have all the details about the issue. In a nutshell: the tshark version bundled in the certification repository has issues parsing some packets. Since we need to switch a new major version, it's easier to just rely on the version present on the environment instead of updating the bundled one. MAP-4.2.2_BH24G:axepoint NOTE: this commit hash needs to be updated again after the relevant MR have been merged, so no SOB for now.
rmelotte
requested review from
adam1985d,
arnout,
kantera800,
mariomaz,
morantr,
tomereli and
vitalybu
as code owners
August 6, 2020 06:58
itayx
approved these changes
Aug 9, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
morantr
reviewed
Aug 11, 2020
mariomaz
approved these changes
Aug 13, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a recurrent concern, but not for this PR, so approving
value = ruid_str; | ||
// No ssid was given, we need to return the backhaul sta mac. | ||
if (!db->get_bsta_mac_by_ruid(ruid, mac_value)) { | ||
LOG(ERROR) << " failed to find the backhaul sta MAC address for ruid '" << ruid_str; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
LOG(ERROR) << " failed to find the backhaul sta MAC address for ruid '" << ruid_str; | |
LOG(ERROR) << "Failed to find the backhaul sta MAC address for ruid " << ruid_str; |
// No ssid was given, we need to return the backhaul sta mac. | ||
if (!db->get_bsta_mac_by_ruid(ruid, mac_value)) { | ||
LOG(ERROR) << " failed to find the backhaul sta MAC address for ruid '" << ruid_str; | ||
value = "backhaul sta MAC not found for ruid '" + ruid_str + "'"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe not for this PR, but using output parameter for both the requested MAC address on success and for an error message on failure is a bad practice.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We used to reply with the ruid, but we have to reply with the mac address of the backhaul sta.
This is needed because the tshark version bundled in the certification repository doesn't recognize some of the packets exchanged during the test (the association request, as well as all the beacon frames).
Specifically, it fails to read the following field in the beacon frames:
Ext Tag: HE Operation (IEEE Std 802.11ax/D3.0)
See the following 2 merge requests:
The MAP-4.2.2_BH24G test used to pass (if the DUT was on the same channel than the certified device - see https://jira.prplfoundation.org/browse/PPM-327) but no longer will (see below).
Due to recent changes we did to the agent database, by the time the UCC asks for the
macaddr
the backhaul has already disconnected (because of https://jira.prplfoundation.org/browse/PPM-330) and we can no longer reply to it.It means the MAP-4.2.2_BH24G test will fail on the following step until https://jira.prplfoundation.org/browse/PPM-330 is fixed:
Fixes https://jira.prplfoundation.org/browse/PPM-323