From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8A7C0A050E; Wed, 18 Dec 2019 07:35:13 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9C00C2BE5; Wed, 18 Dec 2019 07:35:12 +0100 (CET) Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id ED835F90 for ; Wed, 18 Dec 2019 07:35:11 +0100 (CET) Received: by mail-wr1-f67.google.com with SMTP id q6so938107wro.9 for ; Tue, 17 Dec 2019 22:35:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=smartx-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=hNYoJMvtFaTe30Lj80NQU1OWpJO0eFg0MqHdG890w/o=; b=EXNYT872gTn83OIvqPRftm+h+GcQmCv5SynWOnUFShs9qHDI08v9MfzF3PXfuL0sHw SqkD2eA2S8hjbJUeKEWLkm9wcyNTe0HKs0xRg9NtYZtqBONEmPeNIEbeEVzvAMQf2Mk2 WvnkB/ndfuIA1ewah2RnxYHoEcpQ97FeGLI4Io22pV7rkOgbFdgoNHVYoW/zJeVUV/Se wnAa3mUWQonMvipwe5Gw2yrlbRQSJH2C5tNLT5Nw8s/4Ii7+icj0y/7uzxeDscTePfCW MYapx/caBXcmO1U+NH/xGwZnLmF2/IWd5d1QCaTlb4NJ9wK/PzBaVHGfQz6NhfKYw+1t 3LHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=hNYoJMvtFaTe30Lj80NQU1OWpJO0eFg0MqHdG890w/o=; b=XyFa0tH+GRyoUPn19fw6LD12thSmxsIis8iqZtkzMynKoZXOWKsB7Z6mkmbH95Uf4j cFiLId/0O7LanwBMTuWn8vwhA1c87E6Da6sJboU3+qJsdyRds6vhoNt0OgktSvvveexd 4+jV+O3t0d1gA9a24hdAK860TeHye2iDt9dCG3ASTIcVSRd2aExQ4mPk1+Wvi04wMTdb Ag+KTciI44xcdupzn0bp5cGRWqte4kvYxOrAFsZue7Wcdrbg5qiJhi7qNGYoIGlatXH8 qcSNT3rPHLwSFDbIKOti0Jdrle3KaMb0sPO/TNXYVVgmlfBROq3CE8F8ttItU2C33ASY 6KTQ== X-Gm-Message-State: APjAAAXhackwgyXsNBRtvBeRPLfvvVh3tPaqg4HLX4ahUwGBfjfNOIvf TuHlv4df1qd9NFhB+qYbJql56WHb5EAhIsUeQNj43SqdB6O4X68Rc6vFcd9Uq+Fcia/z6DD/1J6 r73P3MB9JfX2v X-Google-Smtp-Source: APXvYqyJfJ3MDMKbrKNWlLK8Wv9u+B6sThC8YQ3Zr1TYZ+jSLjhsdHXPLOsd7FuMxLUyYmMZ3CBwXSvOpCgO9k6oIf4= X-Received: by 2002:a5d:4d8d:: with SMTP id b13mr802185wru.6.1576650911236; Tue, 17 Dec 2019 22:35:11 -0800 (PST) MIME-Version: 1.0 References: <20191202145735.9763-1-fengli@smartx.com> <20191205053833.29068-1-fengli@smartx.com> <20191216050559.GA129906@___> In-Reply-To: <20191216050559.GA129906@___> From: Li Feng Date: Wed, 18 Dec 2019 14:34:59 +0800 Message-ID: To: Tiwei Bie Cc: Maxime Coquelin , Zhihong Wang , dev@dpdk.org, Kyle Zhang Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v3] vhost: add config change slave msg support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Thanks. Tiwei Bie =E4=BA=8E2019=E5=B9=B412=E6=9C=8816=E6=97= =A5=E5=91=A8=E4=B8=80 =E4=B8=8B=E5=8D=881:05=E5=86=99=E9=81=93=EF=BC=9A > > On Thu, Dec 05, 2019 at 01:38:33PM +0800, Li Feng wrote: > > This msg is used to notify qemu that should get the config of backend. > > > > For example, vhost-user-blk uses this msg to notify guest os the > > compacity of backend has changed. > > capacity? > > > > > Signed-off-by: Li Feng > > --- > > v3: > > * Move the declare to rte_vhost.h > > * Add the symbol in rte_vhost_version.map > > > > v2: > > * Fix a little log typo. > > > > lib/librte_vhost/rte_vhost.h | 12 ++++++++++++ > > lib/librte_vhost/rte_vhost_version.map | 1 + > > lib/librte_vhost/vhost_user.c | 31 ++++++++++++++++++++++++++= +++++ > > lib/librte_vhost/vhost_user.h | 1 + > > 4 files changed, 45 insertions(+) > > > > diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.= h > > index 7b5dc87c2..fc28da264 100644 > > --- a/lib/librte_vhost/rte_vhost.h > > +++ b/lib/librte_vhost/rte_vhost.h > > @@ -977,6 +977,18 @@ __rte_experimental > > int > > rte_vhost_get_vdpa_device_id(int vid); > > > > +/** > > + * Notify the guest that should get config from backend. > > + * > > + * @param vid > > + * vhost device ID > > + * @return > > + * 0 on success, < 0 on failure > > + */ > > +__rte_experimental > > +int > > +rte_vhost_user_slave_config_change(int vid); > > Normally the prefix in vhost API is rte_vhost_ > not rte_vhost_user_ > > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/librte_vhost/rte_vhost_version.map b/lib/librte_vhost/= rte_vhost_version.map > > index c512377fe..acf013d6d 100644 > > --- a/lib/librte_vhost/rte_vhost_version.map > > +++ b/lib/librte_vhost/rte_vhost_version.map > > @@ -65,4 +65,5 @@ EXPERIMENTAL { > > rte_vhost_clr_inflight_desc_packed; > > rte_vhost_get_vhost_ring_inflight; > > rte_vhost_get_vring_base_from_inflight; > > + rte_vhost_user_slave_config_change; > > }; > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_use= r.c > > index 0cfb8b792..10f2e47d5 100644 > > --- a/lib/librte_vhost/vhost_user.c > > +++ b/lib/librte_vhost/vhost_user.c > > @@ -2840,6 +2840,37 @@ vhost_user_iotlb_miss(struct virtio_net *dev, ui= nt64_t iova, uint8_t perm) > > return 0; > > } > > > > +static int > > +vhost_user_slave_config_change(struct virtio_net *dev) > > +{ > > + int ret; > > + struct VhostUserMsg msg =3D { > > + .request.slave =3D VHOST_USER_SLAVE_CONFIG_CHANGE_MSG, > > + .flags =3D VHOST_USER_VERSION, > > Will it be better to also set VHOST_USER_NEED_REPLY? This feature could not be supported because the vhost thread will block at the read slave fd, but qemu have sent a get_config message, and wait for the vhost thread response, the stack like this: #2 0x0000555555b435d2 in tcp_chr_recv (chr=3Dchr@entry=3D0x555556b12c80, buf=3Dbuf@entry=3D0x7fffffffd7c0 "\030", len=3Dlen@entry=3D12) at chardev/char-socket.c:332 #3 0x0000555555b44611 in tcp_chr_sync_read (chr=3D0x555556b12c80, buf=3D0x7fffffffd7c0 "\030", len=3D12) at chardev/char-socket.c:543 #4 0x0000555555b3f5e1 in qemu_chr_fe_read_all (be=3Dbe@entry=3D0x555557e82310, buf=3Dbuf@entry=3D0x7fffffffd7c0 "\030", len=3Dlen@entry=3D12) at chardev/char-fe.c:72 #5 0x0000555555884b7b in vhost_user_read_header (dev=3D0x555557e82310, msg=3D0x7fffffffd7c0) at /usr/src/debug/qemu-4.1.0/hw/virtio/vhost-user.c:236 #6 vhost_user_read (msg=3Dmsg@entry=3D0x7fffffffd7c0, dev=3D0x555557e82398= ) at /usr/src/debug/qemu-4.1.0/hw/virtio/vhost-user.c:261 #7 0x00005555558850ff in vhost_user_get_config (dev=3D0x555557e82398, config=3D0x7fffffffda80 "\340(6WUU", config_len=3D60) at /usr/src/debug/qemu-4.1.0/hw/virtio/vhost-user.c:1636 #8 0x0000555555842880 in vhost_user_blk_handle_config_change (dev=3D0x555557e82398) at /usr/src/debug/qemu-4.1.0/hw/block/vhost-user-blk.c:85 #9 0x0000555555886df1 in vhost_user_slave_handle_config_change (dev=3D0x555557e82398) at /usr/src/debug/qemu-4.1.0/hw/virtio/vhost-user.c:912 #10 slave_read (opaque=3D0x555557e82398) at /usr/src/debug/qemu-4.1.0/hw/virtio/vhost-user.c:1050 > > > + .size =3D 0, > > + }; > > + > > + ret =3D send_vhost_message(dev->slave_req_fd, &msg); > > + if (ret < 0) { > > + RTE_LOG(ERR, VHOST_CONFIG, > > + "Failed to send config change (%d)\n", > > + ret); > > Looks better to indent the code by 3 tabs. > > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +int > > +rte_vhost_user_slave_config_change(int vid) > > +{ > > + struct virtio_net *dev; > > + dev =3D get_device(vid); > > + if (!dev) > > + return -ENODEV; > > + return vhost_user_slave_config_change(dev); > > +} > > I'm wondering will it be better to provide a generic API > to allow external backends to send any slave messages? I have thought about this before, and I don't choose external backend becau= se: 1. Sending slave message is more like an API, and external backend acts like a hooker that receives messages from master; If adding a new slave message type, we should also need to add type definition in the dpdk librte_vhost header. 2. There is only three slave message type; > > > + > > static int vhost_user_slave_set_vring_host_notifier(struct virtio_net = *dev, > > int index, int fd, > > uint64_t offset, > > diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_use= r.h > > index 6563f7315..86c364a93 100644 > > --- a/lib/librte_vhost/vhost_user.h > > +++ b/lib/librte_vhost/vhost_user.h > > @@ -62,6 +62,7 @@ typedef enum VhostUserRequest { > > typedef enum VhostUserSlaveRequest { > > VHOST_USER_SLAVE_NONE =3D 0, > > VHOST_USER_SLAVE_IOTLB_MSG =3D 1, > > + VHOST_USER_SLAVE_CONFIG_CHANGE_MSG =3D 2, > > VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG =3D 3, > > VHOST_USER_SLAVE_MAX > > } VhostUserSlaveRequest; > > -- > > 2.11.0 > > > > > > -- > > The SmartX email address is only for business purpose. Any sent message > > that is not related to the business is not authorized or permitted by > > SmartX. > > =E6=9C=AC=E9=82=AE=E7=AE=B1=E4=B8=BA=E5=8C=97=E4=BA=AC=E5=BF=97=E5=87= =8C=E6=B5=B7=E7=BA=B3=E7=A7=91=E6=8A=80=E6=9C=89=E9=99=90=E5=85=AC=E5=8F=B8= =EF=BC=88SmartX=EF=BC=89=E5=B7=A5=E4=BD=9C=E9=82=AE=E7=AE=B1. =E5=A6=82=E6= =9C=AC=E9=82=AE=E7=AE=B1=E5=8F=91=E5=87=BA=E7=9A=84=E9=82=AE=E4=BB=B6=E4=B8= =8E=E5=B7=A5=E4=BD=9C=E6=97=A0=E5=85=B3,=E8=AF=A5=E9=82=AE=E4=BB=B6=E6=9C= =AA=E5=BE=97=E5=88=B0=E6=9C=AC=E5=85=AC=E5=8F=B8=E4=BB=BB=E4=BD=95=E7=9A=84= =E6=98=8E=E7=A4=BA=E6=88=96=E9=BB=98=E7=A4=BA=E7=9A=84=E6=8E=88=E6=9D=83. > > > > --=20 The SmartX email address is only for business purpose. Any sent message=20 that is not related to the business is not authorized or permitted by=20 SmartX. =E6=9C=AC=E9=82=AE=E7=AE=B1=E4=B8=BA=E5=8C=97=E4=BA=AC=E5=BF=97=E5=87=8C=E6= =B5=B7=E7=BA=B3=E7=A7=91=E6=8A=80=E6=9C=89=E9=99=90=E5=85=AC=E5=8F=B8=EF=BC= =88SmartX=EF=BC=89=E5=B7=A5=E4=BD=9C=E9=82=AE=E7=AE=B1. =E5=A6=82=E6=9C=AC= =E9=82=AE=E7=AE=B1=E5=8F=91=E5=87=BA=E7=9A=84=E9=82=AE=E4=BB=B6=E4=B8=8E=E5= =B7=A5=E4=BD=9C=E6=97=A0=E5=85=B3,=E8=AF=A5=E9=82=AE=E4=BB=B6=E6=9C=AA=E5= =BE=97=E5=88=B0=E6=9C=AC=E5=85=AC=E5=8F=B8=E4=BB=BB=E4=BD=95=E7=9A=84=E6=98= =8E=E7=A4=BA=E6=88=96=E9=BB=98=E7=A4=BA=E7=9A=84=E6=8E=88=E6=9D=83.