[ovs-dev] [PATCH 1/3] datapath-windows: add netlink message parsing APIs

Nithin Raju nithin at vmware.com
Thu Aug 21 16:27:42 UTC 2014


Ankur,
Thanks for working on this. I had the following overall comments. I have annotated the patch with specific comments as appropriate.

I didn't realize initially that the Netlink.c, Netlink.h that you are introducing have major modifications compared to the userspace netlink.c and netlink.h.

Some explanation from your side on how you went about developing the patch can be helpful. Eg. the mapping between the userspace function and kernel function can be helpful. Eg. nl_msg_next => NlMsgNext. You can make it part of the commit message or write up as a comment at the top.

Also, there are a few items missing, it would be a good idea to make a note of all these, for future reference.
- policy related APIs are missing
- put related APIs (I know they are coming later)
- CTRL related defines in netlink-protocol.h

I'll have to diff the code w.r.t to the original code to spot any typos etc, but otherwise this patch suffices for the functionality that we require for parsing the message (as you've demonstrated in patch 2/3).

thanks,
Nithin


On Aug 20, 2014, at 9:46 AM, Ankur Sharma <ankursharma at vmware.com> wrote:

> In this change we introduce Netlink.c, Netlink.h and NetlinkProto.h
> in datapath-windows. These files will provide netlink message
> data structures and parsing APIs.
> 
> Changes are on similar lines to userspace netlink code.
> 
> Change-Id: Ic225504eff3a25c0619ce3b27d8f54155d8af409
> Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
> Reported-at: https://urldefense.proofpoint.com/v1/url?u=https://github.com/openvswitch/ovs-issues/issues/18&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=KOPgGejqFVaHXnjHwJvHNOBUOGfqwacPBI7AOIAwvjw%3D%0A&s=a819d7621d250e3cb43970ba56a7c58cae0bf9119020945ac04210bd00e2ecc4
> ---
> datapath-windows/automake.mk           |   5 +-
> datapath-windows/ovsext/Netlink.c      | 339 +++++++++++++++++++++++++++++++++
> datapath-windows/ovsext/Netlink.h      | 104 ++++++++++
> datapath-windows/ovsext/NetlinkProto.h | 118 ++++++++++++
> datapath-windows/ovsext/OvsTypes.h     |   2 +
> datapath-windows/ovsext/ovsext.vcxproj |   3 +
> datapath-windows/ovsext/precomp.h      |   2 +
> 7 files changed, 572 insertions(+), 1 deletion(-)
> create mode 100644 datapath-windows/ovsext/Netlink.c
> create mode 100644 datapath-windows/ovsext/Netlink.h
> create mode 100644 datapath-windows/ovsext/NetlinkProto.h
> 
> diff --git a/datapath-windows/automake.mk b/datapath-windows/automake.mk
> index 47ef225..45348b7 100644
> --- a/datapath-windows/automake.mk
> +++ b/datapath-windows/automake.mk
> @@ -5,7 +5,10 @@ EXTRA_DIST += \
> 	datapath-windows/Package/package.VcxProj.user \
> 	datapath-windows/include/OvsDpInterfaceExt.h \
> 	datapath-windows/include/OvsNetlink.h \
> -	datapath-windows/include/OvsPub.h \
> +	datapath-windows/include/OvsPub.h
> +    datapath-windows/ovsext/Netlink.c \
> +    datapath-windows/ovsext/Netlink.h \
> +    datapath-windows/include/NetlinkProto.h \
> 	datapath-windows/misc/install.cmd \
> 	datapath-windows/misc/uninstall.cmd \
> 	datapath-windows/ovsext.sln \

Don't you need a '\' after OvsPub.h for continuity? Also, indentation seems to be off for the new files added.



> diff --git a/datapath-windows/ovsext/Netlink.c b/datapath-windows/ovsext/Netlink.c
> new file mode 100644
> index 0000000..fe8077f
> --- /dev/null
> +++ b/datapath-windows/ovsext/Netlink.c
> @@ -0,0 +1,339 @@
> +/*
> + * Copyright (c) 2014 VMware, Inc.

You might want to check with Ben on the licensing on this. Since tis is based off of netlink.c in userspace, maybe we should have both the company names. When we wrote up OvsNetlink.c, I remember Ben asking us to preserve the original license banner. Saurabh can confirm.

> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     https://urldefense.proofpoint.com/v1/url?u=http://www.apache.org/licenses/LICENSE-2.0&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=KOPgGejqFVaHXnjHwJvHNOBUOGfqwacPBI7AOIAwvjw%3D%0A&s=48250afd8c943ecd4d9558b0240c077225bd3653ac69eb7e091361f54043c4ee
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +#include "precomp.h"

Hmm, this is a little weird since precomp.h itself includes Netlink.h. We'll figure this out. This is not an issue you are introducing. For now, let's keep with this convention of including precomp.h in all *.C files since that is what we said in the 'CodingStyle' document.

> +#include "NetlinkProto.h"
> +#include "Netlink.h"
> +
> +#ifdef OVS_DBG_MOD
> +#undef OVS_DBG_MOD
> +#endif
> +#define OVS_DBG_MOD OVS_DBG_NETLINK
> +#include "OvsDebug.h"
> +

Did you want to declare the following functions as static? I understand that these are currently not used, but should they be 'static __inline'?

> +/* Netlink message accessing the payload */
> +
> +static __inline PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset)
> +{
> +    return ((char *)nlh + offset);

=> return (PVOID)((PCHAR)nlh + offset);

> +}
> +
> +/* Returns the size of netlink message */
> +static __inline UINT32 NlMsgSize(const PNL_MSG_HDR nlh)
> +{
> +    return nlh->nlmsgLen;
> +}
> +
> +/* Returns pointer to nlmsg payload */
> +static __inline char *NlMsgPayload(const PNL_MSG_HDR nlh)
> +{
> +    return ((char *)nlh + NLMSG_HDRLEN);
> +}
> +
> +/* Returns length of nlmsg payload */
> +static __inline UINT32 NlMsgPayloadLen(const PNL_MSG_HDR nlh)
> +{
> +    return nlh->nlmsgLen - NLMSG_HDRLEN;
> +}
> +
> +/* Returns pointer to nlmsg attributes */
> +static __inline PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh)
> +{
> +    return (PNL_ATTR) (NlMsgPayload(nlh) + GENL_HDRLEN + OVS_HDRLEN);
> +}
> +
> +/* Returns size of to nlmsg attributes */
> +static __inline INT NlMsgAttrLen(const PNL_MSG_HDR nlh)
> +{
> +    return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
> +}
> +
> +/* Netlink message parse */
> +
> +/* Returns next netlink message in the stream */
> +static __inline PNL_MSG_HDR NlMsgNext(const PNL_MSG_HDR nlh)
> +{
> +    return (PNL_MSG_HDR)((unsigned char *)nlh +
> +            NLMSG_ALIGN(nlh->nlmsgLen));

unsigned char * => PCHAR

> +}
> +
> +
> +/* Netlink Attr helper APIs */
> +static __inline INT
> +NlAttrIsValid(const PNL_ATTR nla, UINT32 maxlen)
> +{
> +    return (maxlen >= sizeof *nla
> +            && nla->nlaLen >= sizeof *nla
> +            && nla->nlaLen <= maxlen);
> +}
> +
> +static __inline UINT32
> +NlAttrLenPad(const PNL_ATTR nla, UINT32 maxlen)
> +{
> +    UINT32 len = NLA_ALIGN(nla->nlaLen);
> +
> +    return len <= maxlen ? len : nla->nlaLen;
> +}
> +
> +static UINT32
> +NlAttrMinLen(NL_ATTR_TYPE type)
> +{
> +    switch (type) {
> +    case NL_A_NO_ATTR: return 0;
> +    case NL_A_UNSPEC: return 0;
> +    case NL_A_U8: return 1;
> +    case NL_A_U16: return 2;
> +    case NL_A_U32: return 4;
> +    case NL_A_U64: return 8;
> +    case NL_A_STRING: return 1;
> +    case NL_A_FLAG: return 0;
> +    case NL_A_NESTED: return 0;
> +    case N_NL_ATTR_TYPES: 
> +    default: 
> +    OVS_LOG_WARN("Unsupprted attribute type: %d", type);
> +    ASSERT(0);
> +    }
> +
> +    /* To keep comiler happy */
> +    return 0;
> +}
> +
> +/* Default maximum payload size for each type of attribute. */
> +static UINT32
> +NlAttrMaxLen(NL_ATTR_TYPE type)
> +{
> +    switch (type) {
> +    case NL_A_NO_ATTR: return SIZE_MAX;
> +    case NL_A_UNSPEC: return SIZE_MAX;
> +    case NL_A_U8: return 1;
> +    case NL_A_U16: return 2;
> +    case NL_A_U32: return 4;
> +    case NL_A_U64: return 8;
> +    case NL_A_STRING: return SIZE_MAX;
> +    case NL_A_FLAG: return SIZE_MAX;
> +    case NL_A_NESTED: return SIZE_MAX;
> +    case N_NL_ATTR_TYPES: 
> +    default: 
> +    OVS_LOG_WARN("Unsupprted attribute type: %d", type);
> +    ASSERT(0);
> +    }
> +
> +    /* To keep compiler happy */
> +    return 0;
> +}
> +
> +/* Netlink attribute iteration. */
> +PNL_ATTR NlAttrNext(const PNL_ATTR nla)

