From 429ac78cc64e0e29bab7cbc00ee991abcdec3f81 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Thu, 15 Jun 2006 18:40:49 +0000 Subject: [ospfd] Fix multicast membership drop bug 2006-06-15 Paul Jakma * Reported by Milan Koci * ospf_interface.h: (struct ospf_if_info) Add reference counts for multicast group memberships. Add various macros to help manipulate/check membership state. * ospf_interface.c: (ospf_if_set_multicast) Maintain the ospf_if_info reference counts, and only actually drop memberships if it hits 0, to avoid losing membership when OSPF is disabled on an interface with multiple active OSPF interfaces. * ospf_packet.c: (ospf_{hello,read}) Use the new macros to check/set multicast membership. * ospf_vty.c: (show_ip_ospf_interface_sub) ditto. --- ospfd/ChangeLog | 15 +++++++++++++++ ospfd/ospf_interface.c | 28 +++++++++++++++++----------- ospfd/ospf_interface.h | 24 ++++++++++++++++++++++-- ospfd/ospf_packet.c | 6 +++--- ospfd/ospf_vty.c | 15 +++++++++------ 5 files changed, 66 insertions(+), 22 deletions(-) diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog index 34d7c4df..b7f2c95f 100644 --- a/ospfd/ChangeLog +++ b/ospfd/ChangeLog @@ -1,3 +1,18 @@ +2006-06-15 Paul Jakma + + * ospf_interface.h: (struct ospf_if_info) Add reference counts + for multicast group memberships. Add various macros to help + manipulate/check membership state. + * ospf_interface.c: (ospf_if_set_multicast) Maintain the + ospf_if_info reference counts, and only actually drop + memberships if it hits 0, to avoid losing membership when + OSPF is disabled on an interface with multiple active OSPF + interfaces. + * ospf_packet.c: (ospf_{hello,read}) Use the new macros to + check/set + multicast membership. + * ospf_vty.c: (show_ip_ospf_interface_sub) ditto. + 2006-05-31 Paul Jakma * ospf_lsdb.c: (ospf_lsdb_delete) robustify against NULL arguments, diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index b94cfa3a..2c2c0749 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -748,22 +748,25 @@ ospf_if_set_multicast(struct ospf_interface *oi) (OSPF_IF_PARAM(oi, passive_interface) == OSPF_IF_ACTIVE)) { /* The interface should belong to the OSPF-all-routers group. */ - if (!CHECK_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS) && + if (!OI_MEMBER_CHECK(oi, MEMBER_ALLROUTERS) && (ospf_if_add_allspfrouters(oi->ospf, oi->address, oi->ifp->ifindex) >= 0)) - /* Set the flag only if the system call to join succeeded. */ - SET_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS); + /* Set the flag only if the system call to join succeeded. */ + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); } else { /* The interface should NOT belong to the OSPF-all-routers group. */ - if (CHECK_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS)) + if (OI_MEMBER_CHECK(oi, MEMBER_ALLROUTERS)) { - ospf_if_drop_allspfrouters (oi->ospf, oi->address, oi->ifp->ifindex); + /* Only actually drop if this is the last reference */ + if (OI_MEMBER_COUNT(oi, MEMBER_ALLROUTERS) == 1) + ospf_if_drop_allspfrouters (oi->ospf, oi->address, + oi->ifp->ifindex); /* Unset the flag regardless of whether the system call to leave the group succeeded, since it's much safer to assume that we are not a member. */ - UNSET_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS); + OI_MEMBER_LEFT(oi,MEMBER_ALLROUTERS); } } @@ -773,22 +776,25 @@ ospf_if_set_multicast(struct ospf_interface *oi) (OSPF_IF_PARAM(oi, passive_interface) == OSPF_IF_ACTIVE)) { /* The interface should belong to the OSPF-designated-routers group. */ - if (!CHECK_FLAG(oi->multicast_memberships, MEMBER_DROUTERS) && + if (!OI_MEMBER_CHECK(oi, MEMBER_DROUTERS) && (ospf_if_add_alldrouters(oi->ospf, oi->address, oi->ifp->ifindex) >= 0)) /* Set the flag only if the system call to join succeeded. */ - SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); + OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); } else { /* The interface should NOT belong to the OSPF-designated-routers group */ - if (CHECK_FLAG(oi->multicast_memberships, MEMBER_DROUTERS)) + if (OI_MEMBER_CHECK(oi, MEMBER_DROUTERS)) { - ospf_if_drop_alldrouters(oi->ospf, oi->address, oi->ifp->ifindex); + /* drop only if last reference */ + if (OI_MEMBER_COUNT(oi, MEMBER_DROUTERS) == 1) + ospf_if_drop_alldrouters(oi->ospf, oi->address, oi->ifp->ifindex); + /* Unset the flag regardless of whether the system call to leave the group succeeded, since it's much safer to assume that we are not a member. */ - UNSET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); + OI_MEMBER_LEFT(oi, MEMBER_DROUTERS); } } } diff --git a/ospfd/ospf_interface.h b/ospfd/ospf_interface.h index 3c759405..5a825ea5 100644 --- a/ospfd/ospf_interface.h +++ b/ospfd/ospf_interface.h @@ -68,11 +68,19 @@ struct ospf_if_params DECLARE_IF_PARAM (int, auth_type); /* OSPF authentication type */ }; +enum +{ + MEMBER_ALLROUTERS = 0, + MEMBER_DROUTERS, + MEMBER_MAX, +}; + struct ospf_if_info { struct ospf_if_params *def_params; struct route_table *params; struct route_table *oifs; + unsigned int membership_counts[MEMBER_MAX]; /* multicast group refcnts */ }; struct ospf_interface; @@ -132,8 +140,20 @@ struct ospf_interface /* To which multicast groups do we currently belong? */ u_char multicast_memberships; -#define MEMBER_ALLROUTERS 0x1 -#define MEMBER_DROUTERS 0x2 +#define OI_MEMBER_FLAG(M) (1 << (M)) +#define OI_MEMBER_COUNT(O,M) (IF_OSPF_IF_INFO(oi->ifp)->membership_counts[(M)]) +#define OI_MEMBER_CHECK(O,M) \ + (CHECK_FLAG((O)->multicast_memberships, OI_MEMBER_FLAG(M))) +#define OI_MEMBER_JOINED(O,M) \ + do { \ + SET_FLAG ((O)->multicast_memberships, OI_MEMBER_FLAG(M)); \ + IF_OSPF_IF_INFO((O)->ifp)->membership_counts[(M)]++; \ + } while (0) +#define OI_MEMBER_LEFT(O,M) \ + do { \ + UNSET_FLAG ((O)->multicast_memberships, OI_MEMBER_FLAG(M)); \ + IF_OSPF_IF_INFO((O)->ifp)->membership_counts[(M)]--; \ + } while (0) struct prefix *address; /* Interface prefix */ struct connected *connected; /* Pointer to connected */ diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index a842ca68..569f2513 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -768,7 +768,7 @@ ospf_hello (struct ip *iph, struct ospf_header *ospfh, if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) { /* Try to fix multicast membership. */ - SET_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS); + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); ospf_if_set_multicast(oi); } return; @@ -2390,9 +2390,9 @@ ospf_read (struct thread *thread) ifp->name, if_flag_dump(ifp->flags)); /* Fix multicast memberships? */ if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) - SET_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS); + OI_MEMBER_JOINED(oi, MEMBER_ALLROUTERS); else if (iph->ip_dst.s_addr == htonl(OSPF_ALLDROUTERS)) - SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); + OI_MEMBER_JOINED(oi, MEMBER_DROUTERS); if (oi->multicast_memberships) ospf_if_set_multicast(oi); return 0; diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 8d6ff31f..10580ab7 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -2811,12 +2811,15 @@ show_ip_ospf_interface_sub (struct vty *vty, struct ospf *ospf, } vty_out (vty, " Multicast group memberships:"); - if (CHECK_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS)) - vty_out (vty, " OSPFAllRouters"); - if (CHECK_FLAG(oi->multicast_memberships, MEMBER_DROUTERS)) - vty_out (vty, " OSPFDesignatedRouters"); - if (!CHECK_FLAG(oi->multicast_memberships, - MEMBER_ALLROUTERS|MEMBER_DROUTERS)) + if (OI_MEMBER_CHECK(oi, MEMBER_ALLROUTERS) + || OI_MEMBER_CHECK(oi, MEMBER_DROUTERS)) + { + if (OI_MEMBER_CHECK(oi, MEMBER_ALLROUTERS)) + vty_out (vty, " OSPFAllRouters"); + if (OI_MEMBER_CHECK(oi, MEMBER_DROUTERS)) + vty_out (vty, " OSPFDesignatedRouters"); + } + else vty_out (vty, " "); vty_out (vty, "%s", VTY_NEWLINE); -- cgit v1.2.1