[ovs-dev] [PATCH v2 ovn 2/4] ovn-northd: Add support for CoPP.

Mark Gray mark.d.gray at redhat.com
Thu Jun 3 14:27:25 UTC 2021


On 24/05/2021 19:44, Lorenzo Bianconi wrote:
> From: Dumitru Ceara <dceara at redhat.com>
> 
> Add new 'Copp' (Control plane protection) table to OVN Northbound DB:
> - this stores mappings between control plane protocol names and meters
>   that should be used to rate limit controller-destined traffic for
>   those protocols.
> 
> Add new 'copp' columns to the following OVN Northbound DB tables:
> - Logical_Switch
> - Logical_Router
> 
> For now, no control plane protection policy is installed for any of
> the existing flows that punt packets to ovn-controller. This will be
> added in follow-up patches.
> 
> Add CLI commands in 'ovn-nbctl' to allow the user to manage Control
> Plane Protection Policies at different levels (logical switch,
> logical router).
> 
> Co-authored-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
>  lib/automake.mk           |   2 +
>  lib/copp.c                | 142 ++++++++++++++++++++++++++++++
>  lib/copp.h                |  58 +++++++++++++
>  northd/ovn-northd.c       |  44 +++++++---
>  ovn-nb.ovsschema          |  16 +++-
>  ovn-nb.xml                |  78 +++++++++++++++++
>  tests/ovn-controller.at   |  48 ++++++++++
>  tests/ovn-northd.at       |  81 +++++++++++++++++
>  utilities/ovn-nbctl.8.xml | 116 +++++++++++++++++++++++++
>  utilities/ovn-nbctl.c     | 178 ++++++++++++++++++++++++++++++++++++++
>  10 files changed, 751 insertions(+), 12 deletions(-)
>  create mode 100644 lib/copp.c
>  create mode 100644 lib/copp.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 781be2109..20e296fff 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -9,6 +9,8 @@ lib_libovn_la_SOURCES = \
>  	lib/actions.c \
>  	lib/chassis-index.c \
>  	lib/chassis-index.h \
> +	lib/copp.c \
> +	lib/copp.h \
>  	lib/ovn-dirs.h \
>  	lib/expr.c \
>  	lib/extend-table.h \
> diff --git a/lib/copp.c b/lib/copp.c
> new file mode 100644
> index 000000000..e3d14938a
> --- /dev/null
> +++ b/lib/copp.c
> @@ -0,0 +1,142 @@
> +/* Copyright (c) 2021, Red Hat, 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 <stdlib.h>
> +
> +#include "openvswitch/shash.h"
> +#include "db-ctl-base.h"
> +#include "smap.h"
> +#include "lib/ovn-nb-idl.h"
> +#include "lib/copp.h"
> +
> +static char *copp_proto_names[COPP_PROTO_MAX] = {
> +    [COPP_ARP]           = "arp",
> +    [COPP_ARP_RESOLVE]   = "arp-resolve",
> +    [COPP_DHCPV4_OPTS]   = "dhcpv4-opts",
> +    [COPP_DHCPV6_OPTS]   = "dhcpv6-opts",
> +    [COPP_DNS]           = "dns",
> +    [COPP_EVENT_ELB]     = "event-elb",
> +    [COPP_ICMP4_ERR]     = "icmp4-error",
> +    [COPP_ICMP6_ERR]     = "icmp6-error",
> +    [COPP_IGMP]          = "igmp",
> +    [COPP_ND_NA]         = "nd-na",
> +    [COPP_ND_NS]         = "nd-ns",
> +    [COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
> +    [COPP_ND_RA_OPTS]    = "nd-ra-opts",
> +    [COPP_TCP_RESET]     = "tcp-reset",
> +    [COPP_BFD]           = "bfd",
> +};
> +
> +static const char *
> +copp_proto_get_name(enum copp_proto proto)
> +{
> +    if (proto >= COPP_PROTO_MAX) {
> +        return "<Invalid control protocol ID>";
> +    }
> +    return copp_proto_names[proto];
> +}
> +
> +const char *
> +copp_meter_get(enum copp_proto proto, const struct nbrec_copp *copp,
> +               const struct shash *meter_groups)
> +{
> +    if (!copp || proto >= COPP_PROTO_MAX) {
> +        return NULL;
> +    }
> +
> +    const char *meter = smap_get(&copp->meters, copp_proto_names[proto]);
> +
> +    if (meter && shash_find(meter_groups, meter)) {
> +        return meter;
> +    }
> +
> +    return NULL;
> +}
> +
> +void
> +copp_meter_list(struct ctl_context *ctx, const struct nbrec_copp *copp)
> +{
> +    if (!copp) {
> +        return;
> +    }
> +
> +    struct smap_node *node;
> +
> +    SMAP_FOR_EACH (node, &copp->meters) {
> +        ds_put_format(&ctx->output, "%s: %s\n", node->key, node->value);
> +    }
> +}
> +
> +const struct nbrec_copp *
> +copp_meter_add(struct ctl_context *ctx, const struct nbrec_copp *copp,
> +               const char *proto_name, const char *meter)
> +{
> +    if (!copp) {
> +        copp = nbrec_copp_insert(ctx->txn);
> +    }
> +
> +    struct smap meters;
> +    smap_init(&meters);
> +    smap_clone(&meters, &copp->meters);
> +    smap_replace(&meters, proto_name, meter);
> +    nbrec_copp_set_meters(copp, &meters);
> +    smap_destroy(&meters);
> +
> +    return copp;
> +}
> +
> +void
> +copp_meter_del(const struct nbrec_copp *copp, const char *proto_name)
> +{
> +    if (!copp) {
> +        return;
> +    }
> +
> +    if (proto_name) {
> +        if (smap_get(&copp->meters, proto_name)) {
> +            struct smap meters;
> +            smap_init(&meters);
> +            smap_clone(&meters, &copp->meters);
> +            smap_remove(&meters, proto_name);
> +            nbrec_copp_set_meters(copp, &meters);
> +            smap_destroy(&meters);
> +        }
> +    } else {
> +        nbrec_copp_delete(copp);
> +    }
> +}
> +
> +char *
> +copp_proto_validate(const char *proto_name)
> +{
> +    for (size_t i = COPP_PROTO_FIRST; i < COPP_PROTO_MAX; i++) {
> +        if (!strcmp(proto_name, copp_proto_get_name(i))) {
> +            return NULL;
> +        }
> +    }
> +
> +    struct ds usage = DS_EMPTY_INITIALIZER;
> +
> +    ds_put_cstr(&usage, "Invalid control protocol. Allowed values: ");
> +    for (size_t i = COPP_PROTO_FIRST; i < COPP_PROTO_MAX; i++) {
> +        ds_put_format(&usage, "%s, ", copp_proto_get_name(i));
> +    }
> +    ds_chomp(&usage, ' ');
> +    ds_chomp(&usage, ',');
> +    ds_put_cstr(&usage, ".");
> +
> +    return ds_steal_cstr(&usage);
> +}
> diff --git a/lib/copp.h b/lib/copp.h
> new file mode 100644
> index 000000000..c34e1e029
> --- /dev/null
> +++ b/lib/copp.h
> @@ -0,0 +1,58 @@
> +/* Copyright (c) 2021, Red Hat, 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.
> + */
> +
> +#ifndef OVN_COPP_H
> +#define OVN_COPP_H 1
> +
> +/*
> + * Control plane protection - metered actions.
> + */
> +enum copp_proto {
> +    COPP_PROTO_FIRST,
> +    COPP_ARP = COPP_PROTO_FIRST,
> +    COPP_ARP_RESOLVE,
> +    COPP_DHCPV4_OPTS,
> +    COPP_DHCPV6_OPTS,
> +    COPP_DNS,
> +    COPP_EVENT_ELB,
> +    COPP_ICMP4_ERR,
> +    COPP_ICMP6_ERR,
> +    COPP_IGMP,
> +    COPP_ND_NA,
> +    COPP_ND_NS,
> +    COPP_ND_NS_RESOLVE,
> +    COPP_ND_RA_OPTS,
> +    COPP_TCP_RESET,
> +    COPP_BFD,
> +    COPP_PROTO_MAX,
> +    COPP_PROTO_INVALID = COPP_PROTO_MAX,
> +};
> +
> +struct nbrec_copp;
> +struct ctl_context;
> +
> +const char *copp_meter_get(enum copp_proto proto,
> +                           const struct nbrec_copp *copp,
> +                           const struct shash *meter_groups);
> +
> +void copp_meter_list(struct ctl_context *ctx, const struct nbrec_copp *copp);
> +const struct nbrec_copp *
> +copp_meter_add(struct ctl_context *ctx, const struct nbrec_copp *copp,
> +               const char *proto_name, const char *meter);
> +void
> +copp_meter_del(const struct nbrec_copp *copp, const char *proto_name);
> +char * copp_proto_validate(const char *proto_name);
> +
> +#endif /* lib/copp.h */
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 0e5092a87..de44ecf0a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -32,6 +32,7 @@
>  #include "ovn/lex.h"
>  #include "lib/chassis-index.h"
>  #include "lib/ip-mcast-index.h"
> +#include "lib/copp.h"
>  #include "lib/mcast-group-index.h"
>  #include "lib/ovn-l7.h"
>  #include "lib/ovn-nb-idl.h"
> @@ -4018,6 +4019,7 @@ struct ovn_lflow {
>      char *match;
>      char *actions;
>      char *stage_hint;
> +    char *ctrl_meter;
>      const char *where;
>  };
>  
> @@ -4051,14 +4053,15 @@ ovn_lflow_equal(const struct ovn_lflow *a, const struct ovn_lflow *b)
>              && a->stage == b->stage
>              && a->priority == b->priority
>              && !strcmp(a->match, b->match)
> -            && !strcmp(a->actions, b->actions));
> +            && !strcmp(a->actions, b->actions)
> +            && nullable_string_is_equal(a->ctrl_meter, b->ctrl_meter));
>  }
>  
>  static void
>  ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
>                 enum ovn_stage stage, uint16_t priority,
> -               char *match, char *actions, char *stage_hint,
> -               const char *where)
> +               char *match, char *actions, char *ctrl_meter,
> +               char *stage_hint, const char *where)
>  {
>      hmapx_init(&lflow->od_group);
>      lflow->od = od;
> @@ -4067,6 +4070,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
>      lflow->match = match;
>      lflow->actions = actions;
>      lflow->stage_hint = stage_hint;
> +    lflow->ctrl_meter = ctrl_meter;
>      lflow->where = where;
>  }
>  
> @@ -4106,6 +4110,7 @@ static void
>  ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
>                   enum ovn_stage stage, uint16_t priority,
>                   const char *match, const char *actions, bool shared,
> +                 const char *ctrl_meter,
>                   const struct ovsdb_idl_row *stage_hint, const char *where)
>  {
>      ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
> @@ -4119,6 +4124,7 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
>       * one datapath in a group, so it could be hashed correctly. */
>      ovn_lflow_init(lflow, NULL, stage, priority,
>                     xstrdup(match), xstrdup(actions),
> +                   nullable_xstrdup(ctrl_meter),
>                     ovn_lflow_hint(stage_hint), where);
>  
>      hash = ovn_lflow_hash(lflow);
> @@ -4133,14 +4139,24 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
>  }
>  
>  /* Adds a row with the specified contents to the Logical_Flow table. */
> +#define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
> +                                  ACTIONS, CTRL_METER, STAGE_HINT) \
> +    ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \
> +                     CTRL_METER, STAGE_HINT, OVS_SOURCE_LOCATOR)
> +
>  #define ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
>                                  ACTIONS, STAGE_HINT) \
> -    ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \
> -                     STAGE_HINT, OVS_SOURCE_LOCATOR)
> +    ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
> +                              ACTIONS, NULL, STAGE_HINT)
>  
>  #define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
>      ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \
> -                     NULL, OVS_SOURCE_LOCATOR)
> +                     NULL, NULL, OVS_SOURCE_LOCATOR)
> +
> +#define ovn_lflow_add_ctrl(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
> +                           CTRL_METER) \
> +    ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
> +                              ACTIONS, CTRL_METER, NULL)
>  
>  /* Adds a row with the specified contents to the Logical_Flow table.
>   * Combining of this logical flow with already existing ones, e.g., by using
> @@ -4155,21 +4171,22 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
>  #define ovn_lflow_add_unique_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
>                                         ACTIONS, STAGE_HINT) \
>      ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
> -                     STAGE_HINT, OVS_SOURCE_LOCATOR)
> +                     NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
>  
>  #define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
>      ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
> -                     NULL, OVS_SOURCE_LOCATOR)
> +                     NULL, NULL, OVS_SOURCE_LOCATOR)
>  
>  static struct ovn_lflow *
>  ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
>                 enum ovn_stage stage, uint16_t priority,
> -               const char *match, const char *actions, uint32_t hash)
> +               const char *match, const char *actions, const char *ctrl_meter,
> +               uint32_t hash)
>  {
>      struct ovn_lflow target;
>      ovn_lflow_init(&target, od, stage, priority,
>                     CONST_CAST(char *, match), CONST_CAST(char *, actions),
> -                   NULL, NULL);
> +                   CONST_CAST(char *, ctrl_meter), NULL, NULL);
>  
>      return ovn_lflow_find_by_lflow(lflows, &target, hash);
>  }
> @@ -4185,6 +4202,7 @@ ovn_lflow_destroy(struct hmap *lflows, struct ovn_lflow *lflow)
>          free(lflow->match);
>          free(lflow->actions);
>          free(lflow->stage_hint);
> +        free(lflow->ctrl_meter);
>          free(lflow);
>      }
>  }
> @@ -12387,7 +12405,8 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>          lflow = ovn_lflow_find(
>              &lflows, logical_datapath_od,
>              ovn_stage_build(dp_type, pipeline, sbflow->table_id),
> -            sbflow->priority, sbflow->match, sbflow->actions, sbflow->hash);
> +            sbflow->priority, sbflow->match, sbflow->actions,
> +            sbflow->controller_meter, sbflow->hash);
>          if (lflow) {
>              /* This is a valid lflow.  Checking if the datapath group needs
>               * updates. */
> @@ -12432,6 +12451,7 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>          sbrec_logical_flow_set_priority(sbflow, lflow->priority);
>          sbrec_logical_flow_set_match(sbflow, lflow->match);
>          sbrec_logical_flow_set_actions(sbflow, lflow->actions);
> +        sbrec_logical_flow_set_controller_meter(sbflow, lflow->ctrl_meter);
>  
>          /* Trim the source locator lflow->where, which looks something like
>           * "ovn/northd/ovn-northd.c:1234", down to just the part following the
> @@ -14139,6 +14159,8 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_priority);
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_match);
>      add_column_noalert(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_actions);
> +    add_column_noalert(ovnsb_idl_loop.idl,
> +                       &sbrec_logical_flow_col_controller_meter);
>  
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl,
>                          &sbrec_table_logical_dp_group);
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index faf619a1c..a53d5e851 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>  {
>      "name": "OVN_Northbound",
>      "version": "5.32.0",
> -    "cksum": "204590300 28863",
> +    "cksum": "2501921026 29540",
>      "tables": {
>          "NB_Global": {
>              "columns": {
> @@ -30,6 +30,14 @@
>                  "ipsec": {"type": "boolean"}},
>              "maxRows": 1,
>              "isRoot": true},
> +        "Copp": {
> +            "columns": {
> +                "meters": {
> +                    "type": {"key": "string",
> +                             "value": "string",
> +                             "min": 0,
> +                             "max": "unlimited"}}},
> +            "isRoot": true},
>          "Logical_Switch": {
>              "columns": {
>                  "name": {"type": "string"},
> @@ -58,6 +66,9 @@
>                                           "refType": "weak"},
>                                    "min": 0,
>                                    "max": "unlimited"}},
> +                "copp": {"type": {"key": {"type": "uuid", "refTable": "Copp",
> +                                          "refType": "weak"},
> +                                  "min": 0, "max": 1}},
>                  "other_config": {
>                      "type": {"key": "string", "value": "string",
>                               "min": 0, "max": "unlimited"}},
> @@ -322,6 +333,9 @@
>                                                    "refType": "weak"},
>                                             "min": 0,
>                                             "max": "unlimited"}},
> +                "copp": {"type": {"key": {"type": "uuid", "refTable": "Copp",
> +                                          "refType": "weak"},
> +                                  "min": 0, "max": 1}},
>                  "options": {
>                       "type": {"key": "string",
>                                "value": "string",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 02fd21605..e6ec10bcf 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -354,6 +354,68 @@
>  
>    </table>
>  
> +  <table name="Copp" title="Control plane protection">
> +    <p>
> +      This table is used to define control plane protection policies, i.e.,
> +      associate entries from table <ref table="Meter"/> to control protocol
> +      names.
> +    </p>
> +    <column name="meters" key="arp">
> +      Rate limiting meter for ARP packets (request/reply) used for learning
> +      neighbors.
> +    </column>
> +    <column name="meters" key="arp-resolve">
> +      Rate limiting meter for packets that require resolving the next-hop
> +      (through ARP).
> +    </column>
> +    <column name="meters" key="dhcpv4-opts">
> +      Rate limiting meter for packets that require adding DHCPv4 options.
> +    </column>
> +    <column name="meters" key="dhcpv6-opts">
> +      Rate limiting meter for packets that require adding DHCPv6 options.
> +    </column>
> +    <column name="meters" key="dns">
> +      Rate limiting meter for DNS query packets that need to be replied to.
> +    </column>
> +    <column name="meters" key="event-elb">
> +      Rate limiting meter for empty load balancer events.
> +    </column>
> +    <column name="meters" key="icmp4-error">
> +      Rate limiting meter for packets that require replying with an ICMP
> +      error.
> +    </column>
> +    <column name="meters" key="icmp6-error">
> +      Rate limiting meter for packets that require replying with an ICMPv6
> +      error.
> +    </column>
> +    <column name="meters" key="igmp">
> +      Rate limiting meter for IGMP packets.
> +    </column>
> +    <column name="meters" key="nd-na">
> +      Rate limiting meter for ND neighbor advertisement packets used for
> +      learning neighbors.
> +    </column>
> +    <column name="meters" key="nd-ns">
> +      Rate limiting meter for ND neighbor solicitation packets used for
> +      learning neighbors.
> +    </column>
> +    <column name="meters" key="nd-ns-resolve">
> +      Rate limiting meter for packets that require resolving the next-hop
> +      (through ND).
> +    </column>
> +    <column name="meters" key="nd-ra-opts">
> +      Rate limiting meter for packets that require adding ND router
> +      advertisement options.
> +    </column>
> +    <column name="meters" key="tcp-reset">
> +      Rate limiting meter for packets that require replying with TCP RST
> +      packet.
> +    </column>
> +    <column name="meters" key="bfd">
> +      Rate limiting meter for BFD packets.
> +    </column>
> +  </table>
> +
>    <table name="Logical_Switch" title="L2 logical switch">
>      <p>
>        Each row represents one L2 logical switch.
> @@ -587,6 +649,14 @@
>        </column>
>      </group>
>  
> +    <column name="copp">
> +      <p>
> +        The control plane protection policy from table <ref table="Copp"/>
> +        used for metering packets sent to <code>ovn-controller</code> from
> +        ports of this logical switch.
> +      </p>
> +    </column>
> +
>      <group title="Other options">
>        <column name="other_config" key="vlan-passthru"
>            type='{"type": "boolean"}'>
> @@ -1957,6 +2027,14 @@
>        </column>
>      </group>
>  
> +    <column name="copp">
> +      <p>
> +        The control plane protection policy from table <ref table="Copp"/>
> +        used for metering packets sent to <code>ovn-controller</code> from
> +        logical ports of this router.
> +      </p>
> +    </column>
> +
>      <group title="Options">
>        <p>
>          Additional options for the logical router.
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index d1d758c49..75d5d95f4 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -549,3 +549,51 @@ done
>  
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-controller - ovn action metering])
> +AT_KEYWORDS([action-metering])
> +ovn_start
> +
> +net_add n1
> +sim_add hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +check ovn-nbctl lr-add lr1 \
> +    -- lrp-add lr1 rp-ls1 00:00:01:01:02:03 192.168.1.254/24
> +check ovn-nbctl ls-add ls1 \
> +    -- lsp-add ls1 lsp1 \
> +    -- lsp-set-addresses lsp1 "00:00:00:00:00:01 192.168.1.1" \
> +    -- lsp-add ls1 ls1-rp \
> +    -- set Logical_Switch_Port ls1-rp type=router options:router-port=rp-ls1 \
> +    -- lsp-set-addresses ls1-rp router
> +
> +as hv1
> +check ovs-vsctl \
> +    -- add-port br-int vif1 \
> +    -- set Interface vif1 external_ids:iface-id=lsp1
> +
> +check ovn-nbctl --event lb-add lb1 192.168.1.100:80 ""
> +check ovn-nbctl ls-lb-add ls1 lb1
> +
> +# controller-event metering
> +check ovn-nbctl meter-add event-elb drop 100 pktps 10
> +check ovn-nbctl ls-copp-add ls1 event-elb event-elb
> +
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.0f | grep -q meter_id=1])
> +
> +# reject metering
> +check ovn-nbctl meter-add acl-meter drop 1 pktps 0
> +check ovn-nbctl ls-copp-add ls1 reject acl-meter
> +check ovn-nbctl acl-add ls1 from-lport 1002 'inport == "lsp1" && ip && udp' reject
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.16 | grep -q meter_id=2])
> +
> +# arp metering
> +check ovn-nbctl meter-add arp-meter drop 200 pktps 0
> +check ovn-nbctl lr-copp-add lr1 arp-resolve arp-meter
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.00 | grep -q meter_id=3])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index bff2ade43..869f0e8d8 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3008,6 +3008,87 @@ wait_row_count bfd 3
>  AT_CLEANUP
>  ])
>  
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- check CoPP config])
> +AT_KEYWORDS([northd-CoPP])
> +ovn_start
> +
> +check ovn-nbctl --wait=sb lr-add r0
> +check ovn-nbctl --wait=sb lrp-add r0 r0-sw1 00:00:00:00:00:01 192.168.1.1/24
> +check ovn-nbctl --wait=sb ls-add sw1
> +check ovn-nbctl --wait=sb lsp-add sw1 sw1-r0
> +check ovn-nbctl --wait=sb lsp-set-type sw1-r0 router
> +check ovn-nbctl --wait=sb lsp-set-options sw1-r0 router-port=r0-sw1
> +check ovn-nbctl --wait=sb lsp-set-addresses sw1-r0 00:00:00:00:00:01
> +
> +ovn-nbctl --wait=hv meter-add meter0 drop 100 pktps 10
> +ovn-nbctl --wait=hv ls-copp-add sw1 event-elb meter0
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +event-elb: meter0
> +])
> +
> +ovn-nbctl --wait=hv meter-add meter1 drop 200 pktps 10
> +ovn-nbctl --wait=hv ls-copp-add sw1 arp meter1
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +arp: meter1
> +event-elb: meter0
> +])
> +
> +ovn-nbctl --wait=hv ls-copp-del sw1 arp
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +event-elb: meter0
> +])
> +
> +ovn-nbctl --wait=hv ls-copp-del sw1 event-elb
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +])
> +
> +# let's try to add an usupported protocol "dhcp"
> +AT_CHECK([ovn-nbctl --wait=hv ls-copp-add sw1 dhcp meter1],[1],[],[dnl
> +ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve, dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp, nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject.
> +])
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +])
> +
> +#Let's try to add a valid protocol to an unknown datapath
> +AT_CHECK([ovn-nbctl --wait=hv ls-copp-add sw10 arp meter1],[1],[],[dnl
> +ovn-nbctl: sw10: switch name not found
> +])
> +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
> +])
> +
> +ovn-nbctl --wait=hv lr-copp-add r0 bfd meter0
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> +bfd: meter0
> +])
> +
> +ovn-nbctl --wait=hv lr-copp-add r0 igmp meter1
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> +bfd: meter0
> +igmp: meter1
> +])
> +
> +# let's add igmp meter1 twice
> +AT_CHECK([ovn-nbctl --wait=hv lr-copp-add r0 igmp meter1])
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl> +bfd: meter0
> +igmp: meter1
> +])
> +
> +# let's delete a wrong meter
> +AT_CHECK([ovn-nbctl --wait=hv lr-copp-del r0 event-elb])
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> +bfd: meter0
> +igmp: meter1
> +])
> +
> +ovn-nbctl lr-copp-del r0
> +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
> +])
> +
> +AT_CLEANUP
> +])
> +
>  AT_SETUP([ovn -- check LSP attached to multiple LS])
>  ovn_start
>  

I still think these belong in ovn-nbctl.at.

These are similar tests as they test that logical switches and switch
ports are correctly added to the NB DB:
https://github.com/ovn-org/ovn/blob/308e743736eb3a958c0e3892f1cb858f9af00018/tests/ovn-nbctl.at#L109>>

Also, we don't have any tests that check that the generated SB flows
have been added correctly. These belong in ovn-northd.at.


> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index afe3874e6..bd1134aa7 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1445,6 +1445,122 @@
>        </dd>
>      </dl>
>  
> +    <h2> Control Plane Protection Policy commands</h2>
> +    <p>
> +      These commands manage meters configured in <ref table="Copp"/> table
> +      linking them to logical datapaths through <code>copp</code> column in
> +      <ref table="Logical_Switch" /> or <ref table="Logical_Router" /> tables.
> +      Protocol packets for which CoPP is enforced when sending packets to
> +      ovn-controller (if configured):
> +      <ul>
> +          <li>ARP</li>
> +          <li>ND_NS</li>
> +          <li>ND_NA</li>
> +          <li>ND_RA</li>
> +          <li>ND</li>
> +          <li>DNS</li>
> +          <li>IGMP</li>
> +          <li>packets that require ARP resolution before forwarding</li>
> +          <li>packets that require ND_NS before forwarding</li>
> +          <li>packets that need to be replied to with ICMP Errors</li>
> +          <li>packets that need to be replied to with TCP RST</li>
> +          <li>packets that need to be replied to with DHCP_OPTS</li>
> +          <li>BFD</li>
> +      </ul>
> +    </p>
> +
> +    <dl>
> +      <dt><code>ls-copp-add</code> <var>switch</var> <var>proto</var>
> +      <var>meter</var></dt>
> +      <dd>
> +        Adds the control <code>proto</code> to <code>meter</code> mapping
> +        to the <code>switch</code> control plane protection policy. If no
> +        policy exists yet, it creates one. If a mapping already existed for
> +        <code>proto</code>, this will overwrite it.
> +      </dd>
> +
> +      <dt><code>ls-copp-del</code> <var>switch</var> [<var>proto</var>]</dt>
> +      <dd>
> +        Removes the control <code>proto</code> mapping from the
> +        <code>switch</code> control plane protection policy. If
> +        <code>proto</code> is not specified, the whole control plane
> +        protection policy is destroyed.
> +      </dd>
> +
> +      <dt><code>ls-copp-list</code> <var>switch</var></dt>
> +      <dd>
> +        Display the current control plane protection policy for
> +        <code>switch</code>.
> +      </dd>
> +
> +      <dt><code>lsp-copp-add</code> <var>proto</var> <var>proto</var>
> +      <var>meter</var></dt>
> +      <dd>
> +        Adds the control <code>proto</code> to <code>meter</code> mapping
> +        to the <code>port</code> control plane protection policy. If no
> +        policy exists yet, it creates one. If a mapping already existed for
> +        <code>proto</code>, this will overwrite it.
> +      </dd>
> +
> +      <dt><code>lsp-copp-del</code> <var>port</var> [<var>proto</var>]</dt>
> +      <dd>
> +        Removes the control <code>proto</code> mapping from the
> +        <code>port</code> control plane protection policy. If
> +        <code>proto</code> is not specified, the whole control plane
> +        protection policy is destroyed.
> +      </dd>
> +      <dt><code>lsp-copp-list</code> <var>port</var></dt>
> +      <dd>
> +        Display the current control plane protection policy for
> +        <code>port</code>.
> +      </dd>
> +
> +      <dt><code>lr-copp-add</code> <var>router</var> <var>proto</var>
> +      <var>meter</var></dt>
> +      <dd>
> +        Adds the control <code>proto</code> to <code>meter</code> mapping
> +        to the <code>router</code> control plane protection policy. If no
> +        policy exists yet, it creates one. If a mapping already existed for
> +        <code>proto</code>, this will overwrite it.
> +      </dd>
> +
> +      <dt><code>lr-copp-del</code> <var>router</var> [<var>proto</var>]</dt>
> +      <dd>
> +        Removes the control <code>proto</code> mapping from the
> +        <code>router</code> control plane protection policy. If
> +        <code>proto</code> is not specified, the whole control plane
> +        protection policy is destroyed.
> +      </dd>
> +
> +      <dt><code>lr-copp-list</code> <var>router</var></dt>
> +      <dd>
> +        Display the current control plane protection policy for
> +        <code>router</code>.
> +      </dd>
> +
> +      <dt><code>lrp-copp-add</code> <var>proto</var> <var>proto</var>

I don't think these lrp-* options should be here any more?

> +      <var>meter</var></dt>
> +      <dd>
> +        Adds the control <code>proto</code> to <code>meter</code> mapping
> +        to the <code>port</code> control plane protection policy. If no
> +        policy exists yet, it creates one. If a mapping already existed for
> +        <code>proto</code>, this will overwrite it.
> +      </dd>
> +
> +      <dt><code>lrp-copp-del</code> <var>port</var> [<var>proto</var>]</dt>
> +      <dd>
> +        Removes the control <code>proto</code> mapping from the
> +        <code>port</code> control plane protection policy. If
> +        <code>proto</code> is not specified, the whole control plane
> +        protection policy is destroyed.
> +      </dd>
> +      <dt><code>lrp-copp-list</code> <var>port</var></dt>
> +      <dd>
> +        Display the current control plane protection policy for
> +        <code>port</code>.
> +      </dd>
> +    </dl>
> +
>      <h2>Synchronization Commands</h2>
>  
>      <dl>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index b8fc17364..c56707674 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -27,6 +27,7 @@
>  #include "jsonrpc.h"
>  #include "openvswitch/json.h"
>  #include "lib/acl-log.h"
> +#include "lib/copp.h"
>  #include "lib/ovn-nb-idl.h"
>  #include "lib/ovn-util.h"
>  #include "memory.h"
> @@ -425,6 +426,28 @@ chassis with optional PRIORITY to the HA chassis group GRP\n\
>    ha-chassis-group-remove-chassis GRP CHASSIS Removes the HA chassis\
>  CHASSIS from the HA chassis group GRP\n\
>  \n\
> +Control Plane Protection Policy commands:\n\
> +  ls-copp-add SWITCH PROTO METER\n\
> +                            Add a copp policy for PROTO packets on SWITCH\n\
> +                            based on an existing METER.\n\
> +  ls-copp-del SWITCH [PROTO]\n\
> +                            Delete the copp policy for PROTO packets on\n\
> +                            SWITCH. If PROTO is not specified, delete all\n\
> +                            copp policies on SWITCH.\n\
> +  ls-copp-list SWITCH\n\
> +                            List all copp policies defined for control\n\
> +                            protocols on SWITCH.\n\
> +  lr-copp-add ROUTER PROTO METER\n\
> +                            Add a copp policy for PROTO packets on ROUTER\n\
> +                            based on an existing METER.\n\
> +  lr-copp-del ROUTER [PROTO]\n\
> +                            Delete the copp policy for PROTO packets on\n\
> +                            ROUTER. If PROTO is not specified, delete all\n\
> +                            copp policies on ROUTER.\n\
> +  lr-copp-list ROUTER\n\
> +                            List all copp policies defined for control\n\
> +                            protocols on ROUTER.\n\
> +\n\
>  %s\
>  %s\
>  \n\
> @@ -5961,6 +5984,147 @@ nbctl_lr_route_list(struct ctl_context *ctx)
>      free(ipv6_routes);
>  }
>  
> +static void
> +nbctl_pre_copp(struct ctl_context *ctx)
> +{
> +    nbctl_pre_context(ctx);
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_copp_col_meters);
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_col_copp);
> +    ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_copp);
> +}
> +
> +static void
> +nbctl_ls_copp_add(struct ctl_context *ctx)
> +{
> +    const char *ls_name = ctx->argv[1];
> +    const char *proto_name = ctx->argv[2];
> +    const char *meter = ctx->argv[3];
> +
> +    char *error = copp_proto_validate(proto_name);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    const struct nbrec_logical_switch *ls = NULL;
> +    error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    const struct nbrec_copp *copp =
> +        copp_meter_add(ctx, ls->copp, proto_name, meter);
> +    nbrec_logical_switch_set_copp(ls, copp);
> +}
> +
> +static void
> +nbctl_ls_copp_del(struct ctl_context *ctx)
> +{
> +    const char *ls_name = ctx->argv[1];
> +    const char *proto_name = NULL;
> +    char *error;
> +
> +    if (ctx->argc == 3) {
> +        proto_name = ctx->argv[2];
> +        error = copp_proto_validate(proto_name);
> +        if (error) {
> +            ctx->error = error;
> +            return;
> +        }
> +    }
> +
> +    const struct nbrec_logical_switch *ls = NULL;
> +    error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    copp_meter_del(ls->copp, proto_name);
> +}
> +
> +static void
> +nbctl_ls_copp_list(struct ctl_context *ctx)
> +{
> +    const char *ls_name = ctx->argv[1];
> +
> +    const struct nbrec_logical_switch *ls = NULL;
> +    char *error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    copp_meter_list(ctx, ls->copp);
> +}
> +
> +static void
> +nbctl_lr_copp_add(struct ctl_context *ctx)
> +{
> +    const char *lr_name = ctx->argv[1];
> +    const char *proto_name = ctx->argv[2];
> +    const char *meter = ctx->argv[3];
> +
> +    char *error = copp_proto_validate(proto_name);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    const struct nbrec_logical_router *lr = NULL;
> +    error = lr_by_name_or_uuid(ctx, lr_name, true, &lr);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    const struct nbrec_copp *copp =
> +        copp_meter_add(ctx, lr->copp, proto_name, meter);
> +    nbrec_logical_router_set_copp(lr, copp);
> +}
> +
> +static void
> +nbctl_lr_copp_del(struct ctl_context *ctx)
> +{
> +    const char *lr_name = ctx->argv[1];
> +    const char *proto_name = NULL;
> +    char *error;
> +
> +    if (ctx->argc == 3) {
> +        proto_name = ctx->argv[2];
> +        error = copp_proto_validate(proto_name);
> +        if (error) {
> +            ctx->error = error;
> +            return;
> +        }
> +    }
> +
> +    const struct nbrec_logical_router *lr = NULL;
> +    error = lr_by_name_or_uuid(ctx, lr_name, true, &lr);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    copp_meter_del(lr->copp, proto_name);
> +}
> +
> +static void
> +nbctl_lr_copp_list(struct ctl_context *ctx)
> +{
> +    const char *lr_name = ctx->argv[1];
> +
> +    const struct nbrec_logical_router *lr = NULL;
> +    char *error = lr_by_name_or_uuid(ctx, lr_name, true, &lr);
> +    if (error) {
> +        ctx->error = error;
> +        return;
> +    }
> +
> +    copp_meter_list(ctx, lr->copp);
> +}
> +
>  static void
>  verify_connections(struct ctl_context *ctx)
>  {
> @@ -6717,6 +6881,20 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>       nbctl_pre_dhcp_options_options, nbctl_dhcp_options_get_options,
>       NULL, "", RO },
>  
> +    /* Control plane protection commands */
> +    {"ls-copp-add", 3, 3, "SWITCH PROTO METER", nbctl_pre_copp,
> +      nbctl_ls_copp_add, NULL, "", RW},
> +    {"ls-copp-del", 1, 2, "SWITCH [PROTO]", nbctl_pre_copp,
> +      nbctl_ls_copp_del, NULL, "", RW},
> +    {"ls-copp-list", 1, 1, "SWITCH", nbctl_pre_copp, nbctl_ls_copp_list,
> +      NULL, "", RO},
> +    {"lr-copp-add", 3, 3, "ROUTER PROTO METER", nbctl_pre_copp,
> +     nbctl_lr_copp_add, NULL, "", RW},
> +    {"lr-copp-del", 1, 2, "ROUTER [PROTO]", nbctl_pre_copp,
> +     nbctl_lr_copp_del, NULL, "", RW},
> +    {"lr-copp-list", 1, 1, "ROUTER", nbctl_pre_copp, nbctl_lr_copp_list,
> +     NULL, "", RO},
> +
>      /* 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