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 3421945C4B; Fri, 1 Nov 2024 14:52:22 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1F88E4027C; Fri, 1 Nov 2024 14:52:22 +0100 (CET) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.11]) by mails.dpdk.org (Postfix) with ESMTP id 7E1A94003C for ; Fri, 1 Nov 2024 14:52:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1730469139; x=1762005139; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=EsdF35fQW6X3WoHdqClgpdrNK2Bl02m8o6TBpwMaZQw=; b=ilQoU9F+3vW4sWb0qzQavhB3MN934PQzzrLlumoYOCeVNRJHBRSh4xOk K2E1zprJooWyTHVCIQcFa38tXB6rqr9SyPz+l/fbTWoKrFV5vrAn9lmAx gw/vI7dM+diqUH+UD6mPIhNjzgAbFXJUoFsKzZ1ZXdHeeDxwsJNwux+lc iHeYfpRiCMU5QjToGRBUTK1Sb8FZ+PEx8ttUnEoKy7rzm17UQie7oRAcp XwtXrB8AxJG5eQwIcB+ubxRPpk8yD6v10mgQFWWdmVx4BYWf1Q7Lo4mUF 76XuBGN1/lMGygt0AdB8vs++0aYtuooN0gAI7D2cacv0gPgYv+/OR//yb A==; X-CSE-ConnectionGUID: hd2gYj3dQrytHmyob305Fw== X-CSE-MsgGUID: 0podidUsTgG+zozoQVv5qw== X-IronPort-AV: E=McAfee;i="6700,10204,11222"; a="40779513" X-IronPort-AV: E=Sophos;i="6.11,199,1725346800"; d="scan'208";a="40779513" Received: from orviesa010.jf.intel.com ([10.64.159.150]) by orvoesa103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Nov 2024 06:52:18 -0700 X-CSE-ConnectionGUID: H5CFpOLOQAShyng0QthqTA== X-CSE-MsgGUID: PyjPTzLIRWeO5ho0c1OinQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,249,1725346800"; d="scan'208";a="82890873" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orviesa010.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 01 Nov 2024 06:52:19 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Fri, 1 Nov 2024 06:52:18 -0700 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Fri, 1 Nov 2024 06:52:18 -0700 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.41) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 1 Nov 2024 06:52:17 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=l2e4NYJ9QWw+2LzUNZdxZB1l16AhQSz7hT1/nBVWoMweDgy+MZezua5XBzMHE7Y4xqQSbGtSQmBQIS5g/lxFEF0OOgMCNg51CodX5y8J/QPVjleeg6W9ylCUd3lJkeLGmyGp0ytgrqeW0nB43mS2iJ9DAKGuQFdevPAquxEnp8lvqTXf6wSz5hLCMxK4vmJrxNjEa5zg8e0ICcD0cQkTRZ/rRaimWFGCCiXKrZH90p+8fz0rYxr7FTtgCYmF1vk2QbNYDDY4qsZGXc4nJ2pJ2dzdoaweM+a+ynF/mqaMnd20LwpytzdC++18/1vfdFbTMgZj7FoMYQsxkf1vW7zNCg== 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=0KSmjoMQPrgL4rQuSNQEhc5/4OGACbbqc5vK2vNHhwk=; b=jc+yS6/XdNCWwwRUDFF8lGYv8dFQOyEXWhXgee/rbIM0ML3euTUZiznk4iXsBVlUAaZhu+VUBCTX6xptYAGyYdBTl4QAtIju0ARWMhRfhWBp9VFVEFse2R5H49tlV8Z6HF7mdP4pY3l4dgFaykezIvB/Q1pg5vlZz+ZQXp7HTRIgv3dU0Zr2NCUYqmYd5pyBQUoRQUbN/jlshaFSRxdtuLN5+mUZLP3XvcFPZfw7fbNWmxJhQu785QpfgmDd058eT7BFGWmrxZRiUo86EAdDbOmGvn6dDwhZDNG4ywrwn54AdzCWHhHudx2lrRkr3X6ku9iCbuR8SwbUQPNYYIYCrg== 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 PH7PR11MB7075.namprd11.prod.outlook.com (2603:10b6:510:20e::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8093.34; Fri, 1 Nov 2024 13:52:15 +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; Fri, 1 Nov 2024 13:52:15 +0000 Date: Fri, 1 Nov 2024 13:52:09 +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: <14d31c7f-1a67-4b18-9e9f-d0aadeec49c2@allegro-packets.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <14d31c7f-1a67-4b18-9e9f-d0aadeec49c2@allegro-packets.com> X-ClientProxiedBy: MI1P293CA0009.ITAP293.PROD.OUTLOOK.COM (2603:10a6:290:2::6) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|PH7PR11MB7075:EE_ X-MS-Office365-Filtering-Correlation-Id: a965255e-0ce3-492d-2034-08dcfa7c6474 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|1800799024|376014|366016; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?Amh5Ps9sJzYNbZJGoZgXdHpg3uGwYtiyERsjmclIzRWt8zYVvAb35v/SsOJn?= =?us-ascii?Q?e/1+hI6LUKKWo0K7/5u9Mfn0UCzZePVSEgpmPMjGfmAYsXFdT/zhbFCUYQPf?= =?us-ascii?Q?sQVSh8Tf23zXVcr8XRUpzA0NVbW7IX+TeWBIA1ZOGsSCoJCCVpkAO2bt2Oan?= =?us-ascii?Q?4WBBT1xEFjPZgQ+7QgsQ+Z7WdQJG4V9Z6aeRIKE5BgUlTPgv3PTM4J1xoFap?= =?us-ascii?Q?JnbvnQiC9fbea7zODEs7g/so5kxqifqAR1El8twisBnjdXhq1IESg/olV8kU?= =?us-ascii?Q?ncFQQYbv93IA7xBpF1H9q87OrKRKK9Qln0BGiYhH57Y2tsUKP1UL3PNrQ4/u?= =?us-ascii?Q?E/yneI3iK2IHYEwrHcERSEOT7n63judb24lOUfW0iRgTkIYSYMIOOgQyL6n+?= =?us-ascii?Q?BWU3w2gx5hahvUt4ZiiKhU7jJpvvDbzaC2v6+7E2TpJGExt0urk+mC5d0VRo?= =?us-ascii?Q?72D0lTWFUx9JjABMEWQxTD6yjJJJM8AqbjSBDZNh+e3K3+NlVufoITKlw80w?= =?us-ascii?Q?NcwghNHp7jzb0It7BHfVOcwardNNzxbPq3cIfE0qdVX2c1Y2K2IjwSMgNxD8?= =?us-ascii?Q?wigecyY/AHq+nSxfBPMPT3NVNsGXlRwH/TLFPlBwAvYi8GkIkBNfBs9qgyoB?= =?us-ascii?Q?SKaNuYI3Pwo7jtkQHwYdEp5G0Bb1wyu+IeaGaYkNQL06YS+OXb06uEw+SAqC?= =?us-ascii?Q?kUWRFLtssyHpJj088TABS+GSL3y2zdbNVpajH5NluYuWey9hb2W9rT8q/i6d?= =?us-ascii?Q?rmn4VJ0mQzha+nNrhGRi5TTJRDX8AnAo7H2JgRQHBfXNW1juRrRo16oCkAGr?= =?us-ascii?Q?IARVb611NX3C3S4MGCQ5jCKSftIwHMpqyA172U8vq2Zp+gOUWb4vLuI17TND?= =?us-ascii?Q?uX8ESt9YJoiD4hrlkdOx/W13+aTSWmy6p0Fmkqeh7an7RaOXshk51/9vlQo1?= =?us-ascii?Q?gtXsmSmt/q8VrrmKIpcf9HWiY4DAM8IHWpsg2pHYL2aODmoKnCaF1PxnuEWu?= =?us-ascii?Q?3lhVPzspylsIpNQ847Bvp3jSl+kPd4DotfLjjDkkFn8PfWoH8XJ1sp+wcwj0?= =?us-ascii?Q?BCcPFQuuATBBhXSAjMSs/nB1Gp5NxdBoCoFLygy1T72UYNTf60gR8khUmvbn?= =?us-ascii?Q?DdFyvLt8QXZgPFAiBXpMUYkPCTGx6cum7e68ruF3SXf6KGcQgmbJTcsPOQUh?= =?us-ascii?Q?R99fQx5N9H8PtNf86LVvH8UOgehGl1gk2xAWR4LFJa0EigqMNMJEHV6eyPds?= =?us-ascii?Q?5NzbSLqE/3iu6gkhOORGGKJmVbvLdcuiC2mtffOfghjDKYFrGK+DuuXEKWAz?= =?us-ascii?Q?TUJcK5767cCOJ9B3OEmBs4as?= 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)(1800799024)(376014)(366016); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xNTSxIR7pETCOibyek5kDTGXZichvrAXIbD8RBmjpbMflzUYwU89LuVsQKe4?= =?us-ascii?Q?qQfUavsE14/nfJCl/AcTPzq4C97fc0DTS2j/78ze/pFygwLD94HTvboLaG+6?= =?us-ascii?Q?ZyzuQxlCasg+4x3RLoBVOFDdjQdnBiXU244ES5NALXxHGUMvPebfzwCH0PoR?= =?us-ascii?Q?YOc/nbxL1vE7snvTtREsMiKXUZ831iY2khVtZuE6fnBl+Na2C3n0FrP2Sn/4?= =?us-ascii?Q?MTcSSJVLMTbOUL/kHqoudOgLwBNXpgP+AvzLSc4MmAMjUXIonNQ39vMVL5tL?= =?us-ascii?Q?x3/q1tiIc+Pw64hHaEFvIykYIud3ChQciTE5p3F8z7gcF3+Et1WE9YuXXmW2?= =?us-ascii?Q?p8EVazWeiyEn7Ic/okATCdCRFebea6lSspbY4t2DaP+PKqrFQQepfdv4JImX?= =?us-ascii?Q?nfVBFHJkriEGKqg6ieuB3YFZn7368DsZI+2A3vvLZS0aoOFjkEFzPNp8Plp5?= =?us-ascii?Q?8PF+4R46y7b5RR1JOzMwmYHGYAAUoZHWE8eZGE1hB/5XZXTxP0qzJ197K8sm?= =?us-ascii?Q?A2/3YqGB/ZgO7+AhuVsaL4SVZaTNuDHbgHlDniqs5u48yaWG89YXy9P8rzhh?= =?us-ascii?Q?Gvqq88xxmTSOPuuR9dATajsxwcX8nLUvxLTASInSaMkwbaclpLDTodlrz0Oi?= =?us-ascii?Q?W0wePhkNgR3E66VuD/DKd5Y7UFP6vk7yaX6xwfIPnA+ptvreuzyjmDV3Xc8x?= =?us-ascii?Q?YL4mogNp+fYBsyV3dMTvahono0404sZLJebu1FM0NfGh9EDtv0YHR/y02B0r?= =?us-ascii?Q?ZDF5BbLeaE1R6KNbqeA2KtldyFgqlNuILdGXNMWGHsH9ODm8tZERMiQpDwZq?= =?us-ascii?Q?RR5gsZdd0hwBgqMWYD/DtWv3dT+y8GbS8eA/NckE8AWFw4S7nZ/hNRLh9VJU?= =?us-ascii?Q?xHqjR6QkWa5WoawG+4Mn5q4Y+Ba3GuOrUEyE/api/o9Fm0sUK7peVlJMKzf6?= =?us-ascii?Q?4voPbhNmY1NB813ppw9SjZIIe6dvTUlehMLQyS+GN5ikmxyPvG6wVDI0gO2l?= =?us-ascii?Q?o+k2SnLJJPtJKjKkQqOB4b6hXR3ABSKJxHxcwLk8NYblvYGg7wX8UXw8h2N9?= =?us-ascii?Q?Nu+bF+veqvPQBbCGAol9xHNPFcgYftY5hJno4jYSuH3SERcbgdFgddwPfaGK?= =?us-ascii?Q?24LoRQpYS9sbqL80DfGh0AZujYwKpUXBpavVXrJFZICA5ySATYQA7AImB1qH?= =?us-ascii?Q?X3aNmrXR5Vc+DAhfRMuLBNY7UUXti1Iqam3A9UxrmrlmBAvYucEnVK9mRFIt?= =?us-ascii?Q?Tp5SUARfTjryg4vlkhZPgfsl2nl+kQ+OTFrZPavoJdcM4v5i+7AR/hY+Vj/s?= =?us-ascii?Q?KoMeuSuu/x5tXrOw3U3XpfQoWIbpbUd0/43y4RKDwJnnDYreS2Wne6I1EvRr?= =?us-ascii?Q?PEqV5TCUeQY4KmMWWqbMeUuxnwQhTznETDli5o71oYDRDdS8+3MalOfJj6EA?= =?us-ascii?Q?r9v2a+n8iacO1iSrbsMWdDDXdFhkYkgCFRKWqzdXrzjuTRQk732OP7GoNabQ?= =?us-ascii?Q?K5wnD9+7WJwvZsQa8RyDTu8bMIWY8WtZY2RHJGIl2kGS8TZlIFyxnAJzxq70?= =?us-ascii?Q?Oz9vxBCHDJ7wxA1FFzGycaqAP18t4ddAx3wgX8tSizkEKRbiYmDyuqmJvcWj?= =?us-ascii?Q?xA=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: a965255e-0ce3-492d-2034-08dcfa7c6474 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Nov 2024 13:52:15.0994 (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: avG+qO8UmrhZq0TvpA32SkAh+q0qWUoyeSg6YZDCCT6oMhzH8rtmt+CDGZPR9RtgHZJ0OkXVyyOWWEdIRORJZ5JWK2g+4R2o5YQwnzEOz+w= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7075 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 Fri, Nov 01, 2024 at 02:42:20PM +0100, Martin Weiser wrote: > Hi Bruce, > > thank you very much for your feedback. > Please see my answers inline below. > > I will send a v2 of the patch. > > Best regards, > Martin > > > Am 29.10.24 um 18:42 schrieb Bruce Richardson: > > 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? > > This is exactly right. Took me a bit to understand what was happening here and > I do not know if there might be a better way do this than to change the start > offset of all descriptors. But there is probably a reason why it was done this way. > Initially the issue was detected as forwarding of packets that maxed out the MTU > failed since they now, with the increased length, exceeded the MTU. Only then it > became apparent that the scatter-gather handling was also broken. > > > > > 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. > > This will be part of patch v2. > > > * 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? > > I would actually prefer to keep it that way as not to mix up the the data buffer > address and length handling with the mbuf chain construction. > Ok, that is fine if you think it better. I'll just expect V2 with some added comments to clarify things. Thanks, /Bruce