zebra: Fix connected route deletion when multiple entry exists #13340

leonshaw · 2023-04-20T04:15:26Z

When multiple interfaces have addresses in the same network, deleting one of them may cause the wrong connected route being deleted. For example:

ip link add veth1 type veth peer veth2
ip link set veth1 up
ip link set veth2 up
ip addr add dev veth1 192.168.0.1/24
ip addr add dev veth2 192.168.0.2/24
ip addr flush dev veth1

Zebra deletes the route of interface veth2 rather than veth1.

Should match nexthop against ere->re_nhe instead of ere->re->nhe.

When multiple interfaces have addresses in the same network, deleting one of them may cause the wrong connected route being deleted. For example: ip link add veth1 type veth peer veth2 ip link set veth1 up ip link set veth2 up ip addr add dev veth1 192.168.0.1/24 ip addr add dev veth2 192.168.0.2/24 ip addr flush dev veth1 Zebra deletes the route of interface veth2 rather than veth1. Should match nexthop against ere->re_nhe instead of ere->re->nhe. Signed-off-by: Xiao Liang <[email protected]>

NetDEF-CI · 2023-04-20T07:05:00Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-10994/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

ton31337 · 2023-04-20T19:00:41Z

@Mergifyio backport stable/8.5 stable/8.4

mergify · 2023-04-20T19:00:44Z

backport stable/8.5 stable/8.4

✅ Backports have been created

#14586 zebra: Fix connected route deletion when multiple entry exists (backport #13340) has been created for branch stable/8.5
#14587 zebra: Fix connected route deletion when multiple entry exists (backport #13340) has been created for branch stable/8.4

mjstapp

were both of these changes required to fix the issue, or was it just the second assignment, inside the 'if' clause ?

leonshaw · 2023-04-21T01:51:39Z

were both of these changes required to fix the issue, or was it just the second assignment, inside the 'if' clause ?

The nexthop to be deleted is stored in ere->re_nhe in rib_delete(), so need both.

frr/zebra/zebra_rib.c

Lines 4295 to 4308 in e2bebeb

    
           if (nh) { 
        
           	nhe = zebra_nhg_alloc(); 
        
           	nhe->nhg.nexthop = nexthop_dup(nh, NULL); 
        
           } 
        
           ere = XCALLOC(MTYPE_WQ_WRAPPER, sizeof(*ere)); 
        
           ere->afi = afi; 
        
           ere->safi = safi; 
        
           ere->p = *p; 
        
           if (src_p) 
        
           	ere->src_p = *src_p; 
        
           ere->src_p_provided = !!src_p; 
        
           ere->re = re; 
        
           ere->re_nhe = nhe;

mwinter-osr · 2023-09-25T15:37:26Z

CI:rerun Rerunning as the last execution is several months back

NetDEF-CI · 2023-09-25T18:35:12Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests 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-14329/artifact/TOPO9U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 9: No useful log found

Topotests Ubuntu 18.04 arm8 part 3: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 3: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/artifact/TOPO3U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 3: No useful log found

Topotests Ubuntu 18.04 arm8 part 1: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 1: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/artifact/TOPO1U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 1: No useful log found

Topotests Ubuntu 18.04 arm8 part 8: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 8: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/artifact/TOPO8U18AMD64/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 8: No useful log found

Topotests Ubuntu 18.04 arm8 part 6: Failed (click for details)

Topotests Ubuntu 18.04 arm8 part 6: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/artifact/TOPO6U18ARM8/TopotestDetails/ Topotests Ubuntu 18.04 arm8 part 6: No useful log found

Successful on other platforms/tests

