Re: [PATCH 05/14] IB/srp: Maintain a single connection per I_T nexus

[Vu Pham <vuhuong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>]

Bart Van Assche wrote:
> On 06/13/13 19:50, Vu Pham wrote:
>> Hello Bart,
>>
>>> +/**
>>> + * srp_conn_unique() - check whether the connection to a target is
>>> unique
>>> + */
>>> +static bool srp_conn_unique(struct srp_host *host,
>>> +                struct srp_target_port *target)
>>> +{
>>> +    struct srp_target_port *t;
>>> +    bool ret = false;
>>> +
>>> +    if (target->state == SRP_TARGET_REMOVED)
>>> +        goto out;
>>> +
>>> +    ret = true;
>>> +
>>> +    spin_lock(&host->target_lock);
>>> +    list_for_each_entry(t, &host->target_list, list) {
>>> +        if (t != target &&
>>> +            target->id_ext == t->id_ext &&
>>
>> Targets may advertise/expose on different pkeys
>> You can have multiple connections  (or paths/scsi hosts) to same target
>> with different pkeys.
>> We need extra check to detect the uniqueness:
>>                                              target->path.pkey ==
>> t->path.pkey &&
>
> Hello Vu,
>
> Thanks for the feedback. This is something I have already thinking 
> about myself. Unfortunately I have not found any requirements in the 
> T10 SRP standard with regard to InfiniBand partitions. However, in 
> that document there is a section about single RDMA channel operation. 
> In that section it is explained that an SRP target must log out 
> established sessions upon receipt of a new login request. What I'm not 
> sure about is whether only sessions with the same P_Key must be logged 
> out or all established sessions if a new login request is received. I 
> assume the latter since otherwise that would mean that an SRP target 
> would be required to maintain multiple sessions if it allows 
> connections with more than one P_Key to a target port ? My concern 
> about adding a pkey comparison in the function srp_conn_unique() is 
> that if a target allows an initiator to choose which partition to use 
> when logging in, that this could result in the undesired SRP initiator 
> ping-pong effect this patch tries to avoid.
>
> Bart.
>
Hello Bart,

Yes, you pointed out the unclear/undefined area.

If we stick to single RDMA channel per IT Nexus with unique tuple 
Initiator Port Identier - Target Port Indentifier then newly created 
connection with same tuple (I_port_id, T_port_id) but with different 
P_Key or different DGID is not unique.

Sticking to this rule by excluding P_Key and DGID  out of rdma channel 
indentity, your srp_conn_unique() checking is ok; however, some SRP 
target implementations may include DGID as part of rdma channel 
identifier. I'm not sure about different p_key part.

-vu
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html