From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8ECCBA0351; Mon, 10 Jan 2022 10:36:12 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 181EE41238; Mon, 10 Jan 2022 10:36:12 +0100 (CET) Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by mails.dpdk.org (Postfix) with ESMTP id 3ABD44013F for ; Mon, 10 Jan 2022 10:36:11 +0100 (CET) Received: by mail-io1-f42.google.com with SMTP id l3so16633742iol.10 for ; Mon, 10 Jan 2022 01:36:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=XvVvOxTPK9CZOhe0tahBkYSr4a8DHVmyznUlxBjy9tA=; b=RjXrJs0sZT+Qf7zJvz/5jOFNbWMvW3hwJR6QgZ416H81uVY6AqoPR1DFV9cQnTPZfG kuw14KYDHYsiAXsipO8B8O+ps8DzdxNz7/ZddfciLZPV4WnkGusBsIgi+iLdJCOiZcfj Kx8Lpuaq8BLGxgT7GpQgTS7oaydjFh0aqpEfTZwAhuUgH/JWTrNcgYfmALNyS7wBm2QC GNvHM+qSj4gpqVcH2c5Cafirv+KEYRweH1rYxCQFhMG5RrETmtF7mf4I1WBIs03OyzHe su2KDyXl9rJP6IktOSf492a3qNZIKcGHmKOFPMZixj5Mnf7H/ot/epOzNemsTxc5ulJo 8NCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=XvVvOxTPK9CZOhe0tahBkYSr4a8DHVmyznUlxBjy9tA=; b=DFVsSv2at5zD6BoMkAIMFA6Pbx/k5FSFDLBi/FmLbuIJUSfqEQfFzCB95y7xsBiZf+ EbaL/KZHA6uQZVr3QnBqFShFxELv/HFZRInnBk2TIJEAvMst4favmvjEi/cy5Tm6TsTD 4PQkK4R4py3udZOpw9Z+havRy30ZrWvhXvycCvdTt9mSqG/oA2ZR8PJnR+yHna7uqXKC 6TC4upT2AGATDJTczDUPKJXUUeScM1k5NLgKxispskoVHRCXPF0y77rH1aI7ZzWkfmRh qLmFndA6X1+b57Z5Mw8YKcbaTST62iYomRvOpS4+9bHPliVtkiix7SlwmibZt/KDlIPD WQ0w== X-Gm-Message-State: AOAM532/grFvA1AAqAFRV4peIUb78GHFKzxULuYU4DF7ia0et1N9LiMr aMfx9pptyBaRjtUElZpVMHPlWLSSdXvQLDvXOtI= X-Google-Smtp-Source: ABdhPJxtxgQiJwnIM1nzBRItnOVgJ+R/AfkkH9mBP3a6cRsfbemEVqPu4IFSgzC1gEzF3x+BHRskdIgt9toPKPrAw00= X-Received: by 2002:a5d:87d8:: with SMTP id q24mr34732156ios.154.1641807370505; Mon, 10 Jan 2022 01:36:10 -0800 (PST) MIME-Version: 1.0 References: <20210820082401.3778736-1-jerinj@marvell.com> In-Reply-To: From: Jerin Jacob Date: Mon, 10 Jan 2022 15:05:43 +0530 Message-ID: Subject: Re: [dpdk-dev] [RFC PATCH] ethdev: mtr: enhance input color table features To: "Dumitrescu, Cristian" Cc: "jerinj@marvell.com" , Thomas Monjalon , "Yigit, Ferruh" , Andrew Rybchenko , "dev@dpdk.org" , "arybchenko@solarflare.com" , "lizh@nvidia.com" , "ajit.khaparde@broadcom.com" , "Singh, Jasvinder" , "matan@nvidia.com" , "ndabilpuram@marvell.com" , "skori@marvell.com" , "rkudurumalla@marvell.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Dec 7, 2021 at 11:52 PM Dumitrescu, Cristian wrote: > > HI Jerin, Hi Chistian, > > Sorry for my delay. I am currently in vacation until the beginning on Jan= uary 2022, so my response is slower than usual. I was too on vacation. > > Thanks for the explanation of the capabilities you are trying to enable i= n the API. Based on this, I am reconsidering some of my previous input. Her= e are my current thoughts: > > a) Each meter object can be configured with multiple methods to determine= the input color: IP DSCP and VLAN PCP are just two of them, others are pos= sible (as also listed by you). I think the problem we are trying to solve h= ere is: in case multiple such methods are enabled for a given meter object = AND more than one enabled method is applicable for a particular input packe= t at run-time (e.g. the packet is an IP packet, but it also contains at lea= st one VLAN label, and both IP DSCP and the VLAN PCP methods are enabled fo= r the meter object), how do we decide which method is to be applied? So, IM= O we need to define a priority scheme that always picks a single method: > > - a unique priority for each method; > > - a default method to be used when none of the enabled methods is= applicable to the input packet. > > b) The default method must be usable even when the input packet type is u= nknown to the HW (unsupported), e.g. we should still be able to decide the = input color of a packet that is not an IP packet, such as ARP, and it does = not contain any VLAN labels. For this, I suggest we add a field ins struct = rte_mtr_params called default_input_color of type enum rte_color: > > struct rte_mtr_params { > enum rte_color default_input_color; /* Input color to be = set for the input packet when none of the enabled input color methods is ap= plicable to the current input packet. Ignored when this method is set to th= e color blind method. */ > }; > > c) In terms of methods and their priority, I propose to start with the fo= llowing options, each of which referring to a set of methods, with a clear = way to select a unique method. This is the minimal set needed to support th= e HW capabilities that you mention, this set can be extended as needed in t= he future: > > enum rte_mtr_input_color_method { > RTE_MTR_INPUT_COLOR_METHOD_COLOR_BLIND, /* The input colo= r is always green. The default_input_color is ignored for this method. */ > RTE_ MTR_INPUT_COLOR_METHOD_IP_DSCP, /* If the input pack= et is IPv4 or IPv6, its input color is detected by the DSCP field indexing= into the dscp_table. Otherwise, the default_input_color is applied. */ > RTE_ MTR_INPUT_COLOR_METHOD_OUTER_VLAN_PCP, /* If the inp= ut packet has at least one VLAN label, its input color is detected by the o= utermost VLAN PCP indexing into the vlan_table. Otherwise, the default_inpu= t_color is applied. */ > RTE_ MTR_INPUT_COLOR_METHOD_OUTER_VLAN_PCP_IP_DSCP, /* If= the input packet has at least one VLAN label, its input color is detected = by the outermost VLAN PCP indexing into the vlan_table. Otherwise, if the p= acket is IPv4 or IPv6, its input color is detected by the DSCP field indexi= ng into the dscp_table. Otherwise, the default_input_color is applied. */ > RTE_ MTR_INPUT_COLOR_METHOD_INNER_VLAN_PCP, /* If the inp= ut packet has at least one VLAN label, its input color is detected by the i= nnermost VLAN PCP indexing into the vlan_table. Otherwise, the default_inpu= t_color is applied. */ > RTE_ MTR_INPUT_COLOR_METHOD_INNER_VLAN_PCP_IP_DSCP, /* If= the input packet has at least one VLAN label, its input color is detected = by the innermost VLAN PCP indexing into the vlan_table. Otherwise, if the p= acket is IPv4 or IPv6, its input color is detected by the DSCP field indexi= ng into the dscp_table. Otherwise, the default_input_color is applied. */ > }; > > struct rte_mtr_params { > enum rte_mtr_input_color_method input_color_method; > }; > > d) The above point means that all the protocol dependent tables are indep= endent of each other, so they can all exist at the same time, so we cannot = put all of them into a union or a single unified input color translation ta= ble. In case any of these tables is enabled/required by the input color sch= eme, but it is set to NULL, it must automatically resolve to an "all-green"= table (as it is already the case for the existing dscp_table). > > struct rte_mtr_params { > enum rte_color *ip_dscp_table; /* Used when the IP DSCP i= nput color method is selected for the current input packet. If set to NULL,= it defaults to 64 elements of RTE_COLOR_GREEN. */ > enum rte_color *vlan_table; /* Used when the outermost/in= nermost VLAN PCP input color method is selected for the current input packe= t. If set to NULL, it defaults to all elements being RTE_COLOR_GREEN. */ > }; > > So many details to address in order to avoid any loopholes in this API, b= ut I think this scheme is implementable and robust enough. What do you thin= k? Sounds good. I will next version based on your suggestion. > > > > > > > > > > > > 2. What if the defined input color methodology is not applicable to= the > > current input packet? For example, the user selects VLAN PCP, but some = or > > all of the input packets do not contain any VLAN labels? > > > > > > it picks the rte_mtr_params::fallback_input_color > > > > > We likely need a more complex scheme, see above ;) > > > > > > > > > 3. How do we manage the many packet fields that could be used as th= e > > key for the input color: outer IP DSCP, inner IP DSCP, VLAN 1st label, = VLAN > > 2nd label, MPLS QoS, etc? > > > > - Approach A: Use an enumeration, like you propose, and we can add > > more in the future. Not ideal. > > > > - Approach B: Point to a protocol-agnostic packet field by defining= the > > offset and mask of a 64-bit field. Advantage: we don't need to change t= he > > API every time we add a new option. > > > > > > No strong opinion on doing Approach B. I think, it may be overkill fo= r > > > application and implementation to express. No strong opinion, If you > > > have a strong opinion on that, I will change that to v1. Let me know. > > > > > I would love to have a protocol agnostic scheme, but I am OK to start wit= h a simple scheme for now (if you can call the above scheme as being simple= ;) )and revisit later on if needed. > > > > > > > > > > > > 4. I suggest we use a unified input color table as opposed to one p= er > > protocol, i.e. rename the struct rte_mtr_params::dscp_table to > > color_in_table. The size of this table could be implicitly defined by t= he packet > > field type (in case of enum) or the field mask (in case of protocol-agn= ostic > > field offset and mask), as described above. > > > > > > Will do that. I thought not to do that just because, I don't want to > > > remove the existing dscp_table symbol. > > > Make sense to enable your suggestion. > > > > > > Let me know your views on Questions 1 and 3. I will send the next > > > version based on that. > > > > > Based on the above comments, I no longer thing a unified such table would= work. > > > > > > > > > > > > > > > Regards, > > > > Cristian > > Regards, > Cristian