Skip to content

[acl] Add ACL rules to allow BGP traffic#796

Merged
liat-grozovik merged 1 commit intosonic-net:masterfrom
wangxin:acl-rules
Feb 28, 2019
Merged

[acl] Add ACL rules to allow BGP traffic#796
liat-grozovik merged 1 commit intosonic-net:masterfrom
wangxin:acl-rules

Conversation

@wangxin
Copy link
Collaborator

@wangxin wangxin commented Feb 2, 2019

The current set of ACL rules will block BGP traffic. All the routes
learned via BGP will timeout after the ACL rules are loaded. Then
there is no route for the PTF injected packets. Packets accepted by
ACL rules would not be forwarded anyway. This update is to add two
ACL rules to allow BGP traffic.

Signed-off-by: Xin Wang [email protected]

Description of PR

Summary:
Fixes # (issue)

The current set of ACL rules will block BGP traffic. All the routes
learned via BGP will timeout after the ACL rules are loaded. Then
there is no route for the PTF injected packets. Packets accepted by
ACL rules would not be forwarded anyway. This update is to add two
ACL rules to allow BGP traffic.

Type of change

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

Approach

How did you do it?

Add two ACL rules to always alow BGP traffic.

How did you verify/test it?

Tested on Mellanox platform.

Any platform specific information?

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

Documentation

The current set of ACL rules will block BGP traffic. All the routes
learned via BGP will timeout after the ACL rules are loaded. Then
there is no route for the PTF injected packets. Packets accepted by
ACL rules would not be forwarded anyway. This update is to add two
ACL rules to allow BGP traffic.

Signed-off-by: Xin Wang <[email protected]>
Copy link
Contributor

@stcheng stcheng left a comment

Choose a reason for hiding this comment

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

Why this was not the case before? I wonder if COPP rule already allows BGP traffic to get trapped into CPU?

@liat-grozovik
Copy link
Collaborator

@andriymoroz-mlnx can you review as well and reply to the question above? was there a change which cause it or this somehow missed?

@andriymoroz-mlnx
Copy link
Contributor

Previously we did not have egress ACLs and did not send traffic through the switch.

@wangxin
Copy link
Collaborator Author

wangxin commented Feb 12, 2019

@stcheng ACL rules are handled earlier than trap. At least this is the design of Mellanox platform. When the PTF script is executed, IP routes learned via BGP may have not expired yet. That's the reason sometimes the PTF script can pass. However, the PTF script always failed after switch rebooted with ACL rules saved in config DB according to test results of my side. Not sure how the script results from your side look like.

@stcheng
Copy link
Contributor

stcheng commented Feb 13, 2019

So if the ACLs are applied, the BGP sessions will be expired? Is that the case?

@stcheng
Copy link
Contributor

stcheng commented Feb 13, 2019

@andriymoroz-mlnx is this the case only for egress ACLs or both ingress ACLs and egress ACLs?

@wangxin
Copy link
Collaborator Author

wangxin commented Feb 14, 2019

@stcheng Yes, BGP sessions will be expired if the ACLs are applied. I think it is the same case for both ingress and egress ACLs.

@lguohan
Copy link
Contributor

lguohan commented Feb 14, 2019

which rule is blocking the bgp traffic, can you identify? it looks like you added the bgp rule as the end, usually it means the lowest priority, then how do we expect the rule take effect?

@wangxin
Copy link
Collaborator Author

wangxin commented Feb 14, 2019

@lguohan The DEFAULT_RULE blocks all IP traffic. It is automatically created when adding any ACL rule. And its priority is always the lowest, lower than the BGP rules I added.

@liat-grozovik
Copy link
Collaborator

@lguohan can you please review? we would like to have the ACL test back into the daily regression and it is currently not passing

@liat-grozovik liat-grozovik merged commit 9b8264e into sonic-net:master Feb 28, 2019
yxieca pushed a commit that referenced this pull request Feb 28, 2019
The current set of ACL rules will block BGP traffic. All the routes
learned via BGP will timeout after the ACL rules are loaded. Then
there is no route for the PTF injected packets. Packets accepted by
ACL rules would not be forwarded anyway. This update is to add two
ACL rules to allow BGP traffic.

Signed-off-by: Xin Wang <[email protected]>
@yxieca
Copy link
Collaborator

yxieca commented Feb 28, 2019

Made to 201811 branch on 2/28/2019

@wangxin wangxin deleted the acl-rules branch May 24, 2019 03:34
auspham pushed a commit to auspham/sonic-mgmt that referenced this pull request Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants