On 08.01.2020 21:33, Ben Pfaff wrote: > 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.
Thanks Ben for review! I'll take another look at this patch in a next couple of days. > > 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. */ > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev