From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id C11C65699 for ; Tue, 20 Nov 2018 20:15:58 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 31F7E30820E4; Tue, 20 Nov 2018 19:15:58 +0000 (UTC) Received: from ktraynor.remote.csb (unknown [10.36.118.7]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5235060141; Tue, 20 Nov 2018 19:15:57 +0000 (UTC) From: Kevin Traynor To: Shahed Shaikh Cc: dpdk stable Date: Tue, 20 Nov 2018 19:12:45 +0000 Message-Id: <20181120191252.30277-55-ktraynor@redhat.com> In-Reply-To: <20181120191252.30277-1-ktraynor@redhat.com> References: <20181120191252.30277-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Tue, 20 Nov 2018 19:15:58 +0000 (UTC) Subject: [dpdk-stable] patch 'net/qede: fix Rx buffer size calculation' has been queued to stable release 18.08.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Nov 2018 19:15:59 -0000 Hi, FYI, your patch has been queued to stable release 18.08.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 11/23/18. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. If the code is different (ie: not only metadata diffs), due for example to a change in context or macro names, please double check it. Thanks. Kevin Traynor --- >>From b5829a0470611b1e0bac0413f91ac6996447731d Mon Sep 17 00:00:00 2001 From: Shahed Shaikh Date: Sat, 8 Sep 2018 13:31:03 -0700 Subject: [PATCH] net/qede: fix Rx buffer size calculation [ upstream commit 318d7da3122bac04772418c5eda9f50fcd175d18 ] - HW does not include CRC in received frame when passed to host, so no need to consider CRC length while calculating Rx buffer size. - In scattered Rx mode, driver may allocate Rx buffer larger than the size of mbuf because it tries to adjust the buffer size to cache line size by ceiling it. Fix this by flooring the size instead of ceiling. - Consider the rule imposed by HW regarding the minimum size of Rx buffer in scattered Rx mode - (MTU + Maximum L2 Header Size + 2) / ETH_RX_MAX_BUFF_PER_PKT Fixes: f6033f2497e7 ("net/qede: fix minimum buffer size and scatter Rx check") Signed-off-by: Shahed Shaikh --- drivers/net/qede/qede_ethdev.c | 28 ++++++++-------- drivers/net/qede/qede_rxtx.c | 59 ++++++++++++++++++++++++++++++---- drivers/net/qede/qede_rxtx.h | 15 +++++++-- 3 files changed, 78 insertions(+), 24 deletions(-) diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c index df52ea928..bd906b293 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -1484,5 +1484,5 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->mtu = eth_dev->data->dev_conf.rxmode.max_rx_pkt_len - - ETHER_HDR_LEN - ETHER_CRC_LEN; + ETHER_HDR_LEN - QEDE_ETH_OVERHEAD; if (rxmode->offloads & DEV_RX_OFFLOAD_SCATTER) @@ -2500,17 +2500,16 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) uint32_t max_rx_pkt_len; uint32_t frame_size; - uint16_t rx_buf_size; uint16_t bufsz; bool restart = false; - int i; + int i, rc; PMD_INIT_FUNC_TRACE(edev); qede_dev_info_get(dev, &dev_info); - max_rx_pkt_len = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN; - frame_size = max_rx_pkt_len + QEDE_ETH_OVERHEAD; + max_rx_pkt_len = mtu + QEDE_MAX_ETHER_HDR_LEN; + frame_size = max_rx_pkt_len; if ((mtu < ETHER_MIN_MTU) || (frame_size > dev_info.max_rx_pktlen)) { DP_ERR(edev, "MTU %u out of range, %u is maximum allowable\n", mtu, dev_info.max_rx_pktlen - ETHER_HDR_LEN - - ETHER_CRC_LEN - QEDE_ETH_OVERHEAD); + QEDE_ETH_OVERHEAD); return -EINVAL; } @@ -2540,12 +2539,13 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) bufsz = (uint16_t)rte_pktmbuf_data_room_size( fp->rxq->mb_pool) - RTE_PKTMBUF_HEADROOM; - if (dev->data->scattered_rx) - rx_buf_size = bufsz + ETHER_HDR_LEN + - ETHER_CRC_LEN + QEDE_ETH_OVERHEAD; - else - rx_buf_size = frame_size; - rx_buf_size = QEDE_CEIL_TO_CACHE_LINE_SIZE(rx_buf_size); - fp->rxq->rx_buf_size = rx_buf_size; - DP_INFO(edev, "RX buffer size %u\n", rx_buf_size); + /* cache align the mbuf size to simplfy rx_buf_size + * calculation + */ + bufsz = QEDE_FLOOR_TO_CACHE_LINE_SIZE(bufsz); + rc = qede_calc_rx_buf_size(dev, bufsz, frame_size); + if (rc < 0) + return rc; + + fp->rxq->rx_buf_size = rc; } } diff --git a/drivers/net/qede/qede_rxtx.c b/drivers/net/qede/qede_rxtx.c index 0f157ded2..675c0a033 100644 --- a/drivers/net/qede/qede_rxtx.c +++ b/drivers/net/qede/qede_rxtx.c @@ -36,4 +36,47 @@ static inline int qede_alloc_rx_buffer(struct qede_rx_queue *rxq) } +/* Criterias for calculating Rx buffer size - + * 1) rx_buf_size should not exceed the size of mbuf + * 2) In scattered_rx mode - minimum rx_buf_size should be + * (MTU + Maximum L2 Header Size + 2) / ETH_RX_MAX_BUFF_PER_PKT + * 3) In regular mode - minimum rx_buf_size should be + * (MTU + Maximum L2 Header Size + 2) + * In above cases +2 corrosponds to 2 bytes padding in front of L2 + * header. + * 4) rx_buf_size should be cacheline-size aligned. So considering + * criteria 1, we need to adjust the size to floor instead of ceil, + * so that we don't exceed mbuf size while ceiling rx_buf_size. + */ +int +qede_calc_rx_buf_size(struct rte_eth_dev *dev, uint16_t mbufsz, + uint16_t max_frame_size) +{ + struct qede_dev *qdev = QEDE_INIT_QDEV(dev); + struct ecore_dev *edev = QEDE_INIT_EDEV(qdev); + int rx_buf_size; + + if (dev->data->scattered_rx) { + /* per HW limitation, only ETH_RX_MAX_BUFF_PER_PKT number of + * bufferes can be used for single packet. So need to make sure + * mbuf size is sufficient enough for this. + */ + if ((mbufsz * ETH_RX_MAX_BUFF_PER_PKT) < + (max_frame_size + QEDE_ETH_OVERHEAD)) { + DP_ERR(edev, "mbuf %d size is not enough to hold max fragments (%d) for max rx packet length (%d)\n", + mbufsz, ETH_RX_MAX_BUFF_PER_PKT, max_frame_size); + return -EINVAL; + } + + rx_buf_size = RTE_MAX(mbufsz, + (max_frame_size + QEDE_ETH_OVERHEAD) / + ETH_RX_MAX_BUFF_PER_PKT); + } else { + rx_buf_size = max_frame_size + QEDE_ETH_OVERHEAD; + } + + /* Align to cache-line size if needed */ + return QEDE_FLOOR_TO_CACHE_LINE_SIZE(rx_buf_size); +} + int qede_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, @@ -86,4 +129,6 @@ qede_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, /* Fix up RX buffer size */ bufsz = (uint16_t)rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM; + /* cache align the mbuf size to simplfy rx_buf_size calculation */ + bufsz = QEDE_FLOOR_TO_CACHE_LINE_SIZE(bufsz); if ((rxmode->offloads & DEV_RX_OFFLOAD_SCATTER) || (max_rx_pkt_len + QEDE_ETH_OVERHEAD) > bufsz) { @@ -94,11 +139,11 @@ qede_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx, } - if (dev->data->scattered_rx) - rxq->rx_buf_size = bufsz + ETHER_HDR_LEN + - ETHER_CRC_LEN + QEDE_ETH_OVERHEAD; - else - rxq->rx_buf_size = max_rx_pkt_len + QEDE_ETH_OVERHEAD; - /* Align to cache-line size if needed */ - rxq->rx_buf_size = QEDE_CEIL_TO_CACHE_LINE_SIZE(rxq->rx_buf_size); + rc = qede_calc_rx_buf_size(dev, bufsz, max_rx_pkt_len); + if (rc < 0) { + rte_free(rxq); + return rc; + } + + rxq->rx_buf_size = rc; DP_INFO(edev, "mtu %u mbufsz %u bd_max_bytes %u scatter_mode %d\n", diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h index e710fbaed..8bd8d1c5a 100644 --- a/drivers/net/qede/qede_rxtx.h +++ b/drivers/net/qede/qede_rxtx.h @@ -62,7 +62,14 @@ #define QEDE_CEIL_TO_CACHE_LINE_SIZE(n) (((n) + (QEDE_FW_RX_ALIGN_END - 1)) & \ ~(QEDE_FW_RX_ALIGN_END - 1)) -/* Note: QEDE_LLC_SNAP_HDR_LEN is optional */ -#define QEDE_ETH_OVERHEAD (((2 * QEDE_VLAN_TAG_SIZE)) - (ETHER_CRC_LEN) \ - + (QEDE_LLC_SNAP_HDR_LEN)) +#define QEDE_FLOOR_TO_CACHE_LINE_SIZE(n) RTE_ALIGN_FLOOR(n, \ + QEDE_FW_RX_ALIGN_END) + +/* Note: QEDE_LLC_SNAP_HDR_LEN is optional, + * +2 is for padding in front of L2 header + */ +#define QEDE_ETH_OVERHEAD (((2 * QEDE_VLAN_TAG_SIZE)) \ + + (QEDE_LLC_SNAP_HDR_LEN) + 2) + +#define QEDE_MAX_ETHER_HDR_LEN (ETHER_HDR_LEN + QEDE_ETH_OVERHEAD) #define QEDE_RSS_OFFLOAD_ALL (ETH_RSS_IPV4 |\ @@ -268,4 +275,6 @@ int qede_start_queues(struct rte_eth_dev *eth_dev); void qede_stop_queues(struct rte_eth_dev *eth_dev); +int qede_calc_rx_buf_size(struct rte_eth_dev *dev, uint16_t mbufsz, + uint16_t max_frame_size); /* Fastpath resource alloc/dealloc helpers */ -- 2.19.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2018-11-20 17:53:08.773898109 +0000 +++ 0055-net-qede-fix-Rx-buffer-size-calculation.patch 2018-11-20 17:53:07.000000000 +0000 @@ -1,8 +1,10 @@ -From 318d7da3122bac04772418c5eda9f50fcd175d18 Mon Sep 17 00:00:00 2001 +From b5829a0470611b1e0bac0413f91ac6996447731d Mon Sep 17 00:00:00 2001 From: Shahed Shaikh Date: Sat, 8 Sep 2018 13:31:03 -0700 Subject: [PATCH] net/qede: fix Rx buffer size calculation +[ upstream commit 318d7da3122bac04772418c5eda9f50fcd175d18 ] + - HW does not include CRC in received frame when passed to host, so no need to consider CRC length while calculating Rx buffer size. - In scattered Rx mode, driver may allocate Rx buffer larger than @@ -14,7 +16,6 @@ (MTU + Maximum L2 Header Size + 2) / ETH_RX_MAX_BUFF_PER_PKT Fixes: f6033f2497e7 ("net/qede: fix minimum buffer size and scatter Rx check") -CC: stable@dpdk.org Signed-off-by: Shahed Shaikh --- @@ -24,17 +25,17 @@ 3 files changed, 78 insertions(+), 24 deletions(-) diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c -index b8282730b..3e4cac738 100644 +index df52ea928..bd906b293 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c -@@ -1211,5 +1211,5 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) +@@ -1484,5 +1484,5 @@ static int qede_dev_configure(struct rte_eth_dev *eth_dev) eth_dev->data->mtu = eth_dev->data->dev_conf.rxmode.max_rx_pkt_len - - ETHER_HDR_LEN - ETHER_CRC_LEN; + ETHER_HDR_LEN - QEDE_ETH_OVERHEAD; if (rxmode->offloads & DEV_RX_OFFLOAD_SCATTER) -@@ -2226,17 +2226,16 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) +@@ -2500,17 +2500,16 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) uint32_t max_rx_pkt_len; uint32_t frame_size; - uint16_t rx_buf_size; @@ -56,7 +57,7 @@ + QEDE_ETH_OVERHEAD); return -EINVAL; } -@@ -2266,12 +2265,13 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) +@@ -2540,12 +2539,13 @@ static int qede_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) bufsz = (uint16_t)rte_pktmbuf_data_room_size( fp->rxq->mb_pool) - RTE_PKTMBUF_HEADROOM; - if (dev->data->scattered_rx)