Skip to content

OCPBUGS-77015: wait for patch port to apply drop garp flows #2910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Mar 19, 2026
Merged

OCPBUGS-77015: wait for patch port to apply drop garp flows #2910

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions bindata/network/ovn-kubernetes/common/008-script-lib.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,38 @@ data:
local of_port
of_port=$(ovs-vsctl get interface "$port_name" ofport)
ovs-ofctl add-flow "$bridge" "cookie=$cookie,table=0,priority=$priority,in_port=$of_port,arp,arp_op=1,actions=drop" > /dev/null
break
echo "$(date -Iseconds) - INFO - Added GARP drop flow on $bridge port $port_name (ofport=$of_port)"
return 0
fi
Comment on lines 249 to +252
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the add_garp_drop_flow function and surrounding context
rg -n -C3 'add_garp_drop_flow\(|ovs-ofctl add-flow|return 0|return 1' bindata/network/ovn-kubernetes/common/008-script-lib.yaml | head -100

Repository: openshift/cluster-network-operator

Length of output: 2213

Gate success on ovs-ofctl add-flow exit status.

add_garp_drop_flow() reports success even when flow insertion fails, which stops retries early. The function must check the ovs-ofctl add-flow exit status before returning success.

Proposed fix 
-          ovs-ofctl add-flow "$bridge" "cookie=$cookie,table=0,priority=$priority,in_port=$of_port,arp,arp_op=1,actions=drop" > /dev/null
-          echo "$(date -Iseconds) - INFO - Added GARP drop flow on $bridge port $port_name (ofport=$of_port)"
-          return 0
+          if ovs-ofctl add-flow "$bridge" "cookie=$cookie,table=0,priority=$priority,in_port=$of_port,arp,arp_op=1,actions=drop" > /dev/null; then
+            echo "$(date -Iseconds) - INFO - Added GARP drop flow on $bridge port $port_name (ofport=$of_port)"
+            return 0
+          fi
+          echo "$(date -Iseconds) - WARN - Failed to add GARP drop flow on $bridge port $port_name (ofport=$of_port), will retry"
+          return 1
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bindata/network/ovn-kubernetes/common/008-script-lib.yaml` around lines 249 -
252, The add_garp_drop_flow() function currently echoes success and returns 0
immediately after running ovs-ofctl add-flow even if that command failed; change
the logic to capture and check the exit status of the ovs-ofctl invocation (the
command that writes the flow for $bridge / $of_port) before logging success and
returning 0. If ovs-ofctl returns non-zero, log an error including the exit code
and any stderr (don't let the > /dev/null swallow the check), and return a
non-zero value so the caller can retry; only return 0 when the ovs-ofctl
add-flow command actually succeeded.

done

# Port not found
return 1
}

# Wait for patch port to exist and add GARP drop flow.
wait-and-add-garp-drop-flow() {
local bridge="$1"
local max_retries=90
local retries=0

while [[ "${retries}" -lt "${max_retries}" ]]; do
if add_garp_drop_flow "${bridge}"; then
return 0
fi

(( retries += 1 ))

# Log once after 30 seconds to indicate we're still waiting
if [[ "${retries}" -eq 15 ]]; then
echo "$(date -Iseconds) - INFO - Waiting for patch port on ${bridge} to be ready..."
fi

sleep 2
done

echo "$(date -Iseconds) - WARN - Failed to add GARP drop flow on ${bridge} after ${max_retries} attempts (best-effort workaround)"
return 1
}

# quit-nbdb() will cleanly shut down the northbound dbserver. It is intended
Expand Down Expand Up @@ -512,7 +541,7 @@ data:
# start temp work around
# remove when https://issues.redhat.com/browse/FDP-1537 is available
if ovs-vsctl br-exists "br-ex"; then
add_garp_drop_flow br-ex
(wait-and-add-garp-drop-flow br-ex &)
fi
# end temp work around

Expand Down