[ovs-dev] [PATCH v3 2/4] odp-execute: Support dropping packets.

Jarno Rajahalme jarno at ovn.org
Tue Nov 24 04:54:33 UTC 2015


Meter action can drop or modify packets, so the execution framework is
changed to allow for this.  Also, a meter action can appear alone
(e.g., to measure traffic that is to be dropped), so the existing drop
implementation is not sufficient any more.

The action execution framework is changed in three ways:

1. Action execution can shrink the number of packets in a batch to be
   further processed by the remaining actions.

2. Whenever a packet is 'stolen' (e.g., for output), the corresponding
   packet pointer is changed to NULL. NULLed pointers must be excluded
   from further processing by using the change #1 above.  Sometimes
   this means that the packet pointers are shuffled so that remaining
   action processing never needs to check for the NULL pointers
   explicitly.

3. Packet deletion is centralized to the odp_execute_actions()
   function.  This make memory leaks less likely to appear as new
   kinds of action are added later.

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
 lib/dp-packet.h       | 13 ++++++++--
 lib/dpif-netdev.c     | 53 +++++++++++++++++++++-----------------
 lib/dpif.c            |  6 +++--
 lib/netdev-bsd.c      |  8 +-----
 lib/netdev-dpdk.c     | 55 +++++++++++++---------------------------
 lib/netdev-dummy.c    |  8 +-----
 lib/netdev-linux.c    |  9 +------
 lib/netdev-provider.h |  4 +++
 lib/odp-execute.c     | 70 ++++++++++++++++++++++++++++-----------------------
 lib/odp-execute.h     | 12 +++++----
 lib/packets.c         | 14 ++++-------
 11 files changed, 120 insertions(+), 132 deletions(-)

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 5044ce0..8899bc8 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -108,7 +108,16 @@ struct dp_packet *dp_packet_clone_with_headroom(const struct dp_packet *,
 struct dp_packet *dp_packet_clone_data(const void *, size_t);
 struct dp_packet *dp_packet_clone_data_with_headroom(const void *, size_t,
                                                      size_t headroom);
-static inline void dp_packet_delete(struct dp_packet *);
+static inline void dp_packet_delete__(struct dp_packet *);
+
+/* NULL the packet pointer to mark the packet as deleted. */
+#define dp_packet_delete(PKT)                   \
+    do {                                        \
+        struct dp_packet **pkt__ = &(PKT);      \
+                                                \
+        dp_packet_delete__(*pkt__);             \
+        *pkt__ = NULL;                          \
+    } while (0)
 
 static inline void *dp_packet_at(const struct dp_packet *, size_t offset,
                                  size_t size);
@@ -147,7 +156,7 @@ static inline bool dp_packet_equal(const struct dp_packet *,
 
 /* Frees memory that 'b' points to, as well as 'b' itself. */
 static inline void
-dp_packet_delete(struct dp_packet *b)
+dp_packet_delete__(struct dp_packet *b)
 {
     if (b) {
         if (b->source == DPBUF_DPDK) {
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 10d94e9..9dde5f0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3478,14 +3478,12 @@ dpif_netdev_register_upcall_cb(struct dpif *dpif, upcall_callback *cb,
 }
 
 static void
-dp_netdev_drop_packets(struct dp_packet **packets, int cnt, bool may_steal)
+dp_netdev_drop_packets(struct dp_packet **packets, int cnt)
 {
-    if (may_steal) {
-        int i;
+    int i;
 
-        for (i = 0; i < cnt; i++) {
-            dp_packet_delete(packets[i]);
-        }
+    for (i = 0; i < cnt; i++) {
+        dp_packet_delete(packets[i]);
     }
 }
 
@@ -3519,7 +3517,8 @@ dp_netdev_clone_pkt_batch(struct dp_packet **dst_pkts,
     }
 }
 
-static void
+/* Mark stolen packets by setting the corresponding packet pointer to NULL. */
+static int
 dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
               const struct nlattr *a, bool may_steal)
     OVS_NO_THREAD_SAFETY_ANALYSIS
@@ -3537,7 +3536,6 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
         p = dp_netdev_lookup_port(dp, u32_to_odp(nl_attr_get_u32(a)));
         if (OVS_LIKELY(p)) {
             netdev_send(p->netdev, pmd->tx_qid, packets, cnt, may_steal);
-            return;
         }
         break;
 
@@ -3556,10 +3554,11 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
                 (*depth)++;
                 dp_netdev_input(pmd, packets, cnt);
                 (*depth)--;
-            } else {
-                dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
+                goto stolen;
+            } else if (!may_steal) {
+                /* Drop the cloned packets. */
+                dp_netdev_drop_packets(tnl_pkt, cnt);
             }
-            return;
         }
         break;
 
@@ -3579,7 +3578,6 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
 
                 err = netdev_pop_header(p->netdev, packets, cnt);
                 if (!err) {
-
                     for (i = 0; i < cnt; i++) {
                         packets[i]->md.in_port.odp_port = portno;
                     }
@@ -3587,10 +3585,11 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
                     (*depth)++;
                     dp_netdev_input(pmd, packets, cnt);
                     (*depth)--;
-                } else {
-                    dp_netdev_drop_packets(tnl_pkt, cnt, !may_steal);
+                    goto stolen;
+                } else if (!may_steal) {
+                    /* Drop the cloned packets. */
+                    dp_netdev_drop_packets(tnl_pkt, cnt);
                 }
-                return;
             }
         }
         break;
