On Thu, Jul 10, 2014 at 9:32 AM, Daniele Di Proietto <ddiproie...@vmware.com> wrote: > On Jul 10, 2014, at 2:40 AM, Thomas Graf <tg...@noironetworks.com> wrote: > >> On 07/09/14 at 03:22pm, Daniele Di Proietto wrote: >>> match_validate() enforce that a mask matching on NDP attributes has also an >>> exact match on ICMPv6 type. >>> The ICMPv6 type, which is 8-bit wide, is stored in the 'tp.src' field of >>> 'struct sw_flow_key', which is 16-bit wide. >>> Therefore, an exact match on ICMPv6 type should only check the first 8 bits. >>> >>> This commit fixes a bug that prevented flows with an exact match on NDP >>> field >>> from being installed >>> >>> Signed-off-by: Daniele Di Proietto <ddiproie...@vmware.com> >> >> Using 0xff is obviously correct but wouldn't this break ABI for users with >> existing 0xffff masks? How about allowing both? > > It shouldn’t be possible at all for the mask to be 0xffff. > > In the ICMPv6 case, we create the mask from the netlink attribute > with this line: > (I’m assuming this is the only place where we fill a sw_flow_key mask, is > that right?) > > datapath/flow_netlink.c:786 > SW_FLOW_KEY_PUT(match, tp.src, > htons(icmpv6_key->icmpv6_type), is_mask); > > This cannot set tp.src to htons(0xffff), because icmpv6_key->icmpv6_type is > __u8. > > The bug has always been there, since a1c564be (datapath: Mega flow > implementation). > > datapath/flow.c:201 > > if (match->mask && (match->mask->key.ipv6.tp.src == htons(0xffff))) > > datapath/flow.c:1437 > > SW_FLOW_KEY_PUT(match, ipv6.tp.src, > htons(icmpv6_key->icmpv6_type), is_mask); > > Does this address your concern?
Thanks. I pushed patch to master and branch-2.0 to 2.3. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev