On Wed, 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.

The source of the problem is that we send part of the flow key instead
of the whole thing, which I believe was only done as an optimization.
If this becomes a rare case rather than the common one, I think we can
just remove the optimization and things will continue to work in all
cases.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to