Skip to content

[pytest/ACL] fix acl cannot run on t1-64-lag#1648

Merged
wangxin merged 1 commit intosonic-net:masterfrom
chaoskao:t1-64-lag
Jun 10, 2020
Merged

[pytest/ACL] fix acl cannot run on t1-64-lag#1648
wangxin merged 1 commit intosonic-net:masterfrom
chaoskao:t1-64-lag

Conversation

@chaoskao
Copy link
Copy Markdown
Contributor

@chaoskao chaoskao commented May 7, 2020

Fix ACL failed on t1-64-lag

Summary: ACL cannot bound normal ports which belong to port-channel on t1-64-lag and dataplane cannot send packet by PTF container

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

How did you do it?

  1. remove bound normal ports on t1-64-lag from test case, because normal ports only available on t1 or t1-lag.
  2. create interfaces of dataplane according interfaces of PTF container.

How did you verify/test it?

run test on t1-64-lag show pass

Any platform specific information?

no

Supported testbed topology if it's a new test case?

t1-64-lag

Documentation

@chaoskao chaoskao changed the title fix acl cannot run on t1-64-lag [pytest/ACL]fix acl cannot run on t1-64-lag May 7, 2020
@chaoskao chaoskao changed the title [pytest/ACL]fix acl cannot run on t1-64-lag [pytest/ACL] fix acl cannot run on t1-64-lag May 7, 2020
@wangxin wangxin requested a review from stepanblyschak May 7, 2020 11:19
# get the list of port to be combined to ACL tables
acl_table_ports += tor_ports
if testbed['topo']['name'] in ('t1', 't1-lag'):
acl_table_ports += tor_ports
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not to bind ACL tables with ToR ports for rest of the supported topologies? The testing packets flow have two directions ToR -> Spine and Spine -> ToR.
If the ACL tables are not binded with the ToR ports, I suspect some of the testing will fail.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because TOR port in t1-64-lag and t1-64-lag-clet is port-channel member, you cannot bind bound port-channel member in ACL table

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I see. But hard coding the topology name here is vulnerable if new topologies are introduced in the future. Maybe it is more robust to check if a ToR port is in a port channel. If yes, then skip it.

Copy link
Copy Markdown
Contributor Author

@chaoskao chaoskao Jun 3, 2020

Choose a reason for hiding this comment

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

Yes, it's more robust to check TOR port is a port channel to replace hard coding

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you like to make this improvement in this PR or in a separate one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think separate is ok to me

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@wangxin wangxin merged commit bc5c87d into sonic-net:master Jun 10, 2020
@chaoskao chaoskao deleted the t1-64-lag branch July 30, 2021 06:43
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
- swss:
* 459d09b 2021-02-22 | [acl] Enable VLAN ID qualifier for ACL rules (sonic-net#1648) (HEAD -> 202012) [Danny Allen]
* 60859b1 2021-02-08 | [buffermgr] Support maximum port headroom checking (sonic-net#1607) [Stephen Sun]
* 3161cbb 2021-02-17 | Add SAI_INGRESS_PRIORITY_GROUP_STAT_DROPPED_PACKETS counter, create new FlexCounter group (sonic-net#1600) [Andriy Yurkiv]
* 256ed9c 2021-02-08 | Support shared headroom pool on top of dynamic buffer calculation (sonic-net#1581) [Stephen Sun]

- utilities:
* 30d7069 2021-02-16 | [decode-syseeprom] Refactor to utilize sonic-platform package (sonic-net#1435) (HEAD -> 202012, tag: foo) [Joe LeVeque]
* 891fef4 2021-02-16 | [psuutil] Refactor to utilize sonic-platform package (sonic-net#1434) [Joe LeVeque]
* 8c5e505 2021-02-17 | [sfputil] Refactor to utilize sonic-platform package (sonic-net#1421) [Joe LeVeque]
* ca5dd2c 2021-02-17 | [sfpshow] Cleanup (sonic-net#1405) [Joe LeVeque]
* bf489ea 2021-02-17 | Add new cli for SAI_INGRESS_PRIORITY_GROUP_STAT_DROPPED_PACKETS counter in counterpoll utility (sonic-net#1355) [Andriy Yurkiv]
* 25feed3 2021-02-08 | Support shared headroom pool on top of dynamic buffer calculation (sonic-net#1348) [Stephen Sun]
* aaa323a 2021-02-02 | [vrf]: Fix freezing during interface binding (sonic-net#1325) [maksymbelei95]
* cc0bb6e 2021-01-27 | [show] fix "show interfaces breakout" command (sonic-net#1198) [Dmytro Shevchuk]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
[201911][acl] Enable VLAN ID qualifier for ACL rules (sonic-net#1648) (sonic-net#1651)
Skip setting not implemented brcm attr in buffer profile (sonic-net#1649)
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.

2 participants