| Message ID | 20210407211821.3047146-1-aidan.shribman@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] ovn-nbctl: fails to delete and add lr-nat in trx | expand |
Bleep bloop. Greetings Aidan Shribman, 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 non-spaces leading whitespace
#74 FILE: utilities/ovn-nbctl.c:4545:
free(keep_nat);
Lines checked: 121, Warnings: 1, Errors: 0
build:
/bin/python3 ./build-aux/dpdkstrip.py | \
sed \
-e 's,[@]PKIDIR[@],/usr/local/var/lib/openvswitch/pki,g' \
-e 's,[@]LOGDIR[@],/usr/local/var/log/ovn,g' \
-e 's,[@]DBDIR[@],/usr/local/etc/ovn,g' \
-e 's,[@]PYTHON3[@],/bin/python3,g' \
-e 's,[@]OVN_RUNDIR[@],/usr/local/var/run/ovn,g' \
-e 's,[@]OVSBUILDDIR[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR,g' \
-e 's,[@]VERSION[@],21.03.90,g' \
-e 's,[@]OVSVERSION[@],2.15.90,g' \
-e 's,[@]localstatedir[@],/usr/local/var,g' \
-e 's,[@]pkgdatadir[@],/usr/local/share/ovn,g' \
-e 's,[@]sysconfdir[@],/usr/local/etc,g' \
-e 's,[@]bindir[@],/usr/local/bin,g' \
-e 's,[@]sbindir[@],/usr/local/sbin,g' \
-e 's,[@]abs_builddir[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace,g' \
-e 's,[@]abs_top_srcdir[@],/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace,g' \
> rhel/ovn-fedora.spec.tmp
mv rhel/ovn-fedora.spec.tmp rhel/ovn-fedora.spec
utilities/ovn-nbctl.c
See above for files that use tabs for indentation.
Please use spaces instead.
make[1]: *** [check-tabs] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 184058356..621a5a9fc 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); }
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 <aidan.shribman@gmail.com> Fixed: 1928226 (ovn-nbctl fails to delete/add lr-nat in trx) --- utilities/ovn-nbctl.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-)