Re: [PATCH 2/2] bnx2i : Fix "cid #n not valid" issue
On 07/29/2009 11:50 PM, Anil Veerabhadrappa wrote:
>
> * when bnx2i_adapter_ready() fails, connection handle(cid) = 0 is
>wrongly freed because 'cid' is not yet allocated for the endpoint
> * Fix is to initialize bnx2i_ep->ep_iscsi_cid to '-1' in bnx2i_alloc_ep()
>and not in bnx2i_ep_connect() to avoid releasing invalid 'cid'
> * There is already a check in bnx2i_free_iscsi_cid() not to free
>invalid iscsi connection handle (-1)
>
> Signed-off-by: Anil Veerabhadrappa
This looks ok, but I think I got a review comment to change some of my
code to use the definitions in kernel.h. So below you would want to use
USHORT_MAX.
I am not sure if it will prevent the patch from getting merged or not
since it will work either way.
Reviewed-by: Mike Christie
> ---
> drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> index 9535bb6..08d0bfc 100644
> --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
> +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
> @@ -387,6 +387,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct
> bnx2i_hba *hba)
> bnx2i_ep = ep->dd_data;
> INIT_LIST_HEAD(&bnx2i_ep->link);
> bnx2i_ep->state = EP_STATE_IDLE;
> + bnx2i_ep->ep_iscsi_cid = (u16) -1;
> bnx2i_ep->hba = hba;
> bnx2i_ep->hba_age = hba->age;
> hba->ofld_conns_active++;
> @@ -1678,8 +1679,6 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct
> Scsi_Host *shost,
> goto net_if_down;
> }
>
> - bnx2i_ep->state = EP_STATE_IDLE;
> - bnx2i_ep->ep_iscsi_cid = (u16) -1;
> bnx2i_ep->num_active_cmds = 0;
> iscsi_cid = bnx2i_alloc_iscsi_cid(hba);
> if (iscsi_cid == -1) {
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---
Re: [PATCH 2/2] bnx2i : Fix "cid #n not valid" issue
On 3 Aug 2009 at 12:18, Mike Christie wrote: > > On 07/30/2009 01:08 AM, Ulrich Windl wrote: > > On 29 Jul 2009 at 21:50, Anil Veerabhadrappa wrote: > > > >> + bnx2i_ep->ep_iscsi_cid = (u16) -1; > > > > As a matter of style: Wouldn't it be more logical to write "(u16) ~0" > > instead? > > Casting a negative value to unsigned seems strange to me. > > > > Is there a MAX_U16 type of macro like there is for unsigned long? In HP-UX I found # define USHRT_MAX 017 /* max value of a unsigned short int */ So maybe Linux has something similar for the kernel, but I don't know. Probably. Regards, Ulrich --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH 2/2] bnx2i : Fix "cid #n not valid" issue
On 07/30/2009 01:08 AM, Ulrich Windl wrote: > On 29 Jul 2009 at 21:50, Anil Veerabhadrappa wrote: > >> +bnx2i_ep->ep_iscsi_cid = (u16) -1; > > As a matter of style: Wouldn't it be more logical to write "(u16) ~0" instead? > Casting a negative value to unsigned seems strange to me. > Is there a MAX_U16 type of macro like there is for unsigned long? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH 2/2] bnx2i : Fix "cid #n not valid" issue
On 29 Jul 2009 at 21:50, Anil Veerabhadrappa wrote: > + bnx2i_ep->ep_iscsi_cid = (u16) -1; As a matter of style: Wouldn't it be more logical to write "(u16) ~0" instead? Casting a negative value to unsigned seems strange to me. Regards, Ulrich --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
[PATCH 2/2] bnx2i : Fix "cid #n not valid" issue
* when bnx2i_adapter_ready() fails, connection handle(cid) = 0 is
wrongly freed because 'cid' is not yet allocated for the endpoint
* Fix is to initialize bnx2i_ep->ep_iscsi_cid to '-1' in bnx2i_alloc_ep()
and not in bnx2i_ep_connect() to avoid releasing invalid 'cid'
* There is already a check in bnx2i_free_iscsi_cid() not to free
invalid iscsi connection handle (-1)
Signed-off-by: Anil Veerabhadrappa
---
drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index 9535bb6..08d0bfc 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -387,6 +387,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct
bnx2i_hba *hba)
bnx2i_ep = ep->dd_data;
INIT_LIST_HEAD(&bnx2i_ep->link);
bnx2i_ep->state = EP_STATE_IDLE;
+ bnx2i_ep->ep_iscsi_cid = (u16) -1;
bnx2i_ep->hba = hba;
bnx2i_ep->hba_age = hba->age;
hba->ofld_conns_active++;
@@ -1678,8 +1679,6 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct
Scsi_Host *shost,
goto net_if_down;
}
- bnx2i_ep->state = EP_STATE_IDLE;
- bnx2i_ep->ep_iscsi_cid = (u16) -1;
bnx2i_ep->num_active_cmds = 0;
iscsi_cid = bnx2i_alloc_iscsi_cid(hba);
if (iscsi_cid == -1) {
--
1.5.4.3
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---
