On Tue, Nov 1, 2016 at 11:06 AM, Joe Stringer <j...@ovn.org> wrote:
> On 1 November 2016 at 10:48, Pravin Shelar <pshe...@ovn.org> wrote:
>> On Tue, Nov 1, 2016 at 10:30 AM, Joe Stringer <j...@ovn.org> wrote:
>>> On 31 October 2016 at 22:00, Pravin B Shelar <pshe...@ovn.org> wrote:
>>>> The compat vlan code ignores vlan tag for inner packet
>>>> on egress path. Following patch fixes this by inserting the
>>>> tag for inner packet before tunnel encapsulation.
>>>>
>>>> Signed-off-by: Pravin B Shelar <pshe...@ovn.org>
>>>
>>> Is this a problem upstream and for other tunnels too?
>>>
>> upstream does not has this issue since networking stack would handle
>> vlan tag for geneve device.
>
> Bear with me, but why does upstream vxlan do something like this but
> upstream geneve doesn't?
>
Because Geneve device does not expose VLAN offload features. Btw I
have patch to handle this discrepancy between geneve and vxlan.
>>>> ---
>>>> datapath/linux/compat/geneve.c | 26 ++++++++++++++++++++++++--
>>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/datapath/linux/compat/geneve.c
>>>> b/datapath/linux/compat/geneve.c
>>>> index 7f2b192..6cce5ca 100644
>>>> --- a/datapath/linux/compat/geneve.c
>>>> +++ b/datapath/linux/compat/geneve.c
>>>> @@ -750,11 +750,22 @@ static int geneve_build_skb(struct rtable *rt,
>>>> struct sk_buff *skb,
>>>> skb_scrub_packet(skb, xnet);
>>>>
>>>> min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
>>>> - + GENEVE_BASE_HLEN + opt_len + sizeof(struct
>>>> iphdr);
>>>> + + GENEVE_BASE_HLEN + opt_len + sizeof(struct iphdr)
>>>> + + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0);
>>>> +
>>>> err = skb_cow_head(skb, min_headroom);
>>>> if (unlikely(err))
>>>> goto free_rt;
>>>>
>>>> + if (skb_vlan_tag_present(skb)) {
>>>> + err = __vlan_insert_tag(skb, skb->vlan_proto,
>>>> + skb_vlan_tag_get(skb));
>>>
>>> Does the proto need to be set? I see that the equivalent vxlan code
>>> upstream uses vlan_hwaccel_push_inside() instead.
>>>
>> We can use vlan_hwaccel_push_inside(), but it frees the skb, thats why
>> I prefer __vlan_insert_tag(). It allows us to write simple error
>> handling code.
>
> OK, so skb->proto doesn't need to be set to the vlan proto?
OIC, you meant skb->protocol, Yes I need to set it.
__vlan_insert_tag() does not do it for us.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
