diff mbox series

[v3,2/6] ethdev: add function for checking IOVAs by a device

Message ID 1530708838-2682-3-git-send-email-alejandro.lucero@netronome.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series use IOVAs check based on DMA mask |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Alejandro Lucero July 4, 2018, 12:53 p.m. UTC 
  A PMD should invoke this function for checking memsegs iovas are within
the supported range by the device.

Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
---
 lib/librte_ether/rte_ethdev.h           | 13 +++++++++++++
 lib/librte_ether/rte_ethdev_version.map |  1 +
 2 files changed, 14 insertions(+)

Comments

Andrew Rybchenko July 7, 2018, 5:30 p.m. UTC | #1 
On 04.07.2018 15:53, Alejandro Lucero wrote:
> A PMD should invoke this function for checking memsegs iovas are within
> the supported range by the device.
>
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> ---
>   lib/librte_ether/rte_ethdev.h           | 13 +++++++++++++
>   lib/librte_ether/rte_ethdev_version.map |  1 +
>   2 files changed, 14 insertions(+)
>
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index eba11ca..e51a432 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2799,6 +2799,19 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
>   int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
>   
>   /**
> + * check device dma mask within expected range based on dma mask.
> + *
> + * @param maskbits
> + *  mask length in bits
> + *
> + */
> +static inline int
> +rte_eth_dev_check_dma_mask(uint8_t maskbits)
> +{
> +	return rte_eal_check_dma_mask(maskbits);

I'm afraid I don't understand why do we need the wrapper.
May PMD use EAL function directly?

> +}
> +
> +/**
>    *
>    * Retrieve a burst of input packets from a receive queue of an Ethernet
>    * device. The retrieved packets are stored in *rte_mbuf* structures whose
> diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
> index e9681ac..0b11b8a 100644
> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -191,6 +191,7 @@ DPDK_17.08 {
>   DPDK_17.11 {
>   	global:
>   
> +	rte_eth_dev_check_dma_mask;
>   	rte_eth_dev_get_sec_ctx;
>   	rte_eth_dev_pool_ops_supported;
>   	rte_eth_dev_reset;
Eelco Chaudron July 10, 2018, 8:57 a.m. UTC | #2 
On 4 Jul 2018, at 14:53, Alejandro Lucero wrote:

> A PMD should invoke this function for checking memsegs iovas are 
> within
> the supported range by the device.
>
> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>

Agree with Andrew here, why not call rte_eal_check_dma_mask() directly 
in nfp_net_txq_full()?

> ---
>  lib/librte_ether/rte_ethdev.h           | 13 +++++++++++++
>  lib/librte_ether/rte_ethdev_version.map |  1 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/lib/librte_ether/rte_ethdev.h 
> b/lib/librte_ether/rte_ethdev.h
> index eba11ca..e51a432 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -2799,6 +2799,19 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t 
> port_id,
>  int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int 
> on);
>
>  /**
> + * check device dma mask within expected range based on dma mask.
> + *
> + * @param maskbits
> + *  mask length in bits
> + *
> + */
> +static inline int
> +rte_eth_dev_check_dma_mask(uint8_t maskbits)
> +{
> +	return rte_eal_check_dma_mask(maskbits);
> +}
> +
> +/**
>   *
>   * Retrieve a burst of input packets from a receive queue of an 
> Ethernet
>   * device. The retrieved packets are stored in *rte_mbuf* structures 
> whose
> diff --git a/lib/librte_ether/rte_ethdev_version.map 
> b/lib/librte_ether/rte_ethdev_version.map
> index e9681ac..0b11b8a 100644
> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> @@ -191,6 +191,7 @@ DPDK_17.08 {
>  DPDK_17.11 {
>  	global:
>
> +	rte_eth_dev_check_dma_mask;
>  	rte_eth_dev_get_sec_ctx;
>  	rte_eth_dev_pool_ops_supported;
>  	rte_eth_dev_reset;
> -- 
> 1.9.1
Alejandro Lucero July 10, 2018, 9:42 a.m. UTC | #3 
On Tue, Jul 10, 2018 at 9:57 AM, Eelco Chaudron <echaudro@redhat.com> wrote:

>
>
> On 4 Jul 2018, at 14:53, Alejandro Lucero wrote:
>
> A PMD should invoke this function for checking memsegs iovas are within
>> the supported range by the device.
>>
>> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>>
>
> Agree with Andrew here, why not call rte_eal_check_dma_mask() directly in
> nfp_net_txq_full()?
>
>
My idea was to add this indirection for handling dma mask when just part of
the IOVAs are not usable. Now, if the dma mask finds a problem, the PMD
does not make any port initialization.

Memory management is changing and ideally an app should just allocate
memory safe to be used by the PMD when that memory is going to be used for
sending or receiving data, what is not always the case.

It is true this indirection is not being used for any purpose by now, so
yes, I could use a direct call the the EAL one.


>
> ---
>>  lib/librte_ether/rte_ethdev.h           | 13 +++++++++++++
>>  lib/librte_ether/rte_ethdev_version.map |  1 +
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.
>> h
>> index eba11ca..e51a432 100644
>> --- a/lib/librte_ether/rte_ethdev.h
>> +++ b/lib/librte_ether/rte_ethdev.h
>> @@ -2799,6 +2799,19 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t
>> port_id,
>>  int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
>>
>>  /**
>> + * check device dma mask within expected range based on dma mask.
>> + *
>> + * @param maskbits
>> + *  mask length in bits
>> + *
>> + */
>> +static inline int
>> +rte_eth_dev_check_dma_mask(uint8_t maskbits)
>> +{
>> +       return rte_eal_check_dma_mask(maskbits);
>> +}
>> +
>> +/**
>>   *
>>   * Retrieve a burst of input packets from a receive queue of an Ethernet
>>   * device. The retrieved packets are stored in *rte_mbuf* structures
>> whose
>> diff --git a/lib/librte_ether/rte_ethdev_version.map
>> b/lib/librte_ether/rte_ethdev_version.map
>> index e9681ac..0b11b8a 100644
>> --- a/lib/librte_ether/rte_ethdev_version.map
>> +++ b/lib/librte_ether/rte_ethdev_version.map
>> @@ -191,6 +191,7 @@ DPDK_17.08 {
>>  DPDK_17.11 {
>>         global:
>>
>> +       rte_eth_dev_check_dma_mask;
>>         rte_eth_dev_get_sec_ctx;
>>         rte_eth_dev_pool_ops_supported;
>>         rte_eth_dev_reset;
>> --
>> 1.9.1
>>
>
Alejandro Lucero July 10, 2018, 9:44 a.m. UTC | #4 
On Tue, Jul 10, 2018 at 10:42 AM, Alejandro Lucero <
alejandro.lucero@netronome.com> wrote:

>
>
> On Tue, Jul 10, 2018 at 9:57 AM, Eelco Chaudron <echaudro@redhat.com>
> wrote:
>
>>
>>
>> On 4 Jul 2018, at 14:53, Alejandro Lucero wrote:
>>
>> A PMD should invoke this function for checking memsegs iovas are within
>>> the supported range by the device.
>>>
>>> Signed-off-by: Alejandro Lucero <alejandro.lucero@netronome.com>
>>>
>>
>> Agree with Andrew here, why not call rte_eal_check_dma_mask() directly in
>> nfp_net_txq_full()?
>>
>>
BTW, IOVA checking can not be done inside PMD functions like that one
because that is in the fast path. Any IOVA checking needs to be done at
initialization time.


>
> My idea was to add this indirection for handling dma mask when just part
> of the IOVAs are not usable. Now, if the dma mask finds a problem, the PMD
> does not make any port initialization.
>
> Memory management is changing and ideally an app should just allocate
> memory safe to be used by the PMD when that memory is going to be used for
> sending or receiving data, what is not always the case.
>
> It is true this indirection is not being used for any purpose by now, so
> yes, I could use a direct call the the EAL one.
>
>
>>
>> ---
>>>  lib/librte_ether/rte_ethdev.h           | 13 +++++++++++++
>>>  lib/librte_ether/rte_ethdev_version.map |  1 +
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/lib/librte_ether/rte_ethdev.h
>>> b/lib/librte_ether/rte_ethdev.h
>>> index eba11ca..e51a432 100644
>>> --- a/lib/librte_ether/rte_ethdev.h
>>> +++ b/lib/librte_ether/rte_ethdev.h
>>> @@ -2799,6 +2799,19 @@ int rte_eth_dev_set_vlan_ether_type(uint16_t
>>> port_id,
>>>  int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
>>>
>>>  /**
>>> + * check device dma mask within expected range based on dma mask.
>>> + *
>>> + * @param maskbits
>>> + *  mask length in bits
>>> + *
>>> + */
>>> +static inline int
>>> +rte_eth_dev_check_dma_mask(uint8_t maskbits)
>>> +{
>>> +       return rte_eal_check_dma_mask(maskbits);
>>> +}
>>> +
>>> +/**
>>>   *
>>>   * Retrieve a burst of input packets from a receive queue of an Ethernet
>>>   * device. The retrieved packets are stored in *rte_mbuf* structures
>>> whose
>>> diff --git a/lib/librte_ether/rte_ethdev_version.map
>>> b/lib/librte_ether/rte_ethdev_version.map
>>> index e9681ac..0b11b8a 100644
>>> --- a/lib/librte_ether/rte_ethdev_version.map
>>> +++ b/lib/librte_ether/rte_ethdev_version.map
>>> @@ -191,6 +191,7 @@ DPDK_17.08 {
>>>  DPDK_17.11 {
>>>         global:
>>>
>>> +       rte_eth_dev_check_dma_mask;
>>>         rte_eth_dev_get_sec_ctx;
>>>         rte_eth_dev_pool_ops_supported;
>>>         rte_eth_dev_reset;
>>> --
>>> 1.9.1
>>>
>>
>
diff mbox series

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index eba11ca..e51a432 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2799,6 +2799,19 @@  int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
 int rte_eth_dev_set_vlan_pvid(uint16_t port_id, uint16_t pvid, int on);
 
 /**
+ * check device dma mask within expected range based on dma mask.
+ *
+ * @param maskbits
+ *  mask length in bits
+ *
+ */
+static inline int
+rte_eth_dev_check_dma_mask(uint8_t maskbits)
+{
+	return rte_eal_check_dma_mask(maskbits);
+}
+
+/**
  *
  * Retrieve a burst of input packets from a receive queue of an Ethernet
  * device. The retrieved packets are stored in *rte_mbuf* structures whose
diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index e9681ac..0b11b8a 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -191,6 +191,7 @@  DPDK_17.08 {
 DPDK_17.11 {
 	global:
 
+	rte_eth_dev_check_dma_mask;
 	rte_eth_dev_get_sec_ctx;
 	rte_eth_dev_pool_ops_supported;
 	rte_eth_dev_reset;