-
Notifications
You must be signed in to change notification settings - Fork 666
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
Add utility to configure ECN WRED parameters #153
Conversation
Signed-off-by: Andriy Moroz <c_andriym@mellanox.com>
@yxieca , to review |
config/main.py
Outdated
if ymax: command += " -ymax %d" % ymax | ||
if ymin: command += " -ymin %d" % ymin | ||
if gmax: command += " -gmax %d" % gmax | ||
if gmin: command += " -gmin %d" % gmin |
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.
The way you tested these parameters will ignore a parameter when it is 0 or None. Is this what you intended?
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.
yes, min/max parameters are optional
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.
Sorry for the late reply. Andriy,
My question is more about if you intentionally ignores the value 0? I understand you would like to ignore when it is None.
Regards,
Ying
config/main.py
Outdated
if gmax: command += " -gmax %d" % gmax | ||
if gmin: command += " -gmin %d" % gmin | ||
if verbose: | ||
click.echo(command) |
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.
Do you think you want to further pass verbose option to ecnconfig script?
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.
right, will add
ecn.set_wred_threshold(args.profile, "ymin", args.yellow_min) | ||
if args.red_max: | ||
ecn.set_wred_threshold(args.profile, "rmax", args.red_max) | ||
if args.red_min: |
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 deleting comments is not the best thing.
I figured out that args are returning values in strings, so we don't have a risk of ignoring integer value 0 here.
Signed-off-by: Andriy Moroz <c_andriym@mellanox.com>
f5a0e25
to
12958e5
Compare
Signed-off-by: Andriy Moroz <c_andriym@mellanox.com>
…ic-net#153) Make functions used for media setting python3 compatible
…onic-net#153) In SONiC testbed, y_cable is simulated by OVS in test server. An OVS bridge is created for each of the y_cable. The PTF interface and test server VLAN interfaces are attached to the bridge. The VLAN interfaces are connected to DUT ports through fanout switches. +--------------+ | +----- upper_if PTF (host_if) --+ OVS bridge | | +----- lower_if +--------------+ Open flow rules are configured for the OVS bridge to simulate upstream broadcasting and downstream dropping traffic from standby side. To further simulate y_cable active/standby querying and switching, a process can be started in the test server. The process needs to expose APIs for querying and switching active/standby status. When the APIs are called, the process checks and updates open flow configurations of the OVS bridge accordingly. On SONiC side, we need to intercept the calls to y_cable driver and call the APIs exposed by the process running in test server. This change is to enhance the y_cable driver to support calling hook functions defined in y_cable_simulator_client when the module is found. To do that, a decorator is added to each of the functions in the y_cable driver. When a y_cable function is called, it firstly checks if module y_cable_simulator_client is found and the module has function with the same name. If yes, then call the function of y_cable_simulator_client and return the result. Otherwise, call the original y_cable driver function. Signed-off-by: Xin Wang <xiwang5@microsoft.com>
Signed-off-by: Andriy Moroz c_andriym@mellanox.com
Should be merged AFTER sonic-net/sonic-buildimage#1178