Re: [dpdk-dev] [PATCH v7 2/9] eal: introduce RTE common initialization level

[Ori Kam <orika@mellanox.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> On 7/20/2020 6:26 PM, Ori Kam wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> On 7/20/2020 5:48 PM, Thomas Monjalon wrote:
> >>> 20/07/2020 18:21, Ferruh Yigit:
> >>>> On 7/17/2020 2:49 PM, Parav Pandit wrote:
> >>>>> Currently mlx5_common uses CLASS priority to initialize
> >>>>> common code before initializing the PMD.
> >>>>> However mlx5_common is not really a class, it is the pre-initialization
> >>>>> code needed for the PMDs.
> >>>>>
> >>>>> In subsequent patch a needed initialization sequence is:
> >>>>> (a) Initialize bus (say pci)
> >>>>> (b) Initialize common code of a driver (mlx5_common)
> >>>>> (c) Register mlx5 class PMDs (mlx5 net, mlx5 vdpa)
> >>>>> Information registered by these PMDs is used by mlx5_bus_pci PMD.
> >>>>> This mlx5 class PMDs should not confused with rte_class.
> >>>>> (d) Register mlx5 PCI bus PMD
> >>>>>
> >>>>> Hence, introduce a new RTE priority level RTE_PRIO_COMMON which
> >>>>> can be used for common initialization and RTE_PRIO_CLASS by mlx5
> PMDs
> >>>>> for class driver initialization.
> >>>>>
> >>>>> Signed-off-by: Parav Pandit <parav@mellanox.com>
> >>>>> Acked-by: Matan Azrad <matan@mellanox.com>
> >>>>> ---
> >>>>> Changelog:
> >>>>> v2->v3:
> >>>>>  - new patch
> >>>>> ---
> >>>>>  lib/librte_eal/include/rte_common.h | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/lib/librte_eal/include/rte_common.h
> >> b/lib/librte_eal/include/rte_common.h
> >>>>> index 8f487a563..522afe58e 100644
> >>>>> --- a/lib/librte_eal/include/rte_common.h
> >>>>> +++ b/lib/librte_eal/include/rte_common.h
> >>>>> @@ -135,6 +135,7 @@ typedef uint16_t unaligned_uint16_t;
> >>>>>
> >>>>>  #define RTE_PRIORITY_LOG 101
> >>>>>  #define RTE_PRIORITY_BUS 110
> >>>>> +#define RTE_PRIORITY_COMMON 119
> >>>>>  #define RTE_PRIORITY_CLASS 120
> >>>>>  #define RTE_PRIORITY_LAST 65535
> >>>>>
> >>>>>
> >>>>
> >>>> I guess the name "common" selected because of the intention to use it by
> >> the
> >>>> common piece of the driver, but only from eal perspective the name
> >>>> "PRIORITY_COMMON" looks so vague, it doesn't describe any purpose.
> >>>
> >>> You're right.
> >>>
> >>>> Also the value doesn't leave any gap between the class priority, what else
> >> can
> >>>> be needed in the future in between, right?
> >>>
> >>> And we can imagine a bus requiring a common lib
> >>> to be initialized before.
> >>>
> >>>> @Thomas, @David, I am reluctant to get this eal change through the
> next-
> >> net, can
> >>>> you please review/ack it first?
> >>>
> >>> What about skipping this patch and using "RTE_PRIORITY_CLASS - 1"
> >>> in the code?
> >>>
> >>
> >> For now I think it is OK, in the future if more priority dependency involved
> we
> >> can define the macro.
> >>
> > I'm concerned what if someone else will add priority there may be conflict
> and.
> > Also using -1 means that no one knows that there is use in such priority.
> 
> Is the new constructor priority level a common need, or just specific to this
> "mlx5 pci" bus usage?

I think it can be used by any other vendor that needs a bus for his own PMDs.

> I understand the need of new constructor priority level, but it seems it is not
> clear enough to define it as a generic level, that is why I believe this can be
> local to PMD for now, otherwise your concerns looks valid.
> 
If I understand it can be local to the bus, it is not exposed to the PMD.
But like I said above this may be used by other vendors that wish to combine
some of their devices.

> Also I am thinking if the multi class support can be done as generic bus
> feature, instead of defining a new PMD specific bus for it, but I am aware it is
> too late for this question.
> 
> > What about setting the value to 115?
> >