| Message ID | cbd34ffff12b0a0d35b71ad84249fa09395c595a.1601659682.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] northd: properly reconfigure ipam when subnet is changed | expand |
Acked-by: Mark Michelson <mmichels@redhat.com> On 10/2/20 1:31 PM, Lorenzo Bianconi wrote: > Reconfigure dynamic assigned addresses if subnet is modified > removing IP out of configured netmask if present > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v1: > - do not remove invalid address in ipam_get_unused_ip() > --- > northd/ovn-northd.c | 2 +- > tests/ovn.at | 14 ++++++++++++++ > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 91da31941..afc9b4e91 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -1793,7 +1793,7 @@ dynamic_ip4_changed(const char *lsp_addrs, > } > > uint32_t index = ip4 - ipam->start_ipv4; > - if (index > ipam->total_ipv4s || > + if (index >= ipam->total_ipv4s - 1 || > bitmap_is_set(ipam->allocated_ipv4s, index)) { > /* Previously assigned dynamic IPv4 address can no longer be used. > * It's either outside the subnet, conflicts with an excluded IP, > diff --git a/tests/ovn.at b/tests/ovn.at > index 7769b69ed..f1041bf0b 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -7473,6 +7473,20 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], > ["22:33:44:55:66:77 172.16.1.250" > ]) > > +ovn-nbctl ls-add sw12 > +for i in $(seq 0 1); do > + for j in $(seq 1 99); do > + idx=$((i*100+j)) > + ovn-nbctl lsp-add sw12 sw12-p${idx} -- \ > + lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" > + done > +done > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) > + > as ovn-sb > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > >
On Wed, Oct 7, 2020 at 12:33 AM Mark Michelson <mmichels@redhat.com> wrote: > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks Lorenzo and Mark. I applied this patch to master with the below changes to the test case you added. I added --wait=sb to the ovn-nbctl commands so that the test doesn't fail due to timing issues. @Lorenzo Bianconi Does this require a backport ? ******** diff --git a/tests/ovn.at b/tests/ovn.at index f1041bf0bb..08395fdbbf 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -7481,8 +7481,12 @@ for i in $(seq 0 1); do lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" done done -ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 -ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 +ovn-nbctl --wait=sb set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [0]) +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [0]) +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [0]) + +ovn-nbctl --wait=sb set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) ************* Thanks Numan > > On 10/2/20 1:31 PM, Lorenzo Bianconi wrote: > > Reconfigure dynamic assigned addresses if subnet is modified > > removing IP out of configured netmask if present > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > --- > > Changes since v1: > > - do not remove invalid address in ipam_get_unused_ip() > > --- > > northd/ovn-northd.c | 2 +- > > tests/ovn.at | 14 ++++++++++++++ > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 91da31941..afc9b4e91 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -1793,7 +1793,7 @@ dynamic_ip4_changed(const char *lsp_addrs, > > } > > > > uint32_t index = ip4 - ipam->start_ipv4; > > - if (index > ipam->total_ipv4s || > > + if (index >= ipam->total_ipv4s - 1 || > > bitmap_is_set(ipam->allocated_ipv4s, index)) { > > /* Previously assigned dynamic IPv4 address can no longer be used. > > * It's either outside the subnet, conflicts with an excluded IP, > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 7769b69ed..f1041bf0b 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -7473,6 +7473,20 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], > > ["22:33:44:55:66:77 172.16.1.250" > > ]) > > > > +ovn-nbctl ls-add sw12 > > +for i in $(seq 0 1); do > > + for j in $(seq 1 99); do > > + idx=$((i*100+j)) > > + ovn-nbctl lsp-add sw12 sw12-p${idx} -- \ > > + lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" > > + done > > +done > > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 > > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) > > + > > as ovn-sb > > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
> > On Wed, Oct 7, 2020 at 12:33 AM Mark Michelson <mmichels@redhat.com> wrote: > > > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Thanks Lorenzo and Mark. > > I applied this patch to master with the below changes to the test case > you added. > I added --wait=sb to the ovn-nbctl commands so that the test doesn't > fail due to timing issues. > > @Lorenzo Bianconi Does this require a backport ? > ack, thx Numan. I do not think it is required for the moment. Regards, Lorenzo > ******** > diff --git a/tests/ovn.at b/tests/ovn.at > index f1041bf0bb..08395fdbbf 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -7481,8 +7481,12 @@ for i in $(seq 0 1); do > lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" > done > done > -ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 > -ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 > +ovn-nbctl --wait=sb set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [0]) > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [0]) > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [0]) > + > +ovn-nbctl --wait=sb set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 > AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) > AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) > AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) > ************* > > Thanks > Numan > > > > > > On 10/2/20 1:31 PM, Lorenzo Bianconi wrote: > > > Reconfigure dynamic assigned addresses if subnet is modified > > > removing IP out of configured netmask if present > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > > > --- > > > Changes since v1: > > > - do not remove invalid address in ipam_get_unused_ip() > > > --- > > > northd/ovn-northd.c | 2 +- > > > tests/ovn.at | 14 ++++++++++++++ > > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > > index 91da31941..afc9b4e91 100644 > > > --- a/northd/ovn-northd.c > > > +++ b/northd/ovn-northd.c > > > @@ -1793,7 +1793,7 @@ dynamic_ip4_changed(const char *lsp_addrs, > > > } > > > > > > uint32_t index = ip4 - ipam->start_ipv4; > > > - if (index > ipam->total_ipv4s || > > > + if (index >= ipam->total_ipv4s - 1 || > > > bitmap_is_set(ipam->allocated_ipv4s, index)) { > > > /* Previously assigned dynamic IPv4 address can no longer be used. > > > * It's either outside the subnet, conflicts with an excluded IP, > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index 7769b69ed..f1041bf0b 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -7473,6 +7473,20 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], > > > ["22:33:44:55:66:77 172.16.1.250" > > > ]) > > > > > > +ovn-nbctl ls-add sw12 > > > +for i in $(seq 0 1); do > > > + for j in $(seq 1 99); do > > > + idx=$((i*100+j)) > > > + ovn-nbctl lsp-add sw12 sw12-p${idx} -- \ > > > + lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" > > > + done > > > +done > > > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 > > > +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 > > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) > > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) > > > +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) > > > + > > > as ovn-sb > > > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 91da31941..afc9b4e91 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1793,7 +1793,7 @@ dynamic_ip4_changed(const char *lsp_addrs, } uint32_t index = ip4 - ipam->start_ipv4; - if (index > ipam->total_ipv4s || + if (index >= ipam->total_ipv4s - 1 || bitmap_is_set(ipam->allocated_ipv4s, index)) { /* Previously assigned dynamic IPv4 address can no longer be used. * It's either outside the subnet, conflicts with an excluded IP, diff --git a/tests/ovn.at b/tests/ovn.at index 7769b69ed..f1041bf0b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -7473,6 +7473,20 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], ["22:33:44:55:66:77 172.16.1.250" ]) +ovn-nbctl ls-add sw12 +for i in $(seq 0 1); do + for j in $(seq 1 99); do + idx=$((i*100+j)) + ovn-nbctl lsp-add sw12 sw12-p${idx} -- \ + lsp-set-addresses sw12-p${idx} "00:00:00:00:$i:$j dynamic" + done +done +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/24 +ovn-nbctl set Logical-Switch sw12 other_config:subnet=192.10.2.0/25 +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.127], [1]) +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.128], [1]) +AT_CHECK([ovn-nbctl list Logical-Switch-Port | grep -q 192.10.2.180], [1]) + as ovn-sb OVS_APP_EXIT_AND_WAIT([ovsdb-server])
Reconfigure dynamic assigned addresses if subnet is modified removing IP out of configured netmask if present Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- Changes since v1: - do not remove invalid address in ipam_get_unused_ip() --- northd/ovn-northd.c | 2 +- tests/ovn.at | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-)