[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:05:12 PDT 2009
Hi Sasha,
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. I can
> only see that you try to prevent linking with previously unlinked ports,
> and it is not clear for me why. Could you explain?
The failure scenario is to unlink 2 connected switches and then relink
the first one. It then relinks the second one even though it still
should be unlinked.
>
>>
>> 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.
That's how I started (using previous_remotenode) but it didn't work
correctly for all cases. It worked with the simple case above (unlink
2 switches and relink the first). It didn't work with a 3 switch case.
-- Hal
>> 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