Re: [PATCH v2 10/21] net/ena: proxy AQ calls to primary process

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 2/22/2022 6:11 PM, Michal Krawczyk wrote:
> From: Stanislaw Kardach <kda@semihalf.com>
> 
> Due to how the ena_com compatibility layer is written, all AQ commands
> triggering functions use stack to save results of AQ and then copy them
> to user given function.
> Therefore to keep the compatibility layer common, introduce ENA_PROXY
> macro. It either calls the wrapped function directly (in primary
> process) or proxies it to the primary via DPDK IPC mechanism. Since all
> proxied calls are taken under a lock share the result data through
> shared memory (in struct ena_adapter) to work around 256B IPC parameter
> size limit.
> 
> New proxy calls can be added by
> 1. Adding a new message type at the end of enum ena_mp_req
> 2. Adding new message arguments to the struct ena_mp_body if needed
> 3. Defining proxy request descriptor with ENA_PROXY_DESC. Its arguments
>     include handlers for request preparation and response processing.
>     Any of those may be empty (aside of marking arguments as used).
> 4. Adding request handling logic to ena_mp_primary_handle()
> 5. Replacing proxied function calls with ENA_PROXY(adapter, <func>, ...)
> 
> Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
> Reviewed-by: Michal Krawczyk <mk@semihalf.com>
> Reviewed-by: Dawid Gorecki <dgr@semihalf.com>
> Reviewed-by: Shai Brandes <shaibran@amazon.com>

<...>

> + */
> +#define ENA_PROXY(a, f, ...)						\
> +({									\
> +	struct ena_adapter *_a = (a);					\
> +	struct timespec ts = { .tv_sec = ENA_MP_REQ_TMO };		\
> +	struct ena_mp_body *req, *rsp;					\
> +	struct rte_mp_reply mp_rep;					\
> +	struct rte_mp_msg mp_req;					\
> +	int ret;							\
> +									\
> +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {		\
> +		ret = f(__VA_ARGS__);					\
> +	} else {							\
> +		/* Prepare and send request */				\
> +		req = (struct ena_mp_body *)&mp_req.param;		\
> +		mp_msg_init(&mp_req, mp_type_ ## f, _a->edev_data->port_id); \
> +		mp_prep_ ## f(_a, req, ## __VA_ARGS__);			\
> +									\
> +		ret = rte_mp_request_sync(&mp_req, &mp_rep, &ts);	\
> +		if (likely(!ret)) {					\
> +			RTE_ASSERT(mp_rsp.nb_received == 1);		\

'mp_rsp' is not defined, I assume intention is to use 'mp_rep'.

Build error can be reproduced by enabling RTE_ASSERT (by setting
'RTE_ENABLE_ASSERT').

> +			rsp = (struct ena_mp_body *)&mp_rep.msgs[0].param; \
> +			ret = rsp->result;				\
> +			if (ret == 0) {					\
> +				mp_proc_##f(_a, rsp, ## __VA_ARGS__);	\
> +			} else {					\
> +				PMD_DRV_LOG(ERR,			\
> +					    "%s returned error: %d\n",	\
> +					    mp_name_ ## f, rsp->result);\
> +			}						\
> +			free(mp_rep.msgs);				\
> +		} else if (rte_errno == ENOTSUP) {			\
> +			PMD_DRV_LOG(ERR,				\
> +				    "No IPC, can't proxy to primary\n");\
> +			ret = -rte_errno;				\
> +		} else {						\
> +			PMD_DRV_LOG(ERR, "Request %s failed: %s\n",	\
> +				    mp_name_ ## f,			\
> +				    rte_strerror(rte_errno));		\
> +			ret = -ECOMM;					\
> +		}							\
> +	}								\
> +	ret;								\
> +})
> 
<...>