From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 208237CB6 for ; Wed, 13 Sep 2017 10:13:21 +0200 (CEST) Received: from pure.maildistiller.com (unknown [10.110.50.29]) by dispatch1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTP id 001A12005D; Wed, 13 Sep 2017 08:13:21 +0000 (UTC) X-Virus-Scanned: Proofpoint Essentials engine Received: from mx6-us1.ppe-hosted.com (unknown [10.110.49.251]) by pure.maildistiller.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 344B56004B; Wed, 13 Sep 2017 08:13:20 +0000 (UTC) Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx6-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id B42E14C005F; Wed, 13 Sep 2017 08:13:19 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Wed, 13 Sep 2017 09:13:14 +0100 To: Shahaf Shuler , CC: References: From: Andrew Rybchenko Message-ID: Date: Wed, 13 Sep 2017 11:13:11 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23326.003 X-TM-AS-Result: No--16.427100-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1505290400-a+UavRzIo9jm Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v3 1/2] ethdev: introduce Rx queue offloads API 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: Wed, 13 Sep 2017 08:13:22 -0000 On 09/13/2017 09:37 AM, Shahaf Shuler wrote: > Introduce a new API to configure Rx offloads. > > In the new API, offloads are divided into per-port and per-queue > offloads. The PMD reports capability for each of them. > Offloads are enabled using the existing DEV_RX_OFFLOAD_* flags. > To enable per-port offload, the offload should be set on both device > configuration and queue configuration. To enable per-queue offload, the > offloads can be set only on queue configuration. > > Applications should set the ignore_offload_bitfield bit on rxmode > structure in order to move to the new API. I think it would be useful to have the description in the documentation. It is really important topic on how per-port and per-queue offloads coexist and rules should be 100% clear for PMD maintainers and application developers. Please, also highlight how per-port and per-queue capabilities should be advertised. I mean if per-queue capability should be reported as per-port as well. I'd say no to avoid duplication of per-queue capabilities in two places. If so, could you explain why to enable it should be specified in both places. How should be treated configuration when the offload is enabled on port, but disabled on queue level. > The old Rx offloads API is kept for the meanwhile, in order to enable a > smooth transition for PMDs and application to the new API. > > Signed-off-by: Shahaf Shuler > --- > doc/guides/nics/features.rst | 33 ++++---- > lib/librte_ether/rte_ethdev.c | 156 +++++++++++++++++++++++++++++++++---- > lib/librte_ether/rte_ethdev.h | 51 +++++++++++- > 3 files changed, 210 insertions(+), 30 deletions(-) > > diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst > index 37ffbc68c..4e68144ef 100644 > --- a/doc/guides/nics/features.rst > +++ b/doc/guides/nics/features.rst > @@ -179,7 +179,7 @@ Jumbo frame > > Supports Rx jumbo frames. > > -* **[uses] user config**: ``dev_conf.rxmode.jumbo_frame``, May be it should be removed from documentation when it is removed from sources? I have no strong opinion, but it would be more clear to find it in the documentation with its status specified (obsolete) The note is applicable to all similar cases below. [snip] > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > index 0adf3274a..ba7a2b2dc 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h [snip] > @@ -907,6 +934,18 @@ struct rte_eth_conf { > #define DEV_RX_OFFLOAD_QINQ_STRIP 0x00000020 > #define DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000040 > #define DEV_RX_OFFLOAD_MACSEC_STRIP 0x00000080 > +#define DEV_RX_OFFLOAD_HEADER_SPLIT 0x00000100 > +#define DEV_RX_OFFLOAD_VLAN_FILTER 0x00000200 > +#define DEV_RX_OFFLOAD_VLAN_EXTEND 0x00000400 > +#define DEV_RX_OFFLOAD_JUMBO_FRAME 0x00000800 > +#define DEV_RX_OFFLOAD_CRC_STRIP 0x00001000 > +#define DEV_RX_OFFLOAD_SCATTER 0x00002000 > +#define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \ > + DEV_RX_OFFLOAD_UDP_CKSUM | \ > + DEV_RX_OFFLOAD_TCP_CKSUM) > +#define DEV_RX_OFFLOAD_VLAN (DEV_RX_OFFLOAD_VLAN_STRIP | \ > + DEV_RX_OFFLOAD_VLAN_FILTER | \ > + DEV_RX_OFFLOAD_VLAN_EXTEND) It is not directly related to the patch, but I'd like to highlight that Rx/Tx are asymmetric here since SCTP is missing for Rx, but present for Tx. [snip]