[ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
Amber Hu
qhu at vmware.com
Fri Jan 31 02:42:54 UTC 2020
Formerly, there is no tunnel information appended in the upcall’s
packet data, which is expected by IPFIX in userspace to calculate
the template for exporting the sampled flow record of on egress
tunnel port.
To fix this, during performing OvsOutputUserspaceAction(), we
would check whether it is initiated by the sampling on egress
tunnel which would be indicated by the attribute as
OVS_USERSPACE_ATTR_EGRESS_TUN_PORT in the nested attribute
list. If so, we would append the tunKey in OvsForwardingContext
indexed by OVS_PACKET_ATTR_EGRESS_TUN_KEY to the upcall.
Besides, at this point, the source transport port and source ip
address are not available in the structure, so we have to fill it in the
way how the packet would be capsulated during performing
OvsEncapGeneve(), which is following the
OvsOutputUserspaceAction() unfortunately.
I have tested the IPFIX functionality with the change, we could see the
template is correct and the expected tunnel information could be
packed in the IPFIX packet finally. The traffic for test is generated by
PING utility.
Signed-off-by: Amber Hu <qhu at vmware.com>
From 31e3baa833d4d37686c3402b95db018ad132d3b7 Mon Sep 17 00:00:00 2001
From: Amber Hu <qhu at vmware.com>
Date: Thu, 30 Jan 2020 18:01:32 -0800
Subject: [PATCH] datapath-windows: Append tunnel info to upcall for correct
template
Signed-off-by: Amber Hu <qhu at vmware.com>
---
datapath-windows/ovsext/Actions.c | 24 +++++++++++++++++++++---
datapath-windows/ovsext/Flow.c | 12 ++++++++++++
datapath-windows/ovsext/User.c | 12 +++++++++---
datapath-windows/ovsext/User.h | 1 +
4 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 5c9b5c3..4a11cea 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1815,10 +1815,12 @@ OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
{
NTSTATUS status = NDIS_STATUS_SUCCESS;
PNL_ATTR userdataAttr;
- PNL_ATTR queueAttr;
+ PNL_ATTR egrTunAttr = NULL;
POVS_PACKET_QUEUE_ELEM elem;
POVS_PACKET_HDR_INFO layers = &ovsFwdCtx->layers;
BOOLEAN isRecv = FALSE;
+ OVS_FWD_INFO fwdInfo;
+ OvsIPv4TunnelKey tunKey;
POVS_VPORT_ENTRY vport = OvsFindVportByPortNo(ovsFwdCtx->switchContext,
ovsFwdCtx->srcVportNo);
@@ -1830,13 +1832,29 @@ OvsOutputUserspaceAction(OvsForwardingContext *ovsFwdCtx,
}
}
- queueAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_PID);
userdataAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_USERDATA);
+ /* Indicate the packet is from egress-tunnel direction */
+ egrTunAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_EGRESS_TUN_PORT);
+
+ /* Fill tunnel key to export to usersspace to calculate the template id */
+ if (egrTunAttr) {
+ RtlZeroMemory(&tunKey, sizeof tunKey);
+ RtlCopyMemory(&tunKey, &ovsFwdCtx->tunKey, sizeof tunKey);
+ if (!tunKey.src) {
+ status = OvsLookupIPFwdInfo(tunKey.src, tunKey.dst, &fwdInfo);
+ if (status == NDIS_STATUS_SUCCESS && tunKey.dst == fwdInfo.dstIpAddr) {
+ tunKey.src = fwdInfo.srcIpAddr;
+ }
+ }
+ tunKey.flow_hash = tunKey.flow_hash ? tunKey.flow_hash : MAXINT16;
+ }
elem = OvsCreateQueueNlPacket(NlAttrData(userdataAttr),
NlAttrGetSize(userdataAttr),
OVS_PACKET_CMD_ACTION,
- vport, key, ovsFwdCtx->curNbl,
+ vport, key,
+ egrTunAttr ? &(tunKey) : NULL,
+ ovsFwdCtx->curNbl,
NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl),
isRecv,
layers);
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index fdb1010..ac0582c 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1094,6 +1094,18 @@ MapFlowTunKeyToNlKey(PNL_BUFFER nlBuf,
goto done;
}
+ if (!NlMsgPutTailU16(nlBuf, OVS_TUNNEL_KEY_ATTR_TP_SRC,
+ tunKey->flow_hash)) {
+ rc = STATUS_UNSUCCESSFUL;
+ goto done;
+ }
+
+ if (!NlMsgPutTailU16(nlBuf, OVS_TUNNEL_KEY_ATTR_TP_DST,
+ tunKey->dst_port)) {
+ rc = STATUS_UNSUCCESSFUL;
+ goto done;
+ }
+
done:
NlMsgEndNested(nlBuf, offset);
error_nested_start:
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index ed1fcbe..ee0e38d 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -830,7 +830,7 @@ OvsCreateAndAddPackets(PVOID userData,
nb = NET_BUFFER_LIST_FIRST_NB(nbl);
while (nb) {
elem = OvsCreateQueueNlPacket(userData, userDataLen,
- cmd, vport, key, nbl, nb,
+ cmd, vport, key, NULL, nbl, nb,
isRecv, hdrInfo);
if (elem) {
InsertTailList(list, &elem->link);
@@ -1013,6 +1013,7 @@ OvsCreateQueueNlPacket(PVOID userData,
UINT32 cmd,
POVS_VPORT_ENTRY vport,
OvsFlowKey *key,
+ OvsIPv4TunnelKey *tunnelKey,
PNET_BUFFER_LIST nbl,
PNET_BUFFER nb,
BOOLEAN isRecv,
@@ -1025,7 +1026,6 @@ OvsCreateQueueNlPacket(PVOID userData,
NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
PNDIS_NET_BUFFER_LIST_8021Q_INFO vlanInfo = NULL;
PVOID vlanTag;
- OvsIPv4TunnelKey *tunnelKey = (OvsIPv4TunnelKey *)&key->tunKey;
UINT32 pid;
UINT32 nlMsgSize;
NL_BUFFER nlBuf;
@@ -1127,7 +1127,13 @@ OvsCreateQueueNlPacket(PVOID userData,
}
}
- /* XXX must send OVS_PACKET_ATTR_EGRESS_TUN_KEY if set by vswtchd */
+ /* Set OVS_PACKET_ATTR_EGRESS_TUN_KEY attribute */
+ if (tunnelKey) {
+ if (MapFlowTunKeyToNlKey(&nlBuf, tunnelKey,
+ OVS_PACKET_ATTR_EGRESS_TUN_KEY) != STATUS_SUCCESS) {
+ goto fail;
+ }
+ }
if (userData){
if (!NlMsgPutTailUnspec(&nlBuf, OVS_PACKET_ATTR_USERDATA,
userData, (UINT16)userDataLen)) {
diff --git a/datapath-windows/ovsext/User.h b/datapath-windows/ovsext/User.h
index 3a42888..ccca0ba 100644
--- a/datapath-windows/ovsext/User.h
+++ b/datapath-windows/ovsext/User.h
@@ -75,6 +75,7 @@ POVS_PACKET_QUEUE_ELEM OvsCreateQueueNlPacket(PVOID userData,
UINT32 cmd,
POVS_VPORT_ENTRY vport,
OvsFlowKey *key,
+ OvsIPv4TunnelKey *tunnelKey,
PNET_BUFFER_LIST nbl,
PNET_BUFFER nb,
BOOLEAN isRecv,
--
2.6.2
BR/Amber
On 1/22/20, 12:43, "Alin Serdean" <aserdean at cloudbasesolutions.com> wrote:
Hi Amber,
The patch fails to apply:
"error: corrupt patch at line 198"
Please rebase on master.
Trimming the patch a bit and comments inlined.
> -----Original Message-----
> From: Amber Hu <qhu at vmware.com>
> Sent: Tuesday, January 14, 2020 7:50 AM
> To: dev at openvswitch.org; Alin Serdean <aserdean at cloudbasesolutions.com>
> Cc: Qiong Wang <wqiong at vmware.com>; Wenyu Zhang
> <wenyuz at vmware.com>; Jinjun Gao <jinjung at vmware.com>
> Subject: [patch] datapath-windows: Append tunnel info to upcall for correct
> template
>
> diff --git a/datapath-windows/ovsext/Actions.c b/datapath-
> windows/ovsext/Actions.c
>
> index 0c4d64b..936e68b 100644
>
> --- a/datapath-windows/ovsext/Actions.c
>
> +++ b/datapath-windows/ovsext/Actions.c
>
> @@ -1839,11 +1839,16 @@ OvsOutputUserspaceAction(OvsForwardingContext
> *ovsFwdCtx,
>
> const PNL_ATTR attr)
>
> {
>
> NTSTATUS status = NDIS_STATUS_SUCCESS;
>
> - PNL_ATTR userdataAttr;
>
> - PNL_ATTR queueAttr;
[Alin Serdean] queueAttr can be safely removed since it's not actually used.
>
> + PNL_ATTR userdataAttr = NULL;
>
> + PNL_ATTR queueAttr = NULL;
>
> + PNL_ATTR egrTunAttr = NULL;
>
> + PNL_ATTR a = NULL;
[Alin Serdean] Not sure why did is needed. NlAttrFindNested does the safe version
of NL_ATTR_FOR_EACH_UNSAFE. Any reason to change it?
>
> POVS_PACKET_QUEUE_ELEM elem;
>
> POVS_PACKET_HDR_INFO layers = &ovsFwdCtx->layers;
>
> BOOLEAN isRecv = FALSE;
>
> + INT rem = 0;
>
> + OVS_FWD_INFO fwdInfo;
>
> + OvsIPv4TunnelKey tunKey;
>
> POVS_VPORT_ENTRY vport = OvsFindVportByPortNo(ovsFwdCtx-
> >switchContext,
>
> ovsFwdCtx->srcVportNo);
>
> @@ -1855,13 +1860,41 @@ OvsOutputUserspaceAction(OvsForwardingContext
> *ovsFwdCtx,
>
> }
>
> }
>
> - queueAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_PID);
>
> - userdataAttr = NlAttrFindNested(attr, OVS_USERSPACE_ATTR_USERDATA);
>
> + NL_ATTR_FOR_EACH_UNSAFE(a, rem, NlAttrData(attr), NlAttrGetSize(attr)) {
[Alin Serdean] NlAttrFindNested does the safe version of NL_ATTR_FOR_EACH_UNSAFE.
Any reason to change it?
>
> + switch (NlAttrType(a)) {
>
> + case OVS_USERSPACE_ATTR_PID:
>
> + queueAttr = a;
>
> + break;
>
> + case OVS_USERSPACE_ATTR_USERDATA:
>
> + userdataAttr = a;
>
> + break;
>
> + case OVS_USERSPACE_ATTR_EGRESS_TUN_PORT:
>
> + /* Indicate the packet is from egress tunnel port */
>
> + egrTunAttr = a;
>
> + break;
>
> + default:
>
> + break;
>
> + }
>
> + }
>
> +
>
> + /* Fill tunnel key to export to usersspace to calculate the template id */
>
> + if (egrTunAttr) {
>
[Alin Serdean] It would be safer to zero out tunKey before this.
> + RtlCopyMemory(&tunKey, &ovsFwdCtx->tunKey, sizeof tunKey);
>
> + if (!tunKey.src) {
>
> + status = OvsLookupIPFwdInfo(tunKey.src, tunKey.dst, &fwdInfo);
>
> + if (status == NDIS_STATUS_SUCCESS && tunKey.dst ==
> fwdInfo.dstIpAddr) {
>
> + tunKey.src = fwdInfo.srcIpAddr;
>
> + }
>
> + }
>
> + tunKey.flow_hash = tunKey.flow_hash?tunKey.flow_hash:MAXINT16;
[Alin Serdean] Missing a whitespace before and after '?'
>
> + }
>
> elem = OvsCreateQueueNlPacket(NlAttrData(userdataAttr),
>
> NlAttrGetSize(userdataAttr),
>
> OVS_PACKET_CMD_ACTION,
>
> - vport, key, ovsFwdCtx->curNbl,
>
> + vport, key,
>
> + egrTunAttr?&(tunKey):NULL,
[Alin Serdean] Missing whitespace before and after '?'
>
> + ovsFwdCtx->curNbl,
>
> NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl),
>
> isRecv,
>
> layers);
>
> diff --git a/datapath-windows/ovsext/Flow.c b/datapath-
> windows/ovsext/Flow.c
>
> index fdb1010..ac0582c 100644
>
> --- a/datapath-windows/ovsext/Flow.c
>
> +++ b/datapath-windows/ovsext/Flow.c
>
> @@ -1094,6 +1094,18 @@ MapFlowTunKeyToNlKey(PNL_BUFFER nlBuf,
>
> goto done;
>
> }
>
> + if (!NlMsgPutTailU16(nlBuf, OVS_TUNNEL_KEY_ATTR_TP_SRC,
>
> + tunKey->flow_hash)) {
>
> + rc = STATUS_UNSUCCESSFUL;
>
> + goto done;
>
> + }
>
> +
>
> + if (!NlMsgPutTailU16(nlBuf, OVS_TUNNEL_KEY_ATTR_TP_DST,
>
> + tunKey->dst_port)) {
>
> + rc = STATUS_UNSUCCESSFUL;
>
> + goto done;
>
> + }
>
> +
>
> done:
>
> NlMsgEndNested(nlBuf, offset);
>
> error_nested_start:
>
> diff --git a/datapath-windows/ovsext/User.c b/datapath-
> windows/ovsext/User.c
>
> index 16d6fd2..a1d05e1 100644
>
> --- a/datapath-windows/ovsext/User.c
>
> +++ b/datapath-windows/ovsext/User.c
>
> @@ -833,7 +833,7 @@ OvsCreateAndAddPackets(PVOID userData,
>
> nb = NET_BUFFER_LIST_FIRST_NB(nbl);
>
> while (nb) {
>
> elem = OvsCreateQueueNlPacket(userData, userDataLen,
>
> - cmd, vport, key, nbl, nb,
>
> + cmd, vport, key, NULL, nbl, nb,
>
> isRecv, hdrInfo);
>
> if (elem) {
>
> InsertTailList(list, &elem->link);
>
> @@ -1016,6 +1016,7 @@ OvsCreateQueueNlPacket(PVOID userData,
>
> UINT32 cmd,
>
> POVS_VPORT_ENTRY vport,
>
> OvsFlowKey *key,
>
> + OvsIPv4TunnelKey *tunnelKey,
>
> PNET_BUFFER_LIST nbl,
>
> PNET_BUFFER nb,
>
> BOOLEAN isRecv,
>
> @@ -1028,7 +1029,6 @@ OvsCreateQueueNlPacket(PVOID userData,
>
> NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
>
> PNDIS_NET_BUFFER_LIST_8021Q_INFO vlanInfo = NULL;
>
> PVOID vlanTag;
>
> - OvsIPv4TunnelKey *tunnelKey = (OvsIPv4TunnelKey *)&key->tunKey;
>
> UINT32 pid;
>
> UINT32 nlMsgSize;
>
> NL_BUFFER nlBuf;
>
> @@ -1131,6 +1131,12 @@ OvsCreateQueueNlPacket(PVOID userData,
>
> }
>
[Alin Serdean] I guess this can be removed now 😊.
> /* XXX must send OVS_PACKET_ATTR_EGRESS_TUN_KEY if set by vswtchd */
>
> + if (tunnelKey) {
>
> + if (MapFlowTunKeyToNlKey(&nlBuf, tunnelKey,
>
> + OVS_PACKET_ATTR_EGRESS_TUN_KEY) != STATUS_SUCCESS)
> {
>
> + goto fail;
>
> + }
>
> + }
>
> if (userData){
>
> if (!NlMsgPutTailUnspec(&nlBuf, OVS_PACKET_ATTR_USERDATA,
>
> userData, (UINT16)userDataLen)) {
>
> diff --git a/datapath-windows/ovsext/User.h b/datapath-
> windows/ovsext/User.h
>
> index 3a42888..ccca0ba 100644
>
> --- a/datapath-windows/ovsext/User.h
>
> +++ b/datapath-windows/ovsext/User.h
>
> @@ -75,6 +75,7 @@ POVS_PACKET_QUEUE_ELEM
> OvsCreateQueueNlPacket(PVOID userData,
>
> UINT32 cmd,
>
> POVS_VPORT_ENTRY vport,
>
> OvsFlowKey *key,
>
> + OvsIPv4TunnelKey *tunnelKey,
>
> PNET_BUFFER_LIST nbl,
>
> PNET_BUFFER nb,
>
> BOOLEAN isRecv,
>
> --
>
> 2.6.2
>
>
>
> BR/Amber
More information about the dev
mailing list