[ovs-dev] [PATCH v1 1/5] datapath-windows: Netlink additional APIs.

Nithin Raju nithin at vmware.com
Thu Oct 2 03:43:14 UTC 2014


hi Ankur,
Thanks for making the change. Looks good mostly, I had minor comments.

I'll ack the v2.

On Sep 29, 2014, at 3:34 PM, Ankur Sharma <ankursharma at vmware.com> wrote:

> In this patch we have added following new APIs.
> 
> NlMsgAlignSize => Aligns the size of netlink message.
> NlMsgSetSize => Sets the value of nlmsgLen.

I suppose we should add 'NlFillNlHdr' to the list.

> 
> Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
> ---
> datapath-windows/ovsext/Netlink/Netlink.c | 54 +++++++++++++++++++++++++++++++
> datapath-windows/ovsext/Netlink/Netlink.h |  5 +++
> 2 files changed, 59 insertions(+)
> 
> diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c
> index 0c4f847..df287de 100644
> --- a/datapath-windows/ovsext/Netlink/Netlink.c
> +++ b/datapath-windows/ovsext/Netlink/Netlink.c
> @@ -75,6 +75,37 @@ done:
>     return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
> }
> 
> +NTSTATUS
> +NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType,
> +            UINT16 nlmsgFlags, UINT32 nlmsgSeq,
> +            UINT32 nlmsgPid)

Missing description like other functions. After reading the code, it seems that this you can use this API to write out a netlink message header in the middle of a buffer. Offset = 0 will also work. Documenting it would be useful.

> +{
> +    BOOLEAN writeOk;
> +    PNL_MSG_HDR nlMsg;
> +    NL_MSG_HDR msgOut;
> +    UINT32 offset = NlBufSize(nlBuf);
> +
> +    ASSERT(NlBufAt(nlBuf, offset, sizeof(struct _NL_MSG_HDR)) != 0);


> +
> +    msgOut.nlmsgType = nlmsgType;
> +    msgOut.nlmsgFlags = nlmsgFlags;
> +    msgOut.nlmsgSeq = nlmsgSeq;
> +    msgOut.nlmsgPid = nlmsgPid;
> +
> +    writeOk = NlMsgPutTail(nlBuf, (PCHAR)(&msgOut),
> +                           sizeof(struct _NL_MSG_HDR));
> +
> +    if (!writeOk) {
> +        goto done;
> +    }
> +
> +    nlMsg = (PNL_MSG_HDR)NlBufAt(nlBuf, offset, sizeof(struct _NL_MSG_HDR));
> +    nlMsg->nlmsgLen = sizeof(struct _NL_MSG_HDR);

'nlmsgLen' can be set as part of initializing msgOut. In other functions where we update nlMsg->nlmsgLen, a subsequent update is required since we have added some attributes etc.

> +
> +done:
> +    return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE;
> +}
> +
> /*
>  * ---------------------------------------------------------------------------
>  * Adds Netlink Header to the NL_BUF.
> @@ -554,6 +585,29 @@ NlMsgSize(const PNL_MSG_HDR nlh)
> 
> /*
>  * ---------------------------------------------------------------------------
> + * Aligns the size of Netlink message.
> + * ---------------------------------------------------------------------------
> + */
> +VOID
> +NlMsgAlignSize(const PNL_MSG_HDR nlh)
> +{
> +    nlh->nlmsgLen = NLMSG_ALIGN(nlh->nlmsgLen);
> +    return;
> +}
> +
> +/*
> + * ---------------------------------------------------------------------------
> + * Sets the size of Netlink message.
> + * ---------------------------------------------------------------------------
> + */
> +VOID
> +NlMsgSetSize(const PNL_MSG_HDR nlh, UINT32 msgLen)
> +{
> +    nlh->nlmsgLen = msgLen;
> +}
> +
> +/*
> + * ---------------------------------------------------------------------------
>  * Returns pointer to nlmsg payload.
>  * ---------------------------------------------------------------------------
>  */
> diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h
> index 26772b7..cd55647 100644
> --- a/datapath-windows/ovsext/Netlink/Netlink.h
> +++ b/datapath-windows/ovsext/Netlink/Netlink.h
> @@ -82,10 +82,15 @@ NTSTATUS NlFillOvsMsg(PNL_BUFFER nlBuf,
>                       UINT16 nlmsgType, UINT16 nlmsgFlags,
>                       UINT32 nlmsgSeq, UINT32 nlmsgPid,
>                       UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo);
> +NTSTATUS NlFillNlHdr(PNL_BUFFER nlBuf,
> +                     UINT16 nlmsgType, UINT16 nlmsgFlags,
> +                     UINT32 nlmsgSeq, UINT32 nlmsgPid);
> 
> /* Netlink message accessing the payload */
> PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
> UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
> +VOID NlMsgAlignSize(const PNL_MSG_HDR nlh);
> +VOID NlMsgSetSize(const PNL_MSG_HDR nlh, UINT32 msgLen);
> PCHAR NlHdrPayload(const PNL_MSG_HDR nlh);
> UINT32 NlHdrPayloadLen(const PNL_MSG_HDR nlh);
> PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh);
> -- 
> 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=gy%2Fyp4FcVYviWGjbi9QjVk3dealU0KZiHxaZc3TuRYQ%3D%0A&s=c53bf5c816d455a625d737ea0ef18a26d577a373ee4cd3a43e731a5398538302

Thanks,
-- Nithin




More information about the dev mailing list