Message ID | 20200324014951.9521-1-taoyunxiang@cmss.chinamobile.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,OVN,v2] ovn-nbctl.c: Add an optional way to delete router policy by uuid | expand |
Bleep bloop. Greetings Tao YunXiang, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line has trailing whitespace #46 FILE: utilities/ovn-nbctl.8.xml:725: only when <var>action</var> is <var>reroute</var>. A policy is WARNING: Line lacks whitespace around operator #97 FILE: utilities/ovn-nbctl.c:699: lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\ Lines checked: 162, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Tue, Mar 24, 2020 at 7:20 AM Tao YunXiang <taoyunxiang@cmss.chinamobile.com> wrote: > > We can delete router policy by specify lr and more parameters. > If CMS want to delete it exactly, it must specify detailed "match" field. > It's not an easy way, also maybe deleted by mistake. > This change adds a way to specify lr and uuid, which is optional. > You can still use the previous method to delete. > > usage: > ovn-nbctl lr-policy-del lr0 [UUID0] > > Author: Tao YunXiang <taoyunxiang@cmss.chinamobile.com> > Co-authored-by: Liu Chang <liuchang@cmss.chinamobile.com> > Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com> > Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com> > Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com> > Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com> Can you please add a few tests in the tests/ovn-nbctl.at which would help in regressions ? Thanks Numan > > --- > utilities/ovn-nbctl.8.xml | 51 +++++++++++++++++++++++++++++++++++++++ > utilities/ovn-nbctl.c | 41 ++++++++++++++++++++++--------- > 2 files changed, 81 insertions(+), 11 deletions(-) > > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > index d973be259..b91e39042 100644 > --- a/utilities/ovn-nbctl.8.xml > +++ b/utilities/ovn-nbctl.8.xml > @@ -710,6 +710,57 @@ > </dd> > </dl> > > + <h1>Logical Router Policy Commands</h1> > + > + <dl> > + <dt>lr-policy-add</code> <var>router</var> <var>priority</var> > + <var>match</var> <var>action</var> [<var>nexthop</var>]</dt> > + <dd> > + <p> > + Add Policy to <var>router</var> which provides a way to configure > + permit/deny and reroute policies on the router. Permit/deny policies > + are similar to OVN ACLs, but exist on the logical-router. Reroute > + policies are needed for service-insertion and service-chaining. > + <var>nexthop</var> is an optional parameter. It needs to be provided > + only when <var>action</var> is <var>reroute</var>. A policy is > + uniquely identified by <var>priority</var> and <var>match</var>. > + Multiple policies can have the same <var>priority</var>. > + </p> > + > + <p> > + The following example shows a policy to lr1, which will drop packets > + from<code>192.168.100.0/24</code>. > + </p> > + > + <p> > + <code>lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 drop</code>. > + </p> > + > + </dd> > + > + <dt><code>lr-policy-del</code> <var>router</var> [<var>{priority | uuid} > + [match]</var>]</dt> > + <dd> > + <p> > + Deletes polices from <var>router</var>. If only <var>router</var> > + is supplied, all the polices from the logical router are deleted. If > + <var>priority</var> and/or <var>match</var> are also specified, then > + all the polices that match the conditions will be deleted from the > + logical router. > + </p> > + > + <p> > + If <var>router</var> and <var>uuid</var> are supplied, then the > + policy with sepcified uuid is deleted. > + </p> > + </dd> > + > + <dt><code>lr-policy-list</code> <var>router</var></dt> > + <dd> > + Lists the polices on <var>router</var>. > + </dd> > + </dl> > + > <h1>NAT Commands</h1> > > <dl> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index e80058e61..1983b5f95 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -696,7 +696,7 @@ Route commands:\n\ > Policy commands:\n\ > lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\ > add a policy to router\n\ > - lr-policy-del ROUTER [PRIORITY [MATCH]]\n\ > + lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\ > remove policies from ROUTER\n\ > lr-policy-list ROUTER print policies for ROUTER\n\ > \n\ > @@ -3587,21 +3587,40 @@ nbctl_lr_policy_del(struct ctl_context *ctx) > return; > } > > - error = parse_priority(ctx->argv[2], &priority); > - if (error) { > - ctx->error = error; > - return; > + const struct uuid *lr_policy_uuid = NULL; > + struct uuid uuid_from_cmd; > + if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) { > + lr_policy_uuid = &uuid_from_cmd; > + } else { > + error = parse_priority(ctx->argv[2], &priority); > + if (error) { > + ctx->error = error; > + return; > + } > + > } > - /* If match is not specified, delete all routing policies with the > - * specified priority. */ > + /* If uuid was specified, delete routing policy with the > + * specified uuid. */ > if (ctx->argc == 3) { > struct nbrec_logical_router_policy **new_policies > = xmemdup(lr->policies, > sizeof *new_policies * lr->n_policies); > int n_policies = 0; > - for (int i = 0; i < lr->n_policies; i++) { > - if (priority != lr->policies[i]->priority) { > - new_policies[n_policies++] = lr->policies[i]; > + > + if (lr_policy_uuid) { > + for (size_t i = 0; i < lr->n_policies; i++) { > + if (!uuid_equals(lr_policy_uuid, > + &(lr->policies[i]->header_.uuid))) { > + new_policies[n_policies++] = lr->policies[i]; > + } > + } > + /* If match is not specified, delete all routing policies with the > + * specified priority. */ > + } else { > + for (int i = 0; i < lr->n_policies; i++) { > + if (priority != lr->policies[i]->priority) { > + new_policies[n_policies++] = lr->policies[i]; > + } > } > } > nbrec_logical_router_verify_policies(lr); > @@ -6125,7 +6144,7 @@ static const struct ctl_command_syntax nbctl_commands[] = { > /* Policy commands */ > { "lr-policy-add", 4, 5, "ROUTER PRIORITY MATCH ACTION [NEXTHOP]", NULL, > nbctl_lr_policy_add, NULL, "", RW }, > - { "lr-policy-del", 1, 3, "ROUTER [PRIORITY [MATCH]]", NULL, > + { "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]", NULL, > nbctl_lr_policy_del, NULL, "", RW }, > { "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL, > "", RO }, > -- > 2.17.1 > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index d973be259..b91e39042 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -710,6 +710,57 @@ </dd> </dl> + <h1>Logical Router Policy Commands</h1> + + <dl> + <dt>lr-policy-add</code> <var>router</var> <var>priority</var> + <var>match</var> <var>action</var> [<var>nexthop</var>]</dt> + <dd> + <p> + Add Policy to <var>router</var> which provides a way to configure + permit/deny and reroute policies on the router. Permit/deny policies + are similar to OVN ACLs, but exist on the logical-router. Reroute + policies are needed for service-insertion and service-chaining. + <var>nexthop</var> is an optional parameter. It needs to be provided + only when <var>action</var> is <var>reroute</var>. A policy is + uniquely identified by <var>priority</var> and <var>match</var>. + Multiple policies can have the same <var>priority</var>. + </p> + + <p> + The following example shows a policy to lr1, which will drop packets + from<code>192.168.100.0/24</code>. + </p> + + <p> + <code>lr-policy-add lr1 100 ip4.src == 192.168.100.0/24 drop</code>. + </p> + + </dd> + + <dt><code>lr-policy-del</code> <var>router</var> [<var>{priority | uuid} + [match]</var>]</dt> + <dd> + <p> + Deletes polices from <var>router</var>. If only <var>router</var> + is supplied, all the polices from the logical router are deleted. If + <var>priority</var> and/or <var>match</var> are also specified, then + all the polices that match the conditions will be deleted from the + logical router. + </p> + + <p> + If <var>router</var> and <var>uuid</var> are supplied, then the + policy with sepcified uuid is deleted. + </p> + </dd> + + <dt><code>lr-policy-list</code> <var>router</var></dt> + <dd> + Lists the polices on <var>router</var>. + </dd> + </dl> + <h1>NAT Commands</h1> <dl> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index e80058e61..1983b5f95 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -696,7 +696,7 @@ Route commands:\n\ Policy commands:\n\ lr-policy-add ROUTER PRIORITY MATCH ACTION [NEXTHOP]\n\ add a policy to router\n\ - lr-policy-del ROUTER [PRIORITY [MATCH]]\n\ + lr-policy-del ROUTER [{PRIORITY | UUID} [MATCH]]\n\ remove policies from ROUTER\n\ lr-policy-list ROUTER print policies for ROUTER\n\ \n\ @@ -3587,21 +3587,40 @@ nbctl_lr_policy_del(struct ctl_context *ctx) return; } - error = parse_priority(ctx->argv[2], &priority); - if (error) { - ctx->error = error; - return; + const struct uuid *lr_policy_uuid = NULL; + struct uuid uuid_from_cmd; + if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) { + lr_policy_uuid = &uuid_from_cmd; + } else { + error = parse_priority(ctx->argv[2], &priority); + if (error) { + ctx->error = error; + return; + } + } - /* If match is not specified, delete all routing policies with the - * specified priority. */ + /* If uuid was specified, delete routing policy with the + * specified uuid. */ if (ctx->argc == 3) { struct nbrec_logical_router_policy **new_policies = xmemdup(lr->policies, sizeof *new_policies * lr->n_policies); int n_policies = 0; - for (int i = 0; i < lr->n_policies; i++) { - if (priority != lr->policies[i]->priority) { - new_policies[n_policies++] = lr->policies[i]; + + if (lr_policy_uuid) { + for (size_t i = 0; i < lr->n_policies; i++) { + if (!uuid_equals(lr_policy_uuid, + &(lr->policies[i]->header_.uuid))) { + new_policies[n_policies++] = lr->policies[i]; + } + } + /* If match is not specified, delete all routing policies with the + * specified priority. */ + } else { + for (int i = 0; i < lr->n_policies; i++) { + if (priority != lr->policies[i]->priority) { + new_policies[n_policies++] = lr->policies[i]; + } } } nbrec_logical_router_verify_policies(lr); @@ -6125,7 +6144,7 @@ static const struct ctl_command_syntax nbctl_commands[] = { /* Policy commands */ { "lr-policy-add", 4, 5, "ROUTER PRIORITY MATCH ACTION [NEXTHOP]", NULL, nbctl_lr_policy_add, NULL, "", RW }, - { "lr-policy-del", 1, 3, "ROUTER [PRIORITY [MATCH]]", NULL, + { "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]", NULL, nbctl_lr_policy_del, NULL, "", RW }, { "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL, "", RO },