Introduce the capability to specify CoPP UUID or CoPP name in order to reuse the same CoPP reference on multiple datapaths. Introduce logical_router and logical_switches columns in CoPP table in order to specify datapaths where CoPP is installed. Add external_ids column in CoPP table.
Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852 Signed-off-by: Lorenzo Bianconi <[email protected]> --- Changes since v1: - Use name as table index - make name mandatory - add external_ids column --- ovn-nb.ovsschema | 21 +++++++-- ovn-nb.xml | 12 +++++ tests/ovn-controller.at | 6 +-- tests/ovn-northd.at | 58 +++++++++++++++++++---- tests/ovn.at | 2 +- tests/system-ovn.at | 8 ++-- utilities/ovn-nbctl.8.xml | 12 +++-- utilities/ovn-nbctl.c | 99 +++++++++++++++++++++++++++++++++------ 8 files changed, 179 insertions(+), 39 deletions(-) diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index 55977339a..192036ad5 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.34.1", - "cksum": "2177334725 30782", + "version": "5.35.0", + "cksum": "3892116111 31629", "tables": { "NB_Global": { "columns": { @@ -32,11 +32,26 @@ "isRoot": true}, "Copp": { "columns": { + "name": {"type": "string"}, + "logical_switch": {"type": {"key": {"type": "uuid", + "refTable": "Logical_Switch", + "refType": "strong"}, + "min": 0, + "max": "unlimited"}}, + "logical_router": {"type": {"key": {"type": "uuid", + "refTable": "Logical_Router", + "refType": "strong"}, + "min": 0, + "max": "unlimited"}}, "meters": { "type": {"key": "string", "value": "string", "min": 0, - "max": "unlimited"}}}, + "max": "unlimited"}}, + "external_ids": { + "type": {"key": "string", "value": "string", + "min": 0, "max": "unlimited"}}}, + "indexes": [["name"]], "isRoot": true}, "Logical_Switch": { "columns": { diff --git a/ovn-nb.xml b/ovn-nb.xml index 6a6972856..e327c92ee 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -360,6 +360,15 @@ associate entries from table <ref table="Meter"/> to control protocol names. </p> + <column name="name"> + CoPP name. + </column> + <column name="logical_switch"> + Reference to <ref table="Logical_Switch"/> where the CoPP is installed. + </column> + <column name="logical_router"> + Reference to <ref table="Logical_Router"/> where the CoPP is installed. + </column> <column name="meters" key="arp"> Rate limiting meter for ARP packets (request/reply) used for learning neighbors. @@ -417,6 +426,9 @@ <column name="meters" key="reject"> Rate limiting meter for packets that trigger a reject action </column> + <column name="external_ids"> + See <em>External IDs</em> at the beginning of this document. + </column> </table> <table name="Logical_Switch" title="L2 logical switch"> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 2f39e5f3e..6a5880501 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -713,19 +713,19 @@ 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 --wait=hv ls-copp-add ls1 event-elb event-elb +check ovn-nbctl --wait=hv ls-copp-add copp0 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 ls-copp-add copp1 ls1 reject acl-meter check ovn-nbctl --wait=hv 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 --wait=hv lr-copp-add lr1 arp-resolve arp-meter +check ovn-nbctl --wait=hv lr-copp-add copp2 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]) diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 652903761..5c7587c3e 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -3287,7 +3287,7 @@ check ovn-nbctl --wait=sb lsp-set-addresses sw1-r0 00:00:00:00:00:01 check ovn-nbctl --event lb-add lb0 192.168.1.100:80 "" check ovn-nbctl ls-lb-add sw1 lb0 check ovn-nbctl --wait=hv meter-add meter0 drop 100 pktps 10 -check ovn-nbctl --wait=hv ls-copp-add sw1 event-elb meter0 +check ovn-nbctl --wait=hv ls-copp-add copp0 sw1 event-elb meter0 AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl event-elb: meter0 ]) @@ -3304,7 +3304,7 @@ AT_CHECK([ovn-nbctl meter-list |grep meter1 -A 1], [0], [dnl meter1: bands: drop: 200 pktps, 10 packet burst ]) -check ovn-nbctl --wait=hv lr-copp-add r0 arp meter1 +check ovn-nbctl --wait=hv lr-copp-add copp1 r0 arp meter1 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl arp: meter1 ]) @@ -3318,7 +3318,7 @@ AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl AT_CHECK([ovn-sbctl list logical_flow | grep arp -A 2 | grep -q meter1],[1]) check ovn-nbctl --wait=hv meter-add meter2 drop 400 pktps 10 -check ovn-nbctl --wait=hv lr-copp-add r0 icmp4-error meter2 +check ovn-nbctl --wait=hv lr-copp-add copp2 r0 icmp4-error meter2 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl icmp4-error: meter2 ]) @@ -3329,7 +3329,7 @@ check ovn-nbctl --wait=hv lr-copp-del r0 icmp4-error AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl ]) -check ovn-nbctl --wait=hv lr-copp-add r0 icmp6-error meter2 +check ovn-nbctl --wait=hv lr-copp-add copp3 r0 icmp6-error meter2 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl icmp6-error: meter2 ]) @@ -3340,7 +3340,7 @@ check ovn-nbctl --wait=hv lr-copp-del r0 icmp6-error AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl ]) -check ovn-nbctl --wait=hv lr-copp-add r0 tcp-reset meter2 +check ovn-nbctl --wait=hv lr-copp-add copp4 r0 tcp-reset meter2 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl tcp-reset: meter2 ]) @@ -3358,21 +3358,21 @@ AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl AT_CHECK([ovn-sbctl list logical_flow | grep trigger_event -A 2 | grep -q meter0],[1]) # let's try to add an usupported protocol "dhcp" -AT_CHECK([ovn-nbctl --wait=hv ls-copp-add sw1 dhcp meter1],[1],[],[dnl +AT_CHECK([ovn-nbctl --wait=hv ls-copp-add copp5 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 +AT_CHECK([ovn-nbctl --wait=hv ls-copp-add copp6 sw10 arp meter1],[1],[],[dnl ovn-nbctl: sw10: switch name not found ]) AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl ]) check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.50.2 r0-sw1 -check ovn-nbctl --wait=hv lr-copp-add r0 bfd meter0 +check ovn-nbctl --wait=hv lr-copp-add copp7 r0 bfd meter0 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl bfd: meter0 ]) @@ -3381,14 +3381,14 @@ AT_CHECK([ovn-sbctl list logical_flow | grep bfd -A 2 | grep -q meter0]) check ovn-nbctl --wait=hv set Logical_Switch sw1 \ other_config:mcast_querier="false" \ other_config:mcast_snoop="true" -check ovn-nbctl --wait=hv ls-copp-add sw1 igmp meter1 +check ovn-nbctl --wait=hv ls-copp-add copp8 sw1 igmp meter1 AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl igmp: meter1 ]) AT_CHECK([ovn-sbctl list logical_flow | grep igmp -A 2 | grep -q meter1]) # let's add igmp meter1 twice -AT_CHECK([ovn-nbctl --wait=hv ls-copp-add sw1 igmp meter1]) +AT_CHECK([ovn-nbctl --wait=hv ls-copp-add copp9 sw1 igmp meter1]) AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl igmp: meter1 ]) @@ -3403,6 +3403,44 @@ check ovn-nbctl lr-copp-del r0 AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl ]) +check ovn-nbctl ls-copp-del sw1 +AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl +]) + +check ovn-nbctl --wait=hv lr-copp-add copp0 r0 arp meter0 +AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl +arp: meter0 +]) + +AT_CHECK([fetch_column nb:CoPP name], [0], [dnl +copp0 +]) + +lr_uuid=$(fetch_column nb:Logical_Router _uuid) +copp_lr_uuid=$(fetch_column nb:CoPP logical_router) +AT_CHECK([test "$lr_uuid" = "$copp_lr_uuid"]) + +copp_uuid=$(fetch_column nb:CoPP _uuid) +check ovn-nbctl --wait=hv ls-copp-add $copp_uuid sw1 arp meter0 + +ls_uuid=$(fetch_column nb:Logical_Switch _uuid) +copp_ls_uuid=$(fetch_column nb:CoPP logical_switch) +AT_CHECK([test "$ls_uuid" = "$copp_ls_uuid"]) + +ls_copp_uuid=$(fetch_column nb:Logical_Switch copp) +AT_CHECK([test "$ls_copp_uuid" = "$copp_uuid"]) + +check ovn-nbctl --wait=sb lrp-add r0 r0-sw2 00:00:00:00:00:03 192.168.2.1/24 +check ovn-nbctl --wait=sb ls-add sw2 +check ovn-nbctl --wait=sb lsp-add sw2 sw2-r0 +check ovn-nbctl --wait=sb lsp-set-type sw2-r0 router +check ovn-nbctl --wait=sb lsp-set-options sw2-r0 router-port=r0-sw2 +check ovn-nbctl --wait=sb lsp-set-addresses sw2-r0 00:00:00:00:00:02 + +check ovn-nbctl --wait=hv ls-copp-add copp0 sw2 event-elb meter0 +ls2_copp_uuid=$(ovn-nbctl get Logical_Switch sw2 copp) +AT_CHECK([test "$ls2_copp_uuid" = "$copp_uuid"]) + AT_CLEANUP ]) diff --git a/tests/ovn.at b/tests/ovn.at index 92e284e8a..8de540e57 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -18932,7 +18932,7 @@ ovn-nbctl ls-lb-add sw0 lb2 uuid_lb2=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb2) ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10 -ovn-nbctl --wait=hv ls-copp-add sw0 event-elb event-elb +ovn-nbctl --wait=hv ls-copp-add copp0 sw0 event-elb event-elb OVN_POPULATE_ARP wait_for_ports_up diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 3ae812296..b384a2204 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -6642,7 +6642,7 @@ check ovn-nbctl lsp-add public public1 \ NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &]) check ovn-nbctl meter-add acl-meter drop 1 pktps 0 -check ovn-nbctl --wait=hv ls-copp-add sw0 reject acl-meter +check ovn-nbctl --wait=hv ls-copp-add copp0 sw0 reject acl-meter check ovn-nbctl acl-add sw0 from-lport 1002 'inport == "sw01" && ip && udp' reject AT_CHECK([ovn-nbctl ls-copp-list sw0], [0], [dnl @@ -6678,7 +6678,7 @@ kill $(pidof tcpdump) NS_EXEC([server], [tcpdump -l -n -i s1 arp[[24:4]]=0xac100164 > arp.pcap &]) check ovn-nbctl meter-add arp-meter drop 1 pktps 0 -check ovn-nbctl --wait=hv lr-copp-add R1 arp-resolve arp-meter +check ovn-nbctl --wait=hv lr-copp-add copp1 R1 arp-resolve arp-meter AT_CHECK([ovn-nbctl lr-copp-list R1], [0], [dnl arp-resolve: arp-meter ]) @@ -6696,7 +6696,7 @@ OVS_WAIT_UNTIL([ kill $(pidof tcpdump) check ovn-nbctl meter-add icmp-meter drop 1 pktps 0 -check ovn-nbctl --wait=hv lr-copp-add R1 icmp4-error icmp-meter +check ovn-nbctl --wait=hv lr-copp-add copp2 R1 icmp4-error icmp-meter AT_CHECK([ovn-nbctl lr-copp-list R1 |grep icmp4-error], [0], [dnl icmp4-error: icmp-meter ]) @@ -6715,7 +6715,7 @@ OVS_WAIT_UNTIL([ kill $(pidof tcpdump) check ovn-nbctl meter-add bfd-meter drop 1 pktps 0 -check ovn-nbctl --wait=hv lr-copp-add R1 bfd bfd-meter +check ovn-nbctl --wait=hv lr-copp-add copp3 R1 bfd bfd-meter AT_CHECK([ovn-nbctl lr-copp-list R1 |grep bfd], [0], [dnl bfd: bfd-meter ]) diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 80a564660..b944d7e08 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -1474,13 +1474,15 @@ </p> <dl> - <dt><code>ls-copp-add</code> <var>switch</var> <var>proto</var> - <var>meter</var></dt> + <dt><code>ls-copp-add</code> [<var>UUID</var>|<var>name</var>] + <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. + If <var>UUID</var> <var>name</var> is provided, the already installed + CoPP will be reused. </dd> <dt><code>ls-copp-del</code> <var>switch</var> [<var>proto</var>]</dt> @@ -1497,13 +1499,15 @@ <code>switch</code>. </dd> - <dt><code>lr-copp-add</code> <var>router</var> <var>proto</var> - <var>meter</var></dt> + <dt><code>lr-copp-add</code> [<var>UUID</var>|<var>name</var>] + <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. + If <var>UUID</var> <var>name</var> is provided, the already installed + CoPP will be reused. </dd> <dt><code>lr-copp-del</code> <var>router</var> [<var>proto</var>]</dt> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index d67d2db65..3a89da030 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -437,7 +437,7 @@ chassis with mandatory PRIORITY to the HA chassis group GRP\n\ CHASSIS from the HA chassis group GRP\n\ \n\ Control Plane Protection Policy commands:\n\ - ls-copp-add SWITCH PROTO METER\n\ + ls-copp-add [UUID|NAME] 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\ @@ -447,7 +447,7 @@ Control Plane Protection Policy commands:\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\ + lr-copp-add [UUID|NAME] 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\ @@ -6278,6 +6278,9 @@ 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_copp_col_logical_switch); + ovsdb_idl_add_column(ctx->idl, &nbrec_copp_col_logical_router); + ovsdb_idl_add_column(ctx->idl, &nbrec_copp_col_name); ovsdb_idl_add_column(ctx->idl, &nbrec_logical_switch_col_copp); ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_copp); } @@ -6285,9 +6288,29 @@ nbctl_pre_copp(struct ctl_context *ctx) 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]; + const struct nbrec_copp *copp = NULL; + const char *proto_name = ctx->argv[3]; + const char *ls_name = ctx->argv[2]; + const char *meter = ctx->argv[4]; + const char *copp_name = NULL; + struct uuid uuid; + + if (uuid_from_string(&uuid, ctx->argv[1])) { + copp = nbrec_copp_get_for_uuid(ctx->idl, &uuid); + if (!copp) { + ctx->error = xasprintf("copp %s not found.", ctx->argv[1]); + return; + } + } else { + copp_name = ctx->argv[1]; + const struct nbrec_copp *iter; + NBREC_COPP_FOR_EACH (iter, ctx->idl) { + if (!strcmp(iter->name, copp_name)) { + copp = iter; + break; + } + } + } char *error = copp_proto_validate(proto_name); if (error) { @@ -6302,9 +6325,23 @@ nbctl_ls_copp_add(struct ctl_context *ctx) return; } - const struct nbrec_copp *copp = - copp_meter_add(ctx, ls->copp, proto_name, meter); + if (!copp) { + copp = copp_meter_add(ctx, ls->copp, proto_name, meter); + if (copp_name) { + nbrec_copp_set_name(copp, copp_name); + } + } nbrec_logical_switch_set_copp(ls, copp); + + size_t n_logical_switch = copp->n_logical_switch + 1; + struct nbrec_logical_switch **ls_list = + xmalloc(n_logical_switch * sizeof *ls_list); + for (int i = 0; i < copp->n_logical_switch; i++) { + ls_list[i] = copp->logical_switch[i]; + } + ls_list[copp->n_logical_switch] = (struct nbrec_logical_switch *)ls; + nbrec_copp_set_logical_switch(copp, ls_list, n_logical_switch); + free(ls_list); } static void @@ -6351,9 +6388,29 @@ nbctl_ls_copp_list(struct ctl_context *ctx) 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]; + const struct nbrec_copp *copp = NULL; + const char *proto_name = ctx->argv[3]; + const char *lr_name = ctx->argv[2]; + const char *meter = ctx->argv[4]; + const char *copp_name = NULL; + struct uuid uuid; + + if (uuid_from_string(&uuid, ctx->argv[1])) { + copp = nbrec_copp_get_for_uuid(ctx->idl, &uuid); + if (!copp) { + ctx->error = xasprintf("copp %s not found.", ctx->argv[1]); + return; + } + } else { + copp_name = ctx->argv[1]; + const struct nbrec_copp *iter; + NBREC_COPP_FOR_EACH (iter, ctx->idl) { + if (!strcmp(iter->name, copp_name)) { + copp = iter; + break; + } + } + } char *error = copp_proto_validate(proto_name); if (error) { @@ -6368,9 +6425,23 @@ nbctl_lr_copp_add(struct ctl_context *ctx) return; } - const struct nbrec_copp *copp = - copp_meter_add(ctx, lr->copp, proto_name, meter); + if (!copp) { + copp = copp_meter_add(ctx, lr->copp, proto_name, meter); + if (copp_name) { + nbrec_copp_set_name(copp, copp_name); + } + } nbrec_logical_router_set_copp(lr, copp); + + size_t n_logical_router = copp->n_logical_router + 1; + struct nbrec_logical_router **lr_list = + xmalloc(n_logical_router * sizeof *lr_list); + for (int i = 0; i < copp->n_logical_router; i++) { + lr_list[i] = copp->logical_router[i]; + } + lr_list[copp->n_logical_router] = (struct nbrec_logical_router *)lr; + nbrec_copp_set_logical_router(copp, lr_list, n_logical_router); + free(lr_list); } static void @@ -7177,13 +7248,13 @@ static const struct ctl_command_syntax nbctl_commands[] = { NULL, "", RO }, /* Control plane protection commands */ - {"ls-copp-add", 3, 3, "SWITCH PROTO METER", nbctl_pre_copp, + {"ls-copp-add", 4, 4, "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, + {"lr-copp-add", 4, 4, "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}, -- 2.34.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
