* [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. @ 2012-03-03 15:47 santosh nayak 2012-03-05 11:43 ` Rajesh Borundia 2012-03-09 16:34 ` Rajesh Borundia 0 siblings, 2 replies; 13+ messages in thread From: santosh nayak @ 2012-03-03 15:47 UTC (permalink / raw) To: sony.chacko Cc: rajesh.borundia, netdev, linux-kernel, kernel-janitors, Santosh Nayak From: Santosh Nayak <santoshprasadnayak@gmail.com> Fix endian bug. Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> --- drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++----- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h index 2eeac32..b5de8a7 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { struct nx_vlan_ip_list { struct list_head list; - u32 ip_addr; + __be32 ip_addr; }; /* @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct nx_host_sds_ring *sds_ring, int max); void netxen_p3_free_mac_list(struct netxen_adapter *adapter); int netxen_config_intr_coalesce(struct netxen_adapter *adapter); int netxen_config_rss(struct netxen_adapter *adapter, int enable); -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd); +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd); int netxen_linkevent_request(struct netxen_adapter *adapter, int enable); void netxen_advert_link_change(struct netxen_adapter *adapter, int linkup); void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index 6f37470..0f81287 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter *adapter, int enable) return rv; } -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd) { nx_nic_req_t req; u64 word; + u64 ip_addr; int rv; memset(&req, 0, sizeof(nx_nic_req_t)); @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) req.req_hdr = cpu_to_le64(word); req.words[0] = cpu_to_le64(cmd); - req.words[1] = cpu_to_le64(ip); + ip_addr = be32_to_cpu(ip); + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1); if (rv != 0) { @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) { + if (*mac == ~0ULL) { offset = NX_OLD_MAC_ADDR_OFFSET + (adapter->portnum * sizeof(u64)); @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) + if (*mac == ~0ULL) return -1; } return 0; @@ -2178,7 +2180,7 @@ lock_try: NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter->ahw.pci_base0, waddr); raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); - *data_buff++ = cpu_to_le32(val); + *data_buff++ = val; fl_addr += sizeof(val); } readl((void __iomem *)(adapter->ahw.pci_base0 + NX_FLASH_SEM2_ULK)); diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index 8dc4a134..70783b4 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter *adapter) adapter->driver_mismatch = 1; return; } - ptr32[i] = cpu_to_le32(val); + ptr32[i] = val; offset += sizeof(u32); } -- 1.7.4.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-03 15:47 [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug santosh nayak @ 2012-03-05 11:43 ` Rajesh Borundia 2012-03-05 20:35 ` David Miller 2012-03-09 16:34 ` Rajesh Borundia 1 sibling, 1 reply; 13+ messages in thread From: Rajesh Borundia @ 2012-03-05 11:43 UTC (permalink / raw) To: santosh nayak, Sony Chacko; +Cc: netdev, linux-kernel, kernel-janitors I will review this and get back to you. Rajesh ________________________________________ From: santosh nayak [santoshprasadnayak@gmail.com] Sent: Saturday, March 03, 2012 9:17 PM To: Sony Chacko Cc: Rajesh Borundia; netdev; linux-kernel; kernel-janitors@vger.kernel.org; Santosh Nayak Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. From: Santosh Nayak <santoshprasadnayak@gmail.com> Fix endian bug. Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> --- drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++----- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h index 2eeac32..b5de8a7 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { struct nx_vlan_ip_list { struct list_head list; - u32 ip_addr; + __be32 ip_addr; }; /* @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct nx_host_sds_ring *sds_ring, int max); void netxen_p3_free_mac_list(struct netxen_adapter *adapter); int netxen_config_intr_coalesce(struct netxen_adapter *adapter); int netxen_config_rss(struct netxen_adapter *adapter, int enable); -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd); +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd); int netxen_linkevent_request(struct netxen_adapter *adapter, int enable); void netxen_advert_link_change(struct netxen_adapter *adapter, int linkup); void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index 6f37470..0f81287 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter *adapter, int enable) return rv; } -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd) { nx_nic_req_t req; u64 word; + u64 ip_addr; int rv; memset(&req, 0, sizeof(nx_nic_req_t)); @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) req.req_hdr = cpu_to_le64(word); req.words[0] = cpu_to_le64(cmd); - req.words[1] = cpu_to_le64(ip); + ip_addr = be32_to_cpu(ip); + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1); if (rv != 0) { @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) { + if (*mac == ~0ULL) { offset = NX_OLD_MAC_ADDR_OFFSET + (adapter->portnum * sizeof(u64)); @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) + if (*mac == ~0ULL) return -1; } return 0; @@ -2178,7 +2180,7 @@ lock_try: NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter->ahw.pci_base0, waddr); raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); - *data_buff++ = cpu_to_le32(val); + *data_buff++ = val; fl_addr += sizeof(val); } readl((void __iomem *)(adapter->ahw.pci_base0 + NX_FLASH_SEM2_ULK)); diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index 8dc4a134..70783b4 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter *adapter) adapter->driver_mismatch = 1; return; } - ptr32[i] = cpu_to_le32(val); + ptr32[i] = val; offset += sizeof(u32); } -- 1.7.4.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-05 11:43 ` Rajesh Borundia @ 2012-03-05 20:35 ` David Miller 2012-03-07 19:41 ` Rajesh Borundia 0 siblings, 1 reply; 13+ messages in thread From: David Miller @ 2012-03-05 20:35 UTC (permalink / raw) To: rajesh.borundia Cc: santoshprasadnayak, sony.chacko, netdev, linux-kernel, kernel-janitors From: Rajesh Borundia <rajesh.borundia@qlogic.com> Date: Mon, 5 Mar 2012 05:43:56 -0600 > I will review this and get back to you. Please don't quote patch postings this way. They look like new patch postings, and therefore get added to our patch tracking site. ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-05 20:35 ` David Miller @ 2012-03-07 19:41 ` Rajesh Borundia 0 siblings, 0 replies; 13+ messages in thread From: Rajesh Borundia @ 2012-03-07 19:41 UTC (permalink / raw) To: David Miller Cc: santoshprasadnayak, Sony Chacko, netdev, linux-kernel, kernel-janitors > -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Tuesday, March 06, 2012 2:05 AM > To: Rajesh Borundia > Cc: santoshprasadnayak@gmail.com; Sony Chacko; netdev; linux-kernel; > kernel-janitors@vger.kernel.org > Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > > From: Rajesh Borundia <rajesh.borundia@qlogic.com> > Date: Mon, 5 Mar 2012 05:43:56 -0600 > > > I will review this and get back to you. > > Please don't quote patch postings this way. > > They look like new patch postings, and therefore get added to our patch > tracking site. Sorry about that. I will take care from next time. Rajesh ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-03 15:47 [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug santosh nayak 2012-03-05 11:43 ` Rajesh Borundia @ 2012-03-09 16:34 ` Rajesh Borundia 2012-03-09 18:50 ` santosh prasad nayak 1 sibling, 1 reply; 13+ messages in thread From: Rajesh Borundia @ 2012-03-09 16:34 UTC (permalink / raw) To: santosh nayak, Sony Chacko; +Cc: netdev, linux-kernel, kernel-janitors > -----Original Message----- > From: santosh nayak [mailto:santoshprasadnayak@gmail.com] > Sent: Saturday, March 03, 2012 9:18 PM > To: Sony Chacko > Cc: Rajesh Borundia; netdev; linux-kernel; kernel- > janitors@vger.kernel.org; Santosh Nayak > Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > > From: Santosh Nayak <santoshprasadnayak@gmail.com> > > Fix endian bug. > > Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> > --- > drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- > drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++----- > .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- > 3 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > index 2eeac32..b5de8a7 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { > > struct nx_vlan_ip_list { > struct list_head list; > - u32 ip_addr; > + __be32 ip_addr; > }; > > /* > @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct > nx_host_sds_ring *sds_ring, int max); > void netxen_p3_free_mac_list(struct netxen_adapter *adapter); > int netxen_config_intr_coalesce(struct netxen_adapter *adapter); > int netxen_config_rss(struct netxen_adapter *adapter, int enable); > -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int > cmd); > +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > int cmd); > int netxen_linkevent_request(struct netxen_adapter *adapter, int > enable); > void netxen_advert_link_change(struct netxen_adapter *adapter, int > linkup); > void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > index 6f37470..0f81287 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter > *adapter, int enable) > return rv; > } > > -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int > cmd) > +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > int cmd) > { > nx_nic_req_t req; > u64 word; > + u64 ip_addr; > int rv; > > memset(&req, 0, sizeof(nx_nic_req_t)); > @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter > *adapter, u32 ip, int cmd) > req.req_hdr = cpu_to_le64(word); > > req.words[0] = cpu_to_le64(cmd); > - req.words[1] = cpu_to_le64(ip); > + ip_addr = be32_to_cpu(ip); > + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); Adapter requires ip value in big endian stored at lower 32 bit address. The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and adapter Will get incorrect ip addr. Instead u can do. U32 *ip_addr; ip_addr = (u32 *)&req.words[1]; *ip_addr = ip; > > rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 > *)&req, 1); > if (rv != 0) { > @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct > netxen_adapter *adapter, u64 *mac) > if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == > -1) > return -1; > > - if (*mac == cpu_to_le64(~0ULL)) { > + if (*mac == ~0ULL) { *mac is in little endian format so compare it with cpu_to_le64. > > offset = NX_OLD_MAC_ADDR_OFFSET + > (adapter->portnum * sizeof(u64)); > @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct > netxen_adapter *adapter, u64 *mac) > offset, sizeof(u64), pmac) == -1) > return -1; > > - if (*mac == cpu_to_le64(~0ULL)) > + if (*mac == ~0ULL) *mac here is in little endian format so compare it with cpu_to_le64. > return -1; > } > return 0; > @@ -2178,7 +2180,7 @@ lock_try: > NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter->ahw.pci_base0, > waddr); > raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); > NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); > - *data_buff++ = cpu_to_le32(val); > + *data_buff++ = val; It should be cpu_to_le32 as it is returned to tool which requires output in little endian. > fl_addr += sizeof(val); > } > readl((void __iomem *)(adapter->ahw.pci_base0 + > NX_FLASH_SEM2_ULK)); > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > index 8dc4a134..70783b4 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter > *adapter) > adapter->driver_mismatch = 1; > return; > } > - ptr32[i] = cpu_to_le32(val); > + ptr32[i] = val; Here val should be in little endian (cpu_to_le32) as it will be referenced by byte array to print serial number. > offset += sizeof(u32); > } > > -- > 1.7.4.4 > Sorry for Late reply. Rajesh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-09 16:34 ` Rajesh Borundia @ 2012-03-09 18:50 ` santosh prasad nayak 2012-03-10 19:01 ` Rajesh Borundia 0 siblings, 1 reply; 13+ messages in thread From: santosh prasad nayak @ 2012-03-09 18:50 UTC (permalink / raw) To: Rajesh Borundia; +Cc: Sony Chacko, netdev, linux-kernel, kernel-janitors On Fri, Mar 9, 2012 at 10:04 PM, Rajesh Borundia <rajesh.borundia@qlogic.com> wrote: > >> -----Original Message----- >> From: santosh nayak [mailto:santoshprasadnayak@gmail.com] >> Sent: Saturday, March 03, 2012 9:18 PM >> To: Sony Chacko >> Cc: Rajesh Borundia; netdev; linux-kernel; kernel- >> janitors@vger.kernel.org; Santosh Nayak >> Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. >> >> From: Santosh Nayak <santoshprasadnayak@gmail.com> >> >> Fix endian bug. >> >> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> >> --- >> drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- >> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++----- >> .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- >> 3 files changed, 10 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> index 2eeac32..b5de8a7 100644 >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { >> >> struct nx_vlan_ip_list { >> struct list_head list; >> - u32 ip_addr; >> + __be32 ip_addr; >> }; >> >> /* >> @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct >> nx_host_sds_ring *sds_ring, int max); >> void netxen_p3_free_mac_list(struct netxen_adapter *adapter); >> int netxen_config_intr_coalesce(struct netxen_adapter *adapter); >> int netxen_config_rss(struct netxen_adapter *adapter, int enable); >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int >> cmd); >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, >> int cmd); >> int netxen_linkevent_request(struct netxen_adapter *adapter, int >> enable); >> void netxen_advert_link_change(struct netxen_adapter *adapter, int >> linkup); >> void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> index 6f37470..0f81287 100644 >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter >> *adapter, int enable) >> return rv; >> } >> >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int >> cmd) >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, >> int cmd) >> { >> nx_nic_req_t req; >> u64 word; >> + u64 ip_addr; >> int rv; >> >> memset(&req, 0, sizeof(nx_nic_req_t)); >> @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter >> *adapter, u32 ip, int cmd) >> req.req_hdr = cpu_to_le64(word); >> >> req.words[0] = cpu_to_le64(cmd); >> - req.words[1] = cpu_to_le64(ip); >> + ip_addr = be32_to_cpu(ip); >> + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); > > Adapter requires ip value in big endian stored at lower 32 bit address. > The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and adapter > Will get incorrect ip addr. Instead u can do. > > U32 *ip_addr; > ip_addr = (u32 *)&req.words[1]; > *ip_addr = ip; 1. It looks incomplete. In the function call "netxen_send_cmd_descs" we have to pass "&req" as 2nd argument not "ipaddr". 2. Your above suggestion is with assumption that the data type of 2nd argument "ip" in "netxen_config_ipaddr()" is still "u32". This is not true. Some days back you suggested to change the data type to "__be32". In the present patch the "ip" is in "__be32" format i.e already in Big endian format as per requirement. We need to only convert this 32 bit to 64 bit. There are two ways: a. *(__be64 *)&req.words[1] = ip; // auto conversion b. *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip)); // explicit conversion. Please correct me if I am wrong. regards Santosh > > >> >> rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 >> *)&req, 1); >> if (rv != 0) { >> @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct >> netxen_adapter *adapter, u64 *mac) >> if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == >> -1) >> return -1; >> >> - if (*mac == cpu_to_le64(~0ULL)) { >> + if (*mac == ~0ULL) { > > *mac is in little endian format so compare it with cpu_to_le64. > >> >> offset = NX_OLD_MAC_ADDR_OFFSET + >> (adapter->portnum * sizeof(u64)); >> @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct >> netxen_adapter *adapter, u64 *mac) >> offset, sizeof(u64), pmac) == -1) >> return -1; >> >> - if (*mac == cpu_to_le64(~0ULL)) >> + if (*mac == ~0ULL) > > *mac here is in little endian format so compare it with cpu_to_le64. > >> return -1; >> } >> return 0; >> @@ -2178,7 +2180,7 @@ lock_try: >> NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter->ahw.pci_base0, >> waddr); >> raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); >> NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); >> - *data_buff++ = cpu_to_le32(val); >> + *data_buff++ = val; > > It should be cpu_to_le32 as it is returned to tool which requires > output in little endian. > >> fl_addr += sizeof(val); >> } >> readl((void __iomem *)(adapter->ahw.pci_base0 + >> NX_FLASH_SEM2_ULK)); >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> index 8dc4a134..70783b4 100644 >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter >> *adapter) >> adapter->driver_mismatch = 1; >> return; >> } >> - ptr32[i] = cpu_to_le32(val); >> + ptr32[i] = val; > > Here val should be in little endian (cpu_to_le32) as it will be referenced by byte array to print serial number. > >> offset += sizeof(u32); >> } >> >> -- >> 1.7.4.4 >> > > > Sorry for Late reply. > > Rajesh > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-09 18:50 ` santosh prasad nayak @ 2012-03-10 19:01 ` Rajesh Borundia 2012-03-11 9:16 ` santosh prasad nayak 0 siblings, 1 reply; 13+ messages in thread From: Rajesh Borundia @ 2012-03-10 19:01 UTC (permalink / raw) To: santosh prasad nayak; +Cc: Sony Chacko, netdev, linux-kernel, kernel-janitors > -----Original Message----- > From: santosh prasad nayak [mailto:santoshprasadnayak@gmail.com] > Sent: Saturday, March 10, 2012 12:20 AM > To: Rajesh Borundia > Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@vger.kernel.org > Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > > On Fri, Mar 9, 2012 at 10:04 PM, Rajesh Borundia > <rajesh.borundia@qlogic.com> wrote: > > > >> -----Original Message----- > >> From: santosh nayak [mailto:santoshprasadnayak@gmail.com] > >> Sent: Saturday, March 03, 2012 9:18 PM > >> To: Sony Chacko > >> Cc: Rajesh Borundia; netdev; linux-kernel; kernel- > >> janitors@vger.kernel.org; Santosh Nayak > >> Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > >> > >> From: Santosh Nayak <santoshprasadnayak@gmail.com> > >> > >> Fix endian bug. > >> > >> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> > >> --- > >> drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- > >> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++-- > --- > >> .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- > >> 3 files changed, 10 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> index 2eeac32..b5de8a7 100644 > >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { > >> > >> struct nx_vlan_ip_list { > >> struct list_head list; > >> - u32 ip_addr; > >> + __be32 ip_addr; > >> }; > >> > >> /* > >> @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct > >> nx_host_sds_ring *sds_ring, int max); > >> void netxen_p3_free_mac_list(struct netxen_adapter *adapter); > >> int netxen_config_intr_coalesce(struct netxen_adapter *adapter); > >> int netxen_config_rss(struct netxen_adapter *adapter, int enable); > >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, > int > >> cmd); > >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > >> int cmd); > >> int netxen_linkevent_request(struct netxen_adapter *adapter, int > >> enable); > >> void netxen_advert_link_change(struct netxen_adapter *adapter, int > >> linkup); > >> void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); > >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> index 6f37470..0f81287 100644 > >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter > >> *adapter, int enable) > >> return rv; > >> } > >> > >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, > int > >> cmd) > >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > >> int cmd) > >> { > >> nx_nic_req_t req; > >> u64 word; > >> + u64 ip_addr; > >> int rv; > >> > >> memset(&req, 0, sizeof(nx_nic_req_t)); > >> @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter > >> *adapter, u32 ip, int cmd) > >> req.req_hdr = cpu_to_le64(word); > >> > >> req.words[0] = cpu_to_le64(cmd); > >> - req.words[1] = cpu_to_le64(ip); > >> + ip_addr = be32_to_cpu(ip); > >> + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); > > > > > > Adapter requires ip value in big endian stored at lower 32 bit > address. > > The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and > adapter > > Will get incorrect ip addr. Instead u can do. > > > > U32 *ip_addr; > > ip_addr = (u32 *)&req.words[1]; > > *ip_addr = ip; > > > 1. It looks incomplete. > In the function call "netxen_send_cmd_descs" we have to pass "&req" > as > 2nd argument not "ipaddr". I should have sent a patch. This piece of code was just to show how to copy ip addr in req.words[1]. > > 2. Your above suggestion is with assumption that the data type of 2nd > argument "ip" > in "netxen_config_ipaddr()" is still "u32". This is not true. > > Some days back you suggested to change the data type to "__be32". > In the present patch > the "ip" is in "__be32" format i.e already in Big endian format > as per requirement. > We need to only convert this 32 bit to 64 bit. There are two > ways: > No I did not assume that ip is u32, ip is still __be32(ip value is in form of big endian) though I should have mentioned it explicitly. But the ip value should be copied to lower 32 bit of req.words[1]. > a. *(__be64 *)&req.words[1] = ip; // auto conversion In big endian machine MSB is copied into MSB first. So ip will get copied into higher 32 bit of req.words[1] but adapter requires it in lower 32 bit. > > b. *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip)); > // explicit conversion. > If you follow second cpu_to_be64 it will swap lower 32 bit of ip with higher 32 bit in little endian machine. But adapter requires it in lower 32 bit. Simple solution to copy ip in req.words[1] could be memcpy(&req.words[1], &ip, sizeof(u32)); > > Please correct me if I am wrong. > > > regards > Santosh > > > > > > > > > >> > >> rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 > >> *)&req, 1); > >> if (rv != 0) { > >> @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct > >> netxen_adapter *adapter, u64 *mac) > >> if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) > == > >> -1) > >> return -1; > >> > >> - if (*mac == cpu_to_le64(~0ULL)) { > >> + if (*mac == ~0ULL) { > > > > *mac is in little endian format so compare it with cpu_to_le64. > > > >> > >> offset = NX_OLD_MAC_ADDR_OFFSET + > >> (adapter->portnum * sizeof(u64)); > >> @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct > >> netxen_adapter *adapter, u64 *mac) > >> offset, sizeof(u64), pmac) == > -1) > >> return -1; > >> > >> - if (*mac == cpu_to_le64(~0ULL)) > >> + if (*mac == ~0ULL) > > > > *mac here is in little endian format so compare it with cpu_to_le64. > > > >> return -1; > >> } > >> return 0; > >> @@ -2178,7 +2180,7 @@ lock_try: > >> NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter- > >ahw.pci_base0, > >> waddr); > >> raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); > >> NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); > >> - *data_buff++ = cpu_to_le32(val); > >> + *data_buff++ = val; > > > > It should be cpu_to_le32 as it is returned to tool which requires > > output in little endian. > > > >> fl_addr += sizeof(val); > >> } > >> readl((void __iomem *)(adapter->ahw.pci_base0 + > >> NX_FLASH_SEM2_ULK)); > >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> index 8dc4a134..70783b4 100644 > >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter > >> *adapter) > >> adapter->driver_mismatch = 1; > >> return; > >> } > >> - ptr32[i] = cpu_to_le32(val); > >> + ptr32[i] = val; > > > > Here val should be in little endian (cpu_to_le32) as it will be > referenced by byte array to print serial number. > > > >> offset += sizeof(u32); > >> } > >> > >> -- > >> 1.7.4.4 > >> > > > > > > Sorry for Late reply. > > > > Rajesh > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-10 19:01 ` Rajesh Borundia @ 2012-03-11 9:16 ` santosh prasad nayak 2012-03-12 6:19 ` Rajesh Borundia 2012-03-12 9:37 ` David Laight 0 siblings, 2 replies; 13+ messages in thread From: santosh prasad nayak @ 2012-03-11 9:16 UTC (permalink / raw) To: Rajesh Borundia; +Cc: Sony Chacko, netdev, linux-kernel, kernel-janitors Thanks Rajesh for clarification. Included all your inputs in the following patch. This is for review not a formal one. Once review is done I will send a formal patch. diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h index 2eeac32..b5de8a7 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { struct nx_vlan_ip_list { struct list_head list; - u32 ip_addr; + __be32 ip_addr; }; /* @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct nx_host_sds_ring *sds_ring, int max); void netxen_p3_free_mac_list(struct netxen_adapter *adapter); int netxen_config_intr_coalesce(struct netxen_adapter *adapter); int netxen_config_rss(struct netxen_adapter *adapter, int enable); -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd); +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd); int netxen_linkevent_request(struct netxen_adapter *adapter, int enable); void netxen_advert_link_change(struct netxen_adapter *adapter, int linkup); void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index 6f37470..59d5ee7 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -909,7 +909,7 @@ int netxen_config_rss(struct netxen_adapter *adapter, int enable) return rv; } -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, int cmd) { nx_nic_req_t req; u64 word; @@ -922,7 +922,7 @@ int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int cmd) req.req_hdr = cpu_to_le64(word); req.words[0] = cpu_to_le64(cmd); - req.words[1] = cpu_to_le64(ip); + memcpy(&req.words[1], &ip, sizeof(u32)); rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1); if (rv != 0) { @@ -1050,7 +1050,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) { + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) { offset = NX_OLD_MAC_ADDR_OFFSET + (adapter->portnum * sizeof(u64)); @@ -1059,7 +1059,7 @@ int netxen_get_flash_mac_addr(struct netxen_adapter *adapter, u64 *mac) offset, sizeof(u64), pmac) == -1) return -1; - if (*mac == cpu_to_le64(~0ULL)) + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) return -1; } return 0; @@ -2155,7 +2155,7 @@ static u32 netxen_md_rd_crb(struct netxen_adapter *adapter, static u32 netxen_md_rdrom(struct netxen_adapter *adapter, struct netxen_minidump_entry_rdrom - *romEntry, u32 *data_buff) + *romEntry, __le32 *data_buff) { int i, count = 0; u32 size, lck_val; diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index 7648995..65a718f 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -805,12 +805,12 @@ netxen_check_options(struct netxen_adapter *adapter) char brd_name[NETXEN_MAX_SHORT_NAME]; char serial_num[32]; int i, offset, val, err; - int *ptr32; + __le32 *ptr32; struct pci_dev *pdev = adapter->pdev; adapter->driver_mismatch = 0; - ptr32 = (int *)&serial_num; + ptr32 = (__le32 *)&serial_num; offset = NX_FW_SERIAL_NUM_OFFSET; for (i = 0; i < 8; i++) { if (netxen_rom_fast_read(adapter, offset, &val) == -1) { On Sun, Mar 11, 2012 at 12:31 AM, Rajesh Borundia <rajesh.borundia@qlogic.com> wrote: > > >> -----Original Message----- >> From: santosh prasad nayak [mailto:santoshprasadnayak@gmail.com] >> Sent: Saturday, March 10, 2012 12:20 AM >> To: Rajesh Borundia >> Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@vger.kernel.org >> Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. >> >> On Fri, Mar 9, 2012 at 10:04 PM, Rajesh Borundia >> <rajesh.borundia@qlogic.com> wrote: >> > >> >> -----Original Message----- >> >> From: santosh nayak [mailto:santoshprasadnayak@gmail.com] >> >> Sent: Saturday, March 03, 2012 9:18 PM >> >> To: Sony Chacko >> >> Cc: Rajesh Borundia; netdev; linux-kernel; kernel- >> >> janitors@vger.kernel.org; Santosh Nayak >> >> Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. >> >> >> >> From: Santosh Nayak <santoshprasadnayak@gmail.com> >> >> >> >> Fix endian bug. >> >> >> >> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> >> >> --- >> >> drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- >> >> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 +++++++-- >> --- >> >> .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- >> >> 3 files changed, 10 insertions(+), 8 deletions(-) >> >> >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> >> index 2eeac32..b5de8a7 100644 >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h >> >> @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { >> >> >> >> struct nx_vlan_ip_list { >> >> struct list_head list; >> >> - u32 ip_addr; >> >> + __be32 ip_addr; >> >> }; >> >> >> >> /* >> >> @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct >> >> nx_host_sds_ring *sds_ring, int max); >> >> void netxen_p3_free_mac_list(struct netxen_adapter *adapter); >> >> int netxen_config_intr_coalesce(struct netxen_adapter *adapter); >> >> int netxen_config_rss(struct netxen_adapter *adapter, int enable); >> >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, >> int >> >> cmd); >> >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, >> >> int cmd); >> >> int netxen_linkevent_request(struct netxen_adapter *adapter, int >> >> enable); >> >> void netxen_advert_link_change(struct netxen_adapter *adapter, int >> >> linkup); >> >> void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> >> index 6f37470..0f81287 100644 >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c >> >> @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter >> >> *adapter, int enable) >> >> return rv; >> >> } >> >> >> >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, >> int >> >> cmd) >> >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, >> >> int cmd) >> >> { >> >> nx_nic_req_t req; >> >> u64 word; >> >> + u64 ip_addr; >> >> int rv; >> >> >> >> memset(&req, 0, sizeof(nx_nic_req_t)); >> >> @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct netxen_adapter >> >> *adapter, u32 ip, int cmd) >> >> req.req_hdr = cpu_to_le64(word); >> >> >> >> req.words[0] = cpu_to_le64(cmd); >> >> - req.words[1] = cpu_to_le64(ip); >> >> + ip_addr = be32_to_cpu(ip); >> >> + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); >> > >> >> >> > Adapter requires ip value in big endian stored at lower 32 bit >> address. >> > The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and >> adapter >> > Will get incorrect ip addr. Instead u can do. >> > >> > U32 *ip_addr; >> > ip_addr = (u32 *)&req.words[1]; >> > *ip_addr = ip; >> >> >> 1. It looks incomplete. >> In the function call "netxen_send_cmd_descs" we have to pass "&req" >> as >> 2nd argument not "ipaddr". > > I should have sent a patch. This piece of code was just to show how to > copy ip addr in req.words[1]. > >> >> 2. Your above suggestion is with assumption that the data type of 2nd >> argument "ip" >> in "netxen_config_ipaddr()" is still "u32". This is not true. >> >> Some days back you suggested to change the data type to "__be32". >> In the present patch >> the "ip" is in "__be32" format i.e already in Big endian format >> as per requirement. >> We need to only convert this 32 bit to 64 bit. There are two >> ways: >> > No I did not assume that ip is u32, ip is still __be32(ip value is in form of big endian) > though I should have mentioned it explicitly. But the ip value should be copied to lower 32 bit of req.words[1]. > > >> a. *(__be64 *)&req.words[1] = ip; // auto conversion > In big endian machine MSB is copied into MSB first. So ip will get copied into higher 32 bit of > req.words[1] but adapter requires it in lower 32 bit. >> >> b. *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip)); >> // explicit conversion. >> > If you follow second cpu_to_be64 it will swap lower 32 bit of ip with higher 32 bit in little endian machine. > But adapter requires it in lower 32 bit. > > Simple solution to copy ip in req.words[1] could be memcpy(&req.words[1], &ip, sizeof(u32)); > > >> >> Please correct me if I am wrong. >> >> >> regards >> Santosh >> >> >> >> >> > >> > >> >> >> >> rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 >> >> *)&req, 1); >> >> if (rv != 0) { >> >> @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct >> >> netxen_adapter *adapter, u64 *mac) >> >> if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) >> == >> >> -1) >> >> return -1; >> >> >> >> - if (*mac == cpu_to_le64(~0ULL)) { >> >> + if (*mac == ~0ULL) { >> > >> > *mac is in little endian format so compare it with cpu_to_le64. >> > >> >> >> >> offset = NX_OLD_MAC_ADDR_OFFSET + >> >> (adapter->portnum * sizeof(u64)); >> >> @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct >> >> netxen_adapter *adapter, u64 *mac) >> >> offset, sizeof(u64), pmac) == >> -1) >> >> return -1; >> >> >> >> - if (*mac == cpu_to_le64(~0ULL)) >> >> + if (*mac == ~0ULL) >> > >> > *mac here is in little endian format so compare it with cpu_to_le64. >> > >> >> return -1; >> >> } >> >> return 0; >> >> @@ -2178,7 +2180,7 @@ lock_try: >> >> NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter- >> >ahw.pci_base0, >> >> waddr); >> >> raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); >> >> NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, &val); >> >> - *data_buff++ = cpu_to_le32(val); >> >> + *data_buff++ = val; >> > >> > It should be cpu_to_le32 as it is returned to tool which requires >> > output in little endian. >> > >> >> fl_addr += sizeof(val); >> >> } >> >> readl((void __iomem *)(adapter->ahw.pci_base0 + >> >> NX_FLASH_SEM2_ULK)); >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> >> index 8dc4a134..70783b4 100644 >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c >> >> @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter >> >> *adapter) >> >> adapter->driver_mismatch = 1; >> >> return; >> >> } >> >> - ptr32[i] = cpu_to_le32(val); >> >> + ptr32[i] = val; >> > >> > Here val should be in little endian (cpu_to_le32) as it will be >> referenced by byte array to print serial number. >> > >> >> offset += sizeof(u32); >> >> } >> >> >> >> -- >> >> 1.7.4.4 >> >> >> > >> > >> > Sorry for Late reply. >> > >> > Rajesh >> > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-11 9:16 ` santosh prasad nayak @ 2012-03-12 6:19 ` Rajesh Borundia 2012-03-12 9:37 ` David Laight 1 sibling, 0 replies; 13+ messages in thread From: Rajesh Borundia @ 2012-03-12 6:19 UTC (permalink / raw) To: santosh prasad nayak; +Cc: Sony Chacko, netdev, linux-kernel, kernel-janitors > -----Original Message----- > From: santosh prasad nayak [mailto:santoshprasadnayak@gmail.com] > Sent: Sunday, March 11, 2012 2:47 PM > To: Rajesh Borundia > Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@vger.kernel.org > Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > > Thanks Rajesh for clarification. > Included all your inputs in the following patch. > This is for review not a formal one. Once review is done I will send a > formal patch. > > Looks fine to me. > > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > index 2eeac32..b5de8a7 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { > > struct nx_vlan_ip_list { > struct list_head list; > - u32 ip_addr; > + __be32 ip_addr; > }; > > /* > @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct > nx_host_sds_ring *sds_ring, int max); > void netxen_p3_free_mac_list(struct netxen_adapter *adapter); > int netxen_config_intr_coalesce(struct netxen_adapter *adapter); > int netxen_config_rss(struct netxen_adapter *adapter, int enable); > -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int > cmd); > +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > int cmd); > int netxen_linkevent_request(struct netxen_adapter *adapter, int > enable); > void netxen_advert_link_change(struct netxen_adapter *adapter, int > linkup); > void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 *); > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > index 6f37470..59d5ee7 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > @@ -909,7 +909,7 @@ int netxen_config_rss(struct netxen_adapter > *adapter, int enable) > return rv; > } > > -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, int > cmd) > +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 ip, > int cmd) > { > nx_nic_req_t req; > u64 word; > @@ -922,7 +922,7 @@ int netxen_config_ipaddr(struct netxen_adapter > *adapter, u32 ip, int cmd) > req.req_hdr = cpu_to_le64(word); > > req.words[0] = cpu_to_le64(cmd); > - req.words[1] = cpu_to_le64(ip); > + memcpy(&req.words[1], &ip, sizeof(u32)); > > rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 > *)&req, 1); > if (rv != 0) { > @@ -1050,7 +1050,7 @@ int netxen_get_flash_mac_addr(struct > netxen_adapter *adapter, u64 *mac) > if (netxen_get_flash_block(adapter, offset, sizeof(u64), pmac) == > -1) > return -1; > > - if (*mac == cpu_to_le64(~0ULL)) { > + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) { > > offset = NX_OLD_MAC_ADDR_OFFSET + > (adapter->portnum * sizeof(u64)); > @@ -1059,7 +1059,7 @@ int netxen_get_flash_mac_addr(struct > netxen_adapter *adapter, u64 *mac) > offset, sizeof(u64), pmac) == -1) > return -1; > > - if (*mac == cpu_to_le64(~0ULL)) > + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) > return -1; > } > return 0; > @@ -2155,7 +2155,7 @@ static u32 netxen_md_rd_crb(struct > netxen_adapter *adapter, > static u32 > netxen_md_rdrom(struct netxen_adapter *adapter, > struct netxen_minidump_entry_rdrom > - *romEntry, u32 *data_buff) > + *romEntry, __le32 *data_buff) > { > int i, count = 0; > u32 size, lck_val; > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > index 7648995..65a718f 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > @@ -805,12 +805,12 @@ netxen_check_options(struct netxen_adapter > *adapter) > char brd_name[NETXEN_MAX_SHORT_NAME]; > char serial_num[32]; > int i, offset, val, err; > - int *ptr32; > + __le32 *ptr32; > struct pci_dev *pdev = adapter->pdev; > > adapter->driver_mismatch = 0; > > - ptr32 = (int *)&serial_num; > + ptr32 = (__le32 *)&serial_num; > offset = NX_FW_SERIAL_NUM_OFFSET; > for (i = 0; i < 8; i++) { > if (netxen_rom_fast_read(adapter, offset, &val) == -1) { > > > > On Sun, Mar 11, 2012 at 12:31 AM, Rajesh Borundia > <rajesh.borundia@qlogic.com> wrote: > > > > > >> -----Original Message----- > >> From: santosh prasad nayak [mailto:santoshprasadnayak@gmail.com] > >> Sent: Saturday, March 10, 2012 12:20 AM > >> To: Rajesh Borundia > >> Cc: Sony Chacko; netdev; linux-kernel; kernel- > janitors@vger.kernel.org > >> Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > >> > >> On Fri, Mar 9, 2012 at 10:04 PM, Rajesh Borundia > >> <rajesh.borundia@qlogic.com> wrote: > >> > > >> >> -----Original Message----- > >> >> From: santosh nayak [mailto:santoshprasadnayak@gmail.com] > >> >> Sent: Saturday, March 03, 2012 9:18 PM > >> >> To: Sony Chacko > >> >> Cc: Rajesh Borundia; netdev; linux-kernel; kernel- > >> >> janitors@vger.kernel.org; Santosh Nayak > >> >> Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > >> >> > >> >> From: Santosh Nayak <santoshprasadnayak@gmail.com> > >> >> > >> >> Fix endian bug. > >> >> > >> >> Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> > >> >> --- > >> >> drivers/net/ethernet/qlogic/netxen/netxen_nic.h | 4 ++-- > >> >> drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 12 > +++++++-- > >> --- > >> >> .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- > >> >> 3 files changed, 10 insertions(+), 8 deletions(-) > >> >> > >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> >> index 2eeac32..b5de8a7 100644 > >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h > >> >> @@ -954,7 +954,7 @@ typedef struct nx_mac_list_s { > >> >> > >> >> struct nx_vlan_ip_list { > >> >> struct list_head list; > >> >> - u32 ip_addr; > >> >> + __be32 ip_addr; > >> >> }; > >> >> > >> >> /* > >> >> @@ -1780,7 +1780,7 @@ int netxen_process_rcv_ring(struct > >> >> nx_host_sds_ring *sds_ring, int max); > >> >> void netxen_p3_free_mac_list(struct netxen_adapter *adapter); > >> >> int netxen_config_intr_coalesce(struct netxen_adapter *adapter); > >> >> int netxen_config_rss(struct netxen_adapter *adapter, int > enable); > >> >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, > >> int > >> >> cmd); > >> >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 > ip, > >> >> int cmd); > >> >> int netxen_linkevent_request(struct netxen_adapter *adapter, int > >> >> enable); > >> >> void netxen_advert_link_change(struct netxen_adapter *adapter, > int > >> >> linkup); > >> >> void netxen_pci_camqm_read_2M(struct netxen_adapter *, u64, u64 > *); > >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> >> index 6f37470..0f81287 100644 > >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c > >> >> @@ -909,10 +909,11 @@ int netxen_config_rss(struct netxen_adapter > >> >> *adapter, int enable) > >> >> return rv; > >> >> } > >> >> > >> >> -int netxen_config_ipaddr(struct netxen_adapter *adapter, u32 ip, > >> int > >> >> cmd) > >> >> +int netxen_config_ipaddr(struct netxen_adapter *adapter, __be32 > ip, > >> >> int cmd) > >> >> { > >> >> nx_nic_req_t req; > >> >> u64 word; > >> >> + u64 ip_addr; > >> >> int rv; > >> >> > >> >> memset(&req, 0, sizeof(nx_nic_req_t)); > >> >> @@ -922,7 +923,8 @@ int netxen_config_ipaddr(struct > netxen_adapter > >> >> *adapter, u32 ip, int cmd) > >> >> req.req_hdr = cpu_to_le64(word); > >> >> > >> >> req.words[0] = cpu_to_le64(cmd); > >> >> - req.words[1] = cpu_to_le64(ip); > >> >> + ip_addr = be32_to_cpu(ip); > >> >> + *(__be64 *)&req.words[1] = cpu_to_be64(ip_addr); > >> > > >> > >> > >> > Adapter requires ip value in big endian stored at lower 32 bit > >> address. > >> > The cpu_to_be64 will swap the lower 32 bit with higher 32 bit and > >> adapter > >> > Will get incorrect ip addr. Instead u can do. > >> > > >> > U32 *ip_addr; > >> > ip_addr = (u32 *)&req.words[1]; > >> > *ip_addr = ip; > >> > >> > >> 1. It looks incomplete. > >> In the function call "netxen_send_cmd_descs" we have to pass > "&req" > >> as > >> 2nd argument not "ipaddr". > > > > I should have sent a patch. This piece of code was just to show how > to > > copy ip addr in req.words[1]. > > > >> > >> 2. Your above suggestion is with assumption that the data type of > 2nd > >> argument "ip" > >> in "netxen_config_ipaddr()" is still "u32". This is not true. > >> > >> Some days back you suggested to change the data type to > "__be32". > >> In the present patch > >> the "ip" is in "__be32" format i.e already in Big endian > format > >> as per requirement. > >> We need to only convert this 32 bit to 64 bit. There are two > >> ways: > >> > > No I did not assume that ip is u32, ip is still __be32(ip value is > in form of big endian) > > though I should have mentioned it explicitly. But the ip value > should be copied to lower 32 bit of req.words[1]. > > > > > >> a. *(__be64 *)&req.words[1] = ip; // auto conversion > > In big endian machine MSB is copied into MSB first. So ip will get > copied into higher 32 bit of > > req.words[1] but adapter requires it in lower 32 bit. > >> > >> b. *(__be64 *)&req.words[1] = cpu_to_be64(be32_to_cpu(ip)); > >> // explicit conversion. > >> > > If you follow second cpu_to_be64 it will swap lower 32 bit of ip with > higher 32 bit in little endian machine. > > But adapter requires it in lower 32 bit. > > > > Simple solution to copy ip in req.words[1] could be > memcpy(&req.words[1], &ip, sizeof(u32)); > > > > > >> > >> Please correct me if I am wrong. > >> > >> > >> regards > >> Santosh > >> > >> > >> > >> > >> > > >> > > >> >> > >> >> rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 > >> >> *)&req, 1); > >> >> if (rv != 0) { > >> >> @@ -1050,7 +1052,7 @@ int netxen_get_flash_mac_addr(struct > >> >> netxen_adapter *adapter, u64 *mac) > >> >> if (netxen_get_flash_block(adapter, offset, sizeof(u64), > pmac) > >> == > >> >> -1) > >> >> return -1; > >> >> > >> >> - if (*mac == cpu_to_le64(~0ULL)) { > >> >> + if (*mac == ~0ULL) { > >> > > >> > *mac is in little endian format so compare it with cpu_to_le64. > >> > > >> >> > >> >> offset = NX_OLD_MAC_ADDR_OFFSET + > >> >> (adapter->portnum * sizeof(u64)); > >> >> @@ -1059,7 +1061,7 @@ int netxen_get_flash_mac_addr(struct > >> >> netxen_adapter *adapter, u64 *mac) > >> >> offset, sizeof(u64), pmac) > == > >> -1) > >> >> return -1; > >> >> > >> >> - if (*mac == cpu_to_le64(~0ULL)) > >> >> + if (*mac == ~0ULL) > >> > > >> > *mac here is in little endian format so compare it with > cpu_to_le64. > >> > > >> >> return -1; > >> >> } > >> >> return 0; > >> >> @@ -2178,7 +2180,7 @@ lock_try: > >> >> NX_WR_DUMP_REG(FLASH_ROM_WINDOW, adapter- > >> >ahw.pci_base0, > >> >> waddr); > >> >> raddr = FLASH_ROM_DATA + (fl_addr & 0x0000FFFF); > >> >> NX_RD_DUMP_REG(raddr, adapter->ahw.pci_base0, > &val); > >> >> - *data_buff++ = cpu_to_le32(val); > >> >> + *data_buff++ = val; > >> > > >> > It should be cpu_to_le32 as it is returned to tool which requires > >> > output in little endian. > >> > > >> >> fl_addr += sizeof(val); > >> >> } > >> >> readl((void __iomem *)(adapter->ahw.pci_base0 + > >> >> NX_FLASH_SEM2_ULK)); > >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> >> b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> >> index 8dc4a134..70783b4 100644 > >> >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c > >> >> @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter > >> >> *adapter) > >> >> adapter->driver_mismatch = 1; > >> >> return; > >> >> } > >> >> - ptr32[i] = cpu_to_le32(val); > >> >> + ptr32[i] = val; > >> > > >> > Here val should be in little endian (cpu_to_le32) as it will be > >> referenced by byte array to print serial number. > >> > > >> >> offset += sizeof(u32); > >> >> } > >> >> > >> >> -- > >> >> 1.7.4.4 > >> >> > >> > > >> > > >> > Sorry for Late reply. > >> > > >> > Rajesh > >> > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-11 9:16 ` santosh prasad nayak 2012-03-12 6:19 ` Rajesh Borundia @ 2012-03-12 9:37 ` David Laight 2012-03-12 9:47 ` santosh prasad nayak 1 sibling, 1 reply; 13+ messages in thread From: David Laight @ 2012-03-12 9:37 UTC (permalink / raw) To: santosh prasad nayak, Rajesh Borundia Cc: Sony Chacko, netdev, linux-kernel, kernel-janitors > -----Original Message----- > From: netdev-owner@vger.kernel.org > [mailto:netdev-owner@vger.kernel.org] On Behalf Of santosh > prasad nayak > Sent: 11 March 2012 09:17 > To: Rajesh Borundia > Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@vger.kernel.org > Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. > > Thanks Rajesh for clarification. > Included all your inputs in the following patch. > This is for review not a formal one. Once review is done I will send a > formal patch. I'm not sure of the exact nature of the issues here, but whenever I see code that casts between the addresses of integer types large bells start ringing - such code tends to have unwanted dependencies against the sizes and endiannesses of the relevant fields. This code might be ok, but lines like: > + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) { rather give me the willies. David ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-12 9:37 ` David Laight @ 2012-03-12 9:47 ` santosh prasad nayak 0 siblings, 0 replies; 13+ messages in thread From: santosh prasad nayak @ 2012-03-12 9:47 UTC (permalink / raw) To: David Laight Cc: Rajesh Borundia, Sony Chacko, netdev, linux-kernel, kernel-janitors here "mac" is in "u64" and I have casted it to "__le64". Because its required there. If you have any better suggestion, please let me know. regards santosh On Mon, Mar 12, 2012 at 3:07 PM, David Laight <David.Laight@aculab.com> wrote: > > >> -----Original Message----- >> From: netdev-owner@vger.kernel.org >> [mailto:netdev-owner@vger.kernel.org] On Behalf Of santosh >> prasad nayak >> Sent: 11 March 2012 09:17 >> To: Rajesh Borundia >> Cc: Sony Chacko; netdev; linux-kernel; kernel-janitors@vger.kernel.org >> Subject: Re: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. >> >> Thanks Rajesh for clarification. >> Included all your inputs in the following patch. >> This is for review not a formal one. Once review is done I will send a >> formal patch. > > I'm not sure of the exact nature of the issues here, > but whenever I see code that casts between the addresses > of integer types large bells start ringing - such code > tends to have unwanted dependencies against the sizes > and endiannesses of the relevant fields. > This code might be ok, but lines like: >> + if (*(__le64 *)mac == cpu_to_le64(~0ULL)) { > rather give me the willies. > > David > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. @ 2012-03-02 15:11 santosh nayak 2012-03-02 17:22 ` Rajesh Borundia 0 siblings, 1 reply; 13+ messages in thread From: santosh nayak @ 2012-03-02 15:11 UTC (permalink / raw) To: sony.chacko Cc: rajesh.borundia, netdev, linux-kernel, kernel-janitors, Santosh Nayak From: Santosh Nayak <santoshprasadnayak@gmail.com> Fix endian bug. Add a default case in 'netxen_list_config_vlan_ip' Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> --- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index 8dc4a134..971b286 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter *adapter) adapter->driver_mismatch = 1; return; } - ptr32[i] = cpu_to_le32(val); + ptr32[i] = val; offset += sizeof(u32); } @@ -3028,7 +3028,7 @@ netxen_list_config_vlan_ip(struct netxen_adapter *adapter, list_for_each(head, &adapter->vlan_ip_list) { cur = list_entry(head, struct nx_vlan_ip_list, list); - if (cur->ip_addr == ifa->ifa_address) + if (cur->ip_addr == be32_to_cpu(ifa->ifa_address)) return; } @@ -3039,18 +3039,22 @@ netxen_list_config_vlan_ip(struct netxen_adapter *adapter, return; } - cur->ip_addr = ifa->ifa_address; + cur->ip_addr = be32_to_cpu(ifa->ifa_address); list_add_tail(&cur->list, &adapter->vlan_ip_list); break; case NX_IP_DOWN: list_for_each_entry_safe(cur, tmp_cur, &adapter->vlan_ip_list, list) { - if (cur->ip_addr == ifa->ifa_address) { + if (cur->ip_addr == be32_to_cpu(ifa->ifa_address)) { list_del(&cur->list); kfree(cur); break; } } + break; + default: + printk(KERN_ERR "%ld: Wrong event id \n", event); + break; } } static void @@ -3070,12 +3074,12 @@ netxen_config_indev_addr(struct netxen_adapter *adapter, switch (event) { case NETDEV_UP: netxen_config_ipaddr(adapter, - ifa->ifa_address, NX_IP_UP); + be32_to_cpu(ifa->ifa_address), NX_IP_UP); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP); break; case NETDEV_DOWN: netxen_config_ipaddr(adapter, - ifa->ifa_address, NX_IP_DOWN); + be32_to_cpu(ifa->ifa_address), NX_IP_DOWN); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN); break; default: @@ -3167,11 +3171,11 @@ recheck: switch (event) { case NETDEV_UP: - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP); + netxen_config_ipaddr(adapter, be32_to_cpu(ifa->ifa_address), NX_IP_UP); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP); break; case NETDEV_DOWN: - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN); + netxen_config_ipaddr(adapter, be32_to_cpu(ifa->ifa_address), NX_IP_DOWN); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN); break; default: -- 1.7.4.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. 2012-03-02 15:11 santosh nayak @ 2012-03-02 17:22 ` Rajesh Borundia 0 siblings, 0 replies; 13+ messages in thread From: Rajesh Borundia @ 2012-03-02 17:22 UTC (permalink / raw) To: santosh nayak, Sony Chacko; +Cc: netdev, linux-kernel, kernel-janitors Santosh, Thanks for pointing out the bug. But adapter takes ip in big endian format. So no need to use be32_to_cpu instead data type of ip should be changed to __be32. Also in netxen_config_ipaddr() - req.words[1] = cpu_to_le64(ip); + req.words[1] = ip; Rest looks fine. Rajesh ________________________________________ From: santosh nayak [santoshprasadnayak@gmail.com] Sent: Friday, March 02, 2012 8:41 PM To: Sony Chacko Cc: Rajesh Borundia; netdev; linux-kernel; kernel-janitors@vger.kernel.org; Santosh Nayak Subject: [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug. From: Santosh Nayak <santoshprasadnayak@gmail.com> Fix endian bug. Add a default case in 'netxen_list_config_vlan_ip' Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com> --- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c index 8dc4a134..971b286 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c @@ -818,7 +818,7 @@ netxen_check_options(struct netxen_adapter *adapter) adapter->driver_mismatch = 1; return; } - ptr32[i] = cpu_to_le32(val); + ptr32[i] = val; offset += sizeof(u32); } @@ -3028,7 +3028,7 @@ netxen_list_config_vlan_ip(struct netxen_adapter *adapter, list_for_each(head, &adapter->vlan_ip_list) { cur = list_entry(head, struct nx_vlan_ip_list, list); - if (cur->ip_addr == ifa->ifa_address) + if (cur->ip_addr == be32_to_cpu(ifa->ifa_address)) return; } @@ -3039,18 +3039,22 @@ netxen_list_config_vlan_ip(struct netxen_adapter *adapter, return; } - cur->ip_addr = ifa->ifa_address; + cur->ip_addr = be32_to_cpu(ifa->ifa_address); list_add_tail(&cur->list, &adapter->vlan_ip_list); break; case NX_IP_DOWN: list_for_each_entry_safe(cur, tmp_cur, &adapter->vlan_ip_list, list) { - if (cur->ip_addr == ifa->ifa_address) { + if (cur->ip_addr == be32_to_cpu(ifa->ifa_address)) { list_del(&cur->list); kfree(cur); break; } } + break; + default: + printk(KERN_ERR "%ld: Wrong event id \n", event); + break; } } static void @@ -3070,12 +3074,12 @@ netxen_config_indev_addr(struct netxen_adapter *adapter, switch (event) { case NETDEV_UP: netxen_config_ipaddr(adapter, - ifa->ifa_address, NX_IP_UP); + be32_to_cpu(ifa->ifa_address), NX_IP_UP); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP); break; case NETDEV_DOWN: netxen_config_ipaddr(adapter, - ifa->ifa_address, NX_IP_DOWN); + be32_to_cpu(ifa->ifa_address), NX_IP_DOWN); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN); break; default: @@ -3167,11 +3171,11 @@ recheck: switch (event) { case NETDEV_UP: - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP); + netxen_config_ipaddr(adapter, be32_to_cpu(ifa->ifa_address), NX_IP_UP); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP); break; case NETDEV_DOWN: - netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN); + netxen_config_ipaddr(adapter, be32_to_cpu(ifa->ifa_address), NX_IP_DOWN); netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN); break; default: -- 1.7.4.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-03-12 9:47 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-03-03 15:47 [PATCH 3/3] netxen: qlogic ethernet : Fix Endian Bug santosh nayak 2012-03-05 11:43 ` Rajesh Borundia 2012-03-05 20:35 ` David Miller 2012-03-07 19:41 ` Rajesh Borundia 2012-03-09 16:34 ` Rajesh Borundia 2012-03-09 18:50 ` santosh prasad nayak 2012-03-10 19:01 ` Rajesh Borundia 2012-03-11 9:16 ` santosh prasad nayak 2012-03-12 6:19 ` Rajesh Borundia 2012-03-12 9:37 ` David Laight 2012-03-12 9:47 ` santosh prasad nayak -- strict thread matches above, loose matches on Subject: below -- 2012-03-02 15:11 santosh nayak 2012-03-02 17:22 ` Rajesh Borundia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®