Hi Helin, > -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Vlad Zolotarov > Sent: Friday, August 14, 2015 1:38 PM > To: Zhang, Helin; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 for > all NICs but 82598 > > > > On 08/13/15 23:28, Zhang, Helin wrote: > > Hi Vlad > > > > I don't think the changes are needed. It says in datasheet that the RS > > bit should be set on the last descriptor of every packet, ONLY WHEN > TXDCTL.WTHRESH equals to ZERO. > > Of course it's needed! See below. > Exactly the same spec a few lines above the place u've just quoted states: > > "Software should not set the RS bit when TXDCTL.WTHRESH is greater than > zero." > > And since all three (3) ixgbe xmit callbacks are utilizing RS bit notation > ixgbe PMD > is actually not supporting any value of WTHRESH different from zero. I think Vlad is right. We need to fix this issue. Any suggestion? If not, I'd like to ack this patch.
> > > > > Regards, > > Helin > > > >> -----Original Message----- > >> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com] > >> Sent: Thursday, August 13, 2015 11:07 AM > >> To: dev at dpdk.org > >> Cc: Zhang, Helin; Ananyev, Konstantin; avi at cloudius-systems.com; Vlad > >> Zolotarov > >> Subject: [dpdk-dev] [PATCH v1] ixgbe_pmd: forbid tx_rs_thresh above 1 for > all > >> NICs but 82598 > >> > >> According to 82599 and x540 HW specifications RS bit *must* be set in the > last > >> descriptor of *every* packet. > > There is a condition that if TXDCTL.WTHRESH equal to zero. > > Right and ixgbe PMD requires this condition to be fulfilled in order to > function. See above. > > > > >> This patch fixes the Tx hang we were constantly hitting with a > >> seastar-based > >> application on x540 NIC. > > Could you help to share with us how to reproduce the tx hang issue, with > > using > > typical DPDK examples? > > Sorry. I'm not very familiar with the typical DPDK examples to help u > here. However this is quite irrelevant since without this this patch > ixgbe PMD obviously abuses the HW spec as has been explained above. > > We saw the issue when u stressed the xmit path with a lot of highly > fragmented TCP frames (packets with up to 33 fragments with non-headers > fragments as small as 4 bytes) with all offload features enabled. > > Thanks, > vlad > > > >> Signed-off-by: Vlad Zolotarov <vladz at cloudius-systems.com> > >> --- > >> drivers/net/ixgbe/ixgbe_ethdev.c | 9 +++++++++ > >> drivers/net/ixgbe/ixgbe_rxtx.c | 23 ++++++++++++++++++++++- > >> 2 files changed, 31 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > >> b/drivers/net/ixgbe/ixgbe_ethdev.c > >> index b8ee1e9..6714fd9 100644 > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > >> @@ -2414,6 +2414,15 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, > struct > >> rte_eth_dev_info *dev_info) > >> .txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS | > >> ETH_TXQ_FLAGS_NOOFFLOADS, > >> }; > >> + > >> + /* > >> + * According to 82599 and x540 specifications RS bit *must* be set on > the > >> + * last descriptor of *every* packet. Therefore we will not allow the > >> + * tx_rs_thresh above 1 for all NICs newer than 82598. > >> + */ > >> + if (hw->mac.type > ixgbe_mac_82598EB) > >> + dev_info->default_txconf.tx_rs_thresh = 1; > >> + > >> dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t); > >> dev_info->reta_size = ETH_RSS_RETA_SIZE_128; > >> dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL; diff -- > git > >> a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c index > >> 91023b9..8dbdffc 100644 > >> --- a/drivers/net/ixgbe/ixgbe_rxtx.c > >> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > >> @@ -2085,11 +2085,19 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > >> *dev, > >> struct ixgbe_tx_queue *txq; > >> struct ixgbe_hw *hw; > >> uint16_t tx_rs_thresh, tx_free_thresh; > >> + bool rs_deferring_allowed; > >> > >> PMD_INIT_FUNC_TRACE(); > >> hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > >> > >> /* > >> + * According to 82599 and x540 specifications RS bit *must* be set on > the > >> + * last descriptor of *every* packet. Therefore we will not allow the > >> + * tx_rs_thresh above 1 for all NICs newer than 82598. > >> + */ > >> + rs_deferring_allowed = (hw->mac.type <= ixgbe_mac_82598EB); > >> + > >> + /* > >> * Validate number of transmit descriptors. > >> * It must not exceed hardware maximum, and must be multiple > >> * of IXGBE_ALIGN. > >> @@ -2110,6 +2118,8 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > *dev, > >> * to transmit a packet is greater than the number of free TX > >> * descriptors. > >> * The following constraints must be satisfied: > >> + * tx_rs_thresh must be less than 2 for NICs for which RS deferring is > >> + * forbidden (all but 82598). > >> * tx_rs_thresh must be greater than 0. > >> * tx_rs_thresh must be less than the size of the ring minus 2. > >> * tx_rs_thresh must be less than or equal to tx_free_thresh. > >> @@ -2121,9 +2131,20 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev > *dev, > >> * When set to zero use default values. > >> */ > >> tx_rs_thresh = (uint16_t)((tx_conf->tx_rs_thresh) ? > >> - tx_conf->tx_rs_thresh : DEFAULT_TX_RS_THRESH); > >> + tx_conf->tx_rs_thresh : > >> + (rs_deferring_allowed ? DEFAULT_TX_RS_THRESH : 1)); > >> tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ? > >> tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH); > >> + > >> + if (!rs_deferring_allowed && tx_rs_thresh > 1) { > >> + PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than 2 since RS > " > >> + "must be set for every packet for this HW. " > >> + "(tx_rs_thresh=%u port=%d queue=%d)", > >> + (unsigned int)tx_rs_thresh, > >> + (int)dev->data->port_id, (int)queue_idx); > >> + return -(EINVAL); > >> + } > >> + > >> if (tx_rs_thresh >= (nb_desc - 2)) { > >> PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than the > number " > >> "of TX descriptors minus 2. (tx_rs_thresh=%u " > >> -- > >> 2.1.0
