On Thu, Jan 17, 2019 at 11:14:26AM +0000, Vishal Deep Ajmera wrote:
> Currently OVS supports all ARP protocol fields as OXM match fields to
> implement the relevant ARP procedures for IPv4. This includes support
> for matching copying and setting ARP fields. In IPv6 ARP has been
> replaced by ICMPv6 neighbor discovery (ND) procedures, neighbor
> advertisement and neighbor solicitation.
> 
> The support for ICMPv6 fields in OVS is not complete for the use cases
> equivalent to ARP in IPv4. OVS lacks support for matching, copying and
> setting the “ND option type” and “ND reserved” fields. Without these user
> cannot implement all ICMPv6 ND procedures for IPv6 support.
> 
> This commit adds additional OXM fields to OVS for ICMPv6 “ND option type“
> and ICMPv6 “ND reserved” using the OXM extension mechanism. This allows
> support for parsing these fields from an ICMPv6 packet header and extending
> the OpenFlow protocol with specifications for these new OXM fields for
> matching, copying and setting.
> 
> Signed-off-by: Vishal Deep Ajmera <[email protected]>
> Co-authored-by: Ashvin Lakshmikantha <[email protected]>
> Signed-off-by: Ashvin Lakshmikantha <[email protected]>

Thanks for working to make OVS better!

It looks like miniflow_extract() calls data_pull() for the RSO flags
field without first checking to see whether the message is long enough.
This is dangerous.

This cast should not be needed:

                rso_flags = (uint32_t *) data_pull(&data,
                                               &size, sizeof(uint32_t));

The code for populating opt_type[0] and opt_type[1] into the miniflow is
confusing.  It looks like only one of these can be nonzero, and if
either one is present then we put it in the same spot (in the
tcp_flags)?  If that's so, then why bother distinguishing them during
parsing?  (And how should flow_compose_l4() know where to put them?
Currently it doesn't bother with them at all.)

The comments on struct flow should describe the new uses of tcp_flags
and igmp_group_ip4.

Please add an item to describe the new feature in NEWS.

Please add parsing tests to odp.at.

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to