Hi Alexandra,
Since this is an RFC, I'm going to respond to the entire series here
rather than addressing each patch individually.
There are essentially two things being done in this RFC.
1. If the OVS DB has external_ids:validate-northd-actions, then check
for matching supported actions between ovn-northd and ovn-controller.
ovn-northd places its list of action names in the SB_Global database.
ovn-controller then can compare its action list to northd's. If northd
has actions not supported by ovn-controller, then ovn-controller will
keep its current OF flows in place and will not process updates from
ovn-northd.
The idea is that this could provide a more granular check than simply
checking the northd version number. Even if the northd version number is
different, so long as ovn-controller knows all of northd's actions, then
ovn-controller can operate as normal.
There are some flaws with this:
* The code compares action names from northd with action names from
ovn-controller. This does not account for semantic changes within an
action. For instance, the same action may exist on northd and
ovn-controller. However, the northd version of the action takes 4
parameters, but the ovn-controller version of the action only takes 3.
Or maybe the number of parameters is the same, but the syntax for the
parameters has been altered.
* In some cases, northd makes use of chassis feature config (see
build_chassis_features() in northd/en-global-config.c). northd may
support a particular action, but it will only use that action if all
connected ovn-controllers support the action. Therefore, northd may have
an action ovn-controller does not support, but it may be completely safe
to run ovn-controller as normal since northd is not going to use that
action anyway.
* Even if all actions are the same between northd and ovn-controller, it
doesn't necessarily guarantee compatibility, since other factors may be
at play as well, like OVS or kernel features on the ovn-controller
hypervisor.
* Generally, the guidance for users is to upgrade ovn-controller before
ovn-northd. This way, ovn-controller will know about new actions and
will be aware of syntactic/semantic changes to actions before northd
does. But what about if an action is deprecated and removed? In this
case when ovn-controller is upgraded, it doesn't have the action, but
northd does. Deprecation of an action would be introduced gradually, in
such a way that ovn-northd would have provisions not to use the action
if it is connected to a version of ovn-controller that does not have the
action.
Overall, I'm having trouble finding how this would be better than the
current safeguards we have in place. If users are upgrading
ovn-controller before ovn-northd, then everything should work since that
is how the code is designed to be upgraded. If users are not following
the guidelines and are upgrading ovn-northd first, then the northd
version mismatch check is more likely to catch incompatibilities than
comparing supported action names.
2. If ovn-northd and ovn-controller have mismatched pipeline lengths,
then dynamically update ovn-controller so that it will use the pipeline
lengths that northd expects.
There are some problems with the implementation.
* The ovn-trace and ovn-debug utilities have not been updated to deal
with the mismatch. They just read the constants directly. This could be
a pain to try to fix since I'm not sure how you can know from these
utilities if you are running the same code as ovn-northd or ovn-controller.
* This is going to make debugging more difficult (at least for me :) ).
I tend to reference controller/lflow.h in order to determine the OF
table numbers when looking through an OF dump. Since the initial values
in lib/oftable.c may not correspond to the actual values used on a
particular hypervisor, I can no longer trust the values in the code.
This could be remedied by providing an ovn-appctl command to dump the
table constants.
I think (2) has more potential than (1). If users are following
guidelines and upgrading ovn-controller before ovn-northd, then this
should never be an issue. If the number of stages increases between
versions, then ovn-controller will have the updated higher values before
ovn-northd tries to use them. If the number of stages decreases between
versions, then we likely would keep the PIPELINE_LEN constants the same,
even if we actually are not using as many stages.
However, if users are not following the guidelines and are upgrading
ovn-northd first, then this change has the potential to avoid some
catastrophes if northd thinks the pipeline lengths are longer than
ovn-controller does.
To sum up, I think (1) (patch 2 in the series) should be dropped.
However, I think (2) (patches 1 and 3 in the series) is worth following
up on, with the flaws addressed that I described above.
On 8/29/25 2:16 PM, Alexandra Rukomoinikova wrote:
Replaced macro definitions for OpenFlow table numbers
with runtime variables to allow dynamic configuration.
This change enables future dynamic adjustment of table numbering.
Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
controller/lflow.h | 45 --------------------------------
controller/mac-cache.c | 1 +
controller/statctrl.c | 1 +
include/ovn/actions.h | 1 +
lib/automake.mk | 4 ++-
lib/oftable.c | 55 +++++++++++++++++++++++++++++++++++++++
lib/oftable.h | 58 ++++++++++++++++++++++++++++++++++++++++++
lib/ovn-util.c | 3 +++
lib/ovn-util.h | 4 +--
utilities/ovn-debug.c | 1 +
10 files changed, 125 insertions(+), 48 deletions(-)
create mode 100644 lib/oftable.c
create mode 100644 lib/oftable.h
diff --git a/controller/lflow.h b/controller/lflow.h
index c8a87c886..f4ab70d6c 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -59,51 +59,6 @@ struct simap;
struct sset;
struct uuid;
-/* OpenFlow table numbers.
- *
- * These are heavily documented in ovn-architecture(7), please update it if
- * you make any changes. */
-#define OFTABLE_PHY_TO_LOG 0
-
-/* Start of LOG_PIPELINE_LEN tables. */
-#define OFTABLE_LOG_INGRESS_PIPELINE 8
-#define OFTABLE_OUTPUT_LARGE_PKT_DETECT 41
-#define OFTABLE_OUTPUT_LARGE_PKT_PROCESS 42
-#define OFTABLE_REMOTE_OUTPUT 43
-#define OFTABLE_REMOTE_VTEP_OUTPUT 44
-#define OFTABLE_LOCAL_OUTPUT 45
-#define OFTABLE_CHECK_LOOPBACK 46
-
-/* Start of the OUTPUT section of the pipeline. */
-#define OFTABLE_OUTPUT_INIT OFTABLE_OUTPUT_LARGE_PKT_DETECT
-
-/* Start of LOG_PIPELINE_LEN tables. */
-#define OFTABLE_LOG_EGRESS_PIPELINE 47
-#define OFTABLE_SAVE_INPORT 64
-#define OFTABLE_LOG_TO_PHY 65
-#define OFTABLE_MAC_BINDING 66
-#define OFTABLE_MAC_LOOKUP 67
-#define OFTABLE_CHK_LB_HAIRPIN 68
-#define OFTABLE_CHK_LB_HAIRPIN_REPLY 69
-#define OFTABLE_CT_SNAT_HAIRPIN 70
-#define OFTABLE_GET_FDB 71
-#define OFTABLE_LOOKUP_FDB 72
-#define OFTABLE_CHK_IN_PORT_SEC 73
-#define OFTABLE_CHK_IN_PORT_SEC_ND 74
-#define OFTABLE_CHK_OUT_PORT_SEC 75
-#define OFTABLE_ECMP_NH_MAC 76
-#define OFTABLE_ECMP_NH 77
-#define OFTABLE_CHK_LB_AFFINITY 78
-#define OFTABLE_MAC_CACHE_USE 79
-#define OFTABLE_CT_ZONE_LOOKUP 80
-#define OFTABLE_CT_ORIG_NW_DST_LOAD 81
-#define OFTABLE_CT_ORIG_IP6_DST_LOAD 82
-#define OFTABLE_CT_ORIG_TP_DST_LOAD 83
-#define OFTABLE_FLOOD_REMOTE_CHASSIS 84
-#define OFTABLE_CT_STATE_SAVE 85
-#define OFTABLE_CT_ORIG_PROTO_LOAD 86
-#define OFTABLE_GET_REMOTE_FDB 87
-
/* Common defines shared between some controller components. */
#define CHASSIS_FLOOD_INDEX_START 0x8000
diff --git a/controller/mac-cache.c b/controller/mac-cache.c
index fcf5fa7d4..0fea70333 100644
--- a/controller/mac-cache.c
+++ b/controller/mac-cache.c
@@ -19,6 +19,7 @@
#include "lflow.h"
#include "lib/mac-binding-index.h"
#include "lib/vec.h"
+#include "lib/oftable.h"
#include "local_data.h"
#include "lport.h"
#include "mac-cache.h"
diff --git a/controller/statctrl.c b/controller/statctrl.c
index 454314dda..d84274834 100644
--- a/controller/statctrl.c
+++ b/controller/statctrl.c
@@ -20,6 +20,7 @@
#include "latch.h"
#include "lflow.h"
#include "lib/vec.h"
+#include "lib/oftable.h"
#include "mac-cache.h"
#include "openvswitch/ofp-errors.h"
#include "openvswitch/ofp-flow.h"
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0eaef9112..82d128f33 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -27,6 +27,7 @@
#include "openvswitch/uuid.h"
#include "util.h"
#include "ovn/features.h"
+#include "oftable.h"
struct expr;
struct lexer;
diff --git a/lib/automake.mk b/lib/automake.mk
index a59c722d6..16baeb157 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -10,6 +10,8 @@ lib_libovn_la_LDFLAGS += $(VIF_PLUG_PROVIDER_LDFLAGS)
endif
lib_libovn_la_SOURCES = \
+ lib/oftable.c \
+ lib/oftable.h \
lib/acl-log.c \
lib/acl-log.h \
lib/actions.c \
@@ -22,8 +24,8 @@ lib_libovn_la_SOURCES = \
lib/extend-table.h \
lib/extend-table.c \
lib/features.c \
- lib/ovn-parallel-hmap.h \
lib/ovn-parallel-hmap.c \
+ lib/ovn-parallel-hmap.h \
lib/ip-mcast-index.c \
lib/ip-mcast-index.h \
lib/mac-binding-index.c \
diff --git a/lib/oftable.c b/lib/oftable.c
new file mode 100644
index 000000000..721b3ad49
--- /dev/null
+++ b/lib/oftable.c
@@ -0,0 +1,55 @@
+/* Copyright (c) 2015, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <config.h>
+
+#include "lib/oftable.h"
+
+int OFTABLE_PHY_TO_LOG = 0;
+
+/* Start of LOG_PIPELINE_LEN tables. */
+int OFTABLE_LOG_INGRESS_PIPELINE = 8;
+int OFTABLE_OUTPUT_LARGE_PKT_DETECT = 41, OFTABLE_OUTPUT_INIT = 41;
+int OFTABLE_OUTPUT_LARGE_PKT_PROCESS = 42;
+int OFTABLE_REMOTE_OUTPUT = 43;
+int OFTABLE_REMOTE_VTEP_OUTPUT = 44;
+int OFTABLE_LOCAL_OUTPUT = 45;
+int OFTABLE_CHECK_LOOPBACK = 46;
+
+/* Start of LOG_PIPELINE_LEN tables. */
+int OFTABLE_LOG_EGRESS_PIPELINE = 47;
+int OFTABLE_SAVE_INPORT = 64;
+int OFTABLE_LOG_TO_PHY = 65;
+int OFTABLE_MAC_BINDING = 66;
+int OFTABLE_MAC_LOOKUP = 67;
+int OFTABLE_CHK_LB_HAIRPIN = 68;
+int OFTABLE_CHK_LB_HAIRPIN_REPLY = 69;
+int OFTABLE_CT_SNAT_HAIRPIN = 70;
+int OFTABLE_GET_FDB = 71;
+int OFTABLE_LOOKUP_FDB = 72;
+int OFTABLE_CHK_IN_PORT_SEC = 73;
+int OFTABLE_CHK_IN_PORT_SEC_ND = 74;
+int OFTABLE_CHK_OUT_PORT_SEC = 75;
+int OFTABLE_ECMP_NH_MAC = 76;
+int OFTABLE_ECMP_NH = 77;
+int OFTABLE_CHK_LB_AFFINITY = 78;
+int OFTABLE_MAC_CACHE_USE = 79;
+int OFTABLE_CT_ZONE_LOOKUP = 80;
+int OFTABLE_CT_ORIG_NW_DST_LOAD = 81;
+int OFTABLE_CT_ORIG_IP6_DST_LOAD = 82;
+int OFTABLE_CT_ORIG_TP_DST_LOAD = 83;
+int OFTABLE_FLOOD_REMOTE_CHASSIS = 84;
+int OFTABLE_CT_STATE_SAVE = 85;
+int OFTABLE_CT_ORIG_PROTO_LOAD = 86;
+int OFTABLE_GET_REMOTE_FDB = 87;
diff --git a/lib/oftable.h b/lib/oftable.h
new file mode 100644
index 000000000..54425964d
--- /dev/null
+++ b/lib/oftable.h
@@ -0,0 +1,58 @@
+/* Copyright (c) 2015, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <stdint.h>
+
+/* OpenFlow table numbers.
+ *
+ * These are heavily documented in ovn-architecture(7), please update it if
+ * you make any changes. */
+extern int OFTABLE_PHY_TO_LOG;
+
+/* Start of LOG_PIPELINE_LEN tables. */
+extern int OFTABLE_LOG_INGRESS_PIPELINE;
+extern int OFTABLE_OUTPUT_LARGE_PKT_DETECT;
+extern int OFTABLE_OUTPUT_INIT;
+extern int OFTABLE_OUTPUT_LARGE_PKT_PROCESS;
+extern int OFTABLE_REMOTE_OUTPUT;
+extern int OFTABLE_REMOTE_VTEP_OUTPUT;
+extern int OFTABLE_LOCAL_OUTPUT;
+extern int OFTABLE_CHECK_LOOPBACK;
+
+/* Start of LOG_PIPELINE_LEN tables. */
+extern int OFTABLE_LOG_EGRESS_PIPELINE;
+extern int OFTABLE_SAVE_INPORT;
+extern int OFTABLE_LOG_TO_PHY;
+extern int OFTABLE_MAC_BINDING;
+extern int OFTABLE_MAC_LOOKUP;
+extern int OFTABLE_CHK_LB_HAIRPIN;
+extern int OFTABLE_CHK_LB_HAIRPIN_REPLY;
+extern int OFTABLE_CT_SNAT_HAIRPIN;
+extern int OFTABLE_GET_FDB;
+extern int OFTABLE_LOOKUP_FDB;
+extern int OFTABLE_CHK_IN_PORT_SEC;
+extern int OFTABLE_CHK_IN_PORT_SEC_ND;
+extern int OFTABLE_CHK_OUT_PORT_SEC;
+extern int OFTABLE_ECMP_NH_MAC;
+extern int OFTABLE_ECMP_NH;
+extern int OFTABLE_CHK_LB_AFFINITY;
+extern int OFTABLE_MAC_CACHE_USE;
+extern int OFTABLE_CT_ZONE_LOOKUP;
+extern int OFTABLE_CT_ORIG_NW_DST_LOAD;
+extern int OFTABLE_CT_ORIG_IP6_DST_LOAD;
+extern int OFTABLE_CT_ORIG_TP_DST_LOAD;
+extern int OFTABLE_FLOOD_REMOTE_CHASSIS;
+extern int OFTABLE_CT_STATE_SAVE;
+extern int OFTABLE_CT_ORIG_PROTO_LOAD;
+extern int OFTABLE_GET_REMOTE_FDB;
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 8b583fa6d..47922276e 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -39,6 +39,9 @@ VLOG_DEFINE_THIS_MODULE(ovn_util);
#define DEFAULT_PROBE_INTERVAL_MSEC 5000
+int LOG_PIPELINE_INGRESS_LEN = 32;
+int LOG_PIPELINE_EGRESS_LEN = 14;
+
void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
const char *argv[] OVS_UNUSED, void *idl_)
{
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 63beae3e5..abda66963 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -320,8 +320,8 @@ BUILD_ASSERT_DECL(
#define SCTP_ABORT_CHUNK_FLAG_T (1 << 0)
/* The number of tables for the ingress and egress pipelines. */
-#define LOG_PIPELINE_INGRESS_LEN 32
-#define LOG_PIPELINE_EGRESS_LEN 14
+extern int LOG_PIPELINE_INGRESS_LEN;
+extern int LOG_PIPELINE_EGRESS_LEN;
static inline uint32_t
hash_add_in6_addr(uint32_t hash, const struct in6_addr *addr)
diff --git a/utilities/ovn-debug.c b/utilities/ovn-debug.c
index 0a0d2202b..64d2b8bab 100644
--- a/utilities/ovn-debug.c
+++ b/utilities/ovn-debug.c
@@ -21,6 +21,7 @@
#include "controller/lflow.h"
#include "northd/northd.h"
#include "ovn-util.h"
+#include "lib/oftable.h"
struct ovn_lflow_stage {
const char *name;
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
