[openib-general] [PATCH] IB/SRP Userspace: srptools/srp_daemon - Fix connect bug and add support for user specified initiator extension

[Ishai Rabinovitz]

Thanks for your patch.

I agree with some of the changes you suggest and disagree with others. It
will be useful to post a different patch for each logical change.

> 1. Fixes bug in srp_daemon for the case where if it is invoked with the
'-e' option, it fails to connect to the SRP targets because of a newline
character in the parameter string.

You are right that the '\n' is redundant, but I have not seen the bug it
creates. The last parameter in the string is considered to be a string by
ib_srp and therefore ib_srp will ignore the newline.


> 2. Changes the name of the constant 'MAX_TRAGET_CONFIG_STR_STRING' to
'MAX_TARGET_CONFIG_STR_STRING'.

Thanks, I will apply this change.


> 3. Changes the behavior of the '-n' option to srp_daemon. The earlier
behavior printed the initiator extension. The new behavior allows the
user to specify an initiator extension as an argument to the '-n'
option.

I do not think we want to change the -n behavior to this one.
First of all your approach induces the same initiator_ext to all the
targets discovered by this srp_daemon.
Secondly If someone uses random values for the initiator_ext it may cause
a waste of resources in the target: the target can never tell when a
connection had failed (or an initiator performed a boot)  and will keep
the connection alive. When there is an attempt to reconnect to this target
with the same initiator_ext, the target knows he can close the old
connection.
This is the reason we decided to have a convention. The convention is to
use the target port guid. The advantage of this convention is that it
allows us to have two connection on the same time from an initiator to
both ports of the target.

I understand that some targets may need a different initiator_ext, but you
should add a new flag for actually setting the initiator_ext and leave -n
untouched.
You are welcome to send such a patch.

Ishai