Hi Ferruh, On Wed, Mar 21, 2018 at 07:25:57PM +0000, Ferruh Yigit wrote: > 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 <pbhagavat...@caviumnetworks.com> > > --- > > > > 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.
Thanks for the heads up, will remove the 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. Will fix it in v2. > > > + > > + 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. If CRC strip is removed (implicit) then I think RX offload can be removed. > > > + > > static inline struct octeontx_nic * > > octeontx_pmd_priv(struct rte_eth_dev *dev) > > { > > -- > > 2.16.2 > > > Thanks, Pavan.