DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
@ 2019-10-24 12:18 Anatoly Burakov
  2019-10-24 19:09 ` David Marchand
  2019-11-06 21:11 ` David Marchand
  0 siblings, 2 replies; 19+ messages in thread
From: Anatoly Burakov @ 2019-10-24 12:18 UTC (permalink / raw)
  To: dev; +Cc: Neil Horman, John McNamara, Marko Kovacevic, Bruce Richardson, thomas

The rte_vfio_dma_map/unmap API's have been marked as deprecated in
release 19.05. Remove them.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
    it wasn't marked as __rte_deprecated in code. Should we still remove it?

 doc/guides/rel_notes/deprecation.rst     |  4 ---
 doc/guides/rel_notes/release_19_11.rst   |  3 ++
 lib/librte_eal/common/include/rte_vfio.h | 44 ------------------------
 lib/librte_eal/freebsd/eal/eal.c         | 14 --------
 lib/librte_eal/linux/eal/eal_vfio.c      | 36 -------------------
 lib/librte_eal/rte_eal_version.map       |  2 --
 6 files changed, 3 insertions(+), 100 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 237813b647..f044a0e436 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -44,10 +44,6 @@ Deprecation Notices
 * eal: The ``rte_malloc_virt2phy`` function has been deprecated and replaced
   by ``rte_malloc_virt2iova`` since v17.11 and will be removed.
 
-* vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
-  have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
-  functions.  The due date for the removal targets DPDK 20.02.
-
 * pci: Several exposed functions are misnamed.
   The following functions are deprecated starting from v17.11 and are replaced:
 
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index 856088c5c7..c43d6b29ef 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -137,6 +137,9 @@ Removed Items
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+   * Removed `rte_vfio_dma_map` and `rte_vfio_dma_unmap` API's that have been
+     marked as deprecated in release 19.05.
+
    * Removed duplicated set of commands for RX offloading configuration from app/testpmd:
      “port config all crc-strip|scatter|rx-cksum|rx-timestamp|hw-vlan|hw-vlan-filter|
      hw-vlan-strip|hw-vlan-extend on|off”.
diff --git a/lib/librte_eal/common/include/rte_vfio.h b/lib/librte_eal/common/include/rte_vfio.h
index b360485fad..20ed8c45a9 100644
--- a/lib/librte_eal/common/include/rte_vfio.h
+++ b/lib/librte_eal/common/include/rte_vfio.h
@@ -189,50 +189,6 @@ int rte_vfio_noiommu_is_enabled(void);
 int
 rte_vfio_clear_group(int vfio_group_fd);
 
