On Wed, Nov 17, 2021 at 5:30 PM Jerin Jacob <jerinjac...@gmail.com> wrote: > > On Mon, Oct 11, 2021 at 8:53 PM Dumitrescu, Cristian > <cristian.dumitre...@intel.com> wrote: > > > > Hi Jerin, > Hi Cristian,
Could you share your feedback so that I can send the v1? > > > > > > > -----Original Message----- > > > From: jer...@marvell.com <jer...@marvell.com> > > > Sent: Friday, August 20, 2021 9:24 AM > > > To: Dumitrescu, Cristian <cristian.dumitre...@intel.com>; Thomas Monjalon > > > <tho...@monjalon.net>; Yigit, Ferruh <ferruh.yi...@intel.com>; Andrew > > > Rybchenko <andrew.rybche...@oktetlabs.ru> > > > Cc: dev@dpdk.org; arybche...@solarflare.com; l...@nvidia.com; > > > ajit.khapa...@broadcom.com; Singh, Jasvinder > > > <jasvinder.si...@intel.com>; ma...@nvidia.com; > > > ndabilpu...@marvell.com; sk...@marvell.com; rkuduruma...@marvell.com; > > > Jerin Jacob <jer...@marvell.com> > > > Subject: [dpdk-dev] [RFC PATCH] ethdev: mtr: enhance input color table > > > features > > > > > > From: Jerin Jacob <jer...@marvell.com> > > > > > > Currently, meter object supports only DSCP based on input color table, > > > The patch enhance that to support VLAN based input color table, > > > color table based on inner field for the tunnel use case, and support > > > for fallback color per meter if packet based on a different field. > > > > > > All of the above features are exposed through capability and added > > > additional capability to specify the implementation supports > > > more than one input color table per ethdev port. > > > > > > Signed-off-by: Jerin Jacob <jer...@marvell.com> > > > > 2.33.0 > > > > Allowing the configuration of the packet input color based on multiple > > protocols as opposed to just IP DSCP field makes sense to me. > > > > Apologies for the delay in response. This was missed the 21.11 > timeframe so I don't bother replying. Reviving back. > > > A few questions/suggestions: > > > > 1. How do we decide which field/protocol takes precedence to define the > > packet input color? You are enabling 2 possible options so far, i.e. VLAN > > tag PCP field and IP DSCP field, which one is to be set for the current > > meter object? Using the capabilities to decide is confusing, as a specific > > meter object might be able to support multiple or even all the possible > > options (e.g. the meter object is fine with either IP DSCP or VLAN PCP). > > Therefore, we need a clear mechanism to unequivocally pick one; I think > > the user must explicitly define which input color methodology is to be used > > by explicitly setting a field (to be added) in the meter object parameter > > structure. > > Currently, in our HW, Each meter object needs to tell which pre-color > scheme it is going to use(IP DSCP or VLAN PCP). This is OK in the > overall scheme of things as the meter object is connected to rte_flow. > So, > rte_flow pattern can decide the steering to meter. Having said that, > it is possible to have array for input color as you suggested. If > choose that, path, We need to add capability tell implementation > supports only one input color per meter object. Let me know if this is > OK. > > > > > > 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 > > > > > 3. How do we manage the many packet fields that could be used as the 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 the > > API every time we add a new option. > > No strong opinion on doing Approach B. I think, it may be overkill for > 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. > > > > > > 4. I suggest we use a unified input color table as opposed to one per > > 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 the > > packet field type (in case of enum) or the field mask (in case of > > protocol-agnostic 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. > > > > > > > Regards, > > Cristian
