From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f66.google.com (mail-oi0-f66.google.com [209.85.218.66]) by dpdk.org (Postfix) with ESMTP id B69241B21F for ; Fri, 29 Sep 2017 12:09:48 +0200 (CEST) Received: by mail-oi0-f66.google.com with SMTP id x85so1242361oix.12 for ; Fri, 29 Sep 2017 03:09:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=urRXhNwIQtalzK3kn28fObBX0QtqNZrgMbyvVZZunbg=; b=H8RupNfSrIPOOSfw5sBLsdKEgAaM8UAAOi5Q3cAeIrLSKEuc1nOQeLbTofxgmCc0yl UQQV66I6nDsBixAScE8lFNBO3KTmFFDfDS7x38LqyXNyv6kmMdrKYNn3r9fE4JHYgJuK y4nNhqFRnKTk+vspQXqSzhPT3pr4gKIcQ3cDwVZ5imtEd1D09xgHPaeb5Grgow2l36BO MVZamARfiq7kR6bPxkhSu7Er4//98fb6+c9wDJmJge/HPNSQ9Lvrjw5SGXcm8TAMpT5a SisRSTmfPLDNtio++Qu/vi+/vUCL+WeN4GLvMLLuiW6FxjIJ26PT4YkqIGtqlaI1lCIm 9WLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=urRXhNwIQtalzK3kn28fObBX0QtqNZrgMbyvVZZunbg=; b=mX7nbVn3NbXp+Uv7o28aIoxex7kSeXVH/lsSf3YtJxMqivhawcmmZ9w1eQlkl2qsPi tSRloYKKWVTqN+T/NSVJu+muAtYqPkIWOQJSHqg/439qZmGqcWTzfQL/jaGqULECo0x3 9NI5q06D+nN/EhrpZ0tA8RGgJtjJExLkiFUmPwqi1KUg6CZATiJJXzXihtGH8uJohAVR Xb2q7UtMxi8bDPWDknasj5J3fQohg69tWUR+vY0qoTWkLOJeqZF/DyElwW19jZJL4Gjx n/Y6DAYj9Vtg0N6eFwOtbjLakqv5uTdfWBU/6GtT+eVLgJDWSxKhuj/ZpAIqpqan6LFu On1g== X-Gm-Message-State: AMCzsaVnWkHfAEVebyVFEfknuwS363V43xPqFPh8wbocDhCfhZ09K/Gj /ZlkSNBRrYIPW2pR+jT6K+3IHOclrDSbAR1zOcI= X-Google-Smtp-Source: AOwi7QAt4ITcZ7YQ1yGQqyoKD61jqNJ6j2nNlBm1NFkYKugBXh9z/tljgvEyyJ2IghTSnb70/LV9CWRZrLv7s31ePXM= X-Received: by 10.202.79.75 with SMTP id d72mr2216894oib.117.1506679787999; Fri, 29 Sep 2017 03:09:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.83.193 with HTTP; Fri, 29 Sep 2017 03:09:47 -0700 (PDT) In-Reply-To: <94479800C636CB44BD422CB454846E0132038E8C@SHSMSX101.ccr.corp.intel.com> References: <1506565054-67690-1-git-send-email-beilei.xing@intel.com> <1506662342-18966-1-git-send-email-beilei.xing@intel.com> <1506662342-18966-7-git-send-email-beilei.xing@intel.com> <94479800C636CB44BD422CB454846E0132038E8C@SHSMSX101.ccr.corp.intel.com> From: Sean Harte Date: Fri, 29 Sep 2017 11:09:47 +0100 Message-ID: To: "Xing, Beilei" Cc: "Wu, Jingjing" , "Chilikin, Andrey" , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v6 6/8] net/i40e: add FDIR support for GTP-C and GTP-U X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Sep 2017 10:09:49 -0000 On 29 September 2017 at 10:33, Xing, Beilei wrote: > > >> -----Original Message----- >> From: Sean Harte [mailto:seanbh@gmail.com] >> Sent: Friday, September 29, 2017 4:15 PM >> To: Xing, Beilei >> Cc: Wu, Jingjing ; Chilikin, Andrey >> ; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v6 6/8] net/i40e: add FDIR support for GT= P-C >> and GTP-U >> >> On 29 September 2017 at 06:19, Beilei Xing wrote= : >> > This patch adds FDIR support for GTP-C and GTP-U. The input set of >> > GTP-C and GTP-U is TEID. >> > >> > Signed-off-by: Beilei Xing >> > --- >> > drivers/net/i40e/i40e_ethdev.h | 30 +++++ >> > drivers/net/i40e/i40e_fdir.c | 200 ++++++++++++++++++++++--------- >> > drivers/net/i40e/i40e_flow.c | 263 >> +++++++++++++++++++++++++++++++++++------ >> > 3 files changed, 396 insertions(+), 97 deletions(-) >> >> >> >> > @@ -1173,10 +1196,69 @@ i40e_flow_fdir_construct_pkt(struct i40e_pf >> *pf, >> > rte_cpu_to_be_16(ETHER_TYPE_ARP)) >> > payload +=3D sizeof(struct arp_hdr); >> > set_idx =3D I40E_FLXPLD_L2_IDX; >> > - break; >> > - default: >> > - PMD_DRV_LOG(ERR, "unknown pctype %u.", fdir_input->pct= ype); >> > - return -EINVAL; >> > + } else if (fdir_input->flow_ext.customized_pctype) { >> > + /* If customized pctype is used */ >> > + cus_pctype =3D i40e_flow_fdir_find_customized_pctype(p= f, pctype); >> > + if (cus_pctype->index =3D=3D I40E_CUSTOMIZED_GTPC || >> > + cus_pctype->index =3D=3D I40E_CUSTOMIZED_GTPU_IPV4= || >> > + cus_pctype->index =3D=3D I40E_CUSTOMIZED_GTPU_IPV6= || >> > + cus_pctype->index =3D=3D I40E_CUSTOMIZED_GTPU) { >> > + udp =3D (struct udp_hdr *)(raw_pkt + len); >> > + udp->dgram_len =3D >> > + >> > + rte_cpu_to_be_16(I40E_FDIR_UDP_DEFAULT_LEN); >> > + >> > + gtp =3D (struct rte_flow_item_gtp *) >> > + ((unsigned char *)udp + sizeof(struct = udp_hdr)); >> > + gtp->v_pt_rsv_flags =3D 0x30; >> >> 0x30 isn't valid for GTP-C, the sequence number must be present in GTP-C= so >> it will be 0x32 or more. Is this byte actually matched against by the de= vice >> using the GTP pctypes? > > We construct packets and send the packet to HW to create flow director r= ule for GTP-C and > GTP-U. Actually I didn=E2=80=99t get error info with 0x30. And in my test= , GTP-C packets can hit GTP-C > pctype rule. Will try 0x32 later. > >> >> > + gtp->msg_len =3D >> > + rte_cpu_to_be_16(I40E_FDIR_GTP_DEFAULT= _LEN); >> > + gtp->teid =3D fdir_input->flow.gtp_flow.teid; >> > + gtp->msg_type =3D 0x1; >> >> Why use this value? > > Just for constructing a GTP packet to create a fdir rule for one pctype, = can use other values except 0xFF. > >> >> > + >> > + if (cus_pctype->index =3D=3D I40E_CUSTOMIZED_G= TPC) >> > + udp->dst_port =3D >> > + rte_cpu_to_be_16(2123); >> >> This will only match half of GTP-C messages. GTP-C messages have a UDP >> port destination of 2123, or a UDP source port of 2123. To match all GTP= -C >> packets you need to look at both. > > Yes. But the GTP profile for i40e didn't support response message. That's not clear to a user of the rte_flow API > >> >> > + else >> > + udp->dst_port =3D >> > + rte_cpu_to_be_16(2152); >> > + >> > + if (cus_pctype->index =3D=3D I40E_CUSTOMIZED_G= TPU_IPV4) { >> > + gtp->msg_type =3D 0xFF; >> > + gtp_ipv4 =3D (struct ipv4_hdr *) >> > + ((unsigned char *)gtp + >> > + sizeof(struct >> > + rte_flow_item_gtp)); >> >> This is only valid if v_pt_rsv_flags is 0x30. GTP-U packets are allowed = to have >> a sequence number, which adds an extra 4 bytes to the GTP header. > > For the GTP profile, there're 4 pctypes for GTP packets: GTPC, GTPU, GTPI= PV4, and GTPIPV6. > HW parse which pctype the GTP packets belonge to. > We construct packet to create a fdir rule for one pctype, after that, all= packets whose > pctype matches the rule's pctype will hit the rule. My point is that you can only assume the inner IP header starts at an offset of sizeof(struct rte_flow_item_gtp) if v_pt_rsv_flags is exactly 0x30. If you match only those packets then some GTP-U packets will not be matched. That should be clear to a user of the rte_flow API. > >> >> > + gtp_ipv4->version_ihl =3D >> > + I40E_FDIR_IP_DEFAULT_VERSION_I= HL; >> > + gtp_ipv4->next_proto_id =3D IPPROTO_IP= ; >> > + gtp_ipv4->total_length =3D >> > + rte_cpu_to_be_16( >> > + I40E_FDIR_INNER_IP_DEF= AULT_LEN); >> > + payload =3D (unsigned char *)gtp_ipv4 = + >> > + sizeof(struct ipv4_hdr); >> > + } else if (cus_pctype->index =3D=3D >> > + I40E_CUSTOMIZED_GTPU_IPV6) { >> > + gtp->msg_type =3D 0xFF; >> > + gtp_ipv6 =3D (struct ipv6_hdr *) >> > + ((unsigned char *)gtp + >> > + sizeof(struct >> > + rte_flow_item_gtp)); >> >> This is only valid if v_pt_rsv_flags is 0x30. GTP-U packets are allowed = to have >> a sequence number, which adds an extra 4 bytes to the GTP header. > > Same with above. > >> >> > + gtp_ipv6->vtc_flow =3D >> > + rte_cpu_to_be_32( >> > + I40E_FDIR_IPv6_DEFAULT_= VTC_FLOW | >> > + (0 << I40E_FDIR_IPv6_TC= _OFFSET)); >> > + gtp_ipv6->proto =3D IPPROTO_NONE; >> > + gtp_ipv6->payload_len =3D >> > + rte_cpu_to_be_16( >> > + I40E_FDIR_INNER_IPv6_DEF= AULT_LEN); >> > + gtp_ipv6->hop_limits =3D >> > + I40E_FDIR_IPv6_DEFAULT_HOP_LIM= ITS; >> > + payload =3D (unsigned char *)gtp_ipv6 = + >> > + sizeof(struct ipv6_hdr); >> > + } else >> > + payload =3D (unsigned char *)gtp + >> > + sizeof(struct rte_flow_item_gt= p); >> > + } >> > + } else { >> > + PMD_DRV_LOG(ERR, "unknown pctype %u.", >> > + fdir_input->pctype); >> > + return -1; >> > } >> > >> > /* fill the flexbytes to payload */ >> >>