-/**
- * Map memory region for use with VFIO.
- *
- * @note Require at least one device to be attached at the time of
- *       mapping. DMA maps done via this API will only apply to default
- *       container and will not apply to any of the containers created
- *       via rte_vfio_container_create().
- *
- * @param vaddr
- *   Starting virtual address of memory to be mapped.
- *
- * @param iova
- *   Starting IOVA address of memory to be mapped.
- *
- * @param len
- *   Length of memory segment being mapped.
- *
- * @return
- *   0 if success.
- *   -1 on error.
- */
-int
-rte_vfio_dma_map(uint64_t vaddr, uint64_t iova, uint64_t len);
-
-
-/**
- * Unmap memory region from VFIO.
- *
- * @param vaddr
- *   Starting virtual address of memory to be unmapped.
- *
- * @param iova
- *   Starting IOVA address of memory to be unmapped.
- *
- * @param len
- *   Length of memory segment being unmapped.
- *
- * @return
- *   0 if success.
- *   -1 on error.
- */
-
-int
-rte_vfio_dma_unmap(uint64_t vaddr, uint64_t iova, uint64_t len);
 /**
  * Parse IOMMU group number for a device
  *
diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
index f86e9aa318..3a62e2faf7 100644
--- a/lib/librte_eal/freebsd/eal/eal.c
+++ b/lib/librte_eal/freebsd/eal/eal.c
@@ -1007,20 +1007,6 @@ int rte_vfio_clear_group(__rte_unused int vfio_group_fd)
 	return 0;
 }
 
-int
-rte_vfio_dma_map(uint64_t __rte_unused vaddr, __rte_unused uint64_t iova,
-		  __rte_unused uint64_t len)
-{
-	return -1;
-}
-
-int
-rte_vfio_dma_unmap(uint64_t __rte_unused vaddr, uint64_t __rte_unused iova,
-		    __rte_unused uint64_t len)
-{
-	return -1;
-}
-
 int
 rte_vfio_get_group_num(__rte_unused const char *sysfs_base,
 		       __rte_unused const char *dev_addr,
diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
index d9541b1220..0af1fafa68 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -1829,28 +1829,6 @@ container_dma_unmap(struct vfio_config *vfio_cfg, uint64_t vaddr, uint64_t iova,
 	return ret;
 }
 
-int
-rte_vfio_dma_map(uint64_t vaddr, uint64_t iova, uint64_t len)
-{
-	if (len == 0) {
-		rte_errno = EINVAL;
-		return -1;
-	}
-
-	return container_dma_map(default_vfio_cfg, vaddr, iova, len);
-}
-
-int
-rte_vfio_dma_unmap(uint64_t vaddr, uint64_t iova, uint64_t len)
-{
-	if (len == 0) {
-		rte_errno = EINVAL;
-		return -1;
-	}
-
-	return container_dma_unmap(default_vfio_cfg, vaddr, iova, len);
-}
-
 int
 rte_vfio_noiommu_is_enabled(void)
 {
@@ -2028,20 +2006,6 @@ rte_vfio_container_dma_unmap(int container_fd, uint64_t vaddr, uint64_t iova,
 
 #else
 
-int
-rte_vfio_dma_map(uint64_t __rte_unused vaddr, __rte_unused uint64_t iova,
-		  __rte_unused uint64_t len)
-{
-	return -1;
-}
-
-int
-rte_vfio_dma_unmap(uint64_t __rte_unused vaddr, uint64_t __rte_unused iova,
-		    __rte_unused uint64_t len)
-{
-	return -1;
-}
-
 int
 rte_vfio_setup_device(__rte_unused const char *sysfs_base,
 		__rte_unused const char *dev_addr,
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 7cbf82d37b..33601e8e67 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -248,8 +248,6 @@ DPDK_18.08 {
 	rte_vfio_container_dma_unmap;
 	rte_vfio_container_group_bind;
 	rte_vfio_container_group_unbind;
-	rte_vfio_dma_map;
-	rte_vfio_dma_unmap;
 	rte_vfio_get_container_fd;
 	rte_vfio_get_group_fd;
 	rte_vfio_get_group_num;
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-24 12:18 [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions Anatoly Burakov
@ 2019-10-24 19:09 ` David Marchand
  2019-10-24 20:27   ` Stephen Hemminger
  2019-10-24 22:32   ` Thomas Monjalon
  2019-11-06 21:11 ` David Marchand
  1 sibling, 2 replies; 19+ messages in thread
From: David Marchand @ 2019-10-24 19:09 UTC (permalink / raw)
  To: Anatoly Burakov, Shahaf Shuler, Ray Kinsella
  Cc: dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson, Thomas Monjalon

On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> release 19.05. Remove them.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>     it wasn't marked as __rte_deprecated in code. Should we still remove it?

I can see that vpp is still using this api.
I would prefer we get some ack from their side.

Shahaf?
Ray?

Do you guys have contact with VPP devs?


--
David Marchand


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-24 19:09 ` David Marchand
@ 2019-10-24 20:27   ` Stephen Hemminger
  2019-10-24 22:32   ` Thomas Monjalon
  1 sibling, 0 replies; 19+ messages in thread
From: Stephen Hemminger @ 2019-10-24 20:27 UTC (permalink / raw)
  To: David Marchand
  Cc: Anatoly Burakov, Shahaf Shuler, Ray Kinsella, dev, Neil Horman,
	John McNamara, Marko Kovacevic, Bruce Richardson,
	Thomas Monjalon

On Thu, 24 Oct 2019 21:09:25 +0200
David Marchand <david.marchand@redhat.com> wrote:

> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
> >
> > The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> > release 19.05. Remove them.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> >
> > Notes:
> >     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
> >     it wasn't marked as __rte_deprecated in code. Should we still remove it?  
> 
> I can see that vpp is still using this api.
> I would prefer we get some ack from their side.
> 
> Shahaf?
> Ray?
> 
> Do you guys have contact with VPP devs?
> 
> 
> --
> David Marchand
> 

Maybe use __rte_deprecated for this release to force the issue.

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-24 19:09 ` David Marchand
  2019-10-24 20:27   ` Stephen Hemminger
@ 2019-10-24 22:32   ` Thomas Monjalon
  2019-10-25 11:13     ` Damjan Marion (damarion)
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2019-10-24 22:32 UTC (permalink / raw)
  To: David Marchand
  Cc: Anatoly Burakov, Shahaf Shuler, Ray Kinsella, dev, Neil Horman,
	John McNamara, Marko Kovacevic, Bruce Richardson, Damjan Marion

24/10/2019 21:09, David Marchand:
> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
> >
> > The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> > release 19.05. Remove them.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > ---
> >
> > Notes:
> >     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
> >     it wasn't marked as __rte_deprecated in code. Should we still remove it?
> 
> I can see that vpp is still using this api.
> I would prefer we get some ack from their side.
> 
> Shahaf?
> Ray?
> 
> Do you guys have contact with VPP devs?

+Cc Damjan



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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-24 22:32   ` Thomas Monjalon
@ 2019-10-25 11:13     ` Damjan Marion (damarion)
  2019-10-25 12:23       ` Burakov, Anatoly
  2019-10-27  7:00       ` Shahaf Shuler
  0 siblings, 2 replies; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-10-25 11:13 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: David Marchand, Anatoly Burakov, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 24/10/2019 21:09, David Marchand:
>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>> <anatoly.burakov@intel.com> wrote:
>>> 
>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>> release 19.05. Remove them.
>>> 
>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>> ---
>>> 
>>> Notes:
>>>    Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>    it wasn't marked as __rte_deprecated in code. Should we still remove it?
>> 
>> I can see that vpp is still using this api.
>> I would prefer we get some ack from their side.
>> 
>> Shahaf?
>> Ray?
>> 
>> Do you guys have contact with VPP devs?
> 
> +Cc Damjan

Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.

From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.

Is there alternative way to tell DPDK about DMA mapping?

Thanks,

— 
Damjan


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-25 11:13     ` Damjan Marion (damarion)
@ 2019-10-25 12:23       ` Burakov, Anatoly
  2019-10-25 13:02         ` Damjan Marion (damarion)
  2019-11-06 13:48         ` David Marchand
  2019-10-27  7:00       ` Shahaf Shuler
  1 sibling, 2 replies; 19+ messages in thread
From: Burakov, Anatoly @ 2019-10-25 12:23 UTC (permalink / raw)
  To: Damjan Marion (damarion), Thomas Monjalon
  Cc: David Marchand, Shahaf Shuler, Ray Kinsella, dev, Neil Horman,
	John McNamara, Marko Kovacevic, Bruce Richardson

On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
> 
> 
>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 24/10/2019 21:09, David Marchand:
>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>> <anatoly.burakov@intel.com> wrote:
>>>>
>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>> release 19.05. Remove them.
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>>
>>>> Notes:
>>>>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
>>>
>>> I can see that vpp is still using this api.
>>> I would prefer we get some ack from their side.
>>>
>>> Shahaf?
>>> Ray?
>>>
>>> Do you guys have contact with VPP devs?
>>
>> +Cc Damjan
> 
> Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
> We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
> 
>  From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
> 
> Is there alternative way to tell DPDK about DMA mapping?

The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact 
equivalent of the functions being removed. Also, rte_dev_dma_map() is 
supposed to be the more general DMA mapping API that works with VFIO and 
with any other bus/device-specific DMA mapping.

So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to 
"rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should 
trigger exactly the same behavior.

> 
> Thanks,
> 
> —
> Damjan
> 


-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-25 12:23       ` Burakov, Anatoly
@ 2019-10-25 13:02         ` Damjan Marion (damarion)
  2019-11-04 13:57           ` Damjan Marion (damarion)
  2019-11-06 13:48         ` David Marchand
  1 sibling, 1 reply; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-10-25 13:02 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:

24/10/2019 21:09, David Marchand:
On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

The rte_vfio_dma_map/unmap API's have been marked as deprecated in
release 19.05. Remove them.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
---

Notes:
   Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
   it wasn't marked as __rte_deprecated in code. Should we still remove it?

I can see that vpp is still using this api.
I would prefer we get some ack from their side.

Shahaf?
Ray?

Do you guys have contact with VPP devs?

+Cc Damjan
Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
Is there alternative way to tell DPDK about DMA mapping?

The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.

So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.

Done, will be merged after it passes verify jobs…

https://gerrit.fd.io/r/c/vpp/+/22982

Thanks,

Damjan


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-25 11:13     ` Damjan Marion (damarion)
  2019-10-25 12:23       ` Burakov, Anatoly
@ 2019-10-27  7:00       ` Shahaf Shuler
  2019-11-04 13:58         ` Damjan Marion (damarion)
  1 sibling, 1 reply; 19+ messages in thread
From: Shahaf Shuler @ 2019-10-27  7:00 UTC (permalink / raw)
  To: Damjan Marion (damarion), Thomas Monjalon
  Cc: David Marchand, Anatoly Burakov, Ray Kinsella, dev, Neil Horman,
	John McNamara, Marko Kovacevic, Bruce Richardson



> -----Original Message-----
> From: Damjan Marion (damarion) <damarion@cisco.com>
> Sent: Friday, October 25, 2019 2:14 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: David Marchand <david.marchand@redhat.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>; Shahaf Shuler <shahafs@mellanox.com>; Ray
> Kinsella <mdr@ashroe.eu>; dev <dev@dpdk.org>; Neil Horman
> <nhorman@tuxdriver.com>; John McNamara <john.mcnamara@intel.com>;
> Marko Kovacevic <marko.kovacevic@intel.com>; Bruce Richardson
> <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping
> functions
> 
> 
> 
> > On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net>
> wrote:
> >
> > 24/10/2019 21:09, David Marchand:
> >> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> >> <anatoly.burakov@intel.com> wrote:
> >>>
> >>> The rte_vfio_dma_map/unmap API's have been marked as deprecated
> in
> >>> release 19.05. Remove them.
> >>>
> >>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>> ---
> >>>
> >>> Notes:
> >>>    Although `rte_vfio_dma_map` et al. was marked as deprecated in our
> documentation,
> >>>    it wasn't marked as __rte_deprecated in code. Should we still remove
> it?
> >>
> >> I can see that vpp is still using this api.
> >> I would prefer we get some ack from their side.
> >>
> >> Shahaf?
> >> Ray?
> >>
> >> Do you guys have contact with VPP devs?
> >
> > +Cc Damjan
> 
> Thanks for looping me in. If I remember correctly that was used only to get
> mlx PMDs working.
> We can remove that calls but then mlx PMDs will stop working unless there is
> alternative solution.
> 
> From my perspective it is not big issue as we already have native rdma based
> mlx support, but i would expect that other people will complain.
> 
> Is there alternative way to tell DPDK about DMA mapping?

Damjan I don't follow.

Why would using the rte_dev_dma_map would break Mellanox PMDs? 

> 
> Thanks,
> 
> —
> Damjan


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-25 13:02         ` Damjan Marion (damarion)
@ 2019-11-04 13:57           ` Damjan Marion (damarion)
  2019-11-04 17:27             ` Burakov, Anatoly
  0 siblings, 1 reply; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-11-04 13:57 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com<mailto:damarion@cisco.com>> wrote:



On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:

24/10/2019 21:09, David Marchand:
On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

The rte_vfio_dma_map/unmap API's have been marked as deprecated in
release 19.05. Remove them.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
---

Notes:
   Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
   it wasn't marked as __rte_deprecated in code. Should we still remove it?

I can see that vpp is still using this api.
I would prefer we get some ack from their side.

Shahaf?
Ray?

Do you guys have contact with VPP devs?

+Cc Damjan
Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
Is there alternative way to tell DPDK about DMA mapping?

The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.

So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.

Done, will be merged after it passes verify jobs…

https://gerrit.fd.io/r/c/vpp/+/22982

I just got report that this patch breaks some tests. Is it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you meant RTE_VFIO_DEFAULT_CONTAINER_FD…

—
Damjan







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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-27  7:00       ` Shahaf Shuler
@ 2019-11-04 13:58         ` Damjan Marion (damarion)
  0 siblings, 0 replies; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-11-04 13:58 UTC (permalink / raw)
  To: Shahaf Shuler
  Cc: Thomas Monjalon, David Marchand, Anatoly Burakov, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



On 27 Oct 2019, at 08:00, Shahaf Shuler <shahafs@mellanox.com<mailto:shahafs@mellanox.com>> wrote:



-----Original Message-----
From: Damjan Marion (damarion) <damarion@cisco.com<mailto:damarion@cisco.com>>
Sent: Friday, October 25, 2019 2:14 PM
To: Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>>
Cc: David Marchand <david.marchand@redhat.com<mailto:david.marchand@redhat.com>>; Anatoly Burakov
<anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>; Shahaf Shuler <shahafs@mellanox.com<mailto:shahafs@mellanox.com>>; Ray
Kinsella <mdr@ashroe.eu<mailto:mdr@ashroe.eu>>; dev <dev@dpdk.org<mailto:dev@dpdk.org>>; Neil Horman
<nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>>; John McNamara <john.mcnamara@intel.com<mailto:john.mcnamara@intel.com>>;
Marko Kovacevic <marko.kovacevic@intel.com<mailto:marko.kovacevic@intel.com>>; Bruce Richardson
<bruce.richardson@intel.com<mailto:bruce.richardson@intel.com>>
Subject: Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping
functions



On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>>
wrote:

24/10/2019 21:09, David Marchand:
On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

The rte_vfio_dma_map/unmap API's have been marked as deprecated
in
release 19.05. Remove them.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
---

Notes:
  Although `rte_vfio_dma_map` et al. was marked as deprecated in our
documentation,
  it wasn't marked as __rte_deprecated in code. Should we still remove
it?

I can see that vpp is still using this api.
I would prefer we get some ack from their side.

Shahaf?
Ray?

Do you guys have contact with VPP devs?

+Cc Damjan

Thanks for looping me in. If I remember correctly that was used only to get
mlx PMDs working.
We can remove that calls but then mlx PMDs will stop working unless there is
alternative solution.

From my perspective it is not big issue as we already have native rdma based
mlx support, but i would expect that other people will complain.

Is there alternative way to tell DPDK about DMA mapping?

Damjan I don't follow.

Why would using the rte_dev_dma_map would break Mellanox PMDs?

May be just me confused. I remember you guys pot some hack to get it working in the past, but that patch may be using different way to get DMA mappings…




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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-04 13:57           ` Damjan Marion (damarion)
@ 2019-11-04 17:27             ` Burakov, Anatoly
  2019-11-04 17:34               ` Damjan Marion (damarion)
  0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2019-11-04 17:27 UTC (permalink / raw)
  To: Damjan Marion (damarion)
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson

On 04-Nov-19 1:57 PM, Damjan Marion (damarion) wrote:
> 
> 
>> On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com 
>> <mailto:damarion@cisco.com>> wrote:
>>
>>
>>
>>> On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com 
>>> <mailto:anatoly.burakov@intel.com>> wrote:
>>>
>>> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
>>>>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net 
>>>>> <mailto:thomas@monjalon.net>> wrote:
>>>>>
>>>>> 24/10/2019 21:09, David Marchand:
>>>>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>>>>> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>>>
>>>>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>>>>> release 19.05. Remove them.
>>>>>>>
>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com 
>>>>>>> <mailto:anatoly.burakov@intel.com>>
>>>>>>> ---
>>>>>>>
>>>>>>> Notes:
>>>>>>>    Although `rte_vfio_dma_map` et al. was marked as deprecated in 
>>>>>>> our documentation,
>>>>>>>    it wasn't marked as __rte_deprecated in code. Should we still 
>>>>>>> remove it?
>>>>>>
>>>>>> I can see that vpp is still using this api.
>>>>>> I would prefer we get some ack from their side.
>>>>>>
>>>>>> Shahaf?
>>>>>> Ray?
>>>>>>
>>>>>> Do you guys have contact with VPP devs?
>>>>>
>>>>> +Cc Damjan
>>>> Thanks for looping me in. If I remember correctly that was used only 
>>>> to get mlx PMDs working.
>>>> We can remove that calls but then mlx PMDs will stop working unless 
>>>> there is alternative solution.
>>>> From my perspective it is not big issue as we already have native 
>>>> rdma based mlx support, but i would expect that other people will 
>>>> complain.
>>>> Is there alternative way to tell DPDK about DMA mapping?
>>>
>>> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the 
>>> exact equivalent of the functions being removed. Also, 
>>> rte_dev_dma_map() is supposed to be the more general DMA mapping API 
>>> that works with VFIO and with any other bus/device-specific DMA mapping.
>>>
>>> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to 
>>> "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should 
>>> trigger exactly the same behavior.
>>
>> Done, will be merged after it passes verify jobs…
>>
>> https://gerrit.fd.io/r/c/vpp/+/22982
> 
> I just got report that this patch breaks some tests. Is 
> it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
> Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you 
> meant RTE_VFIO_DEFAULT_CONTAINER_FD…
> 
> —
> Damjan
> 
> 
Yes, i think i can see the bug. Can you rerun the failing test after 
applying the following patch?

diff --git a/lib/librte_eal/linux/eal/eal_vfio.c 
b/lib/librte_eal/linux/eal/eal_vfio.c
index d9541b1220..d7887388f9 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -412,6 +412,9 @@ get_vfio_cfg_by_container_fd(int container_fd)
  {
  	int i;

+	if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD)
+		return default_vfio_cfg;
+
  	for (i = 0; i < VFIO_MAX_CONTAINERS; i++) {
  		if (vfio_cfgs[i].vfio_container_fd == container_fd)
  			return &vfio_cfgs[i];


The problem seems to be that we're looking at actual fd, whereas the 
RTE_VFIO_DEFAULT_CONTAINER_FD value is -1, which will not match anything 
in that list.

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-04 17:27             ` Burakov, Anatoly
@ 2019-11-04 17:34               ` Damjan Marion (damarion)
  2019-11-04 17:42                 ` Burakov, Anatoly
  0 siblings, 1 reply; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-11-04 17:34 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



> On 4 Nov 2019, at 18:27, Burakov, Anatoly <anatoly.burakov@intel.com> wrote:
> 
> On 04-Nov-19 1:57 PM, Damjan Marion (damarion) wrote:
>>> On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com <mailto:damarion@cisco.com>> wrote:
>>> 
>>> 
>>> 
>>>> On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>> 
>>>> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
>>>>>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net <mailto:thomas@monjalon.net>> wrote:
>>>>>> 
>>>>>> 24/10/2019 21:09, David Marchand:
>>>>>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>>>>>> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>>>> 
>>>>>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>>>>>> release 19.05. Remove them.
>>>>>>>> 
>>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>>
>>>>>>>> ---
>>>>>>>> 
>>>>>>>> Notes:
>>>>>>>>    Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>>>>>>    it wasn't marked as __rte_deprecated in code. Should we still remove it?
>>>>>>> 
>>>>>>> I can see that vpp is still using this api.
>>>>>>> I would prefer we get some ack from their side.
>>>>>>> 
>>>>>>> Shahaf?
>>>>>>> Ray?
>>>>>>> 
>>>>>>> Do you guys have contact with VPP devs?
>>>>>> 
>>>>>> +Cc Damjan
>>>>> Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
>>>>> We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
>>>>> From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
>>>>> Is there alternative way to tell DPDK about DMA mapping?
>>>> 
>>>> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.
>>>> 
>>>> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.
>>> 
>>> Done, will be merged after it passes verify jobs…
>>> 
>>> https://gerrit.fd.io/r/c/vpp/+/22982
>> I just got report that this patch breaks some tests. Is it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
>> Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you meant RTE_VFIO_DEFAULT_CONTAINER_FD…
>> —
>> Damjan
> Yes, i think i can see the bug. Can you rerun the failing test after applying the following patch?
> 
> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
> index d9541b1220..d7887388f9 100644
> --- a/lib/librte_eal/linux/eal/eal_vfio.c
> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
> @@ -412,6 +412,9 @@ get_vfio_cfg_by_container_fd(int container_fd)
> {
> 	int i;
> 
> +	if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD)
> +		return default_vfio_cfg;
> +
> 	for (i = 0; i < VFIO_MAX_CONTAINERS; i++) {
> 		if (vfio_cfgs[i].vfio_container_fd == container_fd)
> 			return &vfio_cfgs[i];
> 
> 
> The problem seems to be that we're looking at actual fd, whereas the RTE_VFIO_DEFAULT_CONTAINER_FD value is -1, which will not match anything in that list.

That was exactly my reading, but I didn’t want to rush into conclusion. Will ask guys to test…


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-04 17:34               ` Damjan Marion (damarion)
@ 2019-11-04 17:42                 ` Burakov, Anatoly
  2019-11-04 17:44                   ` Damjan Marion (damarion)
  0 siblings, 1 reply; 19+ messages in thread
From: Burakov, Anatoly @ 2019-11-04 17:42 UTC (permalink / raw)
  To: Damjan Marion (damarion)
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson

On 04-Nov-19 5:34 PM, Damjan Marion (damarion) wrote:
> 
> 
>> On 4 Nov 2019, at 18:27, Burakov, Anatoly <anatoly.burakov@intel.com> wrote:
>>
>> On 04-Nov-19 1:57 PM, Damjan Marion (damarion) wrote:
>>>> On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com <mailto:damarion@cisco.com>> wrote:
>>>>
>>>>
>>>>
>>>>> On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>
>>>>> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
>>>>>>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net <mailto:thomas@monjalon.net>> wrote:
>>>>>>>
>>>>>>> 24/10/2019 21:09, David Marchand:
>>>>>>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>>>>>>> <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>> wrote:
>>>>>>>>>
>>>>>>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>>>>>>> release 19.05. Remove them.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com <mailto:anatoly.burakov@intel.com>>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Notes:
>>>>>>>>>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>>>>>>>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
>>>>>>>>
>>>>>>>> I can see that vpp is still using this api.
>>>>>>>> I would prefer we get some ack from their side.
>>>>>>>>
>>>>>>>> Shahaf?
>>>>>>>> Ray?
>>>>>>>>
>>>>>>>> Do you guys have contact with VPP devs?
>>>>>>>
>>>>>>> +Cc Damjan
>>>>>> Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
>>>>>> We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
>>>>>>  From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
>>>>>> Is there alternative way to tell DPDK about DMA mapping?
>>>>>
>>>>> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.
>>>>>
>>>>> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.
>>>>
>>>> Done, will be merged after it passes verify jobs…
>>>>
>>>> https://gerrit.fd.io/r/c/vpp/+/22982
>>> I just got report that this patch breaks some tests. Is it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
>>> Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you meant RTE_VFIO_DEFAULT_CONTAINER_FD…
>>> —
>>> Damjan
>> Yes, i think i can see the bug. Can you rerun the failing test after applying the following patch?
>>
>> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
>> index d9541b1220..d7887388f9 100644
>> --- a/lib/librte_eal/linux/eal/eal_vfio.c
>> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
>> @@ -412,6 +412,9 @@ get_vfio_cfg_by_container_fd(int container_fd)
>> {
>> 	int i;
>>
>> +	if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD)
>> +		return default_vfio_cfg;
>> +
>> 	for (i = 0; i < VFIO_MAX_CONTAINERS; i++) {
>> 		if (vfio_cfgs[i].vfio_container_fd == container_fd)
>> 			return &vfio_cfgs[i];
>>
>>
>> The problem seems to be that we're looking at actual fd, whereas the RTE_VFIO_DEFAULT_CONTAINER_FD value is -1, which will not match anything in that list.
> 
> That was exactly my reading, but I didn’t want to rush into conclusion. Will ask guys to test…
> 

This should make it easier to test:

http://patches.dpdk.org/patch/62390/

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-04 17:42                 ` Burakov, Anatoly
@ 2019-11-04 17:44                   ` Damjan Marion (damarion)
  0 siblings, 0 replies; 19+ messages in thread
From: Damjan Marion (damarion) @ 2019-11-04 17:44 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Thomas Monjalon, David Marchand, Shahaf Shuler, Ray Kinsella,
	dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson



On 4 Nov 2019, at 18:42, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

On 04-Nov-19 5:34 PM, Damjan Marion (damarion) wrote:
On 4 Nov 2019, at 18:27, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:

On 04-Nov-19 1:57 PM, Damjan Marion (damarion) wrote:
On 25 Oct 2019, at 15:02, Damjan Marion (damarion) <damarion@cisco.com<mailto:damarion@cisco.com> <mailto:damarion@cisco.com>> wrote:



On 25 Oct 2019, at 14:23, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com> <mailto:anatoly.burakov@intel.com>> wrote:

On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net> <mailto:thomas@monjalon.net>> wrote:

24/10/2019 21:09, David Marchand:
On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com> <mailto:anatoly.burakov@intel.com>> wrote:

The rte_vfio_dma_map/unmap API's have been marked as deprecated in
release 19.05. Remove them.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com> <mailto:anatoly.burakov@intel.com>>
---

Notes:
   Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
   it wasn't marked as __rte_deprecated in code. Should we still remove it?

I can see that vpp is still using this api.
I would prefer we get some ack from their side.

Shahaf?
Ray?

Do you guys have contact with VPP devs?

+Cc Damjan
Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
Is there alternative way to tell DPDK about DMA mapping?

The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact equivalent of the functions being removed. Also, rte_dev_dma_map() is supposed to be the more general DMA mapping API that works with VFIO and with any other bus/device-specific DMA mapping.

So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should trigger exactly the same behavior.

Done, will be merged after it passes verify jobs…

https://gerrit.fd.io/r/c/vpp/+/22982
I just got report that this patch breaks some tests. Is it RTE_VFIO_DEFAULT_CONTAINER_FD right value to use here?
Maybe i wrongly assumed that when you said VFIO_DEFAULT_CONTAINER, you meant RTE_VFIO_DEFAULT_CONTAINER_FD…
—
Damjan
Yes, i think i can see the bug. Can you rerun the failing test after applying the following patch?

diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
index d9541b1220..d7887388f9 100644
--- a/lib/librte_eal/linux/eal/eal_vfio.c
+++ b/lib/librte_eal/linux/eal/eal_vfio.c
@@ -412,6 +412,9 @@ get_vfio_cfg_by_container_fd(int container_fd)
{
int i;

+ if (container_fd == RTE_VFIO_DEFAULT_CONTAINER_FD)
+ return default_vfio_cfg;
+
for (i = 0; i < VFIO_MAX_CONTAINERS; i++) {
if (vfio_cfgs[i].vfio_container_fd == container_fd)
return &vfio_cfgs[i];


The problem seems to be that we're looking at actual fd, whereas the RTE_VFIO_DEFAULT_CONTAINER_FD value is -1, which will not match anything in that list.
That was exactly my reading, but I didn’t want to rush into conclusion. Will ask guys to test…

This should make it easier to test:

http://patches.dpdk.org/patch/62390/

This one even easier :)

https://gerrit.fd.io/r/c/vpp/+/23227



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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-25 12:23       ` Burakov, Anatoly
  2019-10-25 13:02         ` Damjan Marion (damarion)
@ 2019-11-06 13:48         ` David Marchand
  2019-11-06 13:50           ` Thomas Monjalon
  1 sibling, 1 reply; 19+ messages in thread
From: David Marchand @ 2019-11-06 13:48 UTC (permalink / raw)
  To: Burakov, Anatoly, Damjan Marion (damarion)
  Cc: Thomas Monjalon, Shahaf Shuler, Ray Kinsella, dev, Neil Horman,
	John McNamara, Marko Kovacevic, Bruce Richardson

On Fri, Oct 25, 2019 at 2:23 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
> >
> >
> >> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net> wrote:
> >>
> >> 24/10/2019 21:09, David Marchand:
> >>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> >>> <anatoly.burakov@intel.com> wrote:
> >>>>
> >>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> >>>> release 19.05. Remove them.
> >>>>
> >>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>> ---
> >>>>
> >>>> Notes:
> >>>>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
> >>>>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
> >>>
> >>> I can see that vpp is still using this api.
> >>> I would prefer we get some ack from their side.
> >>>
> >>> Shahaf?
> >>> Ray?
> >>>
> >>> Do you guys have contact with VPP devs?
> >>
> >> +Cc Damjan
> >
> > Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
> > We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
> >
> >  From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
> >
> > Is there alternative way to tell DPDK about DMA mapping?
>
> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact
> equivalent of the functions being removed. Also, rte_dev_dma_map() is
> supposed to be the more general DMA mapping API that works with VFIO and
> with any other bus/device-specific DMA mapping.
>
> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to
> "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should
> trigger exactly the same behavior.

The issue on VFIO_DEFAULT_CONTAINER seems fixed.
The deprecation had been announced (even if it was for 20.02) and we
have a replacement.

So I am for taking this patch.
Any objection?


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-06 13:48         ` David Marchand
@ 2019-11-06 13:50           ` Thomas Monjalon
  2019-11-06 13:54             ` Burakov, Anatoly
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Monjalon @ 2019-11-06 13:50 UTC (permalink / raw)
  To: David Marchand
  Cc: Burakov, Anatoly, Damjan Marion (damarion),
	Shahaf Shuler, Ray Kinsella, dev, Neil Horman, John McNamara,
	Marko Kovacevic, Bruce Richardson

06/11/2019 14:48, David Marchand:
> On Fri, Oct 25, 2019 at 2:23 PM Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
> >
> > On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
> > >
> > >
> > >> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net> wrote:
> > >>
> > >> 24/10/2019 21:09, David Marchand:
> > >>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> > >>> <anatoly.burakov@intel.com> wrote:
> > >>>>
> > >>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> > >>>> release 19.05. Remove them.
> > >>>>
> > >>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> > >>>> ---
> > >>>>
> > >>>> Notes:
> > >>>>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
> > >>>>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
> > >>>
> > >>> I can see that vpp is still using this api.
> > >>> I would prefer we get some ack from their side.
> > >>>
> > >>> Shahaf?
> > >>> Ray?
> > >>>
> > >>> Do you guys have contact with VPP devs?
> > >>
> > >> +Cc Damjan
> > >
> > > Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
> > > We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
> > >
> > >  From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
> > >
> > > Is there alternative way to tell DPDK about DMA mapping?
> >
> > The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact
> > equivalent of the functions being removed. Also, rte_dev_dma_map() is
> > supposed to be the more general DMA mapping API that works with VFIO and
> > with any other bus/device-specific DMA mapping.
> >
> > So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to
> > "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should
> > trigger exactly the same behavior.
> 
> The issue on VFIO_DEFAULT_CONTAINER seems fixed.
> The deprecation had been announced (even if it was for 20.02) and we
> have a replacement.
> 
> So I am for taking this patch.
> Any objection?

I agree to remove these functions.



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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-06 13:50           ` Thomas Monjalon
@ 2019-11-06 13:54             ` Burakov, Anatoly
  0 siblings, 0 replies; 19+ messages in thread
From: Burakov, Anatoly @ 2019-11-06 13:54 UTC (permalink / raw)
  To: Thomas Monjalon, David Marchand
  Cc: Damjan Marion (damarion),
	Shahaf Shuler, Ray Kinsella, dev, Neil Horman, John McNamara,
	Marko Kovacevic, Bruce Richardson

On 06-Nov-19 1:50 PM, Thomas Monjalon wrote:
> 06/11/2019 14:48, David Marchand:
>> On Fri, Oct 25, 2019 at 2:23 PM Burakov, Anatoly
>> <anatoly.burakov@intel.com> wrote:
>>>
>>> On 25-Oct-19 12:13 PM, Damjan Marion (damarion) wrote:
>>>>
>>>>
>>>>> On 25 Oct 2019, at 00:32, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>>
>>>>> 24/10/2019 21:09, David Marchand:
>>>>>> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
>>>>>> <anatoly.burakov@intel.com> wrote:
>>>>>>>
>>>>>>> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
>>>>>>> release 19.05. Remove them.
>>>>>>>
>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Notes:
>>>>>>>      Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>>>>>>>      it wasn't marked as __rte_deprecated in code. Should we still remove it?
>>>>>>
>>>>>> I can see that vpp is still using this api.
>>>>>> I would prefer we get some ack from their side.
>>>>>>
>>>>>> Shahaf?
>>>>>> Ray?
>>>>>>
>>>>>> Do you guys have contact with VPP devs?
>>>>>
>>>>> +Cc Damjan
>>>>
>>>> Thanks for looping me in. If I remember correctly that was used only to get mlx PMDs working.
>>>> We can remove that calls but then mlx PMDs will stop working unless there is alternative solution.
>>>>
>>>>   From my perspective it is not big issue as we already have native rdma based mlx support, but i would expect that other people will complain.
>>>>
>>>> Is there alternative way to tell DPDK about DMA mapping?
>>>
>>> The rte_vfio_container_dma_map(VFIO_DEFAULT_CONTAINER, ...) is the exact
>>> equivalent of the functions being removed. Also, rte_dev_dma_map() is
>>> supposed to be the more general DMA mapping API that works with VFIO and
>>> with any other bus/device-specific DMA mapping.
>>>
>>> So yes, a simple search and replace for "rte_vfio_dma_(un)?map(" to
>>> "rte_vfio_container_dma_(un)?map(VFIO_DEFAULT_CONTAINER, " should
>>> trigger exactly the same behavior.
>>
>> The issue on VFIO_DEFAULT_CONTAINER seems fixed.
>> The deprecation had been announced (even if it was for 20.02) and we
>> have a replacement.
>>
>> So I am for taking this patch.
>> Any objection?
> 
> I agree to remove these functions.
> 

Nuke them from orbit :)

-- 
Thanks,
Anatoly

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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-10-24 12:18 [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions Anatoly Burakov
  2019-10-24 19:09 ` David Marchand
@ 2019-11-06 21:11 ` David Marchand
  2019-11-07 15:34   ` David Marchand
  1 sibling, 1 reply; 19+ messages in thread
From: David Marchand @ 2019-11-06 21:11 UTC (permalink / raw)
  To: Anatoly Burakov
  Cc: dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson, Thomas Monjalon

On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
<anatoly.burakov@intel.com> wrote:
>
> The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> release 19.05. Remove them.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
>     Although `rte_vfio_dma_map` et al. was marked as deprecated in our documentation,
>     it wasn't marked as __rte_deprecated in code. Should we still remove it?
>
>  doc/guides/rel_notes/deprecation.rst     |  4 ---
>  doc/guides/rel_notes/release_19_11.rst   |  3 ++
>  lib/librte_eal/common/include/rte_vfio.h | 44 ------------------------
>  lib/librte_eal/freebsd/eal/eal.c         | 14 --------
>  lib/librte_eal/linux/eal/eal_vfio.c      | 36 -------------------
>  lib/librte_eal/rte_eal_version.map       |  2 --
>  6 files changed, 3 insertions(+), 100 deletions(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 237813b647..f044a0e436 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -44,10 +44,6 @@ Deprecation Notices
>  * eal: The ``rte_malloc_virt2phy`` function has been deprecated and replaced
>    by ``rte_malloc_virt2iova`` since v17.11 and will be removed.
>
> -* vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
> -  have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
> -  functions.  The due date for the removal targets DPDK 20.02.
> -
>  * pci: Several exposed functions are misnamed.
>    The following functions are deprecated starting from v17.11 and are replaced:
>
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index 856088c5c7..c43d6b29ef 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -137,6 +137,9 @@ Removed Items
>     Also, make sure to start the actual text at the margin.
>     =========================================================
>
> +   * Removed `rte_vfio_dma_map` and `rte_vfio_dma_unmap` API's that have been
> +     marked as deprecated in release 19.05.
> +

This should be in the API change section.
Plus the indentation is incorrect.
Will fix when applying.


>     * Removed duplicated set of commands for RX offloading configuration from app/testpmd:
>       “port config all crc-strip|scatter|rx-cksum|rx-timestamp|hw-vlan|hw-vlan-filter|
>       hw-vlan-strip|hw-vlan-extend on|off”.
> diff --git a/lib/librte_eal/common/include/rte_vfio.h b/lib/librte_eal/common/include/rte_vfio.h
> index b360485fad..20ed8c45a9 100644
> --- a/lib/librte_eal/common/include/rte_vfio.h
> +++ b/lib/librte_eal/common/include/rte_vfio.h
> @@ -189,50 +189,6 @@ int rte_vfio_noiommu_is_enabled(void);
>  int
>  rte_vfio_clear_group(int vfio_group_fd);
>
> -/**
> - * Map memory region for use with VFIO.
> - *
> - * @note Require at least one device to be attached at the time of
> - *       mapping. DMA maps done via this API will only apply to default
> - *       container and will not apply to any of the containers created
> - *       via rte_vfio_container_create().
> - *
> - * @param vaddr
> - *   Starting virtual address of memory to be mapped.
> - *
> - * @param iova
> - *   Starting IOVA address of memory to be mapped.
> - *
> - * @param len
> - *   Length of memory segment being mapped.
> - *
> - * @return
> - *   0 if success.
> - *   -1 on error.
> - */
> -int
> -rte_vfio_dma_map(uint64_t vaddr, uint64_t iova, uint64_t len);
> -
> -
> -/**
> - * Unmap memory region from VFIO.
> - *
> - * @param vaddr
> - *   Starting virtual address of memory to be unmapped.
> - *
> - * @param iova
> - *   Starting IOVA address of memory to be unmapped.
> - *
> - * @param len
> - *   Length of memory segment being unmapped.
> - *
> - * @return
> - *   0 if success.
> - *   -1 on error.
> - */
> -
> -int
> -rte_vfio_dma_unmap(uint64_t vaddr, uint64_t iova, uint64_t len);
>  /**
>   * Parse IOMMU group number for a device
>   *
> diff --git a/lib/librte_eal/freebsd/eal/eal.c b/lib/librte_eal/freebsd/eal/eal.c
> index f86e9aa318..3a62e2faf7 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -1007,20 +1007,6 @@ int rte_vfio_clear_group(__rte_unused int vfio_group_fd)
>         return 0;
>  }
>
> -int
> -rte_vfio_dma_map(uint64_t __rte_unused vaddr, __rte_unused uint64_t iova,
> -                 __rte_unused uint64_t len)
> -{
> -       return -1;
> -}
> -
> -int
> -rte_vfio_dma_unmap(uint64_t __rte_unused vaddr, uint64_t __rte_unused iova,
> -                   __rte_unused uint64_t len)
> -{
> -       return -1;
> -}
> -
>  int
>  rte_vfio_get_group_num(__rte_unused const char *sysfs_base,
>                        __rte_unused const char *dev_addr,
> diff --git a/lib/librte_eal/linux/eal/eal_vfio.c b/lib/librte_eal/linux/eal/eal_vfio.c
> index d9541b1220..0af1fafa68 100644
> --- a/lib/librte_eal/linux/eal/eal_vfio.c
> +++ b/lib/librte_eal/linux/eal/eal_vfio.c
> @@ -1829,28 +1829,6 @@ container_dma_unmap(struct vfio_config *vfio_cfg, uint64_t vaddr, uint64_t iova,
>         return ret;
>  }
>
> -int
> -rte_vfio_dma_map(uint64_t vaddr, uint64_t iova, uint64_t len)
> -{
> -       if (len == 0) {
> -               rte_errno = EINVAL;
> -               return -1;
> -       }
> -
> -       return container_dma_map(default_vfio_cfg, vaddr, iova, len);
> -}
> -
> -int
> -rte_vfio_dma_unmap(uint64_t vaddr, uint64_t iova, uint64_t len)
> -{
> -       if (len == 0) {
> -               rte_errno = EINVAL;
> -               return -1;
> -       }
> -
> -       return container_dma_unmap(default_vfio_cfg, vaddr, iova, len);
> -}
> -
>  int
>  rte_vfio_noiommu_is_enabled(void)
>  {
> @@ -2028,20 +2006,6 @@ rte_vfio_container_dma_unmap(int container_fd, uint64_t vaddr, uint64_t iova,
>
>  #else
>
> -int
> -rte_vfio_dma_map(uint64_t __rte_unused vaddr, __rte_unused uint64_t iova,
> -                 __rte_unused uint64_t len)
> -{
> -       return -1;
> -}
> -
> -int
> -rte_vfio_dma_unmap(uint64_t __rte_unused vaddr, uint64_t __rte_unused iova,
> -                   __rte_unused uint64_t len)
> -{
> -       return -1;
> -}
> -
>  int
>  rte_vfio_setup_device(__rte_unused const char *sysfs_base,
>                 __rte_unused const char *dev_addr,
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index 7cbf82d37b..33601e8e67 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -248,8 +248,6 @@ DPDK_18.08 {
>         rte_vfio_container_dma_unmap;
>         rte_vfio_container_group_bind;
>         rte_vfio_container_group_unbind;
> -       rte_vfio_dma_map;
> -       rte_vfio_dma_unmap;
>         rte_vfio_get_container_fd;
>         rte_vfio_get_group_fd;
>         rte_vfio_get_group_num;
> --
> 2.17.1

Acked-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions
  2019-11-06 21:11 ` David Marchand
@ 2019-11-07 15:34   ` David Marchand
  0 siblings, 0 replies; 19+ messages in thread
From: David Marchand @ 2019-11-07 15:34 UTC (permalink / raw)
  To: Anatoly Burakov
  Cc: dev, Neil Horman, John McNamara, Marko Kovacevic,
	Bruce Richardson, Thomas Monjalon

On Wed, Nov 6, 2019 at 10:11 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Thu, Oct 24, 2019 at 2:18 PM Anatoly Burakov
> <anatoly.burakov@intel.com> wrote:
> >
> > The rte_vfio_dma_map/unmap API's have been marked as deprecated in
> > release 19.05. Remove them.
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: David Marchand <david.marchand@redhat.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>

> > diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> > index 856088c5c7..c43d6b29ef 100644
> > --- a/doc/guides/rel_notes/release_19_11.rst
> > +++ b/doc/guides/rel_notes/release_19_11.rst
> > @@ -137,6 +137,9 @@ Removed Items
> >     Also, make sure to start the actual text at the margin.
> >     =========================================================
> >
> > +   * Removed `rte_vfio_dma_map` and `rte_vfio_dma_unmap` API's that have been
> > +     marked as deprecated in release 19.05.
> > +
>
> This should be in the API change section.
> Plus the indentation is incorrect.
> Will fix when applying.

And, as discussed offlist, added a note:

  ``rte_vfio_container_dma_map`` and ``rte_vfio_container_dma_unmap`` can
  be used as substitutes.


Applied, thanks.

-- 
David Marchand


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

end of thread, other threads:[~2019-11-07 15:35 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 12:18 [dpdk-dev] [PATCH] vfio: remove deprecated DMA mapping functions Anatoly Burakov
2019-10-24 19:09 ` David Marchand
2019-10-24 20:27   ` Stephen Hemminger
2019-10-24 22:32   ` Thomas Monjalon
2019-10-25 11:13     ` Damjan Marion (damarion)
2019-10-25 12:23       ` Burakov, Anatoly
2019-10-25 13:02         ` Damjan Marion (damarion)
2019-11-04 13:57           ` Damjan Marion (damarion)
2019-11-04 17:27             ` Burakov, Anatoly
2019-11-04 17:34               ` Damjan Marion (damarion)
2019-11-04 17:42                 ` Burakov, Anatoly
2019-11-04 17:44                   ` Damjan Marion (damarion)
2019-11-06 13:48         ` David Marchand
2019-11-06 13:50           ` Thomas Monjalon
2019-11-06 13:54             ` Burakov, Anatoly
2019-10-27  7:00       ` Shahaf Shuler
2019-11-04 13:58         ` Damjan Marion (damarion)
2019-11-06 21:11 ` David Marchand
2019-11-07 15:34   ` David Marchand

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).