[ovs-dev] please help review patch "allow DHCPv6 respond multiple IA Address Options"

[Zong Kai Li]

On Thu, Sep 1, 2016 at 6:05 PM, Numan Siddique <nusiddiq at redhat.com> wrote:
>
>
> On Wed, Aug 31, 2016 at 8:44 AM, Zong Kai Li <zealokii at gmail.com> wrote:
>>
>> On Tue, Aug 30, 2016 at 8:28 PM, Numan Siddique <nusiddiq at redhat.com>
>> wrote:
>> >
>> >
>> > On Tue, Aug 30, 2016 at 9:40 AM, Ben Pfaff <blp at ovn.org> wrote:
>> >>
>> >> The patch will get reviewed, there's no need to ask again.
>> >>
>> >> On Tue, Aug 30, 2016 at 11:09:23AM +0800, Zong Kai Li wrote:
>> >> > Hi, Numan and Justin.
>> >> > I wish you can help review patch "allow DHCPv6 respond multiple IA
>> >> > Address Options".
>> >> > http://patchwork.ozlabs.org/patch/663493/
>> >> >
>> >> > For current DHCPv6 implementation, it will respond one IPv6 address
>> >> > for a lswitch port in a DHCPv6 Reply message. The "one IPv6 address",
>> >> > I mean, one in the same address string, like "MAC IPv4-Address
>> >> > IPv6-Address-1 IPv6-Address-2 ...".
>> >> >
>> >> > I don't think it's a good solution, since we can set multiple IA
>> >> > Address Options in a DHCPv6 Reply message, and by that, we can
>> >> > support
>> >> > multiple IPv6 addresses in the same address string for VIF port to
>> >> > configure. It will give us a lflow with action=(reg0[3] =
>> >> > put_dhcpv6_opts(ia_addr = OFFER_IP1, ia_addr = OFFER_IP2, ... ,
>> >> > server_id = MAC, OTHER_OPTS...); next;). This can work.
>> >> >
>> >> > In the patch mentioned above, I kept the CIDR checking, to make sure
>> >> > at least one IPv6 address owned by lswitch port can reach DNS server
>> >> > list in put_dhcpv6_opts action.
>> >> >
>> >
>> >
>> > I haven't reviewed the patch. But I have few comments.
>> > A logical port can have multiple IPv6 addresses, out of which some can
>> > be
>> > from a stateless subnet and some from stateful subnet. This doesn't
>> > handle
>> > it right ? Correct me if I am wrong here.
>> >
>> > May be CMS can store the multiple IA_ADDR options in the
>> > DHCP_Options.options so that ovn-northd doesn't need to bother about it.
>> >
>> > Thanks
>> > Numan
>> >
>>
>> Yes, you're right, this changing doesn't handle that stateful and
>> stateless addresses case. I didn't have a confident solution, so I
>> made this changing aims to let all addresses belong to VM/VIF port can
>> be get at first.
>>
>> I think store the multiple IA_ADDR options in the DHCP_Options.options
>> is a good solution for ovn-northd, but not to CMS, it will lead
>> DHCP_Options.options has higher maintain workload.
>
>
> I would personally prefer this approach. As it is cleaner. I think it should
> be fine for CMS to take care of such things. Else it could become openstack
> specific. My 2 cents.
>

Sounds fair enough, ok, I will try to check multiple IA_ADDR options
in the DHCP_Options.options.
Thanks,
Zongkai, LI