Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-09-01 Thread Sasha Khapyorsky
On 17:18 Tue 31 Aug , Eli Dorfman (Voltaire) wrote:
> Sasha Khapyorsky wrote:
> > On 09:45 Tue 24 Aug , Hal Rosenstock wrote:
> >>> What about the flag? do we still need it if we pass the output after the 
> >>> comment?
> >> I wouldn't think so. I also think we've made commentary changes to the
> >> ibnetdiscover output format like this before. If we wanted to be
> >> absolutely sure it wouldn't break anything, we'd keep the flag though.
> >> It's up to Sasha.
> > 
> > The '-f' flag could make sense since those information actually
> > duplicates previous 4xDDR, etc. and used by ibsim's parser only.
> > 
> > Also it would be nice to make ibsim's parser to understand both
> > possibilities: s=2 and speed=2 (Or even to parse "*x*DR" strings :))
> 
> Is it valid to assume that "*x*DR" string is always the last token?

I think that it would be better to not assume this.

Sasha

> If so, the patch should be changed
> 
> Eli
> > 
> > Sasha
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-09-01 Thread Sasha Khapyorsky
On 16:14 Thu 26 Aug , Doron Shoham wrote:
> add '-f' flag to show full information (ports' speed and witdh).
> mainly to work with ibsim (using links real speed and width).
> 
> Signed-off-by: Doron Shoham 

Applied. Thanks.

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


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-31 Thread Hal Rosenstock
On Tue, Aug 31, 2010 at 10:18 AM, Eli Dorfman (Voltaire)
 wrote:
> Sasha Khapyorsky wrote:
>> On 09:45 Tue 24 Aug     , Hal Rosenstock wrote:
 What about the flag? do we still need it if we pass the output after the 
 comment?
>>> I wouldn't think so. I also think we've made commentary changes to the
>>> ibnetdiscover output format like this before. If we wanted to be
>>> absolutely sure it wouldn't break anything, we'd keep the flag though.
>>> It's up to Sasha.
>>
>> The '-f' flag could make sense since those information actually
>> duplicates previous 4xDDR, etc. and used by ibsim's parser only.
>>
>> Also it would be nice to make ibsim's parser to understand both
>> possibilities: s=2 and speed=2 (Or even to parse "*x*DR" strings :))
>
> Is it valid to assume that "*x*DR" string is always the last token?

Not always but for most cases (other than Xsigo devices).

-- Hal

> If so, the patch should be changed
>
> Eli
>>
>> Sasha
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-31 Thread Eli Dorfman (Voltaire)
Sasha Khapyorsky wrote:
> On 09:45 Tue 24 Aug , Hal Rosenstock wrote:
>>> What about the flag? do we still need it if we pass the output after the 
>>> comment?
>> I wouldn't think so. I also think we've made commentary changes to the
>> ibnetdiscover output format like this before. If we wanted to be
>> absolutely sure it wouldn't break anything, we'd keep the flag though.
>> It's up to Sasha.
> 
> The '-f' flag could make sense since those information actually
> duplicates previous 4xDDR, etc. and used by ibsim's parser only.
> 
> Also it would be nice to make ibsim's parser to understand both
> possibilities: s=2 and speed=2 (Or even to parse "*x*DR" strings :))

Is it valid to assume that "*x*DR" string is always the last token?
If so, the patch should be changed

Eli
> 
> Sasha
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-26 Thread Doron Shoham
add '-f' flag to show full information (ports' speed and witdh).
mainly to work with ibsim (using links real speed and width).

Signed-off-by: Doron Shoham 
---
 infiniband-diags/src/ibnetdiscover.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/infiniband-diags/src/ibnetdiscover.c 
b/infiniband-diags/src/ibnetdiscover.c
index f20058c..92f248b 100644
--- a/infiniband-diags/src/ibnetdiscover.c
+++ b/infiniband-diags/src/ibnetdiscover.c
@@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
 static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
 
 static int report_max_hops = 0;
+static int full_info;
 
 /**
  * Define our own conversion functions to maintain compatibility with the old
@@ -364,6 +365,9 @@ void out_switch_port(ibnd_port_t * port, int group, char 
*out_prefix)
port->remoteport->base_lid,
dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
 
+   if (full_info)
+   fprintf(f, " s=%d w=%d", ispeed, iwidth);
+
if (ibnd_is_xsigo_tca(port->remoteport->guid))
fprintf(f, " slot %d", port->portnum);
else if (ibnd_is_xsigo_hca(port->remoteport->guid))
@@ -397,13 +401,17 @@ void out_ca_port(ibnd_port_t * port, int group, char 
*out_prefix)
   port->remoteport->node->guid,
   port->remoteport->node->nodedesc);
 
-   fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
+   fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s",
port->base_lid, port->lmc, rem_nodename,
port->remoteport->node->type == IB_NODE_SWITCH ?
port->remoteport->node->smalid :
port->remoteport->base_lid,
dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
 
+   if (full_info)
+   fprintf(f, " s=%d w=%d", ispeed, iwidth);
+   fprintf(f, "\n");
+
free(rem_nodename);
 }
 
@@ -926,6 +934,9 @@ static int process_opt(void *context, int ch, char *optarg)
case 's':
cfg->show_progress = 1;
break;
+   case 'f':
+   full_info = 1;
+   break;
case 'l':
list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
break;
@@ -964,6 +975,7 @@ int main(int argc, char **argv)
ibnd_fabric_t *diff_fabric = NULL;
 
const struct ibdiag_opt opts[] = {
+   {"full", 'f', 0, NULL, "show full information (ports' speed and 
witdh)"},
{"show", 's', 0, NULL, "show more information"},
{"list", 'l', 0, NULL, "list of connected nodes"},
{"grouping", 'g', 0, NULL, "show grouping"},
-- 
1.5.4

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


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-26 Thread Hal Rosenstock
On Thu, Aug 26, 2010 at 3:43 AM, Doron Shoham  wrote:
> add '-f' flag to show full information (ports' speed and witdh).
> mainly to work with ibsim (using links real speed and width).

Just a couple of (output formatting) nits below...

> Signed-off-by: Doron Shoham 
> ---
>  infiniband-diags/src/ibnetdiscover.c |   16 ++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/infiniband-diags/src/ibnetdiscover.c 
> b/infiniband-diags/src/ibnetdiscover.c
> index f20058c..df9e5a6 100644
> --- a/infiniband-diags/src/ibnetdiscover.c
> +++ b/infiniband-diags/src/ibnetdiscover.c
> @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
>  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
>
>  static int report_max_hops = 0;
> +static int full_info;
>
>  /**
>  * Define our own conversion functions to maintain compatibility with the old
> @@ -357,13 +358,16 @@ void out_switch_port(ibnd_port_t * port, int group, 
> char *out_prefix)
>                ext_port_str ? ext_port_str : "");
>        if (port->remoteport->node->type != IB_NODE_SWITCH)
>                fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
> -       fprintf(f, "\t\t# \"%s\" lid %d %s%s",
> +       fprintf(f, "\t\t# \"%s\" lid %d %s%s ",

I'd leave this alone and move the space below.

>                rem_nodename,
>                port->remoteport->node->type == IB_NODE_SWITCH ?
>                port->remoteport->node->smalid :
>                port->remoteport->base_lid,
>                dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
>
> +       if (full_info)
> +               fprintf(f, "s=%d w=%d", ispeed, iwidth);
> +

I'd move the space here before s=

>        if (ibnd_is_xsigo_tca(port->remoteport->guid))
>                fprintf(f, " slot %d", port->portnum);
>        else if (ibnd_is_xsigo_hca(port->remoteport->guid))
> @@ -397,13 +401,17 @@ void out_ca_port(ibnd_port_t * port, int group, char 
> *out_prefix)
>                                       port->remoteport->node->guid,
>                                       port->remoteport->node->nodedesc);
>
> -       fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
> +       fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s ",

Ditto,

>                port->base_lid, port->lmc, rem_nodename,
>                port->remoteport->node->type == IB_NODE_SWITCH ?
>                port->remoteport->node->smalid :
>                port->remoteport->base_lid,
>                dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
>
> +       if (full_info)
> +               fprintf(f, "s=%d w=%d", ispeed, iwidth);

Ditto.

-- Hal

> +       fprintf(f, "\n");
> +
>        free(rem_nodename);
>  }
>
> @@ -926,6 +934,9 @@ static int process_opt(void *context, int ch, char 
> *optarg)
>        case 's':
>                cfg->show_progress = 1;
>                break;
> +       case 'f':
> +               full_info = 1;
> +               break;
>        case 'l':
>                list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
>                break;
> @@ -964,6 +975,7 @@ int main(int argc, char **argv)
>        ibnd_fabric_t *diff_fabric = NULL;
>
>        const struct ibdiag_opt opts[] = {
> +               {"full", 'f', 0, NULL, "show full information (ports' speed 
> and witdh)"},
>                {"show", 's', 0, NULL, "show more information"},
>                {"list", 'l', 0, NULL, "list of connected nodes"},
>                {"grouping", 'g', 0, NULL, "show grouping"},
> --
> 1.5.4
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-26 Thread Doron Shoham
add '-f' flag to show full information (ports' speed and witdh).
mainly to work with ibsim (using links real speed and width).

Signed-off-by: Doron Shoham 
---
 infiniband-diags/src/ibnetdiscover.c |   16 ++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/infiniband-diags/src/ibnetdiscover.c 
b/infiniband-diags/src/ibnetdiscover.c
index f20058c..df9e5a6 100644
--- a/infiniband-diags/src/ibnetdiscover.c
+++ b/infiniband-diags/src/ibnetdiscover.c
@@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
 static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
 
 static int report_max_hops = 0;
+static int full_info;
 
 /**
  * Define our own conversion functions to maintain compatibility with the old
@@ -357,13 +358,16 @@ void out_switch_port(ibnd_port_t * port, int group, char 
*out_prefix)
ext_port_str ? ext_port_str : "");
if (port->remoteport->node->type != IB_NODE_SWITCH)
fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
-   fprintf(f, "\t\t# \"%s\" lid %d %s%s",
+   fprintf(f, "\t\t# \"%s\" lid %d %s%s ",
rem_nodename,
port->remoteport->node->type == IB_NODE_SWITCH ?
port->remoteport->node->smalid :
port->remoteport->base_lid,
dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
 
+   if (full_info)
+   fprintf(f, "s=%d w=%d", ispeed, iwidth);
+
if (ibnd_is_xsigo_tca(port->remoteport->guid))
fprintf(f, " slot %d", port->portnum);
else if (ibnd_is_xsigo_hca(port->remoteport->guid))
@@ -397,13 +401,17 @@ void out_ca_port(ibnd_port_t * port, int group, char 
*out_prefix)
   port->remoteport->node->guid,
   port->remoteport->node->nodedesc);
 
-   fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
+   fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s ",
port->base_lid, port->lmc, rem_nodename,
port->remoteport->node->type == IB_NODE_SWITCH ?
port->remoteport->node->smalid :
port->remoteport->base_lid,
dump_linkwidth_compat(iwidth), dump_linkspeed_compat(ispeed));
 
+   if (full_info)
+   fprintf(f, "s=%d w=%d", ispeed, iwidth);
+   fprintf(f, "\n");
+
free(rem_nodename);
 }
 
@@ -926,6 +934,9 @@ static int process_opt(void *context, int ch, char *optarg)
case 's':
cfg->show_progress = 1;
break;
+   case 'f':
+   full_info = 1;
+   break;
case 'l':
list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
break;
@@ -964,6 +975,7 @@ int main(int argc, char **argv)
ibnd_fabric_t *diff_fabric = NULL;
 
const struct ibdiag_opt opts[] = {
+   {"full", 'f', 0, NULL, "show full information (ports' speed and 
witdh)"},
{"show", 's', 0, NULL, "show more information"},
{"list", 'l', 0, NULL, "list of connected nodes"},
{"grouping", 'g', 0, NULL, "show grouping"},
-- 
1.5.4

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


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-25 Thread Sasha Khapyorsky
On 09:45 Tue 24 Aug , Hal Rosenstock wrote:
> 
> > What about the flag? do we still need it if we pass the output after the 
> > comment?
> 
> I wouldn't think so. I also think we've made commentary changes to the
> ibnetdiscover output format like this before. If we wanted to be
> absolutely sure it wouldn't break anything, we'd keep the flag though.
> It's up to Sasha.

The '-f' flag could make sense since those information actually
duplicates previous 4xDDR, etc. and used by ibsim's parser only.

Also it would be nice to make ibsim's parser to understand both
possibilities: s=2 and speed=2 (Or even to parse "*x*DR" strings :))

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


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-24 Thread Hal Rosenstock
On Tue, Aug 24, 2010 at 9:27 AM, Doron Shoham  wrote:
> On 08/24/2010 04:10 PM, Hal Rosenstock wrote:
>> On Tue, Aug 24, 2010 at 7:55 AM, Doron Shoham  wrote:
>>> On 08/24/2010 07:03 AM, Sasha Khapyorsky wrote:
 On 11:30 Wed 18 Aug     , Doron Shoham wrote:
> add '-f' flag to show full information (ports' speed and witdh).
> mainly to work with ibsim (using links real speed and width).

 Seems almost fine. However see the comments below.

>
> Signed-off-by: Doron Shoham 
> ---
>  infiniband-diags/src/ibnetdiscover.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/infiniband-diags/src/ibnetdiscover.c 
> b/infiniband-diags/src/ibnetdiscover.c
> index f20058c..0a020a2 100644
> --- a/infiniband-diags/src/ibnetdiscover.c
> +++ b/infiniband-diags/src/ibnetdiscover.c
> @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
>  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
>
>  static int report_max_hops = 0;
> +static int full_info = 0;
>
>  /**
>   * Define our own conversion functions to maintain compatibility with 
> the old
> @@ -357,6 +358,8 @@ void out_switch_port(ibnd_port_t * port, int group, 
> char *out_prefix)
>              ext_port_str ? ext_port_str : "");
>      if (port->remoteport->node->type != IB_NODE_SWITCH)
>              fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
> +    if (full_info)
> +                    fprintf(f, " s=%d w=%d", ispeed, iwidth);

 I think that in order to not potentially break any ibnetdiscover output
 parsers it would be better to put such "f" output after a comment line.
 Would it work with ibsim in a same way?
>>>
>>> ibsim should still work if the output is after the comment.
>>> I still think that it can potentially break ibnetdiscover output parsers 
>>> (that why I used '-f' flag).
>>> Do you want it with '-f' flag and after the comment?
>>> for example:
>>> [1](2c90320a9)      "S-0008f10500650272"[19]                #  s=2 w=2 
>>> lid 6 lmc 0 "Voltaire sLB-4018    Line 10  Chip 1 4700 #4700-B778" lid 3 
>>> 4xDDR
>>>
>>
>> Personally, I'd prefer it at the end of the line after 4xDDR and use
>> speed/width rather than s=/w=:
>>
>> [1](2c90320a9)      "S-0008f10500650272"[19]                #  lid
>> 6 lmc 0 "Voltaire sLB-4018    Line 10  Chip 1 4700 #4700-B778" lid 3
>> 4xDDR speed 2 width 2
>>
>> I think that looks better and is more readable. Just my $0.02...
>>
>> -- Hal
>
> The problem is the ibsim is expecting s=/w= and not speed  or width 
> .

Oh, I forgot about that; ugh...

> What about the flag? do we still need it if we pass the output after the 
> comment?

I wouldn't think so. I also think we've made commentary changes to the
ibnetdiscover output format like this before. If we wanted to be
absolutely sure it wouldn't break anything, we'd keep the flag though.
It's up to Sasha.

-- Hal

>
> Doron
>>

>      fprintf(f, "\t\t# \"%s\" lid %d %s%s",
>              rem_nodename,
>              port->remoteport->node->type == IB_NODE_SWITCH ?
> @@ -396,7 +399,8 @@ void out_ca_port(ibnd_port_t * port, int group, char 
> *out_prefix)
>      rem_nodename = remap_node_name(node_name_map,
>                                     port->remoteport->node->guid,
>                                     port->remoteport->node->nodedesc);
> -
> +    if (full_info)
> +                    fprintf(f, " s=%d w=%d", ispeed, iwidth);

 Ditto.

 Sasha

>      fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
>              port->base_lid, port->lmc, rem_nodename,
>              port->remoteport->node->type == IB_NODE_SWITCH ?
> @@ -926,6 +930,9 @@ static int process_opt(void *context, int ch, char 
> *optarg)
>      case 's':
>              cfg->show_progress = 1;
>              break;
> +    case 'f':
> +            full_info = 1;
> +            break;
>      case 'l':
>              list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
>              break;
> @@ -964,6 +971,7 @@ int main(int argc, char **argv)
>      ibnd_fabric_t *diff_fabric = NULL;
>
>      const struct ibdiag_opt opts[] = {
> +            {"full", 'f', 0, NULL, "show full information (ports' speed 
> and witdh)"},
>              {"show", 's', 0, NULL, "show more information"},
>              {"list", 'l', 0, NULL, "list of connected nodes"},
>              {"grouping", 'g', 0, NULL, "show grouping"},
> --
> 1.5.4
>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>> the body of a message to majord...@vger.kernel.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 m

Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-24 Thread Doron Shoham
On 08/24/2010 04:10 PM, Hal Rosenstock wrote:
> On Tue, Aug 24, 2010 at 7:55 AM, Doron Shoham  wrote:
>> On 08/24/2010 07:03 AM, Sasha Khapyorsky wrote:
>>> On 11:30 Wed 18 Aug , Doron Shoham wrote:
 add '-f' flag to show full information (ports' speed and witdh).
 mainly to work with ibsim (using links real speed and width).
>>>
>>> Seems almost fine. However see the comments below.
>>>

 Signed-off-by: Doron Shoham 
 ---
  infiniband-diags/src/ibnetdiscover.c |   10 +-
  1 files changed, 9 insertions(+), 1 deletions(-)

 diff --git a/infiniband-diags/src/ibnetdiscover.c 
 b/infiniband-diags/src/ibnetdiscover.c
 index f20058c..0a020a2 100644
 --- a/infiniband-diags/src/ibnetdiscover.c
 +++ b/infiniband-diags/src/ibnetdiscover.c
 @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;

  static int report_max_hops = 0;
 +static int full_info = 0;

  /**
   * Define our own conversion functions to maintain compatibility with the 
 old
 @@ -357,6 +358,8 @@ void out_switch_port(ibnd_port_t * port, int group, 
 char *out_prefix)
  ext_port_str ? ext_port_str : "");
  if (port->remoteport->node->type != IB_NODE_SWITCH)
  fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
 +if (full_info)
 +fprintf(f, " s=%d w=%d", ispeed, iwidth);
>>>
>>> I think that in order to not potentially break any ibnetdiscover output
>>> parsers it would be better to put such "f" output after a comment line.
>>> Would it work with ibsim in a same way?
>>
>> ibsim should still work if the output is after the comment.
>> I still think that it can potentially break ibnetdiscover output parsers 
>> (that why I used '-f' flag).
>> Do you want it with '-f' flag and after the comment?
>> for example:
>> [1](2c90320a9)  "S-0008f10500650272"[19]#  s=2 w=2 
>> lid 6 lmc 0 "Voltaire sLB-4018Line 10  Chip 1 4700 #4700-B778" lid 3 
>> 4xDDR
>>
> 
> Personally, I'd prefer it at the end of the line after 4xDDR and use
> speed/width rather than s=/w=:
> 
> [1](2c90320a9)  "S-0008f10500650272"[19]#  lid
> 6 lmc 0 "Voltaire sLB-4018Line 10  Chip 1 4700 #4700-B778" lid 3
> 4xDDR speed 2 width 2
> 
> I think that looks better and is more readable. Just my $0.02...
> 
> -- Hal

The problem is the ibsim is expecting s=/w= and not speed  or width 
.

What about the flag? do we still need it if we pass the output after the 
comment?

Doron
> 
>>>
  fprintf(f, "\t\t# \"%s\" lid %d %s%s",
  rem_nodename,
  port->remoteport->node->type == IB_NODE_SWITCH ?
 @@ -396,7 +399,8 @@ void out_ca_port(ibnd_port_t * port, int group, char 
 *out_prefix)
  rem_nodename = remap_node_name(node_name_map,
 port->remoteport->node->guid,
 port->remoteport->node->nodedesc);
 -
 +if (full_info)
 +fprintf(f, " s=%d w=%d", ispeed, iwidth);
>>>
>>> Ditto.
>>>
>>> Sasha
>>>
  fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
  port->base_lid, port->lmc, rem_nodename,
  port->remoteport->node->type == IB_NODE_SWITCH ?
 @@ -926,6 +930,9 @@ static int process_opt(void *context, int ch, char 
 *optarg)
  case 's':
  cfg->show_progress = 1;
  break;
 +case 'f':
 +full_info = 1;
 +break;
  case 'l':
  list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
  break;
 @@ -964,6 +971,7 @@ int main(int argc, char **argv)
  ibnd_fabric_t *diff_fabric = NULL;

  const struct ibdiag_opt opts[] = {
 +{"full", 'f', 0, NULL, "show full information (ports' speed 
 and witdh)"},
  {"show", 's', 0, NULL, "show more information"},
  {"list", 'l', 0, NULL, "list of connected nodes"},
  {"grouping", 'g', 0, NULL, "show grouping"},
 --
 1.5.4

>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-24 Thread Hal Rosenstock
On Tue, Aug 24, 2010 at 7:55 AM, Doron Shoham  wrote:
> On 08/24/2010 07:03 AM, Sasha Khapyorsky wrote:
>> On 11:30 Wed 18 Aug     , Doron Shoham wrote:
>>> add '-f' flag to show full information (ports' speed and witdh).
>>> mainly to work with ibsim (using links real speed and width).
>>
>> Seems almost fine. However see the comments below.
>>
>>>
>>> Signed-off-by: Doron Shoham 
>>> ---
>>>  infiniband-diags/src/ibnetdiscover.c |   10 +-
>>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/infiniband-diags/src/ibnetdiscover.c 
>>> b/infiniband-diags/src/ibnetdiscover.c
>>> index f20058c..0a020a2 100644
>>> --- a/infiniband-diags/src/ibnetdiscover.c
>>> +++ b/infiniband-diags/src/ibnetdiscover.c
>>> @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
>>>  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
>>>
>>>  static int report_max_hops = 0;
>>> +static int full_info = 0;
>>>
>>>  /**
>>>   * Define our own conversion functions to maintain compatibility with the 
>>> old
>>> @@ -357,6 +358,8 @@ void out_switch_port(ibnd_port_t * port, int group, 
>>> char *out_prefix)
>>>              ext_port_str ? ext_port_str : "");
>>>      if (port->remoteport->node->type != IB_NODE_SWITCH)
>>>              fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
>>> +    if (full_info)
>>> +                    fprintf(f, " s=%d w=%d", ispeed, iwidth);
>>
>> I think that in order to not potentially break any ibnetdiscover output
>> parsers it would be better to put such "f" output after a comment line.
>> Would it work with ibsim in a same way?
>
> ibsim should still work if the output is after the comment.
> I still think that it can potentially break ibnetdiscover output parsers 
> (that why I used '-f' flag).
> Do you want it with '-f' flag and after the comment?
> for example:
> [1](2c90320a9)      "S-0008f10500650272"[19]                #  s=2 w=2 
> lid 6 lmc 0 "Voltaire sLB-4018    Line 10  Chip 1 4700 #4700-B778" lid 3 4xDDR
>

Personally, I'd prefer it at the end of the line after 4xDDR and use
speed/width rather than s=/w=:

[1](2c90320a9)  "S-0008f10500650272"[19]#  lid
6 lmc 0 "Voltaire sLB-4018Line 10  Chip 1 4700 #4700-B778" lid 3
4xDDR speed 2 width 2

I think that looks better and is more readable. Just my $0.02...

-- Hal

>>
>>>      fprintf(f, "\t\t# \"%s\" lid %d %s%s",
>>>              rem_nodename,
>>>              port->remoteport->node->type == IB_NODE_SWITCH ?
>>> @@ -396,7 +399,8 @@ void out_ca_port(ibnd_port_t * port, int group, char 
>>> *out_prefix)
>>>      rem_nodename = remap_node_name(node_name_map,
>>>                                     port->remoteport->node->guid,
>>>                                     port->remoteport->node->nodedesc);
>>> -
>>> +    if (full_info)
>>> +                    fprintf(f, " s=%d w=%d", ispeed, iwidth);
>>
>> Ditto.
>>
>> Sasha
>>
>>>      fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
>>>              port->base_lid, port->lmc, rem_nodename,
>>>              port->remoteport->node->type == IB_NODE_SWITCH ?
>>> @@ -926,6 +930,9 @@ static int process_opt(void *context, int ch, char 
>>> *optarg)
>>>      case 's':
>>>              cfg->show_progress = 1;
>>>              break;
>>> +    case 'f':
>>> +            full_info = 1;
>>> +            break;
>>>      case 'l':
>>>              list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
>>>              break;
>>> @@ -964,6 +971,7 @@ int main(int argc, char **argv)
>>>      ibnd_fabric_t *diff_fabric = NULL;
>>>
>>>      const struct ibdiag_opt opts[] = {
>>> +            {"full", 'f', 0, NULL, "show full information (ports' speed 
>>> and witdh)"},
>>>              {"show", 's', 0, NULL, "show more information"},
>>>              {"list", 'l', 0, NULL, "list of connected nodes"},
>>>              {"grouping", 'g', 0, NULL, "show grouping"},
>>> --
>>> 1.5.4
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-24 Thread Doron Shoham
On 08/24/2010 07:03 AM, Sasha Khapyorsky wrote:
> On 11:30 Wed 18 Aug , Doron Shoham wrote:
>> add '-f' flag to show full information (ports' speed and witdh).
>> mainly to work with ibsim (using links real speed and width).
> 
> Seems almost fine. However see the comments below.
> 
>>
>> Signed-off-by: Doron Shoham 
>> ---
>>  infiniband-diags/src/ibnetdiscover.c |   10 +-
>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/infiniband-diags/src/ibnetdiscover.c 
>> b/infiniband-diags/src/ibnetdiscover.c
>> index f20058c..0a020a2 100644
>> --- a/infiniband-diags/src/ibnetdiscover.c
>> +++ b/infiniband-diags/src/ibnetdiscover.c
>> @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
>>  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
>>  
>>  static int report_max_hops = 0;
>> +static int full_info = 0;
>>  
>>  /**
>>   * Define our own conversion functions to maintain compatibility with the 
>> old
>> @@ -357,6 +358,8 @@ void out_switch_port(ibnd_port_t * port, int group, char 
>> *out_prefix)
>>  ext_port_str ? ext_port_str : "");
>>  if (port->remoteport->node->type != IB_NODE_SWITCH)
>>  fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
>> +if (full_info)
>> +fprintf(f, " s=%d w=%d", ispeed, iwidth);
> 
> I think that in order to not potentially break any ibnetdiscover output
> parsers it would be better to put such "f" output after a comment line.
> Would it work with ibsim in a same way?

ibsim should still work if the output is after the comment.
I still think that it can potentially break ibnetdiscover output parsers (that 
why I used '-f' flag).
Do you want it with '-f' flag and after the comment?
for example:
[1](2c90320a9)  "S-0008f10500650272"[19]#  s=2 w=2 lid 
6 lmc 0 "Voltaire sLB-4018Line 10  Chip 1 4700 #4700-B778" lid 3 4xDDR

> 
>>  fprintf(f, "\t\t# \"%s\" lid %d %s%s",
>>  rem_nodename,
>>  port->remoteport->node->type == IB_NODE_SWITCH ?
>> @@ -396,7 +399,8 @@ void out_ca_port(ibnd_port_t * port, int group, char 
>> *out_prefix)
>>  rem_nodename = remap_node_name(node_name_map,
>> port->remoteport->node->guid,
>> port->remoteport->node->nodedesc);
>> -
>> +if (full_info)
>> +fprintf(f, " s=%d w=%d", ispeed, iwidth);
> 
> Ditto.
> 
> Sasha
> 
>>  fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
>>  port->base_lid, port->lmc, rem_nodename,
>>  port->remoteport->node->type == IB_NODE_SWITCH ?
>> @@ -926,6 +930,9 @@ static int process_opt(void *context, int ch, char 
>> *optarg)
>>  case 's':
>>  cfg->show_progress = 1;
>>  break;
>> +case 'f':
>> +full_info = 1;
>> +break;
>>  case 'l':
>>  list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
>>  break;
>> @@ -964,6 +971,7 @@ int main(int argc, char **argv)
>>  ibnd_fabric_t *diff_fabric = NULL;
>>  
>>  const struct ibdiag_opt opts[] = {
>> +{"full", 'f', 0, NULL, "show full information (ports' speed and 
>> witdh)"},
>>  {"show", 's', 0, NULL, "show more information"},
>>  {"list", 'l', 0, NULL, "list of connected nodes"},
>>  {"grouping", 'g', 0, NULL, "show grouping"},
>> -- 
>> 1.5.4
>>

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


Re: [PATCH] ibnetdiscover: add '-f' flag to show full information (ports' speed and width).

2010-08-23 Thread Sasha Khapyorsky
On 11:30 Wed 18 Aug , Doron Shoham wrote:
> add '-f' flag to show full information (ports' speed and witdh).
> mainly to work with ibsim (using links real speed and width).

Seems almost fine. However see the comments below.

> 
> Signed-off-by: Doron Shoham 
> ---
>  infiniband-diags/src/ibnetdiscover.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/infiniband-diags/src/ibnetdiscover.c 
> b/infiniband-diags/src/ibnetdiscover.c
> index f20058c..0a020a2 100644
> --- a/infiniband-diags/src/ibnetdiscover.c
> +++ b/infiniband-diags/src/ibnetdiscover.c
> @@ -77,6 +77,7 @@ static char *diff_cache_file = NULL;
>  static unsigned diffcheck_flags = DIFF_FLAG_DEFAULT;
>  
>  static int report_max_hops = 0;
> +static int full_info = 0;
>  
>  /**
>   * Define our own conversion functions to maintain compatibility with the old
> @@ -357,6 +358,8 @@ void out_switch_port(ibnd_port_t * port, int group, char 
> *out_prefix)
>   ext_port_str ? ext_port_str : "");
>   if (port->remoteport->node->type != IB_NODE_SWITCH)
>   fprintf(f, "(%" PRIx64 ") ", port->remoteport->guid);
> + if (full_info)
> + fprintf(f, " s=%d w=%d", ispeed, iwidth);

I think that in order to not potentially break any ibnetdiscover output
parsers it would be better to put such "f" output after a comment line.
Would it work with ibsim in a same way?

>   fprintf(f, "\t\t# \"%s\" lid %d %s%s",
>   rem_nodename,
>   port->remoteport->node->type == IB_NODE_SWITCH ?
> @@ -396,7 +399,8 @@ void out_ca_port(ibnd_port_t * port, int group, char 
> *out_prefix)
>   rem_nodename = remap_node_name(node_name_map,
>  port->remoteport->node->guid,
>  port->remoteport->node->nodedesc);
> -
> + if (full_info)
> + fprintf(f, " s=%d w=%d", ispeed, iwidth);

Ditto.

Sasha

>   fprintf(f, "\t\t# lid %d lmc %d \"%s\" lid %d %s%s\n",
>   port->base_lid, port->lmc, rem_nodename,
>   port->remoteport->node->type == IB_NODE_SWITCH ?
> @@ -926,6 +930,9 @@ static int process_opt(void *context, int ch, char 
> *optarg)
>   case 's':
>   cfg->show_progress = 1;
>   break;
> + case 'f':
> + full_info = 1;
> + break;
>   case 'l':
>   list = LIST_CA_NODE | LIST_SWITCH_NODE | LIST_ROUTER_NODE;
>   break;
> @@ -964,6 +971,7 @@ int main(int argc, char **argv)
>   ibnd_fabric_t *diff_fabric = NULL;
>  
>   const struct ibdiag_opt opts[] = {
> + {"full", 'f', 0, NULL, "show full information (ports' speed and 
> witdh)"},
>   {"show", 's', 0, NULL, "show more information"},
>   {"list", 'l', 0, NULL, "list of connected nodes"},
>   {"grouping", 'g', 0, NULL, "show grouping"},
> -- 
> 1.5.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html