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

[vxlanorch]: p2mp tunnels not stored in VXLAN state table #3480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bradh352
Copy link
Contributor

@bradh352 bradh352 commented Jan 22, 2025

Why I did it

Whether a VXLAN tunnel is brought up as p2p or p2mp depends on the silicon in use and isn't otherwise expressed to the end user. The side effect of this is show vxlan remotevtep shows no destinations when the silicon only supports p2mp such as Mellanox Spectrum1. This can cause a user to believe the vxlan tunnel wasn't properly formed when it really was.

p2mp tunnels do not get a Port created, so do not rely on an operstatus being set by portsorch to know if they are up or down. This is presumably why the tunnel entries were not recorded.

What I did

This introduces a new status of 'p2mp' in this case to denote this difference and records p2mp tunnels in the VXLAN state table.

Due to some past restructuring, calls into updateRemoteEndPointIpRef() were omitted as p2mp tunnels are not calling addTunnelUser(). This has been resolved and this function now calls into addRemoveStateTableEntry() for tracking of those remote vteps.

How I verified it

Brought up vxlan tunnels on an Nvidia SN2201 switch and verified entries are now present in show vxlan remotevtep.

Result:

+------------+------------+-------------------+--------------+
| SIP        | DIP        | Creation Source   | OperStatus   |
+============+============+===================+==============+
| 172.16.0.3 | 172.16.0.1 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
| 172.16.0.3 | 172.16.0.2 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
Total count : 2

Details if related

HLD changes here: sonic-net/SONiC#1897

Signed-off-by: Brad House (@bradh352)

@bradh352 bradh352 requested a review from prsunny as a code owner January 22, 2025 13:10
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

bradh352 added a commit to bradh352/SONiC that referenced this pull request Jan 22, 2025
Complimentary ticket to sonic-net/sonic-swss#3480
in order to allow p2mp vteps to be tracked.  This will mostly aid
in debugging.  The HLD specifically states this when talking about P2P vs
P2MP:

"For an external user, there will be no changes from usability perspective
since the schema is unchanged."

However this is not entirely true since the state table wasn't recording
p2mp remote vteps and therefore `show vxlan remotevtep` would return no
results making a user believe the vxlan tunnel was not properly
established.  This as well as the aforementioned PR corrects this
oversight.

Signed-off-by: Brad House (@bradh352)
bradh352 added a commit to bradh352/sonic-swss that referenced this pull request Jan 22, 2025
…et#3480]

Whether a VXLAN tunnel is brought up as p2p or p2mp depends on the silicon
in use and isn't otherwise expressed to the end user.  The side effect of
this is `show vxlan remotevtep` shows no destinations when the silicon
only supports p2mp such as Mellanox Spectrum1.  This can cause a user to
believe the vxlan tunnel wasn't properly formed when it really was.

p2mp tunnels do not get a Port created, so do not rely on an operstatus
being set by portsorch to know if they are up or down.  This is presumably
why the tunnel entries were not recorded.  This introduces a new status
of 'p2mp' in this case to denote this difference.

Due to some past restructuring, calls into `updateRemoteEndPointIpRef()` were
omitted as p2mp tunnels are not calling `addTunnelUser()`.  This has been
resolved and this function now calls into `addRemoveStateTableEntry()` for
tracking of those remote vteps.

Result:
```
+------------+------------+-------------------+--------------+
| SIP        | DIP        | Creation Source   | OperStatus   |
+============+============+===================+==============+
| 172.16.0.3 | 172.16.0.1 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
| 172.16.0.3 | 172.16.0.2 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
Total count : 2
```

Signed-off-by: Brad House (@bradh352)
github-actions bot pushed a commit to bradh352/sonic-swss that referenced this pull request Jan 23, 2025
…et#3480]

Whether a VXLAN tunnel is brought up as p2p or p2mp depends on the silicon
in use and isn't otherwise expressed to the end user.  The side effect of
this is `show vxlan remotevtep` shows no destinations when the silicon
only supports p2mp such as Mellanox Spectrum1.  This can cause a user to
believe the vxlan tunnel wasn't properly formed when it really was.

p2mp tunnels do not get a Port created, so do not rely on an operstatus
being set by portsorch to know if they are up or down.  This is presumably
why the tunnel entries were not recorded.  This introduces a new status
of 'p2mp' in this case to denote this difference.

Due to some past restructuring, calls into `updateRemoteEndPointIpRef()` were
omitted as p2mp tunnels are not calling `addTunnelUser()`.  This has been
resolved and this function now calls into `addRemoveStateTableEntry()` for
tracking of those remote vteps.

Result:
```
+------------+------------+-------------------+--------------+
| SIP        | DIP        | Creation Source   | OperStatus   |
+============+============+===================+==============+
| 172.16.0.3 | 172.16.0.1 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
| 172.16.0.3 | 172.16.0.2 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
Total count : 2
```

Signed-off-by: Brad House (@bradh352)
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from ed7b055 to 8805632 Compare January 24, 2025 19:58
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from 8805632 to 0c3e0e2 Compare January 24, 2025 21:06
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

tests/evpn_tunnel.py Fixed Show fixed Hide fixed
@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from 0c3e0e2 to f390228 Compare January 24, 2025 21:10
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from f390228 to 41c498a Compare January 24, 2025 21:25
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from 41c498a to f8d7bb5 Compare January 24, 2025 21:29
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Whether a VXLAN tunnel is brought up as p2p or p2mp depends on the silicon
in use and isn't otherwise expressed to the end user.  The side effect of
this is `show vxlan remotevtep` shows no destinations when the silicon
only supports p2mp such as Mellanox Spectrum1.  This can cause a user to
believe the vxlan tunnel wasn't properly formed when it really was.

p2mp tunnels do not get a Port created, so do not rely on an operstatus
being set by portsorch to know if they are up or down.  This is presumably
why the tunnel entries were not recorded.  This introduces a new status
of 'p2mp' in this case to denote this difference.

Due to some past restructuring, calls into `updateRemoteEndPointIpRef()` were
omitted as p2mp tunnels are not calling `addTunnelUser()`.  This has been
resolved and this function now calls into `addRemoveStateTableEntry()` for
tracking of those remote vteps.

Result:
```
+------------+------------+-------------------+--------------+
| SIP        | DIP        | Creation Source   | OperStatus   |
+============+============+===================+==============+
| 172.16.0.3 | 172.16.0.1 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
| 172.16.0.3 | 172.16.0.2 | EVPN              | oper_p2mp    |
+------------+------------+-------------------+--------------+
Total count : 2
```

Signed-off-by: Brad House (@bradh352)
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352 bradh352 force-pushed the bradh352/p2mp-tunnel-vteps branch from 6ec3702 to f55343e Compare January 25, 2025 12:37
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bradh352
Copy link
Contributor Author

test failures are in SRV6, not related to this

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