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 6EA73A0542 for ; Fri, 18 Nov 2022 12:14:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5DBC042D1F; Fri, 18 Nov 2022 12:14:12 +0100 (CET) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2041.outbound.protection.outlook.com [40.107.236.41]) by mails.dpdk.org (Postfix) with ESMTP id 047D840E03; Fri, 18 Nov 2022 12:14:10 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=h+3Xe1Wy7pjc0ehLVZyz2YxOIoenCG0WE1qlNB4vvpqFbpz33qK2dMi2bGvcS0Snk0Or3cquh5TvxDH51DFFn65XgSPQHuxPKY9+dG3J6FPnl2gdVefUwJgUy7g+nto3F4H4wyy04UtkWowXfNsaUBtJHdZ/atlLd5LGvr4i7F1xf9+y3qPPK31khHjhyqTS/l5NjnOAcglX+Gfcdrb/4yxIvHiOzSTAOp2jWfMQ9uIvWHfQnb1++4CXxWCnhKU157QNYU/mAucGInTVhcTpdcJDN5haIILxIOTO7ECCvzudJOpRBONoQFck+pncXfSfjhtTEWXzBbbiXJ8OJOTH/g== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ij+26mKj1p+EWG7B8Qj6CIypNdiMvN17/TkUQ5mbB/g=; b=kdsxEYKNFZnEeq+HnrMicPbbqcZRZR+W9WOR2pVdODBWWR2jz8zRnr1qOKkPdItH3BEHTIPPhY6lu9YSE6nKTkGUfLLEmpuFltY32s4ArAQ4+4Ua+Fkt4ZPzxT6/rYcIzk3DB8tXJ6jcXfm1Rmkqgg2yw3PQJUSEBj7Y6TuhL5q+6UVT47yl6tCtanRCUV+/AKzcXV0eZ92Tt7EvpxnCcP1lBFmiIyWxI3j/KubaWkR7UYwvSfQ6B+6alaD/Tkjw68fTAGAX1xepxIeOTgjIWrbFBF4M1wAynmQFb639yizEtY4LUF80OVRD20WYW1X608kHU58uOQh0BsBHgk+gAA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ij+26mKj1p+EWG7B8Qj6CIypNdiMvN17/TkUQ5mbB/g=; b=TW9SaI9HXIlD8XvYDiAjDOsMUVOR+oeU9JIqxABBCHAwwKeqlRJA/5blfcDN+ekAewe7fB9BsAtp2IJlR5X1usxM/u+hWPW/eh55W6GPHt58PHnPuz4nTGif58rDO5HgZRA7ZaHn/runTG3/PHGcGtmCd+wO9aSv+qvKefZ7DD0= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by LV2PR12MB5871.namprd12.prod.outlook.com (2603:10b6:408:174::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5813.17; Fri, 18 Nov 2022 11:14:07 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::b482:d5bd:c7d0:3842]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::b482:d5bd:c7d0:3842%8]) with mapi id 15.20.5813.020; Fri, 18 Nov 2022 11:14:06 +0000 Message-ID: Date: Fri, 18 Nov 2022 11:14:00 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Content-Language: en-US To: Chaoyong He , dev@dpdk.org Cc: oss-drivers@corigine.com, niklas.soderlund@corigine.com, Long Wu , jin.liu@corigine.com, stable@dpdk.org References: <20221117083320.21815-1-chaoyong.he@corigine.com> From: Ferruh Yigit Subject: Re: [PATCH] net/nfp: fix issue of data len exceeds descriptor limitation In-Reply-To: <20221117083320.21815-1-chaoyong.he@corigine.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P123CA0122.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:192::19) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|LV2PR12MB5871:EE_ X-MS-Office365-Filtering-Correlation-Id: 679be927-1b95-4b12-87e4-08dac95601e6 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 0SdX3sXhAEBcHtkWAKUa/EnFFqQPOnBg8SIQr+4fnHP+jqZOz2dH86QcGzbp2b8x+9QKaP/BJXZ/76C65EJDky4N6kX2VPLUChjhkcYE1Ka+h6o1Ks+eFsI8jA4xy/1UUuLhky1mplftQ1H0ZnK8JKWPEgqEZplC4PybX+y95y5igAI852Caiu8RTa7z/XtquCXOjibMnItI6Flmf1609ki9w4+DWugTHndP1Qww99Nyc5DCpyS+secPdj621SnZDww6JqoPBZXVYnMYR0iX3Tq+YPjOi5m6Tq27F985fDvkHTv04PdlTV6aQ8IGIH/9ZjIVewQBWGe3Mh0fI+IVI2XB9Bt/RD6EL2lJYlN3peR6iCNd0IUXn/1ss+kiGjjE7O30TWNxw1F1n0YTq5hBE1WaCCneg/LN11DsWbVEiNmIYjWIrszVliq/q8m41m7vQC8VjffCqMyC7QmdNyfVi2y0+O24TqjwUqtJYfIckHeMFlGvJdBr4Gc2n1NimCtiLUfNR52dU/d1sKa1BJYH0ze1NROuUXQXlM/DlmzhUmEeopmeMBLrvoOcJU3W59iP6K29/gvo5W+nsCh08DdlDAR3EHKPf6lrLWDyYcSQFbtwifnGJCsWfO0RtfLRs01Zq2abtzk7jQG2+/S5yzBXIXBs6zKzsQmudC0ERIALC+9wSl1edROec6S2ktw/u4QiDR8Zau7EaKVpEic7UT5OWrq3yC6c4uXDrnX00TsS/z4= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(136003)(366004)(346002)(396003)(39860400002)(376002)(451199015)(53546011)(8936002)(2906002)(83380400001)(8676002)(186003)(2616005)(41300700001)(44832011)(86362001)(5660300002)(66574015)(38100700002)(31696002)(36756003)(4326008)(6666004)(31686004)(66946007)(6506007)(6512007)(66476007)(26005)(66556008)(478600001)(6486002)(316002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TW9RVE93QVNCRmFkaWlCdFFXZXh4NkpwUUNlK01jQjArM2FodHB4cE1jOHN1?= =?utf-8?B?SEI1aXM3Z25lN3lESEFqTm1mSi9KMWVHbzBIQ2VKNFk5bEE1YzZpc0xWK0c2?= =?utf-8?B?N1B3eGdyTWNsL1VmZ1BEcnorL01odG9QbzRndnUweWlQc1dDTXNOWTA0VVY5?= =?utf-8?B?N1BKMnhqNVZaVDdIYzltZFZRLzZHblllak5ONm1QRmMyZTJnUUlxQmxUUU4z?= =?utf-8?B?L3pXbXZOVXRsMHJHSGViUnFFWThxVXRvdlZxWjFFN3ZJWi9rU2k2c0dJL1Qy?= =?utf-8?B?VTdSUVBjbWE3elJ4NzB5NEgxMDYrWjBZbkxZZCsxUWxGZlBJVkNrZ3VRa3pG?= =?utf-8?B?WG5mbW1hOXh3ZnNqYVhzSE1hbFovNnBJTTZnZ1FXZ3ZUOU4wY21EQVhLRGc3?= =?utf-8?B?NzRGemZOMHZFaGhmZnc5eUk4Nko2RkJBVHNkaWhRMnowRWZGWnFrcUhFT2s3?= =?utf-8?B?SzgyTGxuM1BiNzYzRXovVHlCanZudGtwMExqa0EyZHVpeWkzb0xPUVlhUW9r?= =?utf-8?B?bzM5SGZVbmZwWDEvbzVLWTUwdzNYL0JtUmxQVi82cUZMeXlhSGQ3TTNPcEFv?= =?utf-8?B?ckxOaEQzcmZGK3FjZkxXeWlyeVNiWUw4WWcvUFB4dENVeVc5NFpkZWhscG1j?= =?utf-8?B?Z0NIa2ZJVFBnNHZZaVpDeFIzK0QzR21Ydjk4TUsyYllZb1gyRDJqMkNQQlNr?= =?utf-8?B?UFhkU1hTS014Z0RjSjBuOUxBd0tIK2g2MGdMYlFQcmhUejVIZDJKc2c0dnNx?= =?utf-8?B?OGMzb09JUXJOeFU3ck91Y3IxbGhTYTV3dm0vWjE2WmJTS3JnbkZEeThKTUJW?= =?utf-8?B?dG40S0tML2tYNzNUbzZaeTBqcHhuOU9xQWY5Z0hqSlgwN0FyU1N1bGxHZ05o?= =?utf-8?B?Y1Y5OGpUZUVjOXVwb2wzazd1d0w2Z3gyY2ZWbFVPUXZNTDE0WE4yaWpaL3Yy?= =?utf-8?B?UEV6djVaNWxkQTZ4MmlVSFpXS0plQUFNZ1V5ZkFDRG1uSFFPdDdVN1NqRkJn?= =?utf-8?B?MWJnQXFBY1BoU2s0TkMyU0RRV3ExNnN0MTdLVVRZbFRVVUdQNjV5N0tUQzFv?= =?utf-8?B?ekFkcTRycm5xZlpiZld2Z3A4ZUZUUnRyYkZNS1l2WThKS3RKazYvWFhXSUFm?= =?utf-8?B?dkpFbWVXT0psOU1JUEdZdXdJT1VJOHczNklWVGNNQ29KeVZBbjdlbE80Zk9M?= =?utf-8?B?YVd3MCtXM014TWx0VnFhQ0R4cnZEVEE1a1lydEpha0U1RndZcXUyMWNBR2N2?= =?utf-8?B?SVhZUTRMbnNObGpiQnJsZysxcm9CNFlDUnlDM2RJQzY3MEFZN2FuRGVPa1dl?= =?utf-8?B?Y1BGczRubXJhK01MbjJJWmV4OXhSOU1wVDNDS1BMdGlzaENhRnJHclB0c01k?= =?utf-8?B?NUc2NHZZVmJ6eEpON0dVTUpNQkpzakpIMStRSHFhMk1GQzYrZzN3ZFphSUpv?= =?utf-8?B?M2tUWGlodVViRUF0S1pOcVZ5ZzJoMjZDTzJKNmo5T3VydHZhTFpMN0hFcG4x?= =?utf-8?B?TWtvNlZLb0E1QTM4dVlFRmhpckt2MmdaKzF6YWVYQmJjUnlYK09LMEdibWZJ?= =?utf-8?B?YlM0d0VmZHd3QjM1dDZqVnczVlZFTE1CVy9uclg4elFVUFYwamM3RkR2T0g4?= =?utf-8?B?Tk9QMncxNGZCejdOK3o5UWZyRGlVQmtmOTI3TVV1by9XcWtnWlN3NHdWRHZE?= =?utf-8?B?L1B2SUFBYjB5a3hXcHFJa1dXSGg2L0xMTFdLQXJFbTBNa2ZZeS9paG1QU1FE?= =?utf-8?B?enIzdmp5cTAyREFPL0oxdXFXbUJjNC9oOFJ1TFhrbVZqR3VreTNFNnJGbHQz?= =?utf-8?B?YnY4K2ZqRkE2cVNBU1l6SWRuWHUveEZmeldFWHJMeVhKSVJhNkF2UXFPcTZ2?= =?utf-8?B?TzQzb21QZjhqb3psNWFya3FWeFZPdXZZYlZFTWs0WSs1dkYyNjJEM1I5bXlR?= =?utf-8?B?VlZrb1loWE02YmswbDVGcHVUbDhjZjJGRGRqOHU1Wll3czVzOXlJTkp0MnBs?= =?utf-8?B?aGdVY290aGdXdHIyWnRKQTArSDdzQit3NVRobHFvdnFCK2MzZGFueHZkMVlz?= =?utf-8?B?NXVhQXpncjJmZHZlQUwvVkNubnM2Tkd0bnhDa3gwYnpOQjR4UmxtZFlCSEU5?= =?utf-8?Q?sedatDBwh4jD9V7NvfSSTRb0N?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 679be927-1b95-4b12-87e4-08dac95601e6 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Nov 2022 11:14:06.5882 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 9i2XXKKicA5RDmTrU+JwEJQCM7hiEZZDiNWq2XYdVXLHzFH4expLkp5vUw8S5+g8 X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR12MB5871 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org On 11/17/2022 8:33 AM, Chaoyong He wrote: > From: Long Wu > > If dma_len is larger than "NFDK_DESC_TX_DMA_LEN_HEAD", the value of > dma_len bitwise and NFDK_DESC_TX_DMA_LEN_HEAD maybe less than packet > head length. Fill maximum dma_len in first tx descriptor to make > sure the whole head is included in the first descriptor. I understand the problem, but impact is not clear. I assume this cause failure on Tx of the package or Tx a corrupted package maybe but can you please explain in the commit log what observed problem is fixed? > In addition, > add some explanation for NFDK code more readable. > > Fixes: c73dced48c8c ("net/nfp: add NFDk Tx") > Cc: jin.liu@corigine.com > Cc: stable@dpdk.org > > Signed-off-by: Long Wu > Reviewed-by: Niklas Söderlund > Reviewed-by: Chaoyong He > --- > drivers/net/nfp/nfp_rxtx.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/nfp/nfp_rxtx.c b/drivers/net/nfp/nfp_rxtx.c > index b8c874d315..ed88d740fa 100644 > --- a/drivers/net/nfp/nfp_rxtx.c > +++ b/drivers/net/nfp/nfp_rxtx.c > @@ -1064,6 +1064,7 @@ nfp_net_nfdk_tx_maybe_close_block(struct nfp_net_txq *txq, struct rte_mbuf *pkt) > if (unlikely(n_descs > NFDK_TX_DESC_GATHER_MAX)) > return -EINVAL; > > + /* Under count by 1 (don't count meta) for the round down to work out */ > n_descs += !!(pkt->ol_flags & RTE_MBUF_F_TX_TCP_SEG); > > if (round_down(txq->wr_p, NFDK_TX_DESC_BLOCK_CNT) != > @@ -1180,6 +1181,7 @@ nfp_net_nfdk_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk > /* Sending packets */ > while ((npkts < nb_pkts) && free_descs) { > uint32_t type, dma_len, dlen_type, tmp_dlen; > + uint32_t tmp_hlen; > int nop_descs, used_descs; > > pkt = *(tx_pkts + npkts); > @@ -1218,8 +1220,23 @@ nfp_net_nfdk_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk > } else { > type = NFDK_DESC_TX_TYPE_GATHER; > } > + > + /* Implicitly truncates to chunk in below logic */ > dma_len -= 1; > - dlen_type = (NFDK_DESC_TX_DMA_LEN_HEAD & dma_len) | > + > + /* > + * We will do our best to pass as much data as we can in descriptor > + * and we need to make sure the first descriptor includes whole > + * head since there is limitation in firmware side. Sometimes the > + * value of dma_len bitwise and NFDK_DESC_TX_DMA_LEN_HEAD will less "bitwise and" is confusing while reading, because of 'and', can you please use '&' instead, I think it is easier to understand that way. > + * than packet head len. > + */ > + if (dma_len > NFDK_DESC_TX_DMA_LEN_HEAD) > + tmp_hlen = NFDK_DESC_TX_DMA_LEN_HEAD; > + else > + tmp_hlen = dma_len; > + What is the point of masking if you already have above check? Why don't you use 'tmp_hlen' directly, instead of "(NFDK_DESC_TX_DMA_LEN_HEAD & tmp_hlen)" after above check? > + dlen_type = (NFDK_DESC_TX_DMA_LEN_HEAD & tmp_hlen) | Since 'tmp_hlen' is only used one, you may prefer ternary operator to get rid of temporary variable, but it is up to you based on readability: dlen_type = (dma_len > NFDK_DESC_TX_DMA_LEN_HEAD ? NFDK_DESC_TX_DMA_LEN_HEAD : dma_len) | > (NFDK_DESC_TX_TYPE_HEAD & (type << 12)); > ktxds->dma_len_type = rte_cpu_to_le_16(dlen_type); > dma_addr = rte_mbuf_data_iova(pkt); > @@ -1229,10 +1246,18 @@ nfp_net_nfdk_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pk > ktxds->dma_addr_lo = rte_cpu_to_le_32(dma_addr & 0xffffffff); > ktxds++; > > + /* > + * Preserve the original dlen_type, this way below the EOP logic > + * can use dlen_type. > + */ > tmp_dlen = dlen_type & NFDK_DESC_TX_DMA_LEN_HEAD; > dma_len -= tmp_dlen; > dma_addr += tmp_dlen + 1; > > + /* > + * The rest of the data (if any) will be in larger dma descritors > + * and is handled with the dma_len loop. > + */ > while (pkt) { > if (*lmbuf) > rte_pktmbuf_free_seg(*lmbuf);