| Message ID | 20200602190113.327995-1-mmichels@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] northd: Fix IPAM IPv4 start address calculation. | expand |
On Wed, Jun 3, 2020 at 12:31 AM Mark Michelson <mmichels@redhat.com> wrote: > IPAM assumes the other_config:subnet of the logical switch is a network > address, which can result in unusual address assignments. > > As an example, consider the following configuration: > > ovn-nbctl set logical_switch ls other_config:subnet=172.16.1.254/29 > > 172.16.1.254 is not a network address of a /29 network, but ovn-northd > doesn't care. ovn-northd starts IP address allocation at 172.16.1.254, > with 7 > assignable addresses in the subnet. The first address (172.16.1.255) is > reserved for router port use. The first IP addresses to a logical switch > port is 172.16.2.0, then 172.16.2.1, and so on. > > This patch changes the behavior by using the provided netmask to change > the starting IP address to the network address of the subnet. In the > previous example, the provided 172.16.1.254/29 would be converted > internally to 172.16.1.248/29 . Therefore, the first IP address > allocated to a switch port would be 172.16.1.250. Further allocations would > continue up until 172.16.1.254. > > Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1823287 > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > Acked-by: Numan Siddique <numans@ovn.org> Thanks Numan > --- > northd/ovn-northd.c | 2 +- > tests/ovn.at | 11 +++++++++++ > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index eb78f317e..668c6c2f9 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -728,7 +728,7 @@ init_ipam_info_for_datapath(struct ovn_datapath *od) > return; > } > > - od->ipam_info.start_ipv4 = ntohl(subnet) + 1; > + od->ipam_info.start_ipv4 = ntohl(subnet & mask) + 1; > od->ipam_info.total_ipv4s = ~ntohl(mask); > od->ipam_info.allocated_ipv4s = > bitmap_allocate(od->ipam_info.total_ipv4s); > diff --git a/tests/ovn.at b/tests/ovn.at > index 15b40ca1e..25b47fdff 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -7098,6 +7098,17 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p102 > dynamic_addresses], [0], > ["00:11:22:a8:6e:0b 192.168.110.10 ae01::2" > ]) > > +# Configure subnet using address from middle of the subnet and ensure > +# address is allocated from the beginning. > + > +ovn-nbctl ls-add sw11 > +ovn-nbctl --wait=sb set Logical-Switch sw11 other_config:subnet= > 172.16.1.254/29 > +ovn-nbctl <http://172.16.1.254/29+ovn-nbctl> --wait=sb lsp-add sw11 p103 > -- lsp-set-addresses p103 "22:33:44:55:66:77 dynamic" > + > +AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], > + ["22:33:44:55:66:77 172.16.1.250" > +]) > + > as ovn-sb > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > -- > 2.25.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Tue, Jun 9, 2020 at 12:51 PM Numan Siddique <nusiddiq@redhat.com> wrote: > > > On Wed, Jun 3, 2020 at 12:31 AM Mark Michelson <mmichels@redhat.com> > wrote: > >> IPAM assumes the other_config:subnet of the logical switch is a network >> address, which can result in unusual address assignments. >> >> As an example, consider the following configuration: >> >> ovn-nbctl set logical_switch ls other_config:subnet=172.16.1.254/29 >> >> 172.16.1.254 is not a network address of a /29 network, but ovn-northd >> doesn't care. ovn-northd starts IP address allocation at 172.16.1.254, >> with 7 >> assignable addresses in the subnet. The first address (172.16.1.255) is >> reserved for router port use. The first IP addresses to a logical switch >> port is 172.16.2.0, then 172.16.2.1, and so on. >> >> This patch changes the behavior by using the provided netmask to change >> the starting IP address to the network address of the subnet. In the >> previous example, the provided 172.16.1.254/29 would be converted >> internally to 172.16.1.248/29 . Therefore, the first IP address >> allocated to a switch port would be 172.16.1.250. Further allocations >> would >> continue up until 172.16.1.254. >> >> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1823287 >> >> Signed-off-by: Mark Michelson <mmichels@redhat.com> >> > > Acked-by: Numan Siddique <numans@ovn.org>\ > I applied this patch to master. Thanks Numan > > Thanks > Numan > > >> --- >> northd/ovn-northd.c | 2 +- >> tests/ovn.at | 11 +++++++++++ >> 2 files changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >> index eb78f317e..668c6c2f9 100644 >> --- a/northd/ovn-northd.c >> +++ b/northd/ovn-northd.c >> @@ -728,7 +728,7 @@ init_ipam_info_for_datapath(struct ovn_datapath *od) >> return; >> } >> >> - od->ipam_info.start_ipv4 = ntohl(subnet) + 1; >> + od->ipam_info.start_ipv4 = ntohl(subnet & mask) + 1; >> od->ipam_info.total_ipv4s = ~ntohl(mask); >> od->ipam_info.allocated_ipv4s = >> bitmap_allocate(od->ipam_info.total_ipv4s); >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 15b40ca1e..25b47fdff 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -7098,6 +7098,17 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p102 >> dynamic_addresses], [0], >> ["00:11:22:a8:6e:0b 192.168.110.10 ae01::2" >> ]) >> >> +# Configure subnet using address from middle of the subnet and ensure >> +# address is allocated from the beginning. >> + >> +ovn-nbctl ls-add sw11 >> +ovn-nbctl --wait=sb set Logical-Switch sw11 other_config:subnet= >> 172.16.1.254/29 >> +ovn-nbctl <http://172.16.1.254/29+ovn-nbctl> --wait=sb lsp-add sw11 >> p103 -- lsp-set-addresses p103 "22:33:44:55:66:77 dynamic" >> + >> +AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], >> + ["22:33:44:55:66:77 172.16.1.250" >> +]) >> + >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> >> -- >> 2.25.4 >> >> _______________________________________________ >> 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 eb78f317e..668c6c2f9 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -728,7 +728,7 @@ init_ipam_info_for_datapath(struct ovn_datapath *od) return; } - od->ipam_info.start_ipv4 = ntohl(subnet) + 1; + od->ipam_info.start_ipv4 = ntohl(subnet & mask) + 1; od->ipam_info.total_ipv4s = ~ntohl(mask); od->ipam_info.allocated_ipv4s = bitmap_allocate(od->ipam_info.total_ipv4s); diff --git a/tests/ovn.at b/tests/ovn.at index 15b40ca1e..25b47fdff 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -7098,6 +7098,17 @@ AT_CHECK([ovn-nbctl get Logical-Switch-Port p102 dynamic_addresses], [0], ["00:11:22:a8:6e:0b 192.168.110.10 ae01::2" ]) +# Configure subnet using address from middle of the subnet and ensure +# address is allocated from the beginning. + +ovn-nbctl ls-add sw11 +ovn-nbctl --wait=sb set Logical-Switch sw11 other_config:subnet=172.16.1.254/29 +ovn-nbctl --wait=sb lsp-add sw11 p103 -- lsp-set-addresses p103 "22:33:44:55:66:77 dynamic" + +AT_CHECK([ovn-nbctl get Logical-Switch-Port p103 dynamic_addresses], [0], + ["22:33:44:55:66:77 172.16.1.250" +]) + as ovn-sb OVS_APP_EXIT_AND_WAIT([ovsdb-server])
IPAM assumes the other_config:subnet of the logical switch is a network address, which can result in unusual address assignments. As an example, consider the following configuration: ovn-nbctl set logical_switch ls other_config:subnet=172.16.1.254/29 172.16.1.254 is not a network address of a /29 network, but ovn-northd doesn't care. ovn-northd starts IP address allocation at 172.16.1.254, with 7 assignable addresses in the subnet. The first address (172.16.1.255) is reserved for router port use. The first IP addresses to a logical switch port is 172.16.2.0, then 172.16.2.1, and so on. This patch changes the behavior by using the provided netmask to change the starting IP address to the network address of the subnet. In the previous example, the provided 172.16.1.254/29 would be converted internally to 172.16.1.248/29 . Therefore, the first IP address allocated to a switch port would be 172.16.1.250. Further allocations would continue up until 172.16.1.254. Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1823287 Signed-off-by: Mark Michelson <mmichels@redhat.com> --- northd/ovn-northd.c | 2 +- tests/ovn.at | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)