From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 5FFC0A05D3 for ; Thu, 23 May 2019 14:18:22 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EA37A1B945; Thu, 23 May 2019 14:18:20 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id CE7E44F91 for ; Thu, 23 May 2019 14:18:18 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 43D4A305F0DC; Thu, 23 May 2019 12:18:10 +0000 (UTC) Received: from [10.36.112.60] (ovpn-112-60.ams2.redhat.com [10.36.112.60]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EF0DE1001E84; Thu, 23 May 2019 12:18:07 +0000 (UTC) To: Ajit Khaparde , dev@dpdk.org Cc: ferruh.yigit@intel.com, Lance Richardson References: <20190521213953.25425-1-ajit.khaparde@broadcom.com> <20190521213953.25425-4-ajit.khaparde@broadcom.com> From: Maxime Coquelin Message-ID: <311341f0-054b-00af-4778-f8b2f5c771c2@redhat.com> Date: Thu, 23 May 2019 14:18:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190521213953.25425-4-ajit.khaparde@broadcom.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Thu, 23 May 2019 12:18:13 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH 03/11] net/bnxt: implement vector mode driver 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 5/21/19 11:39 PM, Ajit Khaparde wrote: > From: Lance Richardson > > Introduce vector mode support for the bnxt pmd. > > Signed-off-by: Lance Richardson > Signed-off-by: Ajit Khaparde > --- > config/common_base | 1 + > drivers/net/bnxt/Makefile | 1 + > drivers/net/bnxt/bnxt_ethdev.c | 92 ++++- > drivers/net/bnxt/bnxt_ring.h | 3 +- > drivers/net/bnxt/bnxt_rxq.c | 5 + > drivers/net/bnxt/bnxt_rxq.h | 4 + > drivers/net/bnxt/bnxt_rxr.h | 9 +- > drivers/net/bnxt/bnxt_rxtx_vec_sse.c | 481 +++++++++++++++++++++++++++ > drivers/net/bnxt/bnxt_txr.h | 11 +- > 9 files changed, 598 insertions(+), 9 deletions(-) > create mode 100644 drivers/net/bnxt/bnxt_rxtx_vec_sse.c > > diff --git a/config/common_base b/config/common_base > index 6b96e0e80..1bbb7c10b 100644 > --- a/config/common_base > +++ b/config/common_base > @@ -212,6 +212,7 @@ CONFIG_RTE_LIBRTE_BNX2X_DEBUG_PERIODIC=n > # Compile burst-oriented Broadcom BNXT PMD driver > # > CONFIG_RTE_LIBRTE_BNXT_PMD=y > +CONFIG_RTE_LIBRTE_BNXT_INC_VECTOR=n > > # > # Compile burst-oriented Chelsio Terminator (CXGBE) PMD > diff --git a/drivers/net/bnxt/Makefile b/drivers/net/bnxt/Makefile > index 8be3cb0e4..9e006b5d1 100644 > --- a/drivers/net/bnxt/Makefile > +++ b/drivers/net/bnxt/Makefile > @@ -41,6 +41,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_vnic.c > SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_irq.c > SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_util.c > SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += rte_pmd_bnxt.c > +SRCS-$(CONFIG_RTE_LIBRTE_BNXT_INC_VECTOR) += bnxt_rxtx_vec_sse.c > > # > # Export include files How do we enable it with Meson build? It seems to be missing. > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c > index e0e0b72c6..064a153ec 100644 > --- a/drivers/net/bnxt/bnxt_ethdev.c > +++ b/drivers/net/bnxt/bnxt_ethdev.c > @@ -645,6 +645,67 @@ static int bnxt_scattered_rx(struct rte_eth_dev *eth_dev) > return 0; > } > > +static eth_rx_burst_t > +bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev) > +{ > +#ifdef RTE_LIBRTE_BNXT_INC_VECTOR > + /* > + * Vector mode receive can be enabled only if scatter rx is not > + * in use and rx offloads are limited to VLAN stripping and > + * CRC stripping. > + */ > + if (!eth_dev->data->scattered_rx && > + !(eth_dev->data->dev_conf.rxmode.offloads & > + ~(DEV_RX_OFFLOAD_VLAN_STRIP | > + DEV_RX_OFFLOAD_KEEP_CRC | > + DEV_RX_OFFLOAD_JUMBO_FRAME | > + DEV_RX_OFFLOAD_IPV4_CKSUM | > + DEV_RX_OFFLOAD_UDP_CKSUM | > + DEV_RX_OFFLOAD_TCP_CKSUM | > + DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM | > + DEV_RX_OFFLOAD_VLAN_FILTER))) { > + PMD_DRV_LOG(INFO, "Using vector mode receive for port %d\n", > + eth_dev->data->port_id); > + return bnxt_recv_pkts_vec; > + } > + PMD_DRV_LOG(INFO, "Vector mode receive disabled for port %d\n", > + eth_dev->data->port_id); > + PMD_DRV_LOG(INFO, > + "Port %d scatter: %d rx offload: %" PRIX64 "\n", > + eth_dev->data->port_id, > + eth_dev->data->scattered_rx, > + eth_dev->data->dev_conf.rxmode.offloads); > +#endif I think we would better have a stub for bnxt_recv_pkts_vec() when RTE_LIBRTE_BNXT_INC_VECTOR isn't set, that would avoid having all these #ifdeferies in this file. > + return bnxt_recv_pkts; > +} > + > +static eth_tx_burst_t > +bnxt_transmit_function(__rte_unused struct rte_eth_dev *eth_dev) > +{ > +#ifdef RTE_LIBRTE_BNXT_INC_VECTOR > + /* > + * Vector mode receive can be enabled only if scatter tx is not > + * in use and tx offloads other than VLAN insertion are not > + * in use. > + */ > + if (!eth_dev->data->scattered_rx && > + !(eth_dev->data->dev_conf.txmode.offloads & > + ~DEV_TX_OFFLOAD_VLAN_INSERT)) { > + PMD_DRV_LOG(INFO, "Using vector mode transmit for port %d\n", > + eth_dev->data->port_id); > + return bnxt_xmit_pkts_vec; > + } > + PMD_DRV_LOG(INFO, "Vector mode transmit disabled for port %d\n", > + eth_dev->data->port_id); > + PMD_DRV_LOG(INFO, > + "Port %d scatter: %d tx offload: %" PRIX64 "\n", > + eth_dev->data->port_id, > + eth_dev->data->scattered_rx, > + eth_dev->data->dev_conf.txmode.offloads); > +#endif > + return bnxt_xmit_pkts; > +} > + > static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) > { > struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private; > @@ -675,6 +736,8 @@ static int bnxt_dev_start_op(struct rte_eth_dev *eth_dev) > if (rc) > goto error; > > + eth_dev->rx_pkt_burst = bnxt_receive_function(eth_dev); > + eth_dev->tx_pkt_burst = bnxt_transmit_function(eth_dev); > bp->flags |= BNXT_FLAG_INIT_DONE; > return 0; > > @@ -1597,6 +1660,8 @@ bnxt_txq_info_get_op(struct rte_eth_dev *dev, uint16_t queue_id, > > static int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu) > { > + uint32_t new_pkt_size = new_mtu + ETHER_HDR_LEN + ETHER_CRC_LEN + > + VLAN_TAG_SIZE * BNXT_NUM_VLANS; > struct bnxt *bp = eth_dev->data->dev_private; > struct rte_eth_dev_info dev_info; > uint32_t rc = 0; > @@ -1610,6 +1675,23 @@ static int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu) > return -EINVAL; > } > > +#ifdef RTE_LIBRTE_BNXT_INC_VECTOR > + /* > + * If vector-mode tx/rx is active, disallow any MTU change that would > + * require scattered receive support. > + */ > + if (eth_dev->data->dev_started && > + (eth_dev->rx_pkt_burst == bnxt_recv_pkts_vec || > + eth_dev->tx_pkt_burst == bnxt_xmit_pkts_vec) && > + (new_pkt_size > > + eth_dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)) { > + PMD_DRV_LOG(ERR, > + "MTU change would require scattered rx support. "); > + PMD_DRV_LOG(ERR, "Stop port before changing MTU.\n"); > + return -EINVAL; > + } > +#endif > + > if (new_mtu > ETHER_MTU) { > bp->flags |= BNXT_FLAG_JUMBO; > bp->eth_dev->data->dev_conf.rxmode.offloads |= > @@ -1620,8 +1702,7 @@ static int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu) > bp->flags &= ~BNXT_FLAG_JUMBO; > } > > - eth_dev->data->dev_conf.rxmode.max_rx_pkt_len = > - new_mtu + ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE * 2; > + eth_dev->data->dev_conf.rxmode.max_rx_pkt_len = new_pkt_size; > > eth_dev->data->mtu = new_mtu; > PMD_DRV_LOG(INFO, "New MTU is %d\n", eth_dev->data->mtu); > @@ -2655,9 +2736,10 @@ bnxt_dev_supported_ptypes_get_op(struct rte_eth_dev *dev) > RTE_PTYPE_UNKNOWN > }; > > - if (dev->rx_pkt_burst == bnxt_recv_pkts) > - return ptypes; > - return NULL; > + if (!dev->rx_pkt_burst) > + return NULL; > + > + return ptypes; > } > > static int bnxt_map_regs(struct bnxt *bp, uint32_t *reg_arr, int count, ... > diff --git a/drivers/net/bnxt/bnxt_txr.h b/drivers/net/bnxt/bnxt_txr.h > index 13ca04676..c28bd9c18 100644 > --- a/drivers/net/bnxt/bnxt_txr.h > +++ b/drivers/net/bnxt/bnxt_txr.h > @@ -37,13 +37,15 @@ struct bnxt_sw_tx_bd { > unsigned short nr_bds; > }; > > -static inline uint32_t bnxt_tx_bds_in_hw(struct bnxt_tx_queue *txq) > +static inline uint32_t > +bnxt_tx_bds_in_hw(struct bnxt_tx_queue *txq) If to be done, it should be done in patch 1. > { > return ((txq->tx_ring->tx_prod - txq->tx_ring->tx_cons) & > txq->tx_ring->tx_ring_struct->ring_mask); > } > > -static inline uint32_t bnxt_tx_avail(struct bnxt_tx_queue *txq) > +static inline uint32_t > +bnxt_tx_avail(struct bnxt_tx_queue *txq) Ditto > { > /* Tell compiler to fetch tx indices from memory. */ > rte_compiler_barrier(); > @@ -57,6 +59,11 @@ int bnxt_init_one_tx_ring(struct bnxt_tx_queue *txq); > int bnxt_init_tx_ring_struct(struct bnxt_tx_queue *txq, unsigned int socket_id); > uint16_t bnxt_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, > uint16_t nb_pkts); > +#ifdef RTE_LIBRTE_BNXT_INC_VECTOR > +uint16_t bnxt_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts, > + uint16_t nb_pkts); > +#endif > + > int bnxt_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id); > int bnxt_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id); > >