On 6/11/14, Jesse Gross <je...@nicira.com> wrote:
> On Wed, Jun 11, 2014 at 7:26 AM, Avinash <avinashand...@gmail.com> wrote:
>> On 6/10/14, Thomas Graf <tg...@suug.ch> wrote:
>>> On 06/10/14 at 07:03pm, Avinash wrote:
>>>> @@ -471,10 +497,14 @@ int ovs_flow_extract(struct sk_buff *skb, u16
>>>> in_port, struct sw_flow_key *key)
>>>>
>>>>         if (vlan_tx_tag_present(skb))
>>>>                 key->eth.tci = htons(vlan_get_tci(skb));
>>>> -       else if (eth->h_proto == htons(ETH_P_8021Q))
>>>> +        else if (eth->h_proto == htons(ETH_P_8021Q) ||
>>>> +                 eth->h_proto == htons(ETH_P_8021AD))
>>>>                 if (unlikely(parse_vlan(skb, key)))
>>>>                         return -ENOMEM;
>>>>
>>>> +        if (unlikely(parse_remaining_vlans(skb)))
>>>> +                return -ENOMEM;
>>>
>>> How about adding the functionality of parse_remaining_vlans() to
>>> parse_vlan() directly instead of checking every single packet?
>>>
>>
>> Combining parse_remaining_vlans() with the parse_vlan() might not skip
>> the inner VLANs present in the packet when the execution enters the
>> condition vlan_tx_tag_present().
>
> You shouldn't blindly skip vlan tags anyways. This patch also
> misreports EtherTypes in Netlink since it doesn't actually store them
> in the flow. You need to find a cleaner approach that will work for
> all situations and be possible to extend in the future.
>

Storing the inner vlans in the flow might not be useful as these are not
processed to the user space. Userspace makes decision considering only
the outer vlan. Also for supporting stacked VLANs (2 or more vlans), storing
may requires an array of vlans.

Do you see any issue storing only the outer VLAN in the flow ?

At times, there is some mismatch in the EtherType. Even if the userspace
send 802.1AD EtherType (0x88A8), in the tcpdump it is shown as (0x8100).
We even verified skb->protocol before sending from vport, where it is
displayed as 0x88A8 but while the EtherType is misreported in the
outgoing packet.
We are debugging this issue.

Approach followed in kernel space:

1. Formation of flow key from packet.
   Recognize 802.1AD along with 802.1Q and skip remaining VLANs.
2. (If no action is present) Query userspace for flow key action.
3. Follow the action.

Provider VLANs are supported by the existing push_vlan and pop_vlan functions.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to