[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <d79b32d366ca49aca7821cb0b0edf52b@EXCH-SVR2013.eberle.local>
Date: Mon, 22 Feb 2021 13:38:06 +0000
From: "Wenzel, Marco" <Marco.Wenzel@...berle.de>
To: George McCollister <george.mccollister@...il.com>
CC: "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Murali Karicheri <m-karicheri2@...com>,
Taehee Yoo <ap420073@...il.com>,
"Amol Grover" <frextrite@...il.com>,
YueHaibing <yuehaibing@...wei.com>,
"Arvid Brodin" <Arvid.Brodin@...n.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: hsr: add support for EntryForgetTime
On Fri, Feb 19, 2021 at 2:14 PM : George McCollister <george.mccollister@...il.com> wrote:
>
> On Fri, Feb 19, 2021 at 2:27 AM Wenzel, Marco <Marco.Wenzel@a-
> eberle.de> wrote:
> >
> > On Thu, Feb 18, 2021 at 6:06 PM : George McCollister
> <george.mccollister@...il.com> wrote:
> > >
> > > On Thu, Feb 18, 2021 at 9:01 AM Marco Wenzel <marco.wenzel@a-
> > > eberle.de> wrote:
> > > >
> > > > In IEC 62439-3 EntryForgetTime is defined with a value of 400 ms.
> > > > When a node does not send any frame within this time, the sequence
> > > > number check for can be ignored. This solves communication issues
> > > > with Cisco IE 2000 in Redbox mode.
> > > >
> > > > Fixes: f421436a591d ("net/hsr: Add support for the
> > > > High-availability Seamless Redundancy protocol (HSRv0)")
> > > > Signed-off-by: Marco Wenzel <marco.wenzel@...berle.de>
> > > > ---
> > > > net/hsr/hsr_framereg.c | 9 +++++++-- net/hsr/hsr_framereg.h | 1
> > > > +
> > > > net/hsr/hsr_main.h | 1 +
> > > > 3 files changed, 9 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c index
> > > > 5c97de459905..805f974923b9 100644
> > > > --- a/net/hsr/hsr_framereg.c
> > > > +++ b/net/hsr/hsr_framereg.c
> > > > @@ -164,8 +164,10 @@ static struct hsr_node *hsr_add_node(struct
> > > hsr_priv *hsr,
> > > > * as initialization. (0 could trigger an spurious ring error warning).
> > > > */
> > > > now = jiffies;
> > > > - for (i = 0; i < HSR_PT_PORTS; i++)
> > > > + for (i = 0; i < HSR_PT_PORTS; i++) {
> > > > new_node->time_in[i] = now;
> > > > + new_node->time_out[i] = now;
> > > > + }
> > > > for (i = 0; i < HSR_PT_PORTS; i++)
> > > > new_node->seq_out[i] = seq_out;
> > > >
> > > > @@ -411,9 +413,12 @@ void hsr_register_frame_in(struct hsr_node
> > > *node,
> > > > struct hsr_port *port, int hsr_register_frame_out(struct hsr_port
> > > > *port,
> > > struct hsr_node *node,
> > > > u16 sequence_nr) {
> > > > - if (seq_nr_before_or_eq(sequence_nr, node->seq_out[port-
> >type]))
> > > > + if (seq_nr_before_or_eq(sequence_nr,
> > > > + node->seq_out[port->type])
> > > &&
> > > > + time_is_after_jiffies(node->time_out[port->type] +
> > > > + msecs_to_jiffies(HSR_ENTRY_FORGET_TIME)))
> > > > return 1;
> > > >
> > > > + node->time_out[port->type] = jiffies;
> > > > node->seq_out[port->type] = sequence_nr;
> > > > return 0;
> > > > }
> > > > diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h index
> > > > 86b43f539f2c..d9628e7a5f05 100644
> > > > --- a/net/hsr/hsr_framereg.h
> > > > +++ b/net/hsr/hsr_framereg.h
> > > > @@ -75,6 +75,7 @@ struct hsr_node {
> > > > enum hsr_port_type addr_B_port;
> > > > unsigned long time_in[HSR_PT_PORTS];
> > > > bool time_in_stale[HSR_PT_PORTS];
> > > > + unsigned long time_out[HSR_PT_PORTS];
> > > > /* if the node is a SAN */
> > > > bool san_a;
> > > > bool san_b;
> > > > diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h index
> > > > 7dc92ce5a134..f79ca55d6986 100644
> > > > --- a/net/hsr/hsr_main.h
> > > > +++ b/net/hsr/hsr_main.h
> > > > @@ -21,6 +21,7 @@
> > > > #define HSR_LIFE_CHECK_INTERVAL 2000 /* ms */
> > > > #define HSR_NODE_FORGET_TIME 60000 /* ms */
> > > > #define HSR_ANNOUNCE_INTERVAL 100 /* ms */
> > > > +#define HSR_ENTRY_FORGET_TIME 400 /* ms */
> > > >
> > > > /* By how much may slave1 and slave2 timestamps of latest
> > > > received
> > > frame from
> > > > * each node differ before we notify of communication problem?
> > > > --
> > > > 2.30.0
> > > >
> > >
> > > scripts/checkpatch.pl gives errors about DOS line endings but once
> > > that is resolved this looks good. I tested it on an HSR network with
> > > the software implementation and the xrs700x which uses offloading
> > > and everything still works. I don't have a way to force anything on
> > > the HSR network to reuse sequence numbers after 400ms.
> > >
> > > Reviewed-by: George McCollister <george.mccollister@...il.com
> > > Tested-by: George McCollister <george.mccollister@...il.com
> >
> > Thank you very much for reviewing, testing and supporting!
> >
> > Where do you see the incorrect line endings? I just ran scripts/checkpath.pl
> as git commit hook and it did not report any errors. When I run it again
> manually, it also does not report any errors:
> >
> > # ./scripts/checkpatch.pl --strict
> > /tmp/0001-net-hsr-add-support-for-EntryForgetTime.patch
> > total: 0 errors, 0 warnings, 0 checks, 38 lines checked
> >
> > /tmp/0001-net-hsr-add-support-for-EntryForgetTime.patch has no obvious
> style problems and is ready for submission.
>
> Sorry about this. It seems when I downloaded the patch with Chromium
> from gmail in Linux it added DOS new lines (this is unexpected). When I
> downloaded it from lore.kernel.org it's fine.
>
> Reviewed-by: George McCollister <george.mccollister@...il.com>
> Tested-by: George McCollister <george.mccollister@...il.com>
>
Thank you for reviewing again! Is there any operation needed from my side in order to officially apply this patch?
> >
> > Regards,
> > Marco Wenzel
Powered by blists - more mailing lists