nhrpd, test: fix nhrp_redundancy topotest #16698
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The expected prefix should be 5.5.5.0/24 otherwise the hosts behind NHRP client 1 nhc1 (aka. r5) are not reachable via NHRP. The issue was not seen in the FRR official CI because the tests were skipped because iptables were missing in CI machines. It solves the 16690 issue. Fixes: FRRouting#16690 Signed-off-by: Louis Scalbert <[email protected]>
Apply black nhrp_redundancy Signed-off-by: Louis Scalbert <[email protected]>
Rename routers in nhrp_redundancy to match the diagram. Cosmetic change. > cd tests/topotests/nhrp_redundancy > git grep r1 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r1|nhs1|g' -i > git grep r2 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r2|nhs2|g' -i > git grep r3 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r3|nhs3|g' -i > git grep r4 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r4|nhc1|g' -i > git grep r5 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r5|nhc2|g' -i > git grep r6 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r6|router|g' -i > git grep r7 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|r7|host|g' -i > > git grep R1 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R1|nhs1|g' -i > git grep R2 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R2|nhs2|g' -i > git grep R3 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R3|nhs3|g' -i > git grep R4 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R4|nhc1|g' -i > git grep R5 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R5|nhc2|g' -i > git grep R6 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R6|router|g' -i > git grep R7 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|R7|host|g' -i > > mv r1 nhs1 > mv r2 nhs2 > mv r3 nhs3 > mv r4 nhc1 > mv r5 nhc2 > mv r6 router > mv r7 host Signed-off-by: Louis Scalbert <[email protected]>
Rename router variables in nhrp_redundancy to match the actual name. Cosmetic change to help debugging. Signed-off-by: Louis Scalbert <[email protected]>
Simplify nhrp_redundancy convergence code. Cosmetic change. Signed-off-by: Louis Scalbert <[email protected]>
After setting down nhs1, the test is checking that nhc1 routing table matches routes in nhc1/nhrp_route.json. It is incorrect because it checks that the NHRP route to nhs1 is still present but it should have disappeared. Signed-off-by: Louis Scalbert <[email protected]>
Fully check the NHRP convergence after setting nhs1 down. Otherwise the ping may pass because the previous shortcut is still present. Signed-off-by: Louis Scalbert <[email protected]>
Replace --nflog-range argument by --nflog-range due to: > 2024-08-30 10:44:54,816 INFO: topo: input: iptables -A FORWARD -i nhs3-gre0 -o nhs3-gre0 -m hashlimit --hashlimit-upto 4/minute --hashlimit-burst 1 --hashlimit-mode srcip,dstip --hashlimit-srcmask 24 --hashlimit-dstmask 24 --hashlimit-name loglimit-0 -j NFLOG --nflog-group 1 --nflog-range 128 > 2024-08-30 10:44:54,819 INFO: topo: output: warn: --nflog-range has never worked and is no longer supported, please use --nflog-size insted Signed-off-by: Louis Scalbert <[email protected]>
Fix show nhrp shortcut json Fixes: 87b9e98 ("nhrpd: add json support to show nhrp vty commands") Signed-off-by: Louis Scalbert <[email protected]>
Check show ip nhrp shorcut in nhrp_redundancy Signed-off-by: Louis Scalbert <[email protected]>
Use private addresses in nhrp_redundancy. > cd tests/topotests/nhrp_redundancy > git grep 176.16 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|176.16|172.16|g' -i > git grep 5.5.5. | cut -f1 -d: | uniq | xargs -L1 sed -e 's|5.5.5.|10.5.5.|g' -i > git grep 4.4.4 | cut -f1 -d: | uniq | xargs -L1 sed -e 's|4.4.4.|10.4.4.|g' -i Signed-off-by: Louis Scalbert <[email protected]>
|
@louis-6wind Since you are overhauling the tests and renaming node names, can you please use unified config too? I.e, use If nhrpd doesn't start after you switch to frr.conf, you may need to add logiic like we did with pim here. Just grep for |
|
@Jafaral I will not do it because I have no time for that rework. The idea of the renaming was to understand the test and then to fix it. Your rework is out of scope |
|
Well, with all of those files renamed, it is hard to know what this fix is about. The diff looks like more like a rewrite of the test rather than just a fix. What really changed here? |
Individual commits are explicit. |
|
ci:rerun |
ton31337
added a commit
that referenced
this pull request
Sep 2, 2024
nhrpd, test: fix nhrp_redundancy topotest (backport #16698)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The expected prefix should be 5.5.5.0/24 otherwise the hosts behind NHRP
client 1 nhc1 (aka. r5) are not reachable via NHRP.
The issue was not seen in the FRR official CI because the tests were
skipped because iptables were missing in CI machines.
Fixes: #16690
This pull-request also fixes some other things in the nhrp_redundancy topotest