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 <horms@verge.net.au>
Co-authored-by: Ben Pfaff <blp@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 4a16e5c..689014b 100644 (file)
--- 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 6286f6b..b2a7572 100644 (file)
--- 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 68d476f..106c18a 100644 (file)
--- 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 b0630cb..1dcc1f4 100644 (file)
--- 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 575227c..66b411a 100644 (file)
--- 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;
     }
 }