diff mbox series

vhost: add __rte_experimental to rte_vhost_va_from_guest_pa

Message ID 156629384840.23278.17587309426304779602.stgit@jrharri1-skx (mailing list archive)
State Changes Requested, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: add __rte_experimental to rte_vhost_va_from_guest_pa |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/iol-Compile-Testing success Compile Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Harris, James R Aug. 20, 2019, 9:37 a.m. UTC 
  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(+)

Comments

Tiwei Bie Aug. 21, 2019, 5:19 a.m. UTC | #1 
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)
>
Maxime Coquelin Sept. 18, 2019, 1:12 p.m. UTC | #2 
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
Ferruh Yigit Sept. 23, 2019, 3:41 p.m. UTC | #3 
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,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~
Harris, James R Sept. 23, 2019, 11:26 p.m. UTC | #4 
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
Ferruh Yigit Sept. 24, 2019, 4:04 p.m. UTC | #5 
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
>  
>
Maxime Coquelin Sept. 24, 2019, 4:05 p.m. UTC | #6 
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
>>  
>>
>
diff mbox series

Patch

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)