zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces #11004

1337kerberos · 2022-04-11T05:14:24Z

If you are in a situation where you have multiple addresses on an
interface, zebra creates one connected route for them.
The issue is that the rib entry is not created if addresses were
added before the interface was running.

We add the address to a running interface in a typical flow.
Therefore, we handle the route & rib creation within a single ADD event.
In the opposite case, we create the route entries without activating them.
These are considered to be active since ZEBRA_IFC_DOWN is not set.
On the following interface UP, we ignore the same ADDR_ADD as it overlaps
with the already active prefixes -> rib is never created.

The minimal reproducible setup:

ip link add name dummy0 type dummy
ip addr flush dev dummy0
ip link set dummy0 down
ip addr add 192.168.1.7/24 dev dummy0
ip addr add 192.168.1.8/24 dev dummy0
ip link set dummy0 up
sudo vtysh -c 'show ip route' | grep dummy0

Signed-off-by: Volodymyr Huti volodymyr.huti@gmail.com

Fixes #10160

1337kerberos · 2022-04-11T05:19:12Z

Thanks to @zdc for the original reproducer
The original issue was introduced in - #8514
However, the logic was modified a bit in - #8659

Open questions:
[ ] I have made symmetrical modifications for both IP4/6, but I`m not sure if the logic applies to IP6 routing
[ ] Is this case covered within tests?

If not, should we implement such a test?
If yes, the issue is not noticed, the test should be fixed

NetDEF-CI · 2022-04-11T07:21:54Z

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-4797/

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.

donaldsharp · 2022-04-19T13:48:06Z

zebra/connected.c

@@ -326,7 +326,7 @@ void connected_add_ipv4(struct interface *ifp, int flags,
 	ifc->metric = metric;
 	/* If we get a notification from the kernel,
 	 * we can safely assume the address is known to the kernel */
-	SET_FLAG(ifc->conf, ZEBRA_IFC_QUEUED);
+	SET_FLAG(ifc->conf, ZEBRA_IFC_DOWN | ZEBRA_IFC_QUEUED);


Let's only add the ZEBRA_IFC_DOWN flag if the interface is actually down

if (if_up(ifp))
SET_FLAG(ifc->conf, ZEBRA_IFC_DOWN);

I have hidden it under if_is_operative(ifp) if I have understood you correctly

donaldsharp · 2022-04-19T19:15:06Z

can you remove the merge commit with a rebase and a force push and I'll get this in

If you are in a situation where you have multiple addresses on an interface, zebra creates one connected route for them. The issue is that the rib entry is not created if addresses were added before the interface was running. We add the address to a running interface in a typical flow. Therefore, we handle the route & rib creation within a single ADD event. In the opposite case, we create the route entries without activating them. These are considered to be active since ZEBRA_IFC_DOWN is not set. On the following interface UP, we ignore the same ADDR_ADD as it overlaps with the existing prefixes -> rib is never created. The minimal reproducible setup: ----------------------------------------- ip link add name dummy0 type dummy ip addr flush dev dummy0 ip link set dummy0 down ip addr add 192.168.1.7/24 dev dummy0 ip addr add 192.168.1.8/24 dev dummy0 ip link set dummy0 up vtysh -c 'show ip route' | grep dummy0 Signed-off-by: Volodymyr Huti <v.huti@vyos.io>

NetDEF-CI · 2022-04-19T22:11: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-4957/

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.

NetDEF-CI · 2022-04-19T22:33:13Z

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-4960/

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.

donaldsharp · 2022-04-19T22:36:46Z

@Mergifyio backport stable/8.2

mergify · 2022-04-19T22:37:43Z

backport stable/8.2

✅ Backports have been created

#11057 zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces (backport #11004) has been created for branch stable/8.2

NetDEF-CI · 2022-04-19T22:42:44Z

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-4956/

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.

NetDEF-CI · 2022-04-19T23:15:12Z

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-4959/

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.

NetDEF-CI · 2022-04-19T23:24:44Z

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-4955/

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.

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces (backport #11004)

frrbot bot added the zebra label Apr 11, 2022

mjstapp self-requested a review April 12, 2022 15:38

donaldsharp requested changes Apr 19, 2022

View reviewed changes

1337kerberos force-pushed the master branch from 14963af to 14ad3f2 Compare April 19, 2022 18:58

1337kerberos requested a review from donaldsharp April 19, 2022 19:10

1337kerberos force-pushed the master branch from c29ae75 to 14ad3f2 Compare April 19, 2022 19:14

1337kerberos force-pushed the master branch from 14ad3f2 to 6be2165 Compare April 19, 2022 19:46

1337kerberos force-pushed the master branch from 6be2165 to 7fb9825 Compare April 19, 2022 19:59

1337kerberos changed the title ~~zebra: default-initialze new connected routes with ZEBRA_IFC_DOWN~~ zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces Apr 19, 2022

donaldsharp merged commit 945f6ce into FRRouting:master Apr 19, 2022

mergify bot mentioned this pull request Apr 19, 2022

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces (backport #11004) #11057

Merged

ton31337 added a commit that referenced this pull request Apr 20, 2022

Merge pull request #11057 from FRRouting/mergify/bp/stable/8.2/pr-11004

56d26ba

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces (backport #11004)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces #11004

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces #11004

1337kerberos commented Apr 11, 2022

1337kerberos commented Apr 11, 2022

NetDEF-CI commented Apr 11, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

donaldsharp Apr 19, 2022

1337kerberos Apr 19, 2022

donaldsharp commented Apr 19, 2022

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

donaldsharp commented Apr 19, 2022

mergify bot commented Apr 19, 2022

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces #11004

zebra: set ZEBRA_IFC_DOWN on connected routes for inactive interfaces #11004

Conversation

1337kerberos commented Apr 11, 2022

The minimal reproducible setup:

1337kerberos commented Apr 11, 2022

NetDEF-CI commented Apr 11, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

donaldsharp Apr 19, 2022

Choose a reason for hiding this comment

1337kerberos Apr 19, 2022

Choose a reason for hiding this comment

donaldsharp commented Apr 19, 2022

NetDEF-CI commented Apr 19, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

donaldsharp commented Apr 19, 2022

mergify bot commented Apr 19, 2022

✅ Backports have been created

NetDEF-CI commented Apr 19, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022 • edited Loading

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 19, 2022

Continuous Integration Result: SUCCESSFUL

NetDEF-CI commented Apr 11, 2022 •

edited

Loading

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

NetDEF-CI commented Apr 19, 2022 •

edited

Loading

NetDEF-CI commented Apr 19, 2022 •

edited

Loading