On Fri, Aug 4, 2017 at 4:09 PM, Numan Siddique <nusid...@redhat.com> wrote:
>
>
> On Fri, Aug 4, 2017 at 3:02 AM, Ben Pfaff <b...@ovn.org> wrote:
>
>> On Mon, Jul 31, 2017 at 06:11:35PM +0530, nusid...@redhat.com wrote:
>> > From: Numan Siddique <nusid...@redhat.com>
>> >
>> > This patch adds a new OVN action 'put_nd_ra_opts' to support native
>> > IPv6 Router Advertisement in OVN. This action can be used to respond
>> > to the IPv6 Router Solicitation requests.
>> >
>> > ovn-controller parses this action and adds a NXT_PACKET_IN2 OF flow
>> > with 'pause' flag set and the RA options stored in 'userdata' field.
>> > This action is similar to 'put_dhcp_opts' and 'put_dhcpv6_opts'.
>> >
>> > When a valid IPv6 RS packet is received by the pinctrl module of
>> > ovn-controller, it frames a new RA packet and sets the RA options
>> > from the 'userdata' field and resumes the packet storing 1 in the
>> > 1-bit result sub-field. If the packet is invalid, it resumes the
>> > packet without any modifications storing 0 in the 1-bit result
>> > sub-field.
>> >
>> > Eg. reg0[5] = put_nd_ra_opts(address_mode = "slaac", mtu = 1450,
>> > slla = 01:02:03:04:05:06, prefix =
>> aef0::/64)
>> >
>> > Note that unlike DHCPv4/v6, a new table to store the supported IPv6 ND
>> RA
>> > options is not added in SB DB since there are only 3 ND RA options.
>> >
>> > Co-authored-by: Zongkai LI <zealo...@gmail.com>
>> > Signed-off-by: Zongkai LI <zealo...@gmail.com>
>> > Signed-off-by: Numan Siddique <nusid...@redhat.com>
>> > Acked-by: Miguel Angel Ajo <majop...@redhat.com>
>>
>> Thanks for working on this.
>>
>> Looking at it, the treatment of some of the options strikes me as odd.
>> Some of the options (SLL) are actually required, and others could be
>> supplied as fixed data since there's a default value (mo_flags, mtu).
>> Only the prefixes seem to really be options in what I think of as the
>> usual sense. It looks to me like those prefixes could be supplied
>> directly in the userdata as bytes to append to the packet. It might be
>> cleaner to make these changes--then there would be less parsing of text
>> options, encoding them as binary options, decoding those binary options,
>> and then re-encoding them again in the packet.
>>
>>
> Thanks for the review Ben. I think what you are suggesting makes sense and
> simplies the code. Based on your comments, this is what I am planning to
> modify this patch.
>
> "put_nd_ra_opts" action would be like
>
> res_bit = put_nd_ra_opts(slla, mtu, mo_flags, ...)
>
> ovn-northd would include 0 or more prefixes following the mo_flags field
> depending the address mode defined by the user.
>
> Eg.
> reg0[4] = put_nd_ra_opts(01:02:03:04:05:06, 1450, 0, aef0::/64)
> reg0[4] = put_nd_ra_opts(01:02:03:04:05:06, 1450, 64, aef0::/64,
> bef0::/64)
> reg0[4] = put_nd_ra_opts(01:02:03:04:05:06, 1450, 128)
>
>
> If this makes sense, I will update the patches accordingly. Please let me
> know.
>
>
And also it is possible to store the complete ICMPv6 response data (which
includes ICMP v6 header, flags and options) in the userdata field as you
mentioned, making the job of framing the reply easier.
Numan
Thanks again
>
> Numan
>
>
> If that doesn't make sense, though, this ALIGNED_CAST in
>> pinctrl_handle_put_nd_ra_opts()looks wrong--I see no reason to believe
>> that opt_data is 32-bit aligned. Probably should use
>> get_unaligned_be32():
>> if (opt_len == sizeof(ovs_be32)) {
>> mtu = *(ALIGNED_CAST(const ovs_be32 *, opt_data));
>> }
>>
>> Thanks,
>>
>> Ben.
>>
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
