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

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Mon May 24 16:24:15 UTC 2021


> On 4/29/21 1:04 PM, Lorenzo Bianconi wrote:
> > From: Dumitru Ceara <dceara at redhat.com>
> > 

[...]

> > +
> > +    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;
> 
> You can replace the above 3 lines with:
> 
> return ds_steal_cstr(&usage);
> 
> Stealing the cstr means you do not need to destroy the dynamic string, and
> it saves you an extra allocation.

ack, I will fix it in v2

> 
[...]
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn -- check CoPP config])
> 
> I think this test should also attempt trying to add a nonsense protocol
> name, and ensure that
> 
> 1) It doesn't cause OVN to blow up.
> 2) It doesn't end up somehow getting configured.
> 
> This should also attempt to ls-copp-add an acceptable protocol but with a
> non-existent meter for similar purposes.

ack, I will fix it in v2

> 
> > +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
> > +])
> > +
> > +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
> 
> "These commands manage meters configured in ..."

ack, I will fix it in v2

> 
> > +      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 there also be a list item here mentioning packets that need to be
> rplied to with SCTP ABORT?

ack, I will fix it in v2

Regards,
Lorenzo

> 
> > +      </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>
> > +
> > +      <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},
> > 
> 


More information about the dev mailing list