From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 240A42836A0 for ; Wed, 13 May 2026 09:07:35 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778663257; cv=none; b=EF2W3hTCH5q1MLhgmwBUNhq/gILISiNeBkjIjUuVTB9WJVEv3XbTm34LRpRR3tjlYDBX5GxyEqvL1YB7uxCbCERQX8cdmJIdx15Y1NWk6IVdcDSSmBrOoMnMgSAHPF0K50ikdCcrkgB5sfhXwrsMP8j6qEL6DuhGcBKBQ9GkgPc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778663257; c=relaxed/simple; bh=ERwsM3PJL5y8q+Lhj4NVYS3FtGsrJrNfL6nETyLWeik=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=KzoAfv1y0dVivmPNXXGQG2VnEOZIUkHP0uz+fe7tmUwwVBvw1oUlyvI87CuOwrV0pWl60cYa/6SnXw1MKqtEFNtd4x5bogVsK0QH6J1SzuXXsVVlTDE8wdJg/ReZMX5X3LJjVdDmuOrGYb8PjbC7jFPoxemWdQYR+zS3WLdJ68U= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=J2cq6pwc; arc=none smtp.client-ip=209.85.128.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="J2cq6pwc" Received: by mail-wm1-f45.google.com with SMTP id 5b1f17b1804b1-48e82c23840so28648105e9.3 for ; Wed, 13 May 2026 02:07:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1778663254; x=1779268054; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=qN/W8ebLgutWz02+3OnJOFBU8z3ZOcJD73mdZlWwDCo=; b=J2cq6pwcDO2IC+Mc/8cYcTi8F1NkF78835iJBAZdO75gebMz+98F2IB9CSI040LdHo 2TydZOoxCXlx0M/S1EEieyq/sTl9Z4NNBwJbZXTbm/N1KARxLuRmgoJv7T2+Y2qZG370 lXUEEiH0Td/Zp1DwCRkpdpKn571UM+Co/UmVUkL6bsxjxOBqQtcq/uU1aVjeBqSQZRuH YVlOk99sMYmJDSeF4mrvRN+6Ysp7unme3EEWSEzpep9OLcnXZfy/kCAP/Ovnya6i8mFJ l7/8CLXdmPMlxir6tuFTkozkzqxw6ur0z04NnA46vCnpe5zuJFoMROHZaDV4Mzv+A2Y9 Bxjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778663254; x=1779268054; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=qN/W8ebLgutWz02+3OnJOFBU8z3ZOcJD73mdZlWwDCo=; b=jVv+vRvIZzF2UCfJLLV8DelsdUgEu8BPuR7da8cHwzfryi3RWXoMPiw8dSSwlIcCLX bbf1/4WboLdl1ci792Y0uJXVbXF7NU8lMJgdXJXJmculMuysNroqL+A0uo2MjV9B9q9x iqzsbRATt3h7r1mAmB8QLREVaoyaU5UKQRQXTI7dSUN7j1rEMG6vZ0hJzgTUTTa/WH78 LbE5NVhIY+3nXt4SYKzIMxsrDunY/gJK3dTAZhJE4jC4UXsJI8P6xPdLqzWlLjxTiJNT 0+lPgS7TZP76yqgUadjAJcQ+ZXmuxwrRRCgYxwwqH/Ukxx+8y+KYpmgACjpNIH+dGPew wRLQ== X-Forwarded-Encrypted: i=1; AFNElJ90Zs8deqVbkWgeOYLFLBZX0FwV9cp3yaEooFS2X/3JvE68Q0smfWBTS0zeNApqDG8W9WK0g+E=@vger.kernel.org X-Gm-Message-State: AOJu0YwDL2tpnFDxhWQmTaFtVCxPWAKmc4SUrOerVdJ93dOUlY44XmcH 2nEjlZUht7Q3z2crjLUbqqxmu9IRvyyAUzgzs8XfRyxXqLbMikYXeTTa X-Gm-Gg: Acq92OEhWFrvZA5AG9jCzOigfq2Vk9UynFXtCuZDQsMgXBs01QIaZ9RVs7gLo/yhWGc 7q8sYUbIx258/gOtLpCiBrGvKenDOehEEm3oRGw4wcDvgcXRan3NS/Cs4mbh1sVI9iotRWPj2fs h5pk8R3CZx2Po30MPPHeAkp0tNnouRyKt+feYlCNRTADGCqaPHsLe3bzxrRW5J08ivaqj1BYdr6 bAawayskDtFQT72sED0HIE6m1RavHlXnfnB0GPylgDHWJxLxKPY6ehHLvNopvCgTYrFGmuJiB7M UZCNRB/6OGb2aUjOD7WuK4TLoI5808wVwmvwqdD14SUEVB+BtgSzL1pGXEy+Iv4PJcYOYtCDfNx e6zqxSvL/z+2bDT+NlkWrOAclqEIqCqsDR3ErzoKT2GIa/jDu1zQzLlpK8K8wRfoMDh5+e7QNSy hdGfFQGIcwYARfWxDQw8493p7MIRBoUSCaM4A7bVcldYzg9eCVmjNcPqyupTNa X-Received: by 2002:a05:600c:3b84:b0:48a:8b02:ae91 with SMTP id 5b1f17b1804b1-48fc9a0e266mr36229105e9.11.1778663254336; Wed, 13 May 2026 02:07:34 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48fce06ca2csm30681655e9.8.2026.05.13.02.07.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 May 2026 02:07:34 -0700 (PDT) Date: Wed, 13 May 2026 10:07:32 +0100 From: David Laight To: John Ousterhout Cc: stable@vger.kernel.org, anthony.l.nguyen@intel.com, intel-wired-lan@lists.osuosl.org, przemyslaw.kitszel@intel.com, netdev@vger.kernel.org, jacob.e.keller@intel.com Subject: Re: [PATCH net v3] ice: fix packet corruption due to extraneous page flip Message-ID: <20260513100732.499e3f49@pumpkin> In-Reply-To: <20260512181953.1689-1-ouster@cs.stanford.edu> References: <20260512181953.1689-1-ouster@cs.stanford.edu> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Tue, 12 May 2026 11:19:53 -0700 John Ousterhout wrote: > Consider the following sequence of events: > * The bottom half of a buffer page is filled with data from > packet A. The page has a net reference count (reference count > - bias) of 1. The page is returned to the NIC, flipped to > use the top half. > * Before the reference on the page is released, the NIC returns > the page with no data in it ('size' is zero in ice_clean_rx_irq). > In this case the bias does not get decremented. The page still > has a net reference count of 1, so it gets returned to the NIC. > However, ice_put_rx_mbuf flipped the page so that the bottom > half is active. > * If the NIC stores another packet in the page before packet A > has released its reference, the data in packet A will be > overwritten with data from the new packet. > * Unfortunately zero-length buffers occur frequently: they seem > to occur whenever a packet uses every available byte in a > buffer, ending precisely at the end of the buffer. When this > happens the NIC seems to generate an extra zero-length > buffer. > The fix is for ice_put_rx_mbuf not to flip pages that have a > size of 0. How is this different from packet B (in the top half) being freed before packet A (in the bottom half)? > This patch applies directly to longterm stable versions 6.18.27 > and 6.12.86; it also seems relevant for 6.6.137 but would need > modifcations for that version. I have not examined earlier > versions. > > Unfortunately there is no upstream commit id for this patch because > the ICE driver has undergone a major revision (libeth refactor and > pagepool conversion) that eliminated the buggy code. Thus the > problem no longer exists in the main line. > > Cc: stable@vger.kernel.org # 6.12+ > Signed-off-by: John Ousterhout > --- > drivers/net/ethernet/intel/ice/ice_txrx.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c > index 51c459a3e722..081c7a7392b7 100644 > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c > @@ -1215,6 +1215,13 @@ static void ice_put_rx_mbuf(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, > xdp_frags = xdp_get_shared_info_from_buff(xdp)->nr_frags; > > while (idx != ntc) { > + union ice_32b_rx_flex_desc *rx_desc; > + unsigned int size; > + > + rx_desc = ICE_RX_DESC(rx_ring, idx); > + size = le16_to_cpu(rx_desc->wb.pkt_len) & > + ICE_RX_FLX_DESC_PKT_LEN_M; > + Looks like you only need to calculate 'size' for the !ICE_XDP_CONSUMED path. You could also use the (likely cheaper) test for zero: if (!(rx_desc->wb.pkt_len & cpu_to_le16(ICE_RX_FLX_DESC_PKT_LEN_M)) -- David > buf = &rx_ring->rx_buf[idx]; > if (++idx == cnt) > idx = 0; > @@ -1224,10 +1231,20 @@ static void ice_put_rx_mbuf(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, > * To do this, only adjust pagecnt_bias for fragments up to > * the total remaining after the XDP program has run. > */ > - if (verdict != ICE_XDP_CONSUMED) > - ice_rx_buf_adjust_pg_offset(buf, xdp->frame_sz); > - else if (i++ <= xdp_frags) > + if (verdict != ICE_XDP_CONSUMED) { > + /* Don't "flip" the page if size is 0: in this case > + * the data in the current half will not be used so > + * it's OK to reuse that half. And, since the bias > + * didn't get decremented for this half, the page can > + * be returned to the NIC even if the other half is > + * still in use, so flipping the page could cause > + * live packet data to be overwritten. > + */ > + if (size != 0) > + ice_rx_buf_adjust_pg_offset(buf, xdp->frame_sz); > + } else if (i++ <= xdp_frags) { > buf->pagecnt_bias++; > + } > > ice_put_rx_buf(rx_ring, buf); > }