[ovs-dev] [PATCH v8 5/5] datapath: add layer 3 support to ovs_packet_cmd_execute()

[Lori Jakab]

On 12/10/2014 02:45 AM, Jarno Rajahalme wrote:
> 
> On Dec 9, 2014, at 11:06 AM, Lori Jakab <lojakab at cisco.com> wrote:
> 
>> On 11/21/2014 02:20 AM, Jesse Gross wrote:
>>> On Thu, Nov 20, 2014 at 4:06 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>>> On Nov 20, 2014, at 12:18 PM, Jesse Gross <jesse at nicira.com> wrote:
>>>>
>>>> On Thu, Nov 20, 2014 at 10:35 AM, Jarno Rajahalme <jrajahalme at nicira.com>
>>>> wrote:
>>>>
>>>> The netlink key for flow put is serialized from struct flow, so even in the
>>>> case of L3 the ethertype would be serialized.
>>>>
>>>> Even the kernel datapath is encoding the ethertype in key_extract(), even
>>>> when it is not in the packet:
>>>>
>>>> + /* Link layer. */
>>>> + if (key->phy.is_layer3) {
>>>> + key->eth.tci = 0;
>>>> + key->eth.type = skb->protocol;
>>>> + } else {
>>>>
>>>> So I guess you are not saying that the ethertype should not be serialized,
>>>> due to it not being in the L3 packet?
>>>>
>>>>
>>>> The EtherType is stored in the flow key for efficiency but it is
>>>> actually not part of the serialized netlink representation currently.
>>>>
>>>>
>>>> OK, I see patch 2/5 skipping past the line where dl_type is serialized in
>>>> odp-util.c.
>>>>
>>>> This revision of the series introduces a patch to change the
>>>> serialization from userspace for the purposes of packet execution but
>>>> it is not done from the kernel or at all in previous revisions. I
>>>> think it is important that userspace, kernel, and the data contained
>>>> in the packet line up as closely as possible.
>>>>
>>>>
>>>> So by encoding the whole key the datapath can detect the presence of
>>>> OVS_KEY_ATTR_IPV4 or OVS_KEY_ATTR_IPV6 and use those to set the
>>>> skb->protocol?
>>>
>>> Yes, that's right.
>>
>> Has there been any progress on this?
> 
> I discussed this offline with Jesse and we came up with a way forward. For what I understand this might seem like backtracking from some earlier discussions, sorry for that.
> 
> As packets may be partially executed in userspace, it is possible that the original flow key does not fully represent the packet being sent to the kernel for execution. This makes it somewhat tricky to use the packet key attributes for deducing the packet type. A new metadata field that can be modified together with the packet as it is being executed would be better.
> 
> We should add a new metadata key field OVS_KEY_ATTR_PACKET_ETHERTYPE, that contains the ethertype of the associated packet attribute. While this would be strictly needed only for L3 packets, it would be cleaner to include it with all packets in packet misses. Then it could be used in flow setup and packet execution as well.
> 
> Other packet type namespaces (like IP protocol) could be later supported defining new netlink attributes.
> 
> A corresponding new field packet_ethertype needs to be added in struct pkt_metadata. This needs to be kept upto date in userspace code pushing and popping headers, so that the correct value gets passed to kernel execution.

Sounds good, thanks for the clarification and pointers.  I'll update my
patches accordingly.

-Lori