[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXpXY0YSjA-ctoBa@thinkpad>
Date: Wed, 28 Jan 2026 19:37:23 +0100
From: Felix Maurer <fmaurer@...hat.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, jkarrenpalo@...il.com,
tglx@...utronix.de, mingo@...nel.org, allison.henderson@...cle.com,
petrm@...dia.com, antonio@...nvpn.net, bigeasy@...utronix.de,
Steffen Lindner <steffen.lindner@...abb.com>
Subject: Re: [PATCH net-next v2 4/9] hsr: Implement more robust duplicate
discard for PRP
On Wed, Jan 28, 2026 at 04:38:24PM +0000, Simon Horman wrote:
> On Thu, Jan 22, 2026 at 03:56:59PM +0100, Felix Maurer wrote:
>
> ...
>
> > diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
>
> ...
>
> > +/* Get the currently active sequence number block. If there is no block yet, or
> > + * the existing one is expired, a new block is created. The idea is to maintain
> > + * a "sparse bitmap" where a bitmap for the whole sequence number space is
> > + * split into blocks and not all blocks exist all the time. The blocks can
> > + * expire after time (in low traffic situations) or when they are replaced in
> > + * the backing fixed size buffer (in high traffic situations).
> > + */
> > +static struct hsr_seq_block *hsr_get_seq_block(struct hsr_node *node,
> > + u16 block_idx)
> > +{
> > + struct hsr_seq_block *block, *res;
> > +
> > + block = xa_load(&node->seq_blocks, block_idx);
> > +
> > + if (block && hsr_seq_block_is_old(block)) {
> > + hsr_forget_seq_block(node, block);
> > + block = NULL;
> > + }
> > +
> > + 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.
Thanks,
Felix
> [1] https://github.com/masoncl/review-prompts/
>
> > + return NULL;
> > +
> > + node->next_block =
> > + (node->next_block + 1) & (HSR_MAX_SEQ_BLOCKS - 1);
> > + }
> > +
> > + return block;
> > +}
> > +
>
> ...
Powered by blists - more mailing lists