Thanks Steffen for the comments. I agree with you that we could have subsequent fields with invalid data. However, we have not been really using any of the later fields, Other than firmware_version_string and iSCSI_features which are used primarily for information (prints) while in debug state.
So, the structure we were using was very old and used by our very early chips(BE1) which were never supported by this driver. Thanks Jay -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Steffen Maier Sent: Wednesday, March 13, 2013 3:30 AM To: [email protected] Cc: [email protected]; [email protected]; [email protected]; Kallickal, Jayamohan; John, Sony Subject: Re: [PATCH 08/15] be2iscsi: Fix displaying the FW Version from driver. On 03/12/2013 05:39 AM, [email protected] wrote: > From: "Jayamohan.Kallickal" <[email protected]> > > This patch fixes the display of proper FW Version from the driver. > > Signed-off-by: John Soni Jose <[email protected]> > Signed-off-by: Jayamohan Kallickal <[email protected]> > --- > drivers/scsi/be2iscsi/be_main.c | 2 ++ > drivers/scsi/be2iscsi/be_main.h | 2 ++ > drivers/scsi/be2iscsi/be_mgmt.c | 21 +++++++++++++++++++++ > drivers/scsi/be2iscsi/be_mgmt.h | 32 ++++++++++++++++++-------------- > 4 files changed, 43 insertions(+), 14 deletions(-) > diff --git a/drivers/scsi/be2iscsi/be_mgmt.h > b/drivers/scsi/be2iscsi/be_mgmt.h index 2e4968a..0a406a4 100644 > --- a/drivers/scsi/be2iscsi/be_mgmt.h > +++ b/drivers/scsi/be2iscsi/be_mgmt.h > @@ -156,25 +156,25 @@ union invalidate_commands_params { > } __packed; > > struct mgmt_hba_attributes { > - u8 flashrom_version_string[32]; > - u8 manufacturer_name[32]; > + u8 flashrom_version_string[BEISCSI_VER_STRLEN]; > + u8 manufacturer_name[BEISCSI_VER_STRLEN]; > u32 supported_modes; > u8 seeprom_version_lo; > u8 seeprom_version_hi; > u8 rsvd0[2]; > - u32 fw_cmd_data_struct_version; > + u32 ioctl_data_struct_version; > u32 ep_fw_data_struct_version; > - u32 future_reserved[12]; > + u8 ncsi_version_string[12]; Hm, this seems to replace 12*u32 by 12*u8, i.e. the subsequent fields would now have a reduced offset. Is this intentional? I would have expected to just use some part of future_reserved for newly defined fields and have the remainder of future_reserved still part of the structure to not shift the offset of subsequent fields; just like you did below in struct mgmt_hba_attributes. > u32 default_extended_timeout; > - u8 controller_model_number[32]; > + u8 controller_model_number[BEISCSI_VER_STRLEN]; > u8 controller_description[64]; > - u8 controller_serial_number[32]; > - u8 ip_version_string[32]; > - u8 firmware_version_string[32]; > - u8 bios_version_string[32]; > - u8 redboot_version_string[32]; > - u8 driver_version_string[32]; > - u8 fw_on_flash_version_string[32]; > + u8 controller_serial_number[BEISCSI_VER_STRLEN]; > + u8 ip_version_string[BEISCSI_VER_STRLEN]; > + u8 firmware_version_string[BEISCSI_VER_STRLEN]; > + u8 bios_version_string[BEISCSI_VER_STRLEN]; > + u8 redboot_version_string[BEISCSI_VER_STRLEN]; > + u8 driver_version_string[BEISCSI_VER_STRLEN]; > + u8 fw_on_flash_version_string[BEISCSI_VER_STRLEN]; > u32 functionalities_supported; > u16 max_cdblength; > u8 asic_revision; > @@ -190,7 +190,8 @@ struct mgmt_hba_attributes { > u32 firmware_post_status; > u32 hba_mtu[8]; > u8 iscsi_features; > - u8 future_u8[3]; > + u8 asic_generation; > + u8 future_u8[2]; > u32 future_u32[3]; > } __packed; > @@ -207,7 +208,7 @@ struct mgmt_controller_attributes { > u64 unique_identifier; > u8 netfilters; > u8 rsvd0[3]; > - u8 future_u32[4]; > + u32 future_u32[4]; > } __packed; While I suppose this doesn't break existing functionality, is it intentional to actually increase the size of the trailing reserved fields and thus the size of the entire struct mgmt_controller_attributes? Steffen Linux on System z Development IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
