Skip to content

OCPBUGS-54682: Fix - NetworkManager restart or crash renders br-ex unusable #5304

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
merged 5 commits into from
Oct 15, 2025
Merged

OCPBUGS-54682: Fix - NetworkManager restart or crash renders br-ex unusable #5304

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5a59b6c
ofport-request dispatcher script takes br-ex(-br) managed by nmstate …
emy Sep 24, 2025
28a1a75
improve of-port request so it uses a common path instead of retrying …
emy Sep 30, 2025
5add825
Revert "improve of-port request so it uses a common path instead of r…
emy Oct 9, 2025
1e8fc02
Revert "ofport-request dispatcher script takes br-ex(-br) managed by …
emy Oct 9, 2025
0a6dfa5
add: fallback check for br-ex-br in ofport NM dispatcher script
emy Oct 9, 2025
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
8 changes: 7 additions & 1 deletion templates/common/_base/files/ofport-request.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ contents:
fi

# Get the bridge name
BRIDGE_NAME=$(nmcli -t -f connection.interface-name conn show "${BRIDGE_ID}" | awk -F ':' '{print $NF}')
BRIDGE_NAME=$(nmcli -t -f connection.interface-name conn show "${BRIDGE_ID}" | awk -F ':' '{print $NF}') || true
if [ "${BRIDGE_NAME}" == "" ]; then
#Check if br-ex is managed by nmstate (br-ex-br)
PORT_CONNECTION_UUID=$(nmcli -t -f device,type,uuid conn | awk -F ':' '{if( $1=="br-ex" && $2~/^ovs-bridge/) print $NF}')
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious how this works. If the connection uuid was incorrectly detected, do we not exit on line 46? Are we still getting a uuid there but it's not the correct one?

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is no connection that matches both "br-ex" and "ovs-bridge" then PORT_CONNECTION_UUID will be empty, thus the next line fails and process exits with code 10 or some other big number.

If there is only one connection that matches both "br-ex" and "ovs-bridge", then PORT_CONNECTION_UUID will get one value and it's a correct value. This is the scenario when everything works okay.

If there are two or more connections that match both "br-ex" and "ovs-bridge", then PORT_CONNECTION_UUID will get a random value out of 2 or more possible ones. This scenario is bad, undesired. How can it happen that we end up in such a scenario? We would need to have 2 connections that have the same device and type. Can it ever happen?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I realize now that you are asking about L46 which uses output of

PORT_CONNECTION_UUID=$(nmcli -t -f device,type,uuid conn | awk -F ':' '{if( ($1=="'${PORT}'" || $3=="'${PORT}'") && $2~/^ovs*/) print $NF}')

but I do not see there how we can get an incorrect one easily. At least, in more scenarios than we do today. Let's start with

[root@master-0 ~]# nmcli -t -f device,type,uuid conn
br-ex:ovs-interface:2225810d-187f-432f-9c09-67bf9727ae88
br-ex:ovs-bridge:ce9f4f14-e059-4d8e-bf2e-2379fc3fed8a
enp2s0:802-3-ethernet:8d1041e5-9992-450e-954f-8e4f981ae9d2
br-ex:ovs-port:bf2011c7-24f6-4e35-85cd-ca639f4769e5
enp2s0:ovs-port:fb1ae9eb-d718-4a72-9b42-f12c1f2c9942
enp1s0:802-3-ethernet:d4a98e92-8232-40d5-9a2c-c69796bbd40c
enp3s0:802-3-ethernet:d4a98e92-8232-40d5-9a2c-c69796bbd40c
enp4s0:802-3-ethernet:d4a98e92-8232-40d5-9a2c-c69796bbd40c
lo:loopback:ec98b9a0-abe4-409a-86c7-ffc9e3fb3ae0
:802-3-ethernet:e5bf500e-35e8-4888-b4eb-74314c6473e5

From that we get

[root@master-0 ~]# export INTERFACE_NAME=enp2s0

