[ovs-dev] [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support

[pravin shelar]

On Wed, May 4, 2016 at 12:36 AM, Simon Horman
<simon.horman at netronome.com> wrote:
> From: Lorand Jakab <lojakab at cisco.com>
>
> Implementation of the pop_eth and push_eth actions in the kernel, and
> layer 3 flow support.
>
> This doesn't actually do anything yet as no layer 2 tunnel ports are
> supported yet. The original patch by Lorand was against the Open vSwtich
> tree which has L2 LISP tunnels but that is not supported in mainline Linux.
> I (Simon) plan to follow up with support for non-TEB GRE ports based on
> work by Thomas Morin.
>
> Cc: Thomas Morin <thomas.morin at orange.com>
> Signed-off-by: Lorand Jakab <lojakab at cisco.com>
> Signed-off-by: Simon Horman <simon.horman at netronome.com>
>
> ---

...

> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 0ea128eeeab2..6e174ea5f2bb 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -468,28 +468,31 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
>
>         skb_reset_mac_header(skb);
>
> -       /* Link layer.  We are guaranteed to have at least the 14 byte Ethernet
> -        * header in the linear data area.
> -        */
> -       eth = eth_hdr(skb);
> -       ether_addr_copy(key->eth.src, eth->h_source);
> -       ether_addr_copy(key->eth.dst, eth->h_dest);
> +       /* Link layer. */
> +       if (key->phy.is_layer3) {
> +               key->eth.tci = 0;
> +               key->eth.type = skb->protocol;
> +       } else {
> +               eth = eth_hdr(skb);
> +               ether_addr_copy(key->eth.src, eth->h_source);
> +               ether_addr_copy(key->eth.dst, eth->h_dest);
>
> -       __skb_pull(skb, 2 * ETH_ALEN);
> -       /* We are going to push all headers that we pull, so no need to
> -        * update skb->csum here.
> -        */
> +               __skb_pull(skb, 2 * ETH_ALEN);
> +               /* We are going to push all headers that we pull, so no need to
> +                * update skb->csum here.
> +                */
>
> -       key->eth.tci = 0;
> -       if (skb_vlan_tag_present(skb))
> -               key->eth.tci = htons(skb->vlan_tci);
> -       else if (eth->h_proto == htons(ETH_P_8021Q))
> -               if (unlikely(parse_vlan(skb, key)))
> -                       return -ENOMEM;
> +               key->eth.tci = 0;
> +               if (skb_vlan_tag_present(skb))
> +                       key->eth.tci = htons(skb->vlan_tci);
> +               else if (eth->h_proto == htons(ETH_P_8021Q))
> +                       if (unlikely(parse_vlan(skb, key)))
> +                               return -ENOMEM;
>
> -       key->eth.type = parse_ethertype(skb);
> -       if (unlikely(key->eth.type == htons(0)))
> -               return -ENOMEM;
> +               key->eth.type = parse_ethertype(skb);
> +               if (unlikely(key->eth.type == htons(0)))
> +                       return -ENOMEM;
> +       }
>
>         skb_reset_network_header(skb);
>         skb_reset_mac_len(skb);
> @@ -696,11 +699,23 @@ int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
>  int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
>                          struct sk_buff *skb, struct sw_flow_key *key)
>  {
> +       bool is_layer3 = false;
> +       bool is_teb = false;
is_layer3 and is_teb are mutually exclusive, so can't we use single
boolean here?


> +       int err;
> +
>         /* Extract metadata from packet. */
>         if (tun_info) {
>                 key->tun_proto = ip_tunnel_info_af(tun_info);
>                 memcpy(&key->tun_key, &tun_info->key, sizeof(key->tun_key));
>
> +               if (OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER) {
> +                       if (skb->protocol == htons(ETH_P_TEB))
> +                               is_teb = true;
> +                       else
> +                               is_layer3 = true;
> +               }
> +
On transmit side you are using mac_len to detect l3 packet, why not do
same while extracting the key?