[ofa-general] Re: [PATCH] ibsim/sim_cmd.c: Only relink port if remote port is currently linked

Hal Rosenstock hal.rosenstock at gmail.com
Tue Sep 29 15:22:54 PDT 2009


Hi again Sasha,

In my previous post, I missed answering some of your (implied) questions.

On Tue, Sep 29, 2009 at 5:53 PM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
> Hi Hal,
>
> On 15:34 Thu 24 Sep     , Hal Rosenstock wrote:
>>
>> When multiple switches are unlinked and then a switch is relinked,
>> it should behave like a cable pull or power down of switch so it
>> depends on the state of the remote peer port (as to linked or not).
>> This is not represented in the IB port/port physical state and is
>> additional state.
>
> I'm not sure that I understand what this patch tries to achieve - I
> cannot see any changes related to port physical state handling.

Right; that's because there is none. My point was that this condition
(e.g. simulated power off switch) cannot be represented in IB port
state or port physical state.

> I can only see that you try to prevent linking with previously unlinked ports,

Yes.

-- Hal

> and it is not clear for me why. Could you explain?
>
>>
>> Signed-off-by: Hal Rosenstock <hal.rosenstock at gmail.com>
>> ---
>>
>> diff --git a/ibsim/sim.h b/ibsim/sim.h
>> index bf85875..52eb73b 100644
>> --- a/ibsim/sim.h
>> +++ b/ibsim/sim.h
>> @@ -210,6 +211,7 @@ struct Port {
>>       int remoteport;
>>       Node *previous_remotenode;
>>       int previous_remoteport;
>> +     int unlinked;
>
> Do you really need this flag? Existence of non NULL previous_remotenode
> pointer should be good indication.
>
>>       int errrate;
>>       uint16_t errattr;
>>       Node *node;
>> diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
>> index cb6e639..d27ab0f 100644
>> --- a/ibsim/sim_cmd.c
>> +++ b/ibsim/sim_cmd.c
>> @@ -1,5 +1,6 @@
>>  /*
>>   * Copyright (c) 2004-2008 Voltaire, Inc. All rights reserved.
>> + * Copyright (c) 2009 HNR Consulting. All rights reserved.
>>   *
>>   * This file is part of ibsim.
>>   *
>> @@ -146,12 +147,18 @@ static int do_link(FILE * f, char *line)
>>
>>       rport = node_get_port(rnode, rportnum);
>>
>> +     if (rport->unlinked) {
>> +             lport->unlinked = 0;
>> +             return -1;
>> +     }
>> +
>
> Why?
>
>>       if (link_ports(lport, rport) < 0)
>>               return -fprintf(f,
>>                               "# can't link: local/remote port are already connected\n");
>>
>>       lport->previous_remotenode = NULL;
>>       rport->previous_remotenode = NULL;
>> +     lport->unlinked = 0;
>>
>>       return 0;
>>  }
>> @@ -194,7 +201,7 @@ static int do_relink(FILE * f, char *line)
>>               numports++;     // To make the for-loop below run up to last port
>>       else
>>               lportnum--;
>> -
>> +
>>       if (lportnum >= 0) {
>>               lport = ports + lnode->portsbase + lportnum;
>>
>> @@ -206,12 +213,18 @@ static int do_relink(FILE * f, char *line)
>>               rport = node_get_port(lport->previous_remotenode,
>>                                     lport->previous_remoteport);
>>
>> +             if (rport->unlinked) {
>> +                     lport->unlinked = 0;
>> +                     return -1;
>> +             }
>> +
>
> Why?
>
>>               if (link_ports(lport, rport) < 0)
>>                       return -fprintf(f,
>>                                       "# can't link: local/remote port are already connected\n");
>>
>>               lport->previous_remotenode = NULL;
>>               rport->previous_remotenode = NULL;
>> +             lport->unlinked = 0;
>>
>>               return 1;
>>       }
>> @@ -224,11 +237,17 @@ static int do_relink(FILE * f, char *line)
>>               rport = node_get_port(lport->previous_remotenode,
>>                                     lport->previous_remoteport);
>>
>> +             if (rport->unlinked) {
>> +                     lport->unlinked = 0;
>> +                     continue;
>> +             }
>> +
>
> Ditto.
>
> Sasha
>
>>               if (link_ports(lport, rport) < 0)
>>                       continue;
>>
>>               lport->previous_remotenode = NULL;
>>               rport->previous_remotenode = NULL;
>> +             lport->unlinked = 0;
>>
>>               relinked++;
>>       }
>> @@ -246,6 +265,7 @@ static void unlink_port(Node * lnode, Port * lport, Node * rnode, int rportnum)
>>       lport->previous_remoteport = lport->remoteport;
>>       rport->previous_remotenode = rport->remotenode;
>>       rport->previous_remoteport = rport->remoteport;
>> +     lport->unlinked = 1;
>>
>>       lport->remotenode = rport->remotenode = 0;
>>       lport->remoteport = rport->remoteport = 0;
>> @@ -406,6 +426,7 @@ static int do_unlink(FILE * f, char *line, int clear)
>>       if (portnum >= 0) {
>>               port = ports + node->portsbase + portnum;
>>               if (!clear && !port->remotenode) {
>> +                     port->unlinked = 1;
>>                       fprintf(f, "# port %d at nodeid \"%s\" is not linked\n",
>>                               portnum, nodeid);
>>                       return -1;
>> @@ -420,8 +441,10 @@ static int do_unlink(FILE * f, char *line, int clear)
>>
>>       for (port = ports + node->portsbase, e = port + numports; port < e;
>>            port++) {
>> -             if (!clear && !port->remotenode)
>> +             if (!clear && !port->remotenode) {
>> +                     port->unlinked = 1;
>>                       continue;
>> +             }
>>               if (port->remotenode)
>>                       unlink_port(node, port, port->remotenode,
>>                                   port->remoteport);
>> diff --git a/ibsim/sim_net.c b/ibsim/sim_net.c
>> index 8a5d281..0092068 100644
>> --- a/ibsim/sim_net.c
>> +++ b/ibsim/sim_net.c
>> @@ -492,6 +492,7 @@ static void init_ports(Node * node, int type, int maxports)
>>               port->linkwidth = LINKWIDTH_4x;
>>               port->linkspeedena = netspeed;
>>               port->linkspeed = LINKSPEED_SDR;
>> +             port->unlinked = 0;
>>
>>               size = (type == SWITCH_NODE && i) ? sw_pkey_size : ca_pkey_size;
>>               if (size) {
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>



More information about the general mailing list