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

Reply via email to