No explanatory message at all? I think this commit needs one.
Calling out to system to run ping to cause ARP to be run makes bug-covering-hack bell ring.
I'm just wondering, would it not suffice to just directly send a unicast packet here from within ospfd? E.g. sending an OSPF Hello unicast should be perfectly safe here and should ARP to be run, if needed.
Dinesh I think indicated before that that caused ANVL to raise conformance issues, but I think OSPF sending a unicast OSPF packet is a more sensible than system()'ing out to call ping.
Also, are we talking regular point-to-point interfaces, is ARP even necessary on PtP? With PPP, peers usually exchange unicast address information with IPCP anyway, and there's no ARP protocol run I thought? So the ping would be a waste of time.
If something weirder: note that setting the interface to NBMA should cause OSPF to go unicast. Would that not work?
Basically, I wonder if this patch perhaps is specific to certain kinds of links, and need at least to be restricted to those. Further, on those links, perhaps it could be done with OSPF packets within ospfd?
On Fri, 13 Nov 2015, Donald Sharp wrote:
From: Ayan Banerjee <[email protected]> Signed-off-by: Ayan Banerjee <[email protected]> Reviewed-by: JR Rivers <[email protected]> Reviewed-by: Shrijeet Mukherjee <[email protected]> --- ospfd/ospf_nsm.c | 25 +++++++++++++++++++++++-- ospfd/ospf_packet.c | 32 ++++++++++++++++++++++++++++++++ ospfd/ospf_packet.h | 2 ++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index 4fecb59..6208912 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -170,6 +170,10 @@ nsm_packet_received (struct ospf_neighbor *nbr) if (nbr->oi->type == OSPF_IFTYPE_NBMA && nbr->nbr_nbma) OSPF_POLL_TIMER_OFF (nbr->nbr_nbma->t_poll); + /* Send proactive ARP requests */ + if (nbr->state < NSM_Exchange) + ospf_proactively_arp (nbr); + return 0; } @@ -184,13 +188,22 @@ nsm_start (struct ospf_neighbor *nbr) OSPF_NSM_TIMER_ON (nbr->t_inactivity, ospf_inactivity_timer, nbr->v_inactivity); + /* Send proactive ARP requests */ + ospf_proactively_arp (nbr); + return 0; } static int nsm_twoway_received (struct ospf_neighbor *nbr) { - return (nsm_should_adj (nbr) ? NSM_ExStart : NSM_TwoWay); + int adj = nsm_should_adj (nbr); + + /* Send proactive ARP requests */ + if (adj) + ospf_proactively_arp (nbr); + + return (adj ? NSM_ExStart : NSM_TwoWay); } int @@ -273,6 +286,9 @@ nsm_negotiation_done (struct ospf_neighbor *nbr) struct ospf_lsa *lsa; struct route_node *rn; + /* Send proactive ARP requests */ + ospf_proactively_arp (nbr); + LSDB_LOOP (ROUTER_LSDB (area), rn, lsa) ospf_db_summary_add (nbr, lsa); LSDB_LOOP (NETWORK_LSDB (area), rn, lsa) @@ -334,7 +350,12 @@ nsm_adj_ok (struct ospf_neighbor *nbr) int adj = nsm_should_adj (nbr); if (nbr->state == NSM_TwoWay && adj == 1) - next_state = NSM_ExStart; + { + next_state = NSM_ExStart; + + /* Send proactive ARP requests */ + ospf_proactively_arp (nbr); + } else if (nbr->state >= NSM_ExStart && adj == 0) next_state = NSM_TwoWay; diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 9a8f1d6..990303b 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -3930,3 +3930,35 @@ ospf_ls_ack_send_delayed (struct ospf_interface *oi) while (listcount (oi->ls_ack)) ospf_ls_ack_send_list (oi, oi->ls_ack, dst); } + +/* + * On pt-to-pt links, all OSPF control packets are sent to the multicast + * address. As a result, the kernel does not need to learn the interface + * MAC of the OSPF neighbor. However, in our world, this will delay + * convergence. Take the case when due to a link flap, all routes now + * want to use an interface which was deemed to be costlier prior to this + * event. For routes that will be installed, the missing MAC will have + * punt-to-CPU set on them. This may overload the CPU control path that + * can be avoided if the MAC was known apriori. + */ +#define OSPF_PING_NBR_STR_MAX (8 + 40 + 20) +void +ospf_proactively_arp (struct ospf_neighbor *nbr) +{ + char ping_nbr[OSPF_PING_NBR_STR_MAX]; + char *str_ptr; + int ret; + + if (!nbr || !nbr->oi || !nbr->oi->ifp || !nbr->oi->ifp->name) + return; + + str_ptr = strcpy (ping_nbr, "ping -c 1 -I "); + str_ptr = strcat (str_ptr, nbr->oi->ifp->name); + str_ptr = strcat (str_ptr, " "); + str_ptr = strcat (str_ptr, inet_ntoa (nbr->address.u.prefix4)); + str_ptr = strcat (str_ptr, " > /dev/null 2>&1 &"); + ret = system (ping_nbr); + if (IS_DEBUG_OSPF_EVENT) + zlog_debug ("Executed %s %s", ping_nbr, + ((ret == 0) ? "successfully" : "but failed")); +} diff --git a/ospfd/ospf_packet.h b/ospfd/ospf_packet.h index 337686a..6076132 100644 --- a/ospfd/ospf_packet.h +++ b/ospfd/ospf_packet.h @@ -174,4 +174,6 @@ extern int ospf_hello_reply_timer (struct thread *); extern const struct message ospf_packet_type_str[]; extern const size_t ospf_packet_type_str_max; +extern void ospf_proactively_arp (struct ospf_neighbor *); + #endif /* _ZEBRA_OSPF_PACKET_H */
-- Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A Fortune: The rich get rich, and the poor get poorer. The haves get more, the have-nots die. _______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
