[ovs-dev] [PATCH v10 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode

[Ben Pfaff]

On Wed, Jan 08, 2020 at 10:48:04AM +0530, Vishal Deep Ajmera wrote:
> Problem:
> --------
> In OVS-DPDK, flows with output over a bond interface of type “balance-tcp”
> (using a hash on TCP/UDP 5-tuple) get translated by the ofproto layer into
> "HASH" and "RECIRC" datapath actions. After recirculation, the packet is
> forwarded to the bond member port based on 8-bits of the datapath hash
> value computed through dp_hash. This causes performance degradation in the
> following ways:

Thanks for the patch!

I have a few minor stylistic suggestions, see below.

I'd like to hear Ilya's opinion on this.

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 78f9cf928de2..c3a54e96346e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -378,11 +378,12 @@ struct dp_netdev {
 
     struct conntrack *conntrack;
     struct pmd_auto_lb pmd_alb;
+
     /* Bonds.
      *
      * Any lookup into 'bonds' requires taking 'bond_mutex'. */
     struct ovs_mutex bond_mutex;
-    struct hmap bonds;
+    struct hmap bonds;          /* Contains "struct tx_bond"s. */
 };
 
 static void meter_lock(const struct dp_netdev *dp, uint32_t meter_id)
@@ -5581,10 +5582,7 @@ pmd_free_cached_bonds(struct dp_netdev_pmd_thread *pmd)
 
     /* Remove bonds from pmd which no longer exists. */
     HMAP_FOR_EACH_SAFE (bond, next, node, &pmd->bond_cache) {
-        struct tx_bond *tx = NULL;
-
-        tx = tx_bond_lookup(&pmd->tx_bonds, bond->bond_id);
-        if (!tx) {
+        if (!tx_bond_lookup(&pmd->tx_bonds, bond->bond_id)) {
             /* Bond no longer exist. Delete it from pmd. */
             hmap_remove(&pmd->bond_cache, &bond->node);
             free(bond);
@@ -5601,10 +5599,11 @@ pmd_load_cached_bonds(struct dp_netdev_pmd_thread *pmd)
     pmd_free_cached_bonds(pmd);
     hmap_shrink(&pmd->bond_cache);
 
-    struct tx_bond *tx_bond, *tx_bond_cached;
+    struct tx_bond *tx_bond;
     HMAP_FOR_EACH (tx_bond, node, &pmd->tx_bonds) {
         /* Check if bond already exist on pmd. */
-        tx_bond_cached = tx_bond_lookup(&pmd->bond_cache, tx_bond->bond_id);
+        struct tx_bond *tx_bond_cached
+            = tx_bond_lookup(&pmd->bond_cache, tx_bond->bond_id);
 
         if (!tx_bond_cached) {
             /* Create new bond entry in cache. */