Skip to content

Add mutli-asic support for ipfwd/test_dip_sip test#22580

Closed
arista-setu wants to merge 2 commits intosonic-net:masterfrom
arista-setu:master-fix-ipfwd-dip-sip-test-on-multi-
Closed

Add mutli-asic support for ipfwd/test_dip_sip test#22580
arista-setu wants to merge 2 commits intosonic-net:masterfrom
arista-setu:master-fix-ipfwd-dip-sip-test-on-multi-

Conversation

@arista-setu
Copy link
Contributor

Description of PR

This PR adds multi-asic support for ipfwd/test_dip_sip.py sonic-mgmt tests.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511

Approach

What is the motivation for this PR?

All test cases in sonic-mgmt test test_dip_sip.py are currently failing on mutli-asic.

How did you do it?

Add a function to determine asic namspace for the nexthop ip before adding or deleting route to make sure nexthop IP is reachable.

How did you verify/test it?

With test changes the test passes on mutli-asic and on single-asic systems :

Results from run on multi-asic:
ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzd541-0-ipv4_same_sip_dip] ✓
ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzd541-0-ipv4_different_sip_dip_connectedroute] ✓
ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzd541-0-ipv4_different_sip_dip_staticrouteprefix]
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzd541-0-ipv6_same_sip_dip] ✓
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzd541-0-ipv6_different_sip_dip_connectedroute] ✓
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzd541-0-ipv6_different_sip_dip_staticrouteprefix] ✓

Results (695.27s (0:11:35)):
6 passed

Results from run on single-asic:

ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzo204-None-ipv4_same_sip_dip] ✓ 17%
ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzo204-None-ipv4_different_sip_dip_connectedroute] ✓33%
ipfwd/test_dip_sip.py::test_ipv4_forwarding[qzo204-None-ipv4_different_sip_dip_staticrouteprefix] ✓50%
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzo204-None-ipv6_same_sip_dip] ✓ 67%
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzo204-None-ipv6_different_sip_dip_connectedroute] ✓83%
ipfwd/test_dip_sip.py::test_ipv6_forwarding[qzo204-None-ipv6_different_sip_dip_staticrouteprefix] ✓100%

Results (330.66s (0:05:30)):
6 passed

Any platform specific information?

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

Documentation

Add a function to correct ASIC for the nexthops before adding or deleting route.

Signed-off-by: setu <setu@arista.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rlhui rlhui requested review from YatishSVC and rawal01 March 4, 2026 18:27
Copy link
Contributor

@YatishSVC YatishSVC left a comment

Choose a reason for hiding this comment

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

lgtm, Thanks

@arista-setu
Copy link
Contributor Author

/azpw run Azure.sonic-mgmt

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

res = asic.command(f"{ip_cmd} route get {nexthop_ip}")
if res['rc'] == 0 and "unreachable" not in res.get('stdout', ''):
return asic
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

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

Since gather_facts gets facts for chosen random frontend asic we do not need this condition if you use enum_rand_one_frontend_asic_index like in PR #19931

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing the PR. I agree using enum_rand_one_frontend_asic_index would be better in this case as then we would not need the for loop and condition.

I see PR #19931 is approved and merge ready. I am okay if we wanna go with that solution.

@arista-setu
Copy link
Contributor Author

Merged PR #19931 which address the same issue. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants