On Wed, Apr 16, 2014 at 2:32 PM, Andy Zhou <az...@nicira.com> wrote:
> On Wed, Apr 16, 2014 at 2:25 PM, Jesse Gross <je...@nicira.com> wrote:
>> On Wed, Apr 16, 2014 at 11:13 AM, Andy Zhou <az...@nicira.com> wrote:
>>> On Wed, Apr 16, 2014 at 11:03 AM, Pravin Shelar <pshe...@nicira.com> wrote:
>>>> On Wed, Apr 16, 2014 at 6:51 AM, Andy Zhou <az...@nicira.com> wrote:
>>>>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>>>>> index 0a1effa..883d9bf 100644
>>>>> --- a/datapath/flow_netlink.c
>>>>> +++ b/datapath/flow_netlink.c
>>>>> @@ -474,6 +476,23 @@ static int metadata_from_nlattrs(struct
>>>>> sw_flow_match *match, u64 *attrs,
>>>>> *attrs &= ~(1ULL << OVS_KEY_ATTR_DP_HASH);
>>>>> }
>>>>>
>>>>> + if (*attrs & (1ULL << OVS_KEY_ATTR_RECIRC_ID)) {
>>>>> + u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]);
>>>>> +
>>>>> + if (is_mask && (recirc_id > 0 && recirc_id < UINT_MAX)) {
>>>>> + OVS_NLERR("Reicrc_id mask is neither wildcard,
>>>>> nor exact match \n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask);
>>>>> + *attrs &= ~(1ULL << OVS_KEY_ATTR_RECIRC_ID);
>>>>> + }
>>>>> +
>>>>> + if (is_mask) {
>>>>> + /* Always exact match recirc_id. */
>>>>> + SW_FLOW_KEY_PUT(match, recirc_id, UINT_MAX, is_mask);
>>>>> + }
>>>>> +
>>>>
>>>> If the recirc_id is zero it should be safe to ignore it. Can you
>>>> explain need for exact match for recirc_id ?
>>>
>>> Consider the following two flows:
>>>
>>> 1) in_port = 1 ; actions= A
>>> 2) recirc_id = 2, in_port =1 : Actions=B
>>>
>>> Without forcing exact match of recirc_id, a packet matches 2) could
>>> also match 1)
>>
>> Presumably anybody that is using recirculation would already make this
>> exact match already, right? Is there any possible use case for a
>> partial mask?
>>
> Partial mask is not allowed for recirc_id. Either exact match or wildcard.
> I am interpreting a missing attribute to be exact match 0. I think Pravin
> is arguing for interpreting it as a wildcard. What do you think?
If we don't think there is ever a case for partial masks then it's not
clear why it's OK to have something fully masked out. Presumably
somebody that is wildcarding the recirculation ID isn't doing
recirculation, in which case matching on ID 0 is fine. However, if
that's the case then I think that we can enforce any masks explicitly
supplied for recirc_id are exact match (rather than only disallowing
partials).
The thing that bothers me about this is that it's introducing a
somewhat special case for something that the kernel doesn't really
care about: there's no inherent reason why we can do a partial match
on the recirc_id, it's just that we can't think of a good reason now.
As we saw with the introduction of megaflows, special cases can come
back to bite us later on.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
