From 8da49ce94fa70977a3705469fb83906d57f0c308 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Thu, 7 Mar 2024 18:32:12 +0100
Subject: [PATCH] Do not read bridge node multiple times

We have guards against non-existing bridge nodes, but when they succeed
we throw away the data we have read. This can lead to theoretical TOCTOE
errors.

Fix this by only reading the bridge node once.

Change-Id: I5e224b350d3c4f5e05d1366fa8f4282b6834dba8
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../ovsdb/transact/QosUpdateCommand.java           |  7 ++++---
 .../ovsdb/transact/QueueUpdateCommand.java         | 14 ++++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QosUpdateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QosUpdateCommand.java
index 339506416..bd4ffadbf 100644
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QosUpdateCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QosUpdateCommand.java
@@ -64,11 +64,12 @@ public class QosUpdateCommand implements TransactCommand {
         for (Entry<InstanceIdentifier<QosEntries>, QosEntries> qosMapEntry: createdOrUpdated.entrySet()) {
             InstanceIdentifier<OvsdbNodeAugmentation> iid =
                     qosMapEntry.getKey().firstIdentifierOf(OvsdbNodeAugmentation.class);
-            if (!state.getBridgeNode(iid).isPresent()) {
+            final var optBridgeNode = state.getBridgeNode(iid);
+            if (optBridgeNode.isEmpty()) {
                 return;
             }
-            OvsdbNodeAugmentation operNode =
-                state.getBridgeNode(iid).orElseThrow().augmentation(OvsdbNodeAugmentation.class);
+
+            OvsdbNodeAugmentation operNode = optBridgeNode.orElseThrow().augmentation(OvsdbNodeAugmentation.class);
 
             QosEntries qosEntry = qosMapEntry.getValue();
             Qos qos = transaction.getTypedRowWrapper(Qos.class);
diff --git a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QueueUpdateCommand.java b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QueueUpdateCommand.java
index 4d40bc02f..5f7218fb1 100644
--- a/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QueueUpdateCommand.java
+++ b/southbound/southbound-impl/src/main/java/org/opendaylight/ovsdb/southbound/ovsdb/transact/QueueUpdateCommand.java
@@ -58,10 +58,11 @@ public class QueueUpdateCommand implements TransactCommand {
     private static void execute(final TransactionBuilder transaction, final BridgeOperationalState state,
             final Map<InstanceIdentifier<Queues>, Queues> createdOrUpdated,
             final InstanceIdentifierCodec instanceIdentifierCodec) {
-        for (Entry<InstanceIdentifier<Queues>, Queues> queueMapEntry: createdOrUpdated.entrySet()) {
+        for (Entry<InstanceIdentifier<Queues>, Queues> queueMapEntry : createdOrUpdated.entrySet()) {
             InstanceIdentifier<OvsdbNodeAugmentation> iid =
                     queueMapEntry.getKey().firstIdentifierOf(OvsdbNodeAugmentation.class);
-            if (!state.getBridgeNode(iid).isPresent()) {
+            final var optBridgeNode = state.getBridgeNode(iid);
+            if (optBridgeNode.isEmpty()) {
                 return;
             }
 
@@ -100,23 +101,20 @@ public class QueueUpdateCommand implements TransactCommand {
                 LOG.warn("Incomplete Queue other_config", e);
             }
 
-            OvsdbNodeAugmentation operNode =
-                state.getBridgeNode(iid).orElseThrow().augmentation(OvsdbNodeAugmentation.class);
+            OvsdbNodeAugmentation operNode = optBridgeNode.orElseThrow().augmentation(OvsdbNodeAugmentation.class);
             Uuid operQueueUuid = getQueueEntryUuid(operNode.getQueues(), queueEntry.key());
             if (operQueueUuid == null) {
                 UUID namedUuid = new UUID(SouthboundConstants.QUEUE_NAMED_UUID_PREFIX
                         + TransactUtils.bytesToHexString(queueEntry.getQueueId().getValue().getBytes(UTF_8)));
                 transaction.add(op.insert(queue).withId(namedUuid.toString()));
-                LOG.info("Added queue Uuid : {} for Ovsdb Node : {}",
-                        namedUuid, operNode);
+                LOG.info("Added queue Uuid : {} for Ovsdb Node : {}", namedUuid, operNode);
             } else {
                 UUID uuid = new UUID(operQueueUuid.getValue());
                 Queue extraQueue = transaction.getTypedRowSchema(Queue.class);
                 extraQueue.getUuidColumn().setData(uuid);
                 transaction.add(op.update(queue)
                         .where(extraQueue.getUuidColumn().getSchema().opEqual(uuid)).build());
-                LOG.info("Updated queue entries: {} for Ovsdb Node : {}",
-                        queue, operNode);
+                LOG.info("Updated queue entries: {} for Ovsdb Node : {}", queue, operNode);
             }
         }
     }
-- 
2.36.6