@@ -3625,7 +3624,6 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
             ofpbuf_uninit(&actions);
             fat_rwlock_unlock(&dp->upcall_rwlock);
 
-            return;
         }
         break;
 
@@ -3634,8 +3632,8 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
             struct dp_packet *recirc_pkts[NETDEV_MAX_BURST];
 
             if (!may_steal) {
-               dp_netdev_clone_pkt_batch(recirc_pkts, packets, cnt);
-               packets = recirc_pkts;
+                dp_netdev_clone_pkt_batch(recirc_pkts, packets, cnt);
+                packets = recirc_pkts;
             }
 
             for (i = 0; i < cnt; i++) {
@@ -3645,11 +3643,10 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
             (*depth)++;
             dp_netdev_input(pmd, packets, cnt);
             (*depth)--;
-
-            return;
+            goto stolen;
+        } else {
+            VLOG_WARN("Packet dropped. Max recirculation depth exceeded.");
         }
-
-        VLOG_WARN("Packet dropped. Max recirculation depth exceeded.");
         break;
 
     case OVS_ACTION_ATTR_CT:
@@ -3660,6 +3657,8 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
         break;
 
     case OVS_ACTION_ATTR_METER:
+        break; /* Never drop. */
+
     case OVS_ACTION_ATTR_PUSH_VLAN:
     case OVS_ACTION_ATTR_POP_VLAN:
     case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -3673,7 +3672,15 @@ dp_execute_cb(void *aux_, struct dp_packet **packets, int cnt,
         OVS_NOT_REACHED();
     }
 
-    dp_netdev_drop_packets(packets, cnt, may_steal);
+    return cnt;
+stolen:
+    /* Mark stolen packets. */
+    if (may_steal) {
+        for (i = 0; i < cnt; i++) {
+            packets[i] = NULL;
+        }
+    }
+    return cnt;
 }
 
 static void
diff --git a/lib/dpif.c b/lib/dpif.c
index 06669d3..7389c8f 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1089,7 +1089,7 @@ struct dpif_execute_helper_aux {
 
 /* This is called for actions that need the context of the datapath to be
  * meaningful. */
-static void
+static int
 dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
                        const struct nlattr *action, bool may_steal OVS_UNUSED)
 {
@@ -1136,7 +1136,8 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
         if (md->tunnel.ip_dst) {
             ofpbuf_uninit(&execute_actions);
         }
-        break;
+
+        return (aux->error == 0) ? cnt : 0;
     }
 
     case OVS_ACTION_ATTR_HASH:
@@ -1152,6 +1153,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
     }
