From 5c33349b3efff36a6acd36c6600b61e7cc2dbffc Mon Sep 17 00:00:00 2001 From: ajs Date: Wed, 23 Feb 2005 15:43:01 +0000 Subject: 2005-02-23 Andrew J. Schorr * ospfd.h: Add new field struct stream *ibuf to struct ospf. * ospfd.c: (ospf_new) Check return code from ospf_sock_init. Allocate ibuf using stream_new(OSPF_MAX_PACKET_SIZE+1). (ospf_finish) Call stream_free(ospf->ibuf. * ospf_packet.c: (ospf_read) Call stream_reset(ospf->ibuf) and then pass it to ospf_recv_packet for use in receiving the packet (instead of allocating a new stream for each packet received). Eliminate all calls to stream_free(ibuf). (ospf_recv_packet) The struct stream *ibuf is now passed in as an argument. No need to use recvfrom to peek at the packet header (to see how big it is), just use ospf->ibuf which is always large enough (this eliminates a system call to recvfrom). Therefore, no need to allocate a stream just for this packet, and no need to free it when done. --- ospfd/ChangeLog | 17 +++++++++++++ ospfd/ospf_packet.c | 73 +++++++++++++++++++---------------------------------- ospfd/ospfd.c | 17 ++++++++++--- ospfd/ospfd.h | 1 + 4 files changed, 58 insertions(+), 50 deletions(-) diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog index ee30e55c..668999f9 100644 --- a/ospfd/ChangeLog +++ b/ospfd/ChangeLog @@ -1,3 +1,20 @@ +2005-02-23 Andrew J. Schorr + + * ospfd.h: Add new field struct stream *ibuf to struct ospf. + * ospfd.c: (ospf_new) Check return code from ospf_sock_init. + Allocate ibuf using stream_new(OSPF_MAX_PACKET_SIZE+1). + (ospf_finish) Call stream_free(ospf->ibuf. + * ospf_packet.c: (ospf_read) Call stream_reset(ospf->ibuf) and then + pass it to ospf_recv_packet for use in receiving the packet + (instead of allocating a new stream for each packet received). + Eliminate all calls to stream_free(ibuf). + (ospf_recv_packet) The struct stream *ibuf is now passed in as + an argument. No need to use recvfrom to peek at the packet + header (to see how big it is), just use ospf->ibuf which is + always large enough (this eliminates a system call to recvfrom). + Therefore, no need to allocate a stream just for this packet, + and no need to free it when done. + 2005-02-23 Vincenzo Eramo * ospf_lsa.h: New flag to the LSA structure for the SPF calculation. diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c index 6b7a796d..87913361 100644 --- a/ospfd/ospf_packet.c +++ b/ospfd/ospf_packet.c @@ -2033,12 +2033,11 @@ ospf_ls_ack (struct ip *iph, struct ospf_header *ospfh, } static struct stream * -ospf_recv_packet (int fd, struct interface **ifp) +ospf_recv_packet (int fd, struct interface **ifp, struct stream *ibuf) { int ret; - struct ip iph; + struct ip *iph; u_int16_t ip_len; - struct stream *ibuf; unsigned int ifindex = 0; struct iovec iov; /* Header and data both require alignment. */ @@ -2051,30 +2050,26 @@ ospf_recv_packet (int fd, struct interface **ifp) msgh.msg_control = (caddr_t) buff; msgh.msg_controllen = sizeof (buff); - /* XXX Is there an upper limit on the size of these packets? If there is, - it would be more efficient to read the whole packet in one shot without - peeking (this would cut down from 2 system calls to 1). And this would - make the error-handling logic a bit more robust. */ - ret = recvfrom (fd, (void *)&iph, sizeof (iph), MSG_PEEK, NULL, 0); - - if (ret != sizeof (iph)) + ret = stream_recvmsg (ibuf, fd, &msgh, 0, OSPF_MAX_PACKET_SIZE+1); + if (ret < 0) { - if (ret > 0) - { - zlog_warn("ospf_recv_packet: discarding runt packet of length %d " - "(ip header size is %u)", - ret, (u_int)sizeof(iph)); - recvfrom (fd, (void *)&iph, ret, 0, NULL, 0); - } - else - zlog_warn("ospf_recv_packet: recvfrom returned %d: %s", - ret, safe_strerror(errno)); + zlog_warn("stream_recvmsg failed: %s", safe_strerror(errno)); + return NULL; + } + if (ret < sizeof(iph)) + { + zlog_warn("ospf_recv_packet: discarding runt packet of length %d " + "(ip header size is %u)", + ret, (u_int)sizeof(iph)); return NULL; } - sockopt_iphdrincl_swab_systoh (&iph); + /* Note that there should not be alignment problems with this assignment + because this is at the beginning of the stream data buffer. */ + iph = (struct ip *) STREAM_DATA(ibuf); + sockopt_iphdrincl_swab_systoh (iph); - ip_len = iph.ip_len; + ip_len = iph->ip_len; #if !defined(GNU_LINUX) && (OpenBSD < 200311) /* @@ -2091,25 +2086,17 @@ ospf_recv_packet (int fd, struct interface **ifp) * * For more details, see . */ - ip_len = ip_len + (iph.ip_hl << 2); + ip_len = ip_len + (iph->ip_hl << 2); #endif - if ( (ibuf = stream_new (ip_len)) == NULL) - return NULL; - iov.iov_base = STREAM_DATA (ibuf); - iov.iov_len = ip_len; - - ret = stream_recvmsg (ibuf, fd, &msgh, 0, ip_len); - ifindex = getsockopt_ifindex (AF_INET, &msgh); *ifp = if_lookup_by_index (ifindex); if (ret != ip_len) { - zlog_warn ("ospf_recv_packet short read. " - "ip_len %d bytes read %d", ip_len, ret); - stream_free (ibuf); + zlog_warn ("ospf_recv_packet read length mismatch: ip_len is %d, " + "but recvmsg returned %d", ip_len, ret); return NULL; } @@ -2360,12 +2347,14 @@ ospf_read (struct thread *thread) ospf->t_read = thread_add_read (master, ospf_read, ospf, ospf->fd); /* read OSPF packet. */ - ibuf = ospf_recv_packet (ospf->fd, &ifp); - if (ibuf == NULL) + stream_reset(ospf->ibuf); + if (!(ibuf = ospf_recv_packet (ospf->fd, &ifp, ospf->ibuf))) return -1; + /* Note that there should not be alignment problems with this assignment + because this is at the beginning of the stream data buffer. */ iph = (struct ip *) STREAM_DATA (ibuf); - sockopt_iphdrincl_swab_systoh (iph); + /* Note that sockopt_iphdrincl_swab_systoh was called in ospf_recv_packet. */ if (ifp == NULL) /* Handle cases where the platform does not support retrieving the ifindex, @@ -2374,10 +2363,7 @@ ospf_read (struct thread *thread) ifp = if_lookup_address (iph->ip_src); if (ifp == NULL) - { - stream_free (ibuf); - return 0; - } + return 0; /* IP Header dump. */ if (IS_DEBUG_OSPF_PACKET(0, RECV)) @@ -2391,7 +2377,6 @@ ospf_read (struct thread *thread) zlog_debug ("ospf_read[%s]: Dropping self-originated packet", inet_ntoa (iph->ip_src)); } - stream_free (ibuf); return 0; } @@ -2418,7 +2403,6 @@ ospf_read (struct thread *thread) zlog_warn ("Packet from [%s] received on link %s" " but no ospf_interface", inet_ntoa (iph->ip_src), ifp->name); - stream_free (ibuf); return 0; } } @@ -2430,7 +2414,6 @@ ospf_read (struct thread *thread) { zlog_warn ("Packet from [%s] received on wrong link %s", inet_ntoa (iph->ip_src), ifp->name); - stream_free (ibuf); return 0; } else if (oi->state == ISM_Down) @@ -2441,7 +2424,6 @@ ospf_read (struct thread *thread) inet_ntop(AF_INET, &iph->ip_src, buf[0], sizeof(buf[0])), inet_ntop(AF_INET, &iph->ip_dst, buf[1], sizeof(buf[1])), ifp->name, if_flag_dump(ifp->flags)); - stream_free (ibuf); /* Fix multicast memberships? */ if (iph->ip_dst.s_addr == htonl(OSPF_ALLSPFROUTERS)) SET_FLAG(oi->multicast_memberships, MEMBER_ALLROUTERS); @@ -2463,7 +2445,6 @@ ospf_read (struct thread *thread) zlog_warn ("Dropping packet for AllDRouters from [%s] via [%s] (ISM: %s)", inet_ntoa (iph->ip_src), IF_NAME (oi), LOOKUP (ospf_ism_state_msg, oi->state)); - stream_free (ibuf); /* Try to fix multicast membership. */ SET_FLAG(oi->multicast_memberships, MEMBER_DROUTERS); ospf_if_set_multicast(oi); @@ -2500,7 +2481,6 @@ ospf_read (struct thread *thread) ospf_packet_type_str[ospfh->type], inet_ntoa (iph->ip_src)); } - stream_free (ibuf); return ret; } @@ -2534,7 +2514,6 @@ ospf_read (struct thread *thread) break; } - stream_free (ibuf); return 0; } diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 931ae49c..a77fb4b1 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -204,9 +204,19 @@ ospf_new () new, new->lsa_refresh_interval); new->lsa_refresher_started = time (NULL); - new->fd = ospf_sock_init (); - if (new->fd >= 0) - new->t_read = thread_add_read (master, ospf_read, new, new->fd); + if ((new->fd = ospf_sock_init()) < 0) + { + zlog_err("ospf_new: fatal error: ospf_sock_init was unable to open " + "a socket"); + exit(1); + } + if ((new->ibuf = stream_new(OSPF_MAX_PACKET_SIZE+1)) == NULL) + { + zlog_err("ospf_new: fatal error: stream_new(%u) failed allocating ibuf", + OSPF_MAX_PACKET_SIZE+1); + exit(1); + } + new->t_read = thread_add_read (master, ospf_read, new, new->fd); new->oi_write_q = list_new (); return new; @@ -360,6 +370,7 @@ ospf_finish (struct ospf *ospf) OSPF_TIMER_OFF (ospf->t_write); close (ospf->fd); + stream_free(ospf->ibuf); #ifdef HAVE_OPAQUE_LSA LSDB_LOOP (OPAQUE_AS_LSDB (ospf), rn, lsa) diff --git a/ospfd/ospfd.h b/ospfd/ospfd.h index 0f778e0d..2a6c23e9 100644 --- a/ospfd/ospfd.h +++ b/ospfd/ospfd.h @@ -248,6 +248,7 @@ struct ospf struct thread *t_write; struct thread *t_read; int fd; + struct stream *ibuf; struct list *oi_write_q; /* Distribute lists out of other route sources. */ -- cgit v1.2.1