[ovs-dev] [PATCH ovn] ovn-nbctl: add --may-exist/--if-exists options for policy routing
Han Zhou
zhouhan at gmail.com
Fri Sep 25 18:00:23 UTC 2020
On Fri, Sep 25, 2020 at 4:22 AM Lorenzo Bianconi <
lorenzo.bianconi at redhat.com> wrote:
>
> Introduce the following options to avoid error reporting for policy
> routing:
> 1) --may-exist: the lr-policy-add does not result in an error if a policy
> with the same priority and match string is already present
> 2) --if-exists: the lr-policy-del does not result in an error if a policy
> with the specified uuid is not present in the db
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> tests/ovn-nbctl.at | 7 ++++++-
> utilities/ovn-nbctl.8.xml | 20 +++++++++++++++-----
> utilities/ovn-nbctl.c | 16 ++++++++++------
> 3 files changed, 31 insertions(+), 12 deletions(-)
>
> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> index baf7a87f5..3dbedc843 100644
> --- a/tests/ovn-nbctl.at
> +++ b/tests/ovn-nbctl.at
> @@ -1651,6 +1651,8 @@ AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src
== 1.1.1.0/24" drop], [1], []
> [ovn-nbctl: Same routing policy already existed on the logical router
lr0.
> ])
>
> +AT_CHECK([ovn-nbctl --may-exist lr-policy-add lr0 100 "ip4.src ==
1.1.1.0/24" drop])
> +
> dnl Add duplicated policy
> AT_CHECK([ovn-nbctl lr-policy-add lr0 103 "ip4.src == 1.1.1.0/24" deny],
[1], [],
> [ovn-nbctl: deny: action must be one of "allow", "drop", and "reroute"
> @@ -1675,10 +1677,13 @@ Routing Policies
>
>
> dnl Delete policy by specified uuid
> -AT_CHECK([ovn-nbctl lr-policy-del lr0 $(ovn-nbctl --bare --column _uuid
list logical_router_policy)])
> +uuid=$(ovn-nbctl --bare --column _uuid list logical_router_policy)
> +AT_CHECK([ovn-nbctl lr-policy-del lr0 $uuid])
> AT_CHECK([ovn-nbctl list logical-router-policy], [0], [dnl
> ])
>
> +AT_CHECK([ovn-nbctl --if-exists lr-policy-del lr0 $uuid])
> +
> dnl Add policy with reroute action
> AT_CHECK([ovn-nbctl lr-policy-add lr0 102 "ip4.src == 3.1.2.0/24"
reroute 3.3.3.3])
>
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index fcc4312dd..59302296b 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -737,8 +737,9 @@
> <h1>Logical Router Policy Commands</h1>
>
> <dl>
> - <dt><code>lr-policy-add</code> <var>router</var>
<var>priority</var>
> - <var>match</var> <var>action</var> [<var>nexthop</var>]
> + <dt>[<code>--may-exist</code>]<code>lr-policy-add</code>
> + <var>router</var> <var>priority</var> <var>match</var>
> + <var>action</var> [<var>nexthop</var>]
> [<var>options key=value]</var>] </dt>
> <dd>
> <p>
> @@ -754,6 +755,13 @@
> The supported option is : <code>pkt_mark</code>.
> </p>
>
> + <p>
> + If <code>--may-exist</code> is specified, adding a duplicated
> + routing policy with the same priority and match string is not
> + really created. Without <code>--may-exist</code>, adding a
> + duplicated routing policy results in error.
> + </p>
> +
> <p>
> The following example shows a policy to lr1, which will drop
packets
> from<code>192.168.100.0/24</code>.
> @@ -771,8 +779,8 @@
> </p>
> </dd>
>
> - <dt><code>lr-policy-del</code> <var>router</var> [<var>{priority |
uuid}
> - [match]</var>]</dt>
> + <dt>[<code>--if-exists</code>] <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>
> @@ -784,7 +792,9 @@
>
> <p>
> If <var>router</var> and <var>uuid</var> are supplied, then the
> - policy with sepcified uuid is deleted.
> + policy with sepcified uuid is deleted. It is an error if
> + <var>uuid</var> does not exist, unless <code>--if-exists</code>
> + is specified.
> </p>
> </dd>
>
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index c54e63937..caf99dfeb 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -3648,12 +3648,15 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
>
> /* Check if same routing policy already exists.
> * A policy is uniquely identified by priority and match */
> + bool may_exist = !!shash_find(&ctx->options, "--may-exist");
> for (int i = 0; i < lr->n_policies; i++) {
> const struct nbrec_logical_router_policy *policy =
lr->policies[i];
> if (policy->priority == priority &&
> !strcmp(policy->match, ctx->argv[3])) {
> - ctl_error(ctx, "Same routing policy already existed on the "
> - "logical router %s.", ctx->argv[1]);
> + if (!may_exist) {
> + ctl_error(ctx, "Same routing policy already existed on
the "
> + "logical router %s.", ctx->argv[1]);
> + }
> return;
> }
> }
> @@ -3733,7 +3736,6 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
> ctx->error = error;
> return;
> }
> -
> }
> /* If uuid was specified, delete routing policy with the
> * specified uuid. */
> @@ -3751,7 +3753,9 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
> }
> }
> if (n_policies == lr->n_policies) {
> - ctl_error(ctx, "Logical router policy uuid is not
found.");
> + if (!shash_find(&ctx->options, "--if-exists")) {
> + ctl_error(ctx, "Logical router policy uuid is not
found.");
> + }
> return;
> }
>
> @@ -6529,9 +6533,9 @@ static const struct ctl_command_syntax
nbctl_commands[] = {
> /* Policy commands */
> { "lr-policy-add", 4, INT_MAX,
> "ROUTER PRIORITY MATCH ACTION [NEXTHOP] [OPTIONS - KEY=VALUE ...]",
> - NULL, nbctl_lr_policy_add, NULL, "", RW },
> + NULL, nbctl_lr_policy_add, NULL, "--may-exist", RW },
> { "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]", NULL,
> - nbctl_lr_policy_del, NULL, "", RW },
> + nbctl_lr_policy_del, NULL, "--if-exists", RW },
> { "lr-policy-list", 1, 1, "ROUTER", NULL, nbctl_lr_policy_list, NULL,
> "", RO },
>
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks Lorenzo, I applied this to master.
More information about the dev
mailing list