[dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Jim Harris]

This function is listed under EXPERIMENTAL in the
rte_vhost_version.map, so it needs to be marked
with __rte_experimental in the header file as well.

Found by check-experimental-syms.sh when trying to compile
DPDK with -finstrument-functions.  This script didn't
catch this in the normal case, since the function is
declared __rte_always_inline.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 lib/librte_vhost/rte_vhost.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 7fb172912..fc27bc21e 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
  * @return
  *  the host virtual address on success, 0 on failure
  */
+__rte_experimental
 static __rte_always_inline uint64_t
 rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
 						   uint64_t gpa, uint64_t *len)

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Tiwei Bie]

On Tue, Aug 20, 2019 at 02:37:28AM -0700, Jim Harris wrote:
> This function is listed under EXPERIMENTAL in the
> rte_vhost_version.map, so it needs to be marked
> with __rte_experimental in the header file as well.
> 
> Found by check-experimental-syms.sh when trying to compile
> DPDK with -finstrument-functions.  This script didn't
> catch this in the normal case, since the function is
> declared __rte_always_inline.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>  lib/librte_vhost/rte_vhost.h |    1 +
>  1 file changed, 1 insertion(+)

Fixes: 41333fba5b98 ("vhost: introduce safe API for GPA translation")
Cc: stable@dpdk.org

Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>

Thanks,
Tiwei

> 
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index 7fb172912..fc27bc21e 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>   * @return
>   *  the host virtual address on success, 0 on failure
>   */
> +__rte_experimental
>  static __rte_always_inline uint64_t
>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>  						   uint64_t gpa, uint64_t *len)
> 

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Maxime Coquelin]

On 8/20/19 11:37 AM, Jim Harris wrote:
> This function is listed under EXPERIMENTAL in the
> rte_vhost_version.map, so it needs to be marked
> with __rte_experimental in the header file as well.
> 
> Found by check-experimental-syms.sh when trying to compile
> DPDK with -finstrument-functions.  This script didn't
> catch this in the normal case, since the function is
> declared __rte_always_inline.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>  lib/librte_vhost/rte_vhost.h |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
> index 7fb172912..fc27bc21e 100644
> --- a/lib/librte_vhost/rte_vhost.h
> +++ b/lib/librte_vhost/rte_vhost.h
> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>   * @return
>   *  the host virtual address on success, 0 on failure
>   */
> +__rte_experimental
>  static __rte_always_inline uint64_t
>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>  						   uint64_t gpa, uint64_t *len)
> 

Fixed commit message to comply with check-git-log tool.

Applied to dpdk-next-virtio/master.

Thanks,
Maxime

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Ferruh Yigit]

On 9/18/2019 2:12 PM, Maxime Coquelin wrote:
> 
> 
> On 8/20/19 11:37 AM, Jim Harris wrote:
>> This function is listed under EXPERIMENTAL in the
>> rte_vhost_version.map, so it needs to be marked
>> with __rte_experimental in the header file as well.
>>
>> Found by check-experimental-syms.sh when trying to compile
>> DPDK with -finstrument-functions.  This script didn't
>> catch this in the normal case, since the function is
>> declared __rte_always_inline.
>>
>> Signed-off-by: Jim Harris <james.r.harris@intel.com>
>> ---
>>  lib/librte_vhost/rte_vhost.h |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
>> index 7fb172912..fc27bc21e 100644
>> --- a/lib/librte_vhost/rte_vhost.h
>> +++ b/lib/librte_vhost/rte_vhost.h
>> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>>   * @return
>>   *  the host virtual address on success, 0 on failure
>>   */
>> +__rte_experimental
>>  static __rte_always_inline uint64_t
>>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>>  						   uint64_t gpa, uint64_t *len)
>>
> 
> Fixed commit message to comply with check-git-log tool.
> 
> Applied to dpdk-next-virtio/master.

This is breaking the 'vhost_scsi' sample application since it is using this
experimental API [1].
Build system should be updated to say sample application is allowed to using
experimental APIs.
Can you please send a new version?


[1]
.../dpdk/examples/vhost_scsi/vhost_scsi.c: In function ‘gpa_to_vva’:

