[ovs-dev] [PATCH v2] dpif-netdev: Free packets on TUNNEL_PUSH if should_steal.

Ilya Maximets i.maximets at samsung.com
Thu May 24 09:51:21 UTC 2018


Unconditional return may cause packet leak in case of
'should_steal == true'.

Additionally, removed redundant checking for depth level.

CC: Sugesh Chandran <sugesh.chandran at intel.com>
Fixes: 7c12dfc527a5 ("tunneling: Avoid datapath-recirc by
                      combining recirc actions at xlate.")
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
---

version 2:
    - Rebase on current master:
      * 'may_steal' --> 'should_steal'
      * Removed comment about broken 'may_steal' concept,
        because it's meaning was changed by recent patch
        from Darrell.

P.S. Version 1 should be used for stable branches:
    https://patchwork.ozlabs.org/patch/913576/

 lib/dpif-netdev.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 68f2a29..98b109d 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5673,12 +5673,16 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
         break;
 
     case OVS_ACTION_ATTR_TUNNEL_PUSH:
-        if (*depth < MAX_RECIRC_DEPTH) {
-            dp_packet_batch_apply_cutlen(packets_);
-            push_tnl_action(pmd, a, packets_);
-            return;
+        if (should_steal) {
+            /* We're requested to push tunnel header, but also we need to take
+             * the ownership of these packets. Thus, we can avoid performing
+             * the action, because the caller will not use the result anyway.
+             * Just break to free the batch. */
+            break;
         }
-        break;
+        dp_packet_batch_apply_cutlen(packets_);
+        push_tnl_action(pmd, a, packets_);
+        return;
 
     case OVS_ACTION_ATTR_TUNNEL_POP:
         if (*depth < MAX_RECIRC_DEPTH) {
-- 
2.7.4



More information about the dev mailing list