[ovs-dev] [VLAN/SNAP 1/3] datapath: Fix handling of 802.1Q and SNAP headers.

Jesse Gross jesse at nicira.com
Fri Aug 6 22:44:01 UTC 2010


On Wed, Jul 28, 2010 at 5:01 PM, Ben Pfaff <blp at nicira.com> wrote:

> -static int is_snap(const struct eth_snap_hdr *esh)
> +       struct qtag_prefix {
> +               __be16 eth_type; /* ETH_P_8021Q */
> +               __be16 tci;
> +       };
>

It surprises me a little bit that we have to define this ourselves.  I
realize that it's looking at a different segment of the header than 'struct
vlan_hdr' but still surprising/annoying to me.


> +           llc->oui[0] != 0 || llc->oui[1] != 0 || llc->oui[2] != 0)
> +               return htons(ODP_DL_TYPE_NOT_ETH_TYPE);
>

I would be tempted to write this as ((llc->oui[0] | llc->oui[1]
| llc->oui[2]) != 0).


> diff --git a/lib/flow.c b/lib/flow.c
> index 490c46b..0605a66 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> +static void
> +parse_vlan(struct ofpbuf *b, flow_t *flow)
>  {
> -    return ofpbuf_try_pull(packet, ETH_HEADER_LEN);
> +       struct qtag_prefix {
> +               uint16_t eth_type;      /* ETH_TYPE_VLAN */
> +               uint16_t tci;
> +       };
>

Looks like some tabs crept in here.

Otherwise looks good.  Thanks for figuring this out and convincing me that
that is actually the correct behavior...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100806/1e8cf2bc/attachment-0003.html>


More information about the dev mailing list