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 3510CA0C4E; Thu, 22 Jul 2021 12:12:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AAB584014D; Thu, 22 Jul 2021 12:12:23 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 36BC940040 for ; Thu, 22 Jul 2021 12:12:22 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10052"; a="272728141" X-IronPort-AV: E=Sophos;i="5.84,260,1620716400"; d="scan'208";a="272728141" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jul 2021 03:12:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,260,1620716400"; d="scan'208";a="658891896" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by fmsmga006.fm.intel.com with ESMTP; 22 Jul 2021 03:12:20 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Thu, 22 Jul 2021 03:12:20 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10; Thu, 22 Jul 2021 03:12:19 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.10 via Frontend Transport; Thu, 22 Jul 2021 03:12:19 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.106) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.10; Thu, 22 Jul 2021 03:12:19 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Nwr7G8f2HyxFkycWgE6hu7fygffyjwQYWY3w9kPa5YIgsOOdTvvy50QpnkhrXCWd6wVY0Gd5EINAhzfCYpJJe2ivBLFKhU1s8kg8j4IregdzAFE6cJxSFK4enf/7oQQ9SlTUjpOq1KSEVpYyRlDaSnQX3qR49/c4s6FsetJIeu/6/CKKqG+Sm2uoycv6OjYr5KB+w333SV9VBJMKLdfbiyiuk5+VcIiCh425OYVcdu8IVbz453jcas2ClQN1sKFYjG8TT79OCbhB9alQYdOLp4R+4ubxEPH9JjvPAqspIaR3078J3SGlfdy39onZn+wSyFoLF7qEhJVD7yhAibpdcA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NUVHvjhXFkUfeOi14Newl3uVbVpJeI4E8uGMUiA62yU=; b=ekIrbqEiaQ1QaPBZfvmnkZiiXr1qiWC33PNqpIxyNVaVizj1Hf+cKfb8WHi2syjRo1SyVKOIhgjJJKeqrr459evhADt3UNRdEkTOvbbxEFvLVIrjS7HsGa2CTqvgbXMYPD5AJADp3NuMxyxbRDs6wffnROGDnyG+pAvA/JcyrbCOfdDy66eNMw8mb7D2tG8ksNdtbSFIO5xdqTLMt5AhJDopwXUXCoDMyxJ9smEBb9Uec0NXKhrLiX2R4lQIDCgwCNTM7oKZ131DpXDRMsf+0MnLRSetMejSU6rAINIkqRgyEVGDw3u21CXBvlpLSd9cGbVMOIVEAb2ZyRUYi88Rjw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=NUVHvjhXFkUfeOi14Newl3uVbVpJeI4E8uGMUiA62yU=; b=dtS9ZYhnwDputeUBdyiHtxY3jCR0WvkFfAlUuQn/wfehzQQ6/eYT76ncPt47tsj6XgT4g+ORXt1Y1/qgnF/8niWh1S7pMc8srcDdfh3OCJm2BQNURb11nxXbmhIP1HNunYFeLZt20iUtikv6o0H59M7U36P7kxIDtHgLpVrOZJw= Authentication-Results: dpdk.org; dkim=none (message not signed) header.d=none;dpdk.org; dmarc=none action=none header.from=intel.com; Received: from PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) by PH0PR11MB4822.namprd11.prod.outlook.com (2603:10b6:510:39::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4331.24; Thu, 22 Jul 2021 10:12:16 +0000 Received: from PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::bde5:66de:e755:c5bb]) by PH0PR11MB5000.namprd11.prod.outlook.com ([fe80::bde5:66de:e755:c5bb%5]) with mapi id 15.20.4352.026; Thu, 22 Jul 2021 10:12:16 +0000 To: Huisong Li CC: "dev@dpdk.org" References: <20210709172923.3369846-1-ferruh.yigit@intel.com> <1a891390-1122-6dcf-03e8-1f0b147b30ec@huawei.com> <4e1ae26c-197b-ea45-0860-66c195d1f820@intel.com> <44ea055b-4f43-5cd5-9911-662b6df51623@huawei.com> From: Ferruh Yigit X-User: ferruhy Message-ID: <78252f7e-3170-344c-fba9-85fcacc36026@intel.com> Date: Thu, 22 Jul 2021 11:12:10 +0100 In-Reply-To: <44ea055b-4f43-5cd5-9911-662b6df51623@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-ClientProxiedBy: PR0P264CA0070.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1d::34) To PH0PR11MB5000.namprd11.prod.outlook.com (2603:10b6:510:41::19) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.0.206] (37.228.236.146) by PR0P264CA0070.FRAP264.PROD.OUTLOOK.COM (2603:10a6:100:1d::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4352.25 via Frontend Transport; Thu, 22 Jul 2021 10:12:15 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 26c3126e-6790-4c97-62ab-08d94cf92e73 X-MS-TrafficTypeDiagnostic: PH0PR11MB4822: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:5236; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: h/ktZicNDQH20bhzfR/oUcB1+HLLnBWAvezdmk/igB3Q0+5o5I092vbQjDg69bbja9YjjYhvdZWncVALXlzFT8bl/r8kHNVcR2wlllVwht/qv3bxWKahx1poTti7Iopo9vI1vEN1IXQk/tUHzUM8yOGacQ5ueoAvHltnZyw+P8eYL4lFXssTk3zjGmdM69c4R8KzhzoUWLxYxcxmWw4DqBSk1Wo2A0kXn1QxIzCuRxqHiSxTAI0hNiR04gD/9Je7wwqVCiec7E7dsu0/HnKGD/fG6Dwx5jIJUvnqNL0Fi+X9mUisFQ18pt4mYDxc+UuPBjoonmdnuGUR3ENFSRbb5VJE1fmGTMXErYKHQjgyy9bPmPS4QxgK1/dDSSNFppsMfh/fTixKzX5BnvqsYmZQQUjaYLqS9bFtum+hoOKjRNMmCImZPi71zQq2eBxwKbUOL4pa7jL7pEHFcLuE8Xu9CxsNWCRHZK5SwOyZy6GocvFKI2/dhYMIgB0Q7or36pdmu24x/izICpbURMy5sS8cz3ucNlqG1vJhtQyo0WVQZEzUTFCcm/4HrSt9uMy+RaPae0a6pRTFpmdt2dG4rciQesdH30DaM3hTjNPYeiYv66CYDk5VGednmS5JCeQc9OSmmL3droG5nd2F22kEA+fkqzvJq9mUToMR7t+9dZA/7/voqu8Ft7Y/pE3I/y+oeYP7BSrBkvglozHmlDM48mzp6w== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR11MB5000.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(39860400002)(136003)(346002)(396003)(376002)(2616005)(6666004)(956004)(38100700002)(16576012)(8936002)(186003)(30864003)(53546011)(83380400001)(6916009)(86362001)(44832011)(26005)(6486002)(478600001)(8676002)(2906002)(4326008)(31696002)(31686004)(5660300002)(316002)(66556008)(36756003)(66476007)(66946007)(45980500001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dmFBdDY3c0t1ZlRsbHFaalY4QkFCSU45K1R3bnJ6T0ppVWxza1NVTDRKSFow?= =?utf-8?B?eElPYVUxVlRJQXdTN0x0amNBMkNTY0ZyQkZWdDF4RGdnWDltZDZDR041VDVv?= =?utf-8?B?c244cVVFYW5nLyttVEtlTm5RN2Y5YXUzUVg5ZFlxTVJTRjVHV1gzbWtwRkdO?= =?utf-8?B?dW1yTDU3WklDeFJIMFlvUlRDUE15TUNBQTBZbDBBak9sNi9OdEJIQlZxaGcx?= =?utf-8?B?RXVLRjFPcVhMV0t1bGVMaVB5WjRLTnJlSTV0cTNkaHg3YW41STRMbEpDc011?= =?utf-8?B?c3RLbk5SU3lKdlRJMk5zUmNQeFE0akJSODlrTXpSTll5c012cHRUdWNPaWZt?= =?utf-8?B?S1ZMZEdpMnM1alNIR2dxczRRTzF0aE1ILzRQM1ptNlh3RXNIN3RQS211a2x2?= =?utf-8?B?a2EvM1M2ZFg4ZzF4cllPRVM1RnNFVDVscDVjZnFHT2lZMzdkYUJodXRESWFT?= =?utf-8?B?dHJMUXlOMGJjUk1xWURnZHcxaXBOdW94ZVlTbmlmeVUreGRtd3p3NWhhUGhT?= =?utf-8?B?RXJXNlpXbzQ5OEszQ29mbzJnZHZWWVNsOFNNTFZVR08yTTNIREpaTFpvV1hW?= =?utf-8?B?SjgzbEtmOVJ0cVo2WlVyVW1Fd2gzQTRVbTk3TGtFYmpNcW5laTBBZ3k0NXV3?= =?utf-8?B?MitSNXZ4Y3ByTU5MVkJRR0pvUXBLc2Fmd1pBTW81L0svNEl2ZHEyb3F2ZWpk?= =?utf-8?B?NWFUcWM2UWZGc0pHcjV6UkdyMFVTdU40TnRYLzFpRWZxR2xxalRraUNsY3lr?= =?utf-8?B?aXN1V0ZJdjdWS056ZHFNbjlEQndLaUszYmVWSXNidVhsNDRGeFFMNDd5UzVw?= =?utf-8?B?ZDJBM0cyNEpZZVdnUmh4OHZEbDJVOGFWS2x6RkRnalJiemtyN3M4b3p4a2F1?= =?utf-8?B?dDZ4eThWbXE4NjdtcE53ZGlja3JXeG5mODl3MThZdFphdkZFN2szVzVsaVFL?= =?utf-8?B?c2YyYWRvbGdzYnNxUlBGc0JFVzBRanhDZVdIeEY5ODlodUo5dVlWbys5QWh3?= =?utf-8?B?TEN4Qi9pY2lycm9zaHRJZWlBYVN2RjcwNDJuTmF3K1VZZjhnb0xiYnBuTjk2?= =?utf-8?B?eFBGZ3NWeExrald4YnFvYS85R282U0JuWjdwbTJWM1pQQ1JGRHM1bnJXZFBR?= =?utf-8?B?R284b01tTS9CVGtrME5TTkhHeFlsaFA1dGk2SmJyREwxQ2NqNWFpeWh2dyt6?= =?utf-8?B?THpwUks0T0RIbHZVR2JEUUFYUDFubUs3dHNxUXNuU2toWUFOUmFGK3padWhh?= =?utf-8?B?eVZsWXZiUWh1SjJ5WDVmMmpOV3k1aTNhcGlXWTVITnorM0ErMXJtemdnb3R5?= =?utf-8?B?R0hjUVZEc1BNU29nbTNTWCtUeGYxUXVzdldRRzlvclIrWlZjdldKNnkza2ZC?= =?utf-8?B?T3B5K1pEeW0xbVBaSWIxeGVUSGZEY1RpdTdBalhYd2luTmNVdVJMaHJPRk1U?= =?utf-8?B?YzFteitOZS95eXBKYWRZaWtpcXBjd3Bqc3F2OERNSHoxTUFNZUt5NE5SVlBO?= =?utf-8?B?b05NU1FQaXdDdmpJektMREY0ZFNoQVpRUThuVitZcnRodXgzN3VmWHJGSWdh?= =?utf-8?B?a2RVbTZ1dUpqMDdxbXpqMGlTaU5oQ0JhdXJkTStaaEZYVjJ5MEF1aHJYNk42?= =?utf-8?B?VkNIZjdjNU55eEdpQ2hZeFVmMGNIL21pUWFyQVJkRjllNzJ1Z3A1S283RThT?= =?utf-8?B?NThzM09Yb2YraWtYdzNJQ3QrV0hSYWhhN0Vkb3lvYW00QWN6YWFFRFhudmg2?= =?utf-8?Q?twoljHb6F+nXXUCKWKHBys9hgcBu+OWGsKHNi9Q?= X-MS-Exchange-CrossTenant-Network-Message-Id: 26c3126e-6790-4c97-62ab-08d94cf92e73 X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5000.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jul 2021 10:12:16.1409 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 7rx2LnxVJFmJIzjk7PzYEvvGSvfVnTwwdfGQdS6k1OLYSpUkhEh8TZ1pq4OXAADN0tJjjxKTJlzeg8G6dWqqSA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4822 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH 1/4] ethdev: fix 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 7/22/2021 8:21 AM, Huisong Li wrote: > > 在 2021/7/21 23:29, Ferruh Yigit 写道: >> On 7/19/2021 4:35 AM, Huisong Li wrote: >>> Hi, Ferruh >>> >> Hi Huisong, >> >> Thanks for the review. >> >>> 在 2021/7/10 1:29, Ferruh Yigit 写道: >>>> There is a confusion on setting max Rx packet length, this patch aims to >>>> clarify it. >>>> >>>> 'rte_eth_dev_configure()' API accepts max Rx packet size via >>>> 'uint32_t max_rx_pkt_len' filed of the config struct 'struct >>>> rte_eth_conf'. >>>> >>>> Also 'rte_eth_dev_set_mtu()' API can be used to set the MTU, and result >>>> stored into '(struct rte_eth_dev)->data->mtu'. >>>> >>>> These two APIs are related but they work in a disconnected way, they >>>> store the set values in different variables which makes hard to figure >>>> out which one to use, also two different related method is confusing for >>>> the users. >>>> >>>> Other issues causing confusion is: >>>> * maximum transmission unit (MTU) is payload of the Ethernet frame. And >>>>     'max_rx_pkt_len' is the size of the Ethernet frame. Difference is >>>>     Ethernet frame overhead, but this may be different from device to >>>>     device based on what device supports, like VLAN and QinQ. >>>> * 'max_rx_pkt_len' is only valid when application requested jumbo frame, >>>>     which adds additional confusion and some APIs and PMDs already >>>>     discards this documented behavior. >>>> * For the jumbo frame enabled case, 'max_rx_pkt_len' is an mandatory >>>>     field, this adds configuration complexity for application. >>>> >>>> As solution, both APIs gets MTU as parameter, and both saves the result >>>> in same variable '(struct rte_eth_dev)->data->mtu'. For this >>>> 'max_rx_pkt_len' updated as 'mtu', and it is always valid independent >>>> from jumbo frame. >>>> >>>> For 'rte_eth_dev_configure()', 'dev->data->dev_conf.rxmode.mtu' is user >>>> request and it should be used only within configure function and result >>>> should be stored to '(struct rte_eth_dev)->data->mtu'. After that point >>>> both application and PMD uses MTU from this variable. >>>> >>>> When application doesn't provide an MTU during 'rte_eth_dev_configure()' >>>> default 'RTE_ETHER_MTU' value is used. >>>> >>>> As additional clarification, MTU is used to configure the device for >>>> physical Rx/Tx limitation. Other related issue is size of the buffer to >>>> store Rx packets, many PMDs use mbuf data buffer size as Rx buffer size. >>>> And compares MTU against Rx buffer size to decide enabling scattered Rx >>>> or not, if PMD supports it. If scattered Rx is not supported by device, >>>> MTU bigger than Rx buffer size should fail. >>>> >>>> Signed-off-by: Ferruh Yigit >> <...> >> >>>> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c >>>> index e51512560e15..8bccdeddb2f7 100644 >>>> --- a/drivers/net/hns3/hns3_ethdev.c >>>> +++ b/drivers/net/hns3/hns3_ethdev.c >>>> @@ -2379,20 +2379,11 @@ hns3_refresh_mtu(struct rte_eth_dev *dev, struct >>>> rte_eth_conf *conf) >>>>    { >>>>        struct hns3_adapter *hns = dev->data->dev_private; >>>>        struct hns3_hw *hw = &hns->hw; >>>> -    uint32_t max_rx_pkt_len; >>>> -    uint16_t mtu; >>>> -    int ret; >>>> - >>>> -    if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME)) >>>> -        return 0; >>>> +    uint32_t max_rx_pktlen; >>>>    -    /* >>>> -     * If jumbo frames are enabled, MTU needs to be refreshed >>>> -     * according to the maximum RX packet length. >>>> -     */ >>>> -    max_rx_pkt_len = conf->rxmode.max_rx_pkt_len; >>>> -    if (max_rx_pkt_len > HNS3_MAX_FRAME_LEN || >>>> -        max_rx_pkt_len <= HNS3_DEFAULT_FRAME_LEN) { >>>> +    max_rx_pktlen = conf->rxmode.mtu + HNS3_ETH_OVERHEAD; >>>> +    if (max_rx_pktlen > HNS3_MAX_FRAME_LEN || >>>> +        max_rx_pktlen <= HNS3_DEFAULT_FRAME_LEN) { >>>>            hns3_err(hw, "maximum Rx packet length must be greater than %u " >>>>                 "and no more than %u when jumbo frame enabled.", >>>>                 (uint16_t)HNS3_DEFAULT_FRAME_LEN, >>> The preceding check for the maximum frame length was based on the scenario where >>> jumbo frames are enabled. >>> >>> Since there is no offload of jumbo frames in this patchset, the maximum frame >>> length does not need to be checked and only ensure conf->rxmode.mtu is valid. >>> >>> These should be guaranteed by dev_configure() in the framework . >>> >> Got it, agree that 'HNS3_DEFAULT_FRAME_LEN' check is now wrong, and as you said >> these checks are becoming redundant, so I will remove them. >> >> In that case 'hns3_refresh_mtu()' becomes just wrapper to 'hns3_dev_mtu_set()', >> I will remove function too. >> >> <...> > ok >> >>>> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c >>>> b/drivers/net/hns3/hns3_ethdev_vf.c >>>> index e582503f529b..ca839fa55fa0 100644 >>>> --- a/drivers/net/hns3/hns3_ethdev_vf.c >>>> +++ b/drivers/net/hns3/hns3_ethdev_vf.c >>>> @@ -784,8 +784,7 @@ hns3vf_dev_configure(struct rte_eth_dev *dev) >>>>        uint16_t nb_rx_q = dev->data->nb_rx_queues; >>>>        uint16_t nb_tx_q = dev->data->nb_tx_queues; >>>>        struct rte_eth_rss_conf rss_conf; >>>> -    uint32_t max_rx_pkt_len; >>>> -    uint16_t mtu; >>>> +    uint32_t max_rx_pktlen; >>>>        bool gro_en; >>>>        int ret; >>>>    @@ -825,29 +824,21 @@ hns3vf_dev_configure(struct rte_eth_dev *dev) >>>>                goto cfg_err; >>>>        } >>>>    -    /* >>>> -     * If jumbo frames are enabled, MTU needs to be refreshed >>>> -     * according to the maximum RX packet length. >>>> -     */ >>>> -    if (conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) { >>>> -        max_rx_pkt_len = conf->rxmode.max_rx_pkt_len; >>>> -        if (max_rx_pkt_len > HNS3_MAX_FRAME_LEN || >>>> -            max_rx_pkt_len <= HNS3_DEFAULT_FRAME_LEN) { >>>> -            hns3_err(hw, "maximum Rx packet length must be greater " >>>> -                 "than %u and less than %u when jumbo frame enabled.", >>>> -                 (uint16_t)HNS3_DEFAULT_FRAME_LEN, >>>> -                 (uint16_t)HNS3_MAX_FRAME_LEN); >>>> -            ret = -EINVAL; >>>> -            goto cfg_err; >>>> -        } >>>> - >>>> -        mtu = (uint16_t)HNS3_PKTLEN_TO_MTU(max_rx_pkt_len); >>>> -        ret = hns3vf_dev_mtu_set(dev, mtu); >>>> -        if (ret) >>>> -            goto cfg_err; >>>> -        dev->data->mtu = mtu; >>>> +    max_rx_pktlen = conf->rxmode.mtu + HNS3_ETH_OVERHEAD; >>>> +    if (max_rx_pktlen > HNS3_MAX_FRAME_LEN || >>>> +        max_rx_pktlen <= HNS3_DEFAULT_FRAME_LEN) { >>>> +        hns3_err(hw, "maximum Rx packet length must be greater " >>>> +             "than %u and less than %u when jumbo frame enabled.", >>>> +             (uint16_t)HNS3_DEFAULT_FRAME_LEN, >>>> +             (uint16_t)HNS3_MAX_FRAME_LEN); >>>> +        ret = -EINVAL; >>>> +        goto cfg_err; >>>>        } >>> Please remove this check now, thanks! >> ack >> >> <...> >> >>>> diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c >>>> index ce8882a45883..f868e5d906c7 100644 >>>> --- a/examples/ip_reassembly/main.c >>>> +++ b/examples/ip_reassembly/main.c >>>> @@ -162,7 +162,7 @@ static struct lcore_queue_conf >>>> lcore_queue_conf[RTE_MAX_LCORE]; >>>>    static struct rte_eth_conf port_conf = { >>>>        .rxmode = { >>>>            .mq_mode        = ETH_MQ_RX_RSS, >>>> -        .max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE, >>>> +        .mtu = JUMBO_FRAME_MAX_SIZE, >>> It feel likes that the replacement of max_rx_pkt_len with MTU is inappropriate. >>> >>> Because "max_rx_pkt_len " is the sum of  "mtu" and "overhead_len". >> You are right, it is not same thing. I will update it to remove overhead. >> >> <...> >> >>>> @@ -1448,49 +1459,45 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t >>>> nb_rx_q, uint16_t nb_tx_q, >>>>        } >>>>          /* >>>> -     * If jumbo frames are enabled, check that the maximum RX packet >>>> -     * length is supported by the configured device. >>>> +     * Check that the maximum RX packet length is supported by the >>>> +     * configured device. >>>>         */ >>>> -    if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) { >>>> -        if (dev_conf->rxmode.max_rx_pkt_len > dev_info.max_rx_pktlen) { >>>> -            RTE_ETHDEV_LOG(ERR, >>>> -                "Ethdev port_id=%u max_rx_pkt_len %u > max valid value %u\n", >>>> -                port_id, dev_conf->rxmode.max_rx_pkt_len, >>>> -                dev_info.max_rx_pktlen); >>>> -            ret = -EINVAL; >>>> -            goto rollback; >>>> -        } else if (dev_conf->rxmode.max_rx_pkt_len < RTE_ETHER_MIN_LEN) { >>>> -            RTE_ETHDEV_LOG(ERR, >>>> -                "Ethdev port_id=%u max_rx_pkt_len %u < min valid value %u\n", >>>> -                port_id, dev_conf->rxmode.max_rx_pkt_len, >>>> -                (unsigned int)RTE_ETHER_MIN_LEN); >>>> -            ret = -EINVAL; >>>> -            goto rollback; >>>> -        } >>>> +    if (dev_conf->rxmode.mtu == 0) >>>> +        dev->data->dev_conf.rxmode.mtu = RTE_ETHER_MTU; >>> Here, it will cause a case that the user configuration is inconsistent with the >>> configuration saved in the framework . >> What is the framework you mentioned? >> >> Previously 'max_rx_pkt_len' was mandatory when jumbo frame is configured even >> user doesn't really case about it and it was causing additional complexity in >> the configuration. >> This check is required to use defaults when application doesn't need a specific >> value, I believe this is a good usability improvement. >> Application who cares about a specific value can set it explicitly and it will >> be in sync with application. >> >>> Is it more reasonable to provide a prompt message? >> Not sure about it. We are not changing a user configured value, but using >> default value when application doesn't set it, and that kind of log will be >> printed by most of the applications, this may cause noise. > This is a good reason. >> >>>> +    max_rx_pktlen = dev->data->dev_conf.rxmode.mtu + overhead_len; >>>> +    if (max_rx_pktlen > dev_info.max_rx_pktlen) { >>>> +        RTE_ETHDEV_LOG(ERR, >>>> +            "Ethdev port_id=%u max_rx_pktlen %u > max valid value %u\n", >>>> +            port_id, max_rx_pktlen, dev_info.max_rx_pktlen); >>>> +        ret = -EINVAL; >>>> +        goto rollback; >>>> +    } else if (max_rx_pktlen < RTE_ETHER_MIN_LEN) { >>>> +        RTE_ETHDEV_LOG(ERR, >>>> +            "Ethdev port_id=%u max_rx_pktlen %u < min valid value %u\n", >>>> +            port_id, max_rx_pktlen, RTE_ETHER_MIN_LEN); >>>> +        ret = -EINVAL; >>>> +        goto rollback; >>>> +    } >>> Above "max_rx_pktlen < RTE_ETHER_MIN_LEN " case will be  inconsistent with >>> dev_set_mtu() API. >>> >>> The reasons are as follows: >>> >>> The value of RTE_ETHER_MIN_LEN is 64. If "overhead_len" is 26 caculated by >>> eth_dev_get_overhead_len(), it means >>> >>> that dev->data->dev_conf.rxmode.mtu equal to 38 is reasonable. >>> >>> But, in  dev_set_mtu() API, the check for mtu is: >>> >>> @@ -3643,12 +3644,27 @@ rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu) >>>             if (mtu < dev_info.min_mtu || mtu > dev_info.max_mtu) >>>               return -EINVAL; >>> >>> It should be noted that dev_info.min_mtu is RTE_ETHER_MIN_MTU (68). >>> >> Agree on the inconsistency. >> >> RTE_ETHER_MIN_MTU is 68, that is min MTU for IPv4 >> RTE_ETHER_MIN_LEN is 64, and min MTU for Ethernet frame >> >> Although we are talking about MTU, we are mainly concerned about Ethernet frame >> payload, not IPv4. >> >> I suggest only using RTE_ETHER_MIN_LEN. >> Since this inconsistency was already there before this patch, I will update it >> in seperate patch instead of fixing in this one. >> . > > Got it. Since the MTU value depends on the type of transmission link. Why does > we define > > a minimum MTU? > I don't think we care about type of transmission in this level, I assume we define min MTU mainly for the HW limitation and configuration. That is why it makes sense to me to use Ethernet frame lenght limitation (not IPv4 one). > True, we don't have to break the current restrictions in this patch. But it is > an indirect > > check on the MTU. Now that "mtu" in Rxmode is an entry for configuring MTU for > driver, > > I prefer to keep the same MTU check in ethdev layer. If there's a better way to > handle it, > > perhaps it would be more appropriate to do it in this patchset. > > I'd like to know how you're going to adjust。 > I am planning to move the MTU checks into common function and use it for both 'rte_eth_dev_configure()' & 'rte_eth_dev_set_mtu()' in a seperate patch. Please check v2.