[ovs-dev] [PATCH] odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format.

[Justin Pettit]

That's quite a packet.  The change looks reasonable to me--and 200 bytes seems adequate to cover that packet.  However, it might be worth having Jesse double-check that that's the correct encapsulation in the example

Do you think it's worth having the total be a #define, and then do a build-time assertion that the total value is less than ODPUTIL_FLOW_KEY_BYTES?

--Justin


On May 15, 2012, at 9:23 AM, Ben Pfaff wrote:

> Before we submitted the kernel module upstream, we updated the flow format
> by adding two fields to the description of packets with VLAN headers, but
> we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes.  The
> result was that a maximum-length flow did not fit in the given space.
> 
> This fixes a crash processing IPv6 neighbor discovery packets with VLAN
> headers received in a tunnel configured with key=flow or in_key=flow.
> 
> Reported-by: Dan Wendlandt <dan at nicira.com>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/odp-util.h |   13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 7c9b588..7e342cf 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -74,15 +74,20 @@ int odp_actions_from_string(const char *, const struct shash *port_names,
>  *  OVS_KEY_ATTR_TUN_ID        8    --     4     12
>  *  OVS_KEY_ATTR_IN_PORT       4    --     4      8
>  *  OVS_KEY_ATTR_ETHERNET     12    --     4     16
> + *  OVS_KEY_ATTR_ETHERTYPE     2     2     4      8  (outer VLAN ethertype)
>  *  OVS_KEY_ATTR_8021Q         4    --     4      8
> - *  OVS_KEY_ATTR_ETHERTYPE     2     2     4      8
> + *  OVS_KEY_ATTR_ENCAP         0    --     4      4  (VLAN encapsulation)
> + *  OVS_KEY_ATTR_ETHERTYPE     2     2     4      8  (inner VLAN ethertype)
>  *  OVS_KEY_ATTR_IPV6         40    --     4     44
>  *  OVS_KEY_ATTR_ICMPV6        2     2     4      8
>  *  OVS_KEY_ATTR_ND           28    --     4     32
>  *  -------------------------------------------------
> - *  total                                       144
> + *  total                                       156
> + *
> + * We include some slack space in case the calculation isn't quite right or we
> + * add another field and forget to adjust this value.
>  */
> -#define ODPUTIL_FLOW_KEY_BYTES 144
> +#define ODPUTIL_FLOW_KEY_BYTES 200
> 
> /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow
>  * key.  An array of "struct nlattr" might not, in theory, be sufficiently
> -- 
> 1.7.2.5
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev