Subject: Re: [PATCH net-next v2 4/9] hsr: Implement more robust duplicate discard for PRP

From: Felix Maurer <[email-masked]>
Date: Tue, 3 Feb 2026 11:23:57 +0100

On Mon, Feb 02, 2026 at 05:57:02PM +0100, Sebastian Andrzej Siewior wrote:
> On 2026-01-28 19:37:23 [+0100], Felix Maurer wrote:
> > > > +	if (!block) {
> > > > +		block = &node->block_buf[node->next_block];
> > > > +		hsr_forget_seq_block(node, block);
> > > > +
> > > > +		memset(block, 0, sizeof(*block));
> > > > +		block->time = jiffies;
> > > > +		block->block_idx = block_idx;
> > > > +
> > > > +		res = xa_store(&node->seq_blocks, block_idx, block, GFP_ATOMIC);
> > > > +		if (xa_is_err(res))
> > >
> > > Hi Felix,
> > >
> > > I ran Claude Code over this with review-prompts [1] and it flags
> > > that in the error path above, the following is needed so that the
> > > block can be re-used.
> > >
> > > 			block->time = 0;
> >
> > I agree. It would nonetheless be reused at some point, but not setting
> > time = 0 may lead to an unexpected block getting removed when it is
> > reused (or at least an attempt to do so). I'll fix this in v3.
>
> Not sure I follow. If xa_store() fails then the block is not added to
> the "sequence-blocks" and it will be attempted once the next packet is
> received, right?. Otherwise node->next_block is not updated at which
> point this block will be added twice which sounds worse.

Yes, it will be attempted again on the next frame that needs a new
block. But every attempt to recycle the next_block starts with calling
hsr_forget_seq_block(), which tries to xa_erase() the block if time!=0.
Generally, time==0 is the marker that a block is not stored in the
xarray. So for consistency it's correct to set time=0 if the block can't
be added to the xarray, even though I don't think it would cause any
trouble at the moment if the time is not reset.

Thanks,
   Felix