From d2fc88962a9a494ecb34167871bb9e7273a25d33 Mon Sep 17 00:00:00 2001 From: ajs Date: Sat, 2 Apr 2005 18:38:43 +0000 Subject: 2005-04-02 Andrew J. Schorr Fix problems when netlink interfaces are renamed (same ifindex used for a new interface). Start cleaning up some problems with the way interface names are handled. * interface.c: (if_new_intern_ifindex) Remove obsolete function. (if_delete_update) After distributing the interface deletion message, set ifp->ifindex to IFINDEX_INTERNAL. (if_dump_vty) Detect pseudo interface by checking if ifp->ifindex is IFINDEX_INTERNAL. (zebra_interface) Check return code from interface_cmd.func. Do not set internal ifindex values to if_new_intern_ifindex(), since we now use IFINDEX_INTERNAL for all pseudo interfaces. * kernel_socket.c: (ifm_read) Fix code and comments to reflect that all internal interfaces now have ifp->ifindex set to IFINDEX_INTERNAL. * rt_netlink.c: (set_ifindex) New function used to update ifp->ifindex. Detects interface rename events by checking if that ifindex is already being used. If it is, delete the old interface before assigning the ifindex to the new interface. (netlink_interface, netlink_link_change) Call set_ifindex to update the ifindex. * if.h: Remove define for IFINDEX_INTERNBASE and add define IFINDEX_INTERNAL 0, since all internal (i.e. non-kernel) pseudo- interfaces should have ifindex set to 0. (if_new) Remove function. (if_delete_retain) New function to delete an interface without removing from iflist and freeing the structure. (ifname2ifindex) New function. * if.c: (if_new) Remove function (absorb into if_create). (if_create) Replace function if_new with call to calloc. Set ifp->ifindex to IFINDEX_INTERNAL. Fix off-by-one error in assert to check length of interface name. Add error message if interface with this name already exists. (if_delete_retain) New function to delete an interface without removing from iflist and freeing the structure. (if_delete) Implement with help of if_delete_retain. (ifindex2ifname) Reimplement using if_lookup_by_index. (ifname2ifindex) New function to complement ifindex2ifname. (interface) The interface command should check the name length and fail with a warning message if it is too long. (no_interface) Fix spelling in warning message. (if_nametoindex) Reimplement using if_lookup_by_name. (if_indextoname, ifaddr_ipv4_lookup) Reimplement using if_lookup_by_index. * bgp_zebra.c: (bgp_interface_delete) After deleting, set ifp->ifindex to IFINDEX_INTERNAL. * isis_zebra.c: (isis_zebra_if_del) Call if_delete_retain instead of if_delete, since it is generally not safe to remove interface structures. After deleting, set ifp->ifindex to IFINDEX_INTERNAL. (zebra_interface_if_lookup) Tighten up code. * ospf6_zebra.c: (ospf6_zebra_if_del) Previously, this whole function was commented out. But this is not safe: we should at least update the ifindex when the interface is deleted. So the new version updates the interface status and sets ifp->ifindex to IFINDEX_INTERNAL. (ospf6_zebra_route_update) Use if_indextoname properly. * ospf_vty.c: (show_ip_ospf_interface_sub) Show ifindex and interface flags to help with debugging. * ospf_zebra.c: (ospf_interface_delete) After deleting, set ifp->ifindex to IFINDEX_INTERNAL. (zebra_interface_if_lookup) Make function static. Tighten up code. * rip_interface.c: (rip_interface_delete) After deleting, set ifp->ifindex to IFINDEX_INTERNAL. * ripng_interface.c: (ripng_interface_delete) After deleting, set ifp->ifindex to IFINDEX_INTERNAL. --- bgpd/ChangeLog | 5 ++ bgpd/bgp_zebra.c | 1 + isisd/ChangeLog | 7 +++ isisd/isis_zebra.c | 18 ++++---- lib/ChangeLog | 26 +++++++++++ lib/if.c | 117 +++++++++++++++++++++-------------------------- lib/if.h | 39 ++++++++++++---- ospf6d/ChangeLog | 9 ++++ ospf6d/ospf6_zebra.c | 28 ++++++++---- ospfd/ChangeLog | 8 ++++ ospfd/ospf_vty.c | 5 +- ospfd/ospf_zebra.c | 14 ++---- ripd/ChangeLog | 5 ++ ripd/rip_interface.c | 1 + ripngd/ChangeLog | 5 ++ ripngd/ripng_interface.c | 1 + zebra/ChangeLog | 19 ++++++++ zebra/interface.c | 45 ++++++------------ zebra/kernel_socket.c | 14 ++---- zebra/rt_netlink.c | 34 ++++++++++++-- 20 files changed, 254 insertions(+), 147 deletions(-) diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog index d107fa6e..4c18fc39 100644 --- a/bgpd/ChangeLog +++ b/bgpd/ChangeLog @@ -1,3 +1,8 @@ +2005-04-02 Andrew J. Schorr + + * bgp_zebra.c: (bgp_interface_delete) After deleting, set ifp->ifindex + to IFINDEX_INTERNAL. + 2005-03-21 Hasso Tepper * bgp_route.c: Don't crash while clearing route tables if there is diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index f54608a3..176e447a 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -81,6 +81,7 @@ bgp_interface_delete (int command, struct zclient *zclient, s = zclient->ibuf; ifp = zebra_interface_state_read (s); + ifp->ifindex = IFINDEX_INTERNAL; return 0; } diff --git a/isisd/ChangeLog b/isisd/ChangeLog index d7fadd0c..6f4f3958 100644 --- a/isisd/ChangeLog +++ b/isisd/ChangeLog @@ -1,3 +1,10 @@ +2005-04-02 Andrew J. Schorr + + * isis_zebra.c: (isis_zebra_if_del) Call if_delete_retain instead + of if_delete, since it is generally not safe to remove interface + structures. After deleting, set ifp->ifindex to IFINDEX_INTERNAL. + (zebra_interface_if_lookup) Tighten up code. + 2005-03-07 Michael Sandee * isis_spf.c: host.name might be NULL. diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c index 40743b29..0f8d1162 100644 --- a/isisd/isis_zebra.c +++ b/isisd/isis_zebra.c @@ -95,30 +95,28 @@ isis_zebra_if_del (int command, struct zclient *zclient, zebra_size_t length) zlog_debug ("Zebra I/F delete: %s index %d flags %ld metric %d mtu %d", ifp->name, ifp->ifindex, ifp->flags, ifp->metric, ifp->mtu); - if_delete (ifp); + + /* Cannot call if_delete because we should retain the pseudo interface + in case there is configuration info attached to it. */ + if_delete_retain(ifp); isis_csm_state_change (IF_DOWN_FROM_Z, circuit_scan_by_ifp (ifp), ifp); + ifp->ifindex = IFINDEX_INTERNAL; + return 0; } static struct interface * zebra_interface_if_lookup (struct stream *s) { - struct interface *ifp; u_char ifname_tmp[INTERFACE_NAMSIZ]; /* Read interface name. */ stream_get (ifname_tmp, s, INTERFACE_NAMSIZ); - /* Lookup this by interface index. */ - ifp = if_lookup_by_name ((char *) ifname_tmp); - - /* If such interface does not exist, indicate an error */ - if (!ifp) - return NULL; - - return ifp; + /* And look it up. */ + return if_lookup_by_name ((char *) ifname_tmp); } static int diff --git a/lib/ChangeLog b/lib/ChangeLog index 567f603a..8aaf3829 100644 --- a/lib/ChangeLog +++ b/lib/ChangeLog @@ -1,3 +1,29 @@ +2005-04-02 Andrew J. Schorr + + * if.h: Remove define for IFINDEX_INTERNBASE and add define + IFINDEX_INTERNAL 0, since all internal (i.e. non-kernel) pseudo- + interfaces should have ifindex set to 0. + (if_new) Remove function. + (if_delete_retain) New function to delete an interface without + removing from iflist and freeing the structure. + (ifname2ifindex) New function. + * if.c: (if_new) Remove function (absorb into if_create). + (if_create) Replace function if_new with call to calloc. + Set ifp->ifindex to IFINDEX_INTERNAL. Fix off-by-one error + in assert to check length of interface name. Add error message + if interface with this name already exists. + (if_delete_retain) New function to delete an interface without + removing from iflist and freeing the structure. + (if_delete) Implement with help of if_delete_retain. + (ifindex2ifname) Reimplement using if_lookup_by_index. + (ifname2ifindex) New function to complement ifindex2ifname. + (interface) The interface command should check the name length + and fail with a warning message if it is too long. + (no_interface) Fix spelling in warning message. + (if_nametoindex) Reimplement using if_lookup_by_name. + (if_indextoname, ifaddr_ipv4_lookup) Reimplement using + if_lookup_by_index. + 2005-04-02 Andrew J. Schorr * zebra.h: Should include str.h to pick up missing functions. diff --git a/lib/if.c b/lib/if.c index 7385ff6e..e35e3ed2 100644 --- a/lib/if.c +++ b/lib/if.c @@ -112,29 +112,23 @@ if_cmp_func (struct interface *ifp1, struct interface *ifp2) } /* Create new interface structure. */ -struct interface * -if_new () -{ - struct interface *ifp; - - ifp = XMALLOC (MTYPE_IF, sizeof (struct interface)); - memset (ifp, 0, sizeof (struct interface)); - return ifp; -} - struct interface * if_create (const char *name, int namelen) { struct interface *ifp; - ifp = if_new (); + ifp = XCALLOC (MTYPE_IF, sizeof (struct interface)); + ifp->ifindex = IFINDEX_INTERNAL; assert (name); - assert (namelen <= (INTERFACE_NAMSIZ + 1)); + assert (namelen <= INTERFACE_NAMSIZ); /* Need space for '\0' at end. */ strncpy (ifp->name, name, namelen); - ifp->name[INTERFACE_NAMSIZ] = '\0'; + ifp->name[namelen] = '\0'; if (if_lookup_by_name(ifp->name) == NULL) listnode_add_sort (iflist, ifp); + else + zlog_err("if_create(%s): corruption detected -- interface with this " + "name exists already!", ifp->name); ifp->connected = list_new (); ifp->connected->del = (void (*) (void *)) connected_free; @@ -144,17 +138,24 @@ if_create (const char *name, int namelen) return ifp; } -/* Delete and free interface structure. */ +/* Delete interface structure. */ void -if_delete (struct interface *ifp) +if_delete_retain (struct interface *ifp) { - listnode_delete (iflist, ifp); - if (if_master.if_delete_hook) (*if_master.if_delete_hook) (ifp); /* Free connected address list */ list_delete (ifp->connected); +} + +/* Delete and free interface structure. */ +void +if_delete (struct interface *ifp) +{ + listnode_delete (iflist, ifp); + + if_delete_retain(ifp); XFREE (MTYPE_IF, ifp); } @@ -194,16 +195,18 @@ if_lookup_by_index (unsigned int index) char * ifindex2ifname (unsigned int index) { - struct listnode *node; struct interface *ifp; - for (node = listhead (iflist); node; nextnode (node)) - { - ifp = getdata (node); - if (ifp->ifindex == index) - return ifp->name; - } - return (char *) "unknown"; + return ((ifp = if_lookup_by_index(index)) != NULL) ? + ifp->name : (char *)"unknown"; +} + +unsigned int +ifname2ifindex (const char *name) +{ + struct interface *ifp; + + return ((ifp = if_lookup_by_name(name)) != NULL) ? ifp->ifindex : 0; } /* Interface existance check by interface name. */ @@ -491,11 +494,20 @@ DEFUN (interface, "Interface's name\n") { struct interface *ifp; + size_t sl; + + if ((sl = strlen(argv[0])) > INTERFACE_NAMSIZ) + { + vty_out (vty, "%% Interface name %s is invalid: length exceeds " + "%d characters%s", + argv[0], INTERFACE_NAMSIZ, VTY_NEWLINE); + return CMD_WARNING; + } ifp = if_lookup_by_name (argv[0]); if (ifp == NULL) - ifp = if_create (argv[0], INTERFACE_NAMSIZ); + ifp = if_create (argv[0], sl); vty->index = ifp; vty->node = INTERFACE_NODE; @@ -515,17 +527,17 @@ DEFUN_NOSH (no_interface, ifp = if_lookup_by_name (argv[0]); if (ifp == NULL) - { - vty_out (vty, "%% Inteface %s does not exist%s", argv[0], VTY_NEWLINE); - return CMD_WARNING; - } + { + vty_out (vty, "%% Interface %s does not exist%s", argv[0], VTY_NEWLINE); + return CMD_WARNING; + } if (CHECK_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE)) - { - vty_out (vty, "%% Only inactive interfaces can be deleted%s", - VTY_NEWLINE); - return CMD_WARNING; - } + { + vty_out (vty, "%% Only inactive interfaces can be deleted%s", + VTY_NEWLINE); + return CMD_WARNING; + } if_delete(ifp); @@ -726,16 +738,9 @@ connected_add_by_prefix (struct interface *ifp, struct prefix *p, unsigned int if_nametoindex (const char *name) { - struct listnode *node; struct interface *ifp; - for (node = listhead (iflist); node; nextnode (node)) - { - ifp = getdata (node); - if (strcmp (ifp->name, name) == 0) - return ifp->ifindex; - } - return 0; + return ((ifp = if_lookup_by_name(name)) != NULL) ? ifp->ifindex : 0; } #endif @@ -743,19 +748,12 @@ if_nametoindex (const char *name) char * if_indextoname (unsigned int ifindex, char *name) { - struct listnode *node; struct interface *ifp; - for (node = listhead (iflist); node; nextnode (node)) - { - ifp = getdata (node); - if (ifp->ifindex == ifindex) - { - memcpy (name, ifp->name, IFNAMSIZ); - return ifp->name; - } - } - return NULL; + if (!(ifp = if_lookup_by_index(ifindex))) + return NULL; + strncpy (name, ifp->name, IFNAMSIZ); + return ifp->name; } #endif @@ -832,16 +830,7 @@ ifaddr_ipv4_lookup (struct in_addr *addr, unsigned int ifindex) return ifp; } else - { - for (node = listhead (iflist); node; nextnode (node)) - { - ifp = getdata (node); - - if (ifp->ifindex == ifindex) - return ifp; - } - } - return NULL; + return if_lookup_by_index(ifindex); } /* Initialize interface list. */ diff --git a/lib/if.h b/lib/if.h index f5efda6e..df9ff605 100644 --- a/lib/if.h +++ b/lib/if.h @@ -36,10 +36,6 @@ Boston, MA 02111-1307, USA. */ #define INTERFACE_NAMSIZ 20 #define INTERFACE_HWADDR_MAX 20 -/* Internal If indexes start at 0xFFFFFFFF and go down to 1 greater - than this */ -#define IFINDEX_INTERNBASE 0x80000000 - #ifdef HAVE_PROC_NET_DEV struct if_stats { @@ -75,11 +71,19 @@ struct if_stats /* Interface structure */ struct interface { - /* Interface name. */ + /* Interface name. This should probably never be changed after the + interface is created, because the configuration info for this interface + is associated with this structure. For that reason, the interface + should also never be deleted (to avoid losing configuration info). + To delete, just set ifindex to IFINDEX_INTERNAL to indicate that the + interface does not exist in the kernel. + */ char name[INTERFACE_NAMSIZ + 1]; - /* Interface index. */ + /* Interface index (should be IFINDEX_INTERNAL for non-kernel or + deleted interfaces). */ unsigned int ifindex; +#define IFINDEX_INTERNAL 0 /* Zebra internal interface status */ u_char status; @@ -208,14 +212,22 @@ struct connected /* Prototypes. */ int if_cmp_func (struct interface *, struct interface *); -struct interface *if_new (void); struct interface *if_create (const char *name, int namelen); struct interface *if_lookup_by_index (unsigned int); struct interface *if_lookup_by_name (const char *); struct interface *if_lookup_exact_address (struct in_addr); struct interface *if_lookup_address (struct in_addr); struct interface *if_get_by_name (const char *); -void if_delete (struct interface *); + +/* Delete the interface, but do not free the structure, and leave it in the + interface list. It is often advisable to leave the pseudo interface + structure because there may be configuration information attached. */ +extern void if_delete_retain (struct interface *); + +/* Delete and free the interface structure: calls if_delete_retain and then + deletes it from the interface list and frees the structure. */ +extern void if_delete (struct interface *); + int if_is_up (struct interface *); int if_is_running (struct interface *); int if_is_operative (struct interface *); @@ -226,9 +238,18 @@ int if_is_multicast (struct interface *); void if_add_hook (int, int (*)(struct interface *)); void if_init (); void if_dump_all (); -char *ifindex2ifname (unsigned int); extern const char *if_flag_dump(unsigned long); +/* Please use ifindex2ifname instead of if_indextoname where possible; + ifindex2ifname uses internal interface info, whereas if_indextoname must + make a system call. */ +extern char *ifindex2ifname (unsigned int); + +/* Please use ifname2ifindex instead of if_nametoindex where possible; + ifname2ifindex uses internal interface info, whereas if_nametoindex must + make a system call. */ +extern unsigned int ifname2ifindex(const char *ifname); + /* Connected address functions. */ struct connected *connected_new (); void connected_free (struct connected *); diff --git a/ospf6d/ChangeLog b/ospf6d/ChangeLog index e0a5775b..61a2e463 100644 --- a/ospf6d/ChangeLog +++ b/ospf6d/ChangeLog @@ -1,3 +1,12 @@ +2005-04-02 Andrew J. Schorr + + * ospf6_zebra.c: (ospf6_zebra_if_del) Previously, this whole function + was commented out. But this is not safe: we should at least update + the ifindex when the interface is deleted. So the new version + updates the interface status and sets ifp->ifindex to + IFINDEX_INTERNAL. + (ospf6_zebra_route_update) Use if_indextoname properly. + 2005-04-02 Andrew J. Schorr * ospf6_route.c: (ospf6_route_show, ospf6_route_show_detail) Protect diff --git a/ospf6d/ospf6_zebra.c b/ospf6d/ospf6_zebra.c index bc15dff6..1ce36298 100644 --- a/ospf6d/ospf6_zebra.c +++ b/ospf6d/ospf6_zebra.c @@ -104,16 +104,24 @@ ospf6_zebra_if_add (int command, struct zclient *zclient, zebra_size_t length) int ospf6_zebra_if_del (int command, struct zclient *zclient, zebra_size_t length) { -#if 0 struct interface *ifp; - ifp = zebra_interface_delete_read (zclient->ibuf); + if (!(ifp = zebra_interface_state_read(zclient->ibuf))) + return 0; + + if (if_is_up (ifp)) + zlog_warn ("Zebra: got delete of %s, but interface is still up", ifp->name); + if (IS_OSPF6_DEBUG_ZEBRA (RECV)) zlog_debug ("Zebra Interface delete: %s index %d mtu %d", ifp->name, ifp->ifindex, ifp->mtu6); +#if 0 + /* Why is this commented out? */ ospf6_interface_if_del (ifp); #endif /*0*/ + + ifp->ifindex = IFINDEX_INTERNAL; return 0; } @@ -348,7 +356,7 @@ static void ospf6_zebra_route_update (int type, struct ospf6_route *request) { struct zapi_ipv6 api; - char buf[64], ifname[IFNAMSIZ]; + char buf[64]; int nhcount; struct in6_addr **nexthops; unsigned int *ifindexes; @@ -432,13 +440,15 @@ ospf6_zebra_route_update (int type, struct ospf6_route *request) for (i = 0; i < nhcount; i++) { if (IS_OSPF6_DEBUG_ZEBRA (SEND)) - { - inet_ntop (AF_INET6, &request->nexthop[i].address, - buf, sizeof (buf)); - if_indextoname (request->nexthop[i].ifindex, ifname); - zlog_debug (" nexthop: %s%%%s(%d)", buf, ifname, + { + char ifname[IFNAMSIZ]; + inet_ntop (AF_INET6, &request->nexthop[i].address, + buf, sizeof (buf)); + if (!if_indextoname(request->nexthop[i].ifindex, ifname)) + strlcpy(ifname, "unknown", sizeof(ifname)); + zlog_debug (" nexthop: %s%%%.*s(%d)", buf, IFNAMSIZ, ifname, request->nexthop[i].ifindex); - } + } nexthops[i] = &request->nexthop[i].address; ifindexes[i] = request->nexthop[i].ifindex; } diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog index 8facd560..b1d6bbec 100644 --- a/ospfd/ChangeLog +++ b/ospfd/ChangeLog @@ -1,3 +1,11 @@ +2005-04-02 Andrew J. Schorr + + * ospf_vty.c: (show_ip_ospf_interface_sub) Show ifindex and interface + flags to help with debugging. + * ospf_zebra.c: (ospf_interface_delete) After deleting, set ifp->ifindex + to IFINDEX_INTERNAL. + (zebra_interface_if_lookup) Make function static. Tighten up code. + 2005-03-31 Andrew J. Schorr * ospf_dump.c: (show_debugging_ospf) Show if ospf event debugging diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 1d2550d3..c0e1f6ce 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -2582,8 +2582,9 @@ show_ip_ospf_interface_sub (struct vty *vty, struct ospf *ospf, /* Is interface up? */ vty_out (vty, "%s is %s%s", ifp->name, ((is_up = if_is_operative(ifp)) ? "up" : "down"), VTY_NEWLINE); - vty_out (vty, " MTU %u bytes, BW %u Kbit%s", - ifp->mtu, ifp->bandwidth, VTY_NEWLINE); + vty_out (vty, " ifindex %u, MTU %u bytes, BW %u Kbit %s%s", + ifp->ifindex, ifp->mtu, ifp->bandwidth, if_flag_dump(ifp->flags), + VTY_NEWLINE); /* Is interface OSPF enabled? */ if (ospf_oi_count(ifp) == 0) diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c index 300103e1..74936df9 100644 --- a/ospfd/ospf_zebra.c +++ b/ospfd/ospf_zebra.c @@ -141,26 +141,20 @@ ospf_interface_delete (int command, struct zclient *zclient, if (rn->info) ospf_if_free ((struct ospf_interface *) rn->info); + ifp->ifindex = IFINDEX_INTERNAL; return 0; } -struct interface * +static struct interface * zebra_interface_if_lookup (struct stream *s) { - struct interface *ifp; u_char ifname_tmp[INTERFACE_NAMSIZ]; /* Read interface name. */ stream_get (ifname_tmp, s, INTERFACE_NAMSIZ); - /* Lookup this by interface index. */ - ifp = if_lookup_by_name ((char *) ifname_tmp); - - /* If such interface does not exist, indicate an error */ - if (!ifp) - return NULL; - - return ifp; + /* And look it up. */ + return if_lookup_by_name ((char *) ifname_tmp); } int diff --git a/ripd/ChangeLog b/ripd/ChangeLog index edb504aa..132fd43a 100644 --- a/ripd/ChangeLog +++ b/ripd/ChangeLog @@ -1,3 +1,8 @@ +2005-04-02 Andrew J. Schorr + + * rip_interface.c: (rip_interface_delete) After deleting, set + ifp->ifindex to IFINDEX_INTERNAL. + 2005-02-04 Paul Jakma * ripd.c: Untangle the construction of RIP auth data. diff --git a/ripd/rip_interface.c b/ripd/rip_interface.c index 0a01d8b5..f58fb337 100644 --- a/ripd/rip_interface.c +++ b/ripd/rip_interface.c @@ -598,6 +598,7 @@ rip_interface_delete (int command, struct zclient *zclient, /* To support pseudo interface do not free interface structure. */ /* if_delete(ifp); */ + ifp->ifindex = IFINDEX_INTERNAL; return 0; } diff --git a/ripngd/ChangeLog b/ripngd/ChangeLog index f4b61132..7c6176c5 100644 --- a/ripngd/ChangeLog +++ b/ripngd/ChangeLog @@ -1,3 +1,8 @@ +2005-04-02 Andrew J. Schorr + + * ripng_interface.c: (ripng_interface_delete) After deleting, set + ifp->ifindex to IFINDEX_INTERNAL. + 2005-01-30 Andrew J. Schorr * ripng_interface.c: (ripng_multicast_join) Save errno before calling diff --git a/ripngd/ripng_interface.c b/ripngd/ripng_interface.c index 5d317ee0..477bfd7f 100644 --- a/ripngd/ripng_interface.c +++ b/ripngd/ripng_interface.c @@ -331,6 +331,7 @@ ripng_interface_delete (int command, struct zclient *zclient, /* To support pseudo interface do not free interface structure. */ /* if_delete(ifp); */ + ifp->ifindex = IFINDEX_INTERNAL; return 0; } diff --git a/zebra/ChangeLog b/zebra/ChangeLog index 150c08a3..572d2687 100644 --- a/zebra/ChangeLog +++ b/zebra/ChangeLog @@ -1,3 +1,22 @@ +2005-04-02 Andrew J. Schorr + + * interface.c: (if_new_intern_ifindex) Remove obsolete function. + (if_delete_update) After distributing the interface deletion message, + set ifp->ifindex to IFINDEX_INTERNAL. + (if_dump_vty) Detect pseudo interface by checking if ifp->ifindex is + IFINDEX_INTERNAL. + (zebra_interface) Check return code from interface_cmd.func. + Do not set internal ifindex values to if_new_intern_ifindex(), + since we now use IFINDEX_INTERNAL for all pseudo interfaces. + * kernel_socket.c: (ifm_read) Fix code and comments to reflect that + all internal interfaces now have ifp->ifindex set to IFINDEX_INTERNAL. + * rt_netlink.c: (set_ifindex) New function used to update ifp->ifindex. + Detects interface rename events by checking if that ifindex is already + being used. If it is, delete the old interface before assigning + the ifindex to the new interface. + (netlink_interface, netlink_link_change) Call set_ifindex to update + the ifindex. + 2005-03-31 Hasso Tepper * rt_netlink.c (netlink_talk_filter): Show always warning message, diff --git a/zebra/interface.c b/zebra/interface.c index 55717545..0f294a56 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -40,27 +40,7 @@ #include "zebra/redistribute.h" #include "zebra/debug.h" #include "zebra/irdp.h" - -/* Allocate a new internal interface index - * This works done from the top so that %d macros - * print a - sign! - */ -static unsigned int -if_new_intern_ifindex (void) -{ - /* Start here so that first one assigned is 0xFFFFFFFF */ - static unsigned int ifindex = IFINDEX_INTERNBASE + 1; - - for (;;) - { - ifindex--; - if ( ifindex <= IFINDEX_INTERNBASE ) - ifindex = 0xFFFFFFFF; - if (if_lookup_by_index(ifindex) == NULL) - return ifindex; - } -} /* Called when new interface is added. */ int @@ -418,6 +398,13 @@ if_delete_update (struct interface *ifp) } } zebra_interface_delete_update (ifp); + + /* Update ifindex after distributing the delete message. This is in + case any client needs to have the old value of ifindex available + while processing the deletion. Each client daemon is responsible + for setting ifindex to IFINDEX_INTERNAL after processing the + interface deletion message. */ + ifp->ifindex = IFINDEX_INTERNAL; } #endif /* (defined(RTM_IFANNOUNCE) || defined(HAVE_NETLINK) */ @@ -680,9 +667,9 @@ if_dump_vty (struct vty *vty, struct interface *ifp) if (ifp->desc) vty_out (vty, " Description: %s%s", ifp->desc, VTY_NEWLINE); - if (ifp->ifindex <= 0) + if (ifp->ifindex == IFINDEX_INTERNAL) { - vty_out(vty, " index %d pseudo interface%s", ifp->ifindex, VTY_NEWLINE); + vty_out(vty, " pseudo interface%s", VTY_NEWLINE); return; } else if (! CHECK_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE)) @@ -861,17 +848,15 @@ DEFUN_NOSH (zebra_interface, struct interface * ifp; /* Call lib interface() */ - ret = interface_cmd.func (self, vty, argc, argv); + if ((ret = interface_cmd.func (self, vty, argc, argv)) != CMD_SUCCESS) + return ret; ifp = vty->index; - /* Set ifindex - this only happens if interface is NOT in kernel */ - if (ifp->ifindex == 0) - { - ifp->ifindex = if_new_intern_ifindex (); - UNSET_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE); - } + if (ifp->ifindex == IFINDEX_INTERNAL) + /* Is this really necessary? Shouldn't status be initialized to 0 + in that case? */ + UNSET_FLAG (ifp->status, ZEBRA_INTERFACE_ACTIVE); return ret; } diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index e6e74449..cdc6822c 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -301,8 +301,8 @@ ifm_read (struct if_msghdr *ifm) * If lookup by index was unsuccessful and we have a name, try * looking up by name. Interfaces specified in the configuration * file for which the ifindex has not been determined will have - * ifindex == -1, and such interfaces are found by this search, and - * then their ifindex values can be filled in. + * ifindex == IFINDEX_INTERNAL, and such interfaces are found by this search, + * and then their ifindex values can be filled in. */ if (ifp == NULL && sdl != NULL) { @@ -322,14 +322,10 @@ ifm_read (struct if_msghdr *ifm) } /* - * If ifp does not exist or has an invalid index (-1), create or + * If ifp does not exist or has an invalid index (IFINDEX_INTERNAL), create or * fill in an interface. */ - /* - * XXX warning: comparison between signed and unsigned - * ifindex should probably be signed - */ - if ((ifp == NULL) || (ifp->ifindex == -1)) + if ((ifp == NULL) || (ifp->ifindex == IFINDEX_INTERNAL)) { /* * To create or fill in an interface, a sockaddr_dl (via @@ -348,7 +344,7 @@ ifm_read (struct if_msghdr *ifm) /* * Fill in newly created interface structure, or larval - * structure with ifindex -1. + * structure with ifindex IFINDEX_INTERNAL. */ ifp->ifindex = ifm->ifm_index; ifp->flags = ifm->ifm_flags; diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index 042ee331..b3b2aab5 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -86,6 +86,33 @@ extern struct zebra_privs_t zserv_privs; extern u_int32_t nl_rcvbufsize; +/* Note: on netlink systems, there should be a 1-to-1 mapping between interface + names and ifindex values. */ +static void +set_ifindex(struct interface *ifp, unsigned int ifi_index) +{ + struct interface *oifp; + + if (((oifp = if_lookup_by_index(ifi_index)) != NULL) && (oifp != ifp)) + { + if (ifi_index == IFINDEX_INTERNAL) + zlog_err("Netlink is setting interface %s ifindex to reserved " + "internal value %u", ifp->name, ifi_index); + else + { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("interface index %d was renamed from %s to %s", + ifi_index, oifp->name, ifp->name); + if (if_is_up(oifp)) + zlog_err("interface rename detected on up interface: index %d " + "was renamed from %s to %s, results are uncertain!", + ifi_index, oifp->name, ifp->name); + if_delete_update(oifp); + } + } + ifp->ifindex = ifi_index; +} + /* Make socket for Linux netlink interface. */ static int netlink_socket (struct nlsock *nl, unsigned long groups) @@ -504,8 +531,7 @@ netlink_interface (struct sockaddr_nl *snl, struct nlmsghdr *h) /* Add interface. */ ifp = if_get_by_name (name); - - ifp->ifindex = ifi->ifi_index; + set_ifindex(ifp, ifi->ifi_index); ifp->flags = ifi->ifi_flags & 0x0000fffff; ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]); ifp->metric = 1; @@ -981,7 +1007,7 @@ netlink_link_change (struct sockaddr_nl *snl, struct nlmsghdr *h) if (ifp == NULL) ifp = if_get_by_name (name); - ifp->ifindex = ifi->ifi_index; + set_ifindex(ifp, ifi->ifi_index); ifp->flags = ifi->ifi_flags & 0x0000fffff; ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]); ifp->metric = 1; @@ -992,7 +1018,7 @@ netlink_link_change (struct sockaddr_nl *snl, struct nlmsghdr *h) else { /* Interface status change. */ - ifp->ifindex = ifi->ifi_index; + set_ifindex(ifp, ifi->ifi_index); ifp->mtu6 = ifp->mtu = *(int *) RTA_DATA (tb[IFLA_MTU]); ifp->metric = 1; -- cgit v1.2.1