Hi Nithin, It may be out of the scope of this change but for consistency reason all NL function should return BOOLEAN rather than NTSTATUS. This should apply NlFillOvsMsg() and NlFillNlHdr() too.
BOOLEAN ok; + ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI, But the function signature is NTSTATUS NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType,: Besides that LG. Thanks, Eitan -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju Sent: Wednesday, December 03, 2014 7:56 AM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH 3/3] datapath-windows: refactor BuildReplyMsgFromMsgIn & BuildErrorMsg In this patch, we consolidate code in Netlink.c. Signed-off-by: Nithin Raju <nit...@vmware.com> --- datapath-windows/ovsext/Datapath.c | 2 +- datapath-windows/ovsext/Flow.c | 4 +- datapath-windows/ovsext/Netlink/Netlink.c | 40 ++++++++--------------------- datapath-windows/ovsext/Netlink/Netlink.h | 6 +--- datapath-windows/ovsext/User.c | 2 +- datapath-windows/ovsext/Vport.c | 28 +++++++++----------- 6 files changed, 30 insertions(+), 52 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index f6e6e50..af8d818 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -1324,7 +1324,7 @@ cleanup: POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index f47d469..f50bb39 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -355,7 +355,7 @@ done: if (nlError != NL_ERROR_SUCCESS) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; rc = STATUS_SUCCESS; } @@ -387,7 +387,7 @@ OvsFlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, (usrParamsCtx->outputBuffer)) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; status = STATUS_SUCCESS; goto done; diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c index 7633f2f..0d48d08 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.c +++ b/datapath-windows/ovsext/Netlink/Netlink.c @@ -102,41 +102,23 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType, return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE; } -static VOID -BuildMsgOut(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 type, - UINT32 length, UINT16 flags) -{ - msgOut->nlMsg.nlmsgType = type; - msgOut->nlMsg.nlmsgFlags = flags; - msgOut->nlMsg.nlmsgSeq = msgIn->nlMsg.nlmsgSeq; - msgOut->nlMsg.nlmsgPid = msgIn->nlMsg.nlmsgPid; - msgOut->nlMsg.nlmsgLen = length; - - msgOut->genlMsg.cmd = msgIn->genlMsg.cmd; - msgOut->genlMsg.version = msgIn->genlMsg.version; - msgOut->genlMsg.reserved = 0; -} - /* - * XXX: should move out these functions to a Netlink.c or to a OvsMessage.c - * or even make them inlined functions in Datapath.h. Can be done after the - * first sprint once we have more code to refactor. + * + ---------------------------------------------------------------------- + ----- + * Prepare a 'OVS_MESSAGE_ERROR' message. + * + ---------------------------------------------------------------------- + ----- */ VOID -BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, UINT16 flags) +NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgError, UINT +errorCode) { - BuildMsgOut(msgIn, msgOut, msgIn->nlMsg.nlmsgType, sizeof(OVS_MESSAGE), - flags); -} + NL_BUFFER nlBuffer; -VOID -BuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, UINT errorCode) -{ - BuildMsgOut(msgIn, (POVS_MESSAGE)msgOut, NLMSG_ERROR, - sizeof(OVS_MESSAGE_ERROR), 0); + NlBufInit(&nlBuffer, (PCHAR)msgError, sizeof *msgError); + NlFillNlHdr(&nlBuffer, NLMSG_ERROR, 0, + msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid); - msgOut->errorMsg.error = errorCode; - msgOut->errorMsg.nlMsg = msgIn->nlMsg; + msgError->errorMsg.error = errorCode; + msgError->errorMsg.nlMsg = msgIn->nlMsg; + msgError->nlMsg.nlmsgLen = sizeof(OVS_MESSAGE_ERROR); } /* diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h index 18e40b1..a7a53e0 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.h +++ b/datapath-windows/ovsext/Netlink/Netlink.h @@ -94,10 +94,8 @@ NTSTATUS NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType, UINT16 nlmsgFlags, UINT32 nlmsgSeq, UINT32 nlmsgPid); -VOID BuildReplyMsgFromMsgIn(POVS_MESSAGE msgIn, POVS_MESSAGE msgOut, - UINT16 flags); -VOID BuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, - UINT errorCode); +VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, + UINT errorCode); /* Netlink message accessing the payload */ PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset); diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 8473f98..70091cb 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -399,7 +399,7 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; status = STATUS_SUCCESS; goto done; diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 35f95bc..2efb363 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -1535,7 +1535,7 @@ cleanup: POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } @@ -1558,16 +1558,15 @@ CreateNetlinkMesgForNetdev(POVS_VPORT_EXT_INFO info, { NL_BUFFER nlBuffer; BOOLEAN ok; - OVS_MESSAGE msgOut; PNL_MSG_HDR nlMsg; UINT32 netdevFlags = 0; NlBufInit(&nlBuffer, outBuffer, outBufLen); - BuildReplyMsgFromMsgIn(msgIn, &msgOut, 0); - msgOut.ovsHdr.dp_ifindex = dpIfIndex; - - ok = NlMsgPutHead(&nlBuffer, (PCHAR)&msgOut, sizeof msgOut); + ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI, + msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid, + msgIn->genlMsg.cmd, msgIn->genlMsg.version, + dpIfIndex) == STATUS_SUCCESS ? TRUE : FALSE; if (!ok) { return STATUS_INSUFFICIENT_RESOURCES; } @@ -1641,15 +1640,14 @@ OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport, NL_BUFFER nlBuffer; OVS_VPORT_FULL_STATS vportStats; BOOLEAN ok; - OVS_MESSAGE msgOut; PNL_MSG_HDR nlMsg; NlBufInit(&nlBuffer, outBuffer, outBufLen); - BuildReplyMsgFromMsgIn(msgIn, &msgOut, NLM_F_MULTI); - msgOut.ovsHdr.dp_ifindex = dpIfIndex; - - ok = NlMsgPutHead(&nlBuffer, (PCHAR)&msgOut, sizeof msgOut); + ok = NlFillOvsMsg(&nlBuffer, msgIn->nlMsg.nlmsgType, NLM_F_MULTI, + msgIn->nlMsg.nlmsgSeq, msgIn->nlMsg.nlmsgPid, + msgIn->genlMsg.cmd, msgIn->genlMsg.version, + dpIfIndex) == STATUS_SUCCESS ? TRUE : FALSE; if (!ok) { return STATUS_INSUFFICIENT_RESOURCES; } @@ -1901,7 +1899,7 @@ Cleanup: POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } @@ -2134,7 +2132,7 @@ Cleanup: OvsFreeMemory(vport); } - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } @@ -2246,7 +2244,7 @@ Cleanup: POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } @@ -2330,7 +2328,7 @@ Cleanup: POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); + NlBuildErrorMsg(msgIn, msgError, nlError); *replyLen = msgError->nlMsg.nlmsgLen; } -- 1.7.4.1 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=uV1ziCRTbQ2ywCwaGRIn51SpQrftiGFTfg1j9QMLsO8&s=jTGQSYmDNTQmSZGjG19ETB8jww9uXdQyc8w5d6F6tRY&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev