frr
frr copied to clipboard
srv6: Introduce IPv6 address in segment-list used by TE Policy
These series of patches introduce new feature: user may configure ipv6-address in segment list.
Format:
index INDEX ipv6-address X:X::X:X
Example:
segment-routing
traffic-eng
segment-list SL1
index 0 ipv6-address 2001:db8::1
Signed-off-by: Dmytro Shytyi [email protected]
Fix warnings by checkpatch.
2 small fixes for isis_sr_te uploaded in the recent push.
Thank you very much for your reviews, @ton31337, @riw777, @donaldsharp.
I was on PTO, thus the response delay. I expect to look at this PR in coming time.
@cscarpitta, WDYT about the cli format for ipv6 address? Does it look good?
ci:rerun
Dear @ton31337, @riw777, @donaldsharp. Thank you for reviews. I have responded on all comments and provided a new code version that addresses the comments. It passes all basics sanity checks and topotests. Could you please have a look at this PR again?
ci:rerun
CI:rerun libyang version may have changed during execution which caused the failure
@donaldsharp, you raised the next question:
What is supposed to happen when we have > 1 million routes? What is a reasonable number of items that are going to be sent. Are we going to run out of packet space? Are we going to just cpuhog like you would not believe?
There is no more a loop over all routes and the max segs is verified and limited if exceeds the defined in header value. Thanks.
ci:rerun
Hello, @donaldsharp @ton31337 @riw777. The PR passes the CI tests and comments are addressed. WDYT?
looks good to me ... waiting on @donaldsharp, I think?
looks good to me ... waiting on @donaldsharp, I think?
Seems like the blocker (changes requested) was introduced by @donaldsharp. It would be perfect to get his reply based on the latest status of PR.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
Rebase was performed to fix emerging conflicts.
ci:rerun
ci:rerun 1 unrelated failure
ci:rerun
unrelated failures...
Hello @ton31337 , @donaldsharp, @riw777 Thank you very much for these reviews. Is there anything you would like to add as a comments for this PR?
This pull request has conflicts, please resolve those before we can evaluate the pull request.
ci:rerun
Hello @donaldsharp,
Basic tests are OK.
Could you please take a look at this PR?
Thanks in advance.
Hello @riw777 @ton31337 @donaldsharp
- basic tests are OK
- There is old "changes requested" that blocks the merging. All comments already were addressed some time ago.
@donaldsharp, no more new comments from reviewers for some time... If you don't have any comments left, please change the status of your review. Hopefully we will be able to unblock and merge it.
I think the only thing holding this is @donaldsharp clearing his change request block ...
When I run the new test under any type of load it fails badly:
================================================================== short test summary info ==================================================================
FAILED ospf6_ecmp_inter_area/test_h.py::test_ecmp_inter_area - AssertionError: 'r1' wrong number of route nexthops
FAILED ospf6_ecmp_inter_area/test_e.py::test_ecmp_inter_area - AssertionError: 'r1' wrong number of route nexthops
FAILED isis_srv6_te_topo1/test_zy.py::test_rib_ipv6_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zs.py::test_srv6_te_policy_additional_route - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zd.py::test_rib_ipv6_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_ze.py::test_srv6_te_policy_activated - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zz.py::test_srv6_te_policy_removed - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zr.py::test_srv6_te_policy_removed - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zi.py::test_srv6_te_policy_additional_route - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zc.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zh.py::test_rib_ipv6_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zv.py::test_srv6_te_policy_additional_route - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_isis_srv6_te_topo1.py::test_srv6_te_policy_removed - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zt.py::test_srv6_te_policy_activated - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zn.py::test_srv6_te_policy_activated - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zm.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zk.py::test_srv6_te_policy_removed - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zo.py::test_srv6_te_policy_removed - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zw.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zj.py::test_srv6_te_policy_activated - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zb.py::test_rib_ipv6_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zu.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_za.py::test_rib_ipv6_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zp.py::test_srv6_te_policy_additional_route - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zf.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zl.py::test_srv6_te_policy_activated - AssertionError: "rt1" JSON output mismatches the expected result
FAILED isis_srv6_te_topo1/test_zq.py::test_srv6_locator_step1 - AssertionError: "rt1" JSON output mismatches the expected result
FAILED ospf6_ecmp_inter_area/test_b.py::test_ecmp_inter_area - AssertionError: 'r1' wrong number of route nexthops
FAILED isis_srv6_te_topo1/test_zx.py::test_srv6_te_policy_additional_route - AssertionError: "rt1" JSON output mismatches the expected result
================================================= 29 failed, 1732 passed, 279 skipped in 819.59s (0:13:39) =============================```
Please spend some time running your new test under different loads and identifying and fixing the problems found.
as a note when I run the test by itself or concurrently with itself it works fine. It's only when scale/load is placed on the system that causes it to fail badly. This means, to me, that the test is starting to look for data before some other event has occurred and we get this output. See the hundreds of fixes in the topotest directory as we fix this sort of problem in the test for guidance on how to approach the problem. #16586, #16583, #16523, #16510 and #16485 as recent examples of this sort of problem.