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

[Sasha Khapyorsky]

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?

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