[root@master-0 ~]# INTERFACE_CONNECTION_UUID=$(nmcli -t -f device,type,uuid conn | awk -F ':' '{if($1=="'${INTERFACE_NAME}'" && $2!~/^ovs*/) print $NF}')
[root@master-0 ~]# echo $INTERFACE_CONNECTION_UUID
8d1041e5-9992-450e-954f-8e4f981ae9d2

[root@master-0 ~]# INTERFACE_OVS_SLAVE_TYPE=$(nmcli -t -f connection.slave-type conn show "${INTERFACE_CONNECTION_UUID}" | awk -F ':' '{print $NF}')
[root@master-0 ~]# echo $INTERFACE_OVS_SLAVE_TYPE
ovs-port

[root@master-0 ~]# PORT=$(nmcli -t -f connection.master conn show "${INTERFACE_CONNECTION_UUID}" | awk -F ':' '{print $NF}')
[root@master-0 ~]# echo $PORT
fb1ae9eb-d718-4a72-9b42-f12c1f2c9942

[root@master-0 ~]# PORT_CONNECTION_UUID=$(nmcli -t -f device,type,uuid conn | awk -F ':' '{if( ($1=="'${PORT}'" || $3=="'${PORT}'") && $2~/^ovs*/) print $NF}')
[root@master-0 ~]# echo $PORT_CONNECTION_UUID
fb1ae9eb-d718-4a72-9b42-f12c1f2c9942

So seems like PORT_CONNECTION_UUID is trying to find type ovs* with the name that is your interface name (e.g. eth0). For those it's okay because we will not have multiple ovs* with such a name.

It could get tricky when you have the run with INTERFACE_NAME=br-ex, but that one finishes very quickly, i.e.

[root@master-0 ~]# export INTERFACE_NAME=br-ex

[root@master-0 ~]# INTERFACE_CONNECTION_UUID=$(nmcli -t -f device,type,uuid conn | awk -F ':' '{if($1=="'${INTERFACE_NAME}'" && $2!~/^ovs*/) print $NF}')
[root@master-0 ~]# echo $INTERFACE_CONNECTION_UUID

[root@master-0 ~]#

So, do we actually miss something?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure. The reason I'm asking is that we're recalculating the port UUID here with a slightly different command than the one above, but if the one above didn't find any UUID then the script would have exited before now. Which leads me to believe that either we get a different UUID from this command for some reason, or we don't need to recalculate it at all.

The latter would make this second call unnecessary, but it would still work fine so I'm mostly making sure I understand the logic correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair.

  1. If we had no PORT_CONNECTION_UUID previously, we can't reach this code here.
  2. If we had correct PORT_CONNECTION_UUID previously, we recalculate it here but we don't need to.
  3. If we had wrong PORT_CONNECTION_UUID previously, it's actually bad

I have a gut feeling we are in the scenario (3). Look that the previous PORT_CONNECTION_UUID, it only matches for ovs and not for ovs-bridge. Given that for br-ex* we have more than one entry matching ovs*, the way of calculating it here is more robust than the way of calculating it the old way.

Maybe we should just move PORT_CONNECTION_UUID=$( from there up to L44 ? As I read it, this should work for both old and new way of defining br-ex. L44 works correctly for old method and may(?) race for the new one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pretty sure I ended up in scenario (3). I'll check back and I agree that we could/should make this a little more solid for cases where a wrong selection could happen.

BRIDGE_ID=$(nmcli -t -f connection.id conn show "${PORT_CONNECTION_UUID}" | awk -F ':' '{print $NF}')
BRIDGE_NAME=$(nmcli -t -f connection.interface-name conn show "${BRIDGE_ID}" | awk -F ':' '{print $NF}')
fi
# Limit this to br-ex and br-ex1 only. If one wanted to enable this for all OVS bridges,
# the condition would be: if [ "$BRIDGE_NAME" == "" ]; then
if [ "${BRIDGE_NAME}" != "br-ex" ] && [ "${BRIDGE_NAME}" != "br-ex1" ]; then
Expand Down