zebra: don't change connected state from zebra/interface.c

Try to avoid changing connected state from zebra/interface.c as this
means making assumptions about kernel behaviour which may be or may
become wrong. This state should rather be updated by events from the
kernel.

Signed-off-by: Christian Franke <chris@opensourcerouting.org>
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This commit is contained in:
Christian Franke 2013-01-24 14:04:49 +00:00 committed by David Lamparter
parent f7f740fe58
commit 02b4805f39
2 changed files with 69 additions and 106 deletions

View File

@ -37,7 +37,7 @@
#include "zebra/connected.h" #include "zebra/connected.h"
extern struct zebra_t zebrad; extern struct zebra_t zebrad;
/* withdraw a connected address */ /* communicate the withdrawal of a connected address */
static void static void
connected_withdraw (struct connected *ifc) connected_withdraw (struct connected *ifc)
{ {
@ -81,24 +81,19 @@ connected_announce (struct interface *ifp, struct connected *ifc)
listnode_add (ifp->connected, ifc); listnode_add (ifp->connected, ifc);
/* Update interface address information to protocol daemon. */ /* Update interface address information to protocol daemon. */
if (! CHECK_FLAG (ifc->conf, ZEBRA_IFC_REAL)) if (ifc->address->family == AF_INET)
if_subnet_add (ifp, ifc);
zebra_interface_address_add_update (ifp, ifc);
if (if_is_operative(ifp))
{ {
if (ifc->address->family == AF_INET) if (ifc->address->family == AF_INET)
if_subnet_add (ifp, ifc); connected_up_ipv4 (ifp, ifc);
SET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
zebra_interface_address_add_update (ifp, ifc);
if (if_is_operative(ifp))
{
if (ifc->address->family == AF_INET)
connected_up_ipv4 (ifp, ifc);
#ifdef HAVE_IPV6 #ifdef HAVE_IPV6
else else
connected_up_ipv6 (ifp, ifc); connected_up_ipv6 (ifp, ifc);
#endif #endif
}
} }
} }
@ -116,7 +111,7 @@ connected_check (struct interface *ifp, struct prefix *p)
return NULL; return NULL;
} }
/* Check if two ifc's describe the same address */ /* Check if two ifc's describe the same address in the same state */
static int static int
connected_same (struct connected *ifc1, struct connected *ifc2) connected_same (struct connected *ifc1, struct connected *ifc2)
{ {
@ -137,6 +132,9 @@ connected_same (struct connected *ifc1, struct connected *ifc2)
if (ifc1->flags != ifc2->flags) if (ifc1->flags != ifc2->flags)
return 0; return 0;
if (ifc1->conf != ifc2->conf)
return 0;
return 1; return 1;
} }
@ -156,7 +154,7 @@ connected_update(struct interface *ifp, struct connected *ifc)
/* Avoid spurious withdraws, this might be just the kernel 'reflecting' /* Avoid spurious withdraws, this might be just the kernel 'reflecting'
* back an address we have already added. * back an address we have already added.
*/ */
if (connected_same (current, ifc) && CHECK_FLAG(current->conf, ZEBRA_IFC_REAL)) if (connected_same (current, ifc))
{ {
/* nothing to do */ /* nothing to do */
connected_free (ifc); connected_free (ifc);
@ -169,8 +167,9 @@ connected_update(struct interface *ifp, struct connected *ifc)
connected_withdraw (current); /* implicit withdraw - freebsd does this */ connected_withdraw (current); /* implicit withdraw - freebsd does this */
} }
/* If the connected is new or has changed, announce it */ /* If the connected is new or has changed, announce it, if it is usable */
connected_announce(ifp, ifc); if (CHECK_FLAG(ifc->conf, ZEBRA_IFC_REAL))
connected_announce(ifp, ifc);
} }
/* Called from if_up(). */ /* Called from if_up(). */
@ -279,6 +278,10 @@ connected_add_ipv4 (struct interface *ifp, int flags, struct in_addr *addr,
if (label) if (label)
ifc->label = XSTRDUP (MTYPE_CONNECTED_LABEL, label); ifc->label = XSTRDUP (MTYPE_CONNECTED_LABEL, label);
/* For all that I know an IPv4 address is always ready when we receive
* the notification. So it should be safe to set the REAL flag here. */
SET_FLAG(ifc->conf, ZEBRA_IFC_REAL);
connected_update(ifp, ifc); connected_update(ifp, ifc);
} }
@ -407,6 +410,14 @@ connected_add_ipv6 (struct interface *ifp, int flags, struct in6_addr *addr,
if (label) if (label)
ifc->label = XSTRDUP (MTYPE_CONNECTED_LABEL, label); ifc->label = XSTRDUP (MTYPE_CONNECTED_LABEL, label);
/* On Linux, we only get here when DAD is complete, therefore we can set
* ZEBRA_IFC_REAL.
*
* On BSD, there currently doesn't seem to be a way to check for completion of
* DAD, so we replicate the old behaviour and set ZEBRA_IFC_REAL, although DAD
* might still be running.
*/
SET_FLAG(ifc->conf, ZEBRA_IFC_REAL);
connected_update(ifp, ifc); connected_update(ifp, ifc);
} }

