Re: [dpdk-dev] [PATCH v2 3/3] net/mlx4: add secondary process support

[Yongseok Koh <yskoh@mellanox.com>]

> On Mar 26, 2019, at 12:33 PM, Shahaf Shuler <shahafs@mellanox.com> wrote:
> 
> Monday, March 25, 2019 9:18 PM, Yongseok Koh:
>> To: Shahaf Shuler <shahafs@mellanox.com>
>> Cc: dev@dpdk.org
>> Subject: [PATCH v2 3/3] net/mlx4: add secondary process support
>> 
>> In order to support secondary process, a few features are required.
>> 
>> a) rdma-core library should allocate device resources using DPDK's memory
>>   allocator.
>> 
>> b) UAR should be remapped for secondary processes. Currently, in order not
>>   to use different data structure for secondary processes, PMD tries to
>>   reserve identical virtual address space for both primary and secondary
>>   processes.
>> 
>> c) IPC channel is necessary, which can be easily set with rte_mp APIs.
>>   Through the channel, Verbs command FD is delivered to the secondary
>>   process and the device stop/start event is also broadcast from primary
>>   process.
>> 
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> ---
>> doc/guides/nics/features/mlx4.ini |   1 +
>> doc/guides/nics/mlx4.rst          |  10 +
>> drivers/net/mlx4/Makefile         |   6 +
>> drivers/net/mlx4/meson.build      |   3 +
>> drivers/net/mlx4/mlx4.c           | 378
>> ++++++++++++++++++++++++++++++++++++--
>> drivers/net/mlx4/mlx4.h           |  60 ++++++
>> drivers/net/mlx4/mlx4_mp.c        | 304
>> ++++++++++++++++++++++++++++++
>> drivers/net/mlx4/mlx4_mr.c        |  32 +++-
>> drivers/net/mlx4/mlx4_prm.h       |   4 +-
>> drivers/net/mlx4/mlx4_rxtx.c      |   2 +
>> drivers/net/mlx4/mlx4_rxtx.h      |   1 +
>> drivers/net/mlx4/mlx4_txq.c       | 111 +++++++++++
>> 12 files changed, 890 insertions(+), 22 deletions(-)  create mode 100644
>> drivers/net/mlx4/mlx4_mp.c
>> 
>> diff --git a/doc/guides/nics/features/mlx4.ini
>> b/doc/guides/nics/features/mlx4.ini
>> index a211aef332..4502aa2a87 100644
>> --- a/doc/guides/nics/features/mlx4.ini
>> +++ b/doc/guides/nics/features/mlx4.ini
>> @@ -29,6 +29,7 @@ Packet type parsing  = Y
>> Basic stats          = Y
>> Stats per queue      = Y
>> FW version           = Y
>> +Multiprocess aware   = Y
>> Other kdrv           = Y
>> Power8               = Y
>> x86-32               = Y
>> diff --git a/doc/guides/nics/mlx4.rst b/doc/guides/nics/mlx4.rst index
>> 4ad361a2c2..cd34838f41 100644
>> --- a/doc/guides/nics/mlx4.rst
>> +++ b/doc/guides/nics/mlx4.rst
>> @@ -145,6 +145,16 @@ below.
>> Limitations
>> -----------
>> 
>> +- For secondary process:
>> +
>> +  - Forked secondary process not supported.
>> +  - All mempools must be initialized before rte_eth_dev_start().
>> +  - External memory unregistered in EAL memseg list cannot be used for
>> DMA
>> +    unless such memory has been registered by
>> ``mlx4_mr_update_ext_mp()`` in
>> +    primary process and remapped to the same virtual address in secondary
>> +    process. If the external memory is registered by primary process but has
>> +    different virtual address in secondary process, unexpected error may
>> happen.
>> +
>> - CRC stripping is supported by default and always reported as "true".
>>   The ability to enable/disable CRC stripping requires OFED version
>>   4.3-1.5.0.0 and above  or rdma-core version v18 and above.
>> diff --git a/drivers/net/mlx4/Makefile b/drivers/net/mlx4/Makefile index
>> b527efd625..8126b0dfc6 100644
>> --- a/drivers/net/mlx4/Makefile
>> +++ b/drivers/net/mlx4/Makefile
>> @@ -18,6 +18,7 @@ ifneq ($(CONFIG_RTE_IBVERBS_LINK_DLOPEN),y)
>> SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_glue.c  endif
>> SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_intr.c
>> +SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_mp.c
>> SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_mr.c
>> SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_rxq.c
>> SRCS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4_rxtx.c @@ -93,6 +94,11
>> @@ mlx4_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
>> 		enum MLX4DV_SET_CTX_ATTR_BUF_ALLOCATORS \
>> 		$(AUTOCONF_OUTPUT)
>> 	$Q sh -- '$<' '$@' \
>> +		HAVE_IBV_MLX4_UAR_MMAP_OFFSET \
>> +		infiniband/mlx4dv.h \
>> +		enum MLX4DV_QP_MASK_UAR_MMAP_OFFSET \
>> +		$(AUTOCONF_OUTPUT)
>> +	$Q sh -- '$<' '$@' \
>> 		HAVE_IBV_MLX4_WQE_LSO_SEG \
>> 		infiniband/mlx4dv.h \
>> 		type 'struct mlx4_wqe_lso_seg' \
>> diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
>> index 650e2c8fbc..de020701d1 100644
>> --- a/drivers/net/mlx4/meson.build
>> +++ b/drivers/net/mlx4/meson.build
>> @@ -33,6 +33,7 @@ if build
>> 		'mlx4_ethdev.c',
>> 		'mlx4_flow.c',
>> 		'mlx4_intr.c',
>> +		'mlx4_mp.c',
>> 		'mlx4_mr.c',
>> 		'mlx4_rxq.c',
>> 		'mlx4_rxtx.c',
>> @@ -76,6 +77,8 @@ if build
>> 	has_sym_args = [
>> 		[ 'HAVE_IBV_MLX4_BUF_ALLOCATORS',
>> 'infiniband/mlx4dv.h',
>> 		'MLX4DV_SET_CTX_ATTR_BUF_ALLOCATORS' ],
>> +		[ 'HAVE_IBV_MLX4_UAR_MMAP_OFFSET',
>> 'infiniband/mlx4dv.h',
>> +		'MLX4DV_QP_MASK_UAR_MMAP_OFFSET' ],
>> 	]
>> 	config = configuration_data()
>> 	foreach arg:has_sym_args
>> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index
>> 0e0b035df0..a5cfcdbee3 100644
>> --- a/drivers/net/mlx4/mlx4.c
>> +++ b/drivers/net/mlx4/mlx4.c
>> @@ -17,6 +17,7 @@
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> +#include <sys/mman.h>
>> #include <unistd.h>
>> 
>> /* Verbs headers do not support -pedantic. */ @@ -48,10 +49,21 @@
>> #include "mlx4_rxtx.h"
>> #include "mlx4_utils.h"
>> 
>> -struct mlx4_dev_list mlx4_mem_event_cb_list =
>> -	LIST_HEAD_INITIALIZER(mlx4_mem_event_cb_list);
>> +#if defined(HAVE_IBV_MLX4_UAR_MMAP_OFFSET) && \
>> +	defined(HAVE_IBV_MLX4_BUF_ALLOCATORS)
>> +#define HAVE_IBV_MLX4_SECONDARY_PROCESS #endif
> 
> Features should not be detected on compilation time rather by run time based on capabilities. 
> On this case, 
> If you are able to register the external allocator (dv call returns w/ success) and the mmap for the uar index also succeed, then you have support for secondary.

A bit confused.
Do you want to have redundant definitions in mlx5_prm.h in order to make the test calls?
Eg., MLX4DV_SET_CTX_ATTR_BUF_ALLOCATORS and MLX4DV_QP_MASK_UAR_MMAP_OFFSET.

Thanks,
Yongseok