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" <aserd...@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 <q...@vmware.com>
> Sent: Tuesday, January 14, 2020 7:50 AM
> To: d...@openvswitch.org; Alin Serdean <aserd...@cloudbasesolutions.com>
> Cc: Qiong Wang <wqi...@vmware.com>; Wenyu Zhang
> <wen...@vmware.com>; Jinjun Gao <jinj...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
