[ovs-dev] [PATCH 1/3] dpif-netdev: Use do_del_port in reconfigure_datapath.

Daniele Di Proietto diproiettod at vmware.com
Sat Dec 3 02:21:09 UTC 2016






On 02/12/2016 06:48, "Ilya Maximets" <i.maximets at samsung.com> wrote:

>This reduces code duplication. Additionally dp_netdev_free
>will not reconfigure datapath on each removed port.
>
>Signed-off-by: Ilya Maximets <i.maximets at samsung.com>

I like reducing code duplication, but I'm not sure I like that
do_del_port() calls reconfigure_datapath() and reconfigure_datapath()
calls do_del_port().  Do you think we could avoid this "recursion"?

Thanks,

Daniele

>---
> lib/dpif-netdev.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index f6552da..8e9a623 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -561,7 +561,8 @@ static void dp_netdev_free(struct dp_netdev *)
> static int do_add_port(struct dp_netdev *dp, const char *devname,
>                        const char *type, odp_port_t port_no)
>     OVS_REQUIRES(dp->port_mutex);
>-static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *)
>+static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *,
>+                        bool port_in_datapath)
>     OVS_REQUIRES(dp->port_mutex);
> static int dpif_netdev_open(const struct dpif_class *, const char *name,
>                             bool create, struct dpif **);
>@@ -1150,12 +1151,13 @@ dp_netdev_free(struct dp_netdev *dp)
> 
>     shash_find_and_delete(&dp_netdevs, dp->name);
> 
>+    dp_netdev_destroy_all_pmds(dp, true);
>+
>     ovs_mutex_lock(&dp->port_mutex);
>     HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) {
>-        do_del_port(dp, port);
>+        do_del_port(dp, port, false);
>     }
>     ovs_mutex_unlock(&dp->port_mutex);
>-    dp_netdev_destroy_all_pmds(dp, true);
>     cmap_destroy(&dp->poll_threads);
> 
>     ovs_mutex_destroy(&dp->non_pmd_mutex);
>@@ -1399,7 +1401,7 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t port_no)
> 
>         error = get_port_by_number(dp, port_no, &port);
>         if (!error) {
>-            do_del_port(dp, port);
>+            do_del_port(dp, port, true);
>         }
>     }
>     ovs_mutex_unlock(&dp->port_mutex);
>@@ -1495,13 +1497,16 @@ has_pmd_port(struct dp_netdev *dp)
> }
> 
> static void
>-do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port)
>+do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port,
>+            bool port_in_datapath)
>     OVS_REQUIRES(dp->port_mutex)
> {
>     hmap_remove(&dp->ports, &port->node);
>     seq_change(dp->port_seq);
> 
>-    reconfigure_datapath(dp);
>+    if (port_in_datapath) {
>+        reconfigure_datapath(dp);
>+    }
> 
>     port_destroy(port);
> }
>@@ -3243,17 +3248,12 @@ reconfigure_datapath(struct dp_netdev *dp)
>      * not being used by any pmd thread at the moment.  If a port fails to
>      * reconfigure we remove it from the datapath. */
>     HMAP_FOR_EACH(port, node, &dp->ports) {
>-        int err;
>-
>         if (!port->need_reconfigure) {
>             continue;
>         }
> 
>-        err = port_reconfigure(port);
>-        if (err) {
>-            hmap_remove(&dp->ports, &port->node);
>-            seq_change(dp->port_seq);
>-            port_destroy(port);
>+        if (port_reconfigure(port)) {
>+            do_del_port(dp, port, false);
>         } else {
>             port->dynamic_txqs = netdev_n_txq(port->netdev) < wanted_txqs;
>         }
>-- 
>2.7.4
>


More information about the dev mailing list