On Wed, Nov 3, 2021 at 3:17 PM svc.eng.git-mail
<[email protected]> wrote:
>
> Hi Lorenzo,
>
> > what will it happen if pinctrl learns a different MAC for the given IP 
> > after the CMS has
> configured the static binding? Will it be overwritten? If so I guess we need a
> flag to avoid it.
>
> 1. Static binding will be overwritten by pinctrl.
> 2. Northd will again overwrite this and propagate the value that is 
> statically configured. Eventual state of the system is the configuration from 
> CMS.
>
> If I have to introduce a flag to prioritize one over the other, I also need 
> to remember the state as to which entries are static.
>
> Option 1:
> - NB MAC_Binding has new columns: is_static: boolean, 
> options:allow_override=boolean
> - SB MAC_Binding has new columns: is_static: boolean, 
> options:allow_override=boolean
> - pinctrl logic would look something like
> sb->is_static && sb->allow_override {
>   update_mac
> }
> - build_mac_binding_table will have to change. When an entry is removed from 
> NB it has to also cleanup the corresponding SB entry in order to not leave 
> behind and static entries. Something like what build_datapaths is doing. I 
> think this iteration is expensive. Linear in terms of the number of 
> MAC_Binding entries.
>
> Option 2:
> - Add a separate SB Static_MAC_Binding table. Now the iteration is linear in 
> terms of the number of Static_MAC_Binding entries.
> - Have the get_arp function look at both SB tables and prioritize based on 
> some flag.
>
> What do you recommend? Do you have any alternatives in mind?

With your present proposed patch,  as you mentioned, eventually
ovn-northd will update the mac binding entry
with the configured value if ovn-controller updates the entry with a
different mac.  This should be fine IMO.
However this can cause a continuous churn if the external entity keeps
using a different mac than the one
configured by CMS if ovn-controller sees the ARP packets.

I think a flag could be beneficial to specify the precedence. Having a
separate mac binding table as you mentioned in (2) makes sense.

ovn-controller adds the mac lookup flows in tables -
OFTABLE_MAC_BINDING and OFTABLE_MAC_LOOKUP.
ovn-controller can add higher priority flows for the
'Static_MAC_Binding' table entries (or the MAC_Binding table)
depending on the flag.

Thanks
Numan