View File

@ -193,6 +193,9 @@ if_subnet_delete (struct interface *ifp, struct connected *ifc)
ifc = listgetdata (listhead (addr_list)); ifc = listgetdata (listhead (addr_list));
zebra_interface_address_delete_update (ifp, ifc); zebra_interface_address_delete_update (ifp, ifc);
UNSET_FLAG (ifc->flags, ZEBRA_IFA_SECONDARY); UNSET_FLAG (ifc->flags, ZEBRA_IFA_SECONDARY);
/* XXX: Linux kernel removes all the secondary addresses when the primary
* address is removed. We could try to work around that, though this is
* non-trivial. */
zebra_interface_address_add_update (ifp, ifc); zebra_interface_address_add_update (ifp, ifc);
} }
@ -296,16 +299,23 @@ if_addr_wakeup (struct interface *ifp)
{ {
if (! if_is_up (ifp)) if (! if_is_up (ifp))
{ {
/* XXX: WTF is it trying to set flags here? /* Assume zebra is configured like following:
* caller has just gotten a new interface, has been *
* handed the flags already. This code has no business * interface gre0
* trying to override administrative status of the interface. * ip addr 192.0.2.1/24
* The only call path to here which doesn't originate from * !
* kernel event is irdp - what on earth is it trying to do? *
* * As soon as zebra becomes first aware that gre0 exists in the
* further RUNNING is not a settable flag on any system * kernel, it will set gre0 up and configure its addresses.
* I (paulj) am aware of. *
*/ * (This may happen at startup when the interface already exists
* or during runtime when the interface is added to the kernel)
*
* XXX: IRDP code is calling here via if_add_update - this seems
* somewhat weird.
* XXX: RUNNING is not a settable flag on any system
* I (paulj) am aware of.
*/
if_set_flags (ifp, IFF_UP | IFF_RUNNING); if_set_flags (ifp, IFF_UP | IFF_RUNNING);
if_refresh (ifp); if_refresh (ifp);
} }
@ -318,23 +328,17 @@ if_addr_wakeup (struct interface *ifp)
continue; continue;
} }
/* Add to subnet chain list. */
if_subnet_add (ifp, ifc);
SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
SET_FLAG (ifc->conf, ZEBRA_IFC_REAL); /* The address will be advertised to zebra clients when the notification
* from the kernel has been received.
zebra_interface_address_add_update (ifp, ifc); * It will also be added to the interface's subnet list then. */
if (if_is_operative(ifp))
connected_up_ipv4 (ifp, ifc);
} }
#ifdef HAVE_IPV6 #ifdef HAVE_IPV6
if (p->family == AF_INET6) if (p->family == AF_INET6)
{ {
if (! if_is_up (ifp)) if (! if_is_up (ifp))
{ {
/* XXX: See long comment above */ /* See long comment above */
if_set_flags (ifp, IFF_UP | IFF_RUNNING); if_set_flags (ifp, IFF_UP | IFF_RUNNING);
if_refresh (ifp); if_refresh (ifp);
} }
@ -346,13 +350,10 @@ if_addr_wakeup (struct interface *ifp)
safe_strerror(errno)); safe_strerror(errno));
continue; continue;
} }
SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
SET_FLAG (ifc->conf, ZEBRA_IFC_REAL); /* The address will be advertised to zebra clients when the notification
* from the kernel has been received. */
zebra_interface_address_add_update (ifp, ifc);
if (if_is_operative(ifp))
connected_up_ipv6 (ifp, ifc);
} }
#endif /* HAVE_IPV6 */ #endif /* HAVE_IPV6 */
} }
@ -450,9 +451,11 @@ if_delete_update (struct interface *ifp)
ifc = listgetdata (anode); ifc = listgetdata (anode);
p = ifc->address; p = ifc->address;
connected_down_ipv4 (ifp, ifc); connected_down_ipv4 (ifp, ifc);
/* XXX: We have to send notifications here explicitly, because we destroy
* the ifc before receiving the notification about the address being deleted.
*/
zebra_interface_address_delete_update (ifp, ifc); zebra_interface_address_delete_update (ifp, ifc);
UNSET_FLAG (ifc->conf, ZEBRA_IFC_REAL); UNSET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
@ -1251,19 +1254,10 @@ ip_address_install (struct vty *vty, struct interface *ifp,
return CMD_WARNING; return CMD_WARNING;
} }
/* Add to subnet chain list (while marking secondary attribute). */
if_subnet_add (ifp, ifc);
/* IP address propery set. */
SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
SET_FLAG (ifc->conf, ZEBRA_IFC_REAL); /* The address will be advertised to zebra clients when the notification
* from the kernel has been received.
/* Update interface address information to protocol daemon. */ * It will also be added to the subnet chain list, then. */
zebra_interface_address_add_update (ifp, ifc);
/* If interface is up register connected route. */
if (if_is_operative(ifp))
connected_up_ipv4 (ifp, ifc);
} }
return CMD_SUCCESS; return CMD_SUCCESS;
@ -1317,35 +1311,9 @@ ip_address_uninstall (struct vty *vty, struct interface *ifp,
safe_strerror(errno), VTY_NEWLINE); safe_strerror(errno), VTY_NEWLINE);
return CMD_WARNING; return CMD_WARNING;
} }
/* success! call returned that the address deletion went through.
* this is a synchronous operation, so we know it succeeded and can
* now update all internal state. */
/* the HAVE_NETLINK check is only here because, on BSD, although the
* call above is still synchronous, we get a second confirmation later
* through the route socket, and we don't want to touch that behaviour
* for now. It should work without the #ifdef, but why take the risk...
* -- equinox 2012-07-13 */
UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
#ifdef HAVE_NETLINK /* we will receive a kernel notification about this route being removed.
* this will trigger its removal from the connected list. */
/* Remove connected route. */
connected_down_ipv4 (ifp, ifc);
/* Redistribute this information. */
zebra_interface_address_delete_update (ifp, ifc);
/* IP address propery set. */
UNSET_FLAG (ifc->conf, ZEBRA_IFC_REAL);
/* remove from interface, remark secondaries */
if_subnet_delete (ifp, ifc);
/* Free address information. */
listnode_delete (ifp->connected, ifc);
connected_free (ifc);
#endif
return CMD_SUCCESS; return CMD_SUCCESS;
} }
@ -1462,16 +1430,9 @@ ipv6_address_install (struct vty *vty, struct interface *ifp,
return CMD_WARNING; return CMD_WARNING;
} }
/* IP address propery set. */
SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); SET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
SET_FLAG (ifc->conf, ZEBRA_IFC_REAL); /* The address will be advertised to zebra clients when the notification
* from the kernel has been received. */
/* Update interface address information to protocol daemon. */
zebra_interface_address_add_update (ifp, ifc);
/* If interface is up register connected route. */
if (if_is_operative(ifp))
connected_up_ipv6 (ifp, ifc);
} }
return CMD_SUCCESS; return CMD_SUCCESS;
@ -1527,17 +1488,8 @@ ipv6_address_uninstall (struct vty *vty, struct interface *ifp,
} }
UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED); UNSET_FLAG (ifc->conf, ZEBRA_IFC_QUEUED);
/* This information will be propagated to the zclients when the
/* Redistribute this information. */ * kernel notification is received. */
zebra_interface_address_delete_update (ifp, ifc);
/* Remove connected route. */
connected_down_ipv6 (ifp, ifc);
/* Free address information. */
listnode_delete (ifp->connected, ifc);
connected_free (ifc);
return CMD_SUCCESS; return CMD_SUCCESS;
} }