diff mbox series

[ovs-dev,RFC,v2,1/5] dpif-netdev: Refactor dp_netdev_flow_offload_put()

Message ID 20200518192731.30804-2-sriharsha.basavapatna@broadcom.com
State New
Headers show
Series RFC for Partial action offload | expand

Commit Message

Sriharsha Basavapatna May 18, 2020, 7:27 p.m. UTC
This patch refactors dp_netdev_flow_offload_put() to prepare for
more changes to support partial action offload, in subsequent patches.

- Add a wrapper function to allocate flow-mark.
- Move netdev_ports_get() to before flow-mark allocation.

Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
---
 lib/dpif-netdev.c | 71 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 26 deletions(-)

Comments

Eli Britstein May 26, 2020, 1:42 p.m. UTC | #1
On 5/18/2020 10:27 PM, Sriharsha Basavapatna wrote:
> This patch refactors dp_netdev_flow_offload_put() to prepare for
> more changes to support partial action offload, in subsequent patches.
>
> - Add a wrapper function to allocate flow-mark.
> - Move netdev_ports_get() to before flow-mark allocation.
Moving netdev_ports_get position is a change of logic, not just 
refactor. It should be in a separate commit.
>
> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
> ---
>   lib/dpif-netdev.c | 71 ++++++++++++++++++++++++++++++-----------------
>   1 file changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index ea7b187b2..781b233f4 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2357,6 +2357,43 @@ dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload)
>       return mark_to_flow_disassociate(offload->pmd, offload->flow);
>   }
>   
> +static int
> +dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification,
> +                          uint32_t *markp)
> +{
> +    uint32_t mark;
> +
> +    if (modification) {
> +        mark = flow->mark;
> +        ovs_assert(mark != INVALID_FLOW_MARK);
> +        *markp = mark;
> +        return 0;
> +    }
> +
> +    /*
> +     * If a mega flow has already been offloaded (from other PMD
> +     * instances), do not offload it again.
> +     */
> +    mark = megaflow_to_mark_find(&flow->mega_ufid);
> +    if (mark != INVALID_FLOW_MARK) {
> +        VLOG_DBG("Flow has already been offloaded with mark %u\n", mark);
> +        if (flow->mark != INVALID_FLOW_MARK) {
> +            ovs_assert(flow->mark == mark);
> +        } else {
> +            mark_to_flow_associate(mark, flow);
> +        }
> +        return 1;
> +    }
> +
> +    mark = netdev_offload_flow_mark_alloc();
> +    if (mark == INVALID_FLOW_MARK) {
> +        VLOG_ERR("Failed to allocate flow mark!\n");
> +    }
> +
> +    *markp = mark;
> +    return 0;
> +}
> +
>   /*
>    * There are two flow offload operations here: addition and modification.
>    *
> @@ -2385,36 +2422,18 @@ dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
>           return -1;
>       }
>   
> -    if (modification) {
> -        mark = flow->mark;
> -        ovs_assert(mark != INVALID_FLOW_MARK);
> -    } else {
> -        /*
> -         * If a mega flow has already been offloaded (from other PMD
> -         * instances), do not offload it again.
> -         */
> -        mark = megaflow_to_mark_find(&flow->mega_ufid);
> -        if (mark != INVALID_FLOW_MARK) {
> -            VLOG_DBG("Flow has already been offloaded with mark %u\n", mark);
> -            if (flow->mark != INVALID_FLOW_MARK) {
> -                ovs_assert(flow->mark == mark);
> -            } else {
> -                mark_to_flow_associate(mark, flow);
> -            }
> -            return 0;
> -        }
> +    port = netdev_ports_get(in_port, dpif_type_str);
> +    if (!port) {
> +        return -1;
> +    }
>   
> -        mark = netdev_offload_flow_mark_alloc();
> -        if (mark == INVALID_FLOW_MARK) {
> -            VLOG_ERR("Failed to allocate flow mark!\n");
> -        }
> +    if (dp_netdev_alloc_flow_mark(flow, modification, &mark)) {
> +            /* flow already offloaded */
> +            netdev_close(port);
> +            return 0;
>       }
>       info.flow_mark = mark;
>   
> -    port = netdev_ports_get(in_port, dpif_type_str);
> -    if (!port) {
> -        goto err_free;
> -    }
>       /* Taking a global 'port_mutex' to fulfill thread safety restrictions for
>        * the netdev-offload-dpdk module. */
>       ovs_mutex_lock(&pmd->dp->port_mutex);
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ea7b187b2..781b233f4 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2357,6 +2357,43 @@  dp_netdev_flow_offload_del(struct dp_flow_offload_item *offload)
     return mark_to_flow_disassociate(offload->pmd, offload->flow);
 }
 
+static int
+dp_netdev_alloc_flow_mark(struct dp_netdev_flow *flow, bool modification,
+                          uint32_t *markp)
+{
+    uint32_t mark;
+
+    if (modification) {
+        mark = flow->mark;
+        ovs_assert(mark != INVALID_FLOW_MARK);
+        *markp = mark;
+        return 0;
+    }
+
+    /*
+     * If a mega flow has already been offloaded (from other PMD
+     * instances), do not offload it again.
+     */
+    mark = megaflow_to_mark_find(&flow->mega_ufid);
+    if (mark != INVALID_FLOW_MARK) {
+        VLOG_DBG("Flow has already been offloaded with mark %u\n", mark);
+        if (flow->mark != INVALID_FLOW_MARK) {
+            ovs_assert(flow->mark == mark);
+        } else {
+            mark_to_flow_associate(mark, flow);
+        }
+        return 1;
+    }
+
+    mark = netdev_offload_flow_mark_alloc();
+    if (mark == INVALID_FLOW_MARK) {
+        VLOG_ERR("Failed to allocate flow mark!\n");
+    }
+
+    *markp = mark;
+    return 0;
+}
+
 /*
  * There are two flow offload operations here: addition and modification.
  *
@@ -2385,36 +2422,18 @@  dp_netdev_flow_offload_put(struct dp_flow_offload_item *offload)
         return -1;
     }
 
-    if (modification) {
-        mark = flow->mark;
-        ovs_assert(mark != INVALID_FLOW_MARK);
-    } else {
-        /*
-         * If a mega flow has already been offloaded (from other PMD
-         * instances), do not offload it again.
-         */
-        mark = megaflow_to_mark_find(&flow->mega_ufid);
-        if (mark != INVALID_FLOW_MARK) {
-            VLOG_DBG("Flow has already been offloaded with mark %u\n", mark);
-            if (flow->mark != INVALID_FLOW_MARK) {
-                ovs_assert(flow->mark == mark);
-            } else {
-                mark_to_flow_associate(mark, flow);
-            }
-            return 0;
-        }
+    port = netdev_ports_get(in_port, dpif_type_str);
+    if (!port) {
+        return -1;
+    }
 
-        mark = netdev_offload_flow_mark_alloc();
-        if (mark == INVALID_FLOW_MARK) {
-            VLOG_ERR("Failed to allocate flow mark!\n");
-        }
+    if (dp_netdev_alloc_flow_mark(flow, modification, &mark)) {
+            /* flow already offloaded */
+            netdev_close(port);
+            return 0;
     }
     info.flow_mark = mark;
 
-    port = netdev_ports_get(in_port, dpif_type_str);
-    if (!port) {
-        goto err_free;
-    }
     /* Taking a global 'port_mutex' to fulfill thread safety restrictions for
      * the netdev-offload-dpdk module. */
     ovs_mutex_lock(&pmd->dp->port_mutex);