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

nhrpd, test: fix nhrp_redundancy topotest (backport #16698) (backport #16699) #16719

Merged
merged 10 commits into from
Oct 3, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Sep 2, 2024

backport #16698 into stable/10.1


This is an automatic backport of pull request #16699 done by Mergify.

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: #16690
Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit cfb057a)
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]>
(cherry picked from commit 3f49c50)
Rename router variables in nhrp_redundancy to match the actual name.
Cosmetic change to help debugging.

Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit 551c329)
Simplify nhrp_redundancy convergence code. Cosmetic change.

Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit dbd888c)
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]>
(cherry picked from commit df630bb)
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]>
(cherry picked from commit 9aa7807)
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]>
(cherry picked from commit b175a3f)
Fix show nhrp shortcut json

Fixes: 87b9e98 ("nhrpd: add json support to show nhrp vty commands")
Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit 7028e39)
Check show ip nhrp shorcut in nhrp_redundancy

Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit 7f587c9)
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]>
(cherry picked from commit 995f6b8)
@donaldsharp
Copy link
Member

Any particular reason we are pushing back these topotest changes to 10.0?

@Jafaral
Copy link
Member

Jafaral commented Sep 5, 2024

Any particular reason we are pushing back these topotest changes to 10.0?

To add to that, we can push the fixes, but should avoid pushing other changes.

@ton31337
Copy link
Member

ton31337 commented Oct 3, 2024

This is why we need this PR to be backported to 10.0, it's annoying for recent releases seeing failures in tests:

Screenshot-2024-10-03-09:32:40

@ton31337 ton31337 merged commit 6585471 into stable/10.0 Oct 3, 2024
15 of 16 checks passed
@ton31337 ton31337 deleted the mergify/bp/stable/10.0/pr-16699 branch October 3, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants