Message-ID: <aXyJTzwJ3x34B-xR@thinkpad>
Date: Fri, 30 Jan 2026 11:34:55 +0100
From: Felix Maurer <fmaurer@...hat.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	jkarrenpalo@...il.com, tglx@...utronix.de, mingo@...nel.org,
	allison.henderson@...cle.com, petrm@...dia.com, antonio@...nvpn.net,
	Yoann Congal <yoann.congal@...le.fr>
Subject: Re: [PATCH net-next v2 6/9] hsr: Implement more robust duplicate
 discard for HSR

On Thu, Jan 29, 2026 at 07:01:02PM +0100, Felix Maurer wrote:
> On Thu, Jan 29, 2026 at 05:17:04PM +0100, Felix Maurer wrote:
> > On Thu, Jan 29, 2026 at 03:43:48PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2026-01-22 15:57:01 [+0100], Felix Maurer wrote:
> > > …
> > > > As the problem (we accidentally skip over a sequence number that has not
> > > > been received but will be received in the future) is similar to PRP, we can
> > > > apply a similar solution. The duplicate discard algorithm based on the
> > > > "sparse bitmap" works well for HSR if it is extended to track one bitmap
> > > > for each port (A, B, master, interlink). To do this, change the sequence
> > > > number blocks to contain a flexible array member as the last member that
> > > > can keep chunks for as many bitmaps as we need. This design makes it easy
> > > > to reuse the same algorithm in a potential PRP RedBox implementation.
> > >
> > > I know you just "copy" the logic based on what we have now but…
> > > Why do we have to track the sequence number for A, B and interlink? The
> > > 'master' port is what we feed into the stack so this needs to be
> > > de-duplicated. I am not sure how 'interlink' works so I keep quiet here.
> > > But A and B? There shouldn't be any duplicates on A and B unless the
> > > destination node forwards the node. Or do I miss something?
> > > I'm bringing this up because limiting to one (or two since I am unsure
> > > about interlink) would save some memory and avoid needless updates. And
> > > if you have HW-offloading enabled then you shouldn't see any packets
> > > which are not directed to _this_ node.
> >
> > About the interlink: that's the interface where you attach devices that
> > know nothing about HSR, i.e., when we are a RedBox. I consider it very
> > similar to the master port, it's our responsibility to de-duplicate what
> > we send out there.
> >
> > I was thinking about exactly this while working on the patch as well and
> > I came to each conclusion (A,B are needed vs. are not needed) at least
> > once. In the end, I think we will need it. It's right that a well
> > behaving node in the ring should not forward "frames for which the node
> > is the unique destination" (5.3.2.1). But there could be frames that
> > have no unique destination in the ring at all: multicast frames or
> > frames addressed to a non-existing MAC address. We should not forward
> > such frames either to prevent them from looping forever.
> >
> > Now, such frames should probably only reach back to us, if we sent them
> > (either from our stack or from the interlink port). We could track
> > sequence numbers sent to master and interlink (for de-duplication) and
> > sent by master and interlink (for loop prevention) for more clarity, but
> > then we're back at four bitmaps again.
>
> Please disregard this. If we are the source, we can easily detect it
> from the frame. I'll consider this again. Like I wrote, I'm jumping
> between necessary and not.

I took another look at this. I think the IEC 62439-3:2021 is not super
clear on this (it often refers to "duplicate frames" without being exact
about in which domain they are duplicates). But the section on HSR Modes
(5.3.2.1) is quite telling when all modes are read in combination.

First, there is the mode H, which is the default and mandatory to
implement. In this mode, a node should forward all frames "except for
frames sent by the node itself, duplicate frames and frames for which
the node is the unique destination". Note that "duplicate frame" is not
further specified here. As we don't implement different modes, we should
follow mode H in our implementation.

In contrast, there is also mode X (sometimes referred to as "traffic
reduction").  It is supposed to work like mode H, but without sending
"counter-duplicates", i.e., frames that are "a duplicate of a frame that
it received [...] from the opposite direction."

To me, this means two things for mode H, i.e., what we should be doing:
- For a frame with one sequence number coming from one node, we should
  be forwarding the frame once in each direction.
- There could be duplicates of these frames in either direction that we
  should not forward. This is also hinted at in other parts of the
  standard, that there could be multiple duplicates, especially when HSR
  rings are coupled.

Therefore, I think it is correct to do the duplicate tracking once for
each port, especially separately for port A and port B.

Thanks,
   Felix


> > I agree that we can and should optimize the HW-offloaded case. I'd
> > suggest to do that in a separate patchset, though, partly because I
> > don't have access to a hardware with HSR offload at the moment.
>

Powered by blists - more mailing lists