From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id EA6071B813 for ; Fri, 9 Feb 2018 17:21:38 +0100 (CET) Received: by mail-wm0-f44.google.com with SMTP id v71so17299785wmv.2 for ; Fri, 09 Feb 2018 08:21:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=iVkjIA2TApF1XHgNQZMoIVUHARS909UKkt0LVKPHr0k=; b=HExHvUY5L1vLB8IvAKg1CYi5YtnNiTUH8QEvhPvrUVZrEYbhhGuSuODz0YxYVEIWsA g6hd/94YmoE50t4tKlSc3JkTR+ElexNmsdZdoTykTm0HgbJe4Y1IDNfzyiBvkvaNYJLw H9kGsk90G8ItuaUm9UQyCb7yMXWWY7vb5NRD9p4rTLzXanDEamTUGfjKAyidzhKyZGU4 qsPy4RmkJayOZbtFS6Q9k1xK4bmWZhJwAH4tM5jOUhzKHBYLDeDEgsLTe3A1sHmPLnIn G6n00rI1rDs8PpAhKynnwEA7TxDx/d80C3k/gq+V4iHup4WHwu+FdV4UQTrdIFkpRcAS PH6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=iVkjIA2TApF1XHgNQZMoIVUHARS909UKkt0LVKPHr0k=; b=DpMky4RAvUYTmhx6sN4vc9kTsE4kjFesn4XBhVAu3Y8u41mt7js/osXuWzWt3H0oOY lIVL0Sfq6VGAG3TWrqDQefA29y8RABDFDvlFHq+lBe3RqKauYud6aLtYFj75DrnzCB7k XahPIwHlqOaNIJgKLWuJeXjwlQ0vmUIDuK0QsMRVhMfkBuUUkTtlGIi3hSod2bjg/GTk i5upW86ZjfMXHbdaCy85pd/eJjeMgaQz3Bi1k5/IxxXst0W9JEzeoMidxrXQwRKk2656 DV73eGGXtjqVJ+wBh24ldlit49ec1qp8lVbN6fYAlCo0k1gmYArQh2U82NlzTLlINtDH R6IA== X-Gm-Message-State: APf1xPD7j5cTirlpqoGEEmA9hJ+HiCXnZMczhxMcVhNqhXxNPRjw+VBA EoIOrCO83wSu9nTh52RAPyP3dGZ/ X-Google-Smtp-Source: AH8x227n7XqLDb+KC/tYJ9hueVhGcgE1DY+FhLDx6aB2t9LraOMI5SiciI6diGNlcHzxAsOg5YdC6A== X-Received: by 10.28.234.200 with SMTP id g69mr2319629wmi.137.1518193298257; Fri, 09 Feb 2018 08:21:38 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id 63sm3597708wms.46.2018.02.09.08.21.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 Feb 2018 08:21:37 -0800 (PST) Date: Fri, 9 Feb 2018 17:21:24 +0100 From: Adrien Mazarguil To: Ophir Munk Cc: dev@dpdk.org, Moti Haimovsky , Thomas Monjalon , Olga Shern , Matan Azrad Message-ID: <20180209162124.GD4256@6wind.com> References: <1518072954-19082-1-git-send-email-ophirmu@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1518072954-19082-1-git-send-email-ophirmu@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v1] doc: update mlx4 flow limitations 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: , X-List-Received-Date: Fri, 09 Feb 2018 16:21:39 -0000 Hi Ophir, On Thu, Feb 08, 2018 at 06:55:54AM +0000, Ophir Munk wrote: > From: Moti Haimovsky Relatively minor, patch author differs from the only sign off below, I don't think it's on purpose. > This patch updates mlx4 documentation with flow > configuration limitations imposed by NIC hardware and > PMD implementation > > Signed-off-by: Ophir Munk Another nit, don't hesitate to spread commit logs to their maximum width of 75 chars. We're not writing poetry :) More comments below. > --- > doc/guides/nics/mlx4.rst | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 77 insertions(+) > > diff --git a/doc/guides/nics/mlx4.rst b/doc/guides/nics/mlx4.rst > index 98b9716..b81a875 100644 > --- a/doc/guides/nics/mlx4.rst > +++ b/doc/guides/nics/mlx4.rst > @@ -515,3 +515,80 @@ devices managed by librte_pmd_mlx4. > Port 3 Link Up - speed 40000 Mbps - full-duplex > Done > testpmd> > + > +Limitations > +----------- > + > +Flow rules > +~~~~~~~~~~ While documenting flow rule limitations is a good idea, I think this approach is not ideal. rte_flow being unbounded, no PMD can support all possible combinations. It's much easier and useful to list what is implemented or more importantly, *tested* and known to work. Ideally it should be written in a format that can be reused by other PMDs for consistency and divided in sections sorted by order of usefulness, something like: 1. Overview of supported combinations of attributes, patterns and actions. 2. Detailed description of each supported pattern (item combinations). 3. Detailed description of all supported action combinations. 4. Description of each supported pattern item and its quirks. 5. Description of each supported action and its quirks. > + > +L2 (eth) > +^^^^^^^^ You need to make clear you're documenting RTE_FLOW_ITEM_TYPE_ETH when used at the beginning of a flow rule, for instance it doesn't apply to an inner ETH used after VXLAN since mlx4 can't match those yet. That's why it's important to also describe the supported combinations. > + > +- Can only use real destination MAC > +- Source MAC is not taken into consideration Neither EtherType (please end all sentences with periods). > + > + For example using testpmd command - src mask must be 00:00:00:00:00:00 > + otherwise the following command will fail > + > +.. code-block:: console > + > + testpmd> flow create 1 ingress pattern eth > + src spec 00:16:3e:2b:e6:47 src mask FF:FF:FF:FF:FF:FF > + / end actions drop / end Remember you're documenting API support limitations primarily for application writers who are not necessarily familiar with testpmd. The problem here is also that "src" is in fact an attribute of either "spec", "last" or "mask", not the other way around, hence you should refer to struct rte_flow_item / rte_flow_item_eth and its fields instead of using a testpmd example. > + > +- Supports only full MASK > + > + For example the following testpmd command will fail > + > +.. code-block:: console > + > + testpmd> flow create 1 ingress pattern eth > + src spec 00:16:3e:2b:e6:47 > + dst spec 4A:11:6C:FA:60:D0 dst mask FF:00:FF:FF:FF:00 > + / end actions drop / end > + Providing spec but no mask for src is valid (mask remains 0), however it's certainly a trap for unsuspecting readers unfamiliar with the flow command. Also providing examples is not bad in itself but they should not appear in the middle of an enumeration list as it makes them unclear. > + > +- When configured to run in promiscuous or all-multicast modes does > + not support additional rules This wording is misleading, the actual limitation is you can't provide additional items in a pattern if you want to match any destination MAC (mask.dst == 0) or only multicast traffic (spec.dst & mask.dst == 01:00:00:00:00:00). > +- Does not support the explicit exclusion of all multicast traffic > +- Does not support partial VLAN TCI VID matching This last item actually documents RTE_FLOW_ITEM_TYPE_VLAN. > + > +L3 (ipv4) > +^^^^^^^^^ > + > +- Supports only 0 or full mask. Prerequisites: Need to have eth dst spec Matching all fields of IPv4 headers is not supported, only source and destination. Not a single word about the lack of IPv6 support? > + > +L4 (tcp/udp) > +^^^^^^^^^^^^ > + > +- Supports only full mask Only on source and destination ports. Empty masks are also supported. > + For example the following testpmd command will fail > + > +.. code-block:: console > + > + testpmd> flow create 0 ingress pattern eth > + src spec e4:1d:2d:2d:8d:22 > + dst spec 00:15:5D:10:8D:00 dst mask FF:FF:FF:FF:FF:FF > + / ipv4 src spec 144.144.92.0 src prefix 16 > + / end actions drop / end Neither TCP nor UDP are part of this example. > + Prerequisites: Need to have eth dst spec and IPv4 before it with all > + its limitations > + > +Flow actions > +~~~~~~~~~~~~ > + > +RSS > +^^^ > + > +RSS is performed on packets to spread them among several queues based on hash > +function calculation and according to provided parameters. You should assume readers are familiar with what RSS does at this point. You only have to provide device-specific information on top of what is already documented by rte_flow.rst, you can link to that documentation if deemed necessary. > + > +- RSS hash is calculated on fixed packet fields including: L3 source and > + destination addresses (ipv4 or ipv6) and L4 source and destination addresses > + (upd or tcp ports) This reads like there's no choice but to have them all in a fixed fashion. Users can actually pick their own hash fields combinations from the supported set (IPv4/IPv6/TCP/UDP). > +- Uses default constant RSS key Wrong, users can provide their own key as well. The fixed key is only used for the default (non-rte_flow) RSS, or when one is not provided. > +- Only power of two number of queues is supported > +- Every Rx queue can be specified only once in RSS action Right, should probably be described a bit more extensively though. This section is titled "Limitations" but contains a mix of features, limitations and quirks, more like "Random thoughts regarding rte_flow support". I think this is not what users might expect from such a documentation which must be exhaustive and consistent. Getting there may involve tables. My suggestion is to first get everyone agree on a common rte_flow capabilities documentation format all PMDs could reuse and then fill in the blanks. What's your opinion? -- Adrien Mazarguil 6WIND