[ovs-dev] [patch] datapath-windows: Append tunnel info to upcall for correct template

Alin Serdean aserdean at cloudbasesolutions.com
Tue Jan 21 18:01:30 UTC 2020


Sure. I will review it tonight.

Thanks,
Alin.

-----Original Message-----
From: Ben Pfaff <blp at ovn.org> 
Sent: Tuesday, January 21, 2020 7:59 PM
To: Amber Hu <qhu at vmware.com>
Cc: dev at openvswitch.org; Alin Serdean <aserdean at cloudbasesolutions.com>; Qiong Wang <wqiong at vmware.com>
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 +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 <qhu at vmware.com>
> Date: Tuesday, January 14, 2020 at 13:49
> To: "dev at openvswitch.org" <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
> 
> 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 <ymeng at vmware.com<mailto:ymeng at vmware.com>>
> Reported-at: https://bugzilla.eng.vmware.com/show_bug.cgi?id=2449045
> Signed-off-by: Amber Hu <qhu at vmware.com>
> 
> From 8262494fb9a353c3adbe02cfc4c932231c34b3ed Mon Sep 17 00:00:00 2001
> From: Amber Hu <qhu at 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 <qhu at 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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list