Looks like there's been a slight slip up in functions after this point w.r.t coding style. Return type should be on a separate line by itself.

> +{
> +    return (PNL_ATTR)((UINT8 *)nla + NLA_ALIGN(nla->nlaLen));
> +}
> +
> + /* Returns the bits of 'nla->nlaType' that are significant for determining
> +  * its type. */
> +UINT16 NlAttrType(const PNL_ATTR nla)
> +{
> +   return nla->nlaType & NLA_TYPE_MASK;
> +}
> +
> +PVOID NlAttrData(const PNL_ATTR nla)
> +{
> +    return ((char *)nla + NLA_HDRLEN);
> +}
> +
> +/* Returns the number of bytes in the payload of attribute 'nla'. */
> +UINT32 NlAttrGetSize(const PNL_ATTR nla)
> +{
> +    return nla->nlaLen - NLA_HDRLEN;
> +}
> +
> +/* Returns the first byte in the payload of attribute 'nla'. */
> +const PVOID NlAttrGet(const PNL_ATTR nla)
> +{
> +    ASSERT(nla->nlaLen >= NLA_HDRLEN);
> +    return nla + 1;
> +}
> +
> +/* Asserts that 'nla''s payload is at least 'size' bytes long, and returns the
> + * first byte of the payload. */
> +const PVOID NlAttrGetUnspec(const PNL_ATTR nla, UINT32 size)
> +{
> +    UNREFERENCED_PARAMETER(size);
> +    ASSERT(nla->nlaLen >= NLA_HDRLEN + size);
> +    return nla + 1;
> +}
> +
> +/* Returns the 64-bit network byte order value in 'nla''s payload.
> + *
> + * Asserts that 'nla''s payload is at least 8 bytes long. */
> +UINT64 NlAttrGetBe64(const PNL_ATTR nla)
> +{   
> +    return NL_ATTR_GET_AS(nla, UINT64);
> +}
> +
> +/* Returns the 32-bit network byte order value in 'nla''s payload.
> + *
> + * Asserts that 'nla''s payload is at least 4 bytes long. */
> +UINT32 NlAttrGetBe32(const PNL_ATTR nla)

BE32 indicates that the contents of the variable are in big endian. Maybe we should define a type called BE32, which obviously typedefs to UINT32.

> +{   
> +    return NL_ATTR_GET_AS(nla, UINT32);
> +}
> +
> +/* Returns the 8-bit value in 'nla''s payload. */
> +UINT8 NlAttrGetU8(const PNL_ATTR nla)
> +{   
> +    return NL_ATTR_GET_AS(nla, UINT8);
> +}
> +
> +/* Returns the 32-bit host byte order value in 'nla''s payload.
> + *
> + * Asserts that 'nla''s payload is at least 4 bytes long. */
> +UINT32 NlAttrGetU32(const PNL_ATTR nla)
> +{   
> +    return NL_ATTR_GET_AS(nla, UINT32);
> +}
> +
> +
> +/* Validate the netlink attribute against the policy */
> +BOOLEAN NlAttrValidate(const PNL_ATTR nla, const PNL_POLICY policy)
> +{
> +    UINT32 minLen;
> +    UINT32 maxLen;
> +    UINT32 len;
> +    BOOLEAN ret = FALSE;
> +
> +    if (policy->type == NL_A_NO_ATTR) {
> +        ret = TRUE;
> +        goto done;
> +    }
> +
> +    /* Figure out min and max length. */
> +    minLen = policy->minLen;
> +    if (!minLen) {
> +        minLen = NlAttrMinLen(policy->type);
> +    }
> +    maxLen = policy->maxLen;
> +    if (!maxLen) {
> +        maxLen = NlAttrMaxLen(policy->type);
> +    }
> +
> +    /* Verify length. */
> +    len = NlAttrGetSize(nla);
> +    if (len < minLen || len > maxLen) {
> +        OVS_LOG_WARN("Attribute: %p, len: %d, not in valid range, "
> +                     "min: %d, max: %d", nla, len, minLen, maxLen);
> +        goto done;
> +    }
> +
> +    /* Strings must be null terminated and must not have embedded nulls. */
> +    if (policy->type == NL_A_STRING) {
> +        if (((char *) nla)[nla->nlaLen - 1]) {
> +            OVS_LOG_WARN("Attributes %p lacks null at the end", nla);
> +            goto done;
> +        }
> +
> +        if (memchr(nla + 1, '\0', len - 1) != NULL) {
> +            OVS_LOG_WARN("Attributes %p has bad length", nla);
> +            goto done;
> +        }
> +    }
> +
> +done:
> +    return ret;
> +}
> +
> +static __inline const PNL_ATTR 
> +NlAttrFind__(const PNL_ATTR attrs, UINT32 size, UINT16 type)

Cool, return type is on a new line from here on.

> +{
> +    PNL_ATTR iter = NULL;
> +    PNL_ATTR ret = NULL;
> +    UINT32 left;
> +
> +    NL_ATTR_FOR_EACH (iter, left, attrs, size) {
> +        if (NlAttrType(iter) == type) {
> +            ret = iter;
> +            goto done;
> +        }
> +    }
> +
> +done:
> +    return ret;
> +}
> +
> +/* Returns the first Netlink attribute within 'nla' with the specified
> + * 'type'.
> + *
> + * This function does not validate the attribute's length. */
> +const PNL_ATTR 
> +NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
> +{   
> +    return NlAttrFind__((const PNL_ATTR)(NlAttrGet(nla)), 
> +                         NlAttrGetSize(nla), type);
> +}
> +
> +BOOLEAN NetlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
> +                      const NL_POLICY policy[], 
> +                      PNL_ATTR attrs[], UINT32 n_attrs)


This seems to be a new function that you've added based on parse_flow_nlattrs(). This is fine. Pls. add a comment about the functionality, and what a caller can expect.

minor: return type should be on a separate line.

> +{
> +    PNL_ATTR nla;
> +    UINT32 left;
> +    UINT32 iter;
> +    BOOLEAN ret = FALSE;
> +
> +    memset(attrs, 0, n_attrs * sizeof *attrs);
> +
> +    if ((NlMsgSize(nlMsg) < attrOffset) || (!(NlMsgAttrLen(nlMsg)))) {
> +        OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d", 
> +                     nlMsg, attrOffset);
> +        goto done;
> +    }
> +
> +    NL_ATTR_FOR_EACH (nla, left, NlMsgAt(nlMsg, attrOffset),
> +                      NlMsgSize(nlMsg) - attrOffset)
> +    {
> +        UINT16 type = NlAttrType(nla); 
> +        if (type < n_attrs && policy[type].type != NL_A_NO_ATTR) {
> +            /* Typecasting to keep the compiler happy */
> +            const PNL_POLICY e = (const PNL_POLICY)(&policy[type]);
> +            if (!NlAttrValidate(nla, e)) {
> +                goto done;
> +            }
> +             
> +            if (attrs[type]) {       
> +                OVS_LOG_WARN("Duplicate attribute in nlMsg: %p, "
> +                             "type: %u", nlMsg, type); 
> +            }
> +
> +            attrs[type] = nla;
> +        }
> +    }
> +
> +    if (left) {
> +        OVS_LOG_ERROR("Attributes followed by garbage");
> +        goto done;
> +    }
> +
> +    for (iter = 0; iter < n_attrs; iter++) {
> +        const PNL_POLICY e = (const PNL_POLICY)(&policy[iter]);
> +        if (e->type != NL_A_NO_ATTR && !attrs[iter]) {
> +            OVS_LOG_ERROR("Required attr:%d missing", iter);
> +            goto done;
> +        }
> +    }
> +
> +    ret = TRUE;
> +
> +done:
> +    return ret;
> +}



> diff --git a/datapath-windows/ovsext/Netlink.h b/datapath-windows/ovsext/Netlink.h
> new file mode 100644
> index 0000000..c72d003
> --- /dev/null
> +++ b/datapath-windows/ovsext/Netlink.h
> @@ -0,0 +1,104 @@
> +/*
> + * Copyright (c) 2008, 2009, 2010, 2011, 2013, 2014 Nicira, Inc.

OK, here you say Nicira compared to saying VMware in Netlink.c. I guess there should be a consistent banner.

> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     https://urldefense.proofpoint.com/v1/url?u=http://www.apache.org/licenses/LICENSE-2.0&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=KOPgGejqFVaHXnjHwJvHNOBUOGfqwacPBI7AOIAwvjw%3D%0A&s=48250afd8c943ecd4d9558b0240c077225bd3653ac69eb7e091361f54043c4ee
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef __NETLINK_H_
> +#define __NETLINK_H_ 1
> +
> +#include "OvsTypes.h"
> +#include "NetlinkProto.h"
> +
> +/* Netlink attribute types. */
> +typedef enum
> +{
> +    NL_A_NO_ATTR = 0,
> +    NL_A_UNSPEC,
> +    NL_A_U8,
> +    NL_A_U16,
> +    NL_A_BE16 = NL_A_U16,
> +    NL_A_U32,
> +    NL_A_BE32 = NL_A_U32,
> +    NL_A_U64,
> +    NL_A_BE64 = NL_A_U64,
> +    NL_A_STRING,
> +    NL_A_FLAG,
> +    NL_A_NESTED,
> +    N_NL_ATTR_TYPES
> +} NL_ATTR_TYPE;
> +
> +/* Netlink attribute policy.
> + * Specifies the policy for parsing for netlink attribute. */
> +typedef struct _NL_POLICY
> +{
> +    NL_ATTR_TYPE type;
> +    UINT32 minLen;
> +    UINT32 maxLen;
> +} NL_POLICY, *PNL_POLICY;
> +
> +/* This macro is careful to check for attributes with bad lengths. */
> +#define NL_ATTR_FOR_EACH(ITER, LEFT, ATTRS, ATTRS_LEN)                  \
> +    for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
> +         NlAttrIsValid(ITER, LEFT);                                     \
> +         (LEFT) -= NlAttrLenPad(ITER, LEFT), (ITER) = NlAttrNext(ITER))
> +
> +/* This macro does not check for attributes with bad lengths.  It should only
> + * be used with messages from trusted sources or with messages that have
> + * already been validated (e.g. with NL_ATTR_FOR_EACH).  */
> +#define NL_ATTR_FOR_EACH_UNSAFE(ITER, LEFT, ATTRS, ATTRS_LEN)           \
> +    for ((ITER) = (ATTRS), (LEFT) = (ATTRS_LEN);                        \
> +         (LEFT) > 0;                                                    \
> +         (LEFT) -= NLA_ALIGN((ITER)->nlaLen), (ITER) = NlAttrNext(ITER))
> +
> +#define NL_ATTR_GET_AS(NLA, TYPE) \
> +        (*(TYPE*) NlAttrGetUnspec(nla, sizeof(TYPE)))
> +
> +/* Netlink message accessing the payload */
> +static __inline PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
> +static __inline UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
> +static __inline char *NlMsgPayload(const PNL_MSG_HDR nlh);

char * => CHAR *

> +static __inline UINT32 NlMsgPayloadLen(const PNL_MSG_HDR nlh);
> +static __inline PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh);
> +static __inline INT NlMsgAttrLen(const PNL_MSG_HDR nlh);
> +
> +/* Netlink message parse */
> +static __inline PNL_MSG_HDR NlMsgNext(const PNL_MSG_HDR nlh);
> +static __inline INT NlAttrIsValid(const PNL_ATTR nla, UINT32 maxlen);
> +static __inline UINT32 NlAttrLenPad(const PNL_ATTR nla, UINT32 maxlen);
> +
> +/* Netlink attribute parsing. */
> +static UINT32 NlAttrMinLen(NL_ATTR_TYPE type);
> +static UINT32 NlAttrMinLen(NL_ATTR_TYPE type);
> +PNL_ATTR NlAttrNext(const PNL_ATTR nla);
> +UINT16 NlAttrType(const PNL_ATTR nla);
> +PVOID NlAttrData(const PNL_ATTR nla);
> +UINT32 NlAttrGetSize(const PNL_ATTR nla);
> +const PVOID NlAttrGet(const PNL_ATTR nla);
> +const PVOID NlAttrGetUnspec(const PNL_ATTR nla, UINT32 size);
> +UINT64 NlAttrGetBe64(const PNL_ATTR nla);
> +UINT32 NlAttrGetBe32(const PNL_ATTR nla);
> +UINT8 NlAttrGetU8(const PNL_ATTR nla);
> +UINT32 NlAttrGetU32(const PNL_ATTR nla);
> +static __inline const PNL_ATTR NlAttrFind__(const PNL_ATTR attrs, 
> +                                            UINT32 size, UINT16 type);
> +const PNL_ATTR NlAttrFindNested(const PNL_ATTR nla, 
> +                                UINT16 type);
> +BOOLEAN NetlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
> +                      const NL_POLICY policy[],
> +                      PNL_ATTR attrs[], UINT32 n_attrs);
> +
> +/* Netlink attribute validation */
> +BOOLEAN NlAttrValidate(const PNL_ATTR, const PNL_POLICY);
> +
> +#endif /* __NETLINK_H_ */


> diff --git a/datapath-windows/ovsext/NetlinkProto.h b/datapath-windows/ovsext/NetlinkProto.h
> new file mode 100644
> index 0000000..4cf3a7e
> --- /dev/null
> +++ b/datapath-windows/ovsext/NetlinkProto.h
> @@ -0,0 +1,118 @@
> +/*
> + * Copyright (c) 2008, 2010, 2011, 2014 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     https://urldefense.proofpoint.com/v1/url?u=http://www.apache.org/licenses/LICENSE-2.0&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=KOPgGejqFVaHXnjHwJvHNOBUOGfqwacPBI7AOIAwvjw%3D%0A&s=48250afd8c943ecd4d9558b0240c077225bd3653ac69eb7e091361f54043c4ee
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef __NETLINK_PROTO_H_
> +#define __NETLINK_PROTO_H_ 1
> +
> +/* Netlink protocol definitions.
> + *
> + * Netlink is a message framing format described in RFC 3549 and used heavily
> + * in Linux to access the network stack.  Open vSwitch uses AF_NETLINK sockets
> + * for this purpose on Linux.  On Windows platform too, Open vSwitch uses 
> + * netlink message format for userspace-kernelspace communication.
> + *
> + * This header provides access to the Netlink message framing definitions
> + * regardless of platform.
> + */
> +#include "OvsTypes.h"
> +
> +#define BUILD_ASSERT(EXPR) \
> +        typedef char AssertOnCompileFailed[(EXPR) ? 1: -1]
> +#define BUILD_ASSERT_DECL(EXPR) BUILD_ASSERT(EXPR)
> +
> +/* Returns X / Y, rounding up.  X must be nonnegative to round correctly. */
> +#define DIV_ROUND_UP(X, Y) (((X) + ((Y) - 1)) / (Y))
> +
> +/* Returns X rounded up to the nearest multiple of Y. */
> +#define ROUND_UP(X, Y) (DIV_ROUND_UP(X, Y) * (Y))
> +
> +/* Netlink message */
> +
> +/* nlmsg_flags bits. */
> +#define NLM_F_REQUEST           0x001
> +#define NLM_F_MULTI             0x002
> +#define NLM_F_ACK               0x004
> +#define NLM_F_ECHO              0x008
> +
> +#define NLM_F_ROOT              0x100
> +#define NLM_F_MATCH             0x200
> +#define NLM_F_EXCL              0x200
> +#define NLM_F_ATOMIC            0x400
> +#define NLM_F_CREATE            0x400
> +#define NLM_F_DUMP              (NLM_F_ROOT | NLM_F_MATCH)
> +
> +/* nlmsg_type values. */
> +#define NLMSG_NOOP              1
> +#define NLMSG_ERROR             2
> +#define NLMSG_DONE              3
> +#define NLMSG_OVERRUN           4
> +
> +#define NLMSG_MIN_TYPE          0x10
> +
> +#define MAX_LINKS               32
> +
> +#define NLMSG_ALIGNTO 4
> +#define NLMSG_ALIGN(SIZE) ROUND_UP(SIZE, NLMSG_ALIGNTO)
> +
> +#define NLA_ALIGNTO 4
> +#define NLA_ALIGN(SIZE) ROUND_UP(SIZE, NLA_ALIGNTO)
> +
> +typedef struct _OvsHeader {
> +    INT dp_ifindex;

dp_ifindex => dpIfIndex.

> +} OVS_HDR, *POVS_HDR;

Also, redefinition of OVS_HDR. There's one definition of it in OvsDpInterface.h (struct ovs_header). At the minimum, we should have an BUILD_ASSERT that both sizes are the same.