.../dpdk/examples/vhost_scsi/vhost_scsi.c:61:2: error:
‘rte_vhost_va_from_guest_pa’ is deprecated: Symbol is not yet part of stable ABI
[-Werror=deprecated-declarations]
   61 |  return rte_vhost_va_from_guest_pa(ctrlr->mem, gpa, len);
      |  ^~~~~~
In file included from .../dpdk/examples/vhost_scsi/vhost_scsi.c:18:
.../dpdk/build32/include/rte_vhost.h:238:1: note: declared here
  238 | rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Harris, James R]

On 9/23/19, 8:41 AM, "Yigit, Ferruh" <ferruh.yigit@intel.com> wrote:

    On 9/18/2019 2:12 PM, Maxime Coquelin wrote:
    > 
    > 
    > On 8/20/19 11:37 AM, Jim Harris wrote:
    >> This function is listed under EXPERIMENTAL in the
    >> rte_vhost_version.map, so it needs to be marked
    >> with __rte_experimental in the header file as well.
    >>
    >> Found by check-experimental-syms.sh when trying to compile
    >> DPDK with -finstrument-functions.  This script didn't
    >> catch this in the normal case, since the function is
    >> declared __rte_always_inline.
    >>
    >> Signed-off-by: Jim Harris <james.r.harris@intel.com>
    >> ---
    >>  lib/librte_vhost/rte_vhost.h |    1 +
    >>  1 file changed, 1 insertion(+)
    >>
    >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
    >> index 7fb172912..fc27bc21e 100644
    >> --- a/lib/librte_vhost/rte_vhost.h
    >> +++ b/lib/librte_vhost/rte_vhost.h
    >> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
    >>   * @return
    >>   *  the host virtual address on success, 0 on failure
    >>   */
    >> +__rte_experimental
    >>  static __rte_always_inline uint64_t
    >>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
    >>  						   uint64_t gpa, uint64_t *len)
    >>
    > 
    > Fixed commit message to comply with check-git-log tool.
    > 
    > Applied to dpdk-next-virtio/master.
    
    This is breaking the 'vhost_scsi' sample application since it is using this
    experimental API [1].
    Build system should be updated to say sample application is allowed to using
    experimental APIs.
    Can you please send a new version?

Thanks Ferruh.  I missed that.  Does this mean adding the following to examples/vhost_scsi/Makefile?
    
CFLAGS += -DALLOW_EXPERIMENTAL_API

And this to examples/vhost_scsi/meson.build?

allow_experimental_apis = true

Maxime - please let me know if you need a patch from me that you'll apply to dpdk-next-virtio/master, or if you would send such a patch yourself.

Thanks,

-Jim
 

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Ferruh Yigit]

On 9/24/2019 12:26 AM, Harris, James R wrote:
> 
> 
> On 9/23/19, 8:41 AM, "Yigit, Ferruh" <ferruh.yigit@intel.com> wrote:
> 
>     On 9/18/2019 2:12 PM, Maxime Coquelin wrote:
>     > 
>     > 
>     > On 8/20/19 11:37 AM, Jim Harris wrote:
>     >> This function is listed under EXPERIMENTAL in the
>     >> rte_vhost_version.map, so it needs to be marked
>     >> with __rte_experimental in the header file as well.
>     >>
>     >> Found by check-experimental-syms.sh when trying to compile
>     >> DPDK with -finstrument-functions.  This script didn't
>     >> catch this in the normal case, since the function is
>     >> declared __rte_always_inline.
>     >>
>     >> Signed-off-by: Jim Harris <james.r.harris@intel.com>
>     >> ---
>     >>  lib/librte_vhost/rte_vhost.h |    1 +
>     >>  1 file changed, 1 insertion(+)
>     >>
>     >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
>     >> index 7fb172912..fc27bc21e 100644
>     >> --- a/lib/librte_vhost/rte_vhost.h
>     >> +++ b/lib/librte_vhost/rte_vhost.h
>     >> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>     >>   * @return
>     >>   *  the host virtual address on success, 0 on failure
>     >>   */
>     >> +__rte_experimental
>     >>  static __rte_always_inline uint64_t
>     >>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>     >>  						   uint64_t gpa, uint64_t *len)
>     >>
>     > 
>     > Fixed commit message to comply with check-git-log tool.
>     > 
>     > Applied to dpdk-next-virtio/master.
>     
>     This is breaking the 'vhost_scsi' sample application since it is using this
>     experimental API [1].
>     Build system should be updated to say sample application is allowed to using
>     experimental APIs.
>     Can you please send a new version?
> 
> Thanks Ferruh.  I missed that.  Does this mean adding the following to examples/vhost_scsi/Makefile?
>     
> CFLAGS += -DALLOW_EXPERIMENTAL_API
> 
> And this to examples/vhost_scsi/meson.build?
> 
> allow_experimental_apis = true

That is it.

> 
> Maxime - please let me know if you need a patch from me that you'll apply to dpdk-next-virtio/master, or if you would send such a patch yourself.
> 
> Thanks,
> 
> -Jim
>  
> 

Re: [dpdk-dev] [PATCH] vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

[Maxime Coquelin]

On 9/24/19 6:04 PM, Ferruh Yigit wrote:
> On 9/24/2019 12:26 AM, Harris, James R wrote:
>>
>>
>> On 9/23/19, 8:41 AM, "Yigit, Ferruh" <ferruh.yigit@intel.com> wrote:
>>
>>     On 9/18/2019 2:12 PM, Maxime Coquelin wrote:
>>     > 
>>     > 
>>     > On 8/20/19 11:37 AM, Jim Harris wrote:
>>     >> This function is listed under EXPERIMENTAL in the
>>     >> rte_vhost_version.map, so it needs to be marked
>>     >> with __rte_experimental in the header file as well.
>>     >>
>>     >> Found by check-experimental-syms.sh when trying to compile
>>     >> DPDK with -finstrument-functions.  This script didn't
>>     >> catch this in the normal case, since the function is
>>     >> declared __rte_always_inline.
>>     >>
>>     >> Signed-off-by: Jim Harris <james.r.harris@intel.com>
>>     >> ---
>>     >>  lib/librte_vhost/rte_vhost.h |    1 +
>>     >>  1 file changed, 1 insertion(+)
>>     >>
>>     >> diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
>>     >> index 7fb172912..fc27bc21e 100644
>>     >> --- a/lib/librte_vhost/rte_vhost.h
>>     >> +++ b/lib/librte_vhost/rte_vhost.h
>>     >> @@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>>     >>   * @return
>>     >>   *  the host virtual address on success, 0 on failure
>>     >>   */
>>     >> +__rte_experimental
>>     >>  static __rte_always_inline uint64_t
>>     >>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>>     >>  						   uint64_t gpa, uint64_t *len)
>>     >>
>>     > 
>>     > Fixed commit message to comply with check-git-log tool.
>>     > 
>>     > Applied to dpdk-next-virtio/master.
>>     
>>     This is breaking the 'vhost_scsi' sample application since it is using this
>>     experimental API [1].
>>     Build system should be updated to say sample application is allowed to using
>>     experimental APIs.
>>     Can you please send a new version?
>>
>> Thanks Ferruh.  I missed that.  Does this mean adding the following to examples/vhost_scsi/Makefile?
>>     
>> CFLAGS += -DALLOW_EXPERIMENTAL_API
>>
>> And this to examples/vhost_scsi/meson.build?
>>
>> allow_experimental_apis = true
> 
> That is it.
> 
>>
>> Maxime - please let me know if you need a patch from me that you'll apply to dpdk-next-virtio/master, or if you would send such a patch yourself.

I checked Ferruh's tree and can see Ferruh dropped your patch, so please
send a new revision.

Thanks,
Maxime
>>
>> Thanks,
>>
>> -Jim
>>  
>>
> 

[dpdk-dev] [PATCH v2] vhost: add experimental flag

[Jim Harris]

This function is listed under EXPERIMENTAL in the
rte_vhost_version.map, so it needs to be marked
with __rte_experimental in the header file as well.

Found by check-experimental-syms.sh when trying to compile
DPDK with -finstrument-functions.  This script didn't
catch this in the normal case, since the function is
declared __rte_always_inline.

This also requires updating the vhost_scsi example to allow
use of this newly marked experimental API.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
---
 examples/vhost_scsi/Makefile    |    2 ++
 examples/vhost_scsi/meson.build |    1 +
 lib/librte_vhost/rte_vhost.h    |    1 +
 3 files changed, 4 insertions(+)

diff --git a/examples/vhost_scsi/Makefile b/examples/vhost_scsi/Makefile
index c5aec269e..6015c0241 100644
--- a/examples/vhost_scsi/Makefile
+++ b/examples/vhost_scsi/Makefile
@@ -27,6 +27,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
 	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
 
diff --git a/examples/vhost_scsi/meson.build b/examples/vhost_scsi/meson.build
index 77e5201bd..384127d5b 100644
--- a/examples/vhost_scsi/meson.build
+++ b/examples/vhost_scsi/meson.build
@@ -15,6 +15,7 @@ if not cc.has_header('linux/virtio_scsi.h')
 endif
 
 deps += 'vhost'
+allow_experimental_apis = true
 sources = files(
 	'scsi.c', 'vhost_scsi.c'
 )
diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 7fb172912..fc27bc21e 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -225,6 +225,7 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
  * @return
  *  the host virtual address on success, 0 on failure
  */
+__rte_experimental
 static __rte_always_inline uint64_t
 rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
 						   uint64_t gpa, uint64_t *len)

Re: [dpdk-dev] [PATCH v2] vhost: add experimental flag

[Maxime Coquelin]

On 9/24/19 11:23 AM, Jim Harris wrote:
> This function is listed under EXPERIMENTAL in the
> rte_vhost_version.map, so it needs to be marked
> with __rte_experimental in the header file as well.
> 
> Found by check-experimental-syms.sh when trying to compile
> DPDK with -finstrument-functions.  This script didn't
> catch this in the normal case, since the function is
> declared __rte_always_inline.
> 
> This also requires updating the vhost_scsi example to allow
> use of this newly marked experimental API.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>  examples/vhost_scsi/Makefile    |    2 ++
>  examples/vhost_scsi/meson.build |    1 +
>  lib/librte_vhost/rte_vhost.h    |    1 +
>  3 files changed, 4 insertions(+)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v2] vhost: add experimental flag

[Maxime Coquelin]

On 9/24/19 11:23 AM, Jim Harris wrote:
> This function is listed under EXPERIMENTAL in the
> rte_vhost_version.map, so it needs to be marked
> with __rte_experimental in the header file as well.
> 
> Found by check-experimental-syms.sh when trying to compile
> DPDK with -finstrument-functions.  This script didn't
> catch this in the normal case, since the function is
> declared __rte_always_inline.
> 
> This also requires updating the vhost_scsi example to allow
> use of this newly marked experimental API.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>  examples/vhost_scsi/Makefile    |    2 ++
>  examples/vhost_scsi/meson.build |    1 +
>  lib/librte_vhost/rte_vhost.h    |    1

Applied to dpdk-next-virtio/master.

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v2] vhost: add experimental flag

[Ferruh Yigit]

On 9/24/2019 10:23 AM, Jim Harris wrote:
> This function is listed under EXPERIMENTAL in the
> rte_vhost_version.map, so it needs to be marked
> with __rte_experimental in the header file as well.
> 
> Found by check-experimental-syms.sh when trying to compile
> DPDK with -finstrument-functions.  This script didn't
> catch this in the normal case, since the function is
> declared __rte_always_inline.
> 
> This also requires updating the vhost_scsi example to allow
> use of this newly marked experimental API.
> 
> Signed-off-by: Jim Harris <james.r.harris@intel.com>
> ---
>  examples/vhost_scsi/Makefile    |    2 ++
>  examples/vhost_scsi/meson.build |    1 +
>  lib/librte_vhost/rte_vhost.h    |    1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/examples/vhost_scsi/Makefile b/examples/vhost_scsi/Makefile
> index c5aec269e..6015c0241 100644
> --- a/examples/vhost_scsi/Makefile
> +++ b/examples/vhost_scsi/Makefile
> @@ -27,6 +27,8 @@ CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
>  LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
>  LDFLAGS_STATIC = -Wl,-Bstatic $(shell $(PKGCONF) --static --libs libdpdk)
>  
> +CFLAGS += -DALLOW_EXPERIMENTAL_API

example still fails to build, option needs to be enabled in the 'else' leg of
the makefile, I am fixing while merging to next-net.