On Mon, Nov 28, 2016 at 6:41 PM, Jarno Rajahalme <ja...@ovn.org> wrote:
> Do not set skb->protocol to be the ethertype of the L3 header, unless
> the packet only has the L3 header. For a non-hardware offloaded VLAN
> frame skb->protocol needs to be one of the VLAN ethertypes.
>
> Any VLAN offloading is undone on the OVS netlink interface. Also any
> VLAN tags added by userspace are non-offloaded.
>
> Incorrect skb->protocol value on a full-size non-offloaded VLAN skb
> causes packet drop due to failing MTU check, as the VLAN header should
> not be counted in when considering MTU in ovs_vport_send().
>
I think we should move to is_skb_forwardable() type of packet length
check in vport-send and get rid of skb-protocol checks altogether.
> Fixes: 5108bbaddc ("openvswitch: add processing of L3 packets")
> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
> ---
> v2: Set skb->protocol when an ETH_P_TEB frame is received via ARPHRD_NONE
> interface.
>
> net/openvswitch/datapath.c | 1 -
> net/openvswitch/flow.c | 30 ++++++++++++++++++++++--------
> 2 files changed, 22 insertions(+), 9 deletions(-)
...
...
> @@ -531,15 +538,22 @@ static int key_extract(struct sk_buff *skb, struct
> sw_flow_key *key)
> if (unlikely(parse_vlan(skb, key)))
> return -ENOMEM;
>
> - skb->protocol = parse_ethertype(skb);
> - if (unlikely(skb->protocol == htons(0)))
> + key->eth.type = parse_ethertype(skb);
> + if (unlikely(key->eth.type == htons(0)))
> return -ENOMEM;
>
> + if (skb->protocol == htons(ETH_P_TEB)) {
> + if (key->eth.vlan.tci & htons(VLAN_TAG_PRESENT)
> + && !skb_vlan_tag_present(skb))
> + skb->protocol = key->eth.vlan.tpid;
> + else
> + skb->protocol = key->eth.type;
> + }
> +
I am not sure if this work in case of nested vlans.
Can we move skb-protocol assignment to parse_vlan() to avoid checking
for non-accelerated vlan case again here?
