Looks good to me, thanks!

Acked-by: Romain Lenglet <rlenglet at vmware.com>
--
Romain Lenglet

On Oct 7, 2013, at 2:33 PM, Ben Pfaff <b...@nicira.com> wrote:

> Before commit e995e3df57ea (Allow OVS_USERSPACE_ATTR_USERDATA to be
> variable length.) userdata attributes in userspace actions were expected
> to be exactly 64 bits long.  The kernel only actually enforced that they
> were at least 64 bits long (the previously referenced commit's log message
> contains misinformation on this account).
> 
> Initially this was no problem, because all of the userdata that userspace
> actually used was exactly 8 bytes long.  Commit 29089a540c (Implement IPFIX
> export), however, exposed a problem by reducing the length of userdata for
> IPFIX support to just 4 bytes.  This meant that IPFIX no longer worked on
> older datapaths, because the userdata was no longer at least 8 bytes long.
> 
> This commit fixes the problem by padding out userdata attributes less than
> 8 bytes long to 8 bytes.
> 
> CC: Romain Lenglet <rleng...@vmware.com>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/odp-util.c |   16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index d2aa748..2cffc6a 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -3284,8 +3284,20 @@ odp_put_userspace_action(uint32_t pid,
>     nl_msg_put_u32(odp_actions, OVS_USERSPACE_ATTR_PID, pid);
>     if (userdata) {
>         userdata_ofs = odp_actions->size + NLA_HDRLEN;
> -        nl_msg_put_unspec(odp_actions, OVS_USERSPACE_ATTR_USERDATA,
> -                          userdata, userdata_size);
> +
> +        /* The OVS kernel module before OVS 1.11 and the upstream Linux 
> kernel
> +         * module before Linux 3.10 required the userdata to be exactly 8 
> bytes
> +         * long:
> +         *
> +         *   - The kernel rejected shorter userdata with -ERANGE.
> +         *
> +         *   - The kernel silently dropped userdata beyond the first 8 bytes.
> +         *
> +         * Thus, for maximum compatibility, always put at least 8 bytes.  (We
> +         * separately disable features that required more than 8 bytes.) */
> +        memcpy(nl_msg_put_unspec_zero(odp_actions, 
> OVS_USERSPACE_ATTR_USERDATA,
> +                                      MAX(8, userdata_size)),
> +               userdata, userdata_size);
>     } else {
>         userdata_ofs = 0;
>     }
> -- 
> 1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to