+    return 0;
 }
 
 /* Executes 'execute' by performing most of the actions in userspace and
diff --git a/lib/netdev-bsd.c b/lib/netdev-bsd.c
index 60e5615..a241c04 100644
--- a/lib/netdev-bsd.c
+++ b/lib/netdev-bsd.c
@@ -684,7 +684,7 @@ netdev_bsd_rxq_drain(struct netdev_rxq *rxq_)
  */
 static int
 netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
-                struct dp_packet **pkts, int cnt, bool may_steal)
+                struct dp_packet **pkts, int cnt, bool may_steal OVS_UNUSED)
 {
     struct netdev_bsd *dev = netdev_bsd_cast(netdev_);
     const char *name = netdev_get_name(netdev_);
@@ -731,12 +731,6 @@ netdev_bsd_send(struct netdev *netdev_, int qid OVS_UNUSED,
     }
 
     ovs_mutex_unlock(&dev->mutex);
-    if (may_steal) {
-        for (i = 0; i < cnt; i++) {
-            dp_packet_delete(pkts[i]);
-        }
-    }
-
     return error;
 }
 
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e3a0771..5512901 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1061,7 +1061,7 @@ netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats,
 
 static void
 __netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts,
-                         int cnt, bool may_steal)
+                         int cnt)
 {
     struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
     struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
@@ -1071,9 +1071,9 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts,
 
     if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
         rte_spinlock_lock(&vhost_dev->stats_lock);
-        vhost_dev->stats.tx_dropped+= cnt;
+        vhost_dev->stats.tx_dropped += cnt;
         rte_spinlock_unlock(&vhost_dev->stats_lock);
-        goto out;
+        return;
     }
 
     /* There is vHost TX single queue, So we need to lock it for TX. */
@@ -1119,20 +1119,13 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts,
     netdev_dpdk_vhost_update_tx_counters(&vhost_dev->stats, pkts, total_pkts,
                                          cnt);
     rte_spinlock_unlock(&vhost_dev->stats_lock);
-
-out:
-    if (may_steal) {
-        int i;
-
-        for (i = 0; i < total_pkts; i++) {
-            dp_packet_delete(pkts[i]);
-        }
-    }
 }
 
+/* Queue 'pkts' for transmission.  The ownership of the referred memory is
+ * taken. */
 inline static void
 dpdk_queue_pkts(struct netdev_dpdk *dev, int qid,
-               struct rte_mbuf **pkts, int cnt)
+                struct rte_mbuf **pkts, int cnt)
 {
     struct dpdk_tx_queue *txq = &dev->tx_q[qid];
     uint64_t diff_tsc;
@@ -1157,6 +1150,9 @@ dpdk_queue_pkts(struct netdev_dpdk *dev, int qid,
             dpdk_queue_flush__(dev, qid);
         }
     }
+
+    /* Clear the transferred pointers to mark them as 'taken'. */
+    memset(pkts, 0, cnt * sizeof *pkts);
 }
 
 /* Tx function. Transmit packets indefinitely */
@@ -1230,20 +1226,13 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct dp_packet **pkts,
 }
 
 static int
-netdev_dpdk_vhost_send(struct netdev *netdev, int qid OVS_UNUSED, struct dp_packet **pkts,
-                 int cnt, bool may_steal)
+netdev_dpdk_vhost_send(struct netdev *netdev, int qid OVS_UNUSED,
+                       struct dp_packet **pkts, int cnt)
 {
     if (OVS_UNLIKELY(pkts[0]->source != DPBUF_DPDK)) {
-        int i;
-
         dpdk_do_tx_copy(netdev, qid, pkts, cnt);
-        if (may_steal) {
-            for (i = 0; i < cnt; i++) {
-                dp_packet_delete(pkts[i]);
-            }
-        }
     } else {
-        __netdev_dpdk_vhost_send(netdev, pkts, cnt, may_steal);
+        __netdev_dpdk_vhost_send(netdev, pkts, cnt);
     }
     return 0;
 }
@@ -1259,17 +1248,8 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
         rte_spinlock_lock(&dev->tx_q[qid].tx_lock);
     }
 
-    if (OVS_UNLIKELY(!may_steal ||
-                     pkts[0]->source != DPBUF_DPDK)) {
-        struct netdev *netdev = &dev->up;
-
-        dpdk_do_tx_copy(netdev, qid, pkts, cnt);
-
-        if (may_steal) {
-            for (i = 0; i < cnt; i++) {
-                dp_packet_delete(pkts[i]);
-            }
-        }
+    if (OVS_UNLIKELY(!may_steal || pkts[0]->source != DPBUF_DPDK)) {
+        dpdk_do_tx_copy(&dev->up, qid, pkts, cnt);
     } else {
         int next_tx_idx = 0;
         int dropped = 0;
@@ -1281,21 +1261,20 @@ netdev_dpdk_send__(struct netdev_dpdk *dev, int qid,
                 if (next_tx_idx != i) {
                     dpdk_queue_pkts(dev, qid,
                                     (struct rte_mbuf **)&pkts[next_tx_idx],
-                                    i-next_tx_idx);
+                                    i - next_tx_idx);
                 }
 
                 VLOG_WARN_RL(&rl, "Too big size %d max_packet_len %d",
                              (int)size , dev->max_packet_len);
 
-                dp_packet_delete(pkts[i]);
                 dropped++;
                 next_tx_idx = i + 1;
             }
         }
         if (next_tx_idx != cnt) {
-           dpdk_queue_pkts(dev, qid,
+            dpdk_queue_pkts(dev, qid,
                             (struct rte_mbuf **)&pkts[next_tx_idx],
-                            cnt-next_tx_idx);
+                            cnt - next_tx_idx);
         }
 
         if (OVS_UNLIKELY(dropped)) {
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 76815c2..528ae9e 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -896,7 +896,7 @@ netdev_dummy_rxq_drain(struct netdev_rxq *rxq_)
 
 static int
 netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
-                  struct dp_packet **pkts, int cnt, bool may_steal)
+                  struct dp_packet **pkts, int cnt, bool may_steal OVS_UNUSED)
 {
     struct netdev_dummy *dev = netdev_dummy_cast(netdev);
     int error = 0;
@@ -960,12 +960,6 @@ netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
         ovs_mutex_unlock(&dev->mutex);
     }
 
