OVS offline with balance-tcp bonding

[rowlap]

Related to #369 , we have reasons (not great reasons, but reasons) to regularly invoke

ovs-vsctl set port bond0 other_config:bond-hash-basis=N

where N is "a different value".

Having recently rebased to 3.5.2, this bond-hash-basis flip is sufficient to drop traffic.

Steps to repro:

• Build a 2-member LACP bond with bond_mode=balance_tcp
• Generate a lot of flows, causing dpctl/dump-flows count to exceed 20000
• Change bond-hash-basis

That's it. My environment is using qemu/kvm VMs with tap interfaces, but I don't know that this is required.

This is the traffic-generating script I've been using. Note that it will actually egress junk MAC addresses to the network, which may be disruptive. The eth0 referred to is from a VM guest using a tap interface, not the system where ovs-vswitchd itself is running.

from scapy.all import Ether, IP, UDP, sendp
import random

pkts = [
    Ether(src="02:00:%02x:%02x:%02x:%02x" % tuple(random.randint(0, 255) for _ in range(4)))
    / IP(dst="10.10.10.10") / UDP(dport=random.randint(1, 65535))
    for _ in range(20000)
]

while True:
    sendp(pkts, iface="eth0")

To recover, from the console we can run a simple/no-op change to trigger bridge reconfigure:

ovs-vsctl set bridge br0 external_ids:bond-nudge="$(date +%s%N)"

The above repro has been crafted, and this bug report written by me, a real human, although the debugging was LLM-assisted with visibility of the ovs codebase which I'm unfamiliar with.

With that LLM disclaimer, here is the one-line summary of what it concludes:

In balance-tcp bonds, changing bond-hash-basis can delete all post-recirc internal rules; if packet xlate repopulates bond->hash before bond_update_post_recirc_rules__ runs, OVS believes no rule update is needed and leaves TBL_INTERNAL without recirc_id/dp_hash output rules, causing persistent drop until a later bridge reconfigure.

[mkp-rh]

Hello Rowlap,

I was able to reproduce this issue in my environment as well. Thanks for reporting! I'm still investigating it, but it appears that recirc flows aren't being updated correctly.

That said, setting hash-basis is a fairly heavyweight operation, especially with a large number of flows like this 20000 number. Are you able to provide more information on why this is needed?

[rowlap]

It's related to #369 - we'd like to obtain balanced egress with bond-rebalance-interval=0. I don't want to die on that hill, or get sidetracked for this ticket; it's just the repro we hit upon, but perhaps there are other ways to get into the dropping state.

One aspect which made it worse is that we were flipping bond-hash-basis hourly as a workaround to overnight switch maintenance (with LACP, so connectivity is preserved). This is unnecessary in the case where both members are being used for egress, but as we made it unconditional the chance of hitting the problem increased.

This isn't to defend the dumb script, but might explain how we've hit upon a problem which has remained hidden until now.

[mkp-rh]

I find this issue goes away with the following change. I'll work on making a unit test and putting a patch together.

diff --git a/ofproto/bond.c b/ofproto/bond.c
index 45a36fabb..5490746de 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -1088,7 +1088,7 @@ void
 bond_update_post_recirc_rules(struct bond *bond, uint32_t *recirc_id,
                               uint32_t *hash_basis)
 {
-    bool may_recirc = bond_may_recirc(bond);
+    bool may_recirc = (bond->balance == BM_TCP && bond->recirc_id);
 
     if (may_recirc) {
         /* To avoid unnecessary locking, bond_may_recirc() is first