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

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Thu Oct 14 16:56:04 UTC 2021


On Oct 11, Mark Gray wrote:
> 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.

I guess we can continue with Karthik's version in this case. What do you think?

Regards,
Lorenzo

> 
> > 
> > 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