2005-02-23 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
* 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.
diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog
index ee30e55..668999f 100644
--- a/ospfd/ChangeLog
+++ b/ospfd/ChangeLog
@@ -1,3 +1,20 @@
+2005-02-23 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
+
+ * 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 <eramo at infocom.ing.uniroma1.it>
* 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 6b7a796..8791336 100644
--- a/ospfd/ospf_packet.c
+++ b/ospfd/ospf_packet.c
@@ -2033,12 +2033,11 @@
}
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 @@
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 @@
*
* For more details, see <netinet/ip_input.c>.
*/
- 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->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 @@
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 @@
zlog_debug ("ospf_read[%s]: Dropping self-originated packet",
inet_ntoa (iph->ip_src));
}
- stream_free (ibuf);
return 0;
}
@@ -2418,7 +2403,6 @@
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 @@
{
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 @@
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 @@
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_packet_type_str[ospfh->type],
inet_ntoa (iph->ip_src));
}
- stream_free (ibuf);
return ret;
}
@@ -2534,7 +2514,6 @@
break;
}
- stream_free (ibuf);
return 0;
}
diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c
index 931ae49..a77fb4b 100644
--- a/ospfd/ospfd.c
+++ b/ospfd/ospfd.c
@@ -204,9 +204,19 @@
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_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 0f778e0..2a6c23e 100644
--- a/ospfd/ospfd.h
+++ b/ospfd/ospfd.h
@@ -248,6 +248,7 @@
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. */