Topotests Ubuntu 18.04 amd64 part 2
Addresssanitizer topotests part 5
Topotests Ubuntu 18.04 i386 part 3
Topotests Ubuntu 18.04 i386 part 8
Topotests debian 10 amd64 part 8
Addresssanitizer topotests part 0
Topotests Ubuntu 18.04 arm8 part 4
Topotests debian 10 amd64 part 4
Topotests debian 10 amd64 part 9
Addresssanitizer topotests part 4
Topotests debian 10 amd64 part 3
Addresssanitizer topotests part 1
Topotests Ubuntu 18.04 i386 part 4
Topotests Ubuntu 18.04 amd64 part 8
Topotests Ubuntu 18.04 arm8 part 7
Addresssanitizer topotests part 9
Topotests Ubuntu 18.04 arm8 part 0
Static analyzer (clang)
Topotests debian 10 amd64 part 0
Topotests debian 10 amd64 part 2
Topotests Ubuntu 18.04 arm8 part 2
Topotests Ubuntu 18.04 amd64 part 9
Debian 10 deb pkg check
Topotests Ubuntu 18.04 i386 part 1
Addresssanitizer topotests part 7
Topotests Ubuntu 18.04 amd64 part 5
Topotests Ubuntu 18.04 i386 part 6
Topotests Ubuntu 18.04 arm8 part 5
Ubuntu 20.04 deb pkg check
Topotests debian 10 amd64 part 1
Topotests debian 10 amd64 part 6
Topotests Ubuntu 18.04 amd64 part 7
Topotests Ubuntu 18.04 i386 part 5
Topotests Ubuntu 18.04 i386 part 0
Addresssanitizer topotests part 3
Topotests Ubuntu 18.04 amd64 part 4
CentOS 7 rpm pkg check
Addresssanitizer topotests part 2
Topotests Ubuntu 18.04 amd64 part 0
Topotests Ubuntu 18.04 i386 part 9
Topotests Ubuntu 18.04 amd64 part 3
Topotests debian 10 amd64 part 7
Topotests debian 10 amd64 part 5
Debian 9 deb pkg check
Addresssanitizer topotests part 8
Topotests Ubuntu 18.04 i386 part 7
Topotests Ubuntu 18.04 amd64 part 6
Topotests Ubuntu 18.04 i386 part 2
Topotests Ubuntu 18.04 amd64 part 1
Addresssanitizer topotests part 6
Ubuntu 18.04 deb pkg check

NetDEF-CI · 2023-09-26T01:35:46Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14329/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

OSPF on IPv4/IPv6 removes the wrong routes in certain cases, causing issues when removing and re-enabling interfaces. This test proofs that. These tests all pass with FRRouting#13340 and the latest master (d2324b7). See FRRouting#14488

OSPF on IPv4/IPv6 removes the wrong routes in certain cases, causing issues when removing and re-enabling interfaces. This test proofs that. These tests all pass with FRRouting#13340 and the latest master (d2324b7). See FRRouting#14488 Signed-off-by: Denis Krienbühl <[email protected]>

donaldsharp · 2023-10-13T12:54:17Z

@Mergifyio backport stable/9.0 dev/9.1

mergify · 2023-10-13T12:54:21Z

backport stable/9.0 dev/9.1

✅ Backports have been created

#14588 zebra: Fix connected route deletion when multiple entry exists (backport #13340) has been created for branch stable/9.0
#14589 zebra: Fix connected route deletion when multiple entry exists (backport #13340) has been created for branch dev/9.1

OSPF on IPv4/IPv6 removes the wrong routes in certain cases, causing issues when removing and re-enabling interfaces. This test proofs that. These tests all pass with FRRouting#13340 and the latest master (d2324b7). See FRRouting#14488 Signed-off-by: Denis Krienbühl <[email protected]>

