Re: [PATCH] BNX2I: register given device with cnic if shost != NULL in ep_connect()

[Mike Christie <michaelc@cs.wisc.edu>]

On 07/09/2009 09:39 AM, Anil Veerabhadrappa wrote:
> On Thu, 2009-07-09 at 00:15 -0700, Michael Chan wrote:
>> Mike Christie wrote:
>>> Anil Veerabhadrappa wrote:
>>>> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c
>>> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
>>>> index f741219..98148f3 100644
>>>> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
>>>> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
>>>> @@ -1653,15 +1653,18 @@ static struct iscsi_endpoint
>>> *bnx2i_ep_connect(struct Scsi_Host *shost,
>>>>      struct iscsi_endpoint *ep;
>>>>      int rc = 0;
>>>>
>>>> -   if (shost)
>>>> +   if (shost) {
>>>>              /* driver is given scsi host to work with */
>>>>              hba = iscsi_host_priv(shost);
>>>> -   else
>>>> +           /* Register the device with cnic if not already
>>> done so */
>>>> +           bnx2i_register_device(hba);
>>>> +   } else
>>>>              /*
>>>>               * check if the given destination can be reached through
>>>>               * a iscsi capable NetXtreme2 device
>>>>               */
>>>>              hba = bnx2i_check_route(dst_addr);
>>>> +
>>>>      if (!hba) {
>>>>              rc = -ENOMEM;
>>>>              goto check_busy;
>>> Do you want to do the test_bit(BNX2I_CNIC_REGISTERED,
>>> &hba->reg_with_cnic) test here instead of down below. If it
>>> failed then
>>> you might try to do a cm_create on a cnic that is not
>>> properly registered.
>>>
>> Good idea.  cm_create() will properly be ok because there is no
>> hardware interaction.  bnx2i_send_conn_ofld_req() will fail if
>> the cnic is not properly registered because there will be no call
>> backs from the hardware events.
>
> following 2 conditions needs to be satisfied to offload an iscsi
> connection,
>    1) device has to be registered with cnic
>    2) device has to support iscsi offload
>
>    bnx2i_adapter_ready() will return success only if both conditions
> (which is flagged by ADAPTER_STATE_UP bit in hba->adapter_state) are
> met. bnx2i_ep_connect() will bailout if bnx2i_adapter_ready() does not
> return correct status and this guarantees bnx2i will not make any cnic
> calls on an unregistered device.
>

Ok then. It seems fine to me.

Reviewed-by Mike Christie <michaelc@cs.wisc.edu>

One other question though. Do you need the BNX2I_CNIC_REGISTERED tst in 
ep_connect then? If not then maybe in separate patch for the next 
feature window we can clean that up.