From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D4876A0A02; Thu, 14 Jan 2021 18:14:15 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9E0571413DD; Thu, 14 Jan 2021 18:14:15 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mails.dpdk.org (Postfix) with ESMTP id A41671413DC for ; Thu, 14 Jan 2021 18:14:13 +0100 (CET) IronPort-SDR: fJG0kap+gq87eaEvgqGmDMMHZ50TelgupPbvb4CGvraPLXAwITEma6Y2O1NMqWHBEiGljMcOGS t3a+TPhao6Bw== X-IronPort-AV: E=McAfee;i="6000,8403,9864"; a="157585470" X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="157585470" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jan 2021 09:14:05 -0800 IronPort-SDR: 7hpRT7/j781Opbm2WUoG3WtGDWmktBqonz6AuAk6p9FDvD+XnuDtEOAX/TkYDlLY3Sv9ZJVjOA 4SWn8x2/Sqpw== X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="382335429" Received: from fyigit-mobl1.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 09:14:03 -0800 From: Ferruh Yigit To: Steve Yang , 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> <53a37e85-fdba-ae58-1028-1facc33884dc@intel.com> Message-ID: <96dd6d7a-7af3-2c59-b25f-e9f0cad974d0@intel.com> Date: Thu, 14 Jan 2021 17:13:58 +0000 MIME-Version: 1.0 In-Reply-To: <53a37e85-fdba-ae58-1028-1facc33884dc@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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 1/14/2021 4:36 PM, Ferruh Yigit wrote: > 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 > > <...> > >> @@ -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. > As I tested removing this check is causing problem because some PMDs are using the 'max_rx_pkt_len' even jumbo frame is not set. Perhaps better to keep it, and make a separate patch later to remove this check, after PMDs fixed. >> +    } >> + >> +    /* 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? Can you still send a new version for above change please?