[ovs-dev] [PATCH ovn] northd: properly reconfigure ipam when subnet is changed

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Fri Oct 2 17:34:34 UTC 2020


>
> On 9/30/20 9:46 AM, Lorenzo Bianconi wrote:

[...]

>
> Adding this side effect to ipam_get_unused_ip() makes it less clear what
> this function's purpose is. Before this change, this function would
> attempt to get the next unused IP and return it. Now, it does that but
> it can also change the logical switch port dynamic addresses.
>
> In the case where you're setting the dynamic addresses, the function
> already returns a "bad" value of 0. The caller of ipam_get_unused_ip()
> should be able to use that return value to set the dynamic addresses of
> the switch port appropriately.
>
> I think update_dynamic_addresses() is the only caller of
> ipam_get_unused_ip(). update_dynamic_addresses() already is attempting
> to set dynamic_addresses on the switch port. It probably just needs to
> be adjusted to set the dynamic_addresses NULL in the case where
> update_dynamic_addresses() does not get a valid IPv4 or IPv6 address to use.
>

Hi Mark,

thx for the review. Ack, I will fix it in v2.

Regards,
Lorenzo

> > +
> > +        return 0;
> > +    }
> > +
> > +    return od->ipam_info.start_ipv4 + new_ip_index;
> > +}
> > +
> >   static enum dynamic_update_type
> >   dynamic_mac_changed(const char *lsp_addresses,
> >                       struct dynamic_address_update *update)
> > @@ -1758,7 +1766,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,
> > @@ -1964,7 +1972,7 @@ update_dynamic_addresses(struct dynamic_address_update *update)
> >           ip4 = update->static_ip;
> >           break;
> >       case DYNAMIC:
> > -        ip4 = htonl(ipam_get_unused_ip(update->od));
> > +        ip4 = htonl(ipam_get_unused_ip(update));
> >           VLOG_INFO("Assigned dynamic IPv4 address '"IP_FMT"' to port '%s'",
> >                     IP_ARGS(ip4), update->op->nbsp->name);
> >       }
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 7769b69ed..e27a74081 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -7473,6 +7473,16 @@ 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 1 200); do
> > +    ovn-nbctl lsp-add sw12 sw12-p$i -- lsp-set-addresses sw12-p$i "00:00:00:00:00:$i dynamic"
> > +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])
> >
> >
>



More information about the dev mailing list