From bfd3367b9e7426ffc931c9b0d010d8b1b5884ec2 Mon Sep 17 00:00:00 2001 From: Simon Horman Date: Wed, 15 Jan 2014 17:17:00 +0900 Subject: [PATCH 1/1] netdev_class: Pass a struct ofpbuf * to rx_recv() Update the netdev_class so that struct ofpbuf * is passed to rx_recv() to provide both the data and size of the data to read a packet into. On success, update struct ofpbuf size inside netdev_class rx_recv implementation and return 0. This moves logic from the caller. On error a positive error code is returned, whereas previously a negative error code was returned. This is a more common convention. This patch should not have any behavioural changes. This patch is in preparation for the netdev-linux variant of rx_recv() making use of headroom in the struct ofpbuf * parameter to push a VLAN tag obtained from auxdata. Signed-off-by: Simon Horman Co-authored-by: Ben Pfaff Signed-off-by: Ben Pfaff --- lib/netdev-bsd.c | 28 ++++++++++++++++------------ lib/netdev-dummy.c | 13 +++++++------ lib/netdev-linux.c | 18 +++++++++++------- lib/netdev-provider.h | 15 ++++++++------- lib/netdev.c | 8 +++----- 5 files changed, 45 insertions(+), 37 deletions(-) diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c index 4a16e5cf3..689014b90 100644 --- a/lib/netdev-bsd.c +++ b/lib/netdev-bsd.c @@ -568,20 +568,21 @@ proc_pkt(u_char *args_, const struct pcap_pkthdr *hdr, const u_char *packet) * from rx->pcap. */ static int -netdev_rx_bsd_recv_pcap(struct netdev_rx_bsd *rx, void *data, size_t size) +netdev_rx_bsd_recv_pcap(struct netdev_rx_bsd *rx, struct ofpbuf *buffer) { struct pcap_arg arg; int ret; /* prepare the pcap argument to store the packet */ - arg.size = size; - arg.data = data; + arg.size = ofpbuf_tailroom(buffer); + arg.data = buffer->data; for (;;) { ret = pcap_dispatch(rx->pcap_handle, 1, proc_pkt, (u_char *) &arg); if (ret > 0) { - return arg.retval; /* arg.retval < 0 is handled in the caller */ + buffer->size += arg.retval; + return 0; } if (ret == -1) { if (errno == EINTR) { @@ -589,7 +590,7 @@ netdev_rx_bsd_recv_pcap(struct netdev_rx_bsd *rx, void *data, size_t size) } } - return -EAGAIN; + return EAGAIN; } } @@ -599,30 +600,33 @@ netdev_rx_bsd_recv_pcap(struct netdev_rx_bsd *rx, void *data, size_t size) * 'rx->fd' is initialized with the tap file descriptor. */ static int -netdev_rx_bsd_recv_tap(struct netdev_rx_bsd *rx, void *data, size_t size) +netdev_rx_bsd_recv_tap(struct netdev_rx_bsd *rx, struct ofpbuf *buffer) { + size_t size = ofpbuf_tailroom(buffer); + for (;;) { - ssize_t retval = read(rx->fd, data, size); + ssize_t retval = read(rx->fd, buffer->data, size); if (retval >= 0) { - return retval; + buffer->size += retval; + return 0; } else if (errno != EINTR) { if (errno != EAGAIN) { VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s", ovs_strerror(errno), netdev_rx_get_name(&rx->up)); } - return -errno; + return errno; } } } static int -netdev_bsd_rx_recv(struct netdev_rx *rx_, void *data, size_t size) +netdev_bsd_rx_recv(struct netdev_rx *rx_, struct ofpbuf *buffer) { struct netdev_rx_bsd *rx = netdev_rx_bsd_cast(rx_); return (rx->pcap_handle - ? netdev_rx_bsd_recv_pcap(rx, data, size) - : netdev_rx_bsd_recv_tap(rx, data, size)); + ? netdev_rx_bsd_recv_pcap(rx, buffer) + : netdev_rx_bsd_recv_tap(rx, buffer)); } /* diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c index 6286f6b50..b2a7572e4 100644 --- a/lib/netdev-dummy.c +++ b/lib/netdev-dummy.c @@ -447,7 +447,7 @@ netdev_dummy_rx_dealloc(struct netdev_rx *rx_) } static int -netdev_dummy_rx_recv(struct netdev_rx *rx_, void *buffer, size_t size) +netdev_dummy_rx_recv(struct netdev_rx *rx_, struct ofpbuf *buffer) { struct netdev_rx_dummy *rx = netdev_rx_dummy_cast(rx_); struct netdev_dummy *netdev = netdev_dummy_cast(rx->up.netdev); @@ -464,19 +464,20 @@ netdev_dummy_rx_recv(struct netdev_rx *rx_, void *buffer, size_t size) ovs_mutex_unlock(&netdev->mutex); if (!packet) { - return -EAGAIN; + return EAGAIN; } - if (packet->size <= size) { - memcpy(buffer, packet->data, packet->size); - retval = packet->size; + if (packet->size <= ofpbuf_tailroom(buffer)) { + memcpy(buffer->data, packet->data, packet->size); + buffer->size += packet->size; + retval = 0; ovs_mutex_lock(&netdev->mutex); netdev->stats.rx_packets++; netdev->stats.rx_bytes += packet->size; ovs_mutex_unlock(&netdev->mutex); } else { - retval = -EMSGSIZE; + retval = EMSGSIZE; } ofpbuf_delete(packet); diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 68d476f18..106c18a3d 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -848,25 +848,29 @@ netdev_linux_rx_dealloc(struct netdev_rx *rx_) } static int -netdev_linux_rx_recv(struct netdev_rx *rx_, void *data, size_t size) +netdev_linux_rx_recv(struct netdev_rx *rx_, struct ofpbuf *buffer) { struct netdev_rx_linux *rx = netdev_rx_linux_cast(rx_); ssize_t retval; + size_t size = ofpbuf_tailroom(buffer); do { retval = (rx->is_tap - ? read(rx->fd, data, size) - : recv(rx->fd, data, size, MSG_TRUNC)); + ? read(rx->fd, buffer->data, size) + : recv(rx->fd, buffer->data, size, MSG_TRUNC)); } while (retval < 0 && errno == EINTR); - if (retval >= 0) { - return retval > size ? -EMSGSIZE : retval; - } else { + if (retval < 0) { if (errno != EAGAIN) { VLOG_WARN_RL(&rl, "error receiving Ethernet packet on %s: %s", ovs_strerror(errno), netdev_rx_get_name(rx_)); } - return -errno; + return errno; + } else if (retval > size) { + return EMSGSIZE; + } else { + buffer->size += retval; + return 0; } } diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index b0630cb4b..1dcc1f45f 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -634,17 +634,18 @@ struct netdev_class { void (*rx_destruct)(struct netdev_rx *); void (*rx_dealloc)(struct netdev_rx *); - /* Attempts to receive a packet from 'rx' into the 'size' bytes in - * 'buffer'. If successful, returns the number of bytes in the received - * packet, otherwise a negative errno value. Returns -EAGAIN immediately - * if no packet is ready to be received. + /* Attempts to receive a packet from 'rx' into the tailroom of 'buffer', + * which should initially be empty. If successful, returns 0 and + * increments 'buffer->size' by the number of bytes in the received packet, + * otherwise a positive errno value. Returns EAGAIN immediately if no + * packet is ready to be received. * - * Must return -EMSGSIZE, and discard the packet, if the received packet - * is longer than 'size' bytes. + * Must return EMSGSIZE, and discard the packet, if the received packet + * is longer than 'ofpbuf_tailroom(buffer)'. * * This function may be set to null if it would always return EOPNOTSUPP * anyhow. */ - int (*rx_recv)(struct netdev_rx *rx, void *buffer, size_t size); + int (*rx_recv)(struct netdev_rx *rx, struct ofpbuf *buffer); /* Registers with the poll loop to wake up from the next call to * poll_block() when a packet is ready to be received with netdev_rx_recv() diff --git a/lib/netdev.c b/lib/netdev.c index 575227c2f..66b411a44 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -551,17 +551,15 @@ netdev_rx_recv(struct netdev_rx *rx, struct ofpbuf *buffer) ovs_assert(buffer->size == 0); ovs_assert(ofpbuf_tailroom(buffer) >= ETH_TOTAL_MIN); - retval = rx->netdev->netdev_class->rx_recv(rx, buffer->data, - ofpbuf_tailroom(buffer)); - if (retval >= 0) { + retval = rx->netdev->netdev_class->rx_recv(rx, buffer); + if (!retval) { COVERAGE_INC(netdev_received); - buffer->size += retval; if (buffer->size < ETH_TOTAL_MIN) { ofpbuf_put_zeros(buffer, ETH_TOTAL_MIN - buffer->size); } return 0; } else { - return -retval; + return retval; } } -- 2.43.0