[EXT] Re: [PATCH v3 3/5] lib: move mbuf next pointer to first cache line

[Shijith Thotton]

>> Swapped position of mbuf next pointer and second dynamic field (dynfield2)
>> if the build is configured to use IOVA as VA. This is to move the mbuf
>> next pointer to first cache line. kni library is disabled for this
>> change as it depends on the offset value of next pointer.
>>
>> Signed-off-by: Shijith Thotton <sthotton at marvell.com>
>> ---
>>  lib/mbuf/rte_mbuf_core.h | 29 +++++++++++++++++++++--------
>>  lib/meson.build          |  3 +++
>>  2 files changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h
>> index 94907f301d..915dcd8653 100644
>> --- a/lib/mbuf/rte_mbuf_core.h
>> +++ b/lib/mbuf/rte_mbuf_core.h
>> @@ -590,11 +590,14 @@ struct rte_mbuf {
>>  		 * working on vector drivers easier.
>>  		 */
>>  		rte_iova_t buf_iova __rte_aligned(sizeof(rte_iova_t));
>> +#if RTE_IOVA_AS_VA
>>  		/**
>> -		 * Reserved for dynamic field in builds where physical address
>> -		 * field is invalid.
>> +		 * Next segment of scattered packet.
>> +		 * This field is valid when physical address field is invalid.
>> +		 * Otherwise next pointer in the second cache line will be used.
>>  		 */
>> -		uint64_t dynfield2;
>> +		struct rte_mbuf *next;
>> +#endif
>>  	};
>>
>>  	/* next 8 bytes are initialised on RX descriptor rearm */
>> @@ -711,11 +714,21 @@ struct rte_mbuf {
>>  	/* second cache line - fields only used in slow path or on TX */
>>  	RTE_MARKER cacheline1 __rte_cache_min_aligned;
>>
>> -	/**
>> -	 * Next segment of scattered packet. Must be NULL in the last segment or
>> -	 * in case of non-segmented packet.
>> -	 */
>> -	struct rte_mbuf *next;
>> +	RTE_STD_C11
>> +	union {
>> +#if !RTE_IOVA_AS_VA
>> +		/**
>> +		 * Next segment of scattered packet. Must be NULL in the last
>> +		 * segment or in case of non-segmented packet.
>> +		 */
>> +		struct rte_mbuf *next;
>> +#endif
>> +		/**
>> +		 * Reserved for dynamic field when the next pointer is in first
>> +		 * cache line (i.e. RTE_IOVA_AS_VA is 1).
>> +		 */
>> +		uint64_t dynfield2;
>> +	};
>
>Same comment than other patches about union vs #if.
 
Okay. Will change.

>
>>
>>  	/* fields to support TX offloads */
>>  	RTE_STD_C11
>> diff --git a/lib/meson.build b/lib/meson.build
>> index c648f7d800..73d93bc803 100644
>> --- a/lib/meson.build
>> +++ b/lib/meson.build
>> @@ -88,6 +88,9 @@ optional_libs = [
>>  disabled_libs = []
>>  opt_disabled_libs = run_command(list_dir_globs, get_option('disable_libs'),
>>          check: true).stdout().split()
>> +if dpdk_conf.get('RTE_IOVA_AS_VA') == 1
>> +    opt_disabled_libs += ['kni']
>> +endif
>
>I guess this should be in the previous patch instead, since kni uses
>m->buf_iova
>
 
Ack.

>>  foreach l:opt_disabled_libs
>>      if not optional_libs.contains(l)
>>          warning('Cannot disable mandatory library "@0@"'.format(l))
>
>
>After this change, the documentation of RTE_IOVA_AS_VA can be enhanced to
>explain that it also moves the next pointer to the first cache line, possibly
>increasing the performance.
 
Okay.