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

[muxorch] Bind all ports to drop ACL table #2027

Merged
merged 1 commit into from
Nov 19, 2021
Merged

[muxorch] Bind all ports to drop ACL table #2027

merged 1 commit into from
Nov 19, 2021

Conversation

devpatha
Copy link
Contributor

What I did
BInd all physical ports to drop ACL table

Why I did it
With current implementation, ACL table is link/bind to the very first port that triggers creation of drop ACL table followed by rule creation for the same.
For subsequent ports only new rule is created.
If a port is not linked/bind to the drop ACL table, the rule is not applied for that port.

How I verified it

  1. Verified using sairedis log and mux logs that was added with new code that all ports are linked/bind to the drop ACL table.
  2. Verified that after this fix all standby ports are dropping packets as expected.

Details if related

@ghost
Copy link

ghost commented Nov 12, 2021

CLA assistant check
All CLA requirements met.

acl_table.stage = ACL_STAGE_INGRESS;
gAclOrch->addAclTable(acl_table);
bindAllPorts(acl_table);
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you can see from line 783, the same table shall be used if pfcwd has already created this table. You may want to apply the same fix here as well -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I can update the PR with requested changes. But cisco platform does not use drop ACL for PFC wd so I won't be able to verify it. Is it fine to change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, then leave it for now and revisit later

@prsunny
Copy link
Collaborator

prsunny commented Nov 16, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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