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 109E3A04F0; Tue, 10 Dec 2019 09:00:42 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DFC9F1B13C; Tue, 10 Dec 2019 09:00:41 +0100 (CET) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id E1E5337A2 for ; Tue, 10 Dec 2019 09:00:40 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id 3BE5E7DB7; Tue, 10 Dec 2019 03:00:38 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Tue, 10 Dec 2019 03:00:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=Qj4lz87u2j9JiG5ivdtkgGSAyyO/BL9siSmX62PgZPQ=; b=TUR3de17NdSK KbqSjujstuGPq61es+KSAi5Eizf/TpUZHBlP015+qM+cixmVKyuS5L4bPXkNcesN tpP6KmHsW5sydWtpqJLOlHO3HqEeBIQ2FB9uPgHPBc8owm/QjwmGct1NKjRvUPn4 thV3LXTznnlsehQeqtWGNcR7Qf2omZE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=Qj4lz87u2j9JiG5ivdtkgGSAyyO/BL9siSmX62PgZ PQ=; b=goq1BgjI9ZKK5Bu/I/MrI+N6NVouQnG+V5i1/fx69X4ZPYX1oxlLoZR8o cHxrufbZGRsjR3/wQ/Ngme1t+EbT/v0K2s226nbhhM6hS4GwzjpIMt/r0rbtHKb9 DiXrYNlInOxJyvOaxQ8c6PtluWsaVTzqI0/X+6OMdj66I97uFla2Ou/MzSxwFpem SA0I6V2+zOxw6OeYhYo0+3tyLRklDbJZrtC6w+gjRcMRistRoO/NNZ55k9AA/rz9 bqnJcbkQycf6tABbV1PalUcfcghQYpnkiO2Ay70QZXuIMVkAtZF/BwUj62KPRrPg mDOm8qW85MWqi3P9U7RP4vAbCEe5w== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrudelvddgudduvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthhqredttddtjeenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ffohhmrghinhepghhithhhuhgsrdgtohhmnecukfhppeejjedrudefgedrvddtfedrudek geenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnh gvthenucevlhhushhtvghrufhiiigvpedt X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 380E430600A8; Tue, 10 Dec 2019 03:00:36 -0500 (EST) From: Thomas Monjalon To: Tiwei Bie Cc: Andrew Rybchenko , Ori Kam , Matan Azrad , "Liang, Cunming" , "Wang, Xiao W" , "maxime.coquelin@redhat.com" , "Wang, Zhihong" , "Yigit, Ferruh" , Shahaf Shuler , "dev@dpdk.org" , Slava Ovsiienko , Asaf Penso , Olga Shern Date: Tue, 10 Dec 2019 09:00:33 +0100 Message-ID: <5690229.RHSIJ4Jb0I@xps> In-Reply-To: <20191210024111.GA104968@___> References: <2bf414d6-f1dc-24c7-a17d-743fc4e8474d@solarflare.com> <20191210024111.GA104968@___> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] discussion: creating a new class for vdpa driversxiao.w.wang@intel.com 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" 10/12/2019 03:41, Tiwei Bie: > On Mon, Dec 09, 2019 at 02:22:27PM +0300, Andrew Rybchenko wrote: > > On 12/9/19 1:41 PM, Ori Kam wrote: > > > From: Andrew Rybchenko > > >> On 12/8/19 10:06 AM, Matan Azrad wrote: > > >>> From: Andrew Rybchenko > > >>>> On 12/6/19 8:32 AM, Liang, Cunming wrote: > > >>>>> From: Bie, Tiwei > > >>>>>> On Thu, Dec 05, 2019 at 01:26:36PM +0000, Matan Azrad wrote: > > >>>>>>> Hi all > > >>>>>>> > > >>>>>>> As described in RFC =E2=80=9C[RFC] net: new vdpa PMD for Mellan= ox devices=E2=80=9D, > > >>>>>>> a new vdpa drivers is going to be added for Mellanox devices = =E2=80=93 > > >>>>>>> mlx5_vdpa > > >>>>>>> > > >>>>>>> The only vdpa driver now is the IFC driver that is located in n= et > > >> directory. > > >>>>>>> > > >>>>>>> The IFC driver and the new mlx5_vdpa driver provide the vdpa ops > > >> and > > >>>>>>> not the eth_dev ops. > > >>>>>>> > > >>>>>>> All the others drivers in net provide the eth-dev ops. > > >>>>>>> > > >>>>>>> I suggest to create a new class for vdpa drivers, to move IFC to > > >>>>>>> this class and to add the mlx5_vdpa to this class too. > > >>>>>>> > > >>>>>>> Later, all the new drivers that implements the vdpa ops will be > > >>>>>>> added to the vdpa class. > > >>>>>> > > >>>>>> +1. Sounds like a good idea to me. > > >>>>> +1 > > >>>> > > >>>> vDPA drivers are vendor-specific and expected to talk to vendor NI= C. I.e. > > >>>> there are significant chances to share code with network drivers (= e.g. > > >> base > > >>>> driver). Should base driver be moved to drivers/common in this cas= e or is > > >> it > > >>>> still allows to have vdpa driver in drivers/net together with ethd= ev driver? > > >>> > > >>> Yes, I think this should be the method, shared code should be moved= to > > >> the drivers/common directory. > > >>> I think there is a precedence with shared code in common which shar= es a > > >> vendor specific code between crypto and net. > > >> > > >> I see motivation behind driver/vdpa. However, vdpa and net > > >> drivers tightly related and I would prefer to avoid extra > > >> complexity here. Right now simplicity over-weights for me. > > >> No strong opinion on the topic, but it definitely requires > > >> better and more clear motivation why a new class should be > > >> introduced and existing drivers restructured. > > >> > > >=20 > > > Why do you think there is extra complexity? > >=20 > > Even grep becomes a bit more complicated J > >=20 > > > I think from design correctness it is more correct to create a dedica= ted class for the following reasons: > > > 1. All of the classes implements a different set of ops. For example = the cryptodev has a defined set of ops, same goes for the compress driver a= nd the ethdev driver. Each ones of them=20 > > > has different ops. Going by this definition since VDPA has a differen= t set of ops, it makes sense that it will be in a different class. > > >=20 > > > 2. even that both drivers (eth/vdpa) handle traffic from the nic most= of the code is different (the difference is also dependent on the manufact= ure) > > > So even the shared code base is not large and can be shared using the= common directory. For example ifc doesn't have any shared code. > > >=20 > > > What do you think? > >=20 > > The true reason is: if the difference in ops implemented > > is a key difference which should enforce location in > > different directories. Or underlying device class is a key. > > Roughly: > > - net driver is a control+data path > > - vdpa driver is a control path only > > My fear is that control path will grow more and more > > (Rx mode, RSS, filters and so on) >=20 > I think this is a reasonable concern. >=20 > One thing needs to be clarified is that, the control > path (ops) in vdpa driver is something very different > with the control path in net driver. vdpa is very > generic (or more precisely vhost-related), instead > of network related: >=20 > https://github.com/DPDK/dpdk/blob/aef1d0733179/lib/librte_vhost/rte_vdpa.= h#L40-L78 >=20 > It's built on top of vhost-user protocol, manipulates > virtqueues, virtio/vhost features, memory table, ... >=20 > Technically, it's possible to have blk, crypto, ... > vdpa devices as well (we already have vhost-user-blk, > vhost-user-crypto, ...). >=20 > But network specific features will come eventually, > e.g. RSS. One possible way to solve it is to define a > generic event callback in vdpa ops, and vdpa driver can > request the corresponding info from vhost based on the > event received. >=20 > Another thing needs to be clarified is that, the control > path supposed to be used by DPDK apps directly in vdpa > should always be generic, it should just be something like: >=20 > int rte_vdpa_find_device_id(struct rte_vdpa_dev_addr *addr); > int rte_vhost_driver_attach_vdpa_device(const char *path, int did); > int rte_vhost_driver_detach_vdpa_device(const char *path); > ... >=20 > That is to say, users just need to bind the vdpa device > to the vhost connection. The control path ops in vdpa is > supposed to be called by vhost-library transparently > based on the events on the vhost-user connection, i.e. > the vdpa device will be configured (including RSS) by > the guest driver in QEMU "directly" via the vhost-user > protocol instead of the DPDK app in the host. Tiwei, in order to be clear, You think vDPA drivers should be in drivers/vdpa directory?