[PATCH 0/5 for rdma-core] Five rxe_cfg patches

[PATCH 0/5 for rdma-core] Five rxe_cfg patches

[Bart Van Assche]

Hello Doug, Jason and Moni,

These five patches is what I came up with after having had a look at the 
rxe_cfg source code. It would be appreciated if these patches would be 
considered for inclusion in the rdma-core repository. The filenames of 
these patches are:

0001-rxe_cfg-Use-Perl-functions-instead-of-parsing-the-ou.patch
0002-rxe_cfg-Initialize-rxe_mtu-even-if-ibv_definfo-fails.patch
0003-rxe_cfg-Do-not-suppress-stderr.patch
0004-rxe_cfg-Remove-dead-code-from-show_module_status.patch
0005-rxe_cfg-Use-printf-instead-of-using-a-loop.patch

Thanks,

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

[PATCH 1/5] rxe_cfg: Use Perl functions instead of parsing the output of shell commands

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
 providers/rxe/rxe_cfg | 67 ++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 36 deletions(-)

diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
index 6783dc78c340..0310d25bad91 100755
--- a/providers/rxe/rxe_cfg
+++ b/providers/rxe/rxe_cfg
@@ -35,6 +35,7 @@
 use warnings;
 use strict;
 
+use File::Basename;
 use Getopt::Long;
 use Switch;
 
@@ -67,20 +68,25 @@ my @mlx4_port;
 my @mlx4_ether;
 my @roce_list;
 
