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 0BDEC46069; Mon, 13 Jan 2025 11:05:40 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9347C402A7; Mon, 13 Jan 2025 11:05:39 +0100 (CET) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by mails.dpdk.org (Postfix) with ESMTP id DAD8140261 for ; Mon, 13 Jan 2025 11:05:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1736762738; x=1768298738; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=mGNMeNqlElivWHfLXI0fLcNzoLHJ/AfKYteUSwCZHZ8=; b=JNRF1vb5pGrx2RpMd0CaBkoBnNCfOLLMfNb4/rfcif1GhIrKrmaKJHxo RDNMq/SEY40Pnvp1aabHIYqoSYyPj4glGgDqVEj2Z9PqZMwsyxdB9iucA VfIbrW8JY4yaonxw8nXwd21Dg4fFImhSZ15Pl//senCvvsGSGfRhHCQTk I316KxMqeSFktmS7a2MHKsuT41mHYD91J24X80TKvVu5moauSX/gEkZH5 fDhr64Atbkt3dc1LyAifAO2uFQriv60uxqP+ARYmUik/Gx/dVGmvacCSN W+QAq42lj7DFfCcgry/juX9uuRRfiSm4tJyQm+6A2HKGmA4GtXsU5nYwX w==; X-CSE-ConnectionGUID: f5rJGxsuS9ONHU4PIW9c1A== X-CSE-MsgGUID: zWEUZI9yS42G7JifbjEUnw== X-IronPort-AV: E=McAfee;i="6700,10204,11313"; a="39830490" X-IronPort-AV: E=Sophos;i="6.12,310,1728975600"; d="scan'208";a="39830490" Received: from fmviesa002.fm.intel.com ([10.60.135.142]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jan 2025 02:05:14 -0800 X-CSE-ConnectionGUID: J0urySbySA+bc/3Odr+fxg== X-CSE-MsgGUID: lR8ObVpERPKEZpl0grKbqA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,224,1728975600"; d="scan'208";a="127681789" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by fmviesa002.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 13 Jan 2025 02:05:14 -0800 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.44; Mon, 13 Jan 2025 02:05:05 -0800 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.44 via Frontend Transport; Mon, 13 Jan 2025 02:05:05 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.47) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.44; Mon, 13 Jan 2025 02:04:57 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Lw5p3GW74o9NQ7XyFow6V7ExpvRS+OU19enXkMQ4Zi46yCTF+VtUjk2i285Ht4In50ReplcwKI4iS/0AUFmhaF2OsBzg3vIrGDcAxzW4YID0mK1JUk7gSJBT/ZUOmpMuVVSg/P8NkjEUzD+AZteMtKOTGri9c01vEDgj5JRi7Nn8zZLxclFDfhpYAG+VpE+P+AL8fevqyeM3ccJ7Wyn2f7AK4juF739/Ky2yQDY0I+IdcwDu51Hm/moftKsSsR0UIrn19YfDV9XA7utA4EurOh67qRUB+cqcbInwSWE9vInUacavUMo9Wr5Fw+r2IsNvul9zLMe9DaLpOTqDoxcxWg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=Gj7bu7XHKWdE+uW9QT/5RrJOZCl2A4HuCR103pxsfdA=; b=prJtjapPhtkzR2OYuK1kiPjtkv2dT2he0pkKSo3vWREGPkFD2MZ2XGnnVoFDatIw7TVhrd6Ks6NpdtUSv8R72ze3o0ChYGLb1xURpYLfSLvtoKLokdTDSMVyaEjAuquEhayD6Wy4L5VWm+JKassKqu5J1y/gudBRjDsV+5BEBHfFLaAU9m6ZrqO8eTc0wRybIRFXg3b1DDPOPDGBl4SOU6LI3+wSfuGTf6xDNwIsRd9k47Xh270eWWnxvUtXRVjSXU66T1GryWbAkH65+31jzSdlAAa/PK404/C4cbXtl4t9KnXzONYezb0BVxc0G9VCcsWQgqGM3KOf2pFH4dfDzA== 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 Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by IA1PR11MB8246.namprd11.prod.outlook.com (2603:10b6:208:445::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8335.18; Mon, 13 Jan 2025 10:04:55 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b%5]) with mapi id 15.20.8335.017; Mon, 13 Jan 2025 10:04:55 +0000 Date: Mon, 13 Jan 2025 10:04:50 +0000 From: Bruce Richardson To: Stephen Hemminger CC: , David Christensen , Ian Stokes , Konstantin Ananyev , Wathsala Vithanage , Vladimir Medvedkin , Anatoly Burakov Subject: Re: [PATCH v4 01/24] net/_common_intel: add pkt reassembly fn for intel drivers Message-ID: References: <20241122125418.2857301-1-bruce.richardson@intel.com> <20241220143925.609044-1-bruce.richardson@intel.com> <20241220143925.609044-2-bruce.richardson@intel.com> <20241220081540.1a6a3da5@hermes.local> <20250111090714.70ef40a1@hermes.local> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20250111090714.70ef40a1@hermes.local> X-ClientProxiedBy: DU2PR04CA0350.eurprd04.prod.outlook.com (2603:10a6:10:2b4::22) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|IA1PR11MB8246:EE_ X-MS-Office365-Filtering-Correlation-Id: 96600a45-ee50-48b0-64c4-08dd33b9ba86 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|1800799024|366016; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?UOWMJ75BOhrOOuE5TY2HG4a92MPV5x4lMXT7BAqy5S53Qz+eTtfuNy6nrMXS?= =?us-ascii?Q?UPrFHruiTNlgSRIGtUeDRe/OoV/NVerrnboGY6Y0isKSyY2aop9zByCTTtd9?= =?us-ascii?Q?0a87MuwqyM61PnFAKxpw9P7jFQYzyO4DCMR4NoweCL4IGD9gMhKIIBWaZqCa?= =?us-ascii?Q?hBj8Rx0Mu5XrORusOv0WXzFX0+E27DkuyUfX2ygJUiX1ADaZ/HXPHWOo2tyW?= =?us-ascii?Q?n6MLqiMSq1Ka4nWQbxITUXOPGla8imC79Yl3ncBTj0j5x3OsQNR+UyWvfWYu?= =?us-ascii?Q?WJI3T3mf+mRaKrkH1MkCC4lyF85D2mZsFdvfMBfnBla3tBFQBjkdCBiDc5mz?= =?us-ascii?Q?Uh0uk2k8qGCpoBoE9bZHxjnM8IFlxFsvi7aDENj7P05JdF/U5RYrwnUzDllS?= =?us-ascii?Q?Q9CWx3ZRaGgAkbbVuO8il2z7x28wj99Oa20QHi2ogVREXQvPLGJAqrRELpa1?= =?us-ascii?Q?VkjEDiLgrmgbyIp/q0mZXudfFBVeoI5hJJZBa8nY3AKSshTZH1KNECxg3Z1R?= =?us-ascii?Q?NwalypBkFPvdXK24gNRQDqqgc/JLFgjnqNrj/7LrsMByDYY9fwtQHV+g+rK6?= =?us-ascii?Q?A9gGQAbcFlg88olztDwA61wKN1TWwF9bGgxJSPUN1kZjnW5e97nCnsUOd1rJ?= =?us-ascii?Q?4oJgX6edYtnTIWqvu5FgQKyx1HIK8smkDaLMFSyLJyyNcs24bvRFMnGPXw5u?= =?us-ascii?Q?ZhuDpSAkpA5PCkMaUAlPbQARH1WneIcrZfRwAfXWQQWpIGySpD5qze07ZhfW?= =?us-ascii?Q?l6UymqU8P5Mu86cSoUiM9+uJApUn7ncMInDitWssO65cdsRmVNwr0z4Tngci?= =?us-ascii?Q?Ca/S+/EijC3L+JH8fnEOwyREAMnFOHUImwwGgSSTosCnnYhMFlAhOts4gZRw?= =?us-ascii?Q?wV30RHo5MtzV6zYq3btzo+CMMasbWCeyO/FgsiILrlk3QRdFj+7MEFNr+GYh?= =?us-ascii?Q?wIAGxdpu9Pyc0iNNX3e79EGmnE69Tz/JJerp8fuMVihmkEL4BqNnOmZCmiQt?= =?us-ascii?Q?lozC8tHHak9v9GKUGaZO65c3N8as7Y+lL4MX1j6qMiVfvEyh+9GV6e4ASfBM?= =?us-ascii?Q?HooJJExxcze0eurP6ekk2EF1Vp300abYpNFH9E0VXiznDxSL/rDFkftj79Xc?= =?us-ascii?Q?oQVwaFyIo604RliOAtPQzKYy2hYP7yzSg5KjIFSUN8Yxto1XdOPH3qC+jzpn?= =?us-ascii?Q?5PAqX4vmFvQrc2mJh2QQcHOH95kDmF4NG3J102SLQMsEvCVQkJCGUXJA58Ih?= =?us-ascii?Q?B/J5N2Bj57UFWk+eUm15vGPYAIm4vDFhS835t8P2IPli2UHEuNJsNYAzDVO6?= =?us-ascii?Q?rVNIqt4yQn15CY+xr1l2b2Pq3RdXPIjNdx5OQftJgWZJTje9tt0d7Hs8AZg3?= =?us-ascii?Q?4QP0oNRF8OKP2E7r+nk71p6u6yua?= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230040)(376014)(1800799024)(366016); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?rcpAWRgQRbleiLdPTPAuf7yzK5OyWFCRWTMNVMAH17TW5x4qSTdP9RIkMC/u?= =?us-ascii?Q?15zuRvrIWmMTM6ARB6u8NeKsVJZlgXca2y5+6psSED8xj1YUCo1Ts8/iQXht?= =?us-ascii?Q?QQ1lRFKMUa34m1eznI4/lpZSUi8G0dhj1y0gSyA6GtWPMaysd0yTxDo/Rrt6?= =?us-ascii?Q?7KbHoXmEqcnE+57Vui8MANepE1oy4drxc1sVwPl7AxhJnGLOhTzwQyh0yErb?= =?us-ascii?Q?bkj8RnTxnWXgFX0CcqO5LBWbASEOESAzfCIDPH+p9l1kApzhqTjFbYfrfRTd?= =?us-ascii?Q?rTKuwOLMER1+WB78WTaWkHTz9AP7BzI+JfsScckq7T3CKePqHIRwm2QQJIud?= =?us-ascii?Q?FSQzk5+fNLLS460K7SlWExV6H1b/uu85+gV6F5UnqXLl2Ax/nWOt0zJSDN4C?= =?us-ascii?Q?oZhEmMhwkwFSdGuan3ufR95p2OMY/Jp+FhgUW2LypxMUquQW+HLlGhP8BRQW?= =?us-ascii?Q?KUDrvQuMCcHhstvLonlq1uv+MJ14OqD/A29ycKNkBc3vpGGxQiXZDhw/5/sU?= =?us-ascii?Q?v4KR4uB9Pvtt9EMMs2xXKpulCNUee1vtLbUmZRzXA8VjC7GifXn3YkXdXHIp?= =?us-ascii?Q?viq7yMscWULojxPOnijzcuZSbOioPZBOXIoHLjUpEGpGc+ruS0VGLx2ldulM?= =?us-ascii?Q?0+eCNCdd3+FaNOhR2mARqHZTxRWZUTt1tCIE5vnkYV96vjD9QfoVuu68i7WF?= =?us-ascii?Q?nufD73tECsvPWifaGk0ev7v9GashTnCZ/akji5Nt+PpuE8sHZJCgMRw8vZ9g?= =?us-ascii?Q?HliZroMuTuLNjDp1osZ/STmRwtzkkpzVaqSZINXPieK9vieqNiZBonaDvsRZ?= =?us-ascii?Q?gKqZNIMv1+FVO+7pIbhkn/DjVFkgYRQ46z4WhlhHq/JAi/Y1O54N+bYF9v3G?= =?us-ascii?Q?gFUiGcJgGNEHb1FoXnvUyMYK6RCQKLrjDqd+Q6ZXyvbrCcO0IvuujUGXmxYu?= =?us-ascii?Q?EFOxC3vvP6v+rYFf0FxbF82LBxHUU3lkyU5VywqOrsnyDTGxOy4wpOoxRMp8?= =?us-ascii?Q?LCA751dKypA8AkGusq8hD4dDTtrBRMYRzUQqer4Jnh83jB3M0w9UfRLXhXAA?= =?us-ascii?Q?caWfKXI7QlzWyjz3NzjXSTmMXVvA29g4QpXqupQSrGugzSka4psmdKbq8YFJ?= =?us-ascii?Q?RAxCSdZgzPJbOgwFdS9pV86FIC1LVlt1mignPybe2DD2AIYG+YL1IiDnuRmF?= =?us-ascii?Q?+rt091gEaLKjzdkvddoc998y3mkLUXE5sca0svcwotHPpUwtzbLIG63nb8dU?= =?us-ascii?Q?4fgQ6IpK69QLWHtrpYCbMXsvvnTKIt2EcIldhunFBRu+oemBv76Ryh1nVVOS?= =?us-ascii?Q?3bj1Lcue+S8RV/fAKPCp6hreWD6qW4PqeXmR6BcUl8nhqj985vwxPjeRnVd/?= =?us-ascii?Q?6GQuyyD7Dq1639QnL5kJ8+XWYBmAFXOouaDB+YNh4ZdzDUrs/Gr5SFibmFBA?= =?us-ascii?Q?3Qwk9XK4wwWUX4lChG8EXFAziGliMaOiIfmgREnNF9kuLDzSOEZ06WocnCyg?= =?us-ascii?Q?E7JenSgav2FgHyMVLAuFjfEhq0Ibl0jfwQrzDp3NqHpncatOg3v8Q2hfM9aC?= =?us-ascii?Q?zY/gN7tDWIqOI+VjccDNZn/NqFM80kR+QkXi9EK5V5dhWjuPkiuWVXzj6Qxo?= =?us-ascii?Q?ig=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 96600a45-ee50-48b0-64c4-08dd33b9ba86 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2025 10:04:55.0123 (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: zzGcCbSst6dbi5FewBfLk/Gqf9BRpwA4qIPpEBhxlgXEIGb2WvR/hkdbpsHFFY7kZCN0Nxr/IKwmV8CpNILXwVHDJrIkxBacb+k6C5WPVGU= X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB8246 X-OriginatorOrg: intel.com 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 On Sat, Jan 11, 2025 at 09:07:14AM -0800, Stephen Hemminger wrote: > On Mon, 6 Jan 2025 14:25:05 +0000 > Bruce Richardson wrote: > > > On Fri, Dec 20, 2024 at 08:15:40AM -0800, Stephen Hemminger wrote: > > > On Fri, 20 Dec 2024 14:38:58 +0000 > > > Bruce Richardson wrote: > > > > > > > + > > > > + if (!split_flags[buf_idx]) { > > > > + /* it's the last packet of the set */ > > > > + start->hash = end->hash; > > > > + start->vlan_tci = end->vlan_tci; > > > > + start->ol_flags = end->ol_flags; > > > > + /* we need to strip crc for the whole packet */ > > > > + start->pkt_len -= crc_len; > > > > + if (end->data_len > crc_len) { > > > > + end->data_len -= crc_len; > > > > + } else { > > > > + /* free up last mbuf */ > > > > + struct rte_mbuf *secondlast = start; > > > > + > > > > + start->nb_segs--; > > > > + while (secondlast->next != end) > > > > + secondlast = secondlast->next; > > > > + secondlast->data_len -= (crc_len - end->data_len); > > > > + secondlast->next = NULL; > > > > + rte_pktmbuf_free_seg(end); > > > > + } > > > > > > The problem with freeing the last buffer is that the CRC will be garbage. > > > What if the CRC is sitting past the last mbuf? > > > > > > +-----------------------+ +-----+ > > > | Data +--->+ CRC | > > > +-----------------------+ +-----+ > > > > > > This part (from original code) will free the second mbuf which contains > > > the CRC. The whole "don't strip CRC and leave it past the mbuf data" model > > > of mbuf's is a danger trap. > > > > Can you explain more clearly what you see as the issue with the current > > code above? > > > > The "crc_len" variable contains the length of the CRC included in the > > packet, which should be removed from that before returning the mbuf from RX. > > It contains "0" if the CRC is HW stripped, and "4" if the CRC needs to be > > removed by software - something that is done just by subtracting the CRC > > length from the packet and buffer lengths. The freeing of the last segment > > occurs only in the case that the last segment contains the CRC only - or > > part of the CRC only, as otherwise we would have an extra empty buffer at > > the end of the chain. > > But if the application is using the driver in "keep CRC" mode, > then it probably intends to look at the CRC. In that case crc_len is 4, > and that trailing buffer may need to be retained. > > It all goes back to the design mistake of the semantics of "keep CRC" > mode. In that mode, the mbuf chain has hidden data (past pkt_len and data_len). > If CRC is not to be stripped, then crc_len should be zero here also, in which case nothing happens for freeing data. [If crc_len is not zero in that case, that's a bug in the config path, not datapath here] /Bruce