Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
>
> On 1/31/20, 10:42, "Amber Hu" wrote:
>
>
> 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
>
> From 31e3baa833d4d37686c3402b95db018ad132d3b7 Mon Sep 17
> 00:00:00 2001
> From: Amber Hu
> 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
[Alin Serdean] The patch will break compilation. You probably want the
following incremental:
$ git diff datapath-windows/ovsext/Tunnel.c
diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index ad2c254f5..5d1be80f4 100644
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -308,7 +308,7 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
datapath->misses++;
elem = OvsCreateQueueNlPacket(NULL, 0, OVS_PACKET_CMD_MISS,
- vport, &key, pNbl, curNb,
+ vport, &key, NULL, pNbl, curNb,
TRUE, &layers);
if (elem) {
/* Complete the packet since it was copied to user buffer. */
Also mark the patch as a new version, it is easier for me to see it.
Alin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
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
From 31e3baa833d4d37686c3402b95db018ad132d3b7 Mon Sep 17 00:00:00 2001
From: Amber Hu
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
---
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
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
Hi Alin,
Will update the patch and change it according to the comments.
Thanks very much.
BR/Amber
On 1/22/20, 12:43, "Alin Serdean" 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
> Sent: Tuesday, January 14, 2020 7:50 AM
> To: d...@openvswitch.org; Alin Serdean
> Cc: Qiong Wang ; Wenyu Zhang
> ; Jinjun Gao
> 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-windo
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
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
> Sent: Tuesday, January 14, 2020 7:50 AM
> To: d...@openvswitch.org; Alin Serdean
> Cc: Qiong Wang ; Wenyu Zhang
> ; Jinjun Gao
> 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
>
> ++
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
Sure. I will review it tonight.
Thanks,
Alin.
-Original Message-
From: Ben Pfaff
Sent: Tuesday, January 21, 2020 7:59 PM
To: Amber Hu
Cc: d...@openvswitch.org; Alin Serdean ; Qiong
Wang
Subject: Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall
for correct template
I expect that, of the OVS maintainers, Alin is most qualified to review it.
Alin, are you planning to take a look at it?
On Tue, Jan 21, 2020 at 02:52:54AM +, Amber Hu via dev wrote:
> Hi,
>
> May I ask the process of reviewing for the below patch
> https://patchwork.ozlabs.org/patch/1222463/
> Thanks for your time.
>
> BR/Amber
>
> From: Amber Hu
> Date: Tuesday, January 14, 2020 at 13:49
> To: "d...@openvswitch.org" , Alin Serdean
>
> Cc: Qiong Wang , Wenyu Zhang ,
> Jinjun Gao
> Subject: [patch] datapath-windows: Append tunnel info to upcall for
> correct template
>
> 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.
>
> Issue: 2449045
> Reported-by: Meng Yang mailto:ym...@vmware.com>>
> Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045
> Signed-off-by: Amber Hu
>
> From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001
> From: Amber Hu
> Date: Sun, 12 Jan 2020 22:40:05 -0800
> Subject: [PATCH] datapath-windows: Append tunnel info to upcall for
> correct template
>
> Issue: #2449045
> Signed-off-by: Amber Hu
> ---
> datapath-windows/ovsext/Actions.c | 43 ++-
> datapath-windows/ovsext/Flow.c| 12 +++
> datapath-windows/ovsext/User.c| 10 +++--
> datapath-windows/ovsext/User.h| 1 +
> 4 files changed, 59 insertions(+), 7 deletions(-)
>
> 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;
> +PNL_ATTR userdataAttr = NULL;
> +PNL_ATTR queueAttr = NULL;
> +PNL_ATTR egrTunAttr = NULL;
> +PNL_ATTR a = NULL;
> 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)) {
> +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
> *
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
I expect that, of the OVS maintainers, Alin is most qualified to
review it. Alin, are you planning to take a look at it?
On Tue, Jan 21, 2020 at 02:52:54AM +, Amber Hu via dev wrote:
> Hi,
>
> May I ask the process of reviewing for the below patch
> https://patchwork.ozlabs.org/patch/1222463/
> Thanks for your time.
>
> BR/Amber
>
> From: Amber Hu
> Date: Tuesday, January 14, 2020 at 13:49
> To: "d...@openvswitch.org" , Alin Serdean
>
> Cc: Qiong Wang , Wenyu Zhang , Jinjun
> Gao
> Subject: [patch] datapath-windows: Append tunnel info to upcall for correct
> template
>
> 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.
>
> Issue: 2449045
> Reported-by: Meng Yang mailto:ym...@vmware.com>>
> Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045
> Signed-off-by: Amber Hu
>
> From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001
> From: Amber Hu
> Date: Sun, 12 Jan 2020 22:40:05 -0800
> Subject: [PATCH] datapath-windows: Append tunnel info to upcall for correct
> template
>
> Issue: #2449045
> Signed-off-by: Amber Hu
> ---
> datapath-windows/ovsext/Actions.c | 43 ++-
> datapath-windows/ovsext/Flow.c| 12 +++
> datapath-windows/ovsext/User.c| 10 +++--
> datapath-windows/ovsext/User.h| 1 +
> 4 files changed, 59 insertions(+), 7 deletions(-)
>
> 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;
> +PNL_ATTR userdataAttr = NULL;
> +PNL_ATTR queueAttr = NULL;
> +PNL_ATTR egrTunAttr = NULL;
> +PNL_ATTR a = NULL;
> 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)) {
> +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) {
> +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, ovsF
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
Hi,
May I ask the process of reviewing for the below patch
https://patchwork.ozlabs.org/patch/1222463/
Thanks for your time.
BR/Amber
From: Amber Hu
Date: Tuesday, January 14, 2020 at 13:49
To: "d...@openvswitch.org" , Alin Serdean
Cc: Qiong Wang , Wenyu Zhang , Jinjun Gao
Subject: [patch] datapath-windows: Append tunnel info to upcall for correct
template
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.
Issue: 2449045
Reported-by: Meng Yang mailto:ym...@vmware.com>>
Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045
Signed-off-by: Amber Hu
From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001
From: Amber Hu
Date: Sun, 12 Jan 2020 22:40:05 -0800
Subject: [PATCH] datapath-windows: Append tunnel info to upcall for correct
template
Issue: #2449045
Signed-off-by: Amber Hu
---
datapath-windows/ovsext/Actions.c | 43 ++-
datapath-windows/ovsext/Flow.c| 12 +++
datapath-windows/ovsext/User.c| 10 +++--
datapath-windows/ovsext/User.h| 1 +
4 files changed, 59 insertions(+), 7 deletions(-)
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;
+PNL_ATTR userdataAttr = NULL;
+PNL_ATTR queueAttr = NULL;
+PNL_ATTR egrTunAttr = NULL;
+PNL_ATTR a = NULL;
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)) {
+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) {
+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 fdb1
Re: [ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
Bleep bloop. Greetings Amber Hu via dev, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: fatal: corrupt patch at line 19 Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 datapath-windows: Append tunnel info to upcall for correct template The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template
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.
Issue: 2449045
Reported-by: Meng Yang mailto:ym...@vmware.com>>
Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045
Signed-off-by: Amber Hu
From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001
From: Amber Hu
Date: Sun, 12 Jan 2020 22:40:05 -0800
Subject: [PATCH] datapath-windows: Append tunnel info to upcall for correct
template
Issue: #2449045
Signed-off-by: Amber Hu
---
datapath-windows/ovsext/Actions.c | 43 ++-
datapath-windows/ovsext/Flow.c| 12 +++
datapath-windows/ovsext/User.c| 10 +++--
datapath-windows/ovsext/User.h| 1 +
4 files changed, 59 insertions(+), 7 deletions(-)
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;
+PNL_ATTR userdataAttr = NULL;
+PNL_ATTR queueAttr = NULL;
+PNL_ATTR egrTunAttr = NULL;
+PNL_ATTR a = NULL;
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)) {
+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) {
+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 (!NlMs
