From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 52A9D2C30 for ; Wed, 25 Apr 2018 19:04:43 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Apr 2018 10:04:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,327,1520924400"; d="scan'208";a="223355203" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.42]) ([10.237.221.42]) by fmsmga006.fm.intel.com with ESMTP; 25 Apr 2018 10:04:40 -0700 To: Wei Dai , thomas@monjalon.net, qi.z.zhang@intel.com Cc: dev@dpdk.org References: <20180328085709.28310-1-wei.dai@intel.com> <1524657013-40960-1-git-send-email-wei.dai@intel.com> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= xsFNBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABzSVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+wsF+BBMBAgAoAhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgAUCWZR3VQUJB33WBQAKCRD5M+tD3xNhH6DWEACVhEb8q1epPwZrUDoxzu7E TS1b8tmabOmnjXZRs6+EXgUVHkp2xxkCfDmL3pa5bC0G/74aJnWjNsdvE05V1cb4YK4kRQ62 FwDQ+hlrFrwFB3PtDZk1tpkzCRHvJgnIil+0MuEh32Y57ig6hy8yO8ql7Lohyrnpfk/nNpm4 jQGEF5qEeHcEFe1AZQlPHN/STno8NZSz2nl0b2cw+cujN1krmvB52Ah/2KugQ6pprVyrGrzB c34ZQO9OsmSjJlETCZk6EZzuhfe16iqBFbOSadi9sPcJRwaUQBid+xdFWl7GQ8qC3zNPibSF HmU43yBZUqJDZlhIcl6/cFpOSjv2sDWdtjEXTDn5y/0FsuY0mFE78ItC4kCTIVk17VZoywcd fmbbnwOSWzDq7hiUYuQGkIudJw5k/A1CMsyLkoUEGN3sLfsw6KASgS4XrrmPO4UVr3mH5bP1 yC7i1OVNpzvOxtahmzm481ID8sk72GC2RktTOHb0cX+qdoiMMfYgo3wRRDYCBt6YoGYUxF1p msjocXyqToKhhnFbXLaZlVfnQ9i2i8jsj9SKig+ewC2p3lkPj6ncye9q95bzhmUeJO6sFhJg Hiz6syOMg8yCcq60j07airybAuHIDNFWk0gaWAmtHZxLObZx2PVn2nv9kLYGohFekw0AOsIW ta++5m48dnCoAc7BTQRX1ky+ARAApzQNvXvE2q1LAS+Z+ni2R13Bb1cDS1ZYq1jgpR13+OKN ipzd8MPngRJilXxBaPTErhgzR0vGcNTYhjGMSyFIHVOoBq1VbP1a0Fi/NqWzJOowo/fDfgVy K4vuitc/gCJs+2se4hdZA4EQJxVlNM51lgYDNpjPGIA43MX15OLAip73+ho6NPBMuc5qse3X pAClNhBKfENRCWN428pi3WVkT+ABRTE0taxjJNP7bb+9TQYNRqGwnGzX5/XISv44asWIQCaq vOkXSUJLd//cdVNTqtL1wreCVVR5pMXj7VIrlk07fmmJVALCmGbFr53BMb8O+8dgK2A5mitM n44d+8KdJWOwziRxcaMk/LclmZS3Iv1TERtiWt98Y9AjeAtcgYPkA3ld0BcUKONogP8pHVz1 Ed3s5rDQ91yr1S0wuAzW91fxGUO4wY+uPmxCtFVuBgd9VT9NAKTUL0qHM7CDgCnZPe0TW6Zj 8OqtdCCyAfvU9cW5xWM7Icxhde6AtPxhDSBwE8fL2ZmrDmaA4jmUKXp3i4JxRPSX84S08b+s DWXHPxy10UFU5A7EK/BEbZAKBwn9ROfm+WK+6X5xOGLoRE++OqNuUudxC1GDyLOPaqCbBCS9 +P6HsTHzxsjyJa27n4jcrcuY3P9TEcFJYSZSeSDh8mVGvugi0exnSJrrBZDyVCcAEQEAAcLB ZQQYAQIADwIbDAUCWZR1ZwUJA59cIQAKCRD5M+tD3xNhH5b+D/9XG44Ci6STdcA5RO/ur05J EE3Ux1DCHZ5V7vNAtX/8Wg4l4GZfweauXwuJ1w7Sp7fklwcNC6wsceI+EmNjGMqfIaukGetG +jBGqsQ7moOZodfXUoCK98gblKgt/BPYMVidzlGC8Q/+lZg1+o29sPnwImW+MXt/Z5az/Z17 Qc265g+p5cqJHzq6bpQdnF7Fu6btKU/kv6wJghENvgMXBuyThqsyFReJWFh2wfaKyuix3Zyj ccq7/blkhzIKmtFWgDcgaSc2UAuJU+x9nuYjihW6WobpKP/nlUDu3BIsbIq09UEke+uE/QK+ FJ8PTJkAsXOf1Bc2C0XbW4Y2hf103+YY6L8weUCBsWC5VH5VtVmeuh26ENURclwfeXhWQ9Og 77yzpTXWr5g1Z0oLpYpWPv745J4bE7pv+dzxOrFdM1xNkzY2pvXph/A8OjxZNQklDkHQ7PIB Lki5L2F4XkEOddUUQchJwzMqTPsggPDmGjgLZrqgO+s4ECZK5+nLD3HEpAbPa3JLDaScy+90 Nu1lAqPUHSnP3vYZVw85ZYm6UCxHE4VLMnnJsN09ZhsOSVR+GyP5Nyw9rT1V3lcsuH7M5Naa 2Xobn9m7l9bRCD/Ji8kG15eV1WTxx1HXVQGjdUYDI7UwegBNbwMLh17XDy+3sn/6SgcqtECA Q6pZKA2mTQxEKMLBZQQYAQIADwIbDAUCWZR3hQUJA59eRwAKCRD5M+tD3xNhH4a/D/4jLAZu UhvU1swWcNEVVCELZ0D3LOV14XcY2MXa3QOpeZ9Bgq7YYJ4S5YXK+SBQS0FkRZdjGNvlGZoG ZdpU+NsQmQFhqHGwX0IT9MeTFM8uvKgxNKGwMVcV9g0IOqwBhGHne+BFboRA9362fgGW5AYQ zT0mzzRKEoOh4r3AQvbM6kLISxo0k1ujdYiI5nj/5WoKDqxTwwfuN1uDUHsWo3tzenRmpMyU NyW3Dc+1ajvXLyo09sRRq7BnM99Rix1EGL8Qhwy+j0YAv+FuspWxUX9FxXYho5PvGLHLsHfK FYQ7x/RRbpMjkJWVfIe/xVnfvn4kz+MTA5yhvsuNi678fLwY9hBP0y4lO8Ob2IhEPdfnTuIs tFVxXuelJ9xAe5TyqP0f+fQjf1ixsBZkqOohsBXDfje0iaUpYa/OQ/BBeej0dUdg2JEu4jAC x41HpVCnP9ipLpD0fYz1d/dX0F/VY2ovW6Eba/y/ngOSAR6C+u881m7oH2l0G47MTwkaQCBA bLGXPj4TCdX3lftqt4bcBPBJ+rFAnJmRHtUuyyaewBnZ81ZU2YAptqFM1kTh+aSvMvGhfVsQ qZL2rk2OPN1hg+KXhErlbTZ6oPtLCFhSHQmuxQ4oc4U147wBTUuOdwNjtnNatUhRCp8POc+3 XphVR5G70mnca1E2vzC77z+XSlTyRA== Message-ID: <4bdecf05-3a7b-1e5d-8a3b-e71e0c37a74d@intel.com> Date: Wed, 25 Apr 2018 18:04:40 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1524657013-40960-1-git-send-email-wei.dai@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v4] ethdev: check Rx/Tx offloads 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, 25 Apr 2018 17:04:43 -0000 On 4/25/2018 12:50 PM, Wei Dai wrote: > This patch check if a requested offloading is supported > in the device capability. > Any offloading is disabled by default if it is not set > in rte_eth_dev_configure( ) and rte_eth_[rt]x_queue_setup(). > A per port offloading can only be enabled in > rte_eth_dev_configure(). If a per port offloading is > sent to rte_eth_[rt]x_queue_setup( ), return error. > Only per queue offloading can be sent to > rte_eth_[rt]x_queue_setup( ). A per queue offloading is > enabled only if it is enabled in rte_eth_dev_configure( ) OR > if it is enabled in rte_eth_[rt]x_queue_setup( ). > If a per queue offloading is enabled in rte_eth_dev_configure(), > it can't be disabled in rte_eth_[rt]x_queue_setup( ). > If a per queue offloading is disabled in rte_eth_dev_configure( ), > it can be enabled or disabled( ) in rte_eth_[rt]x_queue_setup( ). > > This patch can make such checking in a common way in rte_ethdev > layer to avoid same checking in underlying PMD. Hi Wei, For clarification, there is existing API for rc1, and there is a suggested update in API for rc2. I guess this patch is for suggested update in rc2? > Signed-off-by: Wei Dai > > --- > v4: fix a wrong description in git log message. > > v3: rework according to dicision of offloading API in community > > v2: add offloads checking in rte_eth_dev_configure( ). > check if a requested offloading is supported. > --- > lib/librte_ether/rte_ethdev.c | 76 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index f0f53d4..70a7904 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -1196,6 +1196,28 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, > ETHER_MAX_LEN; > } > > + /* Any requested offload must be within its device capability */ > + if ((local_conf.rxmode.offloads & dev_info.rx_offload_capa) != > + local_conf.rxmode.offloads) { > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Rx offloads " > + "0x%" PRIx64 " doesn't match Rx offloads " > + "capability 0x%" PRIx64 "\n", > + port_id, > + local_conf.rxmode.offloads, > + dev_info.rx_offload_capa); > + return -EINVAL; > + } > + if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) != > + local_conf.txmode.offloads) { > + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Tx offloads " > + "0x%" PRIx64 " doesn't match Tx offloads " > + "capability 0x%" PRIx64 "\n", > + port_id, > + local_conf.txmode.offloads, > + dev_info.tx_offload_capa); > + return -EINVAL; > + } +1 having these checks here. > + > /* > * Setup new number of RX/TX queues and reconfigure device. > */ > @@ -1547,6 +1569,33 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id, > &local_conf.offloads); > } > > + /* > + * Only per-queue offload can be enabled from application. > + * If any pure per-port offload is sent to this function, return -EINVAL > + */ > + if ((local_conf.offloads & dev_info.rx_queue_offload_capa) != > + local_conf.offloads) { > + RTE_PMD_DEBUG_TRACE("Ethdev port_id=%d rx_queue_id=%d " > + "Requested offload 0x%" PRIx64 "doesn't " > + "match per-queue capability 0x%" PRIx64 > + " in rte_eth_rx_queue_setup( )\n", > + port_id, > + rx_queue_id, > + local_conf.offloads, > + dev_info.rx_queue_offload_capa); > + return -EINVAL; > + } There is a change here. If requested offload is already enabled in port level, instead of returning error, ignore it. So this removes the restriction for apps that "only an offload from queue capabilities can be send for queue_setup() functions". This is not requirement for application as it has been before, but this is allowed for app now. If app tried to enable a port offload in queue level that is not already enabled, it should still return error. > + > + /* > + * If a per-queue offload is enabled in rte_eth_dev_configure( ), > + * it is also enabled on all queues and can't be disabled here. > + * If it is diabled in rte_eth_dev_configure( ), it can be enabled > + * or disabled here. > + * If a per-port offload is enabled in rte_eth_dev_configure( ), > + * it is also enabled for all queues here. > + */ > + local_conf.offloads |= dev->data->dev_conf.rxmode.offloads; I didn't get this one, why add rxmode.offloads into queue offloads? Based on above change Thomas has an suggestion [1]: " In the case of offload already enabled at port level and repeated in queue setup, ethdev can avoid passing it to the PMD queue setup function. " So almost reverse of what you are doing, strip rxmode.offloads from local_conf.offloads for PMDs. What do you think? [1] https://dpdk.org/ml/archives/dev/2018-April/098956.html