Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wi-Fi Association Bug Fix for BCM #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Roy-Vincent
Copy link

while integrating opensync 6.2 with BCM 5.04.04p3 it was observed that the extender devices could not associate with the onboard AP.

Further analysis revealed that the SDK was set to the default value for BSS (bssmax = 8). Since opensync was trying to bring up interfaces ranging from wlx.1 to wlx.7 by default, there was some conflict that caused the failure in the leaf association.

The following fix addresses the issue if the SDK comes with default values, the leaf will still be able to associate with the GW and bring up the APs. The VIFs wlx.1 to wlx.6 will be brought up in this case.

This can help with the normal operation of the leaf device. However, it will be better to request the ODM for this requirement (bssmax at least 10) in intake form / TRD when they approach for Integration/Certification with the plume team.

@dejan-pojbic-plume
Copy link
Contributor

Hi @Roy-Vincent
Thank you for your feedback and contributing to OpenSync. Under requirements https://opensync.atlassian.net/wiki/spaces/OCC/pages/39920140311/Requirements#WiFi-virtual-access-points-(VAP)
you will find that we need minimal 8 BSS/VAP/AP/STA interfaces.
There is also FUT test case to validate this to pass OpenSync certification. https://github.com/plume-design/opensync-fut-base/blame/5063ccd2471a505afb3a420c602a7bb1145418e2/test/WM2_test.py#L298

@kazikcz can you please provide your view as WiFI OpenSync Architect on this PR request? Thanks!

Copy link

@kazikcz kazikcz left a comment

Choose a reason for hiding this comment

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

The commit log must not be empty. Commit log must describe why the change is necessary, and if it does something that may not be immediately obvious to the reader, why it is done in a particular way. It needs to describe what the system behavior was before and after the change. If it's a bug, it needs to explain what the conditions for the bug were.

As it is, I don't understand anything about this change.

}else{
//Adding a log here would be helpful to inform the developer/integrator
bcmwl_vap_prealloc(p->d_name, bssmax - 2, bcmwl_vap_mac_xfrm);
}
Copy link

Choose a reason for hiding this comment

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

  1. Formatting does not match the existing style. This needs to be fixed.
  2. bssmax will never be greater than 8 by default - the BCMWL_VAP_PREALLOC_MAX is already 8, and will limit bssmax to be 8 at most. Are you building opensync with modified BCMWL_VAP_PREALLOC_MAX? If so, to what value, and why?
  3. Why are you checking bssmax against 8 in particular? Why not 7 or 9?
  4. Why are you decrementing 1 or 2 depending on the condition result? How is this helping, and why is this helping with "Wi-Fi Association Bug".
  5. What is the bug that you're really fixing? Can you describe what you do, and what you see, and what you expect?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants