hi Ankur,
Changes look good. I had some minor comments. You can get rid of the unused
parameters in OvsGetFlowIoctl(). The code has diverged quite a bit from the
non-NL interface now.
Acked-by: Nithin Raju <nit...@vmware.com>
On Sep 29, 2014, at 3:34 PM, Ankur Sharma <ankursha...@vmware.com> wrote:
> In this patch we have implemented the flow get.
>
> Signed-off-by: Ankur Sharma <ankursha...@vmware.com>
> ---
> datapath-windows/ovsext/Flow.c | 200 +++++++++++++++++++++++++++++++++++++----
> 1 file changed, 184 insertions(+), 16 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
> index e818e23..f6ddf7e 100644
> --- a/datapath-windows/ovsext/Flow.c
> +++ b/datapath-windows/ovsext/Flow.c
> @@ -53,7 +53,7 @@ static NTSTATUS _MapNlToFlowPut(POVS_MESSAGE msgIn,
> PNL_ATTR keyAttr,
> OvsFlowPut *mappedFlow);
> static VOID _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
> PNL_ATTR *tunnelAttrs,
> - OvsFlowPut *mappedFlow);
> + OvsFlowKey *destKey);
>
> static VOID _MapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
> PNL_ATTR *tunnelAttrs,
> @@ -359,19 +359,147 @@ OvsFlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT
> usrParamsCtx,
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _FlowNlGetCmdHandler --
> + * Handler for OVS_FLOW_CMD_GET command.
> +
> *----------------------------------------------------------------------------
> + */
> NTSTATUS
> _FlowNlGetCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> UINT32 *replyLen)
> {
> NTSTATUS rc = STATUS_SUCCESS;
> + UINT32 temp = 0; /* To keep compiler happy for calling OvsGetDumpFlow
> */
> +
> + POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)
> + (usrParamsCtx->ovsInstance);
> + POVS_MESSAGE msgIn = instance->dumpState.ovsMsg;
> + PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg);
> + POVS_HDR ovsHdr = &(msgIn->ovsHdr);
> + PNL_MSG_HDR nlMsgOutHdr = NULL;
> + UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN;
> + PNL_ATTR nlAttrs[__OVS_FLOW_ATTR_MAX];
>
> - UNREFERENCED_PARAMETER(usrParamsCtx);
> + OvsFlowGetInput getInput;
> + OvsFlowGetOutput getOutput;
> + NL_BUFFER nlBuf;
> + PNL_ATTR keyAttrs[__OVS_KEY_ATTR_MAX];
> + PNL_ATTR tunnelAttrs[__OVS_TUNNEL_KEY_ATTR_MAX];
> +
> + NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
> + usrParamsCtx->outputLength);
> + RtlZeroMemory(&getInput, sizeof(OvsFlowGetInput));
> + RtlZeroMemory(&getOutput, sizeof(OvsFlowGetOutput));
> + UINT32 keyAttrOffset = 0;
> + UINT32 tunnelKeyAttrOffset = 0;
>
> *replyLen = 0;
>
> + if (usrParamsCtx->inputLength > usrParamsCtx->outputLength) {
> + /* Should not be the case.
> + * We'll be copying the flow keys back from
> + * input buffer to output buffer. */
> + OVS_LOG_ERROR("inputLength: %d GREATER THEN outputLength: %d",
> + usrParamsCtx->inputLength, usrParamsCtx->outputLength);
We should be setting the 'rc' to STATUS_INVALID_PARAMETER or
STATUS_INSUFFICIENT_LENGTH.
> + goto done;
> + }
> +
> + /* Get all the top level Flow attributes */
> + if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrsLen(nlMsgHdr),
> + nlFlowPolicy, nlAttrs, ARRAY_SIZE(nlAttrs)))
> + != TRUE) {
> + OVS_LOG_ERROR("Attr Parsing failed for msg: %p",
> + nlMsgHdr);
> + rc = STATUS_UNSUCCESSFUL;
> + goto done;
> + }
> +
> + keyAttrOffset = (UINT32)((PCHAR) nlAttrs[OVS_FLOW_ATTR_KEY] -
> + (PCHAR)nlMsgHdr);
> +
> + /* Get flow keys attributes */
> + if ((NlAttrParseNested(nlMsgHdr, keyAttrOffset,
> + NlAttrLen(nlAttrs[OVS_FLOW_ATTR_KEY]),
> + nlFlowKeyPolicy, keyAttrs, ARRAY_SIZE(keyAttrs)))
> + != TRUE) {
> + OVS_LOG_ERROR("Key Attr Parsing failed for msg: %p",
> + nlMsgHdr);
> + rc = STATUS_UNSUCCESSFUL;
> + goto done;
> + }
> +
> + if (keyAttrs[OVS_KEY_ATTR_TUNNEL]) {
> + tunnelKeyAttrOffset = (UINT32)((PCHAR)
> + (keyAttrs[OVS_KEY_ATTR_TUNNEL])
> + - (PCHAR)nlMsgHdr);
> +
> + OVS_LOG_ERROR("Parse Flow Tunnel Key Policy");
This is not OVS_LOG_ERROR I suppose.
> +
> + /* Get tunnel keys attributes */
> + if ((NlAttrParseNested(nlMsgHdr, tunnelKeyAttrOffset,
> + NlAttrLen(keyAttrs[OVS_KEY_ATTR_TUNNEL]),
> + nlFlowTunnelKeyPolicy,
> + tunnelAttrs, ARRAY_SIZE(tunnelAttrs)))
> + != TRUE) {
> + OVS_LOG_ERROR("Tunnel key Attr Parsing failed for msg: %p",
> + nlMsgHdr);
> + rc = STATUS_UNSUCCESSFUL;
> + goto done;
> + }
> + }
> +
> + _MapKeyAttrToFlowPut(keyAttrs, tunnelAttrs,
> + &(getInput.key));
> +
> + getInput.dpNo = ovsHdr->dp_ifindex;
> + getInput.getFlags = FLOW_GET_STATS | FLOW_GET_ACTIONS;
> +
> + /* 4th argument is a no op.
> + * We are keeping this argument to be compatible
> + * with our dpif-windows based interface. */
> + rc = OvsGetFlowIoctl(&getInput, sizeof(OvsFlowGetInput), &getOutput,
> + usrParamsCtx->outputLength, &temp);
I think we can nuke the 4th argument. The code has kind of diverged a lot. I
think we can nuke arg #2, and Arg #4 and Arg #5.
> + if (rc != STATUS_SUCCESS) {
> + OVS_LOG_ERROR("OvsGetFlowIoctl failed.");
> + goto done;
> + }
> +
> + /* Lets prepare the reply. */
> + nlMsgOutHdr = (PNL_MSG_HDR)(NlBufAt(&nlBuf, 0, 0));
> +
> + /* Input already has all the attributes for the flow key.
> + * Lets copy the values back. */
> + RtlCopyMemory(usrParamsCtx->outputBuffer, usrParamsCtx->inputBuffer,
> + usrParamsCtx->inputLength);
> +
> + rc = _MapFlowStatsToNlStats(&nlBuf, &((getOutput.info).stats));
> + if (rc != STATUS_SUCCESS) {
> + OVS_LOG_ERROR("_OvsFlowMapFlowKeyToNlStats failed.");
> + goto done;
> + }
> +
> + rc = _MapFlowActionToNlAction(&nlBuf, ((getOutput.info).actionsLen),
> + getOutput.info.actions);
> + if (rc != STATUS_SUCCESS) {
> + OVS_LOG_ERROR("_MapFlowActionToNlAction failed.");
> + goto done;
> + }
> +
> + NlMsgSetSize(nlMsgOutHdr, NlBufSize(&nlBuf));
> + NlMsgAlignSize(nlMsgOutHdr);
> + *replyLen += NlMsgSize(nlMsgOutHdr);
> +
> +done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _FlowNlDumpCmdHandler --
> + * Handler for OVS_FLOW_CMD_GET command.
> +
> *----------------------------------------------------------------------------
> + */
> NTSTATUS
> _FlowNlDumpCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
> UINT32 *replyLen)
> @@ -479,6 +607,12 @@ done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowInfoToNl --
> + * Maps OvsFlowInfo to Netlink attributes.
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowInfoToNl(PNL_BUFFER nlBuf, OvsFlowInfo *flowInfo)
> {
> @@ -507,6 +641,12 @@ done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowStatsToNlStats --
> + * Maps OvsFlowStats to OVS_FLOW_ATTR_STATS attribute.
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowStatsToNlStats(PNL_BUFFER nlBuf, OvsFlowStats *flowStats)
> {
> @@ -541,6 +681,12 @@ done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowActionToNlAction --
> + * Maps flow actions to OVS_FLOW_ATTR_ACTION attribute.
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowActionToNlAction(PNL_BUFFER nlBuf, uint32_t actionsLen,
> PNL_ATTR actions)
> @@ -570,6 +716,12 @@ error_nested_start:
>
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowKeyToNlKey --
> + * Maps OvsFlowKey to OVS_FLOW_ATTR_KEY attribute.
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowKeyToNlKey(PNL_BUFFER nlBuf, OvsFlowKey *flowKey)
> {
> @@ -665,6 +817,12 @@ error_nested_start:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowTunKeyToNlKey --
> + * Maps OvsIPv4TunnelKey to OVS_TUNNEL_KEY_ATTR_ID attribute.
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowTunKeyToNlKey(PNL_BUFFER nlBuf, OvsIPv4TunnelKey *tunKey)
> {
> @@ -720,6 +878,12 @@ error_nested_start:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowTunKeyToNlKey --
> + * Maps OvsIPv4FlowPutKey to OVS_KEY_ATTR_IPV4 attribute.
s/OvsIPv4FlowPutKey/IpKey
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowIpv4KeyToNlKey(PNL_BUFFER nlBuf, IpKey *ipv4FlowPutKey)
> {
> @@ -808,6 +972,12 @@ done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowIpv6KeyToNlKey --
> + * Maps _MapFlowIpv6KeyToNlKey to OVS_KEY_ATTR_IPV6 attribute.
s/_MapFlowIpv6KeyToNlKey/Ipv6Key
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowIpv6KeyToNlKey(PNL_BUFFER nlBuf, Ipv6Key *ipv6FlowPutKey,
> Icmp6Key *icmpv6FlowPutKey)
> @@ -918,6 +1088,12 @@ done:
> return rc;
> }
>
> +/*
> +
> *----------------------------------------------------------------------------
> + * _MapFlowArpKeyToNlKey --
> + * Maps _MapFlowArpKeyToNlKey to OVS_KEY_ATTR_ARP attribute.
s/_MapFlowArpKeyToNlKey/ArpKey
> +
> *----------------------------------------------------------------------------
> + */
> static NTSTATUS
> _MapFlowArpKeyToNlKey(PNL_BUFFER nlBuf, ArpKey *arpFlowPutKey)
> {
> @@ -985,7 +1161,8 @@ _MapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR keyAttr,
>
> /* Get tunnel keys attributes */
> if ((NlAttrParseNested(nlMsgHdr, tunnelKeyAttrOffset,
> - NlAttrLen(keyAttr), nlFlowTunnelKeyPolicy,
> + NlAttrLen(keyAttrs[OVS_KEY_ATTR_TUNNEL]),
> + nlFlowTunnelKeyPolicy,
> tunnelAttrs, ARRAY_SIZE(tunnelAttrs)))
> != TRUE) {
> OVS_LOG_ERROR("Tunnel key Attr Parsing failed for msg: %p",
> @@ -996,7 +1173,7 @@ _MapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR keyAttr,
> }
>
> _MapKeyAttrToFlowPut(keyAttrs, tunnelAttrs,
> - mappedFlow);
> + &(mappedFlow->key));
>
> /* Map the action */
> if (actionAttr) {
> @@ -1055,10 +1232,9 @@ _MapNlToFlowPutFlags(PGENL_MSG_HDR genlMsgHdr,
> static VOID
> _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
> PNL_ATTR *tunnelAttrs,
> - OvsFlowPut *mappedFlow)
> + OvsFlowKey *destKey)
> {
> const struct ovs_key_ethernet *eth_key;
> - OvsFlowKey *destKey = &(mappedFlow->key);
>
> _MapTunAttrToFlowPut(keyAttrs, tunnelAttrs, destKey);
>
> @@ -2095,21 +2271,13 @@ OvsGetFlowIoctl(PVOID inputBuffer,
> UINT32 dpNo;
> LOCK_STATE_EX dpLockState;
>
> - if (inputLength != sizeof(OvsFlowGetInput)
> - || inputBuffer == NULL) {
> - return STATUS_INFO_LENGTH_MISMATCH;
> - }
> + UNREFERENCED_PARAMETER(inputLength);
>
> getInput = (OvsFlowGetInput *) inputBuffer;
> getFlags = getInput->getFlags;
> getActionsLen = getInput->actionsLen;
> - if (getInput->getFlags & FLOW_GET_KEY) {
> - return STATUS_INVALID_PARAMETER;
> - }
>
> - if (outputBuffer == NULL
> - || outputLength != (sizeof *getOutput +
> - getInput->actionsLen)) {
> + if (outputBuffer == NULL) {
> return STATUS_INFO_LENGTH_MISMATCH;
> }
>
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@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=5CgYuhwg3C7QDyrP7CnzkLQrn8mbY1GGvMwlLNSqSyA%3D%0A&s=629a4a102d4ffffe32375596885a7f692cfe54369d22679813a34545472ede8c
Thanks,
-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
