[PATCH v2] utils/net/rtnet.in: add delay during master configuration

[PATCH v2] utils/net/rtnet.in: add delay during master configuration

[Mauro S.]

Some cards are slow to get the connection link up after the
"rtifconfig rteth0 up" command, e.g. on an Atom-x5 with an Intel I210
(rt_igb driver) I detected approximately 3 seconds to get the link up.

On master, the "rtifconfig rteth0 up" is followed by TDMA configuration and
start. After the TDMA start, the sync packet is sent at the defined 
cycle time.

Sometimes, after "rtnet start", the dmesg fills with this error:

   TDMA: Failed to transmit sync frame!

and the rt driver locks. Then, the kernel watchdog is triggered and the
NIC is hw-reset by the kernel, producing more errors and another lock.
Sometimes the dmesg only fills with the error message and the NIC does
not lock.
This happens because the interface is not up and ready to handle
the sync packets when TDMA is started.

This patch introduces a configurable delay between the "rtifconfig 
rteth0 up"
and the TDMA start on master host. This allows to avoid these kind of 
problems.

Signed-off-by: Mauro Salvini <mau.salvi@tin.it>
---
  utils/net/rtnet.in | 21 +++++++++++++++++++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/utils/net/rtnet.in b/utils/net/rtnet.in
index f81a7bb0a..8c2dcdf6e 100644
--- a/utils/net/rtnet.in
+++ b/utils/net/rtnet.in
@@ -15,15 +15,19 @@ debug_func() {
  usage() {
      cat << EOF
  Usage:
-    $0 [-cf <config-file>] [-v] [-c] {start|stop}
+    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] {start|stop}
  	Start or stop station according to configuration file

-    $0 [-cf <config-file>] [-v] [-c] master <slave_ip1> [<slave_ip2> ...]
+    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] master <slave_ip1> 
[<slave_ip2> ...]
  	Start station as master for given list of slaves

      $0 [-cf <config-file>] [-v] capture
  	Start only passive realtime capturing

+    The parameter -d allows to introduce a delay in seconds between the
+	"rtifconfig rtethX up" command and the TDMA start on the host
+	configured as master. Useful to avoid errors/card locks when the
+	RT NIC is slow to get the link up.
      The additional switch -v enables verbose output.
      The additional switch -c enables capturing mode to allow use of a 
network
  	analyzer such as Wireshark (if rtnet was built with --enable-rtcap).
@@ -76,6 +80,10 @@ submit_cfg() {
  	master)
  	    $RTIFCONFIG rteth0 up $STATION_IP

+	    if [ -n "$UPDELAY" ]; then
+		sleep $UPDELAY
+	    fi
+
  	    $TDMACFG rteth0 master $TDMA_CYCLE
  	    eval "$TDMA_SLOTS"

@@ -144,6 +152,10 @@ start_master() {

  	$RTIFCONFIG rteth0 up $IPADDR $NETMASK_OPT

+	if [ -n "$UPDELAY" ]; then
+	    sleep $UPDELAY
+	fi
+
  	$TDMACFG rteth0 master $TDMA_CYCLE
  	$TDMACFG rteth0 slot 0 0

@@ -258,6 +270,11 @@ else
      exit 1
  fi

+if [ "$1" = "-d" ]; then
+    UPDELAY="$2"
+    shift 2
+fi
+
  if [ "$1" = "-v" ]; then
      echo "Turning on verbose mode"
      RTIFCONFIG="debug_func $RTIFCONFIG"
-- 
2.17.1

Re: [PATCH v2] utils/net/rtnet.in: add delay during master configuration

[Florian Bezdeka]

Hi all,

some minor comments, see below:

On 22.04.22 20:58, Mauro S. via Xenomai wrote:
> Some cards are slow to get the connection link up after the
> "rtifconfig rteth0 up" command, e.g. on an Atom-x5 with an Intel I210
> (rt_igb driver) I detected approximately 3 seconds to get the link up.
> 
> On master, the "rtifconfig rteth0 up" is followed by TDMA configuration and
> start. After the TDMA start, the sync packet is sent at the defined
> cycle time.
> 
> Sometimes, after "rtnet start", the dmesg fills with this error:
> 
>   TDMA: Failed to transmit sync frame!
> 
> and the rt driver locks. Then, the kernel watchdog is triggered and the
> NIC is hw-reset by the kernel, producing more errors and another lock.
> Sometimes the dmesg only fills with the error message and the NIC does
> not lock.
> This happens because the interface is not up and ready to handle
> the sync packets when TDMA is started.
> 
> This patch introduces a configurable delay between the "rtifconfig
> rteth0 up"
> and the TDMA start on master host. This allows to avoid these kind of
> problems.
> 
> Signed-off-by: Mauro Salvini <mau.salvi@tin.it>
> ---
>  utils/net/rtnet.in | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/net/rtnet.in b/utils/net/rtnet.in
> index f81a7bb0a..8c2dcdf6e 100644
> --- a/utils/net/rtnet.in
> +++ b/utils/net/rtnet.in
> @@ -15,15 +15,19 @@ debug_func() {
>  usage() {
>      cat << EOF
>  Usage:
> -    $0 [-cf <config-file>] [-v] [-c] {start|stop}
> +    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] {start|stop}
>      Start or stop station according to configuration file
> 
> -    $0 [-cf <config-file>] [-v] [-c] master <slave_ip1> [<slave_ip2> ...]
> +    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] master <slave_ip1>
> [<slave_ip2> ...]
>      Start station as master for given list of slaves
> 
>      $0 [-cf <config-file>] [-v] capture
>      Start only passive realtime capturing
> 
> +    The parameter -d allows to introduce a delay in seconds between the
> +    "rtifconfig rtethX up" command and the TDMA start on the host
> +    configured as master. Useful to avoid errors/card locks when the
> +    RT NIC is slow to get the link up.
>      The additional switch -v enables verbose output.
>      The additional switch -c enables capturing mode to allow use of a
> network
>      analyzer such as Wireshark (if rtnet was built with --enable-rtcap).
> @@ -76,6 +80,10 @@ submit_cfg() {
>      master)
>          $RTIFCONFIG rteth0 up $STATION_IP
> 
> +        if [ -n "$UPDELAY" ]; then

Checked with shellcheck? I would expect ${UPDELAY} here and below.

> +        sleep $UPDELAY

Indention looks broken.

> +        fi
> +
>          $TDMACFG rteth0 master $TDMA_CYCLE
>          eval "$TDMA_SLOTS"
> 
> @@ -144,6 +152,10 @@ start_master() {
> 
>      $RTIFCONFIG rteth0 up $IPADDR $NETMASK_OPT
> 
> +    if [ -n "$UPDELAY" ]; then
> +        sleep $UPDELAY
> +    fi
> +
>      $TDMACFG rteth0 master $TDMA_CYCLE
>      $TDMACFG rteth0 slot 0 0
> 
> @@ -258,6 +270,11 @@ else
>      exit 1
>  fi
> 
> +if [ "$1" = "-d" ]; then
> +    UPDELAY="$2"
> +    shift 2
> +fi
> +
>  if [ "$1" = "-v" ]; then
>      echo "Turning on verbose mode"
>      RTIFCONFIG="debug_func $RTIFCONFIG"

Re: [PATCH v2] utils/net/rtnet.in: add delay during master configuration

[Mauro S.]

Hi Florian,

thanks for your comments. See below mine.

Il 23/04/22 19:26, Florian Bezdeka ha scritto:
> Hi all,
> 
> some minor comments, see below:
> 
> On 22.04.22 20:58, Mauro S. via Xenomai wrote:
>> Some cards are slow to get the connection link up after the
>> "rtifconfig rteth0 up" command, e.g. on an Atom-x5 with an Intel I210
>> (rt_igb driver) I detected approximately 3 seconds to get the link up.
>>
>> On master, the "rtifconfig rteth0 up" is followed by TDMA configuration and
>> start. After the TDMA start, the sync packet is sent at the defined
>> cycle time.
>>
>> Sometimes, after "rtnet start", the dmesg fills with this error:
>>
>>    TDMA: Failed to transmit sync frame!
>>
>> and the rt driver locks. Then, the kernel watchdog is triggered and the
>> NIC is hw-reset by the kernel, producing more errors and another lock.
>> Sometimes the dmesg only fills with the error message and the NIC does
>> not lock.
>> This happens because the interface is not up and ready to handle
>> the sync packets when TDMA is started.
>>
>> This patch introduces a configurable delay between the "rtifconfig
>> rteth0 up"
>> and the TDMA start on master host. This allows to avoid these kind of
>> problems.
>>
>> Signed-off-by: Mauro Salvini <mau.salvi@tin.it>
>> ---
>>   utils/net/rtnet.in | 21 +++++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/utils/net/rtnet.in b/utils/net/rtnet.in
>> index f81a7bb0a..8c2dcdf6e 100644
>> --- a/utils/net/rtnet.in
>> +++ b/utils/net/rtnet.in
>> @@ -15,15 +15,19 @@ debug_func() {
>>   usage() {
>>       cat << EOF
>>   Usage:
>> -    $0 [-cf <config-file>] [-v] [-c] {start|stop}
>> +    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] {start|stop}
>>       Start or stop station according to configuration file
>>
>> -    $0 [-cf <config-file>] [-v] [-c] master <slave_ip1> [<slave_ip2> ...]
>> +    $0 [-cf <config-file>] [-d <delay>] [-v] [-c] master <slave_ip1>
>> [<slave_ip2> ...]
>>       Start station as master for given list of slaves
>>
>>       $0 [-cf <config-file>] [-v] capture
>>       Start only passive realtime capturing
>>
>> +    The parameter -d allows to introduce a delay in seconds between the
>> +    "rtifconfig rtethX up" command and the TDMA start on the host
>> +    configured as master. Useful to avoid errors/card locks when the
>> +    RT NIC is slow to get the link up.
>>       The additional switch -v enables verbose output.
>>       The additional switch -c enables capturing mode to allow use of a
>> network
>>       analyzer such as Wireshark (if rtnet was built with --enable-rtcap).
>> @@ -76,6 +80,10 @@ submit_cfg() {
>>       master)
>>           $RTIFCONFIG rteth0 up $STATION_IP
>>
>> +        if [ -n "$UPDELAY" ]; then
> 
> Checked with shellcheck? I would expect ${UPDELAY} here and below.

In fact, among many (info) and (warning) messages regarding other lines, 
shellcheck says about this row:

   Line 84:
                   sleep $UPDELAY
                         ^-- SC2086 (info): Double quote to prevent 
globbing and word splitting.

   Did you mean: (apply this, apply all SC2086)
                   sleep "$UPDELAY"

What should I do?

> 
>> +        sleep $UPDELAY
> 
> Indention looks broken.

You are right, fixed thanks.

> 
>> +        fi
>> +
>>           $TDMACFG rteth0 master $TDMA_CYCLE
>>           eval "$TDMA_SLOTS"
>>
>> @@ -144,6 +152,10 @@ start_master() {
>>
>>       $RTIFCONFIG rteth0 up $IPADDR $NETMASK_OPT
>>
>> +    if [ -n "$UPDELAY" ]; then
>> +        sleep $UPDELAY
>> +    fi
>> +
>>       $TDMACFG rteth0 master $TDMA_CYCLE
>>       $TDMACFG rteth0 slot 0 0
>>
>> @@ -258,6 +270,11 @@ else
>>       exit 1
>>   fi
>>
>> +if [ "$1" = "-d" ]; then
>> +    UPDELAY="$2"
>> +    shift 2
>> +fi
>> +
>>   if [ "$1" = "-v" ]; then
>>       echo "Turning on verbose mode"
>>       RTIFCONFIG="debug_func $RTIFCONFIG"
> 

-- 
Mauro S.

Re: [PATCH v2] utils/net/rtnet.in: add delay during master configuration

[Bezdeka, Florian]

On Tue, 2022-04-26 at 16:24 +0200, Mauro S. wrote:
> Hi Florian,
> 
> thanks for your comments. See below mine.
> 
> Il 23/04/22 19:26, Florian Bezdeka ha scritto:
> > Hi all,
> > 
> > some minor comments, see below:

[snip] 
> > > 
> > > +    The parameter -d allows to introduce a delay in seconds between the
> > > +    "rtifconfig rtethX up" command and the TDMA start on the host
> > > +    configured as master. Useful to avoid errors/card locks when the
> > > +    RT NIC is slow to get the link up.
> > >       The additional switch -v enables verbose output.
> > >       The additional switch -c enables capturing mode to allow use of a
> > > network
> > >       analyzer such as Wireshark (if rtnet was built with --enable-rtcap).
> > > @@ -76,6 +80,10 @@ submit_cfg() {
> > >       master)
> > >           $RTIFCONFIG rteth0 up $STATION_IP
> > > 
> > > +        if [ -n "$UPDELAY" ]; then
> > 
> > Checked with shellcheck? I would expect ${UPDELAY} here and below.
> 
> In fact, among many (info) and (warning) messages regarding other lines, 
> shellcheck says about this row:
> 
>    Line 84:
>                    sleep $UPDELAY
>                          ^-- SC2086 (info): Double quote to prevent 
> globbing and word splitting.
> 
>    Did you mean: (apply this, apply all SC2086)
>                    sleep "$UPDELAY"
> 
> What should I do?

My understanding is that "${UPDELAY}" is the correct thing here.
In case shellcheck reports more issues in the same file I would prefer
two patches:

1) Fixing shellcheck reportings
2) This one on top

But at the end it's the maintainers' decision.

> 
> > 
> > 
> 

RE: [PATCH v2] utils/net/rtnet.in: add delay during master configuration

[dietmar.schindler]

> -----Original Message-----
> From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Bezdeka, Florian via Xenomai
> Sent: Tuesday, April 26, 2022 5:18 PM
> To: mau.salvi@tin.it; xenomai@xenomai.org
> …
> My understanding is that "${UPDELAY}" is the correct thing here.

                   sleep "$UPDELAY"

is just as correct.

--
Regards,
Dietmar Schindler
________________________________
manroland Goss web systems GmbH | Managing Director: Franz Kriechbaum
Registered Office: Augsburg | Trade Register: AG Augsburg | HRB-No.: 32609 | VAT: DE815764857

Confidentiality note:
This message and any attached documents may contain confidential or proprietary information of manroland|Goss. These materials are intended only for the use of the intended recipient. If you are not the intended recipient of this transmission, you are hereby notified that any distribution, disclosure, printing, copying, storage, modification or the taking of any action in reliance upon this transmission is strictly prohibited. Delivery of this message to any person other than the intended recipient shall not compromise or waive such confidentiality, privilege or exemption from disclosure as to this communication. If you have received this communication in error, please immediately notify the sender and delete the message from your system. All liability for viruses is excluded to the fullest extent permitted by law.
________________________________

Re: [PATCH v2] utils/net/rtnet.in: add delay during master configuration

[Mauro S.]

Il 26/04/22 17:18, Bezdeka, Florian ha scritto:
> On Tue, 2022-04-26 at 16:24 +0200, Mauro S. wrote:
>> Hi Florian,
>>
>> thanks for your comments. See below mine.
>>
>> Il 23/04/22 19:26, Florian Bezdeka ha scritto:
>>> Hi all,
>>>
>>> some minor comments, see below:
> 
> [snip]
>>>>
>>>> +    The parameter -d allows to introduce a delay in seconds between the
>>>> +    "rtifconfig rtethX up" command and the TDMA start on the host
>>>> +    configured as master. Useful to avoid errors/card locks when the
>>>> +    RT NIC is slow to get the link up.
>>>>        The additional switch -v enables verbose output.
>>>>        The additional switch -c enables capturing mode to allow use of a
>>>> network
>>>>        analyzer such as Wireshark (if rtnet was built with --enable-rtcap).
>>>> @@ -76,6 +80,10 @@ submit_cfg() {
>>>>        master)
>>>>            $RTIFCONFIG rteth0 up $STATION_IP
>>>>
>>>> +        if [ -n "$UPDELAY" ]; then
>>>
>>> Checked with shellcheck? I would expect ${UPDELAY} here and below.
>>
>> In fact, among many (info) and (warning) messages regarding other lines,
>> shellcheck says about this row:
>>
>>     Line 84:
>>                     sleep $UPDELAY
>>                           ^-- SC2086 (info): Double quote to prevent
>> globbing and word splitting.
>>
>>     Did you mean: (apply this, apply all SC2086)
>>                     sleep "$UPDELAY"
>>
>> What should I do?
> 
> My understanding is that "${UPDELAY}" is the correct thing here.
> In case shellcheck reports more issues in the same file I would prefer
> two patches:
> 
> 1) Fixing shellcheck reportings
> 2) This one on top
> 
> But at the end it's the maintainers' decision.

Sorry for the delay.

Ok, I will do that.

Thanks.

-- 
Mauro S.