> On 29/04/2021 18:04, Lorenzo Bianconi wrote:
> > From: Dumitru Ceara <[email protected]>
> >
[...]
>
> Could this be changed to copp_proto_get_name()?
ack, I will fix it in v2
>
> > +{
> > + 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_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_add_meter(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_del_meter(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(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(i));
> > + }
> > + ds_chomp(&usage, ' ');
> > + ds_chomp(&usage, ',');
> > + ds_put_cstr(&usage, ".");
> > +
> > + char *usage_str = xstrdup(ds_cstr(&usage));
> > + ds_destroy(&usage);
> > + return usage_str;
> > +}
> > diff --git a/lib/copp.h b/lib/copp.h
> > new file mode 100644
> > index 000000000..82581e7e4
> > --- /dev/null
> > +++ b/lib/copp.h
> > @@ -0,0 +1,60 @@
> > +/* 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_proto_get(enum copp_proto);
>
> I think this could a static and declared in copp.c as it is not used
> outside of there.
ack, I will fix it in v2
>
> > +
> > +const char *copp_meter_get(enum copp_proto proto,
> > + const struct nbrec_copp *copp,
> > + const struct shash *meter_groups);
> > +
> > +void copp_list(struct ctl_context *ctx, const struct nbrec_copp *copp);
> > +const struct nbrec_copp *
> > +copp_add_meter(struct ctl_context *ctx, const struct nbrec_copp *copp,
> > + const char *proto_name, const char *meter);
> > +void
> > +copp_del_meter(const struct nbrec_copp *copp, const char *proto_name);
> > +char * copp_proto_validate(const char *proto_name);
>
> Could you change the names of these function prototypes to be more
> consistent? e.g. copp_meter_add/del/get/list()
ack, I will fix it in v2
>
> > +
> > +#endif /* lib/copp.h */
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 94fae5648..2e9c7de22 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;
> > }
> >
> > @@ -4107,6 +4111,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));
> > @@ -4120,6 +4125,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);
> > @@ -4134,14 +4140,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
> > @@ -4156,21 +4172,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);
> > }
> > @@ -4186,6 +4203,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);
> > }
> > }
> > @@ -12333,7 +12351,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. */
> > @@ -12378,6 +12397,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
> > @@ -14082,6 +14102,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 29019809c..fb0f3a7c0 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> > {
> > "name": "OVN_Northbound",
> > - "version": "5.31.0",
> > - "cksum": "2352750632 28701",
> > + "version": "5.32.0",
> > + "cksum": "2317195278 29378",
> > "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"}},
> > @@ -319,6 +330,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 feb38b5d3..0f03a12f2 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -339,6 +339,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.
> > @@ -572,6 +634,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"}'>
> > @@ -1929,6 +1999,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-northd.at b/tests/ovn-northd.at
> > index 32afb4fa8..7c18e6302 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
>
> Should we be adding ovn-nbctl.at tests? Or do these tests belong in
> ovn-nbctl.at These tests don't test northd but rather test that things
> are written correctly to NBDB.
I guess here we are testing values are properly written to NB db, so we are
testing both ovn-nbctl and the NB db interation, agree? Am I missing somthing?
>
> We probably should have tests (in ovn-northd.c) that configures NBDB and
> then we check that the SBDB has meter flows present? (and various
> negative tests as well)
>
> > @@ -2649,6 +2649,55 @@ 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
> > +])
> > +
> > +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
> > +])
>
> What about a delete all case at the end
>
> ovn-nbctl lr-copp-del r0
>
> Also,
>
> Can you test a few failure cases? For example, deleting a copp that is
> not there, adding a the same copp twice, etc
ack, I will do in v2
>
> > +
> > +AT_CLEANUP
> > +])
> > +
> > AT_SETUP([ovn -- check LSP attached to multiple LS])
> > ovn_start
> >
> > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> > index 03d47dba5..6c95e8104 100644
> > --- a/utilities/ovn-nbctl.8.xml
> > +++ b/utilities/ovn-nbctl.8.xml
> > @@ -1131,6 +1131,122 @@
> > </dd>
> > </dl>
> >
> > + <h1> Control Plane Protection Policy commands</h1>
> > + <p>
> > + These commands manages meters contained 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>
>
> Should event-elb be listed here?
I think it is done in the subsequent patch
>
> > + </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>
> > + <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>
>
> I don't think the lrp-* and lsp-* variants are needed any more?
ack, right. I will fix it in v2
Regards,
Lorenzo
>
> > +
> > + <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>
> > +
> > <h1>Database Commands</h1>
> > <p>These commands query and modify the contents of <code>ovsdb</code>
> > tables.
> > They are a slight abstraction of the <code>ovsdb</code> interface and
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 042c21002..2299bc6ce 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"
> > @@ -835,6 +836,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\
> > @@ -5711,6 +5734,138 @@ nbctl_lr_route_list(struct ctl_context *ctx)
> > free(ipv6_routes);
> > }
> >
> > +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_add_meter(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_del_meter(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_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_add_meter(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_del_meter(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_list(ctx, lr->copp);
> > +}
> > +
> > static void
> > verify_connections(struct ctl_context *ctx)
> > {
> > @@ -6673,6 +6828,18 @@ static const struct ctl_command_syntax
> > nbctl_commands[] = {
> > {"dhcp-options-get-options", 1, 1, "DHCP_OPT_UUID", NULL,
> > nbctl_dhcp_options_get_options, NULL, "", RO },
> >
> > + /* Control plane protection commands */
> > + {"ls-copp-add", 3, 3, "SWITCH PROTO METER", NULL, nbctl_ls_copp_add,
> > NULL,
> > + "", RW},
> > + {"ls-copp-del", 1, 2, "SWITCH [PROTO]", NULL, nbctl_ls_copp_del, NULL,
> > + "", RW},
> > + {"ls-copp-list", 1, 1, "SWITCH", NULL, nbctl_ls_copp_list, NULL, "",
> > RO},
> > + {"lr-copp-add", 3, 3, "ROUTER PROTO METER", NULL, nbctl_lr_copp_add,
> > NULL,
> > + "", RW},
> > + {"lr-copp-del", 1, 2, "ROUTER [PROTO]", NULL, nbctl_lr_copp_del, NULL,
> > + "", RW},
> > + {"lr-copp-list", 1, 1, "ROUTER", NULL, 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},
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
