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

Isis srv6 topo1 ping #17848

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pguibert6WIND
Copy link
Member

Fix a ping test that should never pass.

@pguibert6WIND
Copy link
Member Author

@cscarpitta , are there any suggestions on the srv6 setup on how to avoid doing MAC set on the setup ?

@pguibert6WIND pguibert6WIND requested review from ton31337 and cscarpitta and removed request for ton31337 January 13, 2025 20:45
@riw777
Copy link
Member

riw777 commented Jan 14, 2025

It looks like this still has problems?

E AttributeError: 'str' object has no attribute 'keys'

@riw777 riw777 self-requested a review January 14, 2025 15:02
Copy link
Contributor

@cscarpitta cscarpitta left a comment

Choose a reason for hiding this comment

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

@cscarpitta , are there any suggestions on the srv6 setup on how to avoid doing MAC set on the setup ?

@pguibert6WIND

rt1 should use the MAC of rt2 as destination MAC address. This MAC address should be already in the ip neigh table or can automatically discovered by rt1 (if it is not in ip neigh table yet).

Therefore there is no need to manually setup the MAC.

The reason why it doesn't behave this way is that there is an error in the seg6-route that we install on rt1.

sharp install seg6-routes fc00:0:9::1 nexthop-seg6 2001:db8:1::2 encap fc00:0:1:2:6:f00d:: 1

This seg6-route tells rt1 to steer packets destined to fc00:0:9::1 over this path: rt1->rt2->-rt6.

This path is not correct. Since we are installing this seg6-route on rt1, it means that a packet matching this seg6-route has already reached rt1. So rt1 should not be part of the path.

The correct path should be rt2->rt6, which means the seg6-route should be

sharp install seg6-routes fc00:0:9::1 nexthop-seg6 2001:db8:1::2 encap fc00:0:2:6:f00d:: 1

Changing the seg6-route to the above should solve the topotest failure

The ping test does not detect when the command fails.

> 2025-01-13 15:38:27,494 INFO: topo: [+] check rt1 fc00:0:9::1 0% packet loss
> 2025-01-13 15:38:27,494 WARNING: topo: Waiting time is too small (count=10, wait=1), using default values (count=20, wait=3)
> 2025-01-13 15:38:28,501 WARNING: rt1: Router(rt1): proc failed: rc 1 pid 2028454
> 	args: /usr/bin/nsenter --mount=/proc/2026950/ns/mnt --net=/proc/2026950/ns/net --uts=/proc/2026950/ns/uts -F --wd=/tmp/topotests/isis_srv6_topo1.test_isis_srv6_topo1/rt1 /bin/bash -c ping6 fc00:0:9::1 -c 1 -w 1
> 	stdout: PING fc00:0:9::1(fc00:0:9::1) 56 data bytes
>
> --- fc00:0:9::1 ping statistics ---
> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
> 	stderr: *empty*
> 2025-01-13 15:38:28,501 INFO: topo: PING fc00:0:9::1(fc00:0:9::1) 56 data bytes
>
> --- fc00:0:9::1 ping statistics ---
> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
>
>
> PASSED
>

The match string is not precise enough. Complete it.

Signed-off-by: Philippe Guibert <[email protected]>
There is no connectivity by using the proposed srv6 path.

> From Carmine:
> This seg6-route tells rt1 to steer packets destined to fc00:0:9::1 over this path: rt1->rt2->-rt6.
> This path is not correct. Since we are installing this seg6-route on rt1,
> it means that a packet matching this seg6-route has already reached rt1.
> So rt1 should not be part of the path.
> The correct path should be rt2->rt6.

Fix this by changing the proposed seg6 route. Also, the ping test should
be swapped, because invalidating the RT1 locator does not have any
impacts on the built SRv6 path.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
Copy link
Member Author

@cscarpitta , I made a fix.
About the utility of letting the ping, we should have built a TE path on the reverse direction.
In that way, the ping would have been broken.

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.

3 participants