>
> Thank You,
> Karthik C
>
> On 11/3/21, 10:13 AM, "Lorenzo Bianconi" <[email protected]> wrote:
> > Hi Lorenzo,
> >
> > Thanks for the review. Do we need a flag to control statically added MACs 
> > from CMS vs dynamically learnt MAC from pinctrl for a given IP?
>
> Yes, I think so.
>
> >
> > Typically static configurations for a MAC Table override dynamically learnt 
> > MACs. In this patch even if pinctrl learns a different MAC for the given 
> > IP, statically configured MAC will eventually be programmed.
>
> what will it happen if pinctrl learns a different MAC for the given IP after 
> the CMS has
> configured the static binding? Will it be overwritten? If so I guess we need a
> flag to avoid it.
>
> Regards,
> Lorenzo
>
> >
> > Thank You,
> > Karthik C
> >
> > On 10/25/21, 2:53 AM, "Lorenzo Bianconi" 
> > <[email protected]<mailto:[email protected]>> wrote:
> > > From: Karthik Chandrashekar 
> > > <[email protected]<mailto:[email protected]><mailto:[email protected]><mailto:[email protected]%3e>>
> > >
> >
> > [...]
> > >
> > > +static void
> > > +build_mac_binding_table(struct northd_context *ctx, struct hmap *ports)
> > > +{
> > > +    const struct nbrec_mac_binding *nb_mac_binding;
> > > +    NBREC_MAC_BINDING_FOR_EACH (nb_mac_binding, ctx->ovnnb_idl) {
> > > +        struct ovn_port *op = ovn_port_find(
> > > +            ports, nb_mac_binding->logical_port);
> > > +        if (op && op->nbrp) {
> > > +            struct ovn_datapath *od = op->od;
> > > +            if (od && od->sb) {
> > > +                const struct sbrec_mac_binding *mb =
> > > +                    
> > > mac_binding_lookup(ctx->sbrec_mac_binding_by_lport_ip,
> > > +                                       nb_mac_binding->logical_port,
> > > +                                       nb_mac_binding->ip);
> > > +                if (!mb) {
> > > +                    mb = sbrec_mac_binding_insert(ctx->ovnsb_txn);
> > > +                    sbrec_mac_binding_set_logical_port(
> > > +                        mb, nb_mac_binding->logical_port);
> > > +                    sbrec_mac_binding_set_ip(mb, nb_mac_binding->ip);
> > > +                    sbrec_mac_binding_set_mac(mb, nb_mac_binding->mac);
> > > +                    sbrec_mac_binding_set_datapath(mb, od->sb);
> > > +                } else {
> > > +                    sbrec_mac_binding_set_mac(mb, nb_mac_binding->mac);
> > > +                }
> >
> > I guess we should add some flags here to consider the case both the user/CMS
> > adds a static mac binding and pinctrl discovers the same IP address. What do
> > you think?
> >
> > > +            }
> > > +        }
> > > +    }
> > > +}
> > > +
> > >  /* Returns a string of the IP address of the router port 'op' that
> > >   * overlaps with 'ip_s".  If one is not found, returns NULL.
> > >   *
> > > @@ -14465,6 +14495,7 @@ ovnnb_db_run(struct northd_context *ctx,
> > >      build_mcast_groups(ctx, datapaths, ports, &mcast_groups, 
> > > &igmp_groups);
> > >      build_meter_groups(ctx, &meter_groups);
> > >      build_bfd_table(ctx, &bfd_connections, ports);
> > > +    build_mac_binding_table(ctx, ports);
> > >      stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
> > >      stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
> > >      build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups,
> > > diff --git a/northd/northd.h b/northd/northd.h
> > > index ffa2bbb4e..59f407332 100644
> > > --- a/northd/northd.h
> > > +++ b/northd/northd.h
> > > @@ -27,6 +27,7 @@ struct northd_context {
> > >      struct ovsdb_idl_index *sbrec_ha_chassis_grp_by_name;
> > >      struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp;
> > >      struct ovsdb_idl_index *sbrec_ip_mcast_by_dp;
> > > +    struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip;
> > >
> > >      bool use_parallel_build;
> > >  };
> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > index 39aa96055..5fba98812 100644
> > > --- a/northd/ovn-northd.c
> > > +++ b/northd/ovn-northd.c
> > > @@ -23,6 +23,7 @@
> > >  #include "daemon.h"
> > >  #include "fatal-signal.h"
> > >  #include "lib/ip-mcast-index.h"
> > > +#include "lib/mac-binding-index.h"
> > >  #include "lib/mcast-group-index.h"
> > >  #include "memory.h"
> > >  #include "northd.h"
> > > @@ -904,6 +905,9 @@ main(int argc, char *argv[])
> > >      struct ovsdb_idl_index *sbrec_ip_mcast_by_dp
> > >          = ip_mcast_index_create(ovnsb_idl_loop.idl);
> > >
> > > +    struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip
> > > +        = mac_binding_index_create(ovnsb_idl_loop.idl);
> > > +
> > >      unixctl_command_register("sb-connection-status", "", 0, 0,
> > >                               ovn_conn_show, ovnsb_idl_loop.idl);
> > >
> > > @@ -959,6 +963,7 @@ main(int argc, char *argv[])
> > >                  .sbrec_ha_chassis_grp_by_name = 
> > > sbrec_ha_chassis_grp_by_name,
> > >                  .sbrec_mcast_group_by_name_dp = 
> > > sbrec_mcast_group_by_name_dp,
> > >                  .sbrec_ip_mcast_by_dp = sbrec_ip_mcast_by_dp,
> > > +                .sbrec_mac_binding_by_lport_ip = 
> > > sbrec_mac_binding_by_lport_ip,
> > >                  .use_parallel_build = use_parallel_build,
> > >              };
> > >
> > > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > > index 99772932d..0e96b5de1 100644
> > > --- a/northd/ovn_northd.dl
> > > +++ b/northd/ovn_northd.dl
> > > @@ -1018,6 +1018,15 @@ sb::Out_MAC_Binding (._uuid        = mb._uuid,
> > >      sb::Out_Port_Binding(.logical_port = mb.logical_port),
> > >      sb::Out_Datapath_Binding(._uuid = mb.datapath).
> > >
> > > +sb::Out_MAC_Binding (._uuid        = hash,
> > > +                    .logical_port = nb.logical_port,
> > > +                    .ip           = nb.ip,
> > > +                    .mac          = nb.mac,
> > > +                    .datapath     = router._uuid) :-
> > > +    nb in nb::MAC_Binding(),
> > > +    rp in &RouterPort(.router = router, .json_name = 
> > > json_escape(nb.logical_port)),
> > > +    var hash = hash128((nb.logical_port, nb.ip)).
> > > +
> > >  /*
> > >   * DHCP options: fixed table
> > >   */
> > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > > index 5dee04fe9..27dc91438 100644
> > > --- a/ovn-nb.ovsschema
> > > +++ b/ovn-nb.ovsschema
> > > @@ -1,7 +1,7 @@
> > >  {
> > >      "name": "OVN_Northbound",
> > > -    "version": "5.33.1",
> > > -    "cksum": "1931852754 30731",
> > > +    "version": "5.34.0",
> > > +    "cksum": "1872933846 30998",
> > >      "tables": {
> > >          "NB_Global": {
> > >              "columns": {
> > > @@ -595,5 +595,12 @@
> > >                      "type": {"key": "string", "value": "string",
> > >                               "min": 0, "max": "unlimited"}}},
> > >              "indexes": [["logical_port", "dst_ip"]],
> > > -            "isRoot": true}}
> > > +            "isRoot": true},
> > > +        "MAC_Binding": {
> > > +            "columns": {
> > > +                "logical_port": {"type": "string"},
> > > +                "ip": {"type": "string"},
> > > +                "mac": {"type": "string"}},
> > > +            "indexes": [["logical_port", "ip"]],
> > > +             "isRoot": true}}
> >
> > nit: fix indentation here
> >
> > > Add a new NB MAC_Binding table. This allows programming of
> > > static mac_bindings for a logical_router. OVN northd is
> > > responsible for propagating these static entries to the
> > > existing SB MAC_Binding table.
> > >
> > > Signed-off-by: Karthik Chandrashekar 
> > > <[email protected]<mailto:[email protected]><mailto:[email protected]><mailto:[email protected]%3e>>
> > > ---
> > >  controller/pinctrl.c    |  20 +----
> > >  lib/automake.mk         |   2 +
> > >  lib/mac-binding-index.c |  43 +++++++++
> > >  lib/mac-binding-index.h |  29 ++++++
> > >  northd/northd.c         |  31 +++++++
> > >  northd/northd.h         |   1 +
> > >  northd/ovn-northd.c     |   5 ++
> > >  northd/ovn_northd.dl    |   9 ++
> > >  ovn-nb.ovsschema        |  13 ++-
> > >  ovn-nb.xml              |  25 ++++++
> > >  tests/ovn-nbctl.at      |  69 ++++++++++++++
> > >  tests/ovn-northd.at     |  26 ++++++
> > >  utilities/ovn-nbctl.c   | 193 +++++++++++++++++++++++++++++++++++++++-
> > >  13 files changed, 441 insertions(+), 25 deletions(-)
> > >  create mode 100644 lib/mac-binding-index.c
> > >  create mode 100644 lib/mac-binding-index.h
> > >
> > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> > > index 7d01b18a3..af12909fa 100644
> > > --- a/controller/pinctrl.c
> > > +++ b/controller/pinctrl.c
> > > @@ -48,6 +48,7 @@
> > >  #include "ovn/lex.h"
> > >  #include "lib/acl-log.h"
> > >  #include "lib/ip-mcast-index.h"
> > > +#include "lib/mac-binding-index.h"
> > >  #include "lib/mcast-group-index.h"
> > >  #include "lib/ovn-l7.h"
> > >  #include "lib/ovn-util.h"
> > > @@ -4090,25 +4091,6 @@ send_mac_binding_buffered_pkts(struct rconn 
> > > *swconn)
> > >      ovs_list_init(&buffered_mac_bindings);
> > >  }
> > >
> > > -static const struct sbrec_mac_binding *
> > > -mac_binding_lookup(struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> > > -                   const char *logical_port,
> > > -                   const char *ip)
> > > -{
> > > -    struct sbrec_mac_binding *mb = sbrec_mac_binding_index_init_row(
> > > -        sbrec_mac_binding_by_lport_ip);
> > > -    sbrec_mac_binding_index_set_logical_port(mb, logical_port);
> > > -    sbrec_mac_binding_index_set_ip(mb, ip);
> > > -
> > > -    const struct sbrec_mac_binding *retval
> > > -        = sbrec_mac_binding_index_find(sbrec_mac_binding_by_lport_ip,
> > > -                                       mb);
> > > -
> > > -    sbrec_mac_binding_index_destroy_row(mb);
> > > -
> > > -    return retval;
> > > -}
> > > -
> > >  /* Update or add an IP-MAC binding for 'logical_port'.
> > >   * Caller should make sure that 'ovnsb_idl_txn' is valid. */
> > >  static void
> > > diff --git a/lib/automake.mk b/lib/automake.mk
> > > index 9f9f447d5..b58acf1d0 100644
> > > --- a/lib/automake.mk
> > > +++ b/lib/automake.mk
> > > @@ -21,6 +21,8 @@ lib_libovn_la_SOURCES = \
> > >             lib/ovn-parallel-hmap.c \
> > >             lib/ip-mcast-index.c \
> > >             lib/ip-mcast-index.h \
> > > +          lib/mac-binding-index.c \
> > > +          lib/mac-binding-index.h \
> > >             lib/mcast-group-index.c \
> > >             lib/mcast-group-index.h \
> > >             lib/lex.c \
> > > diff --git a/lib/mac-binding-index.c b/lib/mac-binding-index.c
> > > new file mode 100644
> > > index 000000000..1d34dfbdc
> > > --- /dev/null
> > > +++ b/lib/mac-binding-index.c
> > > @@ -0,0 +1,43 @@
> > > +/* Copyright (c) 2021
> > > + *
> > > + * 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/mac-binding-index.h"
> > > +#include "lib/ovn-sb-idl.h"
> > > +
> > > +struct ovsdb_idl_index *
> > > +mac_binding_index_create(struct ovsdb_idl *idl)
> > > +{
> > > +    return ovsdb_idl_index_create2(idl,
> > > +                                   &sbrec_mac_binding_col_logical_port,
> > > +                                   &sbrec_mac_binding_col_ip);
> > > +}
> > > +
> > > +const struct sbrec_mac_binding *
> > > +mac_binding_lookup(struct ovsdb_idl_index *mac_binding_index,
> > > +                   const char *logical_port, const char *ip)
> > > +{
> > > +    struct sbrec_mac_binding *target = sbrec_mac_binding_index_init_row(
> > > +        mac_binding_index);
> > > +    sbrec_mac_binding_index_set_logical_port(target, logical_port);
> > > +    sbrec_mac_binding_index_set_ip(target, ip);
> > > +
> > > +    struct sbrec_mac_binding *mac_binding =
> > > +        sbrec_mac_binding_index_find(mac_binding_index, target);
> > > +    sbrec_mac_binding_index_destroy_row(target);
> > > +
> > > +    return mac_binding;
> > > +}
> > > diff --git a/lib/mac-binding-index.h b/lib/mac-binding-index.h
> > > new file mode 100644
> > > index 000000000..d7434f688
> > > --- /dev/null
> > > +++ b/lib/mac-binding-index.h
> > > @@ -0,0 +1,29 @@
> > > +/* Copyright (c) 2021
> > > + *
> > > + * 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.
> > > + */
> > > +
> > > +#ifndef OVN_MAC_BINDING_INDEX_H
> > > +#define OVN_MAC_BINDING_INDEX_H 1
> > > +
> > > +struct ovsdb_idl;
> > > +
> > > +struct sbrec_datapath_binding;
> > > +
> > > +struct ovsdb_idl_index *mac_binding_index_create(struct ovsdb_idl *);
> > > +const struct sbrec_mac_binding *mac_binding_lookup(
> > > +    struct ovsdb_idl_index *mac_binding_index,
> > > +    const char *logical_port,
> > > +    const char *ip);
> > > +
> > > +#endif /* lib/mac-binding-index.h */
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index 0bf66e0b2..b2270fd0e 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -29,6 +29,7 @@
> > >  #include "lib/chassis-index.h"
> > >  #include "lib/ip-mcast-index.h"
> > >  #include "lib/copp.h"
> > > +#include "lib/mac-binding-index.h"
> > >  #include "lib/mcast-group-index.h"
> > >  #include "lib/ovn-l7.h"
> > >  #include "lib/ovn-nb-idl.h"
> > > @@ -8368,6 +8369,35 @@ build_bfd_table(struct northd_context *ctx, struct 
> > > hmap *bfd_connections,
> > >      bitmap_free(bfd_src_ports);
> > >  }
> > >      }
> > > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > > index e31578fb6..327039d95 100644
> > > --- a/ovn-nb.xml
> > > +++ b/ovn-nb.xml
> > > @@ -4130,4 +4130,29 @@
> > >        </column>
> > >      </group>
> > >    </table>
> > > +
> > > +  <table name="MAC_Binding">
> > > +    <p>
> > > +      Each record represents a MAC Binding entry.
> > > +    </p>
> > > +
> > > +    <group title="Configuration">
> > > +      <p>
> > > +        <code>ovn-northd</code> reads configuration from these columns
> > > +        and propagates the value to SBDB.
> > > +      </p>
> > > +
> > > +      <column name="logical_port">
> > > +        The logical port on which the binding was discovered.
> > > +      </column>
> > > +
> > > +      <column name="ip">
> > > +        The bound IP address.
> > > +      </column>
> > > +
> > > +      <column name="mac">
> > > +        The Ethernet address to which the IP is bound.
> > > +      </column>
> > > +    </group>
> > > +  </table>
> > >  </database>
> > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> > > index 9b80ae410..673c9637d 100644
> > > --- a/tests/ovn-nbctl.at
> > > +++ b/tests/ovn-nbctl.at
> > > @@ -2057,6 +2057,75 @@ AT_CHECK([ovn-nbctl list forwarding_group], [0], 
> > > [])
> > >
> > >  dnl ---------------------------------------------------------------------
> > >
> > > +OVN_NBCTL_TEST([ovn_nbctl_mac_binding], [lr mac_binding], [
> > > +
> > > +AT_CHECK([ovn-nbctl lr-add lr0])
> > > +AT_CHECK([ovn-nbctl lrp-add lr0 lr0-p0 00:00:01:01:02:03 
> > > 192.168.10.1/24])
> > > +AT_CHECK([ovn-nbctl lrp-add lr0 lr0-p1 00:00:02:02:03:04 
> > > 192.168.11.1/24])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 192.168.10.10 
> > > 00:00:11:22:33:44])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 192.168.10.100 
> > > 00:00:22:33:44:55])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 10.0.0.10 00:00:33:44:55:66])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 172.16.0.11 
> > > 00:00:44:55:66:88])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 foo 00:00:44:55:66:88], [1], 
> > > [],
> > > +  [ovn-nbctl: foo: Not a valid IPv4 or IPv6 address.
> > > +])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 172.16.0.200 foo], [1], [],
> > > +  [ovn-nbctl: invalid mac address foo.
> > > +])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p0 172.16.0.11 
> > > 00:00:44:55:66:77], [1], [],
> > > +  [ovn-nbctl: lr0-p0, 172.16.0.11: a MAC_Binding with this logical_port 
> > > and ip already exists
> > > +])
> > > +
> > > +AT_CHECK([ovn-nbctl --may-exist mac-binding-add lr0-p0 172.16.0.11 
> > > 00:00:44:55:66:77])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p1 10.0.0.10 00:00:33:44:55:66])
> > > +AT_CHECK([ovn-nbctl mac-binding-add lr0-p1 172.16.0.11 
> > > 00:00:44:55:66:88])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-list], [0], [dnl
> > > +LOGICAL_PORT             IP                       MAC
> > > +lr0-p0                   10.0.0.10                00:00:33:44:55:66
> > > +lr0-p0                   172.16.0.11              00:00:44:55:66:77
> > > +lr0-p0                   192.168.10.10            00:00:11:22:33:44
> > > +lr0-p0                   192.168.10.100           00:00:22:33:44:55
> > > +lr0-p1                   10.0.0.10                00:00:33:44:55:66
> > > +lr0-p1                   172.16.0.11              00:00:44:55:66:88
> > > +])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-del lr0-p0 foo], [1], [],
> > > +  [ovn-nbctl: foo: Not a valid IPv4 or IPv6 address.
> > > +])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-del lr0-p1 10.0.0.100], [1], [],
> > > +  [ovn-nbctl: no matching MAC_Binding with port (lr0-p1) and ip 
> > > (10.0.0.100)
> > > +])
> > > +
> > > +AT_CHECK([ovn-nbctl --if-exists mac-binding-del lr0-p1 10.0.0.100])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-del lr0-p0 10.0.0.10])
> > > +AT_CHECK([ovn-nbctl mac-binding-del lr0-p0 192.168.10.100])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-list], [0], [dnl
> > > +LOGICAL_PORT             IP                       MAC
> > > +lr0-p0                   172.16.0.11              00:00:44:55:66:77
> > > +lr0-p0                   192.168.10.10            00:00:11:22:33:44
> > > +lr0-p1                   10.0.0.10                00:00:33:44:55:66
> > > +lr0-p1                   172.16.0.11              00:00:44:55:66:88
> > > +])
> > > +
> > > +AT_CHECK([ovn-nbctl mac-binding-del lr0-p1 10.0.0.10])
> > > +AT_CHECK([ovn-nbctl mac-binding-list], [0], [dnl
> > > +LOGICAL_PORT             IP                       MAC
> > > +lr0-p0                   172.16.0.11              00:00:44:55:66:77
> > > +lr0-p0                   192.168.10.10            00:00:11:22:33:44
> > > +lr0-p1                   172.16.0.11              00:00:44:55:66:88
> > > +])
> > > +
> > > +])
> > > +
> > > +dnl ---------------------------------------------------------------------
> > > +
> > >  OVN_NBCTL_TEST([ovn_nbctl_negative], [basic negative tests], [
> > >  AT_CHECK([ovn-nbctl --id=@ls create logical_switch name=foo -- \
> > >            set logical_switch foo1 name=bar],
> > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > > index 8b9049899..2e6a51f2e 100644
> > > --- a/tests/ovn-northd.at
> > > +++ b/tests/ovn-northd.at
> > > @@ -5401,3 +5401,29 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep 
> > > cr-DR | sed 's/table=../table=??
> > >
> > >  AT_CLEANUP
> > >  ])
> > > +
> > > +OVN_FOR_EACH_NORTHD([
> > > +AT_SETUP([LR NB MAC_Binding table])
> > > +ovn_start
> > > +
> > > +# Create logical routers
> > > +ovn-nbctl lr-add lr0
> > > +ovn-nbctl lrp-add lr0 lr0-p0 00:00:01:01:02:03 192.168.10.1/24
> > > +ovn-nbctl lrp-add lr0 lr0-p1 00:00:02:02:03:04 192.168.11.1/24
> > > +
> > > +ovn-nbctl mac-binding-add lr0-p0 192.168.10.10 00:00:11:22:33:44
> > > +ovn-nbctl mac-binding-add lr0-p0 192.168.10.100 00:00:22:33:44:55
> > > +
> > > +wait_row_count nb:MAC_Binding 2 logical_port=lr0-p0
> > > +wait_row_count MAC_Binding 1 logical_port=lr0-p0 ip=192.168.10.10 
> > > mac="00\:00\:11\:22\:33\:44"
> > > +wait_row_count MAC_Binding 1 logical_port=lr0-p0 ip=192.168.10.100 
> > > mac="00\:00\:22\:33\:44\:55"
> > > +
> > > +ovn-nbctl mac-binding-add lr0-p1 10.0.0.10 00:00:33:44:55:66
> > > +wait_row_count nb:MAC_Binding 1 logical_port=lr0-p1
> > > +wait_row_count MAC_Binding 1 logical_port=lr0-p1 ip=10.0.0.10 
> > > mac="00\:00\:33\:44\:55\:66"
> > > +
> > > +ovn-nbctl --may-exist mac-binding-add lr0-p0 192.168.10.100 
> > > 00:00:22:33:55:66
> > > +wait_row_count MAC_Binding 1 logical_port=lr0-p0 ip=192.168.10.100 
> > > mac="00\:00\:22\:33\:55\:66"
> > > +
> > > +AT_CLEANUP
> > > +])
> > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > > index b04b24d4b..527fe1644 100644
> > > --- a/utilities/ovn-nbctl.c
> > > +++ b/utilities/ovn-nbctl.c
> > > @@ -365,7 +365,8 @@ Policy commands:\n\
> > >    lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\
> > >                              remove policies from ROUTER\n\
> > >    lr-policy-list ROUTER     print policies for ROUTER\n\
> > > -\n\
> > > +\n\n",program_name, program_name);
> > > +    printf("\
> > >  NAT commands:\n\
> > >    [--stateless]\n\
> > >    [--portrange]\n\
> > > @@ -408,8 +409,7 @@ Connection commands:\n\
> > >    del-connection             delete the connections\n\
> > >    [--inactivity-probe=MSECS]\n\
> > >    set-connection TARGET...   set the list of connections to TARGET...\n\
> > > -\n\n",program_name, program_name);
> > > -    printf("\
> > > +\n\
> > >  SSL commands:\n\
> > >    get-ssl                     print the SSL configuration\n\
> > >    del-ssl                     delete the SSL configuration\n\
> > > @@ -450,6 +450,13 @@ Control Plane Protection Policy commands:\n\
> > >                              List all copp policies defined for control\n\
> > >                              protocols on ROUTER.\n\
> > >  \n\
> > > +MAC_Binding commands:\n\
> > > +  mac-binding-add LOGICAL_PORT IP MAC \n\
> > > +                                    Add a MAC_Binding entry\n\
> > > +  mac-binding-del LOGICAL_PORT IP \n\
> > > +                                    Delete MAC_Binding entry\n\
> > > +  mac-binding-list                  List all MAC_Binding \n\
> > > +\n\
> > >  %s\
> > >  %s\
> > >  \n\
> > > @@ -5602,6 +5609,176 @@ nbctl_lrp_get_redirect_type(struct ctl_context 
> > > *ctx)
> > >                    !redirect_type ? "overlay": redirect_type);
> > >  }
> > >
> > > +static const struct nbrec_mac_binding *
> > > +mac_binding_by_port_ip(struct ctl_context *ctx,
> > > +                           const char *logical_port, const char *ip)
> > > +{
> > > +    const struct nbrec_mac_binding *nb_mac_binding = NULL;
> > > +
> > > +    NBREC_MAC_BINDING_FOR_EACH(nb_mac_binding, ctx->idl) {
> > > +        if (!strcmp(nb_mac_binding->logical_port, logical_port) &&
> > > +            !strcmp(nb_mac_binding->ip, ip)) {
> > > +            break;
> > > +        }
> > > +    }
> > > +
> > > +    return nb_mac_binding;
> > > +}
> > > +
> > > +static void
> > > +nbctl_pre_mac_binding_add(struct ctl_context *ctx)
> > > +{
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
> > > +
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_logical_port);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_ip);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_mac);
> > > +}
> > > +
> > > +static void
> > > +nbctl_mac_binding_add(struct ctl_context *ctx)
> > > +{
> > > +    const char *logical_port = ctx->argv[1];
> > > +    const char *ip = ctx->argv[2];
> > > +    const char *mac = ctx->argv[3];
> > > +    char *new_ip = NULL;
> > > +
> > > +    const struct nbrec_logical_router_port *lrp;
> > > +    char *error = lrp_by_name_or_uuid(ctx, logical_port, true, &lrp);
> > > +    if (error) {
> > > +        ctx->error = error;
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    new_ip = normalize_addr_str(ip);
> > > +    if (!new_ip) {
> > > +        ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address.", ip);
> > > +        return;
> > > +    }
> > > +
> > > +    struct eth_addr ea;
> > > +    if (!eth_addr_from_string(mac, &ea)) {
> > > +        ctl_error(ctx, "invalid mac address %s.", mac);
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
> > > +    const struct nbrec_mac_binding *nb_mac_binding = 
> > > mac_binding_by_port_ip(
> > > +        ctx, logical_port, ip);
> > > +    if (nb_mac_binding) {
> > > +        char *old_ip;
> > > +        bool should_return = false;
> > > +        old_ip = normalize_addr_str(nb_mac_binding->ip);
> > > +
> > > +        if (!strcmp(nb_mac_binding->logical_port, logical_port)) {
> > > +            if (!strcmp(old_ip, new_ip)) {
> > > +                if (may_exist) {
> > > +                    nbrec_mac_binding_verify_mac(nb_mac_binding);
> > > +                    nbrec_mac_binding_set_mac(nb_mac_binding, mac);
> > > +                    should_return = true;
> > > +                } else {
> > > +                    ctl_error(ctx, "%s, %s: a MAC_Binding with this "
> > > +                              "logical_port and ip already exists",
> > > +                              logical_port, new_ip);
> > > +                    should_return = true;
> > > +                }
> > > +            }
> > > +        }
> > > +        free(old_ip);
> > > +        if (should_return) {
> > > +            goto cleanup;
> > > +        }
> > > +    }
> > > +
> > > +    /* Create MAC_Binding entry */
> > > +    nb_mac_binding = nbrec_mac_binding_insert(ctx->txn);
> > > +    nbrec_mac_binding_set_logical_port(nb_mac_binding, logical_port);
> > > +    nbrec_mac_binding_set_ip(nb_mac_binding, new_ip);
> > > +    nbrec_mac_binding_set_mac(nb_mac_binding, mac);
> > > +
> > > +cleanup:
> > > +    free(new_ip);
> > > +}
> > > +
> > > +static void
> > > +nbctl_pre_mac_binding_del(struct ctl_context *ctx)
> > > +{
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
> > > +
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_logical_port);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_ip);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_mac);
> > > +}
> > > +
> > > +static void
> > > +nbctl_mac_binding_del(struct ctl_context *ctx)
> > > +{
> > > +    bool must_exist = !shash_find(&ctx->options, "--if-exists");
> > > +    const char *logical_port = ctx->argv[1];
> > > +    const struct nbrec_logical_router_port *lrp;
> > > +    char *error = lrp_by_name_or_uuid(ctx, logical_port, true, &lrp);
> > > +    if (error) {
> > > +        ctx->error = error;
> > > +        return;
> > > +    }
> > > +
> > > +    char *ip = normalize_addr_str(ctx->argv[2]);
> > > +    if (!ip) {
> > > +        ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address.", 
> > > ctx->argv[2]);
> > > +        return;
> > > +    }
> > > +
> > > +    const struct nbrec_mac_binding *nb_mac_binding = 
> > > mac_binding_by_port_ip(
> > > +        ctx, logical_port, ip);
> > > +
> > > +    if (nb_mac_binding) {
> > > +        /* Remove the matching MAC_Binding. */
> > > +        nbrec_mac_binding_delete(nb_mac_binding);
> > > +        goto cleanup;
> > > +    }
> > > +
> > > +    if (must_exist) {
> > > +        ctl_error(ctx, "no matching MAC_Binding with port (%s) and ip 
> > > (%s)",
> > > +                  logical_port, ip);
> > > +    }
> > > +
> > > +cleanup:
> > > +    free(ip);
> > > +}
> > > +
> > > +static void
> > > +nbctl_pre_mac_binding_list(struct ctl_context *ctx)
> > > +{
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_name);
> > > +
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_logical_port);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_ip);
> > > +    ovsdb_idl_add_column(ctx->idl, &nbrec_mac_binding_col_mac);
> > > +}
> > > +
> > > +static void
> > > +nbctl_mac_binding_list(struct ctl_context *ctx)
> > > +{
> > > +    struct smap lr_mac_bindings = SMAP_INITIALIZER(&lr_mac_bindings);
> > > +    const struct nbrec_mac_binding *nb_mac_binding = NULL;
> > > +    NBREC_MAC_BINDING_FOR_EACH(nb_mac_binding, ctx->idl) {
> > > +        char *key = xasprintf("%-25s%-25s", nb_mac_binding->logical_port,
> > > +                              nb_mac_binding->ip);
> > > +        smap_add_format(&lr_mac_bindings, key, "%s", 
> > > nb_mac_binding->mac);
> > > +        free(key);
> > > +    }
> > > +
> > > +    const struct smap_node **nodes = smap_sort(&lr_mac_bindings);
> > > +    if (nodes) {
> > > +        ds_put_format(&ctx->output, "%-25s%-25s%s\n",
> > > +                      "LOGICAL_PORT", "IP", "MAC");
> > > +        for (size_t i = 0; i < smap_count(&lr_mac_bindings); i++) {
> > > +            const struct smap_node *node = nodes[i];
> > > +            ds_put_format(&ctx->output, "%-25s%s\n", node->key, 
> > > node->value);
> > > +        }
> > > +    }
> > > +}
> > > +
> > >  static const struct nbrec_forwarding_group *
> > >  fwd_group_by_name_or_uuid(struct ctl_context *ctx, const char *id)
> > >  {
> > > @@ -7063,6 +7240,16 @@ static const struct ctl_command_syntax 
> > > nbctl_commands[] = {
> > >       pre_ha_ch_grp_set_chassis_prio, cmd_ha_ch_grp_set_chassis_prio, 
> > > NULL,
> > >       "", RW },
> > >
> > > +    /* MAC_Binding commands */
> > > +    { "mac-binding-add", 3, 3, "LOGICAL_PORT IP MAC",
> > > +      nbctl_pre_mac_binding_add, nbctl_mac_binding_add, NULL,
> > > +      "--may-exist", RW },
> > > +    { "mac-binding-del", 2, 2, "LOGICAL_PORT IP",
> > > +      nbctl_pre_mac_binding_del, nbctl_mac_binding_del,
> > > +      NULL, "--if-exists", RW },
> > > +    { "mac-binding-list", 0, 1, "[LOGICAL_PORT]",
> > > +      nbctl_pre_mac_binding_list, nbctl_mac_binding_list, NULL, "", RO },
> > > +
> > >      {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO},
> > >  };
> > >
> > > --
> > > 2.25.1
> > >
> > > _______________________________________________
> > > dev mailing list
> > > [email protected]<mailto:[email protected]><mailto:[email protected]>
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
>
> _______________________________________________
> 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