[ovs-dev] [PATCH 3/3] dpif-netdev: Avoid introducing of port->need_reconfigure.

Daniele Di Proietto diproiettod at vmware.com
Sat Dec 3 02:27:06 UTC 2016






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

>'need_reconfigure' is actually local variable for the
>'reconfigure_datapath()'. It's better to keep common
>structures clean.
>
>Signed-off-by: Ilya Maximets <i.maximets at samsung.com>

That's true, I actually coded it using a hmapx at the beginning,
but it felt a little bit more complicated.  I actually did the same
with pmd->need_reload, and felt that it removed a lot of code.

I see that the lines of codes are the same, but I still slightly
prefer keeping need_reconfigure.  That's just my taste though...

What do you think?

Thanks,

Daniele

>---
> lib/dpif-netdev.c | 40 ++++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 8af2811..3b56ba6 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -305,7 +305,6 @@ struct dp_netdev_port {
>     struct ovs_mutex txq_used_mutex;
>     char *type;                 /* Port type as requested by user. */
>     char *rxq_affinity_list;    /* Requested affinity of rx queues. */
>-    bool need_reconfigure;      /* True if we should reconfigure netdev. */
> };
> 
> /* Contained by struct dp_netdev_flow's 'stats' member.  */
>@@ -605,7 +604,7 @@ static void dp_netdev_add_rxq_to_pmd(struct dp_netdev_pmd_thread *pmd,
> static void dp_netdev_del_rxq_from_pmd(struct dp_netdev_pmd_thread *pmd,
>                                        struct rxq_poll *poll)
>     OVS_REQUIRES(pmd->port_mutex);
>-static void reconfigure_datapath(struct dp_netdev *dp)
>+static void reconfigure_datapath(struct dp_netdev *dp, struct hmapx *new_ports)
>     OVS_REQUIRES(dp->port_mutex);
> static bool dp_netdev_pmd_try_ref(struct dp_netdev_pmd_thread *pmd);
> static void dp_netdev_pmd_unref(struct dp_netdev_pmd_thread *pmd);
>@@ -1321,7 +1320,6 @@ port_create(const char *devname, const char *type,
>     port->netdev = netdev;
>     port->type = xstrdup(type);
>     port->sf = sf;
>-    port->need_reconfigure = true;
>     ovs_mutex_init(&port->txq_used_mutex);
> 
>     *portp = port;
>@@ -1339,6 +1337,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
>     OVS_REQUIRES(dp->port_mutex)
> {
>     struct dp_netdev_port *port;
>+    struct hmapx new_ports = HMAPX_INITIALIZER(&new_ports);
>     int error;
> 
>     /* Reject devices already in 'dp'. */
>@@ -1354,7 +1353,9 @@ do_add_port(struct dp_netdev *dp, const char *devname, const char *type,
>     hmap_insert(&dp->ports, &port->node, hash_port_no(port_no));
>     seq_change(dp->port_seq);
> 
>-    reconfigure_datapath(dp);
>+    hmapx_add(&new_ports, port);
>+    reconfigure_datapath(dp, &new_ports);
>+    hmapx_destroy(&new_ports);
> 
>     return 0;
> }
>@@ -1505,7 +1506,7 @@ do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port,
>     seq_change(dp->port_seq);
> 
>     if (port_in_datapath) {
>-        reconfigure_datapath(dp);
>+        reconfigure_datapath(dp, NULL);
>     }
> 
>     port_destroy(port);
>@@ -2928,8 +2929,6 @@ port_reconfigure(struct dp_netdev_port *port)
>     struct netdev *netdev = port->netdev;
>     int i, err;
> 
>-    port->need_reconfigure = false;
>-
>     /* Closes the existing 'rxq's. */
>     for (i = 0; i < port->n_rxq; i++) {
>         netdev_rxq_close(port->rxqs[i].rx);
>@@ -3179,11 +3178,13 @@ reload_all_pmds(struct dp_netdev *dp)
>  * This creates and destroys pmd threads, reconfigures ports, opens their
>  * rxqs and assigns all rxqs/txqs to pmd threads. */
> static void
>-reconfigure_datapath(struct dp_netdev *dp)
>+reconfigure_datapath(struct dp_netdev *dp, struct hmapx *new_ports)
>     OVS_REQUIRES(dp->port_mutex)
> {
>     struct dp_netdev_pmd_thread *pmd;
>     struct dp_netdev_port *port;
>+    struct hmapx to_reconfigure = HMAPX_INITIALIZER(&to_reconfigure);
>+    struct hmapx_node *node;
>     int wanted_txqs;
> 
>     dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
>@@ -3207,8 +3208,9 @@ reconfigure_datapath(struct dp_netdev *dp)
>      * 'port->reconfigure', because netdev_is_reconf_required() can change at
>      * any time. */
>     HMAP_FOR_EACH(port, node, &dp->ports) {
>-        if (netdev_is_reconf_required(port->netdev)) {
>-            port->need_reconfigure = true;
>+        if (netdev_is_reconf_required(port->netdev)
>+            || (new_ports && hmapx_contains(new_ports, port))) {
>+            hmapx_add(&to_reconfigure, port);
>         }
>     }
> 
>@@ -3222,14 +3224,14 @@ reconfigure_datapath(struct dp_netdev *dp)
>         HMAP_FOR_EACH_SAFE(poll, poll_next, node, &pmd->poll_list) {
>             port = poll->rxq->port;
> 
>-            if (port->need_reconfigure
>+            if (hmapx_contains(&to_reconfigure, port)
>                 || dp_netdev_lookup_port(dp, port->port_no) != port) {
>                 dp_netdev_del_rxq_from_pmd(pmd, poll);
>             }
>         }
>         HMAP_FOR_EACH_SAFE(tx, tx_next, node, &pmd->tx_ports) {
>             port = tx->port;
>-            if (port->need_reconfigure
>+            if (hmapx_contains(&to_reconfigure, port)
>                 || dp_netdev_lookup_port(dp, port->port_no) != port) {
>                 dp_netdev_del_port_tx_from_pmd(pmd, tx);
>             }
>@@ -3247,17 +3249,15 @@ reconfigure_datapath(struct dp_netdev *dp)
>     /* We only reconfigure the ports that we determined above, because they're
>      * 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) {
>-        if (!port->need_reconfigure) {
>-            continue;
>-        }
>-
>+    HMAPX_FOR_EACH (node, &to_reconfigure) {
>+        port = (struct dp_netdev_port *) node->data;
>         if (port_reconfigure(port)) {
>             do_del_port(dp, port, false);
>         } else {
>             port->dynamic_txqs = netdev_n_txq(port->netdev) < wanted_txqs;
>         }
>     }
>+    hmapx_destroy(&to_reconfigure);
> 
>     /* Step 4: Compute new rxq scheduling.  We don't touch the pmd threads
>      * for now, we just update the 'pmd' pointer in each rxq to point to the
>@@ -3378,7 +3378,7 @@ dpif_netdev_run(struct dpif *dpif)
>     }
> 
>     if (dp_netdev_is_reconf_required(dp) || ports_require_restart(dp)) {
>-        reconfigure_datapath(dp);
>+        reconfigure_datapath(dp, NULL);
>     }
>     ovs_mutex_unlock(&dp->port_mutex);
> 
>@@ -4869,12 +4869,12 @@ dpif_dummy_change_port_number(struct unixctl_conn *conn, int argc OVS_UNUSED,
> 
>     /* Remove port. */
>     hmap_remove(&dp->ports, &port->node);
>-    reconfigure_datapath(dp);
>+    reconfigure_datapath(dp, NULL);
> 
>     /* Reinsert with new port number. */
>     port->port_no = port_no;
>     hmap_insert(&dp->ports, &port->node, hash_port_no(port_no));
>-    reconfigure_datapath(dp);
>+    reconfigure_datapath(dp, NULL);
> 
>     seq_change(dp->port_seq);
>     unixctl_command_reply(conn, NULL);
>-- 
>2.7.4
>


More information about the dev mailing list