[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