[ovs-dev,OVN,v2] ovn-nbctl.c: Add an optional way to delete router policy by uuid

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>

---
 utilities/ovn-nbctl.8.xml | 51 +++++++++++++++++++++++++++++++++++++++
 utilities/ovn-nbctl.c     | 41 ++++++++++++++++++++++---------
 2 files changed, 81 insertions(+), 11 deletions(-)

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
>