> +
> +typedef struct _NL_MSG_HDR {
> +    UINT32 nlmsgLen;
> +    UINT16 nlmsgType;
> +    UINT16 nlmsgFlags;
> +    UINT32 nlmsgSeq;
> +    UINT32 nlmsgPid;
> +} NL_MSG_HDR, *PNL_MSG_HDR;
> +BUILD_ASSERT_DECL(sizeof(NL_MSG_HDR) == 16);
> +
> +typedef struct _NlMsgErr
> +{
> +    INT error;
> +    NL_MSG_HDR msg;
> +} NL_MSG_ERR, *PNL_MSG_ERR;
> +BUILD_ASSERT_DECL(sizeof(NL_MSG_ERR) == 20);
> +
> +typedef struct _GENL_MSG_HDR {
> +    UINT8 cmd;
> +    UINT8 version;
> +    UINT16 reserved;
> +} GENL_MSG_HDR, *PGENL_MDG_HDR;
> +BUILD_ASSERT_DECL(sizeof(GENL_MSG_HDR) == 4);
> +
> +/* Netlink attributes */
> +typedef struct _NL_ATTR {
> +    UINT16 nlaLen;
> +    UINT16 nlaType;
> +} NL_ATTR, *PNL_ATTR;
> +BUILD_ASSERT_DECL(sizeof(NL_ATTR) == 4);
> +
> +#ifndef NLA_TYPE_MASK
> +#define NLA_F_NESTED        (1 << 15)
> +#define NLA_F_NET_BYTEORDER (1 << 14)
> +#define NLA_TYPE_MASK       ~(NLA_F_NESTED | NLA_F_NET_BYTEORDER)
> +#endif
> +
> +#define NLMSG_HDRLEN ((INT) NLMSG_ALIGN(sizeof(NL_MSG_HDR)))
> +#define GENL_HDRLEN NLMSG_ALIGN(sizeof(GENL_MSG_HDR))
> +#define OVS_HDRLEN NLMSG_ALIGN(sizeof(OVS_HDR))
> +#define NLA_HDRLEN ((INT) NLA_ALIGN(sizeof(NL_ATTR)))
> +
> +#endif /* NetlinProto.h */


> diff --git a/datapath-windows/ovsext/OvsTypes.h b/datapath-windows/ovsext/OvsTypes.h
> index 402c39f..d07dfdf 100644
> --- a/datapath-windows/ovsext/OvsTypes.h
> +++ b/datapath-windows/ovsext/OvsTypes.h
> @@ -29,4 +29,6 @@ typedef uint8 __u8;
> 
> #define ETH_ALEN 6
> 
> +#define SIZE_MAX 18446744073709551615UL
> +
> #endif /* __OVS_TYPES_H_ */


> diff --git a/datapath-windows/ovsext/ovsext.vcxproj b/datapath-windows/ovsext/ovsext.vcxproj
> index 2b3e6fd..1a618de 100644
> --- a/datapath-windows/ovsext/ovsext.vcxproj
> +++ b/datapath-windows/ovsext/ovsext.vcxproj
> @@ -75,6 +75,8 @@
> <ClInclude Include="OvsBufferMgmt.h" />
> <ClInclude Include="OvsChecksum.h" />
> <ClInclude Include="Datapath.h" />
> +    <ClInclude Include="Netlink.h" />
> +    <ClInclude Include="NetlinkProto.h" />
> <ClInclude Include="OvsDebug.h" />
> <ClInclude Include="OvsEth.h" />
> <ClInclude Include="OvsEvent.h" />
> @@ -125,6 +127,7 @@
> </ClCompile>
> </ItemDefinitionGroup>
> <ItemGroup>
> +    <ClCompile Include="Netlink.c" />
> <ClCompile Include="Datapath.c" />
> <ClCompile Include="OvsDriver.c" />
> <ClCompile Include="OvsJhash.c" />

> diff --git a/datapath-windows/ovsext/precomp.h b/datapath-windows/ovsext/precomp.h
> index 5f23d02..5b6c2a9 100644
> --- a/datapath-windows/ovsext/precomp.h
> +++ b/datapath-windows/ovsext/precomp.h
> @@ -24,6 +24,8 @@
> #include "OvsTypes.h"
> #include "..\include\OvsPub.h"
> #include "OvsUtil.h"
> +#include "Netlink.h"
> +#include "NetlinkProto.h"
> /*
> * Include openvswitch.h from userspace. Changing the location the file from
> * include/linux is pending discussion.
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=KOPgGejqFVaHXnjHwJvHNOBUOGfqwacPBI7AOIAwvjw%3D%0A&s=4e49f61760fb71bf6513e665794ab09666697c2778ce49709e79599e50794459




More information about the dev mailing list