DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
@ 2016-02-02 14:27 krytarowski
  2016-02-02 14:27 ` [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API krytarowski
  2016-02-03  8:47 ` [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit David Marchand
  0 siblings, 2 replies; 9+ messages in thread
From: krytarowski @ 2016-02-02 14:27 UTC (permalink / raw)
  To: dev

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

This change enables drivers needing custom pci (de)initialization functions
through the standard callback overloading.

For example:

 /*
  * virtual function driver struct
  */
 static struct eth_driver rte_DRIVER_pmd = {
 	{
 		.name = "rte_DRIVER_pmd",
 		.id_table = pci_id_DRIVER_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
		.devinit = DRIVER_pci_devinit,
 	},
 	.eth_dev_init = eth_DRIVER_dev_init,
 	.dev_private_size = sizeof(struct DRIVER),
 };

Use-case is as follows: NIC offers several pci virtual functions, while
one of them is to be treated as port, we need to configure the rest in a
specific way for particular device for full interface (port) functionality.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 lib/librte_ether/rte_ethdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 756b234..ac4aeab 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -351,8 +351,10 @@ rte_eth_dev_uninit(struct rte_pci_device *pci_dev)
 void
 rte_eth_driver_register(struct eth_driver *eth_drv)
 {
-	eth_drv->pci_drv.devinit = rte_eth_dev_init;
-	eth_drv->pci_drv.devuninit = rte_eth_dev_uninit;
+	if (!eth_drv->pci_drv.devinit)
+		eth_drv->pci_drv.devinit = rte_eth_dev_init;
+	if (!eth_drv->pci_drv.devuninit)
+		eth_drv->pci_drv.devuninit = rte_eth_dev_uninit;
 	rte_eal_pci_register(&eth_drv->pci_drv);
 }
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API
  2016-02-02 14:27 [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit krytarowski
@ 2016-02-02 14:27 ` krytarowski
  2016-02-11 16:56   ` Panu Matilainen
  2016-02-03  8:47 ` [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit David Marchand
  1 sibling, 1 reply; 9+ messages in thread
From: krytarowski @ 2016-02-02 14:27 UTC (permalink / raw)
  To: dev

From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>

Once pci_drv.devinit is overloaded, it's a function used in the original
rte_eth_dev_init(), still reusable in altered versions.

Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
---
 lib/librte_ether/rte_ethdev.c |  2 +-
 lib/librte_ether/rte_ethdev.h | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ac4aeab..7f5e741 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -214,7 +214,7 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
 	return eth_dev;
 }
 
-static int
+int
 rte_eth_dev_create_unique_device_name(char *name, size_t size,
 		struct rte_pci_device *pci_dev)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8710dd7..b19db9d 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3880,6 +3880,24 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
 			 uint16_t queue_id, size_t size,
 			 unsigned align, int socket_id);
 
+/**
+ * Create unique device name
+ *
+ * @param name
+ *   The port identifier of the Ethernet device.
+ * @param size
+ *   Maximum string length of the generated name
+ * @param pci_dev
+ *   PCI device pointer
+ *
+ * @return
+ *   - 0: Success.
+ *   - <0: Error during generatin
+ *   - -EINVAL: Invalid input parameters.
+ */
+int rte_eth_dev_create_unique_device_name(char *name, size_t size,
+					  struct rte_pci_device *pci_dev);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
  2016-02-02 14:27 [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit krytarowski
  2016-02-02 14:27 ` [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API krytarowski
@ 2016-02-03  8:47 ` David Marchand
  2016-02-03 11:39   ` Kamil Rytarowski
  1 sibling, 1 reply; 9+ messages in thread
From: David Marchand @ 2016-02-03  8:47 UTC (permalink / raw)
  To: krytarowski; +Cc: dev

Hello,

On Tue, Feb 2, 2016 at 3:27 PM,  <krytarowski@caviumnetworks.com> wrote:
> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>
> This change enables drivers needing custom pci (de)initialization functions
> through the standard callback overloading.
>
> For example:
>
>  /*
>   * virtual function driver struct
>   */
>  static struct eth_driver rte_DRIVER_pmd = {
>         {
>                 .name = "rte_DRIVER_pmd",
>                 .id_table = pci_id_DRIVER_map,
>                 .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
>                 .devinit = DRIVER_pci_devinit,
>         },
>         .eth_dev_init = eth_DRIVER_dev_init,
>         .dev_private_size = sizeof(struct DRIVER),
>  };
>
> Use-case is as follows: NIC offers several pci virtual functions, while
> one of them is to be treated as port, we need to configure the rest in a
> specific way for particular device for full interface (port) functionality.

Mmm, why don't you register a custom pci driver rather than a eth_driver ?
And do your custom things in its devinit function ?


-- 
David Marchand

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
  2016-02-03  8:47 ` [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit David Marchand
@ 2016-02-03 11:39   ` Kamil Rytarowski
  2016-02-03 14:08     ` David Marchand
  0 siblings, 1 reply; 9+ messages in thread
From: Kamil Rytarowski @ 2016-02-03 11:39 UTC (permalink / raw)
  To: David Marchand; +Cc: dev



W dniu 03.02.2016 o 09:47, David Marchand pisze:
> Hello,
Hello,

> On Tue, Feb 2, 2016 at 3:27 PM,  <krytarowski@caviumnetworks.com> wrote:
>> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>>
>> This change enables drivers needing custom pci (de)initialization functions
>> through the standard callback overloading.
>>
>> For example:
>>
>>   /*
>>    * virtual function driver struct
>>    */
>>   static struct eth_driver rte_DRIVER_pmd = {
>>          {
>>                  .name = "rte_DRIVER_pmd",
>>                  .id_table = pci_id_DRIVER_map,
>>                  .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
>>                  .devinit = DRIVER_pci_devinit,
>>          },
>>          .eth_dev_init = eth_DRIVER_dev_init,
>>          .dev_private_size = sizeof(struct DRIVER),
>>   };
>>
>> Use-case is as follows: NIC offers several pci virtual functions, while
>> one of them is to be treated as port, we need to configure the rest in a
>> specific way for particular device for full interface (port) functionality.
> Mmm, why don't you register a custom pci driver rather than a eth_driver ?
Both Virtual Functions are of eth_driver type and they share the same 
PCI ID.

They are like 2 (or more) NIC devices working in tandem (it's not 
bonding), with a possibility to join into a single one and offer 
additional functionality. In that case one is master and the other is 
donor of functionality, like additional queue sets.

> And do your custom things in its devinit function ?
>
>
Yes, I'm doing custom things.

I'm requesting from PF the mode of the device to be initialized. This 
part is handled dynamically and depends of the current configuration in PF.

In my use-case there are two device types: primary (master) and 
secondary (slave). For the primary VF I'm creating a DPDK port normally, 
for secondary I retain configured PCI device for further reuse (and 
there is no port created).

I cannot overload functions called after .devinit() as the secondary 
devices will be wrongly treated like a DPDK port.


My use-case doesn't need modified API to be functional.

The missing handling of overloaded .devinit and .devuninit looks like a 
bug - there is API for it, but it will keep overwriting the pointers 
with local functions (rte_eth_dev_init(), rte_eth_dev_uninit()).

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
  2016-02-03 11:39   ` Kamil Rytarowski
@ 2016-02-03 14:08     ` David Marchand
  2016-02-03 15:49       ` Kamil Rytarowski
  0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2016-02-03 14:08 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: dev

On Wed, Feb 3, 2016 at 12:39 PM, Kamil Rytarowski
<krytarowski@caviumnetworks.com> wrote:
> W dniu 03.02.2016 o 09:47, David Marchand pisze:
>> And do your custom things in its devinit function ?
>
> I'm requesting from PF the mode of the device to be initialized. This part
> is handled dynamically and depends of the current configuration in PF.
>
> In my use-case there are two device types: primary (master) and secondary
> (slave). For the primary VF I'm creating a DPDK port normally, for secondary
> I retain configured PCI device for further reuse (and there is no port
> created).

Well, again, if you don't want to associate a port to this pci
resource, why are you registering a eth_driver ?
A eth_driver driver supposes you have a 1 - 1 relation between ethdev
and pci resource.

For your case, register a pci driver, then in your pci probing
function (.devinit), depending on what you want to do, you can either
do nothing (?) or create one or more ethdevs (see mlx* and cxgbe
drivers).



-- 
David Marchand

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
  2016-02-03 14:08     ` David Marchand
@ 2016-02-03 15:49       ` Kamil Rytarowski
  2016-02-03 15:59         ` Kamil Rytarowski
  0 siblings, 1 reply; 9+ messages in thread
From: Kamil Rytarowski @ 2016-02-03 15:49 UTC (permalink / raw)
  To: David Marchand; +Cc: dev



W dniu 03.02.2016 o 15:08, David Marchand pisze:
> On Wed, Feb 3, 2016 at 12:39 PM, Kamil Rytarowski
> <krytarowski@caviumnetworks.com> wrote:
>> W dniu 03.02.2016 o 09:47, David Marchand pisze:
>>> And do your custom things in its devinit function ?
>> I'm requesting from PF the mode of the device to be initialized. This part
>> is handled dynamically and depends of the current configuration in PF.
>>
>> In my use-case there are two device types: primary (master) and secondary
>> (slave). For the primary VF I'm creating a DPDK port normally, for secondary
>> I retain configured PCI device for further reuse (and there is no port
>> created).
> Well, again, if you don't want to associate a port to this pci
> resource, why are you registering a eth_driver ?
> A eth_driver driver supposes you have a 1 - 1 relation between ethdev
> and pci resource.
In my use-case one DPDK port optionally manages more than single PCI 
resource, and these PCI resources compose single interface.

Another example of overloaded .devinit is in app/test/test_pci.c:

/*
  * PCI test
  * ========
  *
  * - Register a driver with a ``devinit()`` function.
  *
  * - Dump all PCI devices.
  *
  * - Check that the ``devinit()`` function is called at least once.
  */

With the current implementation it won't work, as .devinit callback will 
be overwritten by the internal function.

> For your case, register a pci driver, then in your pci probing
> function (.devinit), depending on what you want to do, you can either
> do nothing (?) or create one or more ethdevs (see mlx* and cxgbe
> drivers).
>
>
>

This is what I'm doing right now.

I need to initialize PCI bars and interrupts (resources) - all having 
the same PCI ID and their functionality depending upon PF configuration. 
Depending on this state, I'm making further decisions in 
DRIVER_devinit() and whether to make from it a port or a resource to 
reuse by a master port.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit
  2016-02-03 15:49       ` Kamil Rytarowski
@ 2016-02-03 15:59         ` Kamil Rytarowski
  0 siblings, 0 replies; 9+ messages in thread
From: Kamil Rytarowski @ 2016-02-03 15:59 UTC (permalink / raw)
  To: dev



W dniu 03.02.2016 o 16:49, Kamil Rytarowski pisze:
>
>
> W dniu 03.02.2016 o 15:08, David Marchand pisze:
>> On Wed, Feb 3, 2016 at 12:39 PM, Kamil Rytarowski
>> <krytarowski@caviumnetworks.com> wrote:
>>> W dniu 03.02.2016 o 09:47, David Marchand pisze:
>>>> And do your custom things in its devinit function ?
>>> I'm requesting from PF the mode of the device to be initialized. 
>>> This part
>>> is handled dynamically and depends of the current configuration in PF.
>>>
>>> In my use-case there are two device types: primary (master) and 
>>> secondary
>>> (slave). For the primary VF I'm creating a DPDK port normally, for 
>>> secondary
>>> I retain configured PCI device for further reuse (and there is no port
>>> created).
>> Well, again, if you don't want to associate a port to this pci
>> resource, why are you registering a eth_driver ?
>> A eth_driver driver supposes you have a 1 - 1 relation between ethdev
>> and pci resource.
> In my use-case one DPDK port optionally manages more than single PCI 
> resource, and these PCI resources compose single interface.
>
> Another example of overloaded .devinit is in app/test/test_pci.c:
>
> /*
>  * PCI test
>  * ========
>  *
>  * - Register a driver with a ``devinit()`` function.
>  *
>  * - Dump all PCI devices.
>  *
>  * - Check that the ``devinit()`` function is called at least once.
>  */
>
> With the current implementation it won't work, as .devinit callback 
> will be overwritten by the internal function.
>
>> For your case, register a pci driver, then in your pci probing
>> function (.devinit), depending on what you want to do, you can either
>> do nothing (?) or create one or more ethdevs (see mlx* and cxgbe
>> drivers).
>>
>>
>>
>
> This is what I'm doing right now.
>
> I need to initialize PCI bars and interrupts (resources) - all having 
> the same PCI ID and their functionality depending upon PF 
> configuration. Depending on this state, I'm making further decisions 
> in DRIVER_devinit() and whether to make from it a port or a resource 
> to reuse by a master port.

After sending the mail I noted that I can work with the .init callback 
and I may be wrong.

I will have a closer look and I will check whether it will work for me.

Thank you,

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API
  2016-02-02 14:27 ` [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API krytarowski
@ 2016-02-11 16:56   ` Panu Matilainen
  2016-02-11 17:15     ` David Marchand
  0 siblings, 1 reply; 9+ messages in thread
From: Panu Matilainen @ 2016-02-11 16:56 UTC (permalink / raw)
  To: krytarowski, dev

On 02/02/2016 04:27 PM, krytarowski@caviumnetworks.com wrote:
> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>
> Once pci_drv.devinit is overloaded, it's a function used in the original
> rte_eth_dev_init(), still reusable in altered versions.
>
> Signed-off-by: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
> ---
>   lib/librte_ether/rte_ethdev.c |  2 +-
>   lib/librte_ether/rte_ethdev.h | 18 ++++++++++++++++++
>   2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index ac4aeab..7f5e741 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -214,7 +214,7 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type)
>   	return eth_dev;
>   }
>
> -static int
> +int
>   rte_eth_dev_create_unique_device_name(char *name, size_t size,
>   		struct rte_pci_device *pci_dev)
>   {
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 8710dd7..b19db9d 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -3880,6 +3880,24 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *eth_dev, const char *name,
>   			 uint16_t queue_id, size_t size,
>   			 unsigned align, int socket_id);
>
> +/**
> + * Create unique device name
> + *
> + * @param name
> + *   The port identifier of the Ethernet device.
> + * @param size
> + *   Maximum string length of the generated name
> + * @param pci_dev
> + *   PCI device pointer
> + *
> + * @return
> + *   - 0: Success.
> + *   - <0: Error during generatin
> + *   - -EINVAL: Invalid input parameters.
> + */
> +int rte_eth_dev_create_unique_device_name(char *name, size_t size,
> +					  struct rte_pci_device *pci_dev);
> +
>   #ifdef __cplusplus
>   }
>   #endif
>

To really export it, you'll need to add it to rte_ether_version.map as well.

	- Panu -

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API
  2016-02-11 16:56   ` Panu Matilainen
@ 2016-02-11 17:15     ` David Marchand
  0 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2016-02-11 17:15 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

On Thu, Feb 11, 2016 at 5:56 PM, Panu Matilainen <pmatilai@redhat.com> wrote:
> On 02/02/2016 04:27 PM, krytarowski@caviumnetworks.com wrote:
>>
>> From: Kamil Rytarowski <Kamil.Rytarowski@caviumnetworks.com>
>> +int rte_eth_dev_create_unique_device_name(char *name, size_t size,
>> +                                         struct rte_pci_device *pci_dev);
> To really export it, you'll need to add it to rte_ether_version.map as well.

I already submitted something equivalent, and anyway, this belongs to
eal, not ethdev.
http://dpdk.org/ml/archives/dev/2016-January/032394.html


-- 
David Marchand

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-02-11 17:15 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 14:27 [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit krytarowski
2016-02-02 14:27 ` [dpdk-dev] [PATCH 2/2] ethdev: Export rte_eth_dev_create_unique_device_name() to public API krytarowski
2016-02-11 16:56   ` Panu Matilainen
2016-02-11 17:15     ` David Marchand
2016-02-03  8:47 ` [dpdk-dev] [PATCH 1/2] ethdev: Allow to overload pci_drv.devinit and pci_drv.devuninit David Marchand
2016-02-03 11:39   ` Kamil Rytarowski
2016-02-03 14:08     ` David Marchand
2016-02-03 15:49       ` Kamil Rytarowski
2016-02-03 15:59         ` Kamil Rytarowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).