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. --- lib/ChangeLog | 26 +++++++++++++ lib/if.c | 117 ++++++++++++++++++++++++++-------------------------------- lib/if.h | 39 +++++++++++++++----- 3 files changed, 109 insertions(+), 73 deletions(-) (limited to 'lib') 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 *); -- cgit v1.2.1