Added changes to support error handling for non supported actions. Added changes in packet execute for sending transactional errors.
Signed-off-by: Ankur Sharma <[email protected]> --- datapath-windows/ovsext/Actions.c | 4 +- datapath-windows/ovsext/Netlink/NetlinkError.h | 17 +++ datapath-windows/ovsext/User.c | 38 +++--- v1-0000-cover-letter.patch | 19 +++ ...th-windows-OVS_PACKET_CMD_EXECUTE-handler.patch | 147 +++++++++++++++++++++ ...-windows-Action-not-supported-error-handl.patch | 122 +++++++++++++++++ 6 files changed, 329 insertions(+), 18 deletions(-) create mode 100644 v1-0000-cover-letter.patch create mode 100644 v1-0001-datapath-windows-OVS_PACKET_CMD_EXECUTE-handler.patch create mode 100644 v1-0002-datapath-windows-Action-not-supported-error-handl.patch diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c index 23de67e..408b9be 100644 --- a/datapath-windows/ovsext/Actions.c +++ b/datapath-windows/ovsext/Actions.c @@ -1526,10 +1526,8 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, break; } case OVS_ACTION_ATTR_SAMPLE: - break; - case OVS_ACTION_ATTR_UNSPEC: - case __OVS_ACTION_ATTR_MAX: default: + status = NDIS_STATUS_NOT_SUPPORTED; break; } } diff --git a/datapath-windows/ovsext/Netlink/NetlinkError.h b/datapath-windows/ovsext/Netlink/NetlinkError.h index c41414c..827fa8c 100644 --- a/datapath-windows/ovsext/Netlink/NetlinkError.h +++ b/datapath-windows/ovsext/Netlink/NetlinkError.h @@ -198,3 +198,20 @@ typedef enum _NL_ERROR_ /*the operation would block */ NL_ERROR_WOULDBLOCK = ((ULONG)-140), } NL_ERROR; + +static __inline +NlMapStatusToNlErr(NTSTATUS status) +{ + NL_ERROR ret = NL_ERROR_SUCCESS; + + switch (status) + { + case STATUS_NOT_SUPPORTED: + ret = NL_ERROR_NOTSUPP; + break; + default: + break; + } + + return ret; +} diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 8400b83..f24c4e3 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -369,6 +369,7 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, status = OvsExecuteDpIoctl(&execute); + /* Default reply that we want to send */ if (status == STATUS_SUCCESS) { NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength); @@ -382,20 +383,23 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, if (status == STATUS_SUCCESS) { *replyLen = msgOut->nlMsg.nlmsgLen; } - } - - /* As of now there are no transactional errors in the implementation. - * Once we have them then we need to map status to correct - * nlError value, so that below mentioned code gets hit. */ - if ((nlError != NL_ERROR_SUCCESS) && - (usrParamsCtx->outputBuffer)) { - - POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) - usrParamsCtx->outputBuffer; - BuildErrorMsg(msgIn, msgError, nlError); - *replyLen = msgError->nlMsg.nlmsgLen; - status = STATUS_SUCCESS; - goto done; + } else { + /* Map NTSTATUS to NL_ERROR */ + nlError = NlMapStatusToNlErr(status); + + /* As of now there are no transactional errors in the implementation. + * Once we have them then we need to map status to correct + * nlError value, so that below mentioned code gets hit. */ + if ((nlError != NL_ERROR_SUCCESS) && + (usrParamsCtx->outputBuffer)) { + + POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) + usrParamsCtx->outputBuffer; + BuildErrorMsg(msgIn, msgError, nlError); + *replyLen = msgError->nlMsg.nlmsgLen; + status = STATUS_SUCCESS; + goto done; + } } done: @@ -487,7 +491,11 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute) NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); } if (ndisStatus != NDIS_STATUS_SUCCESS) { - status = STATUS_UNSUCCESSFUL; + if (ndisStatus == NDIS_STATUS_NOT_SUPPORTED) { + status = STATUS_NOT_SUPPORTED; + } else { + status = STATUS_UNSUCCESSFUL; + } } if (pNbl) { diff --git a/v1-0000-cover-letter.patch b/v1-0000-cover-letter.patch new file mode 100644 index 0000000..dcb031f --- /dev/null +++ b/v1-0000-cover-letter.patch @@ -0,0 +1,19 @@ +From f5809556cf9db17691a0e97db903726ee2f55eec Mon Sep 17 00:00:00 2001 +From: Ankur Sharma <[email protected]> +Date: Wed, 15 Oct 2014 15:19:40 -0700 +Subject: [PATCH v1 0/2] *** SUBJECT HERE *** + +*** BLURB HERE *** + +Ankur Sharma (2): + datapath-windows: OVS_PACKET_CMD_EXECUTE handler. + datapath-windows: Action not supported error handling. + + datapath-windows/ovsext/Actions.c | 4 +- + datapath-windows/ovsext/Netlink/NetlinkError.h | 17 ++++ + datapath-windows/ovsext/User.c | 121 ++++++++++++++++++++++++- + 3 files changed, 134 insertions(+), 8 deletions(-) + +-- +1.9.1 + diff --git a/v1-0001-datapath-windows-OVS_PACKET_CMD_EXECUTE-handler.patch b/v1-0001-datapath-windows-OVS_PACKET_CMD_EXECUTE-handler.patch new file mode 100644 index 0000000..e316b94 --- /dev/null +++ b/v1-0001-datapath-windows-OVS_PACKET_CMD_EXECUTE-handler.patch @@ -0,0 +1,147 @@ +From 91076ef8f378a8652e1749067805e97849436608 Mon Sep 17 00:00:00 2001 +From: Ankur Sharma <[email protected]> +Date: Thu, 16 Oct 2014 01:32:57 +0000 +Subject: [PATCH v1 1/2] datapath-windows: OVS_PACKET_CMD_EXECUTE handler. + +In this patch we have implemented the handler for OVS_PACKET_CMD_EXECUTE command. + +Signed-off-by: Ankur Sharma <[email protected]> +Acked-by: Eitan Eliahu <[email protected]> +--- + datapath-windows/ovsext/User.c | 111 +++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 107 insertions(+), 4 deletions(-) + +diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c +index 31c579c..8400b83 100644 +--- a/datapath-windows/ovsext/User.c ++++ b/datapath-windows/ovsext/User.c +@@ -46,6 +46,9 @@ extern PNDIS_SPIN_LOCK gOvsCtrlLock; + extern POVS_SWITCH_CONTEXT gOvsSwitchContext; + OVS_USER_STATS ovsUserStats; + ++static VOID _MapNlAttrToOvsPktExec(PNL_ATTR *nlAttrs, PNL_ATTR *keyAttrs, ++ OvsPacketExecute *execute); ++extern NL_POLICY nlFlowKeyPolicy[]; + + NTSTATUS + OvsUserInit() +@@ -310,12 +313,112 @@ NTSTATUS + OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + UINT32 *replyLen) + { +- NTSTATUS rc = STATUS_SUCCESS; ++ NTSTATUS status = STATUS_SUCCESS; ++ POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer; ++ POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer; ++ PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg); ++ PGENL_MSG_HDR genlMsgHdr = &(msgIn->genlMsg); ++ POVS_HDR ovsHdr = &(msgIn->ovsHdr); ++ ++ PNL_ATTR nlAttrs[__OVS_PACKET_ATTR_MAX]; ++ PNL_ATTR keyAttrs[__OVS_KEY_ATTR_MAX] = {NULL}; ++ ++ UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN; ++ UINT32 keyAttrOffset = 0; ++ OvsPacketExecute execute; ++ NL_ERROR nlError = NL_ERROR_SUCCESS; ++ NL_BUFFER nlBuf; ++ ++ static const NL_POLICY nlPktExecPolicy[] = { ++ [OVS_PACKET_ATTR_PACKET] = {.type = NL_A_UNSPEC, .optional = FALSE}, ++ [OVS_PACKET_ATTR_KEY] = {.type = NL_A_UNSPEC, .optional = FALSE}, ++ [OVS_PACKET_ATTR_ACTIONS] = {.type = NL_A_UNSPEC, .optional = FALSE}, ++ [OVS_PACKET_ATTR_USERDATA] = {.type = NL_A_UNSPEC, .optional = TRUE}, ++ [OVS_PACKET_ATTR_EGRESS_TUN_KEY] = {.type = NL_A_UNSPEC, ++ .optional = TRUE} ++ }; ++ ++ RtlZeroMemory(&execute, sizeof(OvsPacketExecute)); ++ ++ /* Get all the top level Flow attributes */ ++ if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr), ++ nlPktExecPolicy, nlAttrs, ARRAY_SIZE(nlAttrs))) ++ != TRUE) { ++ OVS_LOG_ERROR("Attr Parsing failed for msg: %p", ++ nlMsgHdr); ++ status = STATUS_UNSUCCESSFUL; ++ goto done; ++ } ++ ++ keyAttrOffset = (UINT32)((PCHAR)nlAttrs[OVS_PACKET_ATTR_KEY] - ++ (PCHAR)nlMsgHdr); ++ ++ /* Get flow keys attributes */ ++ if ((NlAttrParseNested(nlMsgHdr, keyAttrOffset, ++ NlAttrLen(nlAttrs[OVS_PACKET_ATTR_KEY]), ++ nlFlowKeyPolicy, keyAttrs, ++ ARRAY_SIZE(keyAttrs))) != TRUE) { ++ OVS_LOG_ERROR("Key Attr Parsing failed for msg: %p", nlMsgHdr); ++ status = STATUS_UNSUCCESSFUL; ++ goto done; ++ } ++ ++ execute.dpNo = ovsHdr->dp_ifindex; ++ ++ _MapNlAttrToOvsPktExec(nlAttrs, keyAttrs, &execute); ++ ++ status = OvsExecuteDpIoctl(&execute); ++ ++ if (status == STATUS_SUCCESS) { ++ NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, ++ usrParamsCtx->outputLength); ++ ++ /* Prepare nl Msg headers */ ++ status = NlFillOvsMsg(&nlBuf, nlMsgHdr->nlmsgType, 0, ++ nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid, ++ genlMsgHdr->cmd, OVS_PACKET_VERSION, ++ ovsHdr->dp_ifindex); ++ ++ if (status == STATUS_SUCCESS) { ++ *replyLen = msgOut->nlMsg.nlmsgLen; ++ } ++ } ++ ++ /* As of now there are no transactional errors in the implementation. ++ * Once we have them then we need to map status to correct ++ * nlError value, so that below mentioned code gets hit. */ ++ if ((nlError != NL_ERROR_SUCCESS) && ++ (usrParamsCtx->outputBuffer)) { ++ ++ POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) ++ usrParamsCtx->outputBuffer; ++ BuildErrorMsg(msgIn, msgError, nlError); ++ *replyLen = msgError->nlMsg.nlmsgLen; ++ status = STATUS_SUCCESS; ++ goto done; ++ } ++ ++done: ++ return status; ++} ++ ++/* ++ *---------------------------------------------------------------------------- ++ * _MapNlAttrToOvsPktExec -- ++ * Maps input Netlink attributes to OvsPacketExecute. ++ *---------------------------------------------------------------------------- ++ */ ++static VOID ++_MapNlAttrToOvsPktExec(PNL_ATTR *nlAttrs, PNL_ATTR *keyAttrs, ++ OvsPacketExecute *execute) ++{ ++ execute->packetBuf = NlAttrGet(nlAttrs[OVS_PACKET_ATTR_PACKET]); ++ execute->packetLen = NlAttrGetSize(nlAttrs[OVS_PACKET_ATTR_PACKET]); + +- UNREFERENCED_PARAMETER(usrParamsCtx); +- UNREFERENCED_PARAMETER(replyLen); ++ execute->actions = NlAttrGet(nlAttrs[OVS_PACKET_ATTR_ACTIONS]); ++ execute->actionsLen = NlAttrGetSize(nlAttrs[OVS_PACKET_ATTR_ACTIONS]); + +- return rc; ++ execute->inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]); + } + + NTSTATUS +-- +1.9.1 + diff --git a/v1-0002-datapath-windows-Action-not-supported-error-handl.patch b/v1-0002-datapath-windows-Action-not-supported-error-handl.patch new file mode 100644 index 0000000..0498854 --- /dev/null +++ b/v1-0002-datapath-windows-Action-not-supported-error-handl.patch @@ -0,0 +1,122 @@ +From f5809556cf9db17691a0e97db903726ee2f55eec Mon Sep 17 00:00:00 2001 +From: Ankur Sharma <[email protected]> +Date: Wed, 15 Oct 2014 15:13:51 -0700 +Subject: [PATCH v1 2/2] datapath-windows: Action not supported error handling. + +Added changes to support error handling for non supported actions. +Added changes in packet execute for sending transactional errors. + +Signed-off-by: Ankur Sharma <[email protected]> +--- + datapath-windows/ovsext/Actions.c | 4 +-- + datapath-windows/ovsext/Netlink/NetlinkError.h | 17 ++++++++++++ + datapath-windows/ovsext/User.c | 38 ++++++++++++++++---------- + 3 files changed, 41 insertions(+), 18 deletions(-) + +diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c +index 23de67e..408b9be 100644 +--- a/datapath-windows/ovsext/Actions.c ++++ b/datapath-windows/ovsext/Actions.c +@@ -1526,10 +1526,8 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext, + break; + } + case OVS_ACTION_ATTR_SAMPLE: +- break; +- case OVS_ACTION_ATTR_UNSPEC: +- case __OVS_ACTION_ATTR_MAX: + default: ++ status = NDIS_STATUS_NOT_SUPPORTED; + break; + } + } +diff --git a/datapath-windows/ovsext/Netlink/NetlinkError.h b/datapath-windows/ovsext/Netlink/NetlinkError.h +index c41414c..827fa8c 100644 +--- a/datapath-windows/ovsext/Netlink/NetlinkError.h ++++ b/datapath-windows/ovsext/Netlink/NetlinkError.h +@@ -198,3 +198,20 @@ typedef enum _NL_ERROR_ + /*the operation would block */ + NL_ERROR_WOULDBLOCK = ((ULONG)-140), + } NL_ERROR; ++ ++static __inline ++NlMapStatusToNlErr(NTSTATUS status) ++{ ++ NL_ERROR ret = NL_ERROR_SUCCESS; ++ ++ switch (status) ++ { ++ case STATUS_NOT_SUPPORTED: ++ ret = NL_ERROR_NOTSUPP; ++ break; ++ default: ++ break; ++ } ++ ++ return ret; ++} +diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c +index 8400b83..3093ba6 100644 +--- a/datapath-windows/ovsext/User.c ++++ b/datapath-windows/ovsext/User.c +@@ -369,6 +369,7 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + + status = OvsExecuteDpIoctl(&execute); + ++ /* Default reply that we want to send */ + if (status == STATUS_SUCCESS) { + NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, + usrParamsCtx->outputLength); +@@ -382,20 +383,23 @@ OvsNlExecuteCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + if (status == STATUS_SUCCESS) { + *replyLen = msgOut->nlMsg.nlmsgLen; + } +- } +- +- /* As of now there are no transactional errors in the implementation. +- * Once we have them then we need to map status to correct +- * nlError value, so that below mentioned code gets hit. */ +- if ((nlError != NL_ERROR_SUCCESS) && +- (usrParamsCtx->outputBuffer)) { +- +- POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) +- usrParamsCtx->outputBuffer; +- BuildErrorMsg(msgIn, msgError, nlError); +- *replyLen = msgError->nlMsg.nlmsgLen; +- status = STATUS_SUCCESS; +- goto done; ++ } else { ++ /* Map NTSTATUS to NL_ERROR */ ++ nlError = NlMapStatusToNlErr(status); ++ ++ /* As of now there are no transactional errors in the implementation. ++ * Once we have them then we need to map status to correct ++ * nlError value, so that below mentioned code gets hit. */ ++ if ((nlError != NL_ERROR_SUCCESS) && ++ (usrParamsCtx->outputBuffer)) { ++ ++ POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) ++ usrParamsCtx->outputBuffer; ++ BuildErrorMsg(msgIn, msgError, nlError); ++ *replyLen = msgError->nlMsg.nlmsgLen; ++ status = STATUS_SUCCESS; ++ goto done; ++ } + } + + done: +@@ -487,7 +491,11 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute) + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + } + if (ndisStatus != NDIS_STATUS_SUCCESS) { +- status = STATUS_UNSUCCESSFUL; ++ if (ndisStatus == NDIS_STATUS_NOT_SUPPORTED) { ++ status = STATUS_NOT_SUPPORTED; ++ } else { ++ status = STATUS_UNSUCCESSFUL; ++ } + } + + if (pNbl) { +-- +1.9.1 + -- 1.9.1 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
