[PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Nathan Hartwell]

rxe_cfg does not always parse the output of ethtool and ifconfig correctly.

Fix two items:
1)  Parsing of ethtool's link speed field to always output a link speed.
2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
ifconfig that have a slightly different output format than in the
past.

Signed-off-by: Nathan Hartwell <nathan.hartwell-YP+SxaqqoVo2oU8LL9ROB9BPR1lH4CV8@public.gmane.org>

--------------------------------------------------------------------------------

diff --git a/providers/rxe/rxe_cfg.in b/providers/rxe/rxe_cfg.in
index 0a8583d..8429b28 100755
--- a/providers/rxe/rxe_cfg.in
+++ b/providers/rxe/rxe_cfg.in
@@ -180,9 +180,9 @@ sub get_dev_info {
                    if ($fields[0] =~ "Link detected") {
                        $link_state{$eth} = $fields[1];
                    }
-           }
-           elsif ($line =~ "10000baseT") {
-               $link_speed{$eth} = "10GigE";
+                   elsif ($fields[0] =~ "Speed") {
+                       $link_speed{$eth} = $fields[1];
+                   }
            }
        }

@@ -197,6 +197,11 @@ sub get_dev_info {
                @fields = split(/\s+/, $line);
                $ipv4_addr{$eth} = $fields[0];
            }
+           elsif ($line =~ /inet /) {
+           $line =~ s/^\s+inet //g;
+               @fields = split(/\s+/, $line);
+               $ipv4_addr{$eth} = $fields[0];
+           }

            # get ethernet mtu
            if ($line =~ /MTU:/) {
@@ -204,6 +209,11 @@ sub get_dev_info {
                @fields = split(/\s+/, $line);
                $eth_mtu{$eth} = $fields[0];
            }
+           elsif ($line =~ /mtu /) {
+               $line =~ s/^.*mtu //g;
+               @fields = split(/\s+/, $line);
+               $eth_mtu{$eth} = $fields[0];
+           }
        }
     }
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

On Wed, Oct 11, 2017 at 01:00:24PM -0600, Nathan Hartwell wrote:
> rxe_cfg does not always parse the output of ethtool and ifconfig correctly.
>
> Fix two items:
> 1)  Parsing of ethtool's link speed field to always output a link speed.
> 2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
> ifconfig that have a slightly different output format than in the
> past.

Can you please specify the version of ifconfig and OS?

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Nathan Hartwell]

On Sun, Oct 15, 2017 at 10:50 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Oct 11, 2017 at 01:00:24PM -0600, Nathan Hartwell wrote:
>> rxe_cfg does not always parse the output of ethtool and ifconfig correctly.
>>
>> Fix two items:
>> 1)  Parsing of ethtool's link speed field to always output a link speed.
>> 2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
>> ifconfig that have a slightly different output format than in the
>> past.
>
> Can you please specify the version of ifconfig and OS?
>
> Thanks


OS: CentOS 7 / 4.14.0-rc3 kernel
ifconfig: net-tools 2.10-alpha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2235 bytes --]

Moni ?????

Thanks

On Wed, Oct 11, 2017 at 01:00:24PM -0600, Nathan Hartwell wrote:
> rxe_cfg does not always parse the output of ethtool and ifconfig correctly.
>
> Fix two items:
> 1)  Parsing of ethtool's link speed field to always output a link speed.
> 2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
> ifconfig that have a slightly different output format than in the
> past.
>
> Signed-off-by: Nathan Hartwell <nathan.hartwell-YP+SxaqqoVo2oU8LL9ROB9BPR1lH4CV8@public.gmane.org>
>
> --------------------------------------------------------------------------------
>
> diff --git a/providers/rxe/rxe_cfg.in b/providers/rxe/rxe_cfg.in
> index 0a8583d..8429b28 100755
> --- a/providers/rxe/rxe_cfg.in
> +++ b/providers/rxe/rxe_cfg.in
> @@ -180,9 +180,9 @@ sub get_dev_info {
>                     if ($fields[0] =~ "Link detected") {
>                         $link_state{$eth} = $fields[1];
>                     }
> -           }
> -           elsif ($line =~ "10000baseT") {
> -               $link_speed{$eth} = "10GigE";
> +                   elsif ($fields[0] =~ "Speed") {
> +                       $link_speed{$eth} = $fields[1];
> +                   }
>             }
>         }
>
> @@ -197,6 +197,11 @@ sub get_dev_info {
>                 @fields = split(/\s+/, $line);
>                 $ipv4_addr{$eth} = $fields[0];
>             }
> +           elsif ($line =~ /inet /) {
> +           $line =~ s/^\s+inet //g;
> +               @fields = split(/\s+/, $line);
> +               $ipv4_addr{$eth} = $fields[0];
> +           }
>
>             # get ethernet mtu
>             if ($line =~ /MTU:/) {
> @@ -204,6 +209,11 @@ sub get_dev_info {
>                 @fields = split(/\s+/, $line);
>                 $eth_mtu{$eth} = $fields[0];
>             }
> +           elsif ($line =~ /mtu /) {
> +               $line =~ s/^.*mtu //g;
> +               @fields = split(/\s+/, $line);
> +               $eth_mtu{$eth} = $fields[0];
> +           }
>         }
>      }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Moni Shoua]

On Wed, Oct 18, 2017 at 5:24 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Moni ?????
>
> Thanks
>
> On Wed, Oct 11, 2017 at 01:00:24PM -0600, Nathan Hartwell wrote:
>> rxe_cfg does not always parse the output of ethtool and ifconfig correctly.
>>
>> Fix two items:
>> 1)  Parsing of ethtool's link speed field to always output a link speed.

>> 2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
>> ifconfig that have a slightly different output format than in the
>> past.
did you consider replacing ifconfig with ip? I can't say for sure but
the recommendation AFAIK is to replace is to replace ifconfig with ip
in scripts that parse the output
Another alternative is to use /sys file system (e.g. /sys/class/net/p2p1/speed)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Bart Van Assche]

On 10/18/17 07:45, Moni Shoua wrote:
> On Wed, Oct 18, 2017 at 5:24 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> Moni ?????
>>
>> Thanks
>>
>> On Wed, Oct 11, 2017 at 01:00:24PM -0600, Nathan Hartwell wrote:
>>> rxe_cfg does not always parse the output of ethtool and ifconfig correctly.
>>>
>>> Fix two items:
>>> 1)  Parsing of ethtool's link speed field to always output a link speed.
> 
>>> 2)  Parsing of ifconfig's IP address / MTU fields with newer builds of
>>> ifconfig that have a slightly different output format than in the
>>> past.
> did you consider replacing ifconfig with ip? I can't say for sure but
> the recommendation AFAIK is to replace is to replace ifconfig with ip
> in scripts that parse the output
> Another alternative is to use /sys file system (e.g. /sys/class/net/p2p1/speed)

Personally I'm in favor of the latter solution (sysfs). For sysfs 
attributes backwards compatibility is guaranteed but not for the output 
of the ifconfig or ip tools.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Moni Shoua]

> Personally I'm in favor of the latter solution (sysfs). For sysfs attributes
> backwards compatibility is guaranteed but not for the output of the ifconfig
> or ip tools.
>
> Bart.

I agree
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Nathan Hartwell]

On Wed, Oct 18, 2017 at 9:15 AM, Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> Personally I'm in favor of the latter solution (sysfs). For sysfs attributes
>> backwards compatibility is guaranteed but not for the output of the ifconfig
>> or ip tools.
>>
>> Bart.
>
> I agree

While I agree in principle with the idea to move to sysfs, if that
work would be applied then I would go so far as to say that a complete
rewrite of the scrip should be looked into. For example, there are a
number of other areas (e.g. the calls to modprobe) that can produce
unhelpful (but harmless) warnings to the console. We are focusing on
other fixes that have been found and need to be applied to rxe at the
moment so I am not sure when / if we will cycle around to revisit
updates to the script.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Moni Shoua]

>
> While I agree in principle with the idea to move to sysfs, if that
> work would be applied then I would go so far as to say that a complete
> rewrite of the scrip should be looked into. For example, there are a
> number of other areas (e.g. the calls to modprobe) that can produce
> unhelpful (but harmless) warnings to the console. We are focusing on
> other fixes that have been found and need to be applied to rxe at the
> moment so I am not sure when / if we will cycle around to revisit
> updates to the script.

You don't have to rewrite the whole script now but only the sections
that you intend to fix anyway.
It shouldn't take long to replace the use of ifconfig/ethtool with
/sysfs for getting MTU and SPEDD
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Jason Gunthorpe]

On Wed, Oct 18, 2017 at 10:12:02AM -0600, Nathan Hartwell wrote:

> work would be applied then I would go so far as to say that a complete
> rewrite of the scrip should be looked into.

Yes. This script is horrid and needs to go away.

rxe links (and soft iwarp!) should be created via netlink using Leon's
new rdma tool. We need a bunch of steps to get there, but that needs
to be the end goal.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rxe_cfg: Fix parsing of ethtool / ifconfig

[Moni Shoua]

> rxe links (and soft iwarp!) should be created via netlink using Leon's
> new rdma tool. We need a bunch of steps to get there, but that needs
> to be the end goal.
>
Agreed. I'll take it with Leon.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html