From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id CFC76A49C for ; Wed, 21 Mar 2018 20:26:00 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Mar 2018 12:25:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,341,1517904000"; d="scan'208";a="213446575" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.63]) ([10.237.221.63]) by fmsmga006.fm.intel.com with ESMTP; 21 Mar 2018 12:25:58 -0700 To: Pavan Nikhilesh , jerin.jacob@caviumnetworks.com, santosh.shukla@caviumnetworks.com Cc: dev@dpdk.org, Shahaf Shuler References: <20180308190716.31061-1-pbhagavatula@caviumnetworks.com> From: Ferruh Yigit Message-ID: <09c1f0a1-46f8-4eb0-55a9-e5630001822f@intel.com> Date: Wed, 21 Mar 2018 19:25:57 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180308190716.31061-1-pbhagavatula@caviumnetworks.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] net/octeontx: use the new offload APIs 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, 21 Mar 2018 19:26:01 -0000 On 3/8/2018 7:07 PM, Pavan Nikhilesh wrote: > Use the new Rx/Tx offload APIs and remove the old style offloads. > > Signed-off-by: Pavan Nikhilesh > --- > > Checkpatch reports falsepositive for PRIx64 > > drivers/net/octeontx/octeontx_ethdev.c | 82 ++++++++++++++++++++++++++-------- > drivers/net/octeontx/octeontx_ethdev.h | 3 ++ > 2 files changed, 67 insertions(+), 18 deletions(-) > > diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c > index b739c0b39..0448e3557 100644 > --- a/drivers/net/octeontx/octeontx_ethdev.c > +++ b/drivers/net/octeontx/octeontx_ethdev.c > @@ -262,6 +262,8 @@ octeontx_dev_configure(struct rte_eth_dev *dev) > struct rte_eth_rxmode *rxmode = &conf->rxmode; > struct rte_eth_txmode *txmode = &conf->txmode; > struct octeontx_nic *nic = octeontx_pmd_priv(dev); > + uint64_t configured_offloads; > + uint64_t unsupported_offloads; > int ret; > > PMD_INIT_FUNC_TRACE(); > @@ -283,34 +285,43 @@ octeontx_dev_configure(struct rte_eth_dev *dev) > return -EINVAL; > } > > - if (!rxmode->hw_strip_crc) { > + configured_offloads = rxmode->offloads; > + > + if (!(configured_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) { > PMD_INIT_LOG(NOTICE, "can't disable hw crc strip"); > - rxmode->hw_strip_crc = 1; > + configured_offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > } Just as a heads up this will be changed in this release [1], CRC_STRIP will be the default behavior without requiring a flag. [1] https://dpdk.org/ml/archives/dev/2018-March/093255.html > > - if (rxmode->hw_ip_checksum) { > + if (configured_offloads & DEV_RX_OFFLOAD_CHECKSUM) { > PMD_INIT_LOG(NOTICE, "rxcksum not supported"); > - rxmode->hw_ip_checksum = 0; > + configured_offloads &= ~DEV_RX_OFFLOAD_CHECKSUM; > } No need to specific check for DEV_RX_OFFLOAD_CHECKSUM, if it is not announced as supported below unsupported_offloads will cover it. > > - if (rxmode->split_hdr_size) { > - octeontx_log_err("rxmode does not support split header"); > - return -EINVAL; > - } > + unsupported_offloads = configured_offloads & ~OCTEONTX_RX_OFFLOADS; > > - if (rxmode->hw_vlan_filter) { > - octeontx_log_err("VLAN filter not supported"); > - return -EINVAL; > + if (unsupported_offloads) { > + PMD_INIT_LOG(ERR, "Rx offloads 0x%" PRIx64 " are not supported. " > + "Requested 0x%" PRIx64 " supported 0x%" PRIx64 "\n", > + unsupported_offloads, configured_offloads, > + (uint64_t)OCTEONTX_RX_OFFLOADS); > + return -ENOTSUP; > } > > - if (rxmode->hw_vlan_extend) { > - octeontx_log_err("VLAN extended not supported"); > - return -EINVAL; > + configured_offloads = txmode->offloads; > + > + if (!(configured_offloads & DEV_TX_OFFLOAD_MT_LOCKFREE)) { > + PMD_INIT_LOG(NOTICE, "cant disable lockfree tx"); > + configured_offloads |= DEV_TX_OFFLOAD_MT_LOCKFREE; > } > > - if (rxmode->enable_lro) { > - octeontx_log_err("LRO not supported"); > - return -EINVAL; > + unsupported_offloads = configured_offloads & ~OCTEONTX_TX_OFFLOADS; > + > + if (unsupported_offloads) { > + PMD_INIT_LOG(ERR, "Tx offloads 0x%" PRIx64 " are not supported." > + "Requested 0x%" PRIx64 " supported 0x%" PRIx64 ".\n", > + unsupported_offloads, configured_offloads, > + (uint64_t)OCTEONTX_TX_OFFLOADS); > + return -ENOTSUP; > } > > if (conf->link_speeds & ETH_LINK_SPEED_FIXED) { > @@ -750,10 +761,11 @@ octeontx_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t qidx, > struct octeontx_txq *txq = NULL; > uint16_t dq_num; > int res = 0; > + uint64_t configured_offloads; > + uint64_t unsupported_offloads; > > RTE_SET_USED(nb_desc); > RTE_SET_USED(socket_id); > - RTE_SET_USED(tx_conf); > > dq_num = (nic->port_id * PKO_VF_NUM_DQ) + qidx; > > @@ -771,6 +783,17 @@ octeontx_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t qidx, > dev->data->tx_queues[qidx] = NULL; > } > > + configured_offloads = tx_conf->offloads; > + > + unsupported_offloads = configured_offloads & ~OCTEONTX_TX_OFFLOADS; > + if (unsupported_offloads) { > + PMD_INIT_LOG(ERR, "Tx offloads 0x%" PRIx64 " are not supported." > + "Requested 0x%" PRIx64 " supported 0x%" PRIx64 ".\n", > + unsupported_offloads, configured_offloads, > + (uint64_t)OCTEONTX_TX_OFFLOADS); > + return -ENOTSUP; > + } There is a discussion about the requirement to have port offload in queue setup or not, more details in [2], any comment is welcome. [2] https://dpdk.org/ml/archives/dev/2018-March/092978.html history: https://dpdk.org/ml/archives/dev/2018-March/092862.html > + > /* Allocating tx queue data structure */ > txq = rte_zmalloc_socket("ethdev TX queue", sizeof(struct octeontx_txq), > RTE_CACHE_LINE_SIZE, nic->node); > @@ -826,6 +849,8 @@ octeontx_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t qidx, > uint8_t gaura; > unsigned int ev_queues = (nic->ev_queues * nic->port_id) + qidx; > unsigned int ev_ports = (nic->ev_ports * nic->port_id) + qidx; > + uint64_t configured_offloads; > + uint64_t unsupported_offloads; > > RTE_SET_USED(nb_desc); > > @@ -848,6 +873,27 @@ octeontx_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t qidx, > > port = nic->port_id; > > + configured_offloads = rx_conf->offloads; > + > + if (!(configured_offloads & DEV_RX_OFFLOAD_CRC_STRIP)) { > + PMD_INIT_LOG(NOTICE, "can't disable hw crc strip"); > + configured_offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > + } Same as above. > + > + if (configured_offloads & DEV_RX_OFFLOAD_CHECKSUM) { > + PMD_INIT_LOG(NOTICE, "rxcksum not supported"); > + configured_offloads &= ~DEV_RX_OFFLOAD_CHECKSUM; > + } Not sure about changing the application configuration, and since this offload is not reported as supported application should not set it, if it does this should return an error. > + > + unsupported_offloads = configured_offloads & ~OCTEONTX_RX_OFFLOADS; > + > + if (unsupported_offloads) { > + PMD_INIT_LOG(ERR, "Rx offloads 0x%" PRIx64 " are not supported. " > + "Requested 0x%" PRIx64 " supported 0x%" PRIx64 "\n", > + unsupported_offloads, configured_offloads, > + (uint64_t)OCTEONTX_RX_OFFLOADS); > + return -ENOTSUP; > + } > /* Rx deferred start is not supported */ > if (rx_conf->rx_deferred_start) { > octeontx_log_err("rx deferred start not supported"); > diff --git a/drivers/net/octeontx/octeontx_ethdev.h b/drivers/net/octeontx/octeontx_ethdev.h > index 10e42e142..9d6c22b0d 100644 > --- a/drivers/net/octeontx/octeontx_ethdev.h > +++ b/drivers/net/octeontx/octeontx_ethdev.h > @@ -28,6 +28,9 @@ > #define OCTEONTX_MAX_BGX_PORTS 4 > #define OCTEONTX_MAX_LMAC_PER_BGX 4 > > +#define OCTEONTX_RX_OFFLOADS DEV_RX_OFFLOAD_CRC_STRIP > +#define OCTEONTX_TX_OFFLOADS DEV_TX_OFFLOAD_MT_LOCKFREE These should be announced in dev_info as [rt]x_offload_capa, rx one seems missing. > + > static inline struct octeontx_nic * > octeontx_pmd_priv(struct rte_eth_dev *dev) > { > -- > 2.16.2 >