diff mbox series

[ovs-dev,v3] ovn-nbctl: add the capability to specify CoPP name

Message ID b6e9d798b9b897dc933b2a773ff7502ad1a86a08.1643147639.git.lorenzo.bianconi@redhat.com
State Changes Requested
Headers show
Series [ovs-dev,v3] ovn-nbctl: add the capability to specify CoPP name | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Lorenzo Bianconi Jan. 25, 2022, 10:22 p.m. UTC 
Introduce the capability to specify CoPP name in order to
reuse the same CoPP reference on multiple datapaths.
Introduce the following CoPP commands:
- ovn-nbctl copp-add <name> <proto> <meter>
- ovn-nbctl copp-del <name> <proto>
- ovn-nbctl copp-list <name>
- ovn-nbctl ls-copp-add <name> <switch>
- ovn-nbctl lr-copp-add <name> <router>

Introduce name and external_ids columns in CoPP table.

Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
Changes since v2:
- introduce new CoPP commands
- get rid of logical_switch and logical_router column in CoPP table.

Changes since v1:
- Use name as table index
- make name mandatory
- add external_ids column
---
 lib/copp.c                |  35 ++++++++++
 lib/copp.h                |   3 +
 ovn-nb.ovsschema          |  11 ++-
 ovn-nb.xml                |   6 ++
 tests/ovn-controller.at   |  19 ++++--
 tests/ovn-northd.at       | 117 +++++++++++++++++++++-----------
 tests/ovn.at              |   3 +-
 tests/system-ovn.at       |  22 +++---
 utilities/ovn-nbctl.8.xml |  37 ++++------
 utilities/ovn-nbctl.c     | 138 ++++++++++++++------------------------
 10 files changed, 222 insertions(+), 169 deletions(-)

Comments

Dumitru Ceara Jan. 28, 2022, 4:50 p.m. UTC | #1 
On 1/25/22 23:22, Lorenzo Bianconi wrote:
> Introduce the capability to specify CoPP name in order to
> reuse the same CoPP reference on multiple datapaths.
> Introduce the following CoPP commands:
> - ovn-nbctl copp-add <name> <proto> <meter>
> - ovn-nbctl copp-del <name> <proto>
> - ovn-nbctl copp-list <name>
> - ovn-nbctl ls-copp-add <name> <switch>
> - ovn-nbctl lr-copp-add <name> <router>
> 
> Introduce name and external_ids columns in CoPP table.
> 
> Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---

Hi Lorenzo,

Overall this change looks OK to me.  There is however one issue, when
updating copp entries by UUID, see below.

Also, I think we should add NEWS entries mentioning the new commands and
also letting users know that the "name" is now mandatory and used as
unique index.

[...]

> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index d67d2db65..e0d5191f1 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c

[...]

> @@ -6295,23 +6290,16 @@ nbctl_ls_copp_add(struct ctl_context *ctx)
>          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);
> +    error = copp_by_name_or_uuid(ctx, ctx->argv[1], false, &copp);
> +    copp = copp_meter_add(ctx, copp, proto_name, meter);
> +    nbrec_copp_set_name(copp, ctx->argv[1]);

If ctx->argv[1] was an UUID we shouldn't reset the name, e.g., this is
wrong:

$ ovn-nbctl copp-add copp1 arp meter-arp
$ ovn-nbctl list copp | grep -e uuid -e name
_uuid               : 04ebc387-ee3e-4d93-b6a3-2037c1da74d9
name                : copp1
$ ovn-nbctl copp-add 04ebc387-ee3e-4d93-b6a3-2037c1da74d9 igmp meter-igmp
$ ovn-nbctl list copp | grep -e uuid -e name
_uuid               : 04ebc387-ee3e-4d93-b6a3-2037c1da74d9
name                : "04ebc387-ee3e-4d93-b6a3-2037c1da74d9"

Regards,
Dumitru
diff mbox series

Patch

diff --git a/lib/copp.c b/lib/copp.c
index bbe66924b..603e3f5bf 100644
--- a/lib/copp.c
+++ b/lib/copp.c
@@ -115,6 +115,9 @@  copp_meter_del(const struct nbrec_copp *copp, const char *proto_name)
             nbrec_copp_set_meters(copp, &meters);
             smap_destroy(&meters);
         }
+        if (smap_is_empty(&copp->meters)) {
+            nbrec_copp_delete(copp);
+        }
     } else {
         nbrec_copp_delete(copp);
     }
