Message ID | 8e1242b09cee0f6bb967496873bea34c0e4ea233.1601032854.git.lorenzo.bianconi@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] ovn-nbctl: add --may-exist/--if-exists options for policy routing | expand |
On Fri, Sep 25, 2020 at 4:22 AM Lorenzo Bianconi < lorenzo.bianconi@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@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks Lorenzo, I applied this to master.
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 },
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@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(-)