diff mbox

[ovs-dev,v2,2/3] bond: Unify hash functions in hash action and entry lookup.

Message ID 1500979599-20441-3-git-send-email-i.maximets@samsung.com
State Accepted
Headers show

Commit Message

Ilya Maximets July 25, 2017, 10:46 a.m. UTC 
'lookup_bond_entry' currently uses 'flow_hash_symmetric_l4' while
OVS_ACTION_ATTR_HASH uses 'flow_hash_5tuple'. This may lead to
inconsistency in slave choosing for the new flows.  In general,
there is no point to unify hash functions, because it's not
required for correct work, but it's logically wrong to use
different hash functions there.

Unfortunately we're not able to use RSS hash here, because we have
no packet at this point, but we may reduce inconsistency by using
'flow_hash_5tuple' instead of 'flow_hash_symmetric_l4' because
symmetric quality is not needed.

'flow_hash_symmetric_l4' was used previously just because there
was no other implemented hash function at the moment and L2
fields was additionally involved in hash calculation. Now we
have 5tuple hash and L2 not used anymore, so, we may replace the
old function.

'flow_hash_5tuple' is preferable solution because it in 2 - 8 times
(depending on the flow) faster than symmetric function.
So, this change will also speed up handling of the new flows and
statistics accounting.

Additionally function 'bond_hash_tcp()' was removed for the reasons
of code simplification and possible additional speed up.

Co-authored-by: Andy Zhou <azhou@ovn.org>
Signed-off-by: Andy Zhou <azhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 ofproto/bond.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)
diff mbox

Patch

diff --git a/ofproto/bond.c b/ofproto/bond.c
index cb25a1d..e4d4b65 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -177,8 +177,6 @@  static void bond_choose_active_slave(struct bond *)
     OVS_REQ_WRLOCK(rwlock);
 static unsigned int bond_hash_src(const struct eth_addr mac,
                                   uint16_t vlan, uint32_t basis);
-static unsigned int bond_hash_tcp(const struct flow *, uint16_t vlan,
-                                  uint32_t basis);
 static struct bond_entry *lookup_bond_entry(const struct bond *,
                                             const struct flow *,
                                             uint16_t vlan)
@@ -1743,24 +1741,12 @@  bond_hash_src(const struct eth_addr mac, uint16_t vlan, uint32_t basis)
 }
 
 static unsigned int
-bond_hash_tcp(const struct flow *flow, uint16_t vlan, uint32_t basis)
-{
-    struct flow hash_flow = *flow;
-    hash_flow.vlans[0].tci = htons(vlan);
-
-    /* The symmetric quality of this hash function is not required, but
-     * flow_hash_symmetric_l4 already exists, and is sufficient for our
-     * purposes, so we use it out of convenience. */
-    return flow_hash_symmetric_l4(&hash_flow, basis);
-}
-
-static unsigned int
 bond_hash(const struct bond *bond, const struct flow *flow, uint16_t vlan)
 {
     ovs_assert(bond->balance == BM_TCP || bond->balance == BM_SLB);
 
     return (bond->balance == BM_TCP
-            ? bond_hash_tcp(flow, vlan, bond->basis)
+            ? flow_hash_5tuple(flow, bond->basis)
             : bond_hash_src(flow->dl_src, vlan, bond->basis));
 }