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

["Anil Veerabhadrappa" <anilgv@broadcom.com>]

On Thu, 2009-07-09 at 10:43 -0700, Mike Christie wrote:
> 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.
> 
> 
I agree there is no incentive to do the extra work if device
registration failed. We will take care of this in the next feature
window.

Thanks!!