On Nov 19, 2014, at 12:51 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:

> 
> On Nov 17, 2014, at 2:33 PM, Jesse Gross <je...@nicira.com> wrote:
> 
>> On Mon, Nov 17, 2014 at 1:50 PM, Lori Jakab <loja...@cisco.com> wrote:
>>> On 11/17/14 8:03 PM, Jesse Gross wrote:
>>>> On Mon, Nov 17, 2014 at 9:24 AM, Lorand Jakab <loja...@cisco.com> wrote:
>>>>> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
>>>>> index 54510c8..8ca3469 100644
>>>>> --- a/datapath/flow_netlink.c
>>>>> +++ b/datapath/flow_netlink.c
>>>>> @@ -692,6 +692,18 @@ static int metadata_from_nlattrs(struct
>>>>> sw_flow_match *match,  u64 *attrs,
>>>>>                else
>>>>>                        SW_FLOW_KEY_PUT(match, phy.is_layer3, true,
>>>>> is_mask);
>>>>>        }
>>>>> +       /* Layer 3 packets from user space have the EtherType as metadata
>>>>> */
>>>>> +       if (*attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) {
>>>> 
>>>> Is this correct? I thought that EtherType wasn't serialized in this
>>>> case - in other places we extract this from the IP/IPv6 attribute
>>>> directly.
>>> 
>>> 
>>> For "ovs_flow" Netlink messages we can do that, but not for "ovs_packet"
>>> messages, which only have packet metadata, not the full flow key.  Packet
>>> metadata didn't include EtherType until now, but I unless we use the nibble
>>> from the IP version, we need to add it for layer 3 packets only.
>> 
>> Hmm, I see. I think that diverging the Netlink encoding for flow
>> installation vs. metadata is probably not a good idea over the long
>> term. If I remember correctly, I believe the reason for only encoding
>> part of the flow key for metadata was to reduce serialization cost.
>> Jarno is thinking about combining the packet execution with flow
>> installation, which would might make this a moot point. If that's the
>> case, then we can probably solve this issue by just using the full
>> flow key.
> 
> I have a patch that adds an optional packet to the flow installation command. 
> However, we need to keep the existing packet execution command for backwards 
> compatibility. Also, there are cases (like exceeded flow limit), where we 
> will not add new flows, but just execute the miss packets.  This means that 
> this problem needs to be solved independently of packet execution.
> 

Meant to write: “… independently of combined packet execution and flow 
installation.”

  Jarno

>  Jarno
> 

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to