From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 28970A00C3;
Thu, 21 Apr 2022 10:05:25 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 0BFB240042;
Thu, 21 Apr 2022 10:05:25 +0200 (CEST)
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
by mails.dpdk.org (Postfix) with ESMTP id 0DC7040040
for <dev@dpdk.org>; Thu, 21 Apr 2022 10:05:23 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
d=intel.com; i=@intel.com; q=dns/txt; s=Intel;
t=1650528324; x=1682064324;
h=from:to:cc:subject:date:message-id:references:
in-reply-to:content-transfer-encoding:mime-version;
bh=L01PhBd55cqOvIZ9Op/Fi5tiCVxglahqxLOrzEGNlyo=;
b=B6PpB3i3PTAWyN7FDsT75baucwPcOss3ywQ/9ztLr0x4VExEqvnwvwxJ
EgXXJ2TkDvJLqPAjF3T/iCzVz6ALKcna78ekgDKgntlIG1acafZqQYYIg
g7rGvn2k/MTYI8nBw9BCMcyS0u3xJHYHKI+yxV524aYDTmGO8ytITCt3x
1VNkftib9TxzhQc1YUGnGpuTOWo9P6RYOHMr6lep3AigPBdF0gR9169id
JXc9bm8jh9nOdfIKLI8clWAxLR8igw/qMmw8DiS5Eo6bRDHPNVwkpuV6g
I3kNdCXzw1OnQwRRlWkI2kFYGjWfZUaR+VKXwuPkDAU48ElttssLDZu/l w==;
X-IronPort-AV: E=McAfee;i="6400,9594,10323"; a="327178259"
X-IronPort-AV: E=Sophos;i="5.90,278,1643702400"; d="scan'208";a="327178259"
Received: from orsmga007.jf.intel.com ([10.7.209.58])
by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
21 Apr 2022 01:05:23 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.90,278,1643702400"; d="scan'208";a="555644032"
Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84])
by orsmga007.jf.intel.com with ESMTP; 21 Apr 2022 01:05:22 -0700
Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by
fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2308.27; Thu, 21 Apr 2022 01:05:22 -0700
Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by
fmsmsx604.amr.corp.intel.com ([10.18.126.84]) with mapi id 15.01.2308.027;
Thu, 21 Apr 2022 01:05:21 -0700
From: "Pei, Andy" <andy.pei@intel.com>
To: "Xia, Chenbo" <chenbo.xia@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "Cao, Gang"
<gang.cao@intel.com>, "Liu, Changpeng" <changpeng.liu@intel.com>
Subject: RE: [PATCH v5 03/16] vhost: add support for VHOST_USER_GET_CONFIG and
VHOST_USER_SET_CONFIG
Thread-Topic: [PATCH v5 03/16] vhost: add support for VHOST_USER_GET_CONFIG
and VHOST_USER_SET_CONFIG
Thread-Index: AQHYQnootaV/d0GWXU+7ZRONidnwW6z4776QgAEtLuA=
Date: Thu, 21 Apr 2022 08:05:21 +0000
Message-ID: <13e8ea5ce45340829b7d34c22946fb53@intel.com>
References: <1643093258-47258-2-git-send-email-andy.pei@intel.com>
<1648451862-123318-1-git-send-email-andy.pei@intel.com>
<1648451862-123318-4-git-send-email-andy.pei@intel.com>
<SN6PR11MB3504DB1FD2B35C181E64BA8D9CF59@SN6PR11MB3504.namprd11.prod.outlook.com>
In-Reply-To: <SN6PR11MB3504DB1FD2B35C181E64BA8D9CF59@SN6PR11MB3504.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
dlp-product: dlpe-windows
dlp-version: 11.6.401.20
dlp-reaction: no-action
x-originating-ip: [10.239.127.36]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
HI Chenbo,
Thanks for your reply.
My reply is inline.
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, April 20, 2022 9:53 PM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Cao, Gang <gang.cao@intel.com>; Liu,
> Changpeng <changpeng.liu@intel.com>
> Subject: RE: [PATCH v5 03/16] vhost: add support for
> VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG
>
> Hi Andy,
>
> > -----Original Message-----
> > From: Pei, Andy <andy.pei@intel.com>
> > Sent: Monday, March 28, 2022 3:17 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com;
> > Cao, Gang <gang.cao@intel.com>; Liu, Changpeng
> > <changpeng.liu@intel.com>
> > Subject: [PATCH v5 03/16] vhost: add support for
> VHOST_USER_GET_CONFIG
> > and VHOST_USER_SET_CONFIG
>
> Let's make the title a bit short...
>
> ./devtools/check-git-log.sh will help you find other similar errors for o=
ther
> patches.
>
OK, I will send out V6 to fix commit log title.
Thanks for your suggestion.
> >
> > Add support for VHOST_USER_GET_CONFIG and
> VHOST_USER_SET_CONFIG.
> > VHOST_USER_GET_CONFIG and VHOST_USER_SET_CONFIG message is only
> > supported by virtio blk VDPA device.
> >
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > ---
> > lib/vhost/vhost_user.c | 50
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > lib/vhost/vhost_user.h | 15 +++++++++++++++
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> > 1d39067..55e8bd0 100644
> > --- a/lib/vhost/vhost_user.c
> > +++ b/lib/vhost/vhost_user.c
> > @@ -80,6 +80,8 @@
> > [VHOST_USER_NET_SET_MTU] =3D "VHOST_USER_NET_SET_MTU",
> > [VHOST_USER_SET_SLAVE_REQ_FD] =3D
> "VHOST_USER_SET_SLAVE_REQ_FD",
> > [VHOST_USER_IOTLB_MSG] =3D "VHOST_USER_IOTLB_MSG",
> > + [VHOST_USER_GET_CONFIG] =3D "VHOST_USER_GET_CONFIG",
> > + [VHOST_USER_SET_CONFIG] =3D "VHOST_USER_SET_CONFIG",
> > [VHOST_USER_CRYPTO_CREATE_SESS] =3D
> "VHOST_USER_CRYPTO_CREATE_SESS",
> > [VHOST_USER_CRYPTO_CLOSE_SESS] =3D
> "VHOST_USER_CRYPTO_CLOSE_SESS",
> > [VHOST_USER_POSTCOPY_ADVISE] =3D
> "VHOST_USER_POSTCOPY_ADVISE", @@
> > -2542,6 +2544,52 @@ static int is_vring_iotlb(struct virtio_net *dev,
> > }
> >
> > static int
> > +vhost_user_get_config(struct virtio_net **pdev,
> > + struct vhu_msg_context *ctx,
> > + int main_fd __rte_unused)
> > +{
> > + struct virtio_net *dev =3D *pdev;
> > + struct rte_vdpa_device *vdpa_dev =3D dev->vdpa_dev;
> > + int ret =3D 0;
> > +
> > + if (vdpa_dev->ops->get_config) {
> > + ret =3D vdpa_dev->ops->get_config(dev->vid,
> > + ctx->msg.payload.cfg.region,
> > + ctx->msg.payload.cfg.size);
> > + if (ret !=3D 0) {
> > + ctx->msg.size =3D 0;
> > + VHOST_LOG_CONFIG(ERR, "get_config() return
> error!\n");
> > + }
> > + } else {
> > + VHOST_LOG_CONFIG(ERR, "get_config() not supported!\n");
> > + }
> > +
> > + return RTE_VHOST_MSG_RESULT_REPLY;
> > +}
> > +
> > +static int
> > +vhost_user_set_config(struct virtio_net **pdev,
> > + struct vhu_msg_context *ctx,
> > + int main_fd __rte_unused)
> > +{
> > + struct virtio_net *dev =3D *pdev;
> > + struct rte_vdpa_device *vdpa_dev =3D dev->vdpa_dev;
> > + int ret =3D 0;
> > +
> > + if (vdpa_dev->ops->set_config) {
> > + ret =3D vdpa_dev->ops->set_config(dev->vid,
> > + ctx->msg.payload.cfg.region,
> > + ctx->msg.payload.cfg.offset,
> > + ctx->msg.payload.cfg.size,
> > + ctx->msg.payload.cfg.flags);
> > + } else {
> > + VHOST_LOG_CONFIG(ERR, "set_config() not supported!\n");
> > + }
> > +
> > + return ret =3D=3D 0 ? RTE_VHOST_MSG_RESULT_OK :
> > +RTE_VHOST_MSG_RESULT_ERR;
>
> I think when set_config fails in vdpa driver, it should not break message
> handler by returning RESULT_ERR here.
>
I will return RTE_VHOST_MSG_RESULT_OK and output some log.
> All error log above, please print dev->ifname too, which will be user-fri=
endly.
>
Sure.Thansk.
> > +}
> > +
> > +static int
> > vhost_user_iotlb_msg(struct virtio_net **pdev,
> > struct vhu_msg_context *ctx,
> > int main_fd __rte_unused)
> > @@ -2782,6 +2830,8 @@ typedef int (*vhost_message_handler_t)(struct
> > virtio_net **pdev,
> > [VHOST_USER_NET_SET_MTU] =3D vhost_user_net_set_mtu,
> > [VHOST_USER_SET_SLAVE_REQ_FD] =3D vhost_user_set_req_fd,
> > [VHOST_USER_IOTLB_MSG] =3D vhost_user_iotlb_msg,
> > + [VHOST_USER_GET_CONFIG] =3D vhost_user_get_config,
> > + [VHOST_USER_SET_CONFIG] =3D vhost_user_set_config,
> > [VHOST_USER_POSTCOPY_ADVISE] =3D
> vhost_user_set_postcopy_advise,
> > [VHOST_USER_POSTCOPY_LISTEN] =3D vhost_user_set_postcopy_listen,
> > [VHOST_USER_POSTCOPY_END] =3D vhost_user_postcopy_end, diff --
> git
> > a/lib/vhost/vhost_user.h b/lib/vhost/vhost_user.h index
> > c946cc2..d3f014e 100644
> > --- a/lib/vhost/vhost_user.h
> > +++ b/lib/vhost/vhost_user.h
> > @@ -50,6 +50,8 @@
> > VHOST_USER_NET_SET_MTU =3D 20,
> > VHOST_USER_SET_SLAVE_REQ_FD =3D 21,
> > VHOST_USER_IOTLB_MSG =3D 22,
> > + VHOST_USER_GET_CONFIG =3D 24,
> > + VHOST_USER_SET_CONFIG =3D 25,
> > VHOST_USER_CRYPTO_CREATE_SESS =3D 26,
> > VHOST_USER_CRYPTO_CLOSE_SESS =3D 27,
> > VHOST_USER_POSTCOPY_ADVISE =3D 28,
> > @@ -125,6 +127,18 @@
> > uint16_t queue_size;
> > } VhostUserInflight;
> >
> > +#ifndef VHOST_USER_MAX_CONFIG_SIZE
> > +#define VHOST_USER_MAX_CONFIG_SIZE 256
> > +#endif
>
> For this config size, maybe '+#define VHOST_USER_MAX_CONFIG_SIZE 256' is
> enough?
>
Sure.
> > +
> > +/** Get/set config msg payload */
> > +struct vhost_user_config {
> > + uint32_t offset;
> > + uint32_t size;
> > + uint32_t flags;
> > + uint8_t region[VHOST_USER_MAX_CONFIG_SIZE];
> > +};
>
> Since the max size is defined, in the handler, we should check the size i=
n the
> msg handler.
Sure.
>
> Thanks,
> Chenbo
>
> > +
> > typedef struct VhostUserMsg {
> > union {
> > uint32_t master; /* a VhostUserRequest value */ @@ -148,6
> +162,7 @@
> > VhostUserCryptoSessionParam crypto_session;
> > VhostUserVringArea area;
> > VhostUserInflight inflight;
> > + struct vhost_user_config cfg;
> > } payload;
> > /* Nothing should be added after the payload */ } __rte_packed
> > VhostUserMsg;
> > --
> > 1.8.3.1
>