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

[Jim Schutt]

Hi Sasha,

On Tue, 2009-09-29 at 15:53 -0600, Sasha Khapyorsky 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?

This patch makes it possible to use ibsim to test failure
scenarios where multiple switches fail or are powered off,
then replaced or powered on, using the ibsim unlink/relink
commands to simulate switch power-down/power-up.

Without this patch ibsim doesn't behave like a real fabric
under the conditions Hal described.

-- Jim

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