targetname not passed during relogin/add conn

[Shivaram U]

Hi,
Attached are two diffs for target name not passed in certain parts of
the code

First Diff
-------------

--- core-iscsi-tools-v3.3/core-iscsi/scripts/install.channel 2005-12-10
06:08:05.000000000 +0530
+++
core-iscsi-tools-v3.3-new/core-iscsi/scripts/install.channel 2006-06-03
13:55:11.000000000 +0530
@@ -143,14 +143,18 @@ do
if [ $RET != 0 ]; then
rm -rf /var/lock/subsys/iscsi-initiator-channel-$1
exit 1
fi

+ TNAME_STR=""
+ if [ $TARGETNAME ]; then
+ TNAME_STR="targetname=$TARGETNAME"
+ fi
if [ $TRANSPORT == 1 ] || [ $TRANSPORT == 2 ]; then
- ${INITIATOR} addconn channel=$1 dev=${DEV} ip=${IP} port=${PORT}
sctp=$TRANSPORT > /dev/null
+ ${INITIATOR} addconn channel=$1 dev=${DEV} ip=${IP} port=${PORT}
sctp=$TRANSPORT $TNAME_STR > /dev/null
else
- ${INITIATOR} addconn channel=$1 dev=${DEV} ip=${IP} port=${PORT} >
/dev/null
+ ${INITIATOR} addconn channel=$1 dev=${DEV} ip=${IP} port=${PORT}
$TNAME_STR > /dev/null
fi

RET=$?

if [ $RET == 0 ]; then

Second Diff (im not sure if the we need a channel state lock below, but
similar code in the file does)
---------------

--- core-iscsi-v1.6.2.5/iscsi_initiator_erl0.c 2006-03-13
08:54:19.000000000 +0530
+++ core-iscsi-v1.6.2.5.new/iscsi_initiator_erl0.c 2006-06-09
12:02:43.000000000 +0530
@@ -994,10 +994,17 @@ static void iscsi_start_extra_conns_afte
lh.ipv4_address = chan_conn->ipv4_address;
lh.port = chan_conn->port;
lh.network_transport = chan_conn->network_transport;
lh.cc = chan_conn;
strncpy(lh.net_dev, chan_conn->net_dev, ISCSI_NETDEV_NAME_SIZE);
+ spin_lock_bh(&channel->channel_state_lock);
+ if (channel->ch_target) {
+ strncpy(lh.lh_targetname, channel->ch_target->targetname,
+ ISCSI_MAX_TARGETNAME_SIZE);
+ lh.lh_flags |= LH_TARGETNAME_PRESENT;
+ }
+ spin_unlock_bh(&channel->channel_state_lock);

ret = iscsi_create_connection(sess, &lh, NULL);
spin_lock(&channel->chan_conn_lock);

if (!ret) {

Thanks,
Shivram U

[n...@kernel.org]

Greetings Shivaram,

Note that the TargetName is automatically included during login
negotiations for non-leading connections in
iscsi_initiator_discovery.c:iscsi_check_for_login_targetaddress().

Since the RFC requires each connection in a session to be moving
packets to a single target node, defining the targetname= parameter
when 'login' is called will ensure for all subsequent 'addconn'
commands with include the original passed targetname=.

Just to make sure, please feel free to grab an ethereal dump and double
check. The TargetName is included in the first Login Request PDU that
gets sent to the target for both leading and non-leading login cases.

Thanks again for your interest! :-)

--nab

[Shivaram U]

Hi Nab,
The login phase does seem to work fine. The problem is that im
hitting the following error check at
iscsi_initiator_discovery.c:iscsi_check_for_login_targetaddress

if (!lh->leading_conn) {
if (SESS_OPS(sess)->TargetPortalGroupTag != ta->tpgt) {
TRACE_ERROR("Existing TPGT: %hu does not match"
" TargetAddress's TPGT: %hu, cannot continue.\n",
SESS_OPS(sess)->TargetPortalGroupTag, ta->tpgt);
return(-1);
}
}

An example scenario of the problem is -
consider the following targets and portal group tag

target 1 - portal group tag 1
target 2 - portal group tag 2.
And in the list iscsi_global->target_address they appear as target1 ->
target2..

Note that target 1 and target 2 are both present at the same login
address say 192.168.1.10

I have forgotten the exact sequence of events to the problem, but if
the target name isnt passed in the diffs i have mentioned, the
following function returns an invalid target.

iscsi_initiatiator_discovery:iscsi_search_for_login_targetaddress

for (t = iscsi_global->target_head; t; t = t->next) {

if ((lh->lh_flags & LH_TARGETNAME_PRESENT) &&
(strncmp(lh->lh_targetname, t->targetname,
strlen(t->targetname))))
continue;

spin_lock(&t->ta_lock);
for (ta = t->ta_head; ta; ta = ta->next) {
if ((ta->ipv4_address != lh->ipv4_address) ||
(ta->port != lh->port))
continue;

strncpy(ta->net_dev, lh->net_dev,
strlen(lh->net_dev));
spin_unlock(&t->ta_lock);
spin_unlock(&iscsi_global->target_lock);
return(ta);

So if target 1 is read of the list first instead of target 2 for which
the search is on, since the name isnt present and since the ipaddress
are the same target 1 is returned. This happens only in the addconn
code path. Since the portal group tag for the targets are different,
we hit the check as mentioned before.

Best Regards,
Shivram U

[n...@kernel.org]

Greetings Shivaram,

Much thanks for the clarification on this and your patch. Commited for
v1.6.2.7 and tools v3.4.

--nab