On 06/10/14 at 07:03pm, Avinash wrote:
> Kernel recognizes and accepts ethertype of provider VLANs (ETH_P_8021AD)
> along with normal VLAN (ETH_P_8021Q). Also in flow key formation,
> only the outermost VLAN is considered. Remaining stacked VLANs are skipped.
> 
> Signed-off-by: Avinash <avinashand...@gmail.com>

Something seems to have corrupted this patch as tabs have been
replaced with spaces.

> 
> +static int parse_remaining_vlans(struct sk_buff *skb)
> +{
> +    struct qtag_prefix {
> +        __be16 eth_type;
> +        __be16 tci;
> +    };
> +
> +    struct qtag_prefix *qp;
> +    if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
> +        return 0;
> +
> +    if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
> +                                sizeof(__be16))))
> +        return -ENOMEM;
> +
> +    qp = (struct qtag_prefix *) skb->data;
> +    while ((qp->eth_type == htons(ETH_P_8021Q) ||
> +            qp->eth_type == htons(ETH_P_8021AD)))

Seeing all of these scattered around, I wonder whether it makes sense
to have something like:

static inline bool
is_vlan_ether_type(const __be16 type)
{
        return type == htons(ETH_P_8021Q) || type == htons(ETH_P_8021AD);
}


> +    {
> +        __skb_pull(skb, sizeof(struct qtag_prefix));
> +        qp = (struct qtag_prefix *) skb->data;
> +    }
> +
> +    return 0;
> +}
> +
>  static __be16 parse_ethertype(struct sk_buff *skb)
>  {
>         struct llc_snap_hdr {
> @@ -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?

> 
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to