diff mbox series

[ovs-dev] treewide: Wider use of packet batch APIs.

Message ID 20181210171753.3375-1-i.maximets@samsung.com
State Accepted
Delegated to: Ian Stokes
Headers show
Series [ovs-dev] treewide: Wider use of packet batch APIs. | expand

Commit Message

Ilya Maximets Dec. 10, 2018, 5:17 p.m. UTC 
This patch replaces most of direct accesses to the dp_packet_batch
internal components by appropriate APIs.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/dpif.c             |  2 +-
 lib/netdev-bsd.c       | 11 +++++------
 lib/netdev-dummy.c     |  5 ++---
 lib/odp-execute.c      |  2 +-
 tests/test-conntrack.c |  4 ++--
 5 files changed, 11 insertions(+), 13 deletions(-)

Comments

Ben Pfaff Dec. 10, 2018, 5:49 p.m. UTC | #1 
On Mon, Dec 10, 2018 at 08:17:53PM +0300, Ilya Maximets wrote:
> This patch replaces most of direct accesses to the dp_packet_batch
> internal components by appropriate APIs.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

I looked over this one and didn't see a problem, but, Ian, I'll leave it
to you because you are probably more familiar with packet batching.
Stokes, Ian Dec. 11, 2018, 8:38 p.m. UTC | #2 
> On Mon, Dec 10, 2018 at 08:17:53PM +0300, Ilya Maximets wrote:
> > This patch replaces most of direct accesses to the dp_packet_batch
> > internal components by appropriate APIs.
> >
> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> 
> I looked over this one and didn't see a problem, but, Ian, I'll leave it
> to you because you are probably more familiar with packet batching.

Thanks Ben, gave this the once over, LGTM and tested, will apply to master.

Thanks
Ian
diff mbox series

Patch

diff --git a/lib/dpif.c b/lib/dpif.c
index 59aa1dc7c..e35f11147 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1166,7 +1166,7 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
     int type = nl_attr_type(action);
     struct dp_packet *packet = packets_->packets[0];
 
-    ovs_assert(packets_->count == 1);
+    ovs_assert(dp_packet_batch_size(packets_) == 1);
 
     switch ((enum ovs_action_attr)type) {
     case OVS_ACTION_ATTR_METER:
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 99d4f4842..46698d547 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -640,8 +640,7 @@  netdev_bsd_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet_batch *batch,
     if (retval) {
         dp_packet_delete(packet);
     } else {
-        batch->packets[0] = packet;
-        batch->count = 1;
+        dp_packet_batch_init_packet(batch, packet);
     }
 
     if (qfill) {
@@ -690,8 +689,8 @@  netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
 {
     struct netdev_bsd *dev = netdev_bsd_cast(netdev_);
     const char *name = netdev_get_name(netdev_);
+    struct dp_packet *packet;
     int error;
-    int i;
 
     ovs_mutex_lock(&dev->mutex);
     if (dev->tap_fd < 0 && !dev->pcap) {
@@ -700,9 +699,9 @@  netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
         error = 0;
     }
 
-    for (i = 0; i < batch->count; i++) {
-        const void *data = dp_packet_data(batch->packets[i]);
-        size_t size = dp_packet_size(batch->packets[i]);
+    DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+        const void *data = dp_packet_data(packet);
+        size_t size = dp_packet_size(packet);
 
         while (!error) {
             ssize_t retval;
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 23817d121..72b4f7adc 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1036,8 +1036,7 @@  netdev_dummy_rxq_recv(struct netdev_rxq *rxq_, struct dp_packet_batch *batch,
     netdev->custom_stats[1].value++;
     ovs_mutex_unlock(&netdev->mutex);
 
-    batch->packets[0] = packet;
-    batch->count = 1;
+    dp_packet_batch_init_packet(batch, packet);
 
     if (qfill) {
         *qfill = -ENOTSUP;
@@ -1091,7 +1090,7 @@  netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
         const void *buffer = dp_packet_data(packet);
         size_t size = dp_packet_size(packet);
 
-        if (batch->packets[i]->packet_type != htonl(PT_ETH)) {
+        if (packet->packet_type != htonl(PT_ETH)) {
             error = EPFNOSUPPORT;
             break;
         }
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5831d1fdb..3b6890e95 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -717,7 +717,7 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
 
                 dp_execute_action(dp, batch, a, should_steal);
 
-                if (last_action || batch->count == 0) {
+                if (last_action || dp_packet_batch_is_empty(batch)) {
                     /* We do not need to free the packets.
                      * Either dp_execute_actions() has stolen them
                      * or the batch is freed due to errors. In either
diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c
index 24d0bb442..07a4857cf 100644
--- a/tests/test-conntrack.c
+++ b/tests/test-conntrack.c
@@ -177,7 +177,7 @@  pcap_batch_execute_conntrack(struct conntrack *ct_,
                               NULL, NULL, 0, 0, NULL, NULL, now);
             dp_packet_batch_init(&new_batch);
         }
-        new_batch.packets[new_batch.count++] = packet;;
+        dp_packet_batch_add(&new_batch, packet);
     }
 
     if (!dp_packet_batch_is_empty(&new_batch)) {
@@ -226,7 +226,7 @@  test_pcap(struct ovs_cmdl_context *ctx)
             }
             dp_packet_batch_add(batch, packet);
         }
-        if (!batch->count) {
+        if (dp_packet_batch_is_empty(batch)) {
             break;
         }
         pcap_batch_execute_conntrack(&ct, batch);