@@ -141,3 +144,35 @@  copp_proto_validate(const char *proto_name)
 
     return ds_steal_cstr(&usage);
 }
+
+char * OVS_WARN_UNUSED_RESULT
+copp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+                     const struct nbrec_copp **copp_p)
+{
+    const struct nbrec_copp *copp = NULL;
+    struct uuid uuid;
+    bool is_uuid = uuid_from_string(&uuid, id);
+
+    *copp_p = NULL;
+    if (is_uuid) {
+        copp = nbrec_copp_get_for_uuid(ctx->idl, &uuid);
+    }
+
+    if (!copp) {
+        const struct nbrec_copp *iter;
+        NBREC_COPP_FOR_EACH (iter, ctx->idl) {
+            if (!strcmp(iter->name, id)) {
+                copp = iter;
+                break;
+            }
+        }
+    }
+
+    if (!copp && must_exist) {
+        return xasprintf("%s: copp %s not found",
+                         id, is_uuid ? "UUID" : "name");
+    }
+
+    *copp_p = copp;
+    return NULL;
+}
diff --git a/lib/copp.h b/lib/copp.h
index e238d963a..f03004aa6 100644
--- a/lib/copp.h
+++ b/lib/copp.h
@@ -55,5 +55,8 @@  copp_meter_add(struct ctl_context *ctx, const struct nbrec_copp *copp,
 void
 copp_meter_del(const struct nbrec_copp *copp, const char *proto_name);
 char * copp_proto_validate(const char *proto_name);
+char * OVS_WARN_UNUSED_RESULT
+copp_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+                     const struct nbrec_copp **copp_p);
 
 #endif /* lib/copp.h */
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 55977339a..eb17b4f4f 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Northbound",
-    "version": "5.34.1",
-    "cksum": "2177334725 30782",
+    "version": "6.0.0",
+    "cksum": "1994796624 31020",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -32,11 +32,16 @@ 
             "isRoot": true},
         "Copp": {
             "columns": {
+                "name": {"type": "string"},
                 "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..e8aa8b863 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -360,6 +360,9 @@ 
       associate entries from table <ref table="Meter"/> to control protocol
       names.
     </p>
+    <column name="name">
+      CoPP name.
+    </column>
     <column name="meters" key="arp">
       Rate limiting meter for ARP packets (request/reply) used for learning
       neighbors.
@@ -417,6 +420,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..e99eec1d6 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -713,20 +713,29 @@  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 copp-add copp0 event-elb event-elb
+check ovn-nbctl --wait=hv ls-copp-add copp0 ls1
 
 AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.0f | grep -q meter_id=1])
 
+check ovn-nbctl copp-del copp0
+AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
+])
+check ovn-nbctl meter-del event-elb
+
 # 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 --wait=hv copp-add copp1 reject acl-meter
+check ovn-nbctl ls-copp-add copp1 ls1
 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])
+
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.16 | grep -q meter_id=1])
 
 # 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
-AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.00 | grep -q meter_id=3])
+check ovn-nbctl --wait=hv copp-add copp2 arp-resolve arp-meter
+check ovn-nbctl --wait=hv lr-copp-add copp2 lr1
+AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep controller | grep userdata=00.00.00.00 | grep -q meter_id=2])
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 652903761..c26b2db9f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3287,8 +3287,9 @@  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
-AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp0 event-elb meter0
+check ovn-nbctl --wait=hv ls-copp-add copp0 sw1
+AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
 event-elb: meter0
 ])
 
@@ -3304,76 +3305,77 @@  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
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp1 arp meter1
+check ovn-nbctl --wait=hv lr-copp-add copp1 r0
+AT_CHECK([ovn-nbctl copp-list copp1], [0], [dnl
 arp: meter1
 ])
 
 AT_CHECK([ovn-sbctl list logical_flow | grep arp -A 2 | grep -q meter1])
 
