In order to properly balance bond traffic, ofproto/bond periodically
reads usage statistics of the post-recirculation rules (which are added
to a hidden internal table).

To do that, each "struct bond_entry" (which represents a hash within a
bond) stores the last seen statistics for its rule. When a hash is moved
to another member (due to a bond rebalance or the previous member going
down), the rule is typically just modified, i.e: same match different
actions. In this case, statistics are preserved and accounting continues
to work.

However, if the rule gets completely deleted (e.g: when all bond members
go down) and then re-created, the new rule will have 0 tx_bytes but its
associated entry will still store a non-zero last-seen value.
This situation leads to an overflow of the delta calculation (computed
as [current_stats_value - last_seen_value]), which can affect traffic
as the hash will be considered to carry a lot of traffic and rebalancing
will kick in.

In order to fix this situation, reset the value of last seen statistics
on rule deletion.

Implementation note:
The field ((struct bond_entry *) -> pr_tx_bytes) is guarded by rwlock
but, as the comment above the function indicates, it's only called
without having locked it when the bond is being deleted. Given that
bond->hash is free()ed before deleting the rules (which makes writing to
the entries a use-after-free anyway), we can use that to avoid
double-locking.

Reported-at: https://github.com/openvswitch/ovs-issues/issues/319
Signed-off-by: Adrian Moreno <[email protected]>
---
 ofproto/bond.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ofproto/bond.c b/ofproto/bond.c
index cfdf44f85..ad56bb96b 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -407,6 +407,10 @@ update_recirc_rules__(struct bond *bond)
 
                 VLOG_ERR("failed to remove post recirculation flow %s", err_s);
                 free(err_s);
+            } else if (bond->hash) {
+                uint32_t hash = pr_op->hmap_node.hash & BOND_MASK;
+                struct bond_entry *entry = &bond->hash[hash];
+                entry->pr_tx_bytes = 0;
             }
 
             hmap_remove(&bond->pr_rule_ops, &pr_op->hmap_node);
-- 
2.43.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to