+# Read a file and return its contents as a string.
+sub read_file {
+    my $filename = shift;
+    my $result = "";
+
+    if (open(FILE, $filename)) {
+	$result = <FILE>;
+	close FILE;
+    }
+    return $result;
+}
+
 #get mapping between rxe and eth devices
 sub get_names {
-    return if (!(-e "/sys/class/infiniband"));
-
-    my @list;
-    my $rxe;
-    my $eth;
     my $i = 0;
     
-    @list = `ls /sys/class/infiniband | grep rxe $z`;
-    foreach $rxe (@list) {
-	chomp($rxe);
-
-	$eth = `cat /sys/class/infiniband/$rxe/parent $z`;
+    foreach my $rxe (glob("/sys/class/infiniband/rxe*")) {
+	$rxe = basename($rxe);
+	my $eth = read_file("/sys/class/infiniband/$rxe/parent");
 	chomp($eth);
 	
 	if (($eth =~ /[\w]+[\d]/)
@@ -98,25 +104,13 @@ sub get_names {
 
 # get list of Mellanox RoCE ports
 sub get_mlx4_list {
-    return if (!(-e "/sys/class/infiniband"));
-
-    my @list;
-    my $mlx4;
-    my @ports;
-    my $port;
-    my $link;
     my $i = 0;
 
-    @list = `ls /sys/class/infiniband | grep mlx4_ $z`;
-    foreach $mlx4 (@list) {
-	chomp($mlx4);
-	chdir("/sys/class/infiniband/$mlx4/ports");
-
-	@ports = `ls $z`;
-	foreach $port (@ports) {
-	    chomp($port);
-
-	    $link= `cat $port/link_layer $z`;
+    foreach my $mlx4 (glob("/sys/class/infiniband/mlx4_*")) {
+	$mlx4 = basename($mlx4);
+	foreach my $port (glob("/sys/class/infiniband/$mlx4/ports/*")) {
+	    $port = basename($port);
+	    my $link = read_file("$port/link_layer");
 	    chomp($link);
 
 	    if ($link =~ "Ethernet") {
@@ -141,10 +135,10 @@ sub get_dev_info {
     get_mlx4_list();
 
     my @my_eth_list = ();
-    my @my_eth_devs = `ls /sys/class/net`;
-    foreach my $my_eth_dev (@my_eth_devs) {
-        chomp($my_eth_dev);
-        my $my_dev_type = `cat /sys/class/net/${my_eth_dev}/type`;
+    foreach my $my_eth_dev (glob("/sys/class/net/*")) {
+	$my_eth_dev = basename($my_eth_dev);
+        my $my_dev_type = read_file("/sys/class/net/${my_eth_dev}/type");
+	chomp($my_dev_type);
         if ($my_dev_type == "1") {
             push(@my_eth_list, "$my_eth_dev");
         }
@@ -378,11 +372,11 @@ sub do_status {
 
 # read file containing list of ethernet devices into a list
 sub populate_persistence {
-    my @lines = `cat $persistence_file $z`;
-    my $line;
     my $i = 0;
     
-    foreach $line (@lines) {
+    open FILE, $persistence_file;
+    while(<FILE>) {
+	my $line = $_;
 	chomp($line);
 	$line =~ s/^\s+//g;
 	if ($line =~ /[\w]+[\d]/) {
@@ -394,6 +388,7 @@ sub populate_persistence {
 	    }
 	}
     }
+    close FILE;
 
     $num_persistent = $i;
 }
@@ -604,7 +599,7 @@ sub do_debug {
 	}
     }
 
-    my $current = `cat $debugfile $z`;
+    my $current = read_file($debugfile);
     chomp($current);
     if ($current > 0) {
 	print "Debug is ON ($current)\n";
@@ -681,7 +676,7 @@ sub main {
     }
 
     # create persistence file if necessary
-    `mkdir -p $persistence_path $z`;
+    mkdir -p $persistence_path;
     if (!(-e $persistence_file)) {
         `touch $persistence_file $z`;
     }
-- 
2.10.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 1/5] rxe_cfg: Use Perl functions instead of parsing the output of shell commands

[Yonatan Cohen]

On 10/7/2016 9:38 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
>  providers/rxe/rxe_cfg | 67 ++++++++++++++++++++++++---------------------------
>  1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 6783dc78c340..0310d25bad91 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -35,6 +35,7 @@
>  use warnings;
>  use strict;
>
> +use File::Basename;
>  use Getopt::Long;
>  use Switch;
>
> @@ -67,20 +68,25 @@ my @mlx4_port;
>  my @mlx4_ether;
>  my @roce_list;
>
> +# Read a file and return its contents as a string.
> +sub read_file {
> +    my $filename = shift;
> +    my $result = "";
> +
> +    if (open(FILE, $filename)) {
> +	$result = <FILE>;
> +	close FILE;
> +    }
> +    return $result;
> +}
> +
>  #get mapping between rxe and eth devices
>  sub get_names {
> -    return if (!(-e "/sys/class/infiniband"));
> -
> -    my @list;
> -    my $rxe;
> -    my $eth;
>      my $i = 0;
>
> -    @list = `ls /sys/class/infiniband | grep rxe $z`;
> -    foreach $rxe (@list) {
> -	chomp($rxe);
> -
> -	$eth = `cat /sys/class/infiniband/$rxe/parent $z`;
> +    foreach my $rxe (glob("/sys/class/infiniband/rxe*")) {
> +	$rxe = basename($rxe);
> +	my $eth = read_file("/sys/class/infiniband/$rxe/parent");
>  	chomp($eth);
>  	
>  	if (($eth =~ /[\w]+[\d]/)
> @@ -98,25 +104,13 @@ sub get_names {
>
>  # get list of Mellanox RoCE ports
>  sub get_mlx4_list {
> -    return if (!(-e "/sys/class/infiniband"));
> -
> -    my @list;
> -    my $mlx4;
> -    my @ports;
> -    my $port;
> -    my $link;
>      my $i = 0;
>
> -    @list = `ls /sys/class/infiniband | grep mlx4_ $z`;
> -    foreach $mlx4 (@list) {
> -	chomp($mlx4);
> -	chdir("/sys/class/infiniband/$mlx4/ports");
> -
> -	@ports = `ls $z`;
> -	foreach $port (@ports) {
> -	    chomp($port);
> -
> -	    $link= `cat $port/link_layer $z`;
> +    foreach my $mlx4 (glob("/sys/class/infiniband/mlx4_*")) {
> +	$mlx4 = basename($mlx4);
> +	foreach my $port (glob("/sys/class/infiniband/$mlx4/ports/*")) {
> +	    $port = basename($port);
> +	    my $link = read_file("$port/link_layer");
>  	    chomp($link);
>
>  	    if ($link =~ "Ethernet") {
> @@ -141,10 +135,10 @@ sub get_dev_info {
>      get_mlx4_list();
>
>      my @my_eth_list = ();
> -    my @my_eth_devs = `ls /sys/class/net`;
> -    foreach my $my_eth_dev (@my_eth_devs) {
> -        chomp($my_eth_dev);
> -        my $my_dev_type = `cat /sys/class/net/${my_eth_dev}/type`;
> +    foreach my $my_eth_dev (glob("/sys/class/net/*")) {
> +	$my_eth_dev = basename($my_eth_dev);
> +        my $my_dev_type = read_file("/sys/class/net/${my_eth_dev}/type");
> +	chomp($my_dev_type);
>          if ($my_dev_type == "1") {
>              push(@my_eth_list, "$my_eth_dev");
>          }
> @@ -378,11 +372,11 @@ sub do_status {
>
>  # read file containing list of ethernet devices into a list
>  sub populate_persistence {
> -    my @lines = `cat $persistence_file $z`;
> -    my $line;
>      my $i = 0;
>
> -    foreach $line (@lines) {
> +    open FILE, $persistence_file;
> +    while(<FILE>) {
> +	my $line = $_;
>  	chomp($line);
>  	$line =~ s/^\s+//g;
>  	if ($line =~ /[\w]+[\d]/) {
> @@ -394,6 +388,7 @@ sub populate_persistence {
>  	    }
>  	}
>      }
> +    close FILE;
>
>      $num_persistent = $i;
>  }
> @@ -604,7 +599,7 @@ sub do_debug {
>  	}
>      }
>
> -    my $current = `cat $debugfile $z`;
> +    my $current = read_file($debugfile);
>      chomp($current);
>      if ($current > 0) {
>  	print "Debug is ON ($current)\n";
> @@ -681,7 +676,7 @@ sub main {
>      }
>
>      # create persistence file if necessary
> -    `mkdir -p $persistence_path $z`;
> +    mkdir -p $persistence_path;
>      if (!(-e $persistence_file)) {
>          `touch $persistence_file $z`;
>      }
>
ack
--
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

[PATCH 2/5] rxe_cfg: Initialize $rxe_mtu even if ibv_definfo fails

[Bart Van Assche]

This patch avoids that the following error message is printed if
ibv_devinfo fails:

Use of uninitialized value $rmtu in string at /usr/local/bin/rxe_cfg line 364.

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
 providers/rxe/rxe_cfg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
index 0310d25bad91..1e6a5db2f2b8 100755
--- a/providers/rxe/rxe_cfg
+++ b/providers/rxe/rxe_cfg
@@ -220,6 +220,7 @@ sub get_dev_info {
 		$rxe_mtu{$rxe} = $line;
 	    }
 	}
+	$rxe_mtu{$rxe} = "(?)" if (!$rxe_mtu{$rxe});
     }
 }
 
-- 
2.10.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 2/5] rxe_cfg: Initialize $rxe_mtu even if ibv_definfo fails

[Yonatan Cohen]

On 10/7/2016 9:38 PM, Bart Van Assche wrote:
> This patch avoids that the following error message is printed if
> ibv_devinfo fails:
>
> Use of uninitialized value $rmtu in string at /usr/local/bin/rxe_cfg line 364.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
>  providers/rxe/rxe_cfg | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 0310d25bad91..1e6a5db2f2b8 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -220,6 +220,7 @@ sub get_dev_info {
>  		$rxe_mtu{$rxe} = $line;
>  	    }
>  	}
> +	$rxe_mtu{$rxe} = "(?)" if (!$rxe_mtu{$rxe});
>      }
>  }
>
>
Ack
--
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

[PATCH 3/5] rxe_cfg: Do not suppress stderr

[Bart Van Assche]

Suppressing stderr if stdout is already redirected is wrong. Hence
do not suppress stderr. If e.g. the ib_uverbs kernel module is not
loaded, without this patch no error messages are printed. With this
patch applied the following is printed:

Failed to get IB devices list: Function not implemented

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
 providers/rxe/rxe_cfg | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
index 1e6a5db2f2b8..a4e9c0407a8b 100755
--- a/providers/rxe/rxe_cfg
+++ b/providers/rxe/rxe_cfg
@@ -39,7 +39,6 @@ use File::Basename;
 use Getopt::Long;
 use Switch;
 
-my $z = "2>/dev/null";
 my $help = 0;
 my $no_persist = 0;
 my $debug = 0;
@@ -150,7 +149,7 @@ sub get_dev_info {
 
 	$eth_list[$i++] = $eth;
 
-	@lines = `ethtool -i $eth $z`;
+	@lines = `ethtool -i $eth`;
 	foreach $line (@lines) {
 	    chomp($line);
 
@@ -171,7 +170,7 @@ sub get_dev_info {
 	$link_state{$eth} = "";
 	$link_speed{$eth} = "";
 
-	@lines = `ethtool $eth $z`;
+	@lines = `ethtool $eth`;
 	foreach $line (@lines) {
 	    chomp($line);
 
@@ -190,7 +189,7 @@ sub get_dev_info {
 	$ipv4_addr{$eth} = "            ";
 	$eth_mtu{$eth} = "";
 
-	@lines = `ifconfig $eth $z`;
+	@lines = `ifconfig $eth`;
 	foreach $line (@lines) {
 	    # get IP address
 	    if ($line =~ /inet addr/) {
@@ -211,7 +210,7 @@ sub get_dev_info {
     # get rxe mtu
     foreach my $rxe (@rxe_array) {
 	
-	@lines  = `ibv_devinfo -d $rxe $z`;
+	@lines = `ibv_devinfo -d $rxe`;
 	foreach $line (@lines) {
 	    if ($line =~ "active_mtu") {
 		$line =~ s/^\s+active_mtu:\s+//g;
@@ -494,7 +493,7 @@ sub get_devinfo {
     my $rxe = $_[0];
 
     my $cmd = "ibv_devinfo -d $rxe";
-    return `$cmd $z`;
+    return `$cmd`;
 }
 
 # allow unsupported modules to load in SLES11 if allowed
@@ -679,7 +678,7 @@ sub main {
     # create persistence file if necessary
     mkdir -p $persistence_path;
     if (!(-e $persistence_file)) {
-        `touch $persistence_file $z`;
+        `touch $persistence_file`;
     }
 
     # Get full context of the configuration
-- 
2.10.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 3/5] rxe_cfg: Do not suppress stderr

[Yonatan Cohen]

On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Suppressing stderr if stdout is already redirected is wrong. Hence
> do not suppress stderr. If e.g. the ib_uverbs kernel module is not
> loaded, without this patch no error messages are printed. With this
> patch applied the following is printed:
>
> Failed to get IB devices list: Function not implemented
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
>  providers/rxe/rxe_cfg | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 1e6a5db2f2b8..a4e9c0407a8b 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -39,7 +39,6 @@ use File::Basename;
>  use Getopt::Long;
>  use Switch;
>
> -my $z = "2>/dev/null";
>  my $help = 0;
>  my $no_persist = 0;
>  my $debug = 0;
> @@ -150,7 +149,7 @@ sub get_dev_info {
>
>  	$eth_list[$i++] = $eth;
>
> -	@lines = `ethtool -i $eth $z`;
> +	@lines = `ethtool -i $eth`;
>  	foreach $line (@lines) {
>  	    chomp($line);
>
> @@ -171,7 +170,7 @@ sub get_dev_info {
>  	$link_state{$eth} = "";
>  	$link_speed{$eth} = "";
>
> -	@lines = `ethtool $eth $z`;
> +	@lines = `ethtool $eth`;
>  	foreach $line (@lines) {
>  	    chomp($line);
>
> @@ -190,7 +189,7 @@ sub get_dev_info {
>  	$ipv4_addr{$eth} = "            ";
>  	$eth_mtu{$eth} = "";
>
> -	@lines = `ifconfig $eth $z`;
> +	@lines = `ifconfig $eth`;
>  	foreach $line (@lines) {
>  	    # get IP address
>  	    if ($line =~ /inet addr/) {
> @@ -211,7 +210,7 @@ sub get_dev_info {
>      # get rxe mtu
>      foreach my $rxe (@rxe_array) {
>  	
> -	@lines  = `ibv_devinfo -d $rxe $z`;
> +	@lines = `ibv_devinfo -d $rxe`;
>  	foreach $line (@lines) {
>  	    if ($line =~ "active_mtu") {
>  		$line =~ s/^\s+active_mtu:\s+//g;
> @@ -494,7 +493,7 @@ sub get_devinfo {
>      my $rxe = $_[0];
>
>      my $cmd = "ibv_devinfo -d $rxe";
> -    return `$cmd $z`;
> +    return `$cmd`;
>  }
>
>  # allow unsupported modules to load in SLES11 if allowed
> @@ -679,7 +678,7 @@ sub main {
>      # create persistence file if necessary
>      mkdir -p $persistence_path;
>      if (!(-e $persistence_file)) {
> -        `touch $persistence_file $z`;
> +        `touch $persistence_file`;
>      }
>
>      # Get full context of the configuration
>
Ack
--
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

[PATCH 4/5] rxe_cfg: Remove dead code from show_module_status()

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
 providers/rxe/rxe_cfg | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
index a4e9c0407a8b..0a3dcf16957c 100755
--- a/providers/rxe/rxe_cfg
+++ b/providers/rxe/rxe_cfg
@@ -300,23 +300,7 @@ sub check_module_status {
 
 # print driver load status and ethertype for rdma_rxe and rdma_rxe_net
 sub show_module_status {
-    my $rxe_loaded = 1;
-
-    if (!(-e $sys)) {
-	$rxe_loaded = 0;
-    }
-
-    if ($rxe_loaded) {
-	return 0;
-    }
-    elsif (!$rxe_loaded) {
-	print "rxe modules not loaded\n";
-	return 1;
-    }
-    else {
-	print "Configuration does not make sense\n";
-	return 1;
-    }
+    print "rdma_rxe module not loaded\n" if (!(-e $sys));
 }
 
 # print rxe status
-- 
2.10.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 4/5] rxe_cfg: Remove dead code from show_module_status()

[Yonatan Cohen]

On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
>  providers/rxe/rxe_cfg | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index a4e9c0407a8b..0a3dcf16957c 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -300,23 +300,7 @@ sub check_module_status {
>
>  # print driver load status and ethertype for rdma_rxe and rdma_rxe_net
>  sub show_module_status {
> -    my $rxe_loaded = 1;
> -
> -    if (!(-e $sys)) {
> -	$rxe_loaded = 0;
> -    }
> -
> -    if ($rxe_loaded) {
> -	return 0;
> -    }
> -    elsif (!$rxe_loaded) {
> -	print "rxe modules not loaded\n";
> -	return 1;
> -    }
> -    else {
> -	print "Configuration does not make sense\n";
> -	return 1;
> -    }
> +    print "rdma_rxe module not loaded\n" if (!(-e $sys));
>  }
>
>  # print rxe status
>
ack
--
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

[PATCH 5/5] rxe_cfg: Use printf() instead of using a loop

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
 providers/rxe/rxe_cfg | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
index 0a3dcf16957c..6c414fbda6fd 100755
--- a/providers/rxe/rxe_cfg
+++ b/providers/rxe/rxe_cfg
@@ -241,7 +241,6 @@ sub status_print {
     my @flen = ();
     my $num_fields = 0;
     my $i;
-    my $j;
     my $pad;
     my $line;
 
@@ -281,9 +280,7 @@ sub status_print {
 	    else {
 		print "   ";
 	    }
-	    for ($j = 0; $j < $pad; $j++) {
-		print " ";
-	    }
+	    printf("%*s", $pad, "");
 	}
 	print "\n";
     }
-- 
2.10.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 5/5] rxe_cfg: Use printf() instead of using a loop

[Yonatan Cohen]

On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
>  providers/rxe/rxe_cfg | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 0a3dcf16957c..6c414fbda6fd 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -241,7 +241,6 @@ sub status_print {
>      my @flen = ();
>      my $num_fields = 0;
>      my $i;
> -    my $j;
>      my $pad;
>      my $line;
>
> @@ -281,9 +280,7 @@ sub status_print {
>  	    else {
>  		print "   ";
>  	    }
> -	    for ($j = 0; $j < $pad; $j++) {
> -		print " ";
> -	    }
> +	    printf("%*s", $pad, "");
>  	}
>  	print "\n";
>      }
>
ack
--
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 0/5 for rdma-core] Five rxe_cfg patches

[Jason Gunthorpe]

On Fri, Oct 07, 2016 at 11:37:43AM -0700, Bart Van Assche wrote:
> Hello Doug, Jason and Moni,
> 
> These five patches is what I came up with after having had a look at the
> rxe_cfg source code. It would be appreciated if these patches would be
> considered for inclusion in the rdma-core repository. The filenames of these
> patches are:

Thanks Bart, it would be best in Moni looks at these when he returns,
but nothing seemed off to my uneducated eye.

While you are looking at this could you also look at the use of
/var/rxe in that script?

That is not a FHS path, so it needs to be something else, and the
script should probably pass through substitution to pick up the right
path, whatever that is.. Based on the name sounds like /var/cache or
/var/lib ?

Thanks,
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 0/5 for rdma-core] Five rxe_cfg patches

[Bart Van Assche]

On 10/07/2016 12:02 PM, Jason Gunthorpe wrote:
> On Fri, Oct 07, 2016 at 11:37:43AM -0700, Bart Van Assche wrote:
>> Hello Doug, Jason and Moni,
>>
>> These five patches is what I came up with after having had a look at the
>> rxe_cfg source code. It would be appreciated if these patches would be
>> considered for inclusion in the rdma-core repository. The filenames of these
>> patches are:
>
> Thanks Bart, it would be best in Moni looks at these when he returns,
> but nothing seemed off to my uneducated eye.
>
> While you are looking at this could you also look at the use of
> /var/rxe in that script?
>
> That is not a FHS path, so it needs to be something else, and the
> script should probably pass through substitution to pick up the right
> path, whatever that is.. Based on the name sounds like /var/cache or
> /var/lib ?

Hello Jason,

Do you think the names of the network interfaces to which the rdma_rxe 
driver should bind should be stored in a separate file? To me this looks 
like a suboptimal approach. Shouldn't we drop the /var/rxe/rxe file 
completely and add a new boolean to the files in which other network 
interface parameters are already stored, e.g. 
/etc/NetworkManager/system-connections/* ?

Thanks,

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 0/5 for rdma-core] Five rxe_cfg patches

[Jason Gunthorpe]

On Fri, Oct 07, 2016 at 01:51:59PM -0700, Bart Van Assche wrote:

> Do you think the names of the network interfaces to which the rdma_rxe
> driver should bind should be stored in a separate file? To me this looks
> like a suboptimal approach. Shouldn't we drop the /var/rxe/rxe file
> completely and add a new boolean to the files in which other network
> interface parameters are already stored, e.g.
> /etc/NetworkManager/system-connections/* ?

I honestly don't know much about how rxe works, I was just looking
at the bogus path.

In today's world this should probably be handled by adding systemd unit
files to set up rxe.. ??

Something with network manager also seems to make some kind of sense
on the surface.

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 0/5 for rdma-core] Five rxe_cfg patches

[Parav Pandit]

Hi Jason, Bart,

On Sat, Oct 8, 2016 at 2:30 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Fri, Oct 07, 2016 at 01:51:59PM -0700, Bart Van Assche wrote:
>
>> Do you think the names of the network interfaces to which the rdma_rxe
>> driver should bind should be stored in a separate file? To me this looks
>> like a suboptimal approach. Shouldn't we drop the /var/rxe/rxe file
>> completely and add a new boolean to the files in which other network
>> interface parameters are already stored, e.g.
>> /etc/NetworkManager/system-connections/* ?
>
> I honestly don't know much about how rxe works, I was just looking
> at the bogus path.
>
> In today's world this should probably be handled by adding systemd unit
> files to set up rxe.. ??
>
> Something with network manager also seems to make some kind of sense
> on the surface.
>

Instead of relying on user land to do configuration of different OS
flavors, I am considering to auto create rxe interfaces via netdev
notifier for all the network interfaces which gets newly added.
May be through some sort of attach()/detach() ops().
netdev driver will have choice to reject/accept such calls. So that
mlx4_ib is already loaded, it can reject his call from rdma_rxe.
Since the ratio of netdev devices which support RDMA to devices which
doesn't support RDMA is much large, I do not intent to add _ops() to
netdev but rather to have API through which this can be done.
Need to think little more to comeup with RFC.
Other than dev envionrments its unlikely that users want to play with
PMTU configuration. So I was thinking to do do least amount of
configuration using rxe_cfg going forward.




> 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
--
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 0/5 for rdma-core] Five rxe_cfg patches

[Jason Gunthorpe]

On Sat, Oct 08, 2016 at 03:12:45AM +0530, Parav Pandit wrote:

> Instead of relying on user land to do configuration of different OS
> flavors, I am considering to auto create rxe interfaces via netdev
> notifier for all the network interfaces which gets newly added.

Interesting, my concern is that rxe seems to currently be buggy and
inadvertently enabling rxe on any non-dev systems at this time is
probably insane from a security perspective :\

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 0/5 for rdma-core] Five rxe_cfg patches

[Parav Pandit]

On Sat, Oct 8, 2016 at 3:37 AM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Sat, Oct 08, 2016 at 03:12:45AM +0530, Parav Pandit wrote:
>
>> Instead of relying on user land to do configuration of different OS
>> flavors, I am considering to auto create rxe interfaces via netdev
>> notifier for all the network interfaces which gets newly added.
>
> Interesting, my concern is that rxe seems to currently be buggy and
> inadvertently enabling rxe on any non-dev systems at this time is
> probably insane from a security perspective :\
>

Sure. Yes. I agree. Once its stable I can take this up.
However if rdma_rxe is not loaded, its should be fine.

> 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 0/5 for rdma-core] Five rxe_cfg patches

[Leon Romanovsky]

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

On Fri, Oct 07, 2016 at 11:37:43AM -0700, Bart Van Assche wrote:
> Hello Doug, Jason and Moni,
>
> These five patches is what I came up with after having had a look at the
> rxe_cfg source code. It would be appreciated if these patches would be
> considered for inclusion in the rdma-core repository. The filenames of these
> patches are:
>
> 0001-rxe_cfg-Use-Perl-functions-instead-of-parsing-the-ou.patch
> 0002-rxe_cfg-Initialize-rxe_mtu-even-if-ibv_definfo-fails.patch
> 0003-rxe_cfg-Do-not-suppress-stderr.patch
> 0004-rxe_cfg-Remove-dead-code-from-show_module_status.patch
> 0005-rxe_cfg-Use-printf-instead-of-using-a-loop.patch

Hi Bart,

Thank you for investing time, Moni started to look/test them and update
us on the result.

Thanks.

>
> Thanks,
>
> 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

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

Re: [PATCH 0/5 for rdma-core] Five rxe_cfg patches

[Leon Romanovsky]

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

On Fri, Oct 07, 2016 at 11:37:43AM -0700, Bart Van Assche wrote:
> Hello Doug, Jason and Moni,
>
> These five patches is what I came up with after having had a look at the
> rxe_cfg source code. It would be appreciated if these patches would be
> considered for inclusion in the rdma-core repository. The filenames of these
> patches are:
>
> 0001-rxe_cfg-Use-Perl-functions-instead-of-parsing-the-ou.patch
> 0002-rxe_cfg-Initialize-rxe_mtu-even-if-ibv_definfo-fails.patch
> 0003-rxe_cfg-Do-not-suppress-stderr.patch
> 0004-rxe_cfg-Remove-dead-code-from-show_module_status.patch
> 0005-rxe_cfg-Use-printf-instead-of-using-a-loop.patch
>
> Thanks,

Thanks, applied
https://github.com/linux-rdma/rdma-core/pull/20

>
> 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

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