From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id BCF70A0A02;
Thu, 14 Jan 2021 17:37:05 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 2E2551413AA;
Thu, 14 Jan 2021 17:37:05 +0100 (CET)
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
by mails.dpdk.org (Postfix) with ESMTP id D002D1413A9
for <dev@dpdk.org>; Thu, 14 Jan 2021 17:37:03 +0100 (CET)
IronPort-SDR: MqlBAYTYU2gY98aHLjOXsTvx9jV/ki3Kf20P1CxIYSC+/XR0z4nf571nYKn8RdlHIqoMpRQgV5
AuZHmQNhC3uw==
X-IronPort-AV: E=McAfee;i="6000,8403,9864"; a="197052297"
X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="197052297"
Received: from orsmga008.jf.intel.com ([10.7.209.65])
by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
14 Jan 2021 08:36:56 -0800
IronPort-SDR: TtbiueyZ8Jp7W+Br80Hn0eV0EveBW8ZJFD2D0HXtwgVNWNH1X41xn2bGsFyTxB4JhAEIlEOLKW
DHPTkPHdaHIA==
X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="382323149"
Received: from mmcmulla-mobl.ger.corp.intel.com (HELO [10.252.31.191])
([10.252.31.191])
by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
14 Jan 2021 08:36:53 -0800
To: Steve Yang <stevex.yang@intel.com>, dev@dpdk.org
Cc: thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru, oulijun@huawei.com
References: <20201217092312.27033-1-stevex.yang@intel.com>
<20210114094537.13661-1-stevex.yang@intel.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <53a37e85-fdba-ae58-1028-1facc33884dc@intel.com>
Date: Thu, 14 Jan 2021 16:36:49 +0000
MIME-Version: 1.0
In-Reply-To: <20210114094537.13661-1-stevex.yang@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v3 01/22] ethdev: fix MTU size exceeds max rx
packet length
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
On 1/14/2021 9:45 AM, Steve Yang wrote:
> Ethdev is using default Ethernet overhead to decide if provided
> 'max_rx_pkt_len' value is bigger than max (non jumbo) MTU value,
> and limits it to MAX if it is.
>
> Since the application/driver used Ethernet overhead is different than
> the ethdev one, check result is wrong.
>
> If the driver is using Ethernet overhead bigger than the default one,
> the provided 'max_rx_pkt_len' is trimmed down, and in the driver when
> correct Ethernet overhead is used to convert back, the resulting MTU is
> less than the intended one, causing some packets to be dropped.
>
> Like,
> app -> max_rx_pkt_len = 1500/*mtu*/ + 22/*overhead*/ = 1522
> ethdev -> 1522 > 1518/*MAX*/; max_rx_pkt_len = 1518
> driver -> MTU = 1518 - 22 = 1496
> Packets with size 1497-1500 are dropped although intention is to be able
> to send/receive them.
>
> The fix is to make ethdev use the correct Ethernet overhead for port,
> instead of default one.
>
> Fixes: 59d0ecdbf0e1 ("ethdev: MTU accessors")
>
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
<...>
> @@ -1410,11 +1422,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
> goto rollback;
> }
> } else {
> - if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN ||
> - dev_conf->rxmode.max_rx_pkt_len > RTE_ETHER_MAX_LEN)
> + uint16_t pktlen = dev_conf->rxmode.max_rx_pkt_len;
> + if (pktlen < RTE_ETHER_MIN_MTU + overhead_len ||
> + pktlen > RTE_ETHER_MTU + overhead_len)
> /* Use default value */
> dev->data->dev_conf.rxmode.max_rx_pkt_len =
> - RTE_ETHER_MAX_LEN;
> + RTE_ETHER_MTU + overhead_len;
What do you think removing the above check, the else block, completely?
Since the 'max_rx_pkt_len' should not be used when jumbo frame is not set.
> + }
> +
> + /* Scale the MTU size to adapt max_rx_pkt_len */
> + if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) {
> + dev->data->mtu = dev->data->dev_conf.rxmode.max_rx_pkt_len -
> + overhead_len;
> }
Above if block has exact same check, why not move it above block?