Hi Sam,
Thanks a lot for the review.
Please find my replies inline.
Thanks.
Regards,
Ankur
________________________________________
From: Samuel Ghinet <sghi...@cloudbasesolutions.com>
Sent: Thursday, September 25, 2014 9:14 AM
To: dev@openvswitch.org
Cc: Ankur Sharma
Subject: RE: [ovs-dev] [PATCH v1 07/10] datapath-windows/Flow.c: FLOW_NEW
command handler.
Hey Ankur,
> + if (keyAttrs[OVS_KEY_ATTR_TUNNEL]) {
> + destKey->tunKey.tunnelId = NlAttrGetU64
> + (tunAttrs[OVS_TUNNEL_KEY_ATTR_ID]);
> + destKey->tunKey.dst = NlAttrGetU32
> + (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_DST]);
> + destKey->tunKey.src = NlAttrGetU32
> + (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_SRC]);
> +
> + /* XXX: There are no attributes to specify tunnel flags.
> + * Also, i do not see these flags being used anywhere in
> + * flowPut code. */
> + destKey->tunKey.flags = 0;
> + destKey->tunKey.tos = NlAttrGetU8
Those flags are actually necessary.
The flag attributes are (OvsDpInterface.h):
[CODE]
OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT, /* No argument, set DF. */
OVS_TUNNEL_KEY_ATTR_CSUM, /* No argument. CSUM packet. */
OVS_TUNNEL_KEY_ATTR_OAM, /* No argument, OAM frame. */
[/CODE]
The tunnelKey must OR together the flags, based on the attributes.
Each attribute that is a flag does not have a length, nor payload, as I
remember. If the attribute flag appears, it means you must take it as set. If
the flag attribute does not appear, you must take it as not set.
The corresponding flags are:
[CODE]
/* Flags for tunneling */
#define OVS_TNL_F_DONT_FRAGMENT (1 << 0)
#define OVS_TNL_F_CSUM (1 << 1)
#define OVS_TNL_F_KEY (1 << 2)
[/CODE]
[ANKUR]: Sure i'll revisit this.
Also,
_OvsFlowMapTunAttrToFlowPut:
> + destKey->tunKey.tunnelId = NlAttrGetU64
> + (tunAttrs[OVS_TUNNEL_KEY_ATTR_ID]);
This will crash if the userspace did not give you a tunnel attribute "tunnel
id". And you also have it specified as "optional" in the policy.
[ANKUR]: I am planning to add an additional check, before calling NlAttrGet*
APIs.
OvsFlowMapKeyAttrToFlowPut:
> destKey->l2.inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]);
The same here.
> eth_key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ETHERNET]);
and here. It is possible that the eth key is required. Or that it is optional
(I think in ovs 1.11 was required, but in 2.3, when masks were allowed, was
optional - experience with our implementation of the driver)
[ANKUR]: I am planning to add an additional check, before calling NlAttrGet*
APIs.
> case ETH_TYPE_ARP:
> case ETH_TYPE_RARP: {
I really believe RARP is no longer used, since some long time ago.
[ANKUR]: Sure, will be removed.
> case ETH_TYPE_IPV4: {
> const struct ovs_key_ipv4 *ipv4Key;
> const struct ovs_key_tcp *tcpKey;
>
> ipv4Key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_IPV4]);
It is possible that the eth type attribute was given "ipv4" but the ipv4 key
was not given. When key masks are allowed, this can happen if the ipv4 key is
considered to be "wildcard" so it is not given as a key, nor mask, at all.
[ANKUR]: I did not consider masking as of now. But to be on safer side i'll add
additional check in v3.
> if (keyAttrs[OVS_KEY_ATTR_TCP]) {
> tcpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP]);
> ipv4FlowPutKey->l4.tpSrc = tcpKey->tcp_src;
> ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
> }
Where is tcp flags set?
[ANKUR]: My bad i missed this, will be fixed in v3.
> arpFlowPutKey->pad[0] = 0;
> arpFlowPutKey->pad[1] = 0;
> arpFlowPutKey->pad[2] = 0;
> destKey->l2.keyLen += OVS_ARP_KEY_SIZE;
What I particularly don't like about the old approach with flows is this
somewhat crypted text.
I wished there was at least some documentation on structs like this:
[CODE]
typedef struct L2Key {
uint32_t inPort; /* Port number of input port. */
union {
struct {
uint16_t offset;
uint16_t keyLen;
};
uint32_t val;
};
[/CODE]
on what the fields are and how they are used.
[ANKUR]: hmm, let me see i can do something about it in this series.
Also,
_OvsFlowMapKeyAttrToFlowPut,
_OvsFlowMapTunAttrToFlowPut,
_OvsFlowMapNlToFlowPutFlags,
_OvsFlowMapNlToFlowPut
The names are very long and very similar to one another. It is very easy to
confuse one with the other, when reading.
Perhaps something like _KeyMapToFlowPut, _FlowMapToFlowPut, _FlowMapToFlags.
Or perhaps use a "FP" for FlowPut, and document this shorthand.
[Ankur]: Sure, i'll improve the function names.
> eth_key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ETHERNET]);
> memcpy(destKey->l2.dlSrc, eth_key->eth_src, ETH_ADDR_LEN);
> memcpy(destKey->l2.dlDst, eth_key->eth_dst, ETH_ADDR_LEN);
For those who prefer the Rtl* style, there is RtlCopyMemory :)
[Ankur]: Sure, will be done.
Regards,
Sam
________________________________________
Date: Wed, 24 Sep 2014 00:15:41 -0700
From: Ankur Sharma <ankursha...@vmware.com>
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 07/10] datapath-windows/Flow.c: FLOW_NEW
command handler.
Message-ID: <1411542944-19374-7-git-send-email-ankursha...@vmware.com>
This patch covers the changes needed to support FLOW_NEW command.
API _OvsFlowMapNlToFlowPutFlags has a bug, which will be fixed
with the patches for FLOW_DEL.
---
datapath-windows/include/OvsPub.h | 2 +-
datapath-windows/ovsext/Flow.c | 384 +++++++++++++++++++++++++++++++++++---
datapath-windows/ovsext/Flow.h | 6 +-
3 files changed, 366 insertions(+), 26 deletions(-)
diff --git a/datapath-windows/include/OvsPub.h
b/datapath-windows/include/OvsPub.h
index 36814c4..fa1d6d4 100644
--- a/datapath-windows/include/OvsPub.h
+++ b/datapath-windows/include/OvsPub.h
@@ -420,7 +420,7 @@ typedef struct OvsFlowPut {
uint32_t actionsLen;
OvsFlowKey key;
uint32_t flags;
- NL_ATTR actions[0]; /* Variable length indicated by actionsLen. */
+ PNL_ATTR actions;
} OvsFlowPut;
#define OVS_MIN_PACKET_SIZE 60
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 25b39c1..e170de6 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -46,6 +46,20 @@ static VOID DeleteAllFlows(OVS_DATAPATH *datapath);
static NTSTATUS AddFlow(OVS_DATAPATH *datapath, OvsFlow *flow);
static VOID FreeFlow(OvsFlow *flow);
static VOID __inline *GetStartAddrNBL(const NET_BUFFER_LIST *_pNB);
+static NTSTATUS _OvsFlowMapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR keyAttr,
+ PNL_ATTR actionAttr,
+ PNL_ATTR flowAttrClear,
+ OvsFlowPut *mappedFlow);
+static VOID _OvsFlowMapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
+ PNL_ATTR *tunnelAttrs,
+ OvsFlowPut *mappedFlow);
+
+static VOID _OvsFlowMapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
+ PNL_ATTR *tunnelAttrs,
+ OvsFlowKey *destKey);
+static VOID _OvsFlowMapNlToFlowPutFlags(PGENL_MSG_HDR genlMsgHdr,
+ PNL_ATTR flowAttrClear,
+ OvsFlowPut *mappedFlow);
#define OVS_FLOW_TABLE_SIZE 2048
#define OVS_FLOW_TABLE_MASK (OVS_FLOW_TABLE_SIZE -1)
@@ -191,20 +205,365 @@ static const NL_POLICY nlFlowActionPolicy[] = {
* Netlink interface for flow commands.
*----------------------------------------------------------------------------
*/
+
+/*
+ *----------------------------------------------------------------------------
+ * OvsFlowNlNewCmdHandler --
+ * Handler for OVS_FLOW_CMD_NEW command.
+ *----------------------------------------------------------------------------
+ */
NTSTATUS
OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
UINT32 *replyLen)
{
NTSTATUS rc = 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_FLOW_ATTR_MAX];
+ UINT32 attrOffset = NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN;
+ POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)
+ usrParamsCtx->ovsInstance;
+ OvsFlowPut mappedFlow;
+ OvsFlowStats stats;
+ struct ovs_flow_stats replyStats;
+
+ NL_BUFFER nlBuf;
+
+ UNREFERENCED_PARAMETER(genlMsgHdr);
+ UNREFERENCED_PARAMETER(ovsHdr);
+ UNREFERENCED_PARAMETER(instance);
+
+ RtlZeroMemory(&mappedFlow, sizeof(OvsFlowPut));
+ RtlZeroMemory(&stats, sizeof(stats));
+ RtlZeroMemory(&replyStats, sizeof(replyStats));
+
+ *replyLen = 0;
+
+ /* Get all the top level Flow attributes */
+ if ((NlAttrParse(nlMsgHdr, attrOffset, NlMsgAttrLen(nlMsgHdr),
+ nlFlowPolicy, nlAttrs, ARRAY_SIZE(nlAttrs)))
+ != TRUE) {
+ OVS_LOG_ERROR("Attr Parsing failed for msg: %p",
+ nlMsgHdr);
+ rc = NDIS_STATUS_FAILURE;
+ goto done;
+ }
+
+ if ((_OvsFlowMapNlToFlowPut(msgIn, nlAttrs[OVS_FLOW_ATTR_KEY],
+ nlAttrs[OVS_FLOW_ATTR_ACTIONS], nlAttrs[OVS_FLOW_ATTR_CLEAR],
+ &mappedFlow))
+ != NDIS_STATUS_SUCCESS) {
+ OVS_LOG_ERROR("Conversion to OvsFlowPut failed");
+ goto done;
+ }
+
+ rc = OvsPutFlowIoctl(&mappedFlow, sizeof (struct OvsFlowPut),
+ &stats);
+ if (rc != STATUS_SUCCESS) {
+ OVS_LOG_ERROR("OvsFlowPut failed.");
+ goto done;
+ }
+
+ if (!(usrParamsCtx->outputBuffer)) {
+ /* No output buffer */
+ OVS_LOG_ERROR("outputBuffer NULL.");
+ goto done;
+ }
+
+ replyStats.n_packets = stats.packetCount;
+ replyStats.n_bytes = stats.byteCount;
+
+ /* So far so good. Prepare the reply for userspace */
+ NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
+ usrParamsCtx->outputLength);
+
+ /* Prepare nl Msg headers */
+ rc = NlFillOvsMsg(msgOut, &nlBuf, nlMsgHdr->nlmsgType, 0,
+ nlMsgHdr->nlmsgSeq, nlMsgHdr->nlmsgPid,
+ genlMsgHdr->cmd, OVS_FLOW_VERSION,
+ ovsHdr->dp_ifindex);
+ ASSERT(rc);
+
+ /* Append OVS_FLOW_ATTR_STATS attribute */
+ if (!NlMsgPutTailUnspec(&nlBuf, OVS_FLOW_ATTR_STATS,
+ (PCHAR)(&replyStats), sizeof(replyStats))) {
+ OVS_LOG_ERROR("Adding OVS_FLOW_ATTR_STATS attribute failed.");
+ rc = NDIS_STATUS_FAILURE;
+ }
+
+ *replyLen = msgOut->nlMsg.nlmsgLen;
+
+done:
+ return rc;
+}
+
+/*
+ *----------------------------------------------------------------------------
+ * _OvsFlowMapNlToFlowPut --
+ * Maps input netlink message to OvsFlowPut.
+ *----------------------------------------------------------------------------
+ */
+static NTSTATUS
+_OvsFlowMapNlToFlowPut(POVS_MESSAGE msgIn, PNL_ATTR keyAttr,
+ PNL_ATTR actionAttr, PNL_ATTR flowAttrClear,
+ OvsFlowPut *mappedFlow)
+{
+ NTSTATUS rc = NDIS_STATUS_SUCCESS;
+ PNL_MSG_HDR nlMsgHdr = &(msgIn->nlMsg);
+ PGENL_MSG_HDR genlMsgHdr = &(msgIn->genlMsg);
+ POVS_HDR ovsHdr = &(msgIn->ovsHdr);
+
+ UINT32 keyAttrOffset = (UINT32)((PCHAR)keyAttr - (PCHAR)nlMsgHdr);
+ UINT32 tunnelKeyAttrOffset;
+
+ PNL_ATTR keyAttrs[__OVS_KEY_ATTR_MAX] = {NULL};
+ PNL_ATTR tunnelAttrs[__OVS_TUNNEL_KEY_ATTR_MAX] = {NULL};
+
+ UNREFERENCED_PARAMETER(ovsHdr);
+ UNREFERENCED_PARAMETER(tunnelKeyAttrOffset);
+
+ /* Get flow keys attributes */
+ if ((NlAttrParseNested(nlMsgHdr, keyAttrOffset, NlAttrLen(keyAttr),
+ nlFlowKeyPolicy, keyAttrs, ARRAY_SIZE(keyAttrs)))
+ != TRUE) {
+ OVS_LOG_ERROR("Key Attr Parsing failed for msg: %p",
+ nlMsgHdr);
+ rc = NDIS_STATUS_FAILURE;
+ 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");
+
+ /* Get tunnel keys attributes */
+ if ((NlAttrParseNested(nlMsgHdr, tunnelKeyAttrOffset,
+ NlAttrLen(keyAttr), nlFlowTunnelKeyPolicy,
+ tunnelAttrs, ARRAY_SIZE(tunnelAttrs)))
+ != TRUE) {
+ OVS_LOG_ERROR("Tunnel key Attr Parsing failed for msg: %p",
+ nlMsgHdr);
+ rc = NDIS_STATUS_FAILURE;
+ goto done;
+ }
+ }
+
+ _OvsFlowMapKeyAttrToFlowPut(keyAttrs, tunnelAttrs,
+ mappedFlow);
+
+ /* Map the action */
+ mappedFlow->actionsLen = NlAttrGetSize(actionAttr);
+ mappedFlow->actions = NlAttrGet(actionAttr);
+ mappedFlow->dpNo = ovsHdr->dp_ifindex;
- UNREFERENCED_PARAMETER(usrParamsCtx);
- UNREFERENCED_PARAMETER(replyLen);
+ _OvsFlowMapNlToFlowPutFlags(genlMsgHdr, flowAttrClear,
+ mappedFlow);
+done:
return rc;
}
/*
*----------------------------------------------------------------------------
+ * _OvsFlowMapNlToFlowPutFlags --
+ * Maps netlink message to OvsFlowPut->flags.
+ *----------------------------------------------------------------------------
+ */
+static VOID
+_OvsFlowMapNlToFlowPutFlags(PGENL_MSG_HDR genlMsgHdr, PNL_ATTR flowAttrClear,
+ OvsFlowPut *mappedFlow)
+{
+ uint32_t flags = 0;
+
+ switch (genlMsgHdr->cmd) {
+ case OVS_FLOW_CMD_NEW:
+ flags |= OVSWIN_FLOW_PUT_CREATE;
+ break;
+ case OVS_FLOW_CMD_DEL:
+ flags |= OVSWIN_FLOW_PUT_DELETE;
+ break;
+ case OVS_FLOW_CMD_SET:
+ flags |= OVSWIN_FLOW_PUT_MODIFY;
+ break;
+ default:
+ ASSERT(0);
+ }
+
+ if (flowAttrClear) {
+ flags |= OVSWIN_FLOW_PUT_CLEAR;
+ }
+
+ mappedFlow->flags = flags;
+}
+
+/*
+ *----------------------------------------------------------------------------
+ * _OvsFlowMapKeyAttrToFlowPut --
+ * Converts FLOW_KEY attribute to OvsFlowPut->key.
+ *----------------------------------------------------------------------------
+ */
+static VOID
+_OvsFlowMapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
+ PNL_ATTR *tunnelAttrs,
+ OvsFlowPut *mappedFlow)
+{
+ const struct ovs_key_ethernet *eth_key;
+ OvsFlowKey *destKey = &(mappedFlow->key);
+
+ _OvsFlowMapTunAttrToFlowPut(keyAttrs, tunnelAttrs, destKey);
+
+ /* ===== L2 headers ===== */
+ destKey->l2.inPort = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_IN_PORT]);
+ eth_key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ETHERNET]);
+ memcpy(destKey->l2.dlSrc, eth_key->eth_src, ETH_ADDR_LEN);
+ memcpy(destKey->l2.dlDst, eth_key->eth_dst, ETH_ADDR_LEN);
+
+ destKey->l2.dlType = ntohs((NlAttrGetU16(keyAttrs
+ [OVS_KEY_ATTR_ETHERTYPE])));
+
+ if (keyAttrs[OVS_KEY_ATTR_VLAN]) {
+ destKey->l2.vlanTci = NlAttrGetU16(keyAttrs
+ [OVS_KEY_ATTR_VLAN]);
+ }
+
+ /* ==== L3 + L4. ==== */
+ destKey->l2.keyLen = OVS_WIN_TUNNEL_KEY_SIZE + OVS_L2_KEY_SIZE
+ - destKey->l2.offset;
+
+ switch (destKey->l2.dlType) {
+ case ETH_TYPE_IPV4: {
+ const struct ovs_key_ipv4 *ipv4Key;
+ const struct ovs_key_tcp *tcpKey;
+
+ ipv4Key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_IPV4]);
+
+ IpKey *ipv4FlowPutKey = &(destKey->ipKey);
+
+ ipv4FlowPutKey->nwSrc = ipv4Key->ipv4_src;
+ ipv4FlowPutKey->nwDst = ipv4Key->ipv4_dst;
+ ipv4FlowPutKey->nwProto = ipv4Key->ipv4_proto;
+ ipv4FlowPutKey->nwTos = ipv4Key->ipv4_tos;
+ ipv4FlowPutKey->nwTtl = ipv4Key->ipv4_ttl;
+ ipv4FlowPutKey->nwFrag = ipv4Key->ipv4_frag;
+
+ if (keyAttrs[OVS_KEY_ATTR_TCP]) {
+ tcpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP]);
+ ipv4FlowPutKey->l4.tpSrc = tcpKey->tcp_src;
+ ipv4FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
+ }
+
+ destKey->l2.keyLen += OVS_IP_KEY_SIZE;
+ break;
+ }
+ case ETH_TYPE_IPV6: {
+ const struct ovs_key_ipv6 *ipv6Key;
+ const struct ovs_key_tcp *tcpKey;
+
+ ipv6Key = NlAttrGet(keyAttrs[OVS_KEY_ATTR_IPV6]);
+
+ Ipv6Key *ipv6FlowPutKey = &(destKey->ipv6Key);
+
+ memcpy(&ipv6FlowPutKey->ipv6Src, ipv6Key->ipv6_src, sizeof
ipv6Key->ipv6_src);
+ memcpy(&ipv6FlowPutKey->ipv6Dst, ipv6Key->ipv6_dst, sizeof
ipv6Key->ipv6_dst);
+ ipv6FlowPutKey->ipv6Label = ipv6Key->ipv6_label;
+ ipv6FlowPutKey->nwProto = ipv6Key->ipv6_proto;
+ ipv6FlowPutKey->nwTos = ipv6Key->ipv6_tclass;
+ ipv6FlowPutKey->nwTtl = ipv6Key->ipv6_hlimit;
+
+ ipv6FlowPutKey->nwFrag = ipv6Key->ipv6_frag;
+
+ if (keyAttrs[OVS_KEY_ATTR_TCP]) {
+ tcpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_TCP]);
+ ipv6FlowPutKey->l4.tpSrc = tcpKey->tcp_src;
+ ipv6FlowPutKey->l4.tpDst = tcpKey->tcp_dst;
+ }
+
+ ipv6FlowPutKey->pad = 0;
+
+ if (ipv6Key->ipv6_proto == IPPROTO_ICMPV6) {
+ const struct ovs_key_nd *ndKey;
+ Icmp6Key *icmp6FlowPutKey= &(destKey->icmp6Key);
+
+ ndKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ND]);
+
+ memcpy(&icmp6FlowPutKey->ndTarget, ndKey->nd_target,
+ sizeof (icmp6FlowPutKey->ndTarget));
+ memcpy(icmp6FlowPutKey->arpSha, ndKey->nd_sll, ETH_ADDR_LEN);
+ memcpy(icmp6FlowPutKey->arpTha, ndKey->nd_tll, ETH_ADDR_LEN);
+
+ destKey->l2.keyLen += OVS_ICMPV6_KEY_SIZE;
+ } else {
+ destKey->l2.keyLen += OVS_IPV6_KEY_SIZE;
+ }
+ break;
+ }
+ case ETH_TYPE_ARP:
+ case ETH_TYPE_RARP: {
+ ArpKey *arpFlowPutKey = &destKey->arpKey;
+ const struct ovs_key_arp *arpKey;
+
+ arpKey = NlAttrGet(keyAttrs[OVS_KEY_ATTR_ARP]);
+
+ arpFlowPutKey->nwSrc = arpKey->arp_sip;
+ arpFlowPutKey->nwDst = arpKey->arp_tip;
+
+ memcpy(arpFlowPutKey->arpSha, arpKey->arp_sha, ETH_ADDR_LEN);
+ memcpy(arpFlowPutKey->arpTha, arpKey->arp_tha, ETH_ADDR_LEN);
+ arpFlowPutKey->nwProto = (UINT8)(arpKey->arp_op);
+ arpFlowPutKey->pad[0] = 0;
+ arpFlowPutKey->pad[1] = 0;
+ arpFlowPutKey->pad[2] = 0;
+ destKey->l2.keyLen += OVS_ARP_KEY_SIZE;
+ break;
+ }
+ }
+}
+
+/*
+ *----------------------------------------------------------------------------
+ * _OvsFlowMapTunAttrToFlowPut --
+ * Converts FLOW_TUNNEL_KEY attribute to OvsFlowKey->tunKey.
+ *----------------------------------------------------------------------------
+ */
+static VOID
+_OvsFlowMapTunAttrToFlowPut(PNL_ATTR *keyAttrs,
+ PNL_ATTR *tunAttrs,
+ OvsFlowKey *destKey)
+{
+ if (keyAttrs[OVS_KEY_ATTR_TUNNEL]) {
+ destKey->tunKey.tunnelId = NlAttrGetU64
+ (tunAttrs[OVS_TUNNEL_KEY_ATTR_ID]);
+ destKey->tunKey.dst = NlAttrGetU32
+ (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_DST]);
+ destKey->tunKey.src = NlAttrGetU32
+ (tunAttrs[OVS_TUNNEL_KEY_ATTR_IPV4_SRC]);
+
+ /* XXX: There are no attributes to specify tunnel flags.
+ * Also, i do not see these flags being used anywhere in
+ * flowPut code. */
+ destKey->tunKey.flags = 0;
+ destKey->tunKey.tos = NlAttrGetU8
+ (tunAttrs[OVS_TUNNEL_KEY_ATTR_TOS]);
+ destKey->tunKey.ttl = NlAttrGetU8
+ (tunAttrs[OVS_TUNNEL_KEY_ATTR_TTL]);
+ destKey->tunKey.pad = 0;
+ destKey->l2.offset = 0;
+ } else {
+ destKey->tunKey.attr[0] = 0;
+ destKey->tunKey.attr[1] = 0;
+ destKey->tunKey.attr[2] = 0;
+ destKey->l2.offset = sizeof destKey->tunKey;
+ }
+}
+
+/*
+ *----------------------------------------------------------------------------
* OvsDeleteFlowTable --
* Results:
* NDIS_STATUS_SUCCESS always.
@@ -837,13 +1196,10 @@ ReportFlowInfo(OvsFlow *flow,
NTSTATUS
OvsPutFlowIoctl(PVOID inputBuffer,
UINT32 inputLength,
- PVOID outputBuffer,
- UINT32 outputLength,
- UINT32 *replyLen)
+ struct OvsFlowStats *stats)
{
NTSTATUS status = STATUS_SUCCESS;
OVS_DATAPATH *datapath = NULL;
- struct OvsFlowStats stats;
ULONG actionsLen;
OvsFlowPut *put;
UINT32 dpNo;
@@ -853,19 +1209,12 @@ OvsPutFlowIoctl(PVOID inputBuffer,
return STATUS_INFO_LENGTH_MISMATCH;
}
- if ((outputLength != sizeof(stats)) || (outputBuffer == NULL)) {
- return STATUS_INFO_LENGTH_MISMATCH;
- }
-
put = (OvsFlowPut *)inputBuffer;
if (put->actionsLen > 0) {
actionsLen = put->actionsLen;
} else {
actionsLen = 0;
}
- if (inputLength != actionsLen + sizeof(*put)) {
- return STATUS_INFO_LENGTH_MISMATCH;
- }
dpNo = put->dpNo;
NdisAcquireSpinLock(gOvsCtrlLock);
@@ -877,17 +1226,10 @@ OvsPutFlowIoctl(PVOID inputBuffer,
datapath = &gOvsSwitchContext->datapath;
ASSERT(datapath);
- RtlZeroMemory(&stats, sizeof(stats));
OvsAcquireDatapathWrite(datapath, &dpLockState, FALSE);
- status = HandleFlowPut(put, datapath, &stats);
+ status = HandleFlowPut(put, datapath, stats);
OvsReleaseDatapath(datapath, &dpLockState);
- if (status == STATUS_SUCCESS) {
- // Copy stats to User mode app
- NdisMoveMemory(outputBuffer, (PVOID)&stats, sizeof(stats));
- *replyLen = sizeof stats;
- }
-
unlock:
NdisReleaseSpinLock(gOvsCtrlLock);
return status;
diff --git a/datapath-windows/ovsext/Flow.h b/datapath-windows/ovsext/Flow.h
index 602e567..e62ba40 100644
--- a/datapath-windows/ovsext/Flow.h
+++ b/datapath-windows/ovsext/Flow.h
@@ -64,16 +64,14 @@ NTSTATUS OvsDumpFlowIoctl(PVOID inputBuffer, UINT32
inputLength,
PVOID outputBuffer, UINT32 outputLength,
UINT32 *replyLen);
NTSTATUS OvsPutFlowIoctl(PVOID inputBuffer, UINT32 inputLength,
- PVOID outputBuffer, UINT32 outputLength,
- UINT32 *replyLen);
+ struct OvsFlowStats *stats);
NTSTATUS OvsGetFlowIoctl(PVOID inputBuffer, UINT32 inputLength,
PVOID outputBuffer, UINT32 outputLength,
UINT32 *replyLen);
NTSTATUS OvsFlushFlowIoctl(PVOID inputBuffer, UINT32 inputLength);
NTSTATUS OvsFlowNlNewCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
- UINT32 *replyLen);
-
+ UINT32 *replyLen);
/* Flags for tunneling */
#define OVS_TNL_F_DONT_FRAGMENT (1 << 0)
--
1.9.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
