Thanks Aditya. I really appreciate the additional test cases this patch adds!

Acked-by: Mark Michelson <[email protected]>

On Tue, Feb 3, 2026 at 4:16 AM Aditya Mehakare
<[email protected]> wrote:
>
> This commit introduces a new 'id' column to the Network_Function
> table in the OVN Northbound database schema. The ID is a mandatory
> integer field (range 1-255) with a unique index constraint.
>
> This change is required to support network function active-active
> mode in future releases. The ID will be used to uniquely identify
> network functions in scenarios where multiple instances need to be
> managed simultaneously.
>
> Since this is a schema change that is not backward compatible, it
> is being introduced in the current release to ensure smoother
> upgrades when active-active mode support is added in subsequent
> releases.
>
> Schema changes:
> - Added 'id' column to Network_Function table (integer, 1-255)
> - Added unique index on 'id' column alongside existing 'name'
>   index
>
> Changes to ovn-nbctl:
> - Updated nf-add command to require ID parameter:
>   nf-add NETWORK-FUNCTION ID PORT-IN PORT-OUT
> - Modified nf-list to display the ID field
> - Updated related documentation and tests
>
> Signed-off-by: Aditya Mehakare <[email protected]>
> Acked-by: Naveen Yerramneni <[email protected]>
> ---
>  ovn-nb.ovsschema          | 10 +++++---
>  ovn-nb.xml                |  5 ++++
>  tests/ovn-nbctl.at        | 48 +++++++++++++++++++++++++++------------
>  tests/ovn-northd.at       |  8 +++----
>  tests/ovn.at              |  6 ++---
>  tests/system-ovn.at       |  4 ++--
>  utilities/ovn-nbctl.8.xml | 19 ++++++++--------
>  utilities/ovn-nbctl.c     | 25 +++++++++++++++-----
>  8 files changed, 84 insertions(+), 41 deletions(-)
>
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 8c2c1d861..2c60d000c 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Northbound",
> -    "version": "7.15.0",
> -    "cksum": "4060410729 43708",
> +    "version": "7.16.0",
> +    "cksum": "3182666148 43912",
>      "tables": {
>          "NB_Global": {
>              "columns": {
> @@ -213,10 +213,14 @@
>                              "refTable": "Network_Function_Health_Check",
>                              "refType": "strong"},
>                      "min": 0, "max": 1}},
> +                "id": {
> +                     "type": {"key": {"type": "integer",
> +                                      "minInteger": 1,
> +                                      "maxInteger": 255}}},
>                  "external_ids": {
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}}},
> -            "indexes": [["name"]],
> +            "indexes": [["name"], ["id"]],
>              "isRoot": true},
>          "Network_Function_Group": {
>              "columns": {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e9ca27413..2b4ab51f5 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -6409,6 +6409,11 @@ or
>        Name of the <ref table="Network_Function"/>. Name should be unique.
>      </column>
>
> +    <column name="id">
> +      A unique integer between 1 and 255 must be assigned to each
> +      <code>Network_Function</code>.
> +    </column>
> +
>      <column name="inport">
>        <ref table="Logical_Switch_Port"/>  where request traffic for 
> from-lport
>        ACL and response traffic for to-lport ACL is redirected.
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index dccf30758..f4cb89b82 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -3245,13 +3245,13 @@ AT_CHECK([check ovn-nbctl set logical_switch_port 
> svc-port1 \
>      options:is-nf=true options:nf-linked-port=svc-port0])
>
>  # Create network-function.
> -AT_CHECK([ovn-nbctl nf-add nf0 svc-port0 svc-port1])
> -AT_CHECK([ovn-nbctl nf-add nf0 svc-port0 svc-port1], [1], [],
> +AT_CHECK([ovn-nbctl nf-add nf0 1 svc-port0 svc-port1])
> +AT_CHECK([ovn-nbctl nf-add nf0 1 svc-port0 svc-port1], [1], [],
>    [ovn-nbctl: nf0: same name network-function already exists
>  ])
> -AT_CHECK([ovn-nbctl --may-exist nf-add nf0 svc-port0 svc-port1])
> +AT_CHECK([ovn-nbctl --may-exist nf-add nf0 1 svc-port0 svc-port1])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> -<0> (nf0) in:svc-port0 out:svc-port1
> +<0> (nf0) id:1 in:svc-port0 out:svc-port1
>  ])
>
>  # Test --may-exist overwrite behavior: update existing network function with 
> new ports
> @@ -3263,25 +3263,25 @@ AT_CHECK([check ovn-nbctl set logical_switch_port 
> svc-port4 \
>  AT_CHECK([check ovn-nbctl set logical_switch_port svc-port5 \
>      options:receive_multicast=false options:lsp_learn_fdb=false \
>      options:is-nf=true options:nf-linked-port=svc-port4])
> -AT_CHECK([ovn-nbctl --may-exist nf-add nf0 svc-port4 svc-port5])
> +AT_CHECK([ovn-nbctl --may-exist nf-add nf0 1 svc-port4 svc-port5])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> -<0> (nf0) in:svc-port4 out:svc-port5
> +<0> (nf0) id:1 in:svc-port4 out:svc-port5
>  ])
>
>  # Create two more network-functions, one with same inport and outport.
>  AT_CHECK([check ovn-nbctl lsp-add ls0 svc-port2])
>  AT_CHECK([check ovn-nbctl lsp-add ls0 svc-port3])
> -AT_CHECK([ovn-nbctl nf-add nf1 svc-port2 svc-port3])
> +AT_CHECK([ovn-nbctl nf-add nf1 2 svc-port2 svc-port3])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> -<0> (nf0) in:svc-port4 out:svc-port5
> -<1> (nf1) in:svc-port2 out:svc-port3
> +<0> (nf0) id:1 in:svc-port4 out:svc-port5
> +<1> (nf1) id:2 in:svc-port2 out:svc-port3
>  ])
>
> -AT_CHECK([ovn-nbctl nf-add nf2 svc-port2 svc-port2])
> +AT_CHECK([ovn-nbctl nf-add nf2 3 svc-port2 svc-port2])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> -<0> (nf0) in:svc-port4 out:svc-port5
> -<1> (nf1) in:svc-port2 out:svc-port3
> -<2> (nf2) in:svc-port2 out:svc-port2
> +<0> (nf0) id:1 in:svc-port4 out:svc-port5
> +<1> (nf1) id:2 in:svc-port2 out:svc-port3
> +<2> (nf2) id:3 in:svc-port2 out:svc-port2
>  ])
>
>  # Create a network-function-group.
> @@ -3334,10 +3334,30 @@ AT_CHECK([ovn-nbctl nfg-list | uuidfilt], [0], [])
>  AT_CHECK([ovn-nbctl nf-del nf1])
>  AT_CHECK([ovn-nbctl nf-del nf0])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> -<0> (nf2) in:svc-port2 out:svc-port2
> +<0> (nf2) id:3 in:svc-port2 out:svc-port2
>  ])
>  AT_CHECK([ovn-nbctl nf-del nf2])
>  AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [])
> +
> +# Test ID validation and uniqueness
> +AT_CHECK([check ovn-nbctl lsp-add ls0 svc-port6])
> +AT_CHECK([check ovn-nbctl lsp-add ls0 svc-port7])
> +# Test invalid ID (out of range)
> +AT_CHECK([ovn-nbctl nf-add nf3 0 svc-port6 svc-port7], [1], [],
> +  [ovn-nbctl: network-function id must be between 1 and 255
> +])
> +AT_CHECK([ovn-nbctl nf-add nf3 256 svc-port6 svc-port7], [1], [],
> +  [ovn-nbctl: network-function id must be between 1 and 255
> +])
> +AT_CHECK([ovn-nbctl nf-add nf3 abc svc-port6 svc-port7], [1], [],
> +  [ovn-nbctl: network-function id must be between 1 and 255
> +])
> +# Test valid ID
> +AT_CHECK([ovn-nbctl nf-add nf3 10 svc-port6 svc-port7])
> +AT_CHECK([ovn-nbctl nf-list | uuidfilt], [0], [dnl
> +<0> (nf3) id:10 in:svc-port6 out:svc-port7
> +])
> +AT_CHECK([ovn-nbctl nf-del nf3])
>  ])
>
>  AT_SETUP([ovn-nbctl - TLS server name indication (SNI) with 
> --ssl-server-name])
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 512e42036..4ccfad7bb 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -18441,7 +18441,7 @@ check ovn-nbctl set logical_switch_port sw0-nf-p1 \
>  check ovn-nbctl set logical_switch_port sw0-nf-p2 \
>      options:receive_multicast=false options:lsp_learn_mac=false \
>      options:is-nf=true options:nf-linked-port=sw0-nf-p1
> -check ovn-nbctl nf-add nf0 sw0-nf-p1 sw0-nf-p2
> +check ovn-nbctl nf-add nf0 1 sw0-nf-p1 sw0-nf-p2
>  check ovn-nbctl nfg-add nfg0 1 inline nf0
>
>  check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 
> "00:00:00:00:00:01 10.0.0.2"
> @@ -18554,7 +18554,7 @@ check ovn-nbctl set logical_switch_port sw0-nf-p3 \
>  check ovn-nbctl set logical_switch_port sw0-nf-p4 \
>      options:receive_multicast=false options:lsp_learn_mac=false \
>      options:is-nf=true options:nf-linked-port=sw0-nf-p3
> -check ovn-nbctl nf-add nf1 sw0-nf-p3 sw0-nf-p4
> +check ovn-nbctl nf-add nf1 2 sw0-nf-p3 sw0-nf-p4
>  check ovn-nbctl nfg-add nfg1 2 inline nf1
>  check ovn-nbctl acl-add pg0 to-lport 1003 "outport == @pg0 && ip4.src == 
> 10.0.0.4" allow-related nfg1
>  check ovn-sbctl lsp-bind sw0-nf-p3 hv1
> @@ -18732,8 +18732,8 @@ check ovn-nbctl set logical_switch_port $nfsw-p4 \
>      options:receive_multicast=false options:lsp_learn_fdb=false \
>      options:is-nf=true options:nf-linked-port=$nfsw-p3
>
> -check ovn-nbctl nf-add nf0 $nfsw-p1 $nfsw-p2
> -check ovn-nbctl nf-add nf1 $nfsw-p3 $nfsw-p4
> +check ovn-nbctl nf-add nf0 1 $nfsw-p1 $nfsw-p2
> +check ovn-nbctl nf-add nf1 2 $nfsw-p3 $nfsw-p4
>  nf0_uuid=$(fetch_column nb:network_function _uuid name=nf0)
>  nf1_uuid=$(fetch_column nb:network_function _uuid name=nf1)
>  AT_CHECK(
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4c1abb045..5bb0489ba 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -35851,7 +35851,7 @@ check ovs-vsctl add-port br-int ls0-hv -- set 
> Interface ls0-hv external-ids:ifac
>  check ovn-nbctl lr-add lr0
>
>  check ovn-nbctl ls-add ls0
> -check ovn-nbctl lsp-add ls0 ls0-lr0
> +check ovn-nbctl lsp-add ls0 ls0-lr0
>  check ovn-nbctl lsp-set-type ls0-lr0 router
>  check ovn-nbctl lsp-set-addresses ls0-lr0 router
>  check ovn-nbctl lrp-add lr0 lr0-ls0 00:00:00:00:00:01 10.0.0.1
> @@ -43598,7 +43598,7 @@ create_logical_topology() {
>      check ovn-nbctl set logical_switch_port $sw-nf-p2 \
>          options:receive_multicast=false options:lsp_learn_mac=false \
>          options:is-nf=true options:nf-linked-port=$sw-nf-p1
> -    check ovn-nbctl nf-add nf0 $sw-nf-p1 $sw-nf-p2
> +    check ovn-nbctl nf-add nf0 1 $sw-nf-p1 $sw-nf-p2
>      check ovn-nbctl nfg-add nfg0 1 inline nf0
>      check ovn-nbctl pg-add pg0 $sw-p1
>      check ovn-nbctl acl-add pg0 from-lport 1002 "inport == @pg0 && ip4.dst 
> == 192.168.0.12" allow-related nfg0
> @@ -43788,7 +43788,7 @@ create_logical_topology() {
>      check ovn-nbctl set logical_switch_port $sw-nf-p2 \
>          options:receive_multicast=false options:lsp_learn_mac=false \
>          options:is-nf=true options:nf-linked-port=$sw-nf-p1
> -    check ovn-nbctl nf-add nf0 $sw-nf-p1 $sw-nf-p2
> +    check ovn-nbctl nf-add nf0 1 $sw-nf-p1 $sw-nf-p2
>      check ovn-nbctl nfg-add nfg0 1 inline nf0
>      check ovn-nbctl pg-add pg0 $sw-p1
>      check ovn-nbctl acl-add pg0 to-lport 1002 "outport == @pg0 && ip4.src == 
> 192.168.0.12" allow-related nfg0
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 67f03e3be..c217540c0 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -19529,7 +19529,7 @@ check ovn-nbctl set logical_switch_port child-4 
> options:receive_multicast=false
>
>  AS_BOX([Test-1: Single NF without health check])
>
> -check ovn-nbctl nf-add nf0 nf-p1 nf-p2
> +check ovn-nbctl nf-add nf0 1 nf-p1 nf-p2
>  nf0_uuid=$(fetch_column nb:network_function _uuid name=nf0)
>  check ovn-nbctl nfg-add nfg0 1 inline nf0
>  nfg_uuid=$(fetch_column nb:network_function_group _uuid name=nfg0)
> @@ -19631,7 +19631,7 @@ validate_single_nf_no_health_check "server" "client" 
> "192.168.1.10" "Outbound"
>  AS_BOX([Test-2: Two NFs with health check config enabled])
>
>  # Add second NF
> -check ovn-nbctl nf-add nf1 nf-p3 nf-p4
> +check ovn-nbctl nf-add nf1 2 nf-p3 nf-p4
>  nf1_uuid=$(fetch_column nb:network_function _uuid name=nf1)
>
>  # Add bridge for nf1
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index 7df902944..253f6d8fc 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -475,16 +475,17 @@
>      <h2>Network Function Commands</h2>
>
>      <dl>
> -      <dt>[<code>--may-exist</code>] <code>nf-add</code> <var>nf</var> 
> <var>inport</var> <var>outport</var></dt>
> +      <dt>[<code>--may-exist</code>] <code>nf-add</code> <var>nf</var> 
> <var>id</var> <var>inport</var> <var>outport</var></dt>
>        <dd>
>          <p>
> -          Creates a new network function named <var>nf</var> with logical
> -          switch ports <var>inport</var> and <var>outport</var>. Both the
> -          ports must be on the same logical switch and must be already
> -          created. When used in an ACL action, traffic matching the ACL
> -          are redirected to the <var>inport</var> if it is from-lport ACL
> -          and to the <var>outport</var> if it is to-lport ACL. The response
> -          packets are sent through the same ports in reverse order.
> +          Creates a new network function named <var>nf</var> with the 
> specified
> +          <var>id</var> (an integer between 1 and 255) and logical switch 
> ports
> +          <var>inport</var> and <var>outport</var>. Both the ports must be on
> +          the same logical switch and must be already created. When used in 
> an
> +          ACL action, traffic matching the ACL are redirected to the
> +          <var>inport</var> if it is from-lport ACL and to <var>outport</var>
> +          if it is to-lport ACL. The response packets are sent through the 
> same
> +          ports in reverse order.
>          </p>
>
>          <p>
> @@ -1498,7 +1499,7 @@
>           The optional argument <var>protocol</var> must be either
>           <code>tcp</code>,  <code>udp</code> or <code>sctp</code>. This 
> argument
>           is useful when a port number is provided as part of the 
> <var>vip</var>.
> -         If the <var>protocol</var> is unspecified and a port number is 
> provided
> +         If the <var>protocol</var> is unspecified and a port number is 
> provided
>           as part of the <var>vip</var>, OVN assumes the <var>protocol</var> 
> to
>           be <code>tcp</code>.
>          </p>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index cdf6b578a..2b674cad3 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -393,7 +393,7 @@ Network function group commands:\n\
>                              network-function-group\n\
>  \n\
>  Network function commands:\n\
> -  nf-add NETWORK-FUNCTION PORT-IN PORT-OUT\n\
> +  nf-add NETWORK-FUNCTION ID PORT-IN PORT-OUT\n\
>                             create a network-function\n\
>    nf-del NETWORK-FUNCTION  delete a network-function\n\
>    nf-list                  print all network-functions\n\
> @@ -2483,6 +2483,7 @@ nbctl_pre_nf_add(struct ctl_context *ctx)
>
>      ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_name);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_name);
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_id);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_inport);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_outport);
>  }
> @@ -2495,17 +2496,25 @@ nbctl_nf_add(struct ctl_context *ctx)
>
>      bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
>
> -    char * error = lsp_by_name_or_uuid(ctx, ctx->argv[2], true, &lsp_in);
> +    char * error = lsp_by_name_or_uuid(ctx, ctx->argv[3], true, &lsp_in);
>      if (error) {
>          ctx->error = error;
>          return;
>      }
> -    error = lsp_by_name_or_uuid(ctx, ctx->argv[3], true, &lsp_out);
> +    error = lsp_by_name_or_uuid(ctx, ctx->argv[4], true, &lsp_out);
>      if (error) {
>          ctx->error = error;
>          return;
>      }
>
> +    /* Validate and parse ID */
> +    int64_t nf_id = 0;
> +    if (!ovs_scan(ctx->argv[2], "%"SCNd64, &nf_id)
> +            || nf_id < 1 || nf_id > 255) {
> +        ctl_error(ctx, "network-function id must be between 1 and 255");
> +        return;
> +    }
> +
>      const char *nf_name = ctx->argv[1];
>
>      /* Check if network function already exists */
> @@ -2528,6 +2537,9 @@ nbctl_nf_add(struct ctl_context *ctx)
>          nbrec_network_function_set_name(nf, nf_name);
>      }
>
> +    /* Set ID */
> +    nbrec_network_function_set_id(nf, nf_id);
> +
>      /* Set/update the ports */
>      nbrec_network_function_set_inport(nf, lsp_in);
>      nbrec_network_function_set_outport(nf, lsp_out);
> @@ -2560,6 +2572,7 @@ static void
>  nbctl_pre_nf_list(struct ctl_context *ctx)
>  {
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_name);
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_id);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_inport);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_network_function_col_outport);
>      ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_port_col_name);
> @@ -2577,9 +2590,9 @@ nbctl_nf_list(struct ctl_context *ctx)
>          const char *linport_name = linport ? linport->name : "<not_set>";
>          const char *loutport_name = loutport ? loutport->name : "<not_set>";
>          smap_add_format(&nfs, nf->name,
> -                        UUID_FMT " (%s) in:%s out:%s",
> +                        UUID_FMT " (%s) id:%"PRId64" in:%s out:%s",
>                          UUID_ARGS(&nf->header_.uuid),
> -                        nf->name, linport_name, loutport_name);
> +                        nf->name, nf->id, linport_name, loutport_name);
>      }
>      const struct smap_node **nodes = smap_sort(&nfs);
>      for (size_t i = 0; i < smap_count(&nfs); i++) {
> @@ -8877,7 +8890,7 @@ static const struct ctl_command_syntax nbctl_commands[] 
> = {
>        nbctl_nf_group_del_network_function, NULL, "--if-exists", RW },
>
>      /* network-function commands. */
> -    { "nf-add", 3, 3, "NETWORK-FUNCTION PORT-IN PORT-OUT",
> +    { "nf-add", 4, 4, "NETWORK-FUNCTION ID PORT-IN PORT-OUT",
>        nbctl_pre_nf_add,
>        nbctl_nf_add,
>        NULL, "--may-exist", RW },
> --
> 2.43.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to