From patchwork Thu Sep 17 15:01:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alin Serdean X-Patchwork-Id: 518908 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (li376-54.members.linode.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 9A9B81414AB for ; Fri, 18 Sep 2015 01:05:16 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 82B9610B5F; Thu, 17 Sep 2015 08:05:15 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v1.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id E4BB8109EA for ; Thu, 17 Sep 2015 08:05:13 -0700 (PDT) Received: from bar3.cudamail.com (bar1 [192.168.15.1]) by mx3v1.cudamail.com (Postfix) with ESMTP id 50FED618FDE for ; Thu, 17 Sep 2015 09:05:13 -0600 (MDT) X-ASG-Debug-ID: 1442502284-03dd7b2d0a06990001-byXFYA Received: from mx3-pf2.cudamail.com ([192.168.14.1]) by bar3.cudamail.com with ESMTP id H8hnshenMvDnid85 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 17 Sep 2015 09:04:44 -0600 (MDT) X-Barracuda-Envelope-From: aserdean@cloudbasesolutions.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.1 Received: from unknown (HELO cbssmtp1.cloudbase.local) (91.232.152.5) by mx3-pf2.cudamail.com with SMTP; 17 Sep 2015 15:02:59 -0000 Received-SPF: pass (mx3-pf2.cudamail.com: SPF record at cloudbasesolutions.com designates 91.232.152.5 as permitted sender) X-Barracuda-Apparent-Source-IP: 91.232.152.5 X-Barracuda-RBL-IP: 91.232.152.5 Received: from localhost (localhost [127.0.0.1]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id 925AB42022 for ; Thu, 17 Sep 2015 18:03:00 +0300 (EEST) X-Virus-Scanned: amavisd-new at cloudbasesolutions.com Received: from cbssmtp1.cloudbase.local ([127.0.0.1]) by localhost (cbssmtp1.cloudbase.local [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Hdn3hkuhHcOW for ; Thu, 17 Sep 2015 18:02:40 +0300 (EEST) Received: from CBSEX1.cloudbase.local (unknown [10.77.78.3]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id ED3834200E for ; Thu, 17 Sep 2015 18:01:48 +0300 (EEST) Received: from CBSEX1.cloudbase.local ([10.77.78.3]) by CBSEX1.cloudbase.local ([10.77.78.3]) with mapi id 14.03.0224.002; Thu, 17 Sep 2015 17:01:48 +0200 X-CudaMail-Envelope-Sender: aserdean@cloudbasesolutions.com From: Alin Serdean To: "dev@openvswitch.org" X-CudaMail-MID: CM-V2-916025511 X-CudaMail-DTE: 091715 X-CudaMail-Originating-IP: 91.232.152.5 Thread-Topic: [PATCH v2] datapath-windows: Append flow attribute key X-ASG-Orig-Subj: [##CM-V2-916025511##][PATCH v2] datapath-windows: Append flow attribute key Thread-Index: AQHQ8VnGrOdER2JYGEqy2Rsce9qQFQ== Date: Thu, 17 Sep 2015 15:01:48 +0000 Message-ID: <1442502114-10716-1-git-send-email-aserdean@cloudbasesolutions.com> Accept-Language: en-US, it-IT Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.77.78.1] MIME-Version: 1.0 X-GBUdb-Analysis: 0, 91.232.152.5, Ugly c=0.25607 p=-0.333333 Source Normal X-MessageSniffer-Rules: 0-0-0-17191-c X-Barracuda-Connect: UNKNOWN[192.168.14.1] X-Barracuda-Start-Time: 1442502284 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.10 X-Barracuda-Spam-Status: No, SCORE=0.10 using per-user scores of TAG_LEVEL=3.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests=RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.22633 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS Subject: [ovs-dev] [PATCH v2] datapath-windows: Append flow attribute key X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" Currently when running the vswitch daemon we get a lot of messages of the form: 2015-09-10T23:04:21Z|07255|dpif(revalidator11)|WARN|system@ovs-system: failed to flow_del (Invalid argument). The userspace expects after sending a delete flow command, to receive the flow key of the deleted flow. Currently we only send back the statiscs. This patch appends back the flow key attribute for to the response buffer for the flow commands new, modify and delete. This patch also responds to the userspace with ENOENT in the case the flow was not modified, deleted, created or retrieved. Also incorporate some refactors. Signed-off-by: Alin Gabriel Serdean Acked-by: Sorin Vinturis Acked-by: Sairam Venugopal --- This patch is intended for branch-2.4 as well. v2 address comments, add acks --- datapath-windows/ovsext/Flow.c | 60 +++++++++++++++++++++---------- datapath-windows/ovsext/Netlink/Netlink.c | 4 ++- datapath-windows/ovsext/Netlink/Netlink.h | 4 +-- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index 58bdd42..32cb086 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -312,7 +312,6 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, if (flowAttrs[OVS_FLOW_ATTR_PROBE]) { OVS_LOG_ERROR("Attribute OVS_FLOW_ATTR_PROBE not supported"); - nlError = NL_ERROR_NOENT; goto done; } @@ -328,6 +327,11 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, &stats); if (rc != STATUS_SUCCESS) { OVS_LOG_ERROR("OvsPutFlowIoctl failed."); + /* + * Report back to the userspace the flow could not be modified, + * created or deleted + */ + nlError = NL_ERROR_NOENT; goto done; } @@ -350,6 +354,15 @@ OvsFlowNlCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, rc = STATUS_SUCCESS; } + /* Append OVS_FLOW_ATTR_KEY attribute. This is need i.e. for flow delete*/ + if (!NlMsgPutNested(&nlBuf, OVS_FLOW_ATTR_KEY, + NlAttrData(flowAttrs[OVS_FLOW_ATTR_KEY]), + NlAttrGetSize(flowAttrs[OVS_FLOW_ATTR_KEY]))) { + OVS_LOG_ERROR("Adding OVS_FLOW_ATTR_KEY attribute failed."); + rc = STATUS_INVALID_BUFFER_SIZE; + goto done; + } + /* Append OVS_FLOW_ATTR_STATS attribute */ if (!NlMsgPutTailUnspec(&nlBuf, OVS_FLOW_ATTR_STATS, (PCHAR)(&replyStats), sizeof(replyStats))) { @@ -385,29 +398,13 @@ OvsFlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, UINT32 *replyLen) { NTSTATUS status = STATUS_SUCCESS; - NL_ERROR nlError = NL_ERROR_SUCCESS; - POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer; if (usrParamsCtx->devOp == OVS_TRANSACTION_DEV_OP) { status = _FlowNlGetCmdHandler(usrParamsCtx, replyLen); - - /* No trasanctional errors as of now. - * If we have something then we need to convert rc to - * nlError. */ - if ((nlError != NL_ERROR_SUCCESS) && - (usrParamsCtx->outputBuffer)) { - POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) - usrParamsCtx->outputBuffer; - NlBuildErrorMsg(msgIn, msgError, nlError); - *replyLen = msgError->nlMsg.nlmsgLen; - status = STATUS_SUCCESS; - goto done; - } } else { status = _FlowNlDumpCmdHandler(usrParamsCtx, replyLen); } -done: return status; } @@ -442,6 +439,7 @@ _FlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, UINT32 keyAttrOffset = 0; UINT32 tunnelKeyAttrOffset = 0; BOOLEAN ok; + NL_ERROR nlError = NL_ERROR_SUCCESS; if (usrParamsCtx->inputLength > usrParamsCtx->outputLength) { /* Should not be the case. @@ -510,6 +508,10 @@ _FlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, rc = OvsGetFlowIoctl(&getInput, &getOutput); if (rc != STATUS_SUCCESS) { OVS_LOG_ERROR("OvsGetFlowIoctl failed."); + /* + * Report back to the userspace the flow could not be found + */ + nlError = NL_ERROR_NOENT; goto done; } @@ -543,6 +545,14 @@ _FlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, *replyLen += NlMsgSize(nlMsgOutHdr); done: + if (nlError != NL_ERROR_SUCCESS) { + POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) + usrParamsCtx->outputBuffer; + NlBuildErrorMsg(msgIn, msgError, nlError); + *replyLen = msgError->nlMsg.nlmsgLen; + rc = STATUS_SUCCESS; + } + return rc; } @@ -558,9 +568,10 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, { NTSTATUS rc = STATUS_SUCCESS; UINT32 temp = 0; /* To keep compiler happy for calling OvsDoDumpFlows */ - + NL_ERROR nlError = NL_ERROR_SUCCESS; POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE) (usrParamsCtx->ovsInstance); + POVS_MESSAGE msgIn = instance->dumpState.ovsMsg; if (usrParamsCtx->devOp == OVS_WRITE_DEV_OP) { /* Dump Start */ @@ -568,7 +579,6 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, goto done; } - POVS_MESSAGE msgIn = instance->dumpState.ovsMsg; PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg); PGENL_MSG_HDR genlMsgHdr = &(msgIn->genlMsg); POVS_HDR ovsHdr = &(msgIn->ovsHdr); @@ -600,6 +610,10 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, rc = OvsDoDumpFlows(&dumpInput, &dumpOutput, &temp); if (rc != STATUS_SUCCESS) { OVS_LOG_ERROR("OvsDoDumpFlows failed with rc: %d", rc); + /* + * Report back to the userspace the flows could not be found + */ + nlError = NL_ERROR_NOENT; break; } @@ -669,6 +683,14 @@ _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, } while(TRUE); done: + if (nlError != NL_ERROR_SUCCESS) { + POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) + usrParamsCtx->outputBuffer; + NlBuildErrorMsg(msgIn, msgError, nlError); + *replyLen = msgError->nlMsg.nlmsgLen; + rc = STATUS_SUCCESS; + } + return rc; } diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c index a66fb38..e2312da 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.c +++ b/datapath-windows/ovsext/Netlink/Netlink.c @@ -560,7 +560,7 @@ NlMsgEndNested(PNL_BUFFER buf, UINT32 offset) * Refer nl_msg_put_nested for more details. * -------------------------------------------------------------------------- */ -VOID +BOOLEAN NlMsgPutNested(PNL_BUFFER buf, UINT16 type, const PVOID data, UINT32 size) { @@ -574,6 +574,8 @@ NlMsgPutNested(PNL_BUFFER buf, UINT16 type, ASSERT(ret); NlMsgEndNested(buf, offset); + + return ret; } /* Accessing netlink message payload */ diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h index a520ccf..d270737 100644 --- a/datapath-windows/ovsext/Netlink/Netlink.h +++ b/datapath-windows/ovsext/Netlink/Netlink.h @@ -203,8 +203,8 @@ BOOLEAN NlMsgPutHeadU64(PNL_BUFFER buf, UINT16 type, UINT64 value); BOOLEAN NlMsgPutHeadString(PNL_BUFFER buf, UINT16 type, PCHAR value); UINT32 NlMsgStartNested(PNL_BUFFER buf, UINT16 type); VOID NlMsgEndNested(PNL_BUFFER buf, UINT32 offset); -VOID NlMsgPutNested(PNL_BUFFER buf, UINT16 type, - const PVOID data, UINT32 size); +BOOLEAN NlMsgPutNested(PNL_BUFFER buf, UINT16 type, + const PVOID data, UINT32 size); /* These variants are convenient for iterating nested attributes. */ #define NL_NESTED_FOR_EACH(ITER, LEFT, A) \