diff mbox

[ovs-dev,v4,2/3] dpif-netdev: Avoid port's reconfiguration on pmd-cpu-mask changes.

Message ID 1500641502-31926-3-git-send-email-i.maximets@samsung.com
State Rejected
Delegated to: Darrell Ball
Headers show

Commit Message

Ilya Maximets July 21, 2017, 12:51 p.m. UTC 
Reconfiguration of HW NICs may lead to packet drops.
In current model all physical ports will be reconfigured each
time number of PMD threads changed. Since we not stopping
threads on pmd-cpu-mask changes, this patch will help to further
decrease port's downtime by setting the maximum possible number
of wanted tx queues to avoid unnecessary reconfigurations.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Tested-by: Ian Stokes <ian.stokes@intel.com>
Acked-by: Ian Stokes <ian.stokes@intel.com>
---
 lib/dpif-netdev.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

Comments

Darrell Ball July 23, 2017, 5:25 p.m. UTC | #1 
This patch was discussed here already.

https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336145.html

We think it needs more work.


-----Original Message-----
From: Ilya Maximets <i.maximets@samsung.com>
Date: Friday, July 21, 2017 at 5:51 AM
To: "ovs-dev@openvswitch.org" <ovs-dev@openvswitch.org>, Darrell Ball <dball@vmware.com>
Cc: Heetae Ahn <heetae82.ahn@samsung.com>, Daniele Di Proietto <diproiettod@ovn.org>, Ben Pfaff <blp@ovn.org>, Pravin Shelar <pshelar@ovn.org>, Ciara Loftus <ciara.loftus@intel.com>, Ian Stokes <ian.stokes@intel.com>, Kevin Traynor <ktraynor@redhat.com>, Cian Ferriter <cian.ferriter@intel.com>, Mark Kavanagh <mark.b.kavanagh@intel.com>, Ilya Maximets <i.maximets@samsung.com>
Subject: [PATCH v4 2/3] dpif-netdev: Avoid port's reconfiguration on pmd-cpu-mask changes.

    Reconfiguration of HW NICs may lead to packet drops.
    In current model all physical ports will be reconfigured each
    time number of PMD threads changed. Since we not stopping
    threads on pmd-cpu-mask changes, this patch will help to further
    decrease port's downtime by setting the maximum possible number
    of wanted tx queues to avoid unnecessary reconfigurations.
    
    Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
    Tested-by: Ian Stokes <ian.stokes@intel.com>
    Acked-by: Ian Stokes <ian.stokes@intel.com>
    ---
     lib/dpif-netdev.c | 26 +++++++++++++++++++++-----
     1 file changed, 21 insertions(+), 5 deletions(-)
    
    diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
    index 4de3678..a1e8c56 100644
    --- a/lib/dpif-netdev.c
    +++ b/lib/dpif-netdev.c
    @@ -3457,7 +3457,7 @@ reconfigure_datapath(struct dp_netdev *dp)
     {
         struct dp_netdev_pmd_thread *pmd;
         struct dp_netdev_port *port;
    -    int wanted_txqs;
    +    int needed_txqs, wanted_txqs;
     
         dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
     
    @@ -3465,7 +3465,15 @@ reconfigure_datapath(struct dp_netdev *dp)
          * on the system and the user configuration. */
         reconfigure_pmd_threads(dp);
     
    -    wanted_txqs = cmap_count(&dp->poll_threads);
    +    /* We need 1 Tx queue for each thread to avoid locking, but we will try
    +     * to allocate the maximum possible value to minimize the number of port
    +     * reconfigurations. */
    +    needed_txqs = cmap_count(&dp->poll_threads);
    +    /* (n_cores + 1) is the maximum that we might need to have.
    +     * Additional queue is for non-PMD threads. */
    +    wanted_txqs = ovs_numa_get_n_cores();
    +    ovs_assert(wanted_txqs != OVS_CORE_UNSPEC);
    +    wanted_txqs++;
     
         /* The number of pmd threads might have changed, or a port can be new:
          * adjust the txqs. */
    @@ -3478,9 +3486,17 @@ reconfigure_datapath(struct dp_netdev *dp)
     
         /* Check for all the ports that need reconfiguration.  We cache this in
          * 'port->need_reconfigure', because netdev_is_reconf_required() can
    -     * change at any time. */
    +     * change at any time.
    +     * Also mark for reconfiguration all ports which will likely change their
    +     * 'dynamic_txqs' parameter. It's required to stop using them before
    +     * changing this setting and it's simpler to mark ports here and allow
    +     * 'pmd_remove_stale_ports' to remove them from threads. There will be
    +     * no actual reconfiguration in 'port_reconfigure' because it's
    +     * unnecessary.  */
         HMAP_FOR_EACH (port, node, &dp->ports) {
    -        if (netdev_is_reconf_required(port->netdev)) {
    +        if (netdev_is_reconf_required(port->netdev)
    +            || (port->dynamic_txqs
    +                != (netdev_n_txq(port->netdev) < needed_txqs))) {
                 port->need_reconfigure = true;
             }
         }
    @@ -3515,7 +3531,7 @@ reconfigure_datapath(struct dp_netdev *dp)
                 seq_change(dp->port_seq);
                 port_destroy(port);
             } else {
    -            port->dynamic_txqs = netdev_n_txq(port->netdev) < wanted_txqs;
    +            port->dynamic_txqs = netdev_n_txq(port->netdev) < needed_txqs;
             }
         }
     
    -- 
    2.7.4
