yes. will add a test On Wed, Apr 21, 2021, 10:51 Numan Siddique <[email protected]> wrote:
> On Thu, Apr 8, 2021 at 3:13 AM Aidan Shribman <[email protected]> > wrote: > > > > The delete operation did not update the internal state cuasing the add > > operation to wrongly believe that the lr-nat entry is still present and > > thus can't be added. > > > > Below is the sequance is failed before and now passes: > > > > *** > > echo "add switch" > > ovn-nbctl ls-del ls0 2>/dev/null > > ovn-nbctl ls-add ls0 > > ovn-nbctl ls-list > > ovn-nbctl show ls0 > > ovn-nbctl lsp-del lsp0 2>/dev/null > > ovn-nbctl lsp-add ls0 lsp0 00:00:00:00:00:01 10.0.0.0/24 > > ovn-nbctl lsp-list ls0 > > > > echo "add router" > > ovn-nbctl ls-del lr0 2>/dev/null > > ovn-nbctl lr-add lr0 > > ovn-nbctl lr-list > > ovn-nbctl show lr0 > > > > echo "add nat" > > ovn-nbctl lr-nat-del lr0 2>/dev/null > > ovn-nbctl lr-nat-add lr0 dnat_and_snat 192.168.0.3 10.0.0.3 \ > > lsp0 f0:00:00:00:00:03 > > ovn-nbctl lr-nat-list lr0 > > > > echo "FIXED: delete nat (all of type) + add nat" > > ovn-nbctl lr-nat-del lr0 dnat_and_snat -- \ > > --may-exist lr-nat-add lr0 dnat_and_snat 192.168.0.3 10.0.0.3 \ > > lsp0 f0:00:00:00:00:03 > > ovn-nbctl lr-nat-list lr0 > > > > echo "FIXED: delete nat (all of external_ip) + add" > > ovn-nbctl lr-nat-del lr0 dnat_and_snat 192.168.0.3 -- \ > > --may-exist lr-nat-add lr0 dnat_and_snat 192.168.0.3 10.0.0.3 \ > > lsp0 f0:00:00:00:00:03 > > ovn-nbctl lr-nat-list lr0 > > *** > > > > Signed-off-by: Aidan Shribman <[email protected]> > > Fixed: 1928226 (ovn-nbctl fails to delete/add lr-nat in trx) > > Hi Aidan, > > I haven't reviewed the code yet. But looking into the commit message, > I think you can add a test case for this fix ? Can you please add the test > case > and submit v3 ? > > Thanks > Numan > > > --- > > utilities/ovn-nbctl.c | 33 ++++++++++++++++++--------------- > > 1 file changed, 18 insertions(+), 15 deletions(-) > > > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > > index 184058356..db1ac4f5b 100644 > > --- a/utilities/ovn-nbctl.c > > +++ b/utilities/ovn-nbctl.c > > @@ -4531,11 +4531,18 @@ nbctl_lr_nat_del(struct ctl_context *ctx) > > > > if (ctx->argc == 3) { > > /*Deletes all NATs with the specified type. */ > > + struct nbrec_nat **keep_nat = xmalloc(lr->n_nat * sizeof > *keep_nat); > > + size_t n_keep_nat = 0; > > for (size_t i = 0; i < lr->n_nat; i++) { > > - if (!strcmp(nat_type, lr->nat[i]->type)) { > > - nbrec_logical_router_update_nat_delvalue(lr, > lr->nat[i]); > > + if (strcmp(nat_type, lr->nat[i]->type)) { > > + keep_nat[n_keep_nat++] = lr->nat[i]; > > } > > } > > + if (n_keep_nat < lr->n_nat) { > > + nbrec_logical_router_verify_nat(lr); > > + nbrec_logical_router_set_nat(lr, keep_nat, n_keep_nat); > > + } > > + free(keep_nat); > > return; > > } > > > > @@ -4547,31 +4554,27 @@ nbctl_lr_nat_del(struct ctl_context *ctx) > > > > int is_snat = !strcmp("snat", nat_type); > > /* Remove the matching NAT. */ > > + struct nbrec_nat **keep_nat = xmalloc(lr->n_nat * sizeof *keep_nat); > > + size_t n_keep_nat = 0; > > for (size_t i = 0; i < lr->n_nat; i++) { > > struct nbrec_nat *nat = lr->nat[i]; > > - bool should_return = false; > > char *old_ip = normalize_prefix_str(is_snat > > ? nat->logical_ip > > : nat->external_ip); > > - if (!old_ip) { > > - continue; > > - } > > - if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip)) { > > - nbrec_logical_router_update_nat_delvalue(lr, nat); > > - should_return = true; > > + if (!old_ip || strcmp(nat_type, nat->type) || strcmp(nat_ip, > old_ip)) { > > + keep_nat[n_keep_nat++] = lr->nat[i]; > > } > > free(old_ip); > > - if (should_return) { > > - goto cleanup; > > - } > > } > > > > - if (must_exist) { > > + if (n_keep_nat < lr->n_nat) { > > + nbrec_logical_router_verify_nat(lr); > > + nbrec_logical_router_set_nat(lr, keep_nat, n_keep_nat); > > + } else if (must_exist) { > > ctl_error(ctx, "no matching NAT with the type (%s) and %s (%s)", > > nat_type, is_snat ? "logical_ip" : "external_ip", > nat_ip); > > } > > - > > -cleanup: > > + free(keep_nat); > > free(nat_ip); > > } > > > > -- > > 2.25.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
