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

[202205] Assign altname for bridge interface on chassis and iptables rules update to allow traffic on it. #16504

Merged
merged 6 commits into from
Sep 22, 2023

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Sep 9, 2023

What I did:
Fixes: #16468

Why I did:
On Some chassis there is no dedicated eth1-midplane interface on supervisor for supervisor and LC communication but instead Linux bridge br1 is used for that. Because of this changes that were done to white-list traffic over eth1-midplane would not work.

How I did:
To fix this we are using altname property of ip link command to set eth1-midplane as altname of br1 interface. This is done to keep design generic across chassis and between supervisor and LC also. IP-table rules are updated to get parent/base interface name of eth1-midplane.

How I verify:
UT Updated
Manual verification on chassis system

abdosi added 6 commits April 24, 2023 02:24
1. Allow traffic with source and destination as chassis eth1-midplane ip.
Needed for Supervisor Redis-db connection after we load acl.json that
has catch-all drop rule

2. Made multi_asic_ns_to_host_fwd as False for ACL service for External
Client. This flag is needed for service SSH and SNMP where traffic can
come in namespace over front-panel ports and we need to send the traffic
in host where corresponding docker/service are running. There is no
use-case of External client service for multi-asic as of now. Having
flag as True creates failure when we try to load acl.json.

Signed-off-by: Abhishek Dosi <[email protected]>
…hat uses

Linux bridge for communication and corresponding ip tables rules update to
allow traffic access via eth1-midplane as white-list

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi requested a review from lguohan as a code owner September 9, 2023 00:06
@abdosi abdosi requested a review from arlakshm September 9, 2023 00:06
@abdosi abdosi linked an issue Sep 9, 2023 that may be closed by this pull request
@lguohan
Copy link
Collaborator

lguohan commented Sep 9, 2023

where is master branch pr?

@lguohan :
#16505
sonic-net/sonic-host-services#75

@lguohan
Copy link
Collaborator

lguohan commented Sep 9, 2023

is there a design doc for eth1-midplane? it is strange that we have a bridge name called eth1-midplane.

@lguohan there is no specific design doc for defining bridge bame as mention here : https://github.com/sonic-net/SONiC/blob/master/doc/voq/architecture.md

image

Some of the chassis vendor as physical HW switch that provide internal/midplane communication between Sup and LC while other uses Linux Soft Bridge to represent this.

All the Vendors create "eth1-midplane" on LC to talk with Supervisor as done by this PR: 0a7dd50#diff-130e5ab75471398db06b73a4c6e4b56517f42e3c1f17870b5309620fc0e2fafbR182.

However on Supervisor it can be br1 for vendors using Linux Bridge or eth1-midplane for vendors using HW Switch.

This PR address this delta by assigning alternate name to br1 interface so that we can use consistent ip table rules.

@abdosi
Copy link
Contributor Author

abdosi commented Sep 13, 2023

@lguohan can you check above comments and review it back

@abdosi
Copy link
Contributor Author

abdosi commented Sep 20, 2023

@lguohan can you review this PR and above comments

@@ -180,6 +180,9 @@ function postStartAction()
ip link add name ns-eth1"$NET_NS" type veth peer name eth1@"$NET_NS"
ip link set dev eth1@"$NET_NS" master br1
ip link set dev eth1@"$NET_NS" up
# For chassis system where Linux bridge is used on supervisor for midplane communication
# assign alternate name as eth1-midplane for generic design
ip link property add dev br1 altname eth1-midplane
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you need to check if br1 exists before adding alias to br1?

Copy link
Contributor Author

@abdosi abdosi Sep 22, 2023

Choose a reason for hiding this comment

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

br1 is created by the platform code. In that matter eth1-midplane is also created by vendor platform code. In the code above here there is already assumption br1 is there since we are attaching all the Fabric ASIC Namespace to the br1 for allowing them to talk to chassis db which is running over br1 network. Please note this all logic only applies to Arista Chassis as they have define midplane_subnet:

. Also since they are using 127.xx.xx.xx network for br1 they had to create eth1 in Fabric namespace and enable local routing to talk to br1. All other vendor fabric namespace reaches chassis db over the docker bridge connected to eth0 in each fabric namespace.

return self.run_commands([chassis_midplane_ip_command])

midplane_ip = self.run_commands([chassis_midplane_ip_command])
return midplane_dev_name, midplane_ip
Copy link
Collaborator

Choose a reason for hiding this comment

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

@lguohan lguohan merged commit 7558d03 into sonic-net:202205 Sep 22, 2023
lguohan pushed a commit that referenced this pull request Sep 23, 2023
…supervisor. (#16505)

Add alternate name eth1-midplane to Linux bridge br1 created on supervisor on some chassis platforms.
See description here: #16504

Signed-off-by: Abhishek Dosi <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Nov 19, 2023
…supervisor. (sonic-net#16505)

Add alternate name eth1-midplane to Linux bridge br1 created on supervisor on some chassis platforms.
See description here: sonic-net#16504

Signed-off-by: Abhishek Dosi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Test failed because of not able to connect to chassisDb
2 participants