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 A913245BC0; Tue, 29 Oct 2024 18:42:50 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7D5CE42E48; Tue, 29 Oct 2024 18:42:50 +0100 (CET) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) by mails.dpdk.org (Postfix) with ESMTP id 4F6F240267 for ; Tue, 29 Oct 2024 18:42:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1730223769; x=1761759769; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=Z9XjjGDUpOYIwvxXuRmZtpIcL7BZRejG4zOQaPampE8=; b=FN8TidOkbtjILhLfDIsoWEDGLylC/7QON43iAZvbvwnjt6Wz9i+gD8Ig nY9nx0obB/xqh74+AtmWmQ4dMDG4Chi5x/TVBaBAESMOMbyaiN9t9cObk r5r3XEv7ikoJYdBHatQ4abPCcxVi/RY7zCHg6aCvDP9TmMR5r90xyInmq NHFAUXBh0mO5jnvuvpvAaU9OMJloIEauxqtoRcOKALDwWh90gj1tAYOJE CCEwkjS+/GiA7tDQ49Rdi6Kf+MxDzF1osLuOEErVIvrNg5pNXhRrY9Paw fqO1Mzcs7dI2t0ydhv9fCycad+c+BSJLoE7w9qb8y8v/sC5HDbKKcqcDa Q==; X-CSE-ConnectionGUID: 5kzA0YlyTqKaGwungmU5GA== X-CSE-MsgGUID: YrP7pr1yQjG5PY2aH0mgtQ== X-IronPort-AV: E=McAfee;i="6700,10204,11240"; a="17518246" X-IronPort-AV: E=Sophos;i="6.11,241,1725346800"; d="scan'208";a="17518246" Received: from fmviesa006.fm.intel.com ([10.60.135.146]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Oct 2024 10:42:48 -0700 X-CSE-ConnectionGUID: 8fCcyhhvSBWnxvEJrtDVlw== X-CSE-MsgGUID: 4krt83P6T6ihwA3WgNl/Gg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,241,1725346800"; d="scan'208";a="81592094" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmviesa006.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 29 Oct 2024 10:42:48 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) 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.39; Tue, 29 Oct 2024 10:42:47 -0700 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.39 via Frontend Transport; Tue, 29 Oct 2024 10:42:47 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.172) 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.39; Tue, 29 Oct 2024 10:42:47 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=OO7jcHE+RRB0ahg0v5tFremQN0DAWibk7Br+Xw8IENxXYujYmFZVj9KDHShciBrI0U2/An+FrSyjI/esaeRYBPx2yG+ItThnvZNAyTViGIYEfiMHvWlyv/MRd0IGbZQvznPdgnWHBAwG/kyQGn7hAY013qvJD4gx0KkhFbyLwueS0bv36vyAtG4R62q9eqTO8/7pv+FDXvgjBqHYC9KU/iGsbRg1xtQsG+zWi3bU+CGGtT3DjcZqQ/xX8tdzU6DrgPwXuSWW7OZLqv3gtbL7VjjwAm7U9Cq5pZydMAbD4Kt0HYad0VLlOxBppJNjQT5LxB6CmjqTP8oIIqk8c4PnVw== 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=VDaREPBc0GT6bM2iAfRv0+jDRP+LYnSbwxU2z6V7kkM=; b=WwvmDFYxRviwCpSsC/N/f1L6MQ/x8kMrXVqPUvIAdppzZNAJkjoBTdFxjqkNSvt352wjLni8sFbBZVxs6IBuRXO1NGJERuZJ47Eh4rMM7WW9oXy+6/W7bFed4532m6+kWDXCBXXhklx9jX9EVA7iAIV/Oe79HL/lDv8hkksAoTRQ4yXp1sABUtkiuPoM/vyi4kFz51rJFugH81Cob+20FYn1dJJ9lAzc4PK9NRLk1XMhMpxWIgfPy2KwGm7UFPMNqs7dvcqzL/03ovMjVjKyyL3RoOghm1mw8sUzkCCQKfubt/k1rD61sSpYQ5sks3aLMCRtwhpqk6dVB4S/HLG8fA== 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 SA2PR11MB5052.namprd11.prod.outlook.com (2603:10b6:806:fa::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8093.32; Tue, 29 Oct 2024 17:42:45 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b%4]) with mapi id 15.20.8093.018; Tue, 29 Oct 2024 17:42:45 +0000 Date: Tue, 29 Oct 2024 17:42:40 +0000 From: Bruce Richardson To: Martin Weiser CC: "dev@dpdk.org" Subject: Re: [PATCH] igc: fix invalid length and corrupted multi-segment mbufs Message-ID: References: Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: DU7P194CA0019.EURP194.PROD.OUTLOOK.COM (2603:10a6:10:553::34) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|SA2PR11MB5052:EE_ X-MS-Office365-Filtering-Correlation-Id: bb4e034e-5111-45b5-33f3-08dcf84118b4 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|366016|376014|1800799024; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?/aE67hveImKuQPhob2LmYAyxqgEcQ0gXADZwEbY+TjIJirPNMrJLU6AKwE7N?= =?us-ascii?Q?p5+/UkRreWsVMgjk1RJiqaTOQ4JaMXO8+hY01pXbw68x1IwTg09Q7zIT4LDh?= =?us-ascii?Q?NuOsKJ+V9cHN5pZ0+jeZrHyPpuvEGVDzQza9DXa2ybtUOXd+KhxU0i2cj0KT?= =?us-ascii?Q?icRSjvmZX+QqdMmPnB2JXJoZ/bpOAqd/Eo3/OI+TjgxW9E1AbsjGxy6DzH/y?= =?us-ascii?Q?2yIj1oSilhi5zmyAUqj+aYtI2mrMWiBxROCrLeyA2UwlyWwBuFMhNBps80pO?= =?us-ascii?Q?LbgI4j4A+YvmQnGtoiqzDAuLwt+QgwdDQUkxJiZAnWnFe6mEb8QbXVZezYhX?= =?us-ascii?Q?4A8wb+A/9A/mbcQ6q6DZedNajs+oKHry9gJbAuIkedhxIBYV2XF14Wlp2UNM?= =?us-ascii?Q?/NchemraHqrQbrd7ma5puaQt5lqwz8HK18GNOUOg2pPj/oI2mhB5QPvlYEyS?= =?us-ascii?Q?YpUu9KyMLBFwsoajqj/1wx86ziISMNY3V+Jfsq8+HnvixOOBMFPdmWf5ioqX?= =?us-ascii?Q?5n5UN/2NZ54vxb+fl3J9NE8rLfvBxKwTDQOFCtF1j9cR1vR3LQOLDfk+1dmg?= =?us-ascii?Q?1aM0IjGU0HfnPxyvLwWS/Yf63x6mySZKRrzRQqdbN7NtgwJMCm7cz7bVNI6n?= =?us-ascii?Q?dAIivxNfEu+jwH8sx7Sq49fpGdxX2j2grZORTtZErGBOLwHqQ6qu4mPED2QF?= =?us-ascii?Q?/stA9OIBRIe9QsnCPZjdaGxegWENc+eipXQZKagzfeoMveP4Xnrk9Q2I+vQt?= =?us-ascii?Q?XU1QY5qD0bEe1jJB4IIfXseNP82sMCUz1Jrm99NfsUGGcXKryGBQ/2qx6KbL?= =?us-ascii?Q?DQTas2fmhg/26cm4Bb29dZ9mzJaZbdrUT+FAa/hAUjBbBCryXj7tnyzTJSzk?= =?us-ascii?Q?uJl+ZgkDnnal9rAo0+4TA512Xako7Mz+VeSJeLWi/E04fMR5XfzsyzJAa+Y/?= =?us-ascii?Q?XWSdSVCmsYfGDidPGxUS2XmlB9hpSaaAdGJexHkE7YJFs67hnFBopXXn009U?= =?us-ascii?Q?ZKLyCVJvhIoyT5YTyh8IdCaYRZXO6/P1XkYl01o8mFsMcLLdqCCYB98RjKyv?= =?us-ascii?Q?2K82azAQs+Uh5XKooiwJrkl3RdYL8Puvzl/KChnib2o07en+4qHK1hNt78Wl?= =?us-ascii?Q?xfynxM2Dr9aVUi5MyZhRLJg7TnF9r+RsI/T7xGakY+oDfDrjeL3NxP00xdQc?= =?us-ascii?Q?fKXJq11IVWYR7H76Tys3bL5scAd3ZWvPgjySH/tNrb9T5AZkSzkLtxGyws/c?= =?us-ascii?Q?Mg9YM1AJ8coZfvWf2SK1h9wEsSH4HiOWhdAhT5nfEg2afJp0tzJ13vFZIduv?= =?us-ascii?Q?ihaYSmbpPnff4TMO+KeWxgRo?= 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)(366016)(376014)(1800799024); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qfQYNfc44CYu4Hd4W/DP1thJWZxLz8DYHGtlHc1FMJhi6NZJ02b2RFQ4DbzF?= =?us-ascii?Q?NTa1QztjwiF+2R/HvcqxOktqa7vO/n+enF0CP9495IuBzB2Wd5bvLexlm6AN?= =?us-ascii?Q?WSeLrNUs/uVEap1dG7ATfNFryK786o6xEZ2JtYhkAZrWJYyqStsT4nYG7aCB?= =?us-ascii?Q?Z9FMOny54BgAxdGtfqcHEj22Y1blxu2KwsNzGzKpUjNfS+6XNP8cufgknHYj?= =?us-ascii?Q?AHRKhSXFrcwIry7idGTFLTTzsw4SzNO4SdO+1z2yuWR8H6dpvKfXcuGL/kK7?= =?us-ascii?Q?mN7ZBRz3l7BP3+9NU3EJCtfvw3ixPMgXE9peTEPxIdZN3zFTMePLr67PCOhe?= =?us-ascii?Q?wn20IlLqlkQ8UY2UK+Bt6tnOqD8ILKF1BjspSeBbUNxxAlxeKHE/qZikQf+7?= =?us-ascii?Q?mxilhSzQXOcJGQJw9lEvA41RdRkuYKG6A71lP3cT3vGnXhU9NCb56qq2XePN?= =?us-ascii?Q?7KiEG9lZzCjnuRmwxiBJgIH1DQspERP1wPoqt8t1YfhP+CKDSBehhFgQf7rg?= =?us-ascii?Q?QCFMhj4B6obOz7Tbi5AMbP0MPH9Jnqo52oO7BA9buMJpf2lungVDrERdAPQe?= =?us-ascii?Q?65nMrbgO3XAySVqVfBZLcUtzP6ampWvBbBPXCyqoH9gOHcb0fxd8e52F2J4G?= =?us-ascii?Q?wH6SRQtsd9ssjMF41Lqf4PoFKIf6oUkqG/a5Xv/u90zs4//9qfxZTv4FHMeN?= =?us-ascii?Q?Z7oMyK1QtRar4/ZLZmQImcwHMbuDO4tEDPp8G40tqad/oHcXm67P4Zr1lk+X?= =?us-ascii?Q?dqUNu+mmEavBNtklcT97wOFv36i8tD5N+QfKg9StgmBGZD+HRDG0L+KC0OyN?= =?us-ascii?Q?mCuIr3Vpt5Hds5loFNlu4b31KX8GpJCDEFTTpYy4rqSGW0dnVIB04unlKOdL?= =?us-ascii?Q?7tstgUN8wciqb6r3MRerbEiOa/i34GPtRFCFuEto6Uv53i/LFVCaX4miqL1C?= =?us-ascii?Q?oPeVJAEDxaupTmJPLFkFXfXWh3HA4BDp+pKMiA16UDmcGAMqEoRX/kKx1qId?= =?us-ascii?Q?wKTHT3Ex3Yeb4/Z7y20FXstbhJrZPtsj0nCUvThPhj9O+QMaa2wHTkjGu4k6?= =?us-ascii?Q?T/H1rt4rJXeZE9LKX2CvASGlCEaigsPTWu8W0elAIp8hLu4Y/SkEYGRVfE9A?= =?us-ascii?Q?ZhLOOmjMTJHvvB6VICloUXNlimAYYtCZ/GEEsO4BSBudtYm62pkNKXG+CO69?= =?us-ascii?Q?Rc95FuxsVoivvsqJrzRTVYPd4c+ZZjhLMPcPyf/8X5uVQa9ukdUGwfjLVjyk?= =?us-ascii?Q?MDNCc/bbexnTIJFiRT/DzzeTS8Wg2Om8gpdOJuAxAhXRKPkZFlv5BAd5SCnM?= =?us-ascii?Q?a4vWpkKStQKmt2wIjP802RC2s4HRuol00XbkczaOFlmdrZnhZ3955RziSl2x?= =?us-ascii?Q?o5EaT6pAroNL7cSbWY745PoBxCk62HMOfuosNUtj+SAsmpHDGCjxHQcBaTq7?= =?us-ascii?Q?HkObL5GhltNCOo5qUFgPsKsu1aZekxQNZda8ycbfMSCZq55xdmLjGmSVL+Wi?= =?us-ascii?Q?Bu90k53xD9otElH+csMbNP79dsqDstN6KAPNtAUDOb+sbacgmqLFeVJPCa8w?= =?us-ascii?Q?/1FVe6EJ3jiFo/cKRGD9UWW6IjFYp+Nue7EHpldzasL6eXiw34rY3rysYL1n?= =?us-ascii?Q?3g=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: bb4e034e-5111-45b5-33f3-08dcf84118b4 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Oct 2024 17:42:45.2194 (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: RKbyZk8/CZl6epJAMffJkSgAcG5xlf74tWc7ym0IUCmsYE1hCVjbKfLZt2zWhpWrCfkleeYg+tSSVelZX1pcHWiZUTQ/7qTaC1FOQVyHkuc= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5052 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 Mon, Oct 28, 2024 at 03:17:07PM +0100, Martin Weiser wrote: > > The issue only appeared with hardware-timestamping enabled > (RTE_ETH_RX_OFFLOAD_TIMESTAMP). > > The length of the prepended hardware timestamp was not subtracted from > the data length so that received packets were 16 bytes longer than > expected. > > In scatter-gather mode only the first mbuf has a timestamp but the > data offset of the follow-up mbufs was not adjusted accordingly. > This caused 16 bytes of packet data to be missing between > the segments. > > Signed-off-by: Martin Weiser > --- Hi, thanks for the patch. Some comments inline below. /Bruce > drivers/net/igc/igc_txrx.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c > index d0cee1b016..2fafa91bd5 100644 > --- a/drivers/net/igc/igc_txrx.c > +++ b/drivers/net/igc/igc_txrx.c > @@ -347,6 +347,8 @@ igc_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > > rxm->data_off = RTE_PKTMBUF_HEADROOM; > data_len = rte_le_to_cpu_16(rxd.wb.upper.length) - rxq->crc_len; > + if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) > + data_len -= IGC_TS_HDR_LEN; > rxm->data_len = data_len; > rxm->pkt_len = data_len; > rxm->nb_segs = 1; > @@ -509,6 +511,12 @@ igc_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, > */ > rxm->data_off = RTE_PKTMBUF_HEADROOM; > data_len = rte_le_to_cpu_16(rxd.wb.upper.length); > + if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) { > + if (first_seg == NULL) > + data_len -= IGC_TS_HDR_LEN; > + else > + rxm->data_off -= IGC_TS_HDR_LEN; This initially confused me, because I was assuming that data_off was a typo for data_len. However, then I realised on closer examination that, when timestamp offload is enabled, the actual buffer addresses sent down to the hardware are offset by IGC_TS_HDR_LEN (meaning the first buffer of a pkt has the start of data at "normal" data_offset in mbuf, but subsequent buffers need adjustment). Is my understanding of the issue correct? In either case, I have two small bits of feedback on this: * Firstly, I think this needs a comment explaining the logic here, to avoid others being confused as I was. * Secondly, a very minor point, but is the code clearer or shorter, if you merge this extra code down into the next block which is already checking for the first_segment of a packet or not? > + } > rxm->data_len = data_len; > > /* > @@ -557,6 +565,7 @@ igc_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, > last_seg->data_len = last_seg->data_len - > (RTE_ETHER_CRC_LEN - data_len); > last_seg->next = NULL; > + rxm = last_seg; > } else { > rxm->data_len = (uint16_t) > (data_len - RTE_ETHER_CRC_LEN); > -- > 2.47.0