On Mon, Apr 27, 2020 at 10:29:48PM +0530, Jerin Jacob wrote: > External Email > > ---------------------------------------------------------------------- > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <[email protected]> wrote: > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote: > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <[email protected]> > > > wrote: > > >> > > >> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote: > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: Yigit, Ferruh <[email protected]> > > >>>> Sent: Saturday, April 25, 2020 9:09 PM > > >>>> To: Dumitrescu, Cristian <[email protected]>; Nithin > > >>>> Dabilpuram > > >>>> <[email protected]>; Singh, Jasvinder <[email protected]>; > > >>>> Thomas Monjalon <[email protected]>; Andrew Rybchenko > > >>>> <[email protected]> > > >>>> Cc: [email protected]; [email protected]; [email protected]; Nithin > > >>>> Dabilpuram <[email protected]> > > >>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config > > >>>> in pkt > > >>>> mode > > >>>> > > >>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote: > > >>>>> > > >>>>> > > >>>>>> -----Original Message----- > > >>>>>> From: Nithin Dabilpuram <[email protected]> > > >>>>>> Sent: Wednesday, April 22, 2020 6:21 PM > > >>>>>> To: Singh, Jasvinder <[email protected]>; Dumitrescu, > > >>>>>> Cristian > > >>>>>> <[email protected]>; Thomas Monjalon > > >>>>>> <[email protected]>; Yigit, Ferruh <[email protected]>; Andrew > > >>>>>> Rybchenko <[email protected]> > > >>>>>> Cc: [email protected]; [email protected]; [email protected]; Nithin > > >>>>>> Dabilpuram <[email protected]> > > >>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in > > >>>>>> pkt > > >>>>>> mode > > >>>>>> > > >>>>>> From: Nithin Dabilpuram <[email protected]> > > >>>>>> > > >>>>>> Some NIC hardware support shaper to work in packet mode i.e > > >>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as > > >>>>>> opposed to default bytes per second (BPS). Hence this patch > > >>>>>> adds support to configure shared or private shaper in packet mode, > > >>>>>> provide rate in PPS and add related tm capabilities in > > >>>>>> port/level/node > > >>>>>> capability structures. > > >>>>>> > > >>>>>> This patch also updates tm port/level/node capability structures with > > >>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte > > >>>>>> mode > > >>>>>> and private/shared shaper byte mode. > > >>>>>> > > >>>>>> SoftNIC PMD is also updated with new capabilities. > > >>>>>> > > >>>>>> Signed-off-by: Nithin Dabilpuram <[email protected]> > > >>>>>> --- > > >>>>>> v3..v4: > > >>>>>> - Update text under packet_mode as per Cristian. > > >>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments. > > >>>>>> - Add error enum > > >>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE > > >>>>>> - Fix shaper_profile_check() with packet mode check > > >>>>>> - Fix typo's > > >>>>>> > > >>>>> > > >>>>> Acked-by: Cristian Dumitrescu <[email protected]> > > >>>>> > > >>>> > > >>>> Hi Nithin, > > >>>> > > >>>> It looks like patch is causing ABI break, I am getting following > > >>>> warning [1], > > >>>> can you please check? > > >>>> > > >>>> [1] > > >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=xJB0Qb2Q-1bl0hEDeknUjJqrCDc3z-h0F0e7kj8KvvI&s=R6xtRQRRIIzAilc5z52oYjyHNlvvJB_9SUsKBkpPC6g&e= > > >>>> > > >>> > > >>> > > >>> Hi Ferruh, > > >>> > > >>> The RTE_TM API is marked as experimental, but it looks that this was > > >>> not correctly marked when __rte_experimental ABI checker was introduced. > > >>> > > >>> It is marked as experimental at the top of the rte_tm.h, similarly to > > >>> other APIs introduced around same time, but it was not correctly picked > > >>> up by the ABI check procedure when later introduced, so > > >>> __rte_experimental was not added to every function. > > >>> > > >> > > >> :( > > >> > > >> Is it time to mature them? > > >> > > >> As you said they are not marked as experimental both in header file > > >> (function > > >> declarations) and .map file. > > >> > > >> The problem is, they are not marked as experimental in DPDK_20.0 ABI > > >> (v19.11), > > >> so marking them as experimental now will break the ABI. Not sure what to > > >> do, > > >> cc'ed a few ABI related names for comment. > > >> > > >> For me, we need to proceed as the experimental tag removed and APIs > > >> become > > >> mature starting from v19.11, since this is what happened in practice, > > >> and remove > > >> a few existing being experimental references in the doxygen comments. > > > > > > I think, accidentally we can not make a library as NON-experimental. > > > TM never went through experimental to mature transition(see git log > > > lib/librte_ethdev/rte_tm.h) > > > It was a bug to not mark as experimental in each function in the ABI > > > process. > > > Some of the features like packet marking are not even implemented by any > > > HW. > > > I think, we can make API stable only all the features are implemented > > > by one or two HW. > > > > Fair enough, specially if the API is not ready yet. > > > > But they were part of stable ABI, and marking them as experimental now will > > break the old applications using these APIs. > > it is still marked as EXPERIMENTAL everywhere and API is not ready yet. > Anyway, we need to break the ABI to make it work on various HW. > I am not sure what to do? > IMO, We need to send a patch as Fixes: for the bug of not adding > __rte_experimental in each function. > > Traffic Management API - EXPERIMENTAL > M: Cristian Dumitrescu <[email protected]> > T: git://dpdk.org/next/dpdk-next-qos > F: lib/librte_ethdev/rte_tm*
Ray, Neil, David, Luca, Kevin, Ferruh Any thoughts on this proposal ? If it is fine, I can send a "Fixes:" patch to update experimental attribute in rte_tm.h for all functions so that 20.05 is having the right marking. > > > > > > > >> > > >> Ray, Neil, David, Luca, Kevin, what do you think? > >
