On 24/05/2021 19:44, Lorenzo Bianconi wrote: > From: Dumitru Ceara <[email protected]> > > 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 <[email protected]> > Signed-off-by: Lorenzo Bianconi <[email protected]> > Signed-off-by: Dumitru Ceara <[email protected]> > --- > 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}, > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