-check ovn-nbctl --wait=hv lr-copp-del r0 arp
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-del copp1 arp
+AT_CHECK([ovn-nbctl copp-list copp1], [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
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp2 icmp4-error meter2
+check ovn-nbctl --wait=hv lr-copp-add copp2 r0
+AT_CHECK([ovn-nbctl copp-list copp2], [0], [dnl
 icmp4-error: meter2
 ])
 
 AT_CHECK([ovn-sbctl list logical_flow | grep icmp4 -A 2 | grep -q meter2])
 
-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 copp-del copp2 icmp4-error
+AT_CHECK([ovn-nbctl copp-list copp2], [0], [dnl
 ])
 
-check ovn-nbctl --wait=hv lr-copp-add r0 icmp6-error meter2
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp3 icmp6-error meter2
+check ovn-nbctl --wait=hv lr-copp-add copp3 r0
+AT_CHECK([ovn-nbctl copp-list copp3], [0], [dnl
 icmp6-error: meter2
 ])
 
 AT_CHECK([ovn-sbctl list logical_flow | grep icmp6 -A 2 | grep -q meter2])
 
-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 copp-del copp3 icmp6-error
+AT_CHECK([ovn-nbctl copp-list copp3], [0], [dnl
 ])
 
-check ovn-nbctl --wait=hv lr-copp-add r0 tcp-reset meter2
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp4 tcp-reset meter2
+check ovn-nbctl --wait=hv lr-copp-add copp4 r0
+AT_CHECK([ovn-nbctl copp-list copp4], [0], [dnl
 tcp-reset: meter2
 ])
 
 AT_CHECK([ovn-sbctl list logical_flow | grep tcp -A 2 | grep -q meter2])
 
-check ovn-nbctl --wait=hv lr-copp-del r0 tcp-reset
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-del copp4 tcp-reset
+AT_CHECK([ovn-nbctl copp-list copp4], [0], [dnl
 ])
 
-check ovn-nbctl --wait=hv ls-copp-del sw1 event-elb
-AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
+check ovn-nbctl --wait=hv copp-del copp0 event-elb
+AT_CHECK([ovn-nbctl copp-list copp0], [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 copp-add copp5 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
+check ovn-nbctl --wait=hv copp-add copp6 arp meter1
+AT_CHECK([ovn-nbctl --wait=hv ls-copp-add copp6 sw10],[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
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp7 bfd meter0
+check ovn-nbctl --wait=hv lr-copp-add copp7 r0
+AT_CHECK([ovn-nbctl copp-list copp7], [0], [dnl
 bfd: meter0
 ])
 AT_CHECK([ovn-sbctl list logical_flow | grep bfd -A 2 | grep -q meter0])
@@ -3381,28 +3383,67 @@  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
-AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp8 igmp meter1
+check ovn-nbctl --wait=hv ls-copp-add copp8 sw1
+AT_CHECK([ovn-nbctl copp-list copp8], [0], [dnl
 igmp: meter1
 ])
 AT_CHECK([ovn-sbctl list logical_flow | grep igmp -A 2 | grep -q meter1])
 
+check ovn-nbctl copp-del copp8
+AT_CHECK([ovn-nbctl copp-list copp8], [0], [dnl
+])
+
 # let's add igmp meter1 twice
-AT_CHECK([ovn-nbctl --wait=hv ls-copp-add sw1 igmp meter1])
-AT_CHECK([ovn-nbctl ls-copp-list sw1], [0], [dnl
+AT_CHECK([ovn-nbctl --wait=hv copp-add copp9 igmp meter1])
+AT_CHECK([ovn-nbctl copp-list copp9], [0], [dnl
 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
+AT_CHECK([ovn-nbctl --wait=hv copp-del copp9 event-elb])
+AT_CHECK([ovn-nbctl copp-list copp9], [0], [dnl
+igmp: meter1
+])
+
+check ovn-nbctl copp-del copp9
+AT_CHECK([ovn-nbctl copp-list copp9], [0], [dnl
 ])
 
-check ovn-nbctl lr-copp-del r0
-AT_CHECK([ovn-nbctl lr-copp-list r0], [0], [dnl
+check ovn-nbctl copp-del copp6
+check ovn-nbctl copp-del copp7
+AT_CHECK([ovn-nbctl list copp], [0], [dnl
 ])
 
+check ovn-nbctl --wait=hv copp-add copp0 arp meter0
+check ovn-nbctl --wait=hv lr-copp-add copp0 r0
+AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
+arp: meter0
+])
+
+AT_CHECK([fetch_column nb:CoPP name], [0], [dnl
+copp0
+])
+
+copp_uuid=$(fetch_column nb:CoPP _uuid)
+check ovn-nbctl --wait=hv copp-add copp0 arp meter0
+check ovn-nbctl --wait=hv ls-copp-add copp0 sw1
+
+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 copp-add copp0 event-elb meter0
+check ovn-nbctl --wait=hv ls-copp-add copp0 sw2
+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 957eb7850..81a7aaad8 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -18932,7 +18932,8 @@  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 copp-add copp0 event-elb event-elb
+ovn-nbctl --wait=hv ls-copp-add copp0 sw0
 
 OVN_POPULATE_ARP
 wait_for_ports_up
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 3ae812296..2dcd7e906 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -6642,10 +6642,11 @@  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 copp-add copp0 reject acl-meter
+check ovn-nbctl --wait=hv ls-copp-add copp0 sw0
 check ovn-nbctl acl-add sw0 from-lport 1002 'inport == "sw01" && ip && udp' reject
 
-AT_CHECK([ovn-nbctl ls-copp-list sw0], [0], [dnl
+AT_CHECK([ovn-nbctl copp-list copp0], [0], [dnl
 reject: acl-meter
 ])
 
@@ -6663,7 +6664,7 @@  kill $(pidof tcpdump)
 
 rm -f reject.pcap
 NS_EXEC([sw01], [tcpdump -l -n -i sw01 icmp -Q in > reject.pcap &])
-check ovn-nbctl --wait=hv ls-copp-del sw0 reject
+check ovn-nbctl --wait=hv copp-del copp0 reject
 
 ip netns exec sw01 scapy -H <<-EOF
 p = IP(src="192.168.1.2", dst="192.168.1.1") / UDP(dport = 12345) / Raw(b"X"*64)
@@ -6678,8 +6679,9 @@  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
-AT_CHECK([ovn-nbctl lr-copp-list R1], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp1 arp-resolve arp-meter
+check ovn-nbctl --wait=hv lr-copp-add copp1 R1
+AT_CHECK([ovn-nbctl copp-list copp1], [0], [dnl
 arp-resolve: arp-meter
 ])
 
@@ -6696,8 +6698,9 @@  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
-AT_CHECK([ovn-nbctl lr-copp-list R1 |grep icmp4-error], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp2 icmp4-error icmp-meter
+check ovn-nbctl --wait=hv lr-copp-add copp2 R1
+AT_CHECK([ovn-nbctl copp-list copp2 |grep icmp4-error], [0], [dnl
 icmp4-error: icmp-meter
 ])
 
@@ -6715,8 +6718,9 @@  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
-AT_CHECK([ovn-nbctl lr-copp-list R1 |grep bfd], [0], [dnl
+check ovn-nbctl --wait=hv copp-add copp3 bfd bfd-meter
+check ovn-nbctl --wait=hv lr-copp-add copp3 R1
+AT_CHECK([ovn-nbctl copp-list copp3 |grep bfd], [0], [dnl
 bfd: bfd-meter
 ])
 
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
index 80a564660..545f3bf27 100644
--- a/utilities/ovn-nbctl.8.xml
+++ b/utilities/ovn-nbctl.8.xml
@@ -1474,50 +1474,39 @@ 
     </p>
 
     <dl>
-      <dt><code>ls-copp-add</code> <var>switch</var> <var>proto</var>
+      <dt><code>copp-add</code> <var>name</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
+        to the control plane protection policy <code>name</code>. 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>
+      <dt><code>copp-del</code> <var>name</var> [<var>proto</var>]</dt>
       <dd>
-        Removes the control <code>proto</code> mapping from the
-        <code>switch</code> control plane protection policy. If
+        Removes the control <code>proto</code> mapping for the
+        <code>name</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>
+      <dt><code>copp-list</code> <var>name</var></dt>
       <dd>
         Display the current control plane protection policy for
-        <code>switch</code>.
+        <code>name</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>
+      <dt><code>ls-copp-add</code> <var>name</var> <var>switch</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.
+        Adds the control plane protection policy <code>name</code> to the
+        logical switch <code>switch</code>.
       </dd>
 
-      <dt><code>lr-copp-list</code> <var>router</var></dt>
+      <dt><code>lr-copp-add</code> <var>name</var> <var>router</var></dt>
       <dd>
-        Display the current control plane protection policy for
-        <code>router</code>.
+        Adds the control plane protection policy <code>name</code> to the
+        logical router <code>router</code>.
       </dd>
     </dl>
 
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index d67d2db65..e0d5191f1 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -437,26 +437,20 @@  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\
-                            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\
+  copp-add NAME PROTO METER\n\
+                            Add a copp policy for PROTO packets on NAME\n\
+                            CoPP policy based on an existing METER.\n\
+  copp-del NAME [PROTO]\n\
+                            Delete the copp policy for PROTO packets for\n\
+                            NAME copp. If PROTO is not specified, delete all\n\
+                            copp policies defined for NAME.\n\
+  copp-list NAME\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\
+                            protocols NAME.\n\
+  ls-copp-add NAME SWITCH\n\
+                            Add a NAME copp policy on SWITCH logical switch.\n\
+  lr-copp-add NAME ROUTER\n\
+                            Add a NAME copp policy on ROUTER logical router.\n\
 \n\
 %s\
 %s\
@@ -6278,16 +6272,17 @@  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_name);
     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)
+nbctl_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;
 
     char *error = copp_proto_validate(proto_name);
     if (error) {
@@ -6295,23 +6290,16 @@  nbctl_ls_copp_add(struct ctl_context *ctx)
         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);
+    error = copp_by_name_or_uuid(ctx, ctx->argv[1], false, &copp);
+    copp = copp_meter_add(ctx, copp, proto_name, meter);
+    nbrec_copp_set_name(copp, ctx->argv[1]);
 }
 
 static void
-nbctl_ls_copp_del(struct ctl_context *ctx)
+nbctl_copp_del(struct ctl_context *ctx)
 {
-    const char *ls_name = ctx->argv[1];
     const char *proto_name = NULL;
+    const struct nbrec_copp *copp;
     char *error;
 
     if (ctx->argc == 3) {
@@ -6323,95 +6311,69 @@  nbctl_ls_copp_del(struct ctl_context *ctx)
         }
     }
 
-    const struct nbrec_logical_switch *ls = NULL;
-    error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
+    error = copp_by_name_or_uuid(ctx, ctx->argv[1], false, &copp);
     if (error) {
         ctx->error = error;
         return;
     }
 
-    copp_meter_del(ls->copp, proto_name);
+    copp_meter_del(copp, proto_name);
 }
 
 static void
-nbctl_ls_copp_list(struct ctl_context *ctx)
+nbctl_copp_list(struct ctl_context *ctx)
 {
-    const char *ls_name = ctx->argv[1];
+    const struct nbrec_copp *copp;
 
-    const struct nbrec_logical_switch *ls = NULL;
-    char *error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
+    char *error = copp_by_name_or_uuid(ctx, ctx->argv[1], false, &copp);
     if (error) {
         ctx->error = error;
         return;
     }
 
-    copp_meter_list(ctx, ls->copp);
+    copp_meter_list(ctx, copp);
 }
 
 static void
-nbctl_lr_copp_add(struct ctl_context *ctx)
+nbctl_ls_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_logical_switch *ls = NULL;
+    const char *ls_name = ctx->argv[2];
+    const struct nbrec_copp *copp;
 
-    char *error = copp_proto_validate(proto_name);
+    char *error = ls_by_name_or_uuid(ctx, ls_name, true, &ls);
     if (error) {
         ctx->error = error;
         return;
     }
 
-    const struct nbrec_logical_router *lr = NULL;
-    error = lr_by_name_or_uuid(ctx, lr_name, true, &lr);
+    error = copp_by_name_or_uuid(ctx, ctx->argv[1], true, &copp);
     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);
+    nbrec_logical_switch_set_copp(ls, copp);
 }
 
 static void
-nbctl_lr_copp_del(struct ctl_context *ctx)
+nbctl_lr_copp_add(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);
+    const char *lr_name = ctx->argv[2];
+    const struct nbrec_copp *copp;
+
+    char *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);
+    error = copp_by_name_or_uuid(ctx, ctx->argv[1], true, &copp);
     if (error) {
         ctx->error = error;
         return;
     }
-
-    copp_meter_list(ctx, lr->copp);
+    nbrec_logical_router_set_copp(lr, copp);
 }
 
 static void
@@ -7177,18 +7139,16 @@  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,
-      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,
+    {"copp-add", 3, 3, "NAME PROTO METER", nbctl_pre_copp,
+      nbctl_copp_add, NULL, "", RW},
+    {"copp-del", 1, 2, "NAME [PROTO]", nbctl_pre_copp,
+      nbctl_copp_del, NULL, "", RW},
+    {"copp-list", 1, 1, "NAME", nbctl_pre_copp, nbctl_copp_list,
       NULL, "", RO},
-    {"lr-copp-add", 3, 3, "ROUTER PROTO METER", nbctl_pre_copp,
+    {"ls-copp-add", 2, 2, "NAME SWITCH", nbctl_pre_copp,
+      nbctl_ls_copp_add, NULL, "", RW},
+    {"lr-copp-add", 2, 2, "NAME ROUTER", 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},