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

SRv6 uSID BGP L3VPN services #15510

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

Conversation

cscarpitta
Copy link

Add test cases for SRv6

Type of change

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

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

T0

@cscarpitta cscarpitta marked this pull request as draft November 12, 2024 19:06
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/srv6/test_srv6.py

fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/srv6/test_srv6.py:70:13: E127 continuation line over-indented for visual indent
tests/srv6/test_srv6.py:111:13: E127 continuation line over-indented for visual indent
tests/srv6/test_srv6.py:149:121: E501 line too long (147 > 120 characters)
tests/srv6/test_srv6.py:150:121: E501 line too long (135 > 120 characters)
tests/srv6/test_srv6.py:159:121: E501 line too long (129 > 120 characters)
...
[truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
    the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
    docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@cscarpitta cscarpitta marked this pull request as ready for review November 13, 2024 15:43
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/srv6/test_srv6.py

fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Passed
flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
    the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
    docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@BYGX-wcr
Copy link
Contributor

@cscarpitta , you should add a test plan file

Signed-off-by: carmine <[email protected]>
Signed-off-by: Carmine Scarpitta <[email protected]>
@cscarpitta
Copy link
Author

@cscarpitta , you should add a test plan file

@BYGX-wcr Thanks for the review. I added the test plan.

cmd = "redis-cli -n 0 hgetall \"SRV6_SID_LIST_TABLE:Vrf10:{}\"".format(V6_PREFIX_NBR)
result = duthost.shell(cmd)
result = result['stdout']
Logger.info("Route table nexthops are %s", result)
Copy link
Contributor

Choose a reason for hiding this comment

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

The log statements seems to be not updated accordingly? You are checking SID_LIST_TABLE here right? The log statement for MY_SID table seems to be not correct as well.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, indeed the log statement was incorrect. I fixed it. Thanks.

" -c 'exit'"
" -c 'exit'"
" -c 'exit'"
" -c 'router bgp 64600'"
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do we configure the BGP session in this FRR config?

Copy link
Author

Choose a reason for hiding this comment

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

In the FRR config, the line below configures the BGP session with the other node.

 -c 'neighbor 2001:db8:2::1 remote-as 65100'

duthost.shell(cmd)
cmd = "config interface vrf bind Loopback1 Vrf10"
duthost.shell(cmd)
cmd = "config interface ip add Loopback1 2001:db8:2::1"
Copy link
Contributor

Choose a reason for hiding this comment

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

although this may not have any functional impact, but we'd better use a different Loopback address for DUT instead of reusing the Loopback address.

Copy link
Author

Choose a reason for hiding this comment

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

I agree with your point. I assigned the neighbor a Loopback address different from the DUT

result = result['stdout']
Logger.info("Route table nexthops are %s", result)
py_assert(result != "", "The DUT did not program SRv6 MySid entry")
py_assert("action: udt6" not in result, "The DUT did not program SRv6 MySid entry, missing 'action' field")
Copy link
Contributor

Choose a reason for hiding this comment

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

where do you configure the udt6 action?

Copy link
Author

Choose a reason for hiding this comment

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

uDT6 SID is created automatically by BGP.

When you set sid vpn export auto under the address-family ipv6 unicast section in the FRR config, BGP automatically creates the uDT6 SID.

@cscarpitta cscarpitta changed the title Add SRv6 test cases SRv6 uSID BGP L3VPN services Jan 14, 2025
Signed-off-by: Carmine Scarpitta <[email protected]>
…the Loopback address of DUT

Signed-off-by: Carmine Scarpitta <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

…the Loopback address of DUT

Signed-off-by: Carmine Scarpitta <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Carmine Scarpitta <[email protected]>
@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BYGX-wcr
Copy link
Contributor

Hi @cscarpitta,

Two issues:

  1. You did not clean up the config you added to the DUT. That would cause side-effect to the DUT, which is discouraged
  2. I tried running your test with 202412 image and failed to see any SID_LIST or SIDs in the APPL_DB. Is there any special requirement for the image?

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.

4 participants