linux-kernel - RE: InfiniBand/RDMA merge plans for 2.6.25

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5E701717F2B2ED4EA60F87C8AA57B7CC0794FEB4@venom2>
Date:	Thu, 24 Jan 2008 07:54:36 -0600
From:	"Glenn Streiff" <gstreiff@...Effect.com>
To:	"Roland Dreier" <rdreier@...co.com>,
	"Christoph Hellwig" <hch@...radead.org>
Cc:	<linux-kernel@...r.kernel.org>, <general@...ts.openfabrics.org>
Subject: RE: InfiniBand/RDMA merge plans for 2.6.25


> From: Roland Dreier [mailto:rdreier@...co.com]
> To: Christoph Hellwig; Glenn Streiff 
> 
> Rather than arguing over whether we have to have sparse clean code, I
> decided to annotate the code myself.  Here's a patch that fixes most
> of the sparse warnings in the nes driver.  There's still some stuff
> that actually looks buggy, like the way hte_index stuff is handled.
> You initialize hte_index_mask as:
> 
> 	hte_index_mask = ((u32)1 << ((u32temp & 0x001f)+1))-1;
> 	nesadapter->hte_index_mask = hte_index_mask;
> 
> but then compute hte_index stuff with:
> 
> 	nesqp->hte_index = cpu_to_be32(
> 			crc32c(~0, (void *)&nes_quad, 
> sizeof(nes_quad)) ^ 0xffffffff);
> 
> and then do:
> 
> 	nesqp->hte_index &= nesadapter->hte_index_mask;
> 
> which seems odd to say the least (hte_index is big-endian,
> hte_index_mask is cpu-endian).
> 
> And also, there's code with the loc_addr/rem_addr etc that seem very
> confused.  For example
> 
> 	cm_info->loc_addr = htonl(cm_info->loc_addr);
> 	cm_info->rem_addr = htonl(cm_info->rem_addr);
> 	cm_info->loc_port = htons(cm_info->loc_port);
> 	cm_info->rem_port = htons(cm_info->rem_port);
> 
> which is obviously impossible to annotate correctly, and I couldn't
> keep track of the endianness stuff elsewhere.

Thanks for the additional review and patch.  I take your point.  

The part is little endian and the driver is functional for little and big endian 
platforms.  There may have been some expedience with the declarations there.  
I think it can be improved.  Let me take it up with the person who wrote that code.

Also, I want everyone to understand that my skill set is weighted more
towards build/install/config.  And I guess I'll be patch wrangling as well.
So I'll rely on input from my developers for issues that drill down or
I'll have them post directly.  I respect the work you guys do.

For now, let me get some qa cycles with your patch across x86_64 and power
(and probably a couple others).

Regards,

Glenn

> 
> Anyway this is what I have in case the promised cleanups don't turn up
> in time...
> 
> Signed-off-by: Roland Dreier <rolandd@...co.com>
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/