[ovs-dev] [PATCH v2 3/3] datapath: add layer 3 flow/port support

Jesse Gross jesse at nicira.com
Tue Apr 15 23:57:59 UTC 2014


On Fri, Apr 11, 2014 at 4:30 AM, Lori Jakab <lojakab at cisco.com> wrote:
> On 4/11/14, 1:47 AM, Jesse Gross wrote:
>> On Thu, Mar 20, 2014 at 4:37 AM, Lori Jakab <lojakab at cisco.com> wrote:
>>> On 1/6/14, 7:55 PM, Jesse Gross wrote:
>>>> On Tue, Dec 24, 2013 at 9:02 AM, Lorand Jakab<lojakab at cisco.com>  wrote:
>> More importantly though, what happens if we have a pop followed by a
>> set action? I'm not sure that this will catch that.
>
>
> Right.  Fixed with the following:

Is there any case for pop, then push, then set? It's obviously not
very optimal but it doesn't seem inherently illegal. On one hand, it
would be nice to just modify the flow key as we go but on the other
hand it would probably be expensive to preserve everything. It would
be nice to have that behavior though.

>>>>> +noethernet:
>>>>>           if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE,
>>>>> output->eth.type))
>>>>>                   goto nla_put_failure;
>>>>
>>>> Does it still make sense to send the EtherType? It's not present in
>>>> the packet and I believe that the information is contained in the
>>>> attributes that we do send (i.e. IPv4 or v6 attributes).
>>>
>>>
>>> We had a discussion about this in August last year:
>>>
>>>>>>>>> One such decision is how to handle the flow key.  I set all fields
>>>>>>>>> in
>>>>>>>>> key->eth to 0, except the type, because we still need to know what
>>>>>>>>> kind
>>>>>>>>> of L3 packet do we have.  Since a lot of code is accessing
>>>>>>>>> key->eth.type, this is easier than having this information in a
>>>>>>>>> different place, although it would be more elegant to set this
>>>>>>>>> field
>>>>>>>>> to
>>>>>>>>> 0 as well.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Can we use skb->protocol for the cases where we currently access the
>>>>>>>> EtherType? Are there cases that you are particularly concerned
>>>>>>>> about?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The only case I'm concerned about is in the action validation code,
>>>>>>> particularly the validate_set() and validate_tp_port() functions,
>>>>>>> since
>>>>>>> they
>>>>>>> don't have access to the skb, and need to know the layer 3 protocol
>>>>>>> of
>>>>>>> the
>>>>>>> flow.  In validate_set() we could relax the check for eth.type for
>>>>>>> OVS_KEY_ATTR_IPV4 and OVS_KEY_ATTR_IPV6, but I'm not sure what to do
>>>>>>> about
>>>>>>> validate_tp_port().
>>>>>>>
>>>>>>> In general, I think it would be a good idea for the flow key to have
>>>>>>> a
>>>>>>> field
>>>>>>> specifying the layer 3 protocol, when an ethernet header is not
>>>>>>> present.
>>>>>>
>>>>>>
>>>>>> That makes sense to me.
>>>>>
>>>>>
>>>>> You mean that we keep eth.type as the L3 protocol field, or define a
>>>>> new
>>>>> one, separate from the eth union?
>>>>
>>>> I think it's fine to keep using eth.type as the protocol field. I
>>>> think we can probably some holes to move the is_layer3 flag into the
>>>> non-tunnel portion of the flow though.
>>>
>>>
>>> Should we revisit this discussion?
>>
>> I was just referring to the Netlink encoding here. Can we populate the
>> flow key in the kernel when we translate the flow?
>
>
> Not sure I understand the question.
>
> Going through the code, I see that omitting OVS_KEY_ATTR_ETHERTYPE currently
> means an 802.2 packet, if the mask is set to 0xffff.  Are you suggesting to
> omit OVS_KEY_ATTR_ETHERTYPE for layer 3 packets both in the flow key and the
> mask?

All I was trying to say is that is that the Netlink and struct
sw_flow_key representations don't necessarily have to be exactly the
same as long as we can translate back and forth. I'm not sure that the
previous discussion applies - this is more about Netlink and that
seemed to apply to struct sw_flow_key.

I'm starting to think that the path we went down with the megaflow
Netlink encoding is not particularly sustainable because it means that
while the keys can do something fairly reasonable, the masks must
always list all the protocols that it is not, such as Ethernet and
EtherType in this case.

I wonder if it actually makes more sense to switch over to something
similar to what Jarno is planning for the actions - a key and mask
paired together and then we can also fix some left over oddities from
the exact match days.

Jarno, Pravin, any thoughts?



More information about the dev mailing list