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 +0000, 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 <q...@vmware.com> > Date: Tuesday, January 14, 2020 at 13:49 > To: "d...@openvswitch.org" <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 > > 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 <ym...@vmware.com<mailto:ym...@vmware.com>> > Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045 > Signed-off-by: Amber Hu <q...@vmware.com> > > From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001 > From: Amber Hu <q...@vmware.com> > 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 <q...@vmware.com> > --- > 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 (!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, > } > /* 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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev