From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 75313A0577; Tue, 7 Apr 2020 07:37:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1D7551BE80; Tue, 7 Apr 2020 07:37:14 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 39BB22BE9 for ; Tue, 7 Apr 2020 07:37:12 +0200 (CEST) IronPort-SDR: Dk/uRlPwF3smNS0CRND0wpCl89z+o5HZteTX8pDzje8tlQBvLVp3Mq06/Ri5pqGLzkIYcCUq3n me/lXHuEr1LQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Apr 2020 22:37:11 -0700 IronPort-SDR: IfkLqzg6iOxkcnq3pRWSl+iyCdmu2UjUPk9ePWcLv4kCg+qpeclug++EuH1Smhl3fxowxBYwJq o2bpD6ZRUA8A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,353,1580803200"; d="scan'208,217";a="330086185" Received: from jguo15x-mobl.ccr.corp.intel.com (HELO [10.67.68.154]) ([10.67.68.154]) by orsmga001.jf.intel.com with ESMTP; 06 Apr 2020 22:37:08 -0700 To: Ori Kam , "xiaolong.ye@intel.com" , "qi.z.zhang@intel.com" Cc: "dev@dpdk.org" , "jingjing.wu@intel.com" , "yahui.cao@intel.com" , "simei.su@intel.com" References: <20200318170401.7938-5-jia.guo@intel.com> <20200326164039.36687-1-jia.guo@intel.com> <20200326164039.36687-4-jia.guo@intel.com> <1948373f-a575-0a37-ec45-b2cac47f069e@intel.com> From: Jeff Guo Message-ID: <3a28be2d-36eb-3ed0-3bc7-f49dd4a0261d@intel.com> Date: Tue, 7 Apr 2020 13:37:08 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" hi, Ori On 4/5/2020 11:56 PM, Ori Kam wrote: > Hi Jeff, > >> -----Original Message----- >> From: Jeff Guo >> Sent: Tuesday, March 31, 2020 11:50 AM >> To: Ori Kam ; xiaolong.ye@intel.com; >> qi.z.zhang@intel.com >> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com; >> simei.su@intel.com >> Subject: Re: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type >> >> yes, Ori, please check the comment below. >> >> >> On 3/30/2020 6:18 PM, Ori Kam wrote: >>> Hi Jeff, >>> >>> My name is Ori 😊 >>> >>> I'm not an expert in GTP so this is just my thinking and maybe I'm >>> missing something, this is why a good explanation helps 😊 >>> >>>> -----Original Message----- >>>> From: Jeff Guo >>>> Sent: Monday, March 30, 2020 11:30 AM >>>> To: Ori Kam ; xiaolong.ye@intel.com; >>>> qi.z.zhang@intel.com >>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com; >>>> simei.su@intel.com >>>> Subject: Re: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU >> type >>>> hi, orika >>>> >>>> >>>> On 3/29/2020 4:44 PM, Ori Kam wrote: >>>>> Hi Jeff, >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: dev On Behalf Of Jeff Guo >>>>>> Sent: Thursday, March 26, 2020 6:41 PM >>>>>> To: xiaolong.ye@intel.com; qi.z.zhang@intel.com >>>>>> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com; >>>>>> simei.su@intel.com; jia.guo@intel.com >>>>>> Subject: [dpdk-dev] [dpdk-dev v2 3/4] app/testpmd: support GTP PDU type >>>>>> >>>>>> Add gtp pdu type configure in the cmdline. >>>>> Why not use ITEM_GTP_PSC_PDU? >>>> I guess you mean ITEM_GTP_PSC_PDU_T, rihgt? We know  we have got >>>> ITEM_GTP_PSC_QFI/ITEM_GTP_PSC_PDU_T but not define the >>>> >>>> spec for them, so what i use is add the spec into the ITEM_GTP_PSC_PDU_T >>>> to let the pdu type to be configured. >>>> >>> Yes you are correct, from rte_flow we have the >> RTE_FLOW_ITEM_TYPE_GTP_PSC >>> Item that include pdu_type. This is the field you need right? >>> >>> In testpmd we have the ITEM_GTP_PSC_PDU_T which should support adding >>> the pdu type. >>> Basically you just need to type the following cmd line: >>> flow create 0 ingress pattern gtp_psc pdu_t is xxx >>> if this command is not working we need to understand why. >>> >>> >> please check the part before this patch as below: >> >>         [ITEM_GTP_PSC_PDU_T] = { >>                 .name = "pdu_t", >>                 .help = "PDU type", >>                .next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), >> item_param), >> >> sure, we got the ITEM_GTP_PSC_PDU_T at prior but the NEXT_ENTRY is >> UNSIGNED, that means we still not implement >> > Sorry I don't understand your comment, what do you mean it is not implemented? > Yes it means that the parameter is should be unsigned value. I mean that if it is a unsigned value, user could not set the pdu_t to be a 0 or 1, or any other we define for that. >> the spec to let the pdu type to be configurable, so what the patch do is >> to fix this issue. > What do you mean configurable? > > Lets start at the beginning, maybe I'm just missing some key point. > What is the PDU type? What values can he hold? > How do you want the command to look like? the command should be like as below flow create 0 ingress pattern eth / ipv4 / udp / gtpu / gtp_psc pdu_t is 0/ ipv4 / end actions rss types ipv4-udp l3-dst-only endkey_len 0queues end / end It is eventually the same as you described about the command before.  User could set pdu_t to be 0 or 1, so what is need is modify NEXT_ENTRY(UNSIGNED) to be "SIGNED" and defined. >> >>>>>> Signed-off-by: Jeff Guo >>>>>> --- >>>>>> v1: >>>>>> no change >>>>>> --- >>>>>> app/test-pmd/cmdline_flow.c | 11 ++++++++++- >>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c >>>>>> index a78154502..c1bd02919 100644 >>>>>> --- a/app/test-pmd/cmdline_flow.c >>>>>> +++ b/app/test-pmd/cmdline_flow.c >>>>>> @@ -49,6 +49,7 @@ enum index { >>>>>> PORT_ID, >>>>>> GROUP_ID, >>>>>> PRIORITY_LEVEL, >>>>>> + GTP_PSC_PDU_T, >>>>>> >>>>>> /* Top-level command. */ >>>>>> SET, >>>>>> @@ -1626,6 +1627,13 @@ static const struct token token_list[] = { >>>>>> .call = parse_int, >>>>>> .comp = comp_none, >>>>>> }, >>>>>> + [GTP_PSC_PDU_T] = { >>>>>> + .name = "{GTPU pdu type}", >>>>>> + .type = "INTEGER", >>>>>> + .help = "gtpu pdu uplink/downlink identifier", >>>>>> + .call = parse_int, >>>>>> + .comp = comp_none, >>>>>> + }, >>>>> Why is this created at this level? >>>>> This looks like is should be written totally differently. >>>> As i said above,  the item we got but spec or say next token still need >>>> to be added, do you mean it should not in the group of Common tokens? If >>>> so, let me think about that, and please explicit your proposal if you >>>> already have one. >>>> >>> Please see above response. >>> >>>>>> /* Top-level command. */ >>>>>> [FLOW] = { >>>>>> .name = "flow", >>>>>> @@ -2615,7 +2623,8 @@ static const struct token token_list[] = { >>>>>> [ITEM_GTP_PSC_PDU_T] = { >>>>>> .name = "pdu_t", >>>>>> .help = "PDU type", >>>>>> - .next = NEXT(item_gtp_psc, NEXT_ENTRY(UNSIGNED), >>>>>> item_param), >>>>>> + .next = NEXT(item_gtp_psc, NEXT_ENTRY(GTP_PSC_PDU_T), >>>>>> + item_param), >>>>>> .args = ARGS(ARGS_ENTRY_HTON(struct >>>>>> rte_flow_item_gtp_psc, >>>>>> pdu_type)), >>>>>> }, >>>>>> -- >>>>>> 2.20.1