pimd: Fix missing pimreg interface #13617
Conversation
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11780/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-11780/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11780/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-11780/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7 Topotests Ubuntu 18.04 arm8 part 7: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11780/artifact/TOPO7U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 7: No useful log foundTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11780/test Topology Tests failed for Topotests debian 10 amd64 part 9 Topotests debian 10 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7DEB10AMD64-11780/test Topology Tests failed for Topotests debian 10 amd64 part 7 Successful on other platforms/tests
|
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 arm8 part 9: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11784/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log foundTopotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-11784/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Topotests Ubuntu 18.04 i386 part 3: Failed (click for details)Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11784/artifact/TOPO3U18I386/TopotestDetails/Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3U18I386-11784/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3 Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11784/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7U18AMD64-11784/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 7 Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11784/test Topology Tests failed for Topotests debian 10 amd64 part 9 Successful on other platforms/tests
|
|
@anlancs can you add a topotest? Possible? |
|
Really sorry, i’m not familiar with topotest. @ton31337 |
`pimregX` of specific vrf can be deleted from kernel after this vrf is deleted. However, then `pimdregX` will never come back to kernel after adding it ( the same vrf ) back. That is to say, it exists only in daemon, but not in kernel. The root cause is this `pimregX` is not really deleted for its special usage, and `pim_if_create_pimreg()` wants reusing it. I have tried 4 solutions: 1. Remove the `configured` flag of `pimregX`, allow its deletion. A few timers still use `pimregX` after its deletion. So, not adopted. 2. Remove `pimregX` by vrf hook in `pim_instance_terminate()`. It has no vrf id there. So, not adopted. 3. Reuse `pimregX` in `pim_if_create_pimreg()`. If `pim->regiface->info` is true, then reuse old `pimregX` and only call `pim_if_add_vif()` to install it into kernel. But at that time, it maybe is in default VRF. The `pim_zebra_interface_set_master()` doesn't work at that time because it shouldn't wait there for its changing into correct VRF. So, not adopted. 4. Not reuse it. If `pim->regiface->info` is true, there must have been pim instance with VRF deleted and created before. Actually delele old one in `pim_if_create_pimreg()`, then recreate new one. Finally, this PR adopted the fourth solution. Fixes FRRouting#13454 Signed-off-by: anlan_cs <[email protected]>
anlancs
force-pushed
the
fix/pimd-remove-pimreg-vrf
branch
from
e0cdf71 to
b016b55
Compare
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11823/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests Ubuntu 18.04 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18AMD64-11823/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 9 Topotests Ubuntu 18.04 arm8 part 7: Failed (click for details)Topotests Ubuntu 18.04 arm8 part 7: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11823/artifact/TOPO7U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 7: No useful log foundSuccessful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9U18I386-11953/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 9 Topotests debian 10 amd64 part 7: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO7DEB10AMD64-11953/test Topology Tests failed for Topotests debian 10 amd64 part 7 Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11953/test Topology Tests failed for Topotests debian 10 amd64 part 9 Successful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 amd64 part 0: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPOU1804-11971/test Topology Tests failed for Topotests Ubuntu 18.04 amd64 part 0 Topotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-11971/test Topology Tests failed for Topotests debian 10 amd64 part 9 Successful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests Ubuntu 18.04 i386 part 3: Failed (click for details)Topotests Ubuntu 18.04 i386 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-11973/artifact/TOPO3U18I386/TopotestDetails/Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO3U18I386-11973/test Topology Tests failed for Topotests Ubuntu 18.04 i386 part 3 Successful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: SuccessfulBasic Tests: FailedTopotests debian 10 amd64 part 9: Failed (click for details)Topology Test Results are at https://ci1.netdef.org/browse/FRR-PULLREQ2-TOPO9DEB10AMD64-12062/test Topology Tests failed for Topotests debian 10 amd64 part 9 Successful on other platforms/tests
|
|
ci:rerun |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-12204/ This is a comment from an automated CI system. |
|
@eqvinox @mobash-rasool @donaldsharp Can you take a look at it? Thanks! |
|
@anlancs : Why solution 2 does not work in your case? |
|
With solution 2, I remembered i had tried solution 2, and really deleted this old |
|
No comment? Any comment would be appreciated. |
There was a problem hiding this comment.
I have few questions regarding this:
- Why does pim instance exists after vrf deletion? As per the issue you are trying to fix, you have deleted the vrf but you say pimreg exists in the pim instance.
- Another thing, when the vrf is being deleted, we must delete the pimreg in that flow i.e pim_instance_terminate flow.
|
@mobash-rasool |
|
@anlancs : What are the steps to repro this issue? |
|
After pimreg interface of one vrf is created, then delete this vrf from both kernel (use "ip link del" ) and vtysh (use "no vrf x"). |
|
@Mergifyio backport dev/9.1 stable/9.0 |
✅ Backports have been created
|
pimd: Fix missing pimreg interface (backport #13617)
pimregXof specific vrf can be deleted from kernel after this vrfis deleted. However, then
pimdregXwill never come back tokernel after adding it ( the same vrf ) back. That is to say, it exists
only in daemon, but not in kernel.
The root cause is this
pimregXis not really deleted for its specialusage, and
pim_if_create_pimreg()wants reusing it.I have tried 4 solutions:
configuredflag ofpimregX, allow its deletion.A few timers still use
pimregXafter its deletion. So, not adopted.pimregXby vrf hook inpim_instance_terminate().It has no vrf id there, (and it maybe already is moved to default VRF). So, not adopted.
pimregXinpim_if_create_pimreg().If
pim->regiface->infois true, then reuse oldpimregXand only callpim_if_add_vif()to install it into kernel. But at that time, it maybeis in default VRF. The
pim_zebra_interface_set_master()doesn't workat that time because it shouldn't wait there for its changing into
correct VRF. So, not adopted.
If
pim->regiface->infois true, there must have been pim instance withVRF deleted and created before. Actually delele old one in
pim_if_create_pimreg(), then recreate new one.Finally, this PR adopted the fourth solution.
Fixes #13454