zebra: Fix connected route deletion when multiple entry exists (backport #13340)

OSPF on IPv4/IPv6 removes the wrong routes in certain cases, causing issues when removing and re-enabling interfaces. This test proofs that. These tests all pass with #13340 and the latest master (d2324b7). See #14488 Signed-off-by: Denis Krienbühl <[email protected]> (cherry picked from commit 616e1fa)

OSPF on IPv4/IPv6 removes the wrong routes in certain cases, causing issues when removing and re-enabling interfaces. This test proofs that. These tests all pass with FRRouting#13340 and the latest master (d2324b7). See FRRouting#14488 Ticket: #3684268 Signed-off-by: Denis Krienbühl <[email protected]> (cherry picked from commit 616e1fa)

frrbot bot added the zebra label Apr 20, 2023

github-actions bot added master size/XS labels Apr 20, 2023

github-actions bot added the backport label Apr 20, 2023

mjstapp reviewed Apr 20, 2023

View reviewed changes

donaldsharp self-requested a review May 2, 2023 15:35

mwinter-osr mentioned this pull request Sep 25, 2023

Wrong delete/add of connected routes when interface flaps #14488

Closed

href mentioned this pull request Oct 12, 2023

tests: Add OSPF test for issue 14488 #14582

Merged

donaldsharp merged commit 4f7dc77 into FRRouting:master Oct 13, 2023

donaldsharp added a commit that referenced this pull request Oct 13, 2023

Merge pull request #14587 from FRRouting/mergify/bp/stable/8.4/pr-13340

86c9e50

zebra: Fix connected route deletion when multiple entry exists (backport #13340)

donaldsharp added a commit that referenced this pull request Oct 13, 2023

Merge pull request #14589 from FRRouting/mergify/bp/dev/9.1/pr-13340

e811461

zebra: Fix connected route deletion when multiple entry exists (backport #13340)

donaldsharp added a commit that referenced this pull request Oct 13, 2023

Merge pull request #14586 from FRRouting/mergify/bp/stable/8.5/pr-13340

5ad85a1

zebra: Fix connected route deletion when multiple entry exists (backport #13340)

donaldsharp added a commit that referenced this pull request Oct 13, 2023

Merge pull request #14588 from FRRouting/mergify/bp/stable/9.0/pr-13340

633a030

zebra: Fix connected route deletion when multiple entry exists (backport #13340)

leonshaw deleted the fix/rib-del-connected branch October 16, 2023 02:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zebra: Fix connected route deletion when multiple entry exists #13340

zebra: Fix connected route deletion when multiple entry exists #13340

leonshaw commented Apr 20, 2023

NetDEF-CI commented Apr 20, 2023 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

ton31337 commented Apr 20, 2023

mergify bot commented Apr 20, 2023 •

edited

Loading

mjstapp left a comment

leonshaw commented Apr 21, 2023

mwinter-osr commented Sep 25, 2023

NetDEF-CI commented Sep 25, 2023 •

edited

Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Sep 26, 2023

donaldsharp commented Oct 13, 2023

mergify bot commented Oct 13, 2023 •

edited

Loading

zebra: Fix connected route deletion when multiple entry exists #13340

zebra: Fix connected route deletion when multiple entry exists #13340

Conversation

leonshaw commented Apr 20, 2023

NetDEF-CI commented Apr 20, 2023 • edited Loading

Continuous Integration Result: SUCCESSFUL

ton31337 commented Apr 20, 2023

mergify bot commented Apr 20, 2023 • edited Loading

✅ Backports have been created

mjstapp left a comment

Choose a reason for hiding this comment

leonshaw commented Apr 21, 2023

mwinter-osr commented Sep 25, 2023

NetDEF-CI commented Sep 25, 2023 • edited Loading

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

NetDEF-CI commented Sep 26, 2023

Continuous Integration Result: SUCCESSFUL

donaldsharp commented Oct 13, 2023

mergify bot commented Oct 13, 2023 • edited Loading

✅ Backports have been created

NetDEF-CI commented Apr 20, 2023 •

edited

Loading

mergify bot commented Apr 20, 2023 •

edited

Loading

NetDEF-CI commented Sep 25, 2023 •

edited

Loading

mergify bot commented Oct 13, 2023 •

edited

Loading