-
Notifications
You must be signed in to change notification settings - Fork 740
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
Tests for ACL DSCP change table. #14791
base: master
Are you sure you want to change the base?
Conversation
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Can you paste the link to the test plan to this PR? |
tests/acl/test_acl_dscp_change.py
Outdated
@pytest.fixture(scope='module') | ||
def prepare_test_port(rand_selected_dut, tbinfo): | ||
mg_facts = rand_selected_dut.get_extended_minigraph_facts(tbinfo) | ||
if tbinfo["topo"]["type"] == "mx": |
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.
Why mx
is handled? The test should be enabled only on T1.
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.
fixed
tests/acl/test_acl_dscp_change.py
Outdated
upstream_port_ids = [] | ||
for interface, neighbor in list(mg_facts["minigraph_neighbors"].items()): | ||
port_id = mg_facts["minigraph_ptf_indices"][interface] | ||
if (topo == "t1" and "T2" in neighbor["name"]) or (topo == "t0" and "T1" in neighbor["name"]) or \ |
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.
This is not required as the topo is t1
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.
fixed
tests/acl/test_acl_dscp_change.py
Outdated
if asic_type in ["cisco-8000", "mellanox"]: | ||
data['tolerance'] = 0.03 | ||
else: | ||
raise RuntimeError("Pls update this script for your platform.") |
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.
This will cause test error on Non-Cisco or Non-Mellanox platform. Suggest skipping the test on non-supported platforms.
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.
fixed
tests/acl/test_acl_dscp_change.py
Outdated
|
||
asic_type = duthost.facts["asic_type"] | ||
if asic_type in ["cisco-8000", "mellanox"]: | ||
data['tolerance'] = 0.03 |
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.
Where is data['tolerance']
used?
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.
this is not needed here. i have removed it.
tests/acl/test_acl_dscp_change.py
Outdated
py_assert( | ||
duthost.shell("redis-cli -n 6 hgetall 'ACL_TABLE_TABLE|OVERLAY_MARK_META_TEST'")['stdout'] == 'status\nActive') | ||
py_assert(duthost.shell( | ||
"redis-cli -n 6 hgetall 'ACL_TABLE_TABLE|OVERLAY_MARK_META_TESTV6'")['stdout'] == 'status\nActive') |
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.
Suggest redis-cli -n 6 hget 'ACL_TABLE_TABLE|OVERLAY_MARK_META_TEST' 'status'
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.
done
|
||
exp_pkt.mask[inner_ether_hdr_end + 8] = 0x00 # TTL is changed | ||
exp_pkt.mask[inner_ether_hdr_end + 10] = 0x00 # checksum is changed | ||
exp_pkt.mask[inner_ether_hdr_end + 11] = 0x00 # checksum is changed |
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.
What's the purpose to check TTL and checksum to 0?
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.
This part adds the packet mask. setting the mask means that this portion of the received packet should be ignored when comparing with the expected packet.
tests/acl/test_acl_dscp_change.py
Outdated
duthost.shell("redis-cli -n 4 del 'ACL_TABLE|OVERLAY_MARK_META_TESTV6' ") | ||
|
||
|
||
def create_acl_Rule(duthost, tableName, ruleNumber, priority, dscpAction, dstIp, qualifer=None): |
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.
def create_acl_Rule(duthost, tableName, ruleNumber, priority, dscpAction, dstIp, qualifer=None): | |
def create_acl_rule(duthost, tableName, ruleNumber, priority, dscpAction, dstIp, qualifer=None): |
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.
done
pkt = create_inner_packet(setup_vnet, duthost, encap_type, match_fields) | ||
exp_pkt = create_expected_packet(setup_vnet, duthost, encap_type, expectedDscp, pkt) | ||
setup_vnet['ptfadapter'].dataplane.flush() | ||
testutils.send(test=setup_vnet['ptfadapter'], port_id=setup_vnet['ptf_src_port'], pkt=pkt) |
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.
What's the traffic direction here? Is it from T1 to T0? How do you ensure that?
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.
THere is no set direction as overlay ecmp packets can come form T2 and go back to default route as well. Son in this case we are injecting thetraffic from a randomly selected interface and capture on all interfaces.
Please add the test result to the PR description. Thanks! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@bingwang-ms I have added a testplan and updted wthe PR based on review comments. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description of PR
Summary:
Fixes # (issue)
NOTE: This PR must merge after sonic-net/sonic-swss#3307
These Tests are for a new feature ACL DSCP change table.
Type of change
Back port request
Approach
What is the motivation for this PR?
These tests for added to check the functionality of newly proposed feature ACL DSCP change table.
These Tables changes the outer DSCP value of the vxlan tunneled packet based on the ACL match criteria of the inner packet.
These table are onyl supported on mlnx platforms. Support for Cisco-8000 is expected soon.
How did you do it?
The tests adds Vxlan tunnels on the device. It then adds the Underlay SET dscp acl. THis followd by sending packtes which match the ACL criteria. The encapsualted packets are recieved and verified against the orignal packet and expected outer DSCP header value.
How did you verify/test it?
Ran it.
Any platform specific information?
THis test can only run on Cisco-8000 and mlnx platforms as these platforms support the ACL_META_DATA fields.
Supported testbed topology if it's a new test case?
T1
Documentation