sonic-mgmt icon indicating copy to clipboard operation
sonic-mgmt copied to clipboard

Handle p bit properly in bfd_responder

Open abdbaig opened this issue 1 year ago • 3 comments

Description of PR

Summary: On cisco platform, ipv4/ipv6 multihop tests are failing because BFD injected packets are not using correct queue (should be UC7 instead of UC0). The reason is because once BFD session is up, BFD responder always sends packets with P bit set. When DUT receives these packets, it punts them to control plane which injects packets with F bit set in response. Why these packets with F bit don't go out of UC7 is a known issue which is already fixed in cisco sdk. However, even without the sdk fix, there should gone be constant exchange of packets with P/F bits when session is UP. BFD responder currently uses first packet it receives from DUT as template and always uses flags from that packet for subsequent packets (which has P bit set). The fix is in BFD responder to respond with F bit set if a packet with P bit is received, otherwise do not set any flags for other packets.

Type of change

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

Back port request

  • [ ] 202012
  • [ ] 202205
  • [ ] 202305
  • [ x] 202311
  • [x ] 202405

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

abdbaig avatar Oct 25 '24 04:10 abdbaig

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.................................................Passed
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

ansible/roles/test/files/helpers/bfd_responder.py:95:13: E265 block comment should start with '# '
ansible/roles/test/files/helpers/bfd_responder.py:96:35: F821 undefined name 'BFD_FLAG_P_BIT'
ansible/roles/test/files/helpers/bfd_responder.py:97:75: F821 undefined name 'BFD_FLAG_F_BIT'

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>

mssonicbld avatar Oct 25 '24 04:10 mssonicbld

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.................................................Passed
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

ansible/roles/test/files/helpers/bfd_responder.py:20:1: E302 expected 2 blank lines, found 1

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>

mssonicbld avatar Oct 25 '24 04:10 mssonicbld

Hi @siqbal1986 , can you help review this PR?

wsycqyz avatar Oct 31 '24 06:10 wsycqyz

@siqbal1986 Can you help on this PR?

wsycqyz avatar Nov 15 '24 02:11 wsycqyz

Cherry-pick PR to 202405: https://github.com/sonic-net/sonic-mgmt/pull/15861

mssonicbld avatar Dec 04 '24 00:12 mssonicbld