[ovs-dev] [PATCH ovn] sbctl: add the capability to configure static mac_bindings entries

Mark Gray mark.d.gray at redhat.com
Mon Oct 11 10:37:44 UTC 2021


On 28/09/2021 19:28, Lorenzo Bianconi wrote:
> Introduce the two following commands to configure and dump static
> L2 address bindings:
> 
> $ovn-sbctl set-mac-binding 192.168.1.100 00:11:22:33:44:55 vif0
> $ovn-sbctl list-mac-binding
> [ 192.168.1.100 00:11:22:33:44:55 S ]

Is there any reason why this is an `ovn-sbctl` command rather than an
`ovn-nbctl` command?

As the CMS should only really be interacting with OVN via the NBDB, I
think this should be configured via ovn-nbctl and should form part of
the northbound database somewhere.

> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1845111
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
>  controller/pinctrl.c  |  3 +-
>  ovn-sb.ovsschema      |  7 +++--
>  ovn-sb.xml            |  3 ++
>  tests/ovn-sbctl.at    | 39 +++++++++++++++++++++++++
>  utilities/ovn-sbctl.c | 68 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 116 insertions(+), 4 deletions(-)
> 
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index cc3edaaf4..c56e800cf 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -4135,7 +4135,8 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn *ovnsb_idl_txn,
>          sbrec_mac_binding_set_ip(b, ip);
>          sbrec_mac_binding_set_mac(b, mac_string);
>          sbrec_mac_binding_set_datapath(b, dp);
> -    } else if (strcmp(b->mac, mac_string)) {
> +        sbrec_mac_binding_set_static_(b, false);
> +    } else if (strcmp(b->mac, mac_string) && !b->static_) {
>          sbrec_mac_binding_set_mac(b, mac_string);
>      }
>  }
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index e5ab41db9..0eb9df19c 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Southbound",
> -    "version": "20.20.0",
> -    "cksum": "605270161 26670",
> +    "version": "20.21.0",
> +    "cksum": "4055366137 26717",
>      "tables": {
>          "SB_Global": {
>              "columns": {
> @@ -241,7 +241,8 @@
>                  "ip": {"type": "string"},
>                  "mac": {"type": "string"},
>                  "datapath": {"type": {"key": {"type": "uuid",
> -                                              "refTable": "Datapath_Binding"}}}},
> +                                              "refTable": "Datapath_Binding"}}},
> +                "static": {"type": "boolean"}},
>              "indexes": [["logical_port", "ip"]],
>              "isRoot": true},
>          "DHCP_Options": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 2d4d47d10..e6c74a328 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3378,6 +3378,9 @@ tcp.flags = RST;
>      <column name="datapath">
>        The logical datapath to which the logical port belongs.
>      </column>
> +    <column name="static">
> +      The entry has been manually created by the CMS and not discovered by OVN.
> +    </column>
>    </table>
>  
>    <table name="DHCP_Options" title="DHCP Options supported by native OVN DHCP">
> diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
> index 1d1eee23e..ea343ff84 100644
> --- a/tests/ovn-sbctl.at
> +++ b/tests/ovn-sbctl.at
> @@ -243,3 +243,42 @@ Total number of logical flows = 0
>  ])
>  ])
>  
> +dnl ---------------------------------------------------------------------
> +
> +OVN_SBCTL_TEST([ovn_sbctl_mac_bindings], [ovn-sbctl - mac-bindings], [
> +AT_CHECK([ovn-nbctl ls-add sw0])
> +AT_CHECK([ovn-nbctl lsp-add sw0 vif0])
> +AT_CHECK([ovn-nbctl lsp-set-addresses vif0 "f0:ab:cd:ef:01:02 192.168.1.2"])
> +AT_CHECK([ovn-nbctl --wait=sb sync])
> +
> +AT_CHECK([ovn-sbctl set-mac-binding 192.168.1.100 00:11:22:33:44:55 vif0])
> +AT_CHECK([ovn-sbctl list-mac-binding], [0], [dnl
> +[[ 192.168.1.100 00:11:22:33:44:55 S ]]
> +])
> +
> +AT_CHECK([ovn-sbctl set-mac-binding 192.168.1.101 00:11:22:33:44:66 vif0])
> +AT_CHECK([ovn-sbctl list-mac-binding |sort], [0], [dnl
> +[[ 192.168.1.100 00:11:22:33:44:55 S ]]
> +[[ 192.168.1.101 00:11:22:33:44:66 S ]]
> +])
> +
> +# sanity checks
> +AT_CHECK([ovn-sbctl set-mac-binding 192.168.1.101 00:11:22:33:44:66 vif1])
> +AT_CHECK([ovn-sbctl list-mac-binding |sort], [0], [dnl
> +[[ 192.168.1.100 00:11:22:33:44:55 S ]]
> +[[ 192.168.1.101 00:11:22:33:44:66 S ]]
> +])
> +
> +AT_CHECK([ovn-sbctl set-mac-binding 192.168.1.101.2 00:11:22:33:44:66 vif0])
> +AT_CHECK([ovn-sbctl list-mac-binding |sort], [0], [dnl
> +[[ 192.168.1.100 00:11:22:33:44:55 S ]]
> +[[ 192.168.1.101 00:11:22:33:44:66 S ]]
> +])
> +
> +AT_CHECK([ovn-sbctl set-mac-binding 192.168.1.102 00:11:22:33:44:66:77 vif0])
> +AT_CHECK([ovn-sbctl list-mac-binding |sort], [0], [dnl
> +[[ 192.168.1.100 00:11:22:33:44:55 S ]]
> +[[ 192.168.1.101 00:11:22:33:44:66 S ]]
> +])
> +])
> +
> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c
> index 4d7e7d670..6d581887b 100644
> --- a/utilities/ovn-sbctl.c
> +++ b/utilities/ovn-sbctl.c
> @@ -333,6 +333,7 @@ pre_get_info(struct ctl_context *ctx)
>      ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_logical_port);
>      ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_ip);
>      ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_mac);
> +    ovsdb_idl_add_column(ctx->idl, &sbrec_mac_binding_col_static_);
>  
>      ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_datapaths);
>      ovsdb_idl_add_column(ctx->idl, &sbrec_load_balancer_col_vips);
> @@ -1176,6 +1177,67 @@ sbctl_ip_mcast_flush(struct ctl_context *ctx)
>      }
>  }
>  
> +static void
> +sbctl_list_mac_binding(struct ctl_context *ctx)
> +{
> +    const struct sbrec_mac_binding *iter;
> +    SBREC_MAC_BINDING_FOR_EACH (iter, ctx->idl) {
> +        ds_put_format(&ctx->output, "[ %s %s %s ]\n",
> +                      iter->ip, iter->mac,
> +                      iter->static_ ? "S" : "R");

You will probably need to update the man page.

What does 'R' stand for?

> +    }
> +}
> +
> +static void
> +sbctl_set_mac_binding(struct ctl_context *ctx)
> +{
> +    if (ctx->argc != 4) {
> +        return;
> +    }
> +
> +    /* sanity checks. */
> +    struct in6_addr ipv6;
> +    ovs_be32 ip;
> +    if (!ip_parse(ctx->argv[1], &ip) && !ipv6_parse(ctx->argv[1], &ipv6)) {
> +        return;
> +    }
> +
> +    struct eth_addr ea;
> +    int index = 0;
> +    if (!ovs_scan_len(ctx->argv[2], &index, ETH_ADDR_SCAN_FMT,
> +                      ETH_ADDR_SCAN_ARGS(ea)) ||
> +        index != strlen(ctx->argv[2])) {
> +        return;
> +    }
> +
> +    const struct sbrec_port_binding *pb_iter, *pb = NULL;
> +    SBREC_PORT_BINDING_FOR_EACH (pb_iter, ctx->idl) {
> +        if (!strcmp(pb_iter->logical_port, ctx->argv[3])) {
> +            pb = pb_iter;
> +            break;
> +        }
> +    }
> +    if (!pb) {
> +        return;
> +    }
> +
> +    const struct sbrec_mac_binding *mb_iter, *mb = NULL;
> +    SBREC_MAC_BINDING_FOR_EACH (mb_iter, ctx->idl) {
> +        if (!strcmp(mb_iter->ip, ctx->argv[1])) {
> +            mb = mb_iter;
> +            break;
> +        }
> +    }
> +    if (!mb) {
> +        mb = sbrec_mac_binding_insert(ctx->txn);
> +        sbrec_mac_binding_set_ip(mb, ctx->argv[1]);
> +    }
> +    sbrec_mac_binding_set_mac(mb, ctx->argv[2]);
> +    sbrec_mac_binding_set_logical_port(mb, ctx->argv[3]);
> +    sbrec_mac_binding_set_datapath(mb, pb->datapath);
> +    sbrec_mac_binding_set_static_(mb, true);
> +}
> +
>  static void
>  verify_connections(struct ctl_context *ctx)
>  {
> @@ -1482,6 +1544,12 @@ static const struct ctl_command_syntax sbctl_commands[] = {
>      {"ip-multicast-flush", 0, 1, "SWITCH",
>       pre_get_info, sbctl_ip_mcast_flush, NULL, "", RW },
>  
> +    /* Mac bindings commands. */
> +    {"list-mac-binding", 0, 0, "", pre_get_info, sbctl_list_mac_binding,
> +     NULL, "", RO},
> +    {"set-mac-binding", 3, 3, "IP MAC PORT", pre_get_info,
> +     sbctl_set_mac_binding, NULL, "", RW},
> +
>      /* Connection commands. */
>      {"get-connection", 0, 0, "", pre_connection, cmd_get_connection, NULL, "", RO},
>      {"del-connection", 0, 0, "", pre_connection, cmd_del_connection, NULL, "", RW},
> 



More information about the dev mailing list