All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matan Azrad <matan@mellanox.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
	Tiwei Bie <tiwei.bie@intel.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>
Cc: Zhihong Wang <zhihong.wang@intel.com>,
	Xiao Wang <xiao.w.wang@intel.com>,
	 Ferruh Yigit <ferruh.yigit@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v1 2/3] doc: add vDPA feature table
Date: Wed, 8 Jan 2020 17:01:35 +0000	[thread overview]
Message-ID: <AM0PR0502MB4019538E5D1FFD309BF6F3E2D23E0@AM0PR0502MB4019.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <25a39d8f-f790-e2e3-8566-dc8d51e5725c@solarflare.com>

Hi Andrew

From: Andrew Rybchenko
> Sent: Wednesday, January 8, 2020 3:11 PM
> To: Matan Azrad <matan@mellanox.com>; Tiwei Bie <tiwei.bie@intel.com>;
> Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Zhihong Wang <zhihong.wang@intel.com>; Xiao Wang
> <xiao.w.wang@intel.com>; Ferruh Yigit <ferruh.yigit@intel.com>;
> dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v1 2/3] doc: add vDPA feature table
> 
> On 1/8/20 1:42 PM, Matan Azrad wrote:
> > Hi all
> >
> > Thanks very much for the review.
> > Please see below.
> >
> > From: Andrew Rybchenko
> >> On 1/8/20 8:28 AM, Tiwei Bie wrote:
> >>> On Tue, Jan 07, 2020 at 06:39:36PM +0100, Maxime Coquelin wrote:
> >>>> On 12/25/19 4:19 PM, Matan Azrad wrote:
> >>>>> Add vDPA devices features table and explanation.
> >>>>>
> >>>>> Any vDPA driver can add its own supported features by ading a new
> >>>>> ini file to the features directory in doc/guides/vdpadevs/features.
> >>>>>
> >>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
> >>>>> ---
> >>>>>  doc/guides/conf.py                        |  5 +++
> >>>>>  doc/guides/vdpadevs/features/default.ini  | 55
> >>>>> ++++++++++++++++++++++++++
> >> doc/guides/vdpadevs/features_overview.rst | 65
> >> +++++++++++++++++++++++++++++++
> >>>>>  doc/guides/vdpadevs/index.rst             |  1 +
> >>>>>  4 files changed, 126 insertions(+)  create mode 100644
> >>>>> doc/guides/vdpadevs/features/default.ini
> >>>>>  create mode 100644 doc/guides/vdpadevs/features_overview.rst
> >>>>>
> >>>>> diff --git a/doc/guides/conf.py b/doc/guides/conf.py index
> >>>>> 0892c06..c368fa5 100644
> >>>>> --- a/doc/guides/conf.py
> >>>>> +++ b/doc/guides/conf.py
> >>>>> @@ -401,6 +401,11 @@ def setup(app):
> >>>>>                              'Features',
> >>>>>                              'Features availability in compression drivers',
> >>>>>                              'Feature')
> >>>>> +    table_file = dirname(__file__) +
> >> '/vdpadevs/overview_feature_table.txt'
> >>>>> +    generate_overview_table(table_file, 1,
> >>>>> +                            'Features',
> >>>>> +                            'Features availability in vDPA drivers',
> >>>>> +                            'Feature')
> >>>>>
> >>>>>      if LooseVersion(sphinx_version) < LooseVersion('1.3.1'):
> >>>>>          print('Upgrade sphinx to version >= 1.3.1 for '
> >>>>> diff --git a/doc/guides/vdpadevs/features/default.ini
> >>>>> b/doc/guides/vdpadevs/features/default.ini
> >>>>> new file mode 100644
> >>>>> index 0000000..a3e0bc7
> >>>>> --- /dev/null
> >>>>> +++ b/doc/guides/vdpadevs/features/default.ini
> >>>>> @@ -0,0 +1,55 @@
> >>>>> +;
> >>>>> +; Features of a default vDPA driver.
> >>>>> +;
> >>>>> +; This file defines the features that are valid for inclusion in
> >>>>> +; the other driver files and also the order that they appear in ;
> >>>>> +the features table in the documentation. The feature description
> >>>>> +; string should not exceed feature_str_len defined in conf.py.
> >>>>> +;
> >>>> I think some entries below could be removed for vDPA.
> >>> +1
> >>>
> >>>>> +[Features]
> >>>>> +csum                 =
> >>>>> +guest csum           =
> >>>>> +mac                  =
> >>>>> +gso                  =
> >>>>> +guest tso4           =
> >>>>> +guest tso6           =
> >>>>> +ecn                  =
> >>>>> +ufo                  =
> >>>>> +host tso4            =
> >>>>> +host tso6            =
> >>>>> +mrg rxbuf            =
> >>>>> +ctrl vq              =
> >>>>> +ctrl rx              =
> >>>>> +any layout           =
> >>>>> +guest announce       =
> >>>>> +mq                   =
> >>>>> +version 1            =
> >>>>> +log all              =
> >>>>> +protocol features    =
> >>> We may not need to list this. The proto * would imply it.
> >
> > So can you explain why this flag is exposed by the vhost features?
> >
> >>>>> +indirect desc        =
> >>>>> +event idx            =
> >>>>> +mtu                  =
> >>>>> +in_order             =
> >>>>> +IOMMU platform       =
> >>>>> +packed               =
> >>>>> +proto mq             =
> >>>>> +proto log shmfd      =
> >>>>> +proto rarp           =
> >>>>> +proto reply ack      =
> >>>>> +proto slave req      =
> >>> Ditto. This feature is to be used by other features.
> >>> Features like host notifier would imply it.
> >
> > So can you explain why this flag is exposed by the vhost protocol features?
> >
> >>>>> +proto crypto session =
> >>> We don't need to list this before we officially support the crypto
> >>> vDPA device.
> >>>
> >
> > Ok, will remove.
> >
> >>>>> +proto host notifier  =
> >>>>> +proto pagefault      =
> >>>>> +Multiprocess aware   =
> >>> There is no support for this in library currently.
> >>> To support it, we need to sync vhost fds and messages among
> processes.
> >>>
> >
> > Ok, will remove.
> >
> >>>>> +BSD nic_uio          =
> >>>>> +Linux UIO            =
> >>>> E.g. UIO, which cannot be used since vDPA requires an IOMMU.
> >
> > Ok, will remove.
> >
> >>>>> +Linux VFIO           =
> >>>>> +Other kdrv           =
> >>>>> +ARMv7                =
> >>>>> +ARMv8                =
> >>>>> +Power8               =
> >>>>> +x86-32               =
> >>>>> +x86-64               =
> >>>>> +Usage doc            =
> >>>>> +Design doc           =
> >>>>> +Perf doc             =
> >>>>> \ No newline at end of file
> >>>>> diff --git a/doc/guides/vdpadevs/features_overview.rst
> >>>>> b/doc/guides/vdpadevs/features_overview.rst
> >>>>> new file mode 100644
> >>>>> index 0000000..c7745b7
> >>>>> --- /dev/null
> >>>>> +++ b/doc/guides/vdpadevs/features_overview.rst
> >>>>> @@ -0,0 +1,65 @@
> >>>>> +..  SPDX-License-Identifier: BSD-3-Clause
> >>>>> +    Copyright 2019 Mellanox Technologies, Ltd
> >>>>> +
> >>>>> +Overview of vDPA drivers features
> >>>>> +=================================
> >>>>> +
> >>>>> +This section explains the supported features that are listed in
> >>>>> +the table
> >> below.
> >>>>> +
> >>>>> +  * csum - Device can handle packets with partial checksum.
> >>>>> +  * guest csum - Guest can handle packets with partial checksum.
> >>>>> +  * mac - Device has given MAC address.
> >>>>> +  * gso - Device can handle packets with any GSO type.
> >>>>> +  * guest tso4 - Guest can receive TSOv4.
> >>>>> +  * guest tso6 - Guest can receive TSOv6.
> >>>>> +  * ecn - Device can receive TSO with ECN.
> >>>>> +  * ufo - Device can receive UFO.
> >>>>> +  * host tso4 - Device can receive TSOv4.
> >>>>> +  * host tso6 - Device can receive TSOv6.
> >>>>> +  * mrg rxbuf - Guest can merge receive buffers.
> >>>>> +  * ctrl vq - Control channel is available.
> >>>>> +  * ctrl rx - Control channel RX mode support.
> >>>>> +  * any layout - Device can handle any descriptor layout.
> >>>>> +  * guest announce - Guest can send gratuitous packets.
> >>>>> +  * mq - Device supports Receive Flow Steering.
> >>>>> +  * version 1 - v1.0 compliant.
> >>>>> +  * log all - Device can log all write descriptors (live migration).
> >>>>> +  * protocol features - Protocol features negotiation support.
> >>>>> +  * indirect desc - Indirect buffer descriptors support.
> >>>>> +  * event idx - Support for avail_idx and used_idx fields.
> >>>>> +  * mtu - Host can advise the guest with its maximum supported
> MTU.
> >>>>> +  * in_order - Device can use descriptors in ring order.
> >>>>> +  * IOMMU platform - Device support IOMMU addresses.
> >>>>> +  * packed - Device support packed virtio queues.
> >>>>> +  * proto mq - Support the number of queues query.
> >>>>> +  * proto log shmfd - Guest support setting log base.
> >>>>> +  * proto rarp - Host can broadcast a fake RARP after live migration.
> >>>>> +  * proto reply ack - Host support requested operation status ack.
> >>>>> +  * proto slave req - Allow the slave to make requests to the master.
> >>>>> +  * proto crypto session - Support crypto session creation.
> >>>>> +  * proto host notifier - Host can register memory region based
> >>>>> + host
> >> notifiers.
> >>>>> +  * proto pagefault - Slave expose page-fault FD for migration
> process.
> >>>>> +  * Multiprocess aware - Driver can be used for primary-secondary
> >> process model.
> >>>>> +  * BSD nic_uio - BSD ``nic_uio`` module supported.
> >>>>> +  * Linux UIO - Works with ``igb_uio`` kernel module.
> >>>>> +  * Linux VFIO - Works with ``vfio-pci`` kernel module.
> >>>>> +  * Other kdrv - Kernel module other than above ones supported.
> >>>>> +  * ARMv7 - Support armv7 architecture.
> >>>>> +  * ARMv8 - Support armv8a (64bit) architecture.
> >>>>> +  * Power8 - Support PowerPC architecture.
> >>>>> +  * x86-32 - Support 32bits x86 architecture.
> >>>>> +  * x86-64 - Support 64bits x86 architecture.
> >>>>> +  * Usage doc - Documentation describes usage, In
> >> ``doc/guides/vdpadevs/``.
> >>>>> +  * Design doc - Documentation describes design. In
> >> ``doc/guides/vdpadevs/``.
> >>>>> +  * Perf doc - Documentation describes performance values, In
> >> ``doc/perf/``.
> >>
> >> Are you going to put Y mark for all these features in v20.02 release cycle?
> >
> > No.
> >
> >> Basically the question is: is it OK to have features that no driver supports?
> >> "Dead" features do not look nice.
> >> I would say yes for architecture support since it is better to list
> >> all architectures supported by DPDK and make it clear which
> >> architectures are supported by particular vDPA driver.
> >> May be it is OK for features which are directly correspond to
> >> virtio/vhost features (of course, it is very useful to know spec
> compliance).
> >> In this case I think it would be very helpful to add references to
> >> spec sections.
> >
> > These features are supported in vhost library. So I think it may sense to add
> them.
> > I think, especially in vDPA case, It is good for the vhost users to see what is
> supported and what is not supported by the vDPA driver.
> >
> > Which spec are you talking about?
> 
> Vhost user protocol specification and virtio specification (since some features
> directly correspond to virtio features).
> 
> [1]
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.
> oasis-open.org%2Fvirtio%2Fvirtio%2Fv1.1%2Fcsprd01%2Fvirtio-v1.1-
> csprd01.html&amp;data=02%7C01%7Cmatan%40mellanox.com%7C85f5ae0b
> 4d154e5e1d1a08d7943c4b1f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%
> 7C0%7C637140859000068810&amp;sdata=f1sw2FwlsckXaCsLeZeGmcHieUxsE
> uQ8WfTVnHjp970%3D&amp;reserved=0
> [2]
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fqem
> u.weilnetz.de%2Fdoc%2Finterop%2Fvhost-
> user.html&amp;data=02%7C01%7Cmatan%40mellanox.com%7C85f5ae0b4d1
> 54e5e1d1a08d7943c4b1f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0
> %7C637140859000068810&amp;sdata=twLCnQOKaWXgwXJ4PPUTcn4gF4S%2
> FptyeVL9kxpNYuWk%3D&amp;reserved=0
> 

Thanks, will add this useful links.

> >> Also I like doc/guides/nics/features.rst and would like to know why
> >> the practice is not used here. I'm talking about features description
> format.
> >
> > Yes, but except nic class, no class uses it.
> 
> IMO it is one of best practices in DPDK which should be used by other classes
> as well.
> > Maybe it is because there are not a lot of different ways to add the
> configuration\capability.
> > Here, for example, most of them are just in feature bitmap.
> 
> More formal description simplifies search and helps driver developers and
> maintainers a lot.


I can add more description how to expose the capability for the features.
All of them are really the same, so IMO we should not rewrite the same again and again. 

What do you think?

> >> [snip]
> >


  reply	other threads:[~2020-01-08 17:01 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-25 15:19 [dpdk-dev] [PATCH v1 0/3] Introduce new class for vDPA device drivers Matan Azrad
2019-12-25 15:19 ` [dpdk-dev] [PATCH v1 1/3] drivers: introduce vDPA class Matan Azrad
2020-01-07 17:32   ` Maxime Coquelin
2020-01-08 21:28     ` Thomas Monjalon
2020-01-09  8:00       ` Maxime Coquelin
2019-12-25 15:19 ` [dpdk-dev] [PATCH v1 2/3] doc: add vDPA feature table Matan Azrad
2020-01-07 17:39   ` Maxime Coquelin
2020-01-08  5:28     ` Tiwei Bie
2020-01-08  7:20       ` Andrew Rybchenko
2020-01-08 10:42         ` Matan Azrad
2020-01-08 13:11           ` Andrew Rybchenko
2020-01-08 17:01             ` Matan Azrad [this message]
2020-01-09  2:15           ` Tiwei Bie
2020-01-09  8:08             ` Matan Azrad
2019-12-25 15:19 ` [dpdk-dev] [PATCH v1 3/3] drivers: move ifc driver to the vDPA class Matan Azrad
2020-01-07 18:17   ` Maxime Coquelin
2020-01-07  7:57 ` [dpdk-dev] [PATCH v1 0/3] Introduce new class for vDPA device drivers Matan Azrad
2020-01-08  5:44   ` Xu, Rosen
2020-01-08 10:45     ` Matan Azrad
2020-01-08 12:39       ` Xu, Rosen
2020-01-08 12:58         ` Thomas Monjalon
2020-01-09  2:27           ` Xu, Rosen
2020-01-09  8:41             ` Thomas Monjalon
2020-01-09  9:23               ` Maxime Coquelin
2020-01-09  9:49                 ` Xu, Rosen
2020-01-09 10:42                   ` Maxime Coquelin
2020-01-10  2:40                     ` Xu, Rosen
2020-01-09 10:42                   ` Maxime Coquelin
2020-01-09 10:53               ` Xu, Rosen
2020-01-09 11:34                 ` Matan Azrad
2020-01-10  2:38                   ` Xu, Rosen
2020-01-10  9:21                     ` Thomas Monjalon
2020-01-10 14:18                       ` Xu, Rosen
2020-01-10 16:27                         ` Thomas Monjalon
2020-01-09 11:00 ` [dpdk-dev] [PATCH v2 " Matan Azrad
2020-01-09 11:00   ` [dpdk-dev] [PATCH v2 1/3] drivers: introduce vDPA class Matan Azrad
2020-01-09 11:00   ` [dpdk-dev] [PATCH v2 2/3] doc: add vDPA feature table Matan Azrad
2020-01-10 18:26     ` Thomas Monjalon
2020-01-13 22:40     ` Thomas Monjalon
2020-01-09 11:00   ` [dpdk-dev] [PATCH v2 3/3] drivers: move ifc driver to the vDPA class Matan Azrad
2020-01-09 17:25     ` Matan Azrad
2020-01-10  1:55       ` Wang, Haiyue
2020-01-10  9:07         ` Matan Azrad
2020-01-10  9:13           ` Thomas Monjalon
2020-01-10 12:31             ` Wang, Haiyue
2020-01-10 12:34               ` Maxime Coquelin
2020-01-10 12:59                 ` Thomas Monjalon
2020-01-10 19:17                   ` Kevin Traynor
2020-01-13 22:57     ` Thomas Monjalon
2020-01-13 23:08   ` [dpdk-dev] [PATCH v2 0/3] Introduce new class for vDPA device drivers Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0502MB4019538E5D1FFD309BF6F3E2D23E0@AM0PR0502MB4019.eurprd05.prod.outlook.com \
    --to=matan@mellanox.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=thomas@monjalon.net \
    --cc=tiwei.bie@intel.com \
    --cc=xiao.w.wang@intel.com \
    --cc=zhihong.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.