diff mbox

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4de3678..a1e8c56 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3457,7 +3457,7 @@  reconfigure_datapath(struct dp_netdev *dp)
 {
     struct dp_netdev_pmd_thread *pmd;
     struct dp_netdev_port *port;
-    int wanted_txqs;
+    int needed_txqs, wanted_txqs;
 
     dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq);
 
@@ -3465,7 +3465,15 @@  reconfigure_datapath(struct dp_netdev *dp)
      * on the system and the user configuration. */
     reconfigure_pmd_threads(dp);
 
-    wanted_txqs = cmap_count(&dp->poll_threads);
+    /* We need 1 Tx queue for each thread to avoid locking, but we will try
+     * to allocate the maximum possible value to minimize the number of port
+     * reconfigurations. */
+    needed_txqs = cmap_count(&dp->poll_threads);
+    /* (n_cores + 1) is the maximum that we might need to have.
+     * Additional queue is for non-PMD threads. */
+    wanted_txqs = ovs_numa_get_n_cores();
+    ovs_assert(wanted_txqs != OVS_CORE_UNSPEC);
+    wanted_txqs++;
 
     /* The number of pmd threads might have changed, or a port can be new:
      * adjust the txqs. */
@@ -3478,9 +3486,17 @@  reconfigure_datapath(struct dp_netdev *dp)
 
     /* Check for all the ports that need reconfiguration.  We cache this in
      * 'port->need_reconfigure', because netdev_is_reconf_required() can
-     * change at any time. */
+     * change at any time.
+     * Also mark for reconfiguration all ports which will likely change their
+     * 'dynamic_txqs' parameter. It's required to stop using them before
+     * changing this setting and it's simpler to mark ports here and allow
+     * 'pmd_remove_stale_ports' to remove them from threads. There will be
+     * no actual reconfiguration in 'port_reconfigure' because it's
+     * unnecessary.  */
     HMAP_FOR_EACH (port, node, &dp->ports) {
-        if (netdev_is_reconf_required(port->netdev)) {
+        if (netdev_is_reconf_required(port->netdev)
+            || (port->dynamic_txqs
+                != (netdev_n_txq(port->netdev) < needed_txqs))) {
             port->need_reconfigure = true;
         }
     }
@@ -3515,7 +3531,7 @@  reconfigure_datapath(struct dp_netdev *dp)
             seq_change(dp->port_seq);
             port_destroy(port);
         } else {
-            port->dynamic_txqs = netdev_n_txq(port->netdev) < wanted_txqs;
+            port->dynamic_txqs = netdev_n_txq(port->netdev) < needed_txqs;
         }
     }