[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-10 Thread Jan Viktorin
On Wed, 10 Feb 2016 12:38:20 +0100
David Marchand  wrote:

> On Wed, Feb 10, 2016 at 11:20 AM, Jan Viktorin  
> wrote:
> > On Wed, 10 Feb 2016 10:27:14 +0100
> > David Marchand  wrote:  
> >> #define RTE_EAL_PCI_REGISTER(name, d)\
> >> void pciinitfn_ ##name(void);\
> >> void __attribute__((constructor, used)) pciinitfn_ ##name(void)\
> >> {\
> >> rte_eal_pci_register(d);\  
> >
> > I meant
> > rte_eal_pci_register(&(d)->pci_drv);\
> >
> > Perhaps, my assumption that a PCI driver is always referred as pci_drv is 
> > wrong...  
> 
> Well, I suppose we will always have a pci driver embedded in some
> other internal pmd structure.
> So we can always expect it to be called pci_drv ...
> 
> Btw, for drivers like mlx or virtio that need to do some more stuff in
> their constructor (the iopl stuff for virtio is the most interesting
> case, since the pci register happens only if iopl succeeded), we might
> need some RTE_MODULE_INIT for those.
> 
> But in such a case, I think having RTE_MODULE_INIT in all pmds would
> make more sense, and RTE_EAL_PCI_REGISTER looks unneeded ?
> 

The Linux Kernel solves this by allowing both ways. You can do it by
hand and for most cases you can use just a simple oneliner.

So, we can have the RTE_(EAL_)MODULE_INIT for the general case. And then
eg. RTE_EAL_PCI_MODULE_INIT that makes the most common glue code to
register a single driver and who finally calls the RTE_EAL_MODULE_INIT.

Regards
Jan


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-10 Thread David Marchand
On Wed, Feb 10, 2016 at 11:20 AM, Jan Viktorin  
wrote:
> On Wed, 10 Feb 2016 10:27:14 +0100
> David Marchand  wrote:
>> #define RTE_EAL_PCI_REGISTER(name, d)\
>> void pciinitfn_ ##name(void);\
>> void __attribute__((constructor, used)) pciinitfn_ ##name(void)\
>> {\
>> rte_eal_pci_register(d);\
>
> I meant
> rte_eal_pci_register(&(d)->pci_drv);\
>
> Perhaps, my assumption that a PCI driver is always referred as pci_drv is 
> wrong...

Well, I suppose we will always have a pci driver embedded in some
other internal pmd structure.
So we can always expect it to be called pci_drv ...

Btw, for drivers like mlx or virtio that need to do some more stuff in
their constructor (the iopl stuff for virtio is the most interesting
case, since the pci register happens only if iopl succeeded), we might
need some RTE_MODULE_INIT for those.

But in such a case, I think having RTE_MODULE_INIT in all pmds would
make more sense, and RTE_EAL_PCI_REGISTER looks unneeded ?

-- 
David Marchand


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-10 Thread Jan Viktorin
On Wed, 10 Feb 2016 10:27:14 +0100
David Marchand  wrote:

> On Wed, Feb 10, 2016 at 9:51 AM, David Marchand
>  wrote:
> > On Tue, Feb 9, 2016 at 6:05 PM, Jan Viktorin  
> > wrote:  
> >> What about introducing a macro for this?
> >>
> >> RTE_REGISTER_PCI_DRIVER(rte_qad_pmd);  
> >
> > Yes.  
> 
> The only problem here, is that rte_qad_pmd is a crypto structure (same
> problem with ethdev), so I can't just pass it to eal.
> I can't just pass the pci driver, for the cases where multiple drivers
> are registered in a single file (look at ixgbe driver).
> 
> So, how about :
> 
> In rte_pci.h :
> 
> #define RTE_EAL_PCI_REGISTER(name, d)\
> void pciinitfn_ ##name(void);\
> void __attribute__((constructor, used)) pciinitfn_ ##name(void)\
> {\
> rte_eal_pci_register(d);\

I meant
rte_eal_pci_register(&(d)->pci_drv);\

Perhaps, my assumption that a PCI driver is always referred as pci_drv is 
wrong...

> }
> 
> Then, in qat driver :
> RTE_EAL_PCI_REGISTER(qat, _qat_pmd.pci_drv);
> 

However, I think that this is a detail. Passing rte_pci_driver is good.

Regards
Jan

-- 
   Jan Viktorin  E-mail: Viktorin at RehiveTech.com
   System Architect  Web:www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-10 Thread David Marchand
On Wed, Feb 10, 2016 at 9:51 AM, David Marchand
 wrote:
> On Tue, Feb 9, 2016 at 6:05 PM, Jan Viktorin  
> wrote:
>> What about introducing a macro for this?
>>
>> RTE_REGISTER_PCI_DRIVER(rte_qad_pmd);
>
> Yes.

The only problem here, is that rte_qad_pmd is a crypto structure (same
problem with ethdev), so I can't just pass it to eal.
I can't just pass the pci driver, for the cases where multiple drivers
are registered in a single file (look at ixgbe driver).

So, how about :

In rte_pci.h :

#define RTE_EAL_PCI_REGISTER(name, d)\
void pciinitfn_ ##name(void);\
void __attribute__((constructor, used)) pciinitfn_ ##name(void)\
{\
rte_eal_pci_register(d);\
}

Then, in qat driver :
RTE_EAL_PCI_REGISTER(qat, _qat_pmd.pci_drv);


-- 
David Marchand


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-10 Thread David Marchand
On Tue, Feb 9, 2016 at 6:05 PM, Jan Viktorin  wrote:
> Maybe, a better subject?
>
> drivers: init pdev drivers in constructors

Why not, I will try to find a best one, and if I can't, I will go with this.

> On Fri, 29 Jan 2016 15:08:30 +0100
> David Marchand  wrote:
>> -static int
>> -rte_qat_pmd_init(const char *name __rte_unused, const char *params 
>> __rte_unused)
>> +/* Driver registration */
>> +static void __attribute__((constructor, used))
>> +rte_qat_pmd_init(void)
>>  {
>>   PMD_INIT_FUNC_TRACE();
>>   rte_eal_pci_register(_qat_pmd.pci_drv);
>> - return 0;
>>  }
>> -
>> -static struct rte_driver pmd_qat_drv = {
>> - .type = PMD_PDEV,
>> - .init = rte_qat_pmd_init,
>> -};
>> -
>> -PMD_REGISTER_DRIVER(pmd_qat_drv);
>
> What about introducing a macro for this?
>
> RTE_REGISTER_PCI_DRIVER(rte_qad_pmd);

Yes.


-- 
David Marchand


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-02-09 Thread Jan Viktorin
Maybe, a better subject?

drivers: init pdev drivers in constructors

On Fri, 29 Jan 2016 15:08:30 +0100
David Marchand  wrote:

> Now that pdev drivers have been converted to pci drivers, there is nothing
> left in their init functions that can't go in a constructor.
> pdev / vdev drivers init order is changed by this commit, but I can't see
> why we would need to preserve it.
> 
> Signed-off-by: David Marchand 
> ---
>  drivers/crypto/qat/rte_qat_cryptodev.c | 13 +++-
>  drivers/net/bnx2x/bnx2x_ethdev.c   | 26 +++-
>  drivers/net/cxgbe/cxgbe_ethdev.c   | 19 +++---
>  drivers/net/e1000/em_ethdev.c  | 13 +++-
>  drivers/net/e1000/igb_ethdev.c | 34 
>  drivers/net/enic/enic_ethdev.c | 18 +++--
>  drivers/net/fm10k/fm10k_ethdev.c   | 19 +++---
>  drivers/net/i40e/i40e_ethdev.c | 19 +++---
>  drivers/net/i40e/i40e_ethdev_vf.c  | 19 +++---
>  drivers/net/ixgbe/ixgbe_ethdev.c   | 36 
> +++---
>  drivers/net/mlx4/mlx4.c| 19 +++---
>  drivers/net/mlx5/mlx5.c| 19 +++---
>  drivers/net/nfp/nfp_net.c  | 14 +++--
>  drivers/net/virtio/virtio_ethdev.c | 22 -
>  drivers/net/vmxnet3/vmxnet3_ethdev.c   | 18 +++--
>  15 files changed, 47 insertions(+), 261 deletions(-)
> 
> diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c 
> b/drivers/crypto/qat/rte_qat_cryptodev.c
> index 6853aee..ded5d60 100644
> --- a/drivers/crypto/qat/rte_qat_cryptodev.c
> +++ b/drivers/crypto/qat/rte_qat_cryptodev.c
> @@ -124,17 +124,10 @@ static struct rte_cryptodev_driver rte_qat_pmd = {
>   .dev_private_size = sizeof(struct qat_pmd_private),
>  };
>  
> -static int
> -rte_qat_pmd_init(const char *name __rte_unused, const char *params 
> __rte_unused)
> +/* Driver registration */
> +static void __attribute__((constructor, used))
> +rte_qat_pmd_init(void)
>  {
>   PMD_INIT_FUNC_TRACE();
>   rte_eal_pci_register(_qat_pmd.pci_drv);
> - return 0;
>  }
> -
> -static struct rte_driver pmd_qat_drv = {
> - .type = PMD_PDEV,
> - .init = rte_qat_pmd_init,
> -};
> -
> -PMD_REGISTER_DRIVER(pmd_qat_drv);

What about introducing a macro for this?

RTE_REGISTER_PCI_DRIVER(rte_qad_pmd);

Regards
Jan


[dpdk-dev] [PATCH 3/9] drivers: no more pdev drivers

2016-01-29 Thread David Marchand
Now that pdev drivers have been converted to pci drivers, there is nothing
left in their init functions that can't go in a constructor.
pdev / vdev drivers init order is changed by this commit, but I can't see
why we would need to preserve it.

Signed-off-by: David Marchand 
---
 drivers/crypto/qat/rte_qat_cryptodev.c | 13 +++-
 drivers/net/bnx2x/bnx2x_ethdev.c   | 26 +++-
 drivers/net/cxgbe/cxgbe_ethdev.c   | 19 +++---
 drivers/net/e1000/em_ethdev.c  | 13 +++-
 drivers/net/e1000/igb_ethdev.c | 34 
 drivers/net/enic/enic_ethdev.c | 18 +++--
 drivers/net/fm10k/fm10k_ethdev.c   | 19 +++---
 drivers/net/i40e/i40e_ethdev.c | 19 +++---
 drivers/net/i40e/i40e_ethdev_vf.c  | 19 +++---
 drivers/net/ixgbe/ixgbe_ethdev.c   | 36 +++---
 drivers/net/mlx4/mlx4.c| 19 +++---
 drivers/net/mlx5/mlx5.c| 19 +++---
 drivers/net/nfp/nfp_net.c  | 14 +++--
 drivers/net/virtio/virtio_ethdev.c | 22 -
 drivers/net/vmxnet3/vmxnet3_ethdev.c   | 18 +++--
 15 files changed, 47 insertions(+), 261 deletions(-)

diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c 
b/drivers/crypto/qat/rte_qat_cryptodev.c
index 6853aee..ded5d60 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -124,17 +124,10 @@ static struct rte_cryptodev_driver rte_qat_pmd = {
.dev_private_size = sizeof(struct qat_pmd_private),
 };

-static int
-rte_qat_pmd_init(const char *name __rte_unused, const char *params 
__rte_unused)
+/* Driver registration */
+static void __attribute__((constructor, used))
+rte_qat_pmd_init(void)
 {
PMD_INIT_FUNC_TRACE();
rte_eal_pci_register(_qat_pmd.pci_drv);
-   return 0;
 }
-
-static struct rte_driver pmd_qat_drv = {
-   .type = PMD_PDEV,
-   .init = rte_qat_pmd_init,
-};
-
-PMD_REGISTER_DRIVER(pmd_qat_drv);
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 916b9da..a4b1599 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -523,31 +523,11 @@ static struct eth_driver rte_bnx2xvf_pmd = {
.dev_private_size = sizeof(struct bnx2x_softc),
 };

-static int rte_bnx2x_pmd_init(const char *name __rte_unused,
- const char *params __rte_unused)
+/* Driver registration */
+static void __attribute__((constructor, used))
+rte_bnx2x_pmd_init(void)
 {
PMD_INIT_FUNC_TRACE();
rte_eal_pci_register(_bnx2x_pmd.pci_drv);
-   return 0;
-}
-
-static int rte_bnx2xvf_pmd_init(const char *name __rte_unused,
-   const char *params __rte_unused)
-{
-   PMD_INIT_FUNC_TRACE();
rte_eal_pci_register(_bnx2xvf_pmd.pci_drv);
-   return 0;
 }
-
-static struct rte_driver rte_bnx2x_driver = {
-   .type = PMD_PDEV,
-   .init = rte_bnx2x_pmd_init,
-};
-
-static struct rte_driver rte_bnx2xvf_driver = {
-   .type = PMD_PDEV,
-   .init = rte_bnx2xvf_pmd_init,
-};
-
-PMD_REGISTER_DRIVER(rte_bnx2x_driver);
-PMD_REGISTER_DRIVER(rte_bnx2xvf_driver);
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index e425ef2..9654ced 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -858,23 +858,10 @@ static struct eth_driver rte_cxgbe_pmd = {
.dev_private_size = sizeof(struct port_info),
 };

-/*
- * Driver initialization routine.
- * Invoked once at EAL init time.
- * Register itself as the [Poll Mode] Driver of PCI CXGBE devices.
- */
-static int rte_cxgbe_pmd_init(const char *name __rte_unused,
- const char *params __rte_unused)
+/* Driver registration */
+static void __attribute__((constructor, used))
+rte_cxgbe_pmd_init(void)
 {
CXGBE_FUNC_TRACE();
rte_eal_pci_register(_cxgbe_pmd.pci_drv);
-   return 0;
 }
-
-static struct rte_driver rte_cxgbe_driver = {
-   .name = "cxgbe_driver",
-   .type = PMD_PDEV,
-   .init = rte_cxgbe_pmd_init,
-};
-
-PMD_REGISTER_DRIVER(rte_cxgbe_driver);
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 973ec97..19b2645 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -348,11 +348,11 @@ static struct eth_driver rte_em_pmd = {
.dev_private_size = sizeof(struct e1000_adapter),
 };

-static int
-rte_em_pmd_init(const char *name __rte_unused, const char *params __rte_unused)
+/* Driver registration */
+static void __attribute__((constructor, used))
+rte_em_pmd_init(void)
 {
rte_eal_pci_register(_em_pmd.pci_drv);
-   return 0;
 }

 static int
@@ -1738,10 +1738,3 @@ eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
e1000_update_mc_addr_list(hw, (u8 *)mc_addr_set,