-    if (may_steal) {
-        for (i = 0; i < cnt; i++) {
-            dp_packet_delete(pkts[i]);
-        }
-    }
-
     return error;
 }
 
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index e047be5..a308815 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1120,7 +1120,7 @@ netdev_linux_rxq_drain(struct netdev_rxq *rxq_)
  * expected to do additional queuing of packets. */
 static int
 netdev_linux_send(struct netdev *netdev_, int qid OVS_UNUSED,
-                  struct dp_packet **pkts, int cnt, bool may_steal)
+                  struct dp_packet **pkts, int cnt, bool may_steal OVS_UNUSED)
 {
     int i;
     int error = 0;
@@ -1200,19 +1200,12 @@ netdev_linux_send(struct netdev *netdev_, int qid OVS_UNUSED,
         i++;
     }
 
-    if (may_steal) {
-        for (i = 0; i < cnt; i++) {
-            dp_packet_delete(pkts[i]);
-        }
-    }
-
     if (error && error != EAGAIN) {
             VLOG_WARN_RL(&rl, "error sending Ethernet packet on %s: %s",
                          netdev_get_name(netdev_), ovs_strerror(error));
     }
 
     return error;
-
 }
 
 /* Registers with the poll loop to wake up from the next call to poll_block()
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index a33bb3b..5428688 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -306,6 +306,10 @@ struct netdev_class {
      * been sent anyway.
      *
      * To retain ownership of 'buffers' caller can set may_steal to false.
+     * When 'may_steal' is set to 'true', the network device will set the
+     * pointer of each packet in 'buffers' whose ownership was taken ('stolen')
+     * to NULL.  The caller retains ownership of all packets whose pointer is
+     * non-NULL on return.
      *
      * The network device is expected to maintain one or more packet
      * transmission queues, so that the caller does not ordinarily have to
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 0d56354..62ced0c 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -26,11 +26,11 @@
 
 #include "dp-packet.h"
 #include "dpif.h"
+#include "flow.h"
 #include "netlink.h"
 #include "odp-netlink.h"
 #include "odp-util.h"
 #include "packets.h"
-#include "flow.h"
 #include "unaligned.h"
 #include "util.h"
 
@@ -444,9 +444,14 @@ odp_execute_masked_set_action(struct dp_packet *packet,
 }
 
 static void
-odp_execute_sample(void *dp, struct dp_packet *packet, bool steal,
+odp_execute_actions__(void *dp, struct dp_packet **packets, int cnt,
+                      const struct nlattr *actions, size_t actions_len,
+                      odp_execute_cb dp_execute_action, bool may_steal);
+
+static void
+odp_execute_sample(void *dp, struct dp_packet **packet_p,
                    const struct nlattr *action,
-                   odp_execute_cb dp_execute_action)
+                   odp_execute_cb dp_execute_action, bool may_steal)
 {
     const struct nlattr *subactions = NULL;
     const struct nlattr *a;
@@ -458,8 +463,8 @@ odp_execute_sample(void *dp, struct dp_packet *packet, bool steal,
         switch ((enum ovs_sample_attr) type) {
         case OVS_SAMPLE_ATTR_PROBABILITY:
             if (random_uint32() >= nl_attr_get_u32(a)) {
-                if (steal) {
-                    dp_packet_delete(packet);
+                if (may_steal) {
+                    dp_packet_delete(*packet_p);
                 }
                 return;
             }
@@ -476,8 +481,9 @@ odp_execute_sample(void *dp, struct dp_packet *packet, bool steal,
         }
     }
 
-    odp_execute_actions(dp, &packet, 1, steal, nl_attr_get(subactions),
-                        nl_attr_get_size(subactions), dp_execute_action);
+    odp_execute_actions__(dp, packet_p, 1, nl_attr_get(subactions),
+                          nl_attr_get_size(subactions), dp_execute_action,
+                          may_steal);
 }
 
 static bool
@@ -514,10 +520,10 @@ requires_datapath_assistance(const struct nlattr *a)
     return false;
 }
 
-void
-odp_execute_actions(void *dp, struct dp_packet **packets, int cnt, bool steal,
-                    const struct nlattr *actions, size_t actions_len,
-                    odp_execute_cb dp_execute_action)
+static void
+odp_execute_actions__(void *dp, struct dp_packet **packets, int cnt,
+                      const struct nlattr *actions, size_t actions_len,
+                      odp_execute_cb dp_execute_action, bool may_steal)
 {
     const struct nlattr *a;
     unsigned int left;
@@ -530,16 +536,13 @@ odp_execute_actions(void *dp, struct dp_packet **packets, int cnt, bool steal,
         if (requires_datapath_assistance(a)) {
             if (dp_execute_action) {
                 /* Allow 'dp_execute_action' to steal the packet data if we do
-                 * not need it any more. */
-                bool may_steal = steal && last_action;
-
-                dp_execute_action(dp, packets, cnt, a, may_steal);
-
-                if (last_action) {
-                    /* We do not need to free the packets. dp_execute_actions()
-                     * has stolen them */
-                    return;
-                }
+                 * not need it any more.  As a precaution, packets actually
+                 * stolen have their pointers set to NULL, so that we can
+                 * catch bugs where the stolen packet is referenced afterwards.
+                 * 'dp_execute_action' may also effectively drop packets by
+                 * returning a count smaller than 'cnt' parameter. */
+                cnt = dp_execute_action(dp, packets, cnt, a,
+                                        may_steal && last_action);
             }
             continue;
         }
