-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[xcvrd] Handle port config change on fly in xcvrd #839
Conversation
@Junchao-Mellanox please add a tracking PRs to the PR description. |
@zhangyanzhao could you please suggest a reviewer? |
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.
As per our discussion, I see following issues due to usage of event queue (which is not required IMO)
- For each port change event, main task reads the port change from config DB (1st read), writes it into the event queue of both the observers (2 writes). Each of the observer then reads the event from their respective event queue(2 reads). So total 3 reads and 2 writes.
- Since, the same event queue is accessed for write by (main task) and read by observer threads, a lock is a must. For each port change event there will be a total of 4 lock and unlock operations (because of 2 reads and 2 writes in the event queue of observers)
Consider a worst case where there are 36 ports in x4 breakout so a total of 144 logical ports entries in the event queue needs to be added or deleted which means a total of 144 * 4 = 576 lock/unlock operations.
Lets consider an alternative approach, without the event queue. Since, DB allows to share information across multiple process, we can just let the observers listen to CONFIG_DB changes instead of only main thread. This way each observer can fetch the port mapping independently by reading the PORT_TABLE in APP_DB. No queue required and ZERO locks required. Please note, three select() is lightweight operation compared to lock/unlocks. In this approach there will be only 3 reads required compared to 3 reads and 2 writes and additional overhead due to mutually exclusive access to the event queue.
Currently, xcvrd assumes that port mapping information is never changed, so it always read static port mapping information from platform.json/port_config.ini and save it to a global data structure. However, things changed since dynamic port breakout feature introduced. Port can be added/created on the fly, xcvrd cannot update transceiver information, DOM information and transceiver status information without knowing the ports change. This change introduces a way to handle port configuration change on fly in xcvrd.
Related PRs: