From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D71B0A050F; Wed, 27 Apr 2022 06:12:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CA433410E8; Wed, 27 Apr 2022 06:12:09 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id 7B6E140E78 for ; Wed, 27 Apr 2022 06:12:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1651032728; x=1682568728; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=3uDkwQRKtIuEb2a+snCZ9jxqN3ha+IQNTlDXOfe5M9Q=; b=CNCbbqsH5j9MqY418KaDd0/0RTVDqIJB/9nB+iqgGB6c2eAVUOgJ25Gl +DlTznBlQxc3Qx62WEq3+X20AM+1R4vhpo1we715gpH3KkgEMxbFQgFKg 6tH7rxOB2mpWZC5lzc9wK637vKqbjyzjz3h8ml1VDcbvI/O+1AfBFz0gY ug/xkTudc0/Qcarlpi0IUt/hiqI5Lcnt48UUOLBY5bYaqGvwfa+sp3PoW sNcnRA3bHDuOERl0p3KBABlWyLyb+1V9UMeEAzE+FwCXaH+B+TfzfD/Ji L6L04HTBAOPKhEPGAsPUmT63dAZVT30nnhB2gTaJAX+KJcwPZQu/CuxEW g==; X-IronPort-AV: E=McAfee;i="6400,9594,10329"; a="290955946" X-IronPort-AV: E=Sophos;i="5.90,292,1643702400"; d="scan'208";a="290955946" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Apr 2022 21:12:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,292,1643702400"; d="scan'208";a="564893078" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by fmsmga007.fm.intel.com with ESMTP; 26 Apr 2022 21:12:05 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Tue, 26 Apr 2022 21:12:05 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.27; Tue, 26 Apr 2022 21:12:04 -0700 Received: from fmsmsx612.amr.corp.intel.com ([10.18.126.92]) by fmsmsx612.amr.corp.intel.com ([10.18.126.92]) with mapi id 15.01.2308.027; Tue, 26 Apr 2022 21:12:04 -0700 From: "Pei, Andy" To: "Xia, Chenbo" , "dev@dpdk.org" CC: "maxime.coquelin@redhat.com" , "Cao, Gang" , "Liu, Changpeng" Subject: RE: [PATCH v6 03/16] vhost: add vhost msg support Thread-Topic: [PATCH v6 03/16] vhost: add vhost msg support Thread-Index: AQHYVWDyg9aKWqY3Fkmb/iZeErWw3q0Alh8AgAFMdyCAAITJgIAAx5UQ Date: Wed, 27 Apr 2022 04:12:04 +0000 Message-ID: References: <1643093258-47258-2-git-send-email-andy.pei@intel.com> <1650530034-59744-1-git-send-email-andy.pei@intel.com> <1650530034-59744-4-git-send-email-andy.pei@intel.com> <08f0027681ff4583bf46a835a5e8c9ef@intel.com> In-Reply-To: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Chenbo,=20 Thanks for your reply. My reply is inline. > -----Original Message----- > From: Xia, Chenbo > Sent: Tuesday, April 26, 2022 5:17 PM > To: Pei, Andy ; dev@dpdk.org > Cc: maxime.coquelin@redhat.com; Cao, Gang ; Liu, > Changpeng > Subject: RE: [PATCH v6 03/16] vhost: add vhost msg support >=20 > > -----Original Message----- > > From: Pei, Andy > > Sent: Tuesday, April 26, 2022 4:56 PM > > To: Xia, Chenbo ; dev@dpdk.org > > Cc: maxime.coquelin@redhat.com; Cao, Gang ; Liu, > > Changpeng > > Subject: RE: [PATCH v6 03/16] vhost: add vhost msg support > > > > HI Chenbo, > > > > Thanks for your reply. > > My reply is inline. > > > > > -----Original Message----- > > > From: Xia, Chenbo > > > Sent: Monday, April 25, 2022 8:42 PM > > > To: Pei, Andy ; dev@dpdk.org > > > Cc: maxime.coquelin@redhat.com; Cao, Gang ; Liu, > > > Changpeng > > > Subject: RE: [PATCH v6 03/16] vhost: add vhost msg support > > > > > > Hi Andy, > > > > > > > -----Original Message----- > > > > From: Pei, Andy > > > > Sent: Thursday, April 21, 2022 4:34 PM > > > > To: dev@dpdk.org > > > > Cc: Xia, Chenbo ; > > > > maxime.coquelin@redhat.com; Cao, Gang ; Liu, > > > > Changpeng > > > > Subject: [PATCH v6 03/16] vhost: add vhost msg support > > > > > > > > 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 > > > > --- > > > > lib/vhost/vhost_user.c | 69 > > > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > > lib/vhost/vhost_user.h | 13 ++++++++++ > > > > 2 files changed, 82 insertions(+) > > > > > > > > diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index > > > > 1d39067..3780804 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,71 @@ 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, > > > > + "(%s) get_config() return error!\n", > > > > + dev->ifname); > > > > +} > > > > +} else { > > > > +VHOST_LOG_CONFIG(ERR, "(%s) get_config() not > > > supportted!\n", > > > > > > Supported > > > > > I will send out a new version to fix this. > > > > + dev->ifname); > > > > +} > > > > + > > > > +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 (ctx->msg.size !=3D sizeof(struct vhost_user_config)) { > > > > > > I think you should do sanity check on payload.cfg.size and make sure > > it's > > > smaller than VHOST_USER_MAX_CONFIG_SIZE > > > > > > and same check for offset > > > > > I think payload.cfg.size can be smaller than or equal to > > VHOST_USER_MAX_CONFIG_SIZE. > > payload.cfg.ofset can be smaller than or equal to > > VHOST_USER_MAX_CONFIG_SIZE as well >=20 > After double check: offset is the config space offset, so this should be > checked in vdpa driver. Size check on vhost lib layer should be just <=3D > MAX_you_defined >=20 OK. > Thanks, > Chenbo >=20 > > > > > > +VHOST_LOG_CONFIG(ERR, > > > > +"(%s) invalid set config msg size: %"PRId32" !=3D %d\n", > > > > +dev->ifname, ctx->msg.size, > > > > > > Based on you will change the log too, payload.cfg.size is uint32_t, > > > so > > PRId32 -> > > > PRIu32 > > > > > > > +(int)sizeof(struct vhost_user_config)); > > > > > > So this can be %u > > > > > Sure. > > > > +goto OUT; > > > > +} > > > > + > > > > +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); > > > > +if (ret) > > > > +VHOST_LOG_CONFIG(ERR, > > > > + "(%s) set_config() return error!\n", > > > > + dev->ifname); > > > > +} else { > > > > +VHOST_LOG_CONFIG(ERR, "(%s) set_config() not > > > supportted!\n", > > > > > > Supported > > > > > I will send out a new version to fix this. > > > > + dev->ifname); > > > > +} > > > > + > > > > +return RTE_VHOST_MSG_RESULT_OK; > > > > + > > > > +OUT: > > > > > > Lower case looks better > > > > > OK. I will send out a new version to fix this. > > > > +return RTE_VHOST_MSG_RESULT_ERR; > > > > +} > > > > > > Almost all handlers need check on expected fd num (this case is 0), > > > so > > the > > > above new handlers should also do that. Please refer to > > > validate_msg_fds > > in > > > other handlers. > > > > > > BTW, you can wait for review for other patches and send new versions > > later. > > > > > I will send out new patch after vhost: validate fds attached to > > messages from David Marchand is merged. > > > Thanks, > > > Chenbo > > > > > > > + > > > > +static int > > > > vhost_user_iotlb_msg(struct virtio_net **pdev, struct > > > > vhu_msg_context *ctx, int main_fd __rte_unused) @@ -2782,6 > > > > +2849,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..97cfb2f 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,16 @@ > > > > uint16_t queue_size; } VhostUserInflight; > > > > > > > > +#define VHOST_USER_MAX_CONFIG_SIZE256 > > > > + > > > > +/** 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]; > > > > +}; > > > > + > > > > typedef struct VhostUserMsg { > > > > union { > > > > uint32_t master; /* a VhostUserRequest value */ @@ -148,6 > > > +160,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 > > > > > >=20