[dpdk-dev] [PATCH v2 4/4] vfio: use the generic multi-process channel

[Burakov, Anatoly]

On 11-Jan-18 4:07 AM, Jianfeng Tan wrote:

<snip>

> -			}
> -			/* fall-through on error */
> -		default:
> -			RTE_LOG(ERR, EAL, "  cannot get container fd!\n");
> -			close(socket_fd);
> -			return -1;
> -		}
> +	vfio_group_fd = -1;
> +	ret = rte_eal_mp_request("vfio", &p, sizeof(p), &vfio_group_fd, 0, 1);
> +	if (ret > 0 && p.result == SOCKET_OK) {

Thanks, this looks much more clear than the previous revision! In an 
ideal world we would've been able to have separate response and reply 
(as it's perfectly possible to imagine a situation where the request 
would be small but the response would be huge), but for now this works 
as well. Maybe put this API down under EXPERIMENTAL tag? (btw wasn't 
this official policy now?)

> +		cur_grp->group_no = iommu_group_no;
> +		cur_grp->fd = vfio_group_fd;
> +		vfio_cfg.vfio_active_groups++;
> +		return vfio_group_fd;
>   	}
> +
> +	RTE_LOG(ERR, EAL, "  cannot request group fd\n");
>   	return -1;

check for SOCKET_NO_FD? Previously, that branch returned 0, now it will 
return -1.

>   }
>   
> @@ -200,7 +174,8 @@ int
>   clear_group(int vfio_group_fd)
>   {
>   	int i;
> -	int socket_fd, ret;
> +	int ret;
> +	struct vfio_mp_param p;
>   
>   	if (internal_config.process_type == RTE_PROC_PRIMARY) {
>   
> @@ -214,43 +189,14 @@ clear_group(int vfio_group_fd)
>   		return 0;
>   	}
>   
> -	/* This is just for SECONDARY processes */
> -	socket_fd = vfio_mp_sync_connect_to_primary();
> -
> -	if (socket_fd < 0) {
> -		RTE_LOG(ERR, EAL, "  cannot connect to primary process!\n");
> -		return -1;
> -	}
> -
> -	if (vfio_mp_sync_send_request(socket_fd, SOCKET_CLR_GROUP) < 0) {
> -		RTE_LOG(ERR, EAL, "  cannot request container fd!\n");
> -		close(socket_fd);
> -		return -1;
> -	}
> -
> -	if (vfio_mp_sync_send_request(socket_fd, vfio_group_fd) < 0) {
> -		RTE_LOG(ERR, EAL, "  cannot send group fd!\n");
> -		close(socket_fd);
> -		return -1;
> -	}
> +	p.req = SOCKET_CLR_GROUP;
> +	p.group_no = vfio_group_fd;
>   
> -	ret = vfio_mp_sync_receive_request(socket_fd);
> -	switch (ret) {
> -	case SOCKET_NO_FD:
> -		RTE_LOG(ERR, EAL, "  BAD VFIO group fd!\n");
> -		close(socket_fd);
> -		break;
> -	case SOCKET_OK:
> -		close(socket_fd);
> +	ret = rte_eal_mp_request("vfio", &p, sizeof(p), NULL, 0, 0);
> +	if (ret > 0 && p.result == SOCKET_OK)
>   		return 0;
> -	case SOCKET_ERR:
> -		RTE_LOG(ERR, EAL, "  Socket error\n");
> -		close(socket_fd);
> -		break;
> -	default:
> -		RTE_LOG(ERR, EAL, "  UNKNOWN reply, %d\n", ret);
> -		close(socket_fd);
> -	}
> +
> +	RTE_LOG(ERR, EAL, "  BAD VFIO group fd!\n");

The error message lumps together two cases - bad VFIO group fd, and a 
socket error.

>   	return -1;
>   }
>   
> @@ -561,6 +507,7 @@ int
>   vfio_get_container_fd(void)
>   {
>   	int ret, vfio_container_fd;
> +	struct vfio_mp_param p;
>   
>   	/* if we're in a primary process, try to open the container */
>   	if (internal_config.process_type == RTE_PROC_PRIMARY) {
> @@ -591,33 +538,19 @@ vfio_get_container_fd(void)
>   		}

<snip>


-- 
Thanks,
Anatoly