@@ -613,14 +616,8 @@ odp_execute_actions(void *dp, struct dp_packet **packets, int cnt, bool steal,
 
         case OVS_ACTION_ATTR_SAMPLE:
             for (i = 0; i < cnt; i++) {
-                odp_execute_sample(dp, packets[i], steal && last_action, a,
-                                   dp_execute_action);
-            }
-
-            if (last_action) {
-                /* We do not need to free the packets. odp_execute_sample() has
-                 * stolen them*/
-                return;
+                odp_execute_sample(dp, &packets[i], a, dp_execute_action,
+                                   may_steal && last_action);
             }
             break;
 
@@ -639,10 +636,21 @@ odp_execute_actions(void *dp, struct dp_packet **packets, int cnt, bool steal,
             OVS_NOT_REACHED();
         }
     }
+}
 
+void
+odp_execute_actions(void *dp, struct dp_packet **packets, int cnt, bool steal,
+                    const struct nlattr *actions, size_t actions_len,
+                    odp_execute_cb dp_execute_action)
+{
+    odp_execute_actions__(dp, packets, cnt, actions, actions_len,
+                          dp_execute_action, steal);
     if (steal) {
-        for (i = 0; i < cnt; i++) {
-            dp_packet_delete(packets[i]);
+        for (int i = 0; i < cnt; i++) {
+            /* Packets may already have been taken, e.g. by output actions. */
+            if (packets[i]) {
+                dp_packet_delete(packets[i]);
+            }
         }
     }
 }
diff --git a/lib/odp-execute.h b/lib/odp-execute.h
index c602bb4..c9cb43f 100644
--- a/lib/odp-execute.h
+++ b/lib/odp-execute.h
@@ -27,13 +27,15 @@ struct nlattr;
 struct dp_packet;
 struct pkt_metadata;
 
-typedef void (*odp_execute_cb)(void *dp, struct dp_packet **packets, int cnt,
-                               const struct nlattr *action, bool may_steal);
+/* Remaining packets are dropped if the callback returns a value < 'cnt'. */
+typedef int (*odp_execute_cb)(void *dp, struct dp_packet **packets, int cnt,
+                              const struct nlattr *action, bool may_steal);
 
 /* Actions that need to be executed in the context of a datapath are handed
- * to 'dp_execute_action', if non-NULL.  Currently this is called only for
- * actions OVS_ACTION_ATTR_OUTPUT and OVS_ACTION_ATTR_USERSPACE so
- * 'dp_execute_action' needs to handle only these. */
+ * to 'dp_execute_action', if non-NULL.
+ * The caller relinquishes ownership of the 'packets' if 'steal' is 'true'.
+ * In this case the packet pointers in 'packets' will be set to NULL to
+ * enforce the ownership transfer. */
 void odp_execute_actions(void *dp, struct dp_packet **packets, int cnt,
                          bool steal,
                          const struct nlattr *actions, size_t actions_len,
diff --git a/lib/packets.c b/lib/packets.c
index 701a5ec..2481589 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -350,20 +350,16 @@ pop_mpls(struct dp_packet *packet, ovs_be16 ethtype)
 const char *
 eth_from_hex(const char *hex, struct dp_packet **packetp)
 {
-    struct dp_packet *packet;
-
     /* Use 2 bytes of headroom to 32-bit align the L3 header. */
-    packet = *packetp = dp_packet_new_with_headroom(strlen(hex) / 2, 2);
+    *packetp = dp_packet_new_with_headroom(strlen(hex) / 2, 2);
 
-    if (dp_packet_put_hex(packet, hex, NULL)[0] != '\0') {
-        dp_packet_delete(packet);
-        *packetp = NULL;
+    if (dp_packet_put_hex(*packetp, hex, NULL)[0] != '\0') {
+        dp_packet_delete(*packetp);
         return "Trailing garbage in packet data";
     }
 
-    if (dp_packet_size(packet) < ETH_HEADER_LEN) {
-        dp_packet_delete(packet);
-        *packetp = NULL;
+    if (dp_packet_size(*packetp) < ETH_HEADER_LEN) {
+        dp_packet_delete(*packetp);
         return "Packet data too short for Ethernet";
     }
 
-- 
2.1.4




More information about the dev mailing list