Acked-by: Eitan Eliahu <elia...@vmware.com> Thanks for addressing my comment. Eitan
-----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju Sent: Monday, December 08, 2014 9:43 AM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH 2/2] datapath-windows: return bool from NlFillOvs[Msg/Hdr] Per review comment, in this patch, we update the return values of NlFillOvsMsg() and NlFillOvsHdr() from NTSTATUS to BOOLEAN to make them consistent with the Nl* functions. Signed-off-by: Nithin Raju <nit...@vmware.com> --- datapath-windows/ovsext/Flow.c | 31 ++++++++++++++++++---------- datapath-windows/ovsext/Netlink/Netlink.c | 8 +++--- datapath-windows/ovsext/Netlink/Netlink.h | 12 +++++----- datapath-windows/ovsext/User.c | 12 +++++++--- datapath-windows/ovsext/Vport.c | 30 ++++++++++++++-------------- 5 files changed, 53 insertions(+), 40 deletions(-) diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index f50bb39..93f37a0 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -247,6 +247,7 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, UINT32 *replyLen) { NTSTATUS rc = STATUS_SUCCESS; + BOOLEAN ok; POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer; POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer; PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg); @@ -258,7 +259,6 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, OvsFlowStats stats; struct ovs_flow_stats replyStats; NL_ERROR nlError = NL_ERROR_SUCCESS; - NL_BUFFER nlBuf; RtlZeroMemory(&mappedFlow, sizeof(OvsFlowPut)); @@ -294,13 +294,14 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, usrParamsCtx->outputLength); /* Prepare nl Msg headers */ - rc = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, + ok = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid, genlMsgHdr->cmd, OVS_FLOW_VERSION, ovsHdr->dp_ifindex); - - if (rc == STATUS_SUCCESS) { + if (ok) { *replyLen = msgOut->nlMsg.nlmsgLen; + } else { + rc = STATUS_INVALID_BUFFER_SIZE; } } @@ -330,13 +331,15 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, usrParamsCtx->outputLength); /* Prepare nl Msg headers */ - rc = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, + ok = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid, genlMsgHdr->cmd, OVS_FLOW_VERSION, ovsHdr->dp_ifindex); - - if (rc != STATUS_SUCCESS) { + if (!ok) { + rc = STATUS_INVALID_BUFFER_SIZE; goto done; + } else { + rc = STATUS_SUCCESS; } /* Append OVS_FLOW_ATTR_STATS attribute */ @@ -586,15 +589,20 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, /* Done with Dump, send NLMSG_DONE */ if (!(dumpOutput.n)) { + BOOLEAN ok; + OVS_LOG_INFO("Dump Done"); nlMsgOutHdr = (PNL_MSG_HDR)(NlBufAt(&nlBuf, NlBufSize(&nlBuf), 0)); - rc = NlFillNlHdr(&nlBuf, NLMSG_DONE, NLM_F_MULTI, + ok = NlFillNlHdr(&nlBuf, NLMSG_DONE, NLM_F_MULTI, nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid); - if (rc != STATUS_SUCCESS) { + if (!ok) { + rc = STATUS_INVALID_BUFFER_SIZE; OVS_LOG_ERROR("Unable to prepare DUMP_DONE reply."); break; + } else { + rc = STATUS_SUCCESS; } NlMsgAlignSize(nlMsgOutHdr); @@ -603,17 +611,18 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, FreeUserDumpState(instance); break; } else { + BOOLEAN ok; hdrOffset = NlBufSize(&nlBuf); nlMsgOutHdr = (PNL_MSG_HDR)(NlBufAt(&nlBuf, hdrOffset, 0)); /* Netlink header */ - rc = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, NLM_F_MULTI, + ok = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, NLM_F_MULTI, nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid, genlMsgHdr->cmd, genlMsgHdr->version, ovsHdr->dp_ifindex); - if (rc != STATUS_SUCCESS) { + if (!ok) { /* Reset rc to success so that we can * send already added messages to user space. */ rc = STATUS_SUCCESS; diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c index 0d48d08..589e3a1 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.c +++ b/datapath-windows/ovsext/Netlink/Netlink.c @@ -39,7 +39,7 @@ * Attributes should be added by caller. * --------------------------------------------------------------------------- */ -NTSTATUS +BOOLEAN NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType, UINT16 nlmsgFlags, UINT32 nlmsgSeq, UINT32 nlmsgPid, UINT8 genlCmd, @@ -68,7 +68,7 @@ NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType, writeOk = NlMsgPutTail(nlBuf, (PCHAR)(&msgOut), sizeof (struct _OVS_MESSAGE)); - return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE; + return writeOk; } /* @@ -77,7 +77,7 @@ NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType, * input NlBuf. * --------------------------------------------------------------------------- */ -NTSTATUS +BOOLEAN NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType, UINT16 nlmsgFlags, UINT32 nlmsgSeq, UINT32 nlmsgPid) @@ -99,7 +99,7 @@ NlFillNlHdr(PNL_BUFFER nlBuf, UINT16 nlmsgType, writeOk = NlMsgPutTail(nlBuf, (PCHAR)(&msgOut), sizeof(struct _NL_MSG_HDR)); - return writeOk ? STATUS_SUCCESS : STATUS_INVALID_BUFFER_SIZE; + return writeOk; } /* diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h index a7a53e0..36ffee9 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.h +++ b/datapath-windows/ovsext/Netlink/Netlink.h @@ -86,13 +86,13 @@ typedef struct _NL_POLICY #define NL_ATTR_GET_AS(NLA, TYPE) \ (*(TYPE*) NlAttrGetUnspec(nla, sizeof(TYPE))) -NTSTATUS NlFillOvsMsg(PNL_BUFFER nlBuf, - UINT16 nlmsgType, UINT16 nlmsgFlags, - UINT32 nlmsgSeq, UINT32 nlmsgPid, - UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo); -NTSTATUS NlFillNlHdr(PNL_BUFFER nlBuf, +BOOLEAN NlFillOvsMsg(PNL_BUFFER nlBuf, UINT16 nlmsgType, UINT16 nlmsgFlags, - UINT32 nlmsgSeq, UINT32 nlmsgPid); + UINT32 nlmsgSeq, UINT32 nlmsgPid, + UINT8 genlCmd, UINT8 genlVer, UINT32 dpNo); +BOOLEAN NlFillNlHdr(PNL_BUFFER nlBuf, + UINT16 nlmsgType, UINT16 nlmsgFlags, + UINT32 nlmsgSeq, UINT32 nlmsgPid); VOID NlBuildErrorMsg(POVS_MESSAGE msgIn, POVS_MESSAGE_ERROR msgOut, UINT errorCode); diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 70091cb..d8a657e 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -375,17 +375,21 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, /* Default reply that we want to send */ if (status == STATUS_SUCCESS) { + BOOLEAN ok; + NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength); /* Prepare nl Msg headers */ - status = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, + ok = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid, genlMsgHdr->cmd, OVS_PACKET_VERSION, ovsHdr->dp_ifindex); - if (status == STATUS_SUCCESS) { + if (ok) { *replyLen = msgOut->nlMsg.nlmsgLen; + } else { + status = STATUS_INVALID_BUFFER_SIZE; } } else { /* Map NTSTATUS to NL_ERROR */ @@ -1088,9 +1092,9 @@ OvsCreateQueueNlPacket(PVOID userData, * Since we are pre allocating memory for the NL buffer * the attribute settings should not fail */ - if (NlFillOvsMsg(&nlBuf, OVS_WIN_NL_PACKET_FAMILY_ID, 0, + if (!NlFillOvsMsg(&nlBuf, OVS_WIN_NL_PACKET_FAMILY_ID, 0, 0, pid, (UINT8)cmd, OVS_PACKET_VERSION, - gOvsSwitchContext->dpNo) != STATUS_SUCCESS) { + gOvsSwitchContext->dpNo)) { goto fail; } diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 2efb363..c9dfaea 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -1566,37 +1566,37 @@ CreateNetlinkMesgForNetdev(POVS_VPORT_EXT_INFO info, 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; + dpIfIndex); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailU32(&nlBuffer, OVS_WIN_NETDEV_ATTR_PORT_NO, info->portNo); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailU32(&nlBuffer, OVS_WIN_NETDEV_ATTR_TYPE, info->type); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailString(&nlBuffer, OVS_WIN_NETDEV_ATTR_NAME, info->name); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailUnspec(&nlBuffer, OVS_WIN_NETDEV_ATTR_MAC_ADDR, (PCHAR)info->macAddress, sizeof (info->macAddress)); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailU32(&nlBuffer, OVS_WIN_NETDEV_ATTR_MTU, info->mtu); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } if (info->status != OVS_EVENT_CONNECT) { @@ -1605,7 +1605,7 @@ CreateNetlinkMesgForNetdev(POVS_VPORT_EXT_INFO info, ok = NlMsgPutTailU32(&nlBuffer, OVS_WIN_NETDEV_ATTR_IF_FLAGS, netdevFlags); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } /* @@ -1647,24 +1647,24 @@ OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport, 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; + dpIfIndex); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_PORT_NO, vport->portNo); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_TYPE, vport->ovsType); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } ok = NlMsgPutTailString(&nlBuffer, OVS_VPORT_ATTR_NAME, vport->ovsName); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } /* @@ -1677,7 +1677,7 @@ OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport, ok = NlMsgPutTailU32(&nlBuffer, OVS_VPORT_ATTR_UPCALL_PID, vport->upcallPid); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } /*stats*/ @@ -1694,7 +1694,7 @@ OvsCreateMsgFromVport(POVS_VPORT_ENTRY vport, (PCHAR)&vportStats, sizeof(OVS_VPORT_FULL_STATS)); if (!ok) { - return STATUS_INSUFFICIENT_RESOURCES; + return STATUS_INVALID_BUFFER_SIZE; } /* -- 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=k4w_IZIGN68UnKO-YCGfApFe3Lf22R9ajkLAApKEIqc&s=MAH0ZRHuoL6SnB37XIIW_I_TdWnDN6vOkHMvOFktkls&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
