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 <[email protected]>
---
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
On Thu, Feb 18, 2021 at 9:01 AM Marco Wenzel <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]
Tested-by: George McCollister <[email protected]
On Thu, Feb 18, 2021 at 6:06 PM : George McCollister <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]
> Tested-by: George McCollister <[email protected]
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.
Regards,
Marco Wenzel
On Fri, Feb 19, 2021 at 2:27 AM Wenzel, Marco <[email protected]> wrote:
>
> On Thu, Feb 18, 2021 at 6:06 PM : George McCollister <[email protected]> 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 <[email protected]>
> > > ---
> > > 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 <[email protected]
> > Tested-by: George McCollister <[email protected]
>
> 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 <[email protected]>
Tested-by: George McCollister <[email protected]>
>
>
> Regards,
> Marco Wenzel
On Fri, Feb 19, 2021 at 2:14 PM : George McCollister <[email protected]> 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
> <[email protected]> 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 <[email protected]>
> > > > ---
> > > > 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 <[email protected]
> > > Tested-by: George McCollister <[email protected]
> >
> > 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 <[email protected]>
> Tested-by: George McCollister <[email protected]>
>
Thank you for reviewing again! Is there any operation needed from my side in order to officially apply this patch?
> >
> > Regards,
> > Marco Wenzel
> > You must decide if you want to send it for net or net-next. If you want to
> > send it for net-next you must wait Linus has closed the merge window and
> > this shows open:
> > http://vger.kernel.org/~davem/net-next.html
> >
> > To send for net use the subject prefix "[PATCH net]".
> > To send for net-next use the subject prefix "[PATCH net-next]".
> >
> > If you're using git format-patch you can use the following:
> > git format-patch --subject-prefix='PATCH net-next'
> >
> > If you're just using git send-email you can use the --annotate option to edit
> > the patch subject manually.
> >
> > Thanks and sorry for not mentioning this before, George McCollister
>
> Thanks again for the very helpful hints. I hope the patch will be correct now.
Hi Marco
I know there is a lot of learn, doing the submission correct can be
harder than writing the code, at least for the first few patches.
One thing you missed is the patch version number in the Subject:
line. When there are multiple versions of a patch flying around, it
makes it easier to keep track of, if there is a version number.
Please try to remember this for you next patch. No need to resend just
because of this.
Andrew