[ovs-dev] [PATCH] Add Geneve support on Windows datapath.

[Jesse Gross]

On Tue, May 17, 2016 at 8:27 PM, Yin Lin <linyi at vmware.com> wrote:
> +static __inline NTSTATUS
> +OvsTunnelAttrToGeneveOptions(PNL_ATTR attr,
> +                             OvsIPv4TunnelKey *tunKey)
> +{
> +    UINT32 optLen = NlAttrGetSize(attr);
> +    GeneveOptionHdr *option;
> +    if (optLen > TUN_OPT_MAX_LEN) {
> +        OVS_LOG_ERROR("Geneve option length err (len %d, max %Iu).",
> +                      optLen, TUN_OPT_MAX_LEN);
> +        return STATUS_INFO_LENGTH_MISMATCH;
> +    } else if (optLen % 4 != 0) {
> +        OVS_LOG_ERROR("Geneve opt len %d is not a multiple of 4.", optLen);
> +        return STATUS_INFO_LENGTH_MISMATCH;
> +    }
> +    tunKey->tunOptLen = (UINT8)optLen;
> +    option = (GeneveOptionHdr *)TunnelKeyGetOptions(tunKey);
> +    memcpy(option, NlAttrData(attr), optLen);
> +    while(optLen > 0) {
> +        UINT32 len;
> +        if (optLen < sizeof(*option)) {
> +            return STATUS_INFO_LENGTH_MISMATCH;
> +        }
> +        len = sizeof(*option) + option->length * 4;
> +        if (len > optLen) {
> +            return STATUS_INFO_LENGTH_MISMATCH;
> +        }
> +        if (option->type & GENEVE_CRIT_OPT_TYPE) {
> +            tunKey->flags |= OVS_TNL_F_CRT_OPT;
> +        }
> +        optLen -= len;
> +    }
> +    return STATUS_SUCCESS;
> +

I think 'option' is not being advanced in this loop so you'll end up
looking at the same option over and over again.

> +/*
> + *----------------------------------------------------------------------------
> + *  OvsTunnelAttrToGeneveOptions --
> + *    Converts OVS_KEY_ATTR_TUNNEL attribute to tunKey.
> + *----------------------------------------------------------------------------
> + */
> +NTSTATUS
> +OvsTunnelAttrToIPv4TunnelKey(PNL_ATTR attr,
> +                             OvsIPv4TunnelKey *tunKey)

Looks like the function name in the comment was accidentally copied
from the previous one.