[v2,2/2] drivers/net: fix removing jumbo offload flag
Checks
Commit Message
After DEV_RX_OFFLOAD_JUMBO_FRAME flag removed, drivers give jumbo frame
decisions based on MTU value checks, but some of the checks were wrong
by mistake, causing device initialization to fail, fixing them.
Fixes: b563c1421282 ("ethdev: remove jumbo offload flag")
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Some of the check mistakes are residue of local changes, I remember
fixing them, but it looks like something went wrong while sending
patches.
v2:
* Split doc changes
---
drivers/net/e1000/igb_rxtx.c | 2 +-
drivers/net/i40e/i40e_rxtx.c | 27 +++++++--------------------
drivers/net/iavf/iavf_ethdev.c | 31 ++++++++-----------------------
drivers/net/ice/ice_dcf_ethdev.c | 31 ++++++++-----------------------
drivers/net/ice/ice_rxtx.c | 27 +++++++--------------------
drivers/net/igc/igc_txrx.c | 2 +-
drivers/net/ixgbe/ixgbe_rxtx.c | 2 +-
7 files changed, 33 insertions(+), 89 deletions(-)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: 2021年10月22日 20:57
> To: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Hyong Youb
> Kim <hyonkim@cisco.com>; Michal Krawczyk <mk@semihalf.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Somnath Kotur
> <somnath.kotur@broadcom.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/2] drivers/net: fix removing jumbo offload
> flag
>
> After DEV_RX_OFFLOAD_JUMBO_FRAME flag removed, drivers give jumbo
> frame decisions based on MTU value checks, but some of the checks were
> wrong by mistake, causing device initialization to fail, fixing them.
>
> Fixes: b563c1421282 ("ethdev: remove jumbo offload flag")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Yu Jiang <YuX.Jiang@intel.com>
Verify patchset v2 https://patches.dpdk.org/project/dpdk/patch/20211022125716.2706937-2-ferruh.yigit@intel.com/ on dpdk main branch(f4acb429d0 (HEAD, origin/main) hash: promote some functions to stable)
Tested on i40e(iavf) and it looks good, will test more on others.
On 10/22/2021 3:10 PM, Jiang, YuX wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>> Sent: 2021年10月22日 20:57
>> To: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
>> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
>> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Hyong Youb
>> Kim <hyonkim@cisco.com>; Michal Krawczyk <mk@semihalf.com>; Andrew
>> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Somnath Kotur
>> <somnath.kotur@broadcom.com>
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v2 2/2] drivers/net: fix removing jumbo offload
>> flag
>>
>> After DEV_RX_OFFLOAD_JUMBO_FRAME flag removed, drivers give jumbo
>> frame decisions based on MTU value checks, but some of the checks were
>> wrong by mistake, causing device initialization to fail, fixing them.
>>
>> Fixes: b563c1421282 ("ethdev: remove jumbo offload flag")
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
> Tested-by: Yu Jiang <YuX.Jiang@intel.com>
> Verify patchset v2 https://patches.dpdk.org/project/dpdk/patch/20211022125716.2706937-2-ferruh.yigit@intel.com/ on dpdk main branch(f4acb429d0 (HEAD, origin/main) hash: promote some functions to stable)
> Tested on i40e(iavf) and it looks good, will test more on others.
>
Thanks Yu,
I can see it is not fully validated, and most likely it won't be
before -rc1, but it would be good to have this fix for -rc1 to not
block testing.
Since the previous code is clearly wrong, and fixes are similar
between drivers, if one is good I will take risk and merge
the fix for -rc1.
On 10/22/2021 4:20 PM, Ferruh Yigit wrote:
> On 10/22/2021 3:10 PM, Jiang, YuX wrote:
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>>> Sent: 2021年10月22日 20:57
>>> To: Wang, Haiyue <haiyue.wang@intel.com>; Xing, Beilei
>>> <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
>>> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Hyong Youb
>>> Kim <hyonkim@cisco.com>; Michal Krawczyk <mk@semihalf.com>; Andrew
>>> Rybchenko <andrew.rybchenko@oktetlabs.ru>; Somnath Kotur
>>> <somnath.kotur@broadcom.com>
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
>>> Subject: [dpdk-dev] [PATCH v2 2/2] drivers/net: fix removing jumbo offload
>>> flag
>>>
>>> After DEV_RX_OFFLOAD_JUMBO_FRAME flag removed, drivers give jumbo
>>> frame decisions based on MTU value checks, but some of the checks were
>>> wrong by mistake, causing device initialization to fail, fixing them.
>>>
>>> Fixes: b563c1421282 ("ethdev: remove jumbo offload flag")
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Tested-by: Yu Jiang <YuX.Jiang@intel.com>
>> Verify patchset v2 https://patches.dpdk.org/project/dpdk/patch/20211022125716.2706937-2-ferruh.yigit@intel.com/ on dpdk main branch(f4acb429d0 (HEAD, origin/main) hash: promote some functions to stable)
>> Tested on i40e(iavf) and it looks good, will test more on others.
>>
>
> Thanks Yu,
>
> I can see it is not fully validated, and most likely it won't be
> before -rc1, but it would be good to have this fix for -rc1 to not
> block testing.
> Since the previous code is clearly wrong, and fixes are similar
> between drivers, if one is good I will take risk and merge
> the fix for -rc1.
Series applied to dpdk-next-net/main, thanks.
@@ -2331,7 +2331,7 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
* Configure support of jumbo frames, if any.
*/
max_len = dev->data->mtu + E1000_ETH_OVERHEAD;
- if ((dev->data->mtu & RTE_ETHER_MTU) != 0) {
+ if (dev->data->mtu > RTE_ETHER_MTU) {
rctl |= E1000_RCTL_LPE;
/*
@@ -2901,26 +2901,13 @@ i40e_rx_queue_config(struct i40e_rx_queue *rxq)
rxq->max_pkt_len =
RTE_MIN(hw->func_caps.rx_buf_chain_len * rxq->rx_buf_len,
data->mtu + I40E_ETH_OVERHEAD);
- if (data->mtu > RTE_ETHER_MTU) {
- if (rxq->max_pkt_len <= I40E_ETH_MAX_LEN ||
- rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
- PMD_DRV_LOG(ERR, "maximum packet length must "
- "be larger than %u and smaller than %u,"
- "as jumbo frame is enabled",
- (uint32_t)I40E_ETH_MAX_LEN,
- (uint32_t)I40E_FRAME_SIZE_MAX);
- return I40E_ERR_CONFIG;
- }
- } else {
- if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
- rxq->max_pkt_len > I40E_ETH_MAX_LEN) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is disabled",
- (uint32_t)RTE_ETHER_MIN_LEN,
- (uint32_t)I40E_ETH_MAX_LEN);
- return I40E_ERR_CONFIG;
- }
+ if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
+ rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
+ PMD_DRV_LOG(ERR, "maximum packet length must be "
+ "larger than %u and smaller than %u",
+ (uint32_t)RTE_ETHER_MIN_LEN,
+ (uint32_t)I40E_FRAME_SIZE_MAX);
+ return I40E_ERR_CONFIG;
}
return 0;
@@ -585,29 +585,14 @@ iavf_init_rxq(struct rte_eth_dev *dev, struct iavf_rx_queue *rxq)
rxq->rx_buf_len * IAVF_MAX_CHAINED_RX_BUFFERS,
frame_size);
- /* Check if the jumbo frame and maximum packet length are set
- * correctly.
- */
- if (dev->data->mtu & RTE_ETHER_MTU) {
- if (max_pkt_len <= IAVF_ETH_MAX_LEN ||
- max_pkt_len > IAVF_FRAME_SIZE_MAX) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is enabled",
- (uint32_t)IAVF_ETH_MAX_LEN,
- (uint32_t)IAVF_FRAME_SIZE_MAX);
- return -EINVAL;
- }
- } else {
- if (max_pkt_len < RTE_ETHER_MIN_LEN ||
- max_pkt_len > IAVF_ETH_MAX_LEN) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is disabled",
- (uint32_t)RTE_ETHER_MIN_LEN,
- (uint32_t)IAVF_ETH_MAX_LEN);
- return -EINVAL;
- }
+ /* Check if maximum packet length is set correctly. */
+ if (max_pkt_len <= RTE_ETHER_MIN_LEN ||
+ max_pkt_len > IAVF_FRAME_SIZE_MAX) {
+ PMD_DRV_LOG(ERR, "maximum packet length must be "
+ "larger than %u and smaller than %u",
+ (uint32_t)IAVF_ETH_MAX_LEN,
+ (uint32_t)IAVF_FRAME_SIZE_MAX);
+ return -EINVAL;
}
rxq->max_pkt_len = max_pkt_len;
@@ -69,29 +69,14 @@ ice_dcf_init_rxq(struct rte_eth_dev *dev, struct ice_rx_queue *rxq)
max_pkt_len = RTE_MIN(ICE_SUPPORT_CHAIN_NUM * rxq->rx_buf_len,
dev->data->mtu + ICE_ETH_OVERHEAD);
- /* Check if the jumbo frame and maximum packet length are set
- * correctly.
- */
- if (dev_data->mtu > RTE_ETHER_MTU) {
- if (max_pkt_len <= ICE_ETH_MAX_LEN ||
- max_pkt_len > ICE_FRAME_SIZE_MAX) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is enabled",
- (uint32_t)ICE_ETH_MAX_LEN,
- (uint32_t)ICE_FRAME_SIZE_MAX);
- return -EINVAL;
- }
- } else {
- if (max_pkt_len < RTE_ETHER_MIN_LEN ||
- max_pkt_len > ICE_ETH_MAX_LEN) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is disabled",
- (uint32_t)RTE_ETHER_MIN_LEN,
- (uint32_t)ICE_ETH_MAX_LEN);
- return -EINVAL;
- }
+ /* Check maximum packet length is set correctly. */
+ if (max_pkt_len <= RTE_ETHER_MIN_LEN ||
+ max_pkt_len > ICE_FRAME_SIZE_MAX) {
+ PMD_DRV_LOG(ERR, "maximum packet length must be "
+ "larger than %u and smaller than %u",
+ (uint32_t)RTE_ETHER_MIN_LEN,
+ (uint32_t)ICE_FRAME_SIZE_MAX);
+ return -EINVAL;
}
rxq->max_pkt_len = max_pkt_len;
@@ -281,26 +281,13 @@ ice_program_hw_rx_queue(struct ice_rx_queue *rxq)
RTE_MIN((uint32_t)ICE_SUPPORT_CHAIN_NUM * rxq->rx_buf_len,
frame_size);
- if (dev_data->mtu > RTE_ETHER_MTU) {
- if (rxq->max_pkt_len <= ICE_ETH_MAX_LEN ||
- rxq->max_pkt_len > ICE_FRAME_SIZE_MAX) {
- PMD_DRV_LOG(ERR, "maximum packet length must "
- "be larger than %u and smaller than %u,"
- "as jumbo frame is enabled",
- (uint32_t)ICE_ETH_MAX_LEN,
- (uint32_t)ICE_FRAME_SIZE_MAX);
- return -EINVAL;
- }
- } else {
- if (rxq->max_pkt_len < RTE_ETHER_MIN_LEN ||
- rxq->max_pkt_len > ICE_ETH_MAX_LEN) {
- PMD_DRV_LOG(ERR, "maximum packet length must be "
- "larger than %u and smaller than %u, "
- "as jumbo frame is disabled",
- (uint32_t)RTE_ETHER_MIN_LEN,
- (uint32_t)ICE_ETH_MAX_LEN);
- return -EINVAL;
- }
+ if (rxq->max_pkt_len <= RTE_ETHER_MIN_LEN ||
+ rxq->max_pkt_len > ICE_FRAME_SIZE_MAX) {
+ PMD_DRV_LOG(ERR, "maximum packet length must "
+ "be larger than %u and smaller than %u",
+ (uint32_t)RTE_ETHER_MIN_LEN,
+ (uint32_t)ICE_FRAME_SIZE_MAX);
+ return -EINVAL;
}
if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
@@ -1080,7 +1080,7 @@ igc_rx_init(struct rte_eth_dev *dev)
IGC_WRITE_REG(hw, IGC_RCTL, rctl & ~IGC_RCTL_EN);
/* Configure support of jumbo frames, if any. */
- if (dev->data->mtu & RTE_ETHER_MTU)
+ if (dev->data->mtu > RTE_ETHER_MTU)
rctl |= IGC_RCTL_LPE;
else
rctl &= ~IGC_RCTL_LPE;
@@ -5078,7 +5078,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
/*
* Configure jumbo frame support, if any.
*/
- if ((dev->data->mtu & RTE_ETHER_MTU) != 0) {
+ if (dev->data->mtu > RTE_ETHER_MTU) {
hlreg0 |= IXGBE_HLREG0_JUMBOEN;
maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS);
maxfrs &= 0x0000FFFF;