All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Paul Moses <p@1g4.org>
Cc: <anthony.l.nguyen@intel.com>, <przemyslaw.kitszel@intel.com>,
	<andrew+netdev@lunn.ch>, <davem@davemloft.net>,
	<edumazet@kernel.org>, <kuba@kernel.org>, <pabeni@redhat.com>,
	<ast@kernel.org>, <daniel@iogearbox.net>, <hawk@kernel.org>,
	<john.fastabend@gmail.com>, <sdf@fomichev.me>,
	<vedang.patel@intel.com>, <jithu.joseph@intel.com>,
	<andre.guedes@intel.com>, <intel-wired-lan@lists.osuosl.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<bpf@vger.kernel.org>
Subject: Re: [PATCH net] igc: track rx frame boundaries independent of skbs
Date: Tue, 27 Oct 2026 09:23:57 +0100	[thread overview]
Message-ID: <auBfnRqpsuF4B+J9@boxer> (raw)
In-Reply-To: <XxgY3anjXL5MwVEpWmTA-tZLeDhfNvdTDJ3n_8jTLgnomanMs92AQS6-QHZyNzAi5tSQM4UDHEZyKs1sG39MrfHwma2BkaoxScclyny5KOw=@1g4.org>

On Mon, Oct 26, 2026 at 10:12:51PM +0000, Paul Moses wrote:
> 
> 
> 
> 
> On Monday, October 26th, 2026 at 2:46 PM, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:
> 
> > On Sun, Oct 25, 2026 at 06:52:06AM +0000, Paul Moses wrote:
> > > TSIP remains set on continuation descriptors, but the inline timestamp
> > > header exists only in the first buffer. Stripping it from every
> > > descriptor truncates split frames and can underflow short buffers.
> > >
> > > skb ownership cannot identify frame boundaries when XDP consumes the
> > > first buffer or AF_XDP uses zero-copy.
> > >
> > > Track rx_in_frame per ring and update it as descriptors are consumed.
> > > Strip timestamps only from first buffers, reject unsupported split
> > > frames before XDP and drain continuations. Account discarded
> > > descriptors against NAPI work without counting successful RX.
> > >
> > > Fixes: e1ed4f92a625 ("igc: Refactor Rx timestamp handling")
> > > Fixes: 26575105d6ed ("igc: Add initial XDP support")
> > > Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Paul Moses <p@1g4.org>
> > > ---
> > > - This supersedes "igc: only strip RX timestamp header from first buffer"
> > >   following feedback from Simon Horman, Maciej Fijalkowski and Sashiko.
> > >
> > > Changes:
> > > - track frame boundaries independently of skb ownership
> > > - guard timestamp stripping and drain unsupported split frames
> > > - cover the AF_XDP zero-copy RX path
> > > ---
> > >  drivers/net/ethernet/intel/igc/igc.h      |  1 +
> > >  drivers/net/ethernet/intel/igc/igc_main.c | 87 ++++++++++++++++-------
> > >  2 files changed, 64 insertions(+), 24 deletions(-)
> > 
> > Hi Paul,
> > 
> > I think this implementation went off track. We should make sure we *never*
> > enter data path for XDP/AF_XDP multi-buffer scenario until we add real
> > support to it within this driver. Now you sorta handle fragmented packets
> > in data path.
> > 
> > I tried to argue with bot reports that it hallucinated for
> > igc_clean_rx_irq() content (which was concise and to the point) and agreed
> > the AF_XDP should *not* be exposed to any fragmented traffic, which should
> > be addressed on HW RX ring configuration side, not within data path.
> > 
> > Also this should have been a v3 with a link to previous revisions so we
> > could keep track what has been changing between versions.
> > 
> > Here's something that would address ZC fragmentation issue. Could you give
> > it a spin on your side as I don't currently have igc in my setup handy.

[...]

> 
> Looks good to me, passes basic testing, and I would rather you/intel do this patch
> 
> For posterity, I did email Tony with handoff notes stating I'd rather not do the 
> xdp/xsk/af_xdp/fpe side of things, but hadn't heard back so I kept going. 

I think this was within the thread where AI hallucinated and I stepped in.
If you are not fed up yet you can take your v2 and the patch I added here
and send this as a change set. From my POV it would be better as you have
the HW to test this against and ironically my igc card went into
/dev/null's lab.

> 

  reply	other threads:[~2026-10-27  8:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-25  6:52 [PATCH net] igc: track rx frame boundaries independent of skbs Paul Moses
2026-10-25  6:54 ` netdev-bot+sinfo
2026-10-26  6:52 ` sashiko-bot
2026-10-26  7:53   ` Paul Moses
2026-10-26  7:43 ` netdev-bot+sashiko
2026-10-26  8:09   ` Paul Moses
2026-10-26 19:46 ` Maciej Fijalkowski
2026-10-26 22:12   ` Paul Moses
2026-10-27  8:23     ` Maciej Fijalkowski [this message]
2026-10-27 16:30       ` Paul Moses

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auBfnRqpsuF4B+J9@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=andre.guedes@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=hawk@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jithu.joseph@intel.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=p@1g4.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=sdf@fomichev.me \
    --cc=vedang.patel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.