[ovs-dev] [PATCH v6 2/2] TCP flags matching support.

[Jarno Rajahalme]

On Oct 25, 2013, at 9:27 AM, Jesse Gross <jesse at nicira.com> wrote:

> On Fri, Oct 25, 2013 at 9:13 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>> 
>>> On Oct 25, 2013, at 9:06 AM, Jesse Gross <jesse at nicira.com> wrote:
>>> 
>>>> On Thu, Oct 24, 2013 at 9:01 AM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>>>> index 515a9f6..fc6f42e 100644
>>>> --- a/datapath/flow_netlink.c
>>>> +++ b/datapath/flow_netlink.c
>>>> @@ -154,8 +155,10 @@ static bool match_validate(const struct sw_flow_match *match,
>>>> 
>>>>                       if (match->key->ip.proto == IPPROTO_TCP) {
>>>>                               key_expected |= 1ULL << OVS_KEY_ATTR_TCP;
>>>> -                               if (match->mask && (match->mask->key.ip.proto == 0xff))
>>>> +                               if (match->mask && (match->mask->key.ip.proto == 0xff)) {
>>>>                                       mask_allowed |= 1ULL << OVS_KEY_ATTR_TCP;
>>>> +                                       mask_allowed |= 1ULL << OVS_KEY_ATTR_TCP_FLAGS;
>>> 
>>> One thing that I should mention about this is that it doesn't require
>>> the TCP_FLAGS key to be present if the protocol indicates that the
>>> protocol is TCP. Requiring it would be the most consistent with the
>>> rest of our netlink interface. However, I'm not sure that it's really
>>> necessary any more now that we don't require exact match on all fields
>>> with megaflows.
>> 
>> I made it that way on purpose for compatibility with older userspaces. Maybe this is worth a comment, though.
> 
> I don't think that it's necessary from a compatibility point of view
> because older userspace will just echo back the fields that they
> received. The protocol is supposed to be able to handle the addition
> of new fields like this.
> 

I see quite a many uses of odp_flow_key_from_flow() that would indicate that this not always a case. This is used, for example, when userspace wants to send a packet not originally received from the kernel.

> This will kill performance on those versions though. I thought that I
> had gone through it before and decided that this wouldn't be a problem
> but I can't remember what I was thinking now.

Can you be more specific? You mean that the allowance of omitting the TCP flags key causes performance problems, or the fact that kernel is reporting a key attribute that the userspace does not understand?

  Jarno