Hello, Anybody have objections to the patches posted here? If not, I'll push them upstream.
Cheers, Dejan On Wed, Mar 13, 2013 at 07:01:33PM +0100, Dejan Muhamedagic wrote: > Hi, > > On Sat, Mar 09, 2013 at 07:53:34PM +0000, Tim Small wrote: > > Hi, > > > > I've been using the ocf:heartbeat:SendArp script and notice a couple of > > issues - some problems with starting and monitoring the service, and > > also a file descriptor leak in the binary (which would cause it to > > terminate). > > > > I've detailed the problems and supplied some patches: > > > > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=701913 > > Cannot just replace the whole RA. Sorry. If you could split the > patch we can consider them on a one-by-one basis. Otherwise, I > found some patch in my local queue, which never got pushed for > some reason. Don't know if that would help (attached). > > > and > > > > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=701914 > > Can you try the attached send_arp.libnet.c patch. It does first > packet build then reuses them. > > Cheers, > > Dejan > > > ... they're not perfect, but an improvement I think. > > > > HTH, > > > > Tim. > > > > -- > > South East Open Source Solutions Limited > > Registered in England and Wales with company number 06134732. > > Registered Office: 2 Powell Gardens, Redhill, Surrey, RH1 1TQ > > VAT number: 900 6633 53 http://seoss.co.uk/ +44-(0)1273-808309 > > > > > > _______________________________________________ > > Pacemaker mailing list: [email protected] > > http://oss.clusterlabs.org/mailman/listinfo/pacemaker > > > > Project Home: http://www.clusterlabs.org > > Getting started: http://www.clusterlabs.org/doc/Cluster_from_Scratch.pdf > > Bugs: http://bugs.clusterlabs.org > From 9dae34616ef62b98b762a1f821f9e1ee749e6315 Mon Sep 17 00:00:00 2001 > From: Dejan Muhamedagic <[email protected]> > Date: Wed, 13 Mar 2013 18:19:10 +0100 > Subject: [PATCH] Medium: tools: send_arp.libnet: reuse ARP packets > > --- > tools/send_arp.libnet.c | 174 > ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 115 insertions(+), 59 deletions(-) > > diff --git a/tools/send_arp.libnet.c b/tools/send_arp.libnet.c > index 71148bb..2abeecb 100644 > --- a/tools/send_arp.libnet.c > +++ b/tools/send_arp.libnet.c > @@ -49,17 +49,18 @@ > > #ifdef HAVE_LIBNET_1_0_API > # define LTYPE struct libnet_link_int > + static u_char *mk_packet(u_int32_t ip, u_char *device, u_char *macaddr, > u_char *broadcast, u_char *netmask, u_short arptype); > + static int send_arp(struct libnet_link_int *l, u_char *device, u_char > *buf); > #endif > #ifdef HAVE_LIBNET_1_1_API > # define LTYPE libnet_t > + static libnet_t *mk_packet(libnet_t* lntag, u_int32_t ip, u_char > *device, u_char macaddr[6], u_char *broadcast, u_char *netmask, u_short > arptype); > + int send_arp(libnet_t* lntag); > #endif > > #define PIDDIR HA_VARRUNDIR "/" PACKAGE > #define PIDFILE_BASE PIDDIR "/send_arp-" > > -static int send_arp(LTYPE* l, u_int32_t ip, u_char *device, u_char mac[6] > -, u_char *broadcast, u_char *netmask, u_short arptype); > - > static char print_usage[]={ > "send_arp: sends out custom ARP packet.\n" > " usage: send_arp [-i repeatinterval-ms] [-r repeatcount] [-p pidfile] \\\n" > @@ -135,7 +136,6 @@ main(int argc, char *argv[]) > char* netmask; > u_int32_t ip; > u_char src_mac[6]; > - LTYPE* l; > int repeatcount = 1; > int j; > long msinterval = 1000; > @@ -143,6 +143,13 @@ main(int argc, char *argv[]) > char pidfilenamebuf[64]; > char *pidfilename = NULL; > > +#ifdef HAVE_LIBNET_1_0_API > + LTYPE* l; > + u_char *request, *reply; > +#elif defined(HAVE_LIBNET_1_1_API) > + LTYPE *request, *reply; > +#endif > + > CL_SIGNAL(SIGTERM, byebye); > CL_SIGINTERRUPT(SIGTERM, 1); > > @@ -201,6 +208,24 @@ main(int argc, char *argv[]) > return EXIT_FAILURE; > } > > + if (!strcasecmp(macaddr, AUTO_MAC_ADDR)) { > + if (get_hw_addr(device, src_mac) < 0) { > + cl_log(LOG_ERR, "Cannot find mac address for %s", > + device); > + unlink(pidfilename); > + return EXIT_FAILURE; > + } > + } > + else { > + convert_macaddr((unsigned char *)macaddr, src_mac); > + } > + > +/* > + * We need to send both a broadcast ARP request as well as the ARP response > we > + * were already sending. All the interesting research work for this fix was > + * done by Masaki Hasegawa <[email protected]> and his colleagues. > + */ > + > #if defined(HAVE_LIBNET_1_0_API) > #ifdef ON_DARWIN > if ((ip = libnet_name_resolve((unsigned char*)ipaddr, 1)) == -1UL) { > @@ -219,49 +244,65 @@ main(int argc, char *argv[]) > unlink(pidfilename); > return EXIT_FAILURE; > } > + request = mk_packet(ip, (unsigned char*)device, src_mac > + , (unsigned char*)broadcast, (unsigned char*)netmask > + , ARPOP_REQUEST); > + reply = mk_packet(ip, (unsigned char*)device, src_mac > + , (unsigned char *)broadcast > + , (unsigned char *)netmask, ARPOP_REPLY); > + if (!request || !reply) { > + cl_log(LOG_ERR, "could not create packets"); > + unlink(pidfilename); > + return EXIT_FAILURE; > + } > + for (j=0; j < repeatcount; ++j) { > + c = send_arp(l, (unsigned char*)device, request); > + if (c < 0) { > + break; > + } > + mssleep(msinterval / 2); > + c = send_arp(l, (unsigned char*)device, reply); > + if (c < 0) { > + break; > + } > + if (j != repeatcount-1) { > + mssleep(msinterval / 2); > + } > + } > #elif defined(HAVE_LIBNET_1_1_API) > - if ((l=libnet_init(LIBNET_LINK, device, errbuf)) == NULL) { > + if ((request=libnet_init(LIBNET_LINK, device, errbuf)) == NULL) { > cl_log(LOG_ERR, "libnet_init failure on %s: %s", device, > errbuf); > unlink(pidfilename); > return EXIT_FAILURE; > } > - if ((signed)(ip = libnet_name2addr4(l, ipaddr, 1)) == -1) { > - cl_log(LOG_ERR, "Cannot resolve IP address [%s]", ipaddr); > + if ((reply=libnet_init(LIBNET_LINK, device, errbuf)) == NULL) { > + cl_log(LOG_ERR, "libnet_init failure on %s: %s", device, > errbuf); > unlink(pidfilename); > return EXIT_FAILURE; > } > -#else > -# error "Must have LIBNET API version defined." > -#endif > - > - if (!strcasecmp(macaddr, AUTO_MAC_ADDR)) { > - if (get_hw_addr(device, src_mac) < 0) { > - cl_log(LOG_ERR, "Cannot find mac address for %s", > - device); > - unlink(pidfilename); > - return EXIT_FAILURE; > - } > + if ((signed)(ip = libnet_name2addr4(request, ipaddr, 1)) == -1) { > + cl_log(LOG_ERR, "Cannot resolve IP address [%s]", ipaddr); > + unlink(pidfilename); > + return EXIT_FAILURE; > } > - else { > - convert_macaddr((unsigned char *)macaddr, src_mac); > + request = mk_packet(request, ip, (unsigned char*)device, src_mac > + , (unsigned char*)broadcast, (unsigned char*)netmask > + , ARPOP_REQUEST); > + reply = mk_packet(reply, ip, (unsigned char*)device, src_mac > + , (unsigned char *)broadcast > + , (unsigned char *)netmask, ARPOP_REPLY); > + if (!request || !reply) { > + cl_log(LOG_ERR, "could not create packets"); > + unlink(pidfilename); > + return EXIT_FAILURE; > } > - > -/* > - * We need to send both a broadcast ARP request as well as the ARP response > we > - * were already sending. All the interesting research work for this fix was > - * done by Masaki Hasegawa <[email protected]> and his colleagues. > - */ > for (j=0; j < repeatcount; ++j) { > - c = send_arp(l, ip, (unsigned char*)device, src_mac > - , (unsigned char*)broadcast, (unsigned char*)netmask > - , ARPOP_REQUEST); > + c = send_arp(request); > if (c < 0) { > break; > } > mssleep(msinterval / 2); > - c = send_arp(l, ip, (unsigned char*)device, src_mac > - , (unsigned char *)broadcast > - , (unsigned char *)netmask, ARPOP_REPLY); > + c = send_arp(reply); > if (c < 0) { > break; > } > @@ -269,6 +310,9 @@ main(int argc, char *argv[]) > mssleep(msinterval / 2); > } > } > +#else > +# error "Must have LIBNET API version defined." > +#endif > > unlink(pidfilename); > return c < 0 ? EXIT_FAILURE : EXIT_SUCCESS; > @@ -387,10 +431,9 @@ get_hw_addr(char *device, u_char mac[6]) > */ > > #ifdef HAVE_LIBNET_1_0_API > -int > -send_arp(struct libnet_link_int *l, u_int32_t ip, u_char *device, u_char > *macaddr, u_char *broadcast, u_char *netmask, u_short arptype) > +u_char * > +mk_packet(u_int32_t ip, u_char *device, u_char *macaddr, u_char *broadcast, > u_char *netmask, u_short arptype) > { > - int n; > u_char *buf; > u_char *target_mac; > u_char device_mac[6]; > @@ -400,7 +443,7 @@ send_arp(struct libnet_link_int *l, u_int32_t ip, u_char > *device, u_char *macadd > > if (libnet_init_packet(LIBNET_ARP_H + LIBNET_ETH_H, &buf) == -1) { > cl_log(LOG_ERR, "libnet_init_packet memory:"); > - return -1; > + return NULL; > } > > /* Convert ASCII Mac Address to 6 Hex Digits. */ > @@ -409,14 +452,14 @@ send_arp(struct libnet_link_int *l, u_int32_t ip, > u_char *device, u_char *macadd > if (get_hw_addr((char*)device, device_mac) < 0) { > cl_log(LOG_ERR, "Cannot find mac address for %s", > device); > - return -1; > + return NULL; > } > > if (libnet_build_ethernet(bcast_mac, device_mac, ETHERTYPE_ARP, NULL, 0 > , buf) == -1) { > cl_log(LOG_ERR, "libnet_build_ethernet failed:"); > libnet_destroy_packet(&buf); > - return -1; > + return NULL; > } > > if (arptype == ARPOP_REQUEST) { > @@ -426,8 +469,8 @@ send_arp(struct libnet_link_int *l, u_int32_t ip, u_char > *device, u_char *macadd > target_mac = macaddr; > } > else { > - cl_log(LOG_ERR, "unkonwn arptype:"); > - return -1; > + cl_log(LOG_ERR, "unknown arptype"); > + return NULL; > } > > /* > @@ -447,16 +490,9 @@ send_arp(struct libnet_link_int *l, u_int32_t ip, u_char > *device, u_char *macadd > buf + LIBNET_ETH_H) == -1) { > cl_log(LOG_ERR, "libnet_build_arp failed:"); > libnet_destroy_packet(&buf); > - return -1; > + return NULL; > } > - > - n = libnet_write_link_layer(l, (char*)device, buf, LIBNET_ARP_H + > LIBNET_ETH_H); > - if (n == -1) { > - cl_log(LOG_ERR, "libnet_build_ethernet failed:"); > - } > - > - libnet_destroy_packet(&buf); > - return (n); > + return buf; > } > #endif /* HAVE_LIBNET_1_0_API */ > > @@ -464,10 +500,9 @@ send_arp(struct libnet_link_int *l, u_int32_t ip, u_char > *device, u_char *macadd > > > #ifdef HAVE_LIBNET_1_1_API > -int > -send_arp(libnet_t* lntag, u_int32_t ip, u_char *device, u_char macaddr[6], > u_char *broadcast, u_char *netmask, u_short arptype) > +libnet_t* > +mk_packet(libnet_t* lntag, u_int32_t ip, u_char *device, u_char macaddr[6], > u_char *broadcast, u_char *netmask, u_short arptype) > { > - int n; > u_char *target_mac; > u_char device_mac[6]; > u_char bcast_mac[6] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; > @@ -481,7 +516,7 @@ send_arp(libnet_t* lntag, u_int32_t ip, u_char *device, > u_char macaddr[6], u_cha > } > else { > cl_log(LOG_ERR, "unkonwn arptype:"); > - return -1; > + return NULL; > } > > /* > @@ -502,28 +537,49 @@ send_arp(libnet_t* lntag, u_int32_t ip, u_char *device, > u_char macaddr[6], u_cha > 0 /* packet id */ > ) == -1 ) { > cl_log(LOG_ERR, "libnet_build_arp failed:"); > - return -1; > + return NULL; > } > > /* Ethernet header */ > if (get_hw_addr((char *)device, device_mac) < 0) { > cl_log(LOG_ERR, "Cannot find mac address for %s", > device); > - return -1; > + return NULL; > } > > if (libnet_build_ethernet(bcast_mac, device_mac, ETHERTYPE_ARP, NULL, 0 > , lntag, 0) == -1 ) { > cl_log(LOG_ERR, "libnet_build_ethernet failed:"); > - return -1; > + return NULL; > } > + return lntag; > +} > +#endif /* HAVE_LIBNET_1_1_API */ > > - n = libnet_write(lntag); > +#ifdef HAVE_LIBNET_1_0_API > +int > +send_arp(struct libnet_link_int *l, u_char *device, u_char *buf) > +{ > + int n; > + > + n = libnet_write_link_layer(l, (char*)device, buf, LIBNET_ARP_H + > LIBNET_ETH_H); > if (n == -1) { > - cl_log(LOG_ERR, "libnet_build_ethernet failed:"); > + cl_log(LOG_ERR, "libnet_write_link_layer failed"); > } > - libnet_clear_packet(lntag); > + return (n); > +} > +#endif /* HAVE_LIBNET_1_0_API */ > > +#ifdef HAVE_LIBNET_1_1_API > +int > +send_arp(libnet_t* lntag) > +{ > + int n; > + > + n = libnet_write(lntag); > + if (n == -1) { > + cl_log(LOG_ERR, "libnet_write failed"); > + } > return (n); > } > #endif /* HAVE_LIBNET_1_1_API */ > -- > 1.8.0 > > diff --git a/heartbeat/SendArp b/heartbeat/SendArp > index 553469b..611bb47 100755 > --- a/heartbeat/SendArp > +++ b/heartbeat/SendArp > @@ -123,19 +123,6 @@ usage: $0 {start|stop|monitor|validate-all|meta-data} > Expects to have a fully populated OCF RA-compliant environment set. > END > } > -# > -# Check if ip-address is running. > -# We return stopped or running. > -# > -sendarp_status() { > - if > - [ -f "$SENDARPPIDFILE" ] > - then > - return $OCF_SUCCESS > - else > - return $OCF_NOT_RUNNING > - fi > -} > > # > # Send gratuitous arp > @@ -144,12 +131,12 @@ sendarp_start() { > > sendarp_validate > if [ $? = $OCF_ERR_ARGS ]; then > - return $OCF_ERR_ARGS; > + return $OCF_ERR_ARGS > fi > > - sendarp_status > + sendarp_monitor > if [ $? = $OCF_SUCCESS ]; then > - return $OCF_SUCCESS; > + return $OCF_SUCCESS > fi > > [ -r ${HA_CONFDIR}/arp_config ] && . ${HA_CONFDIR}/arp_config > @@ -169,20 +156,21 @@ sendarp_start() { > $SENDARP $ARGS || (ocf_log err "Could not send gratuitous arps"; > return $OCF_ERR_GENERIC) > ;; > esac > - return $OCF_SUCCESS; > + ha_pseudo_resource SendArp_${OCF_RESOURCE_INSTANCE} start > + return $OCF_SUCCESS > } > > # > # Stop sending gratuitous arp > # > sendarp_stop() { > - sendarp_validate > - if [ $? = $OCF_ERR_ARGS ]; then > - return $OCF_ERR_ARGS; > + sendarp_monitor > + if [ $? -eq $OCF_NOT_RUNNING ]; then > + return $OCF_SUCCESS > fi > - > + > rc=$OCF_SUCCESS > - > + > if > [ -f "$SENDARPPIDFILE" ] > then > @@ -204,6 +192,7 @@ sendarp_stop() { > case $rc in > $OCF_SUCCESS) > ocf_log info "SendArp for $BASEIP/$INTERFACE released" > + ha_pseudo_resource SendArp_${OCF_RESOURCE_INSTANCE} stop > ;; > *) > ocf_log warn "SendArp for $BASEIP/$INTERFACE NOT released" > @@ -215,7 +204,7 @@ sendarp_stop() { > # This is always active, because it doesn't do much > # > sendarp_monitor() { > - return $OCF_SUCCESS > + ha_pseudo_resource SendArp_${OCF_RESOURCE_INSTANCE} monitor > } > > sendarp_validate() { > @@ -236,7 +225,7 @@ stop) sendarp_stop > ;; > monitor) sendarp_monitor > ;; > -status) sendarp_status > +status) sendarp_monitor > if [ $? = $OCF_SUCCESS ]; then > echo "running" > exit $OCF_SUCCESS; > _______________________________________________________ > Linux-HA-Dev: [email protected] > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev > Home Page: http://linux-ha.org/ _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
