The 2.6.25 will open soon, so it's time to review what my plans are

for the merge window opens.
A note on reviewing: the upstream push for "Reviewed-by:" tags seems

to have lost some momentum, but it is still the case that if you can

get someone other than me to review your work, then the chances of it

being merged increase dramatically.  I'm talking about a real review--

ideally, someone independent (a different domain after the '@' would

be good ;) who is willing to provide a "Reviewed-by:" line that means

the reviewer has really looked at and thought about the patch.  There

should be a mailing list thread you can point me at where the reviewer

comments on the patch and a new version of that patch addressing all

comments is posted (or in exceptional cases, where the patch is

perfect to start with, where the reviewer says the patch is great).
Anyway, here are all the pending things that I'm aware of.  As usual,

if something isn't already in my tree and isn't listed below, I

probably missed it or dropped it by mistake.  Please remind me again

in that case.
Core:
 - Sean's inter-subnet CM changes.  My first reaction is that I'll

   probably merge it, but I need to find the time to really read it

   over first.
ULPs:
 - Rolf's IPoIB MGID scope changes.  I have the core changes queued

   up, so at least recompiling the whole kernel should no longer be

   necessary.  I need to look over the rest of the patches more

   carefully and make sure we have the correct userspace interface at

   least.
 - Eli's IPoIB stateless offload (checksum offload, LSO, interrupt

   moderation, etc).  It's a big series that makes quite a few core

   changes.  I hope to merge at least part of this queue soon.

   Outside opinions are welcome though.
 - Remove LLTX from IPoIB.  I haven't posted a draft of this patch

   yet, so I guess it will probably wait for 2.6.26...
 - I still have a few SRP changes from David Dillow to review and

   apply.  Nothing looked to radical so they should be ...
[ message continues ]

Are there any plans to merge the SDP (Sockets Direct Protocol) implementation ?
Bart.

--

> > Anyway, here are all the pending things that I'm aware of.  As usual,

 > > if something isn't already in my tree and isn't listed below, I

 > > probably missed it or dropped it by mistake.  Please remind me again

 > > in that case.

 >

 > Are there any plans to merge the SDP (Sockets Direct Protocol) implementation ?
No.  To my knowledge, it has never been proposed for merging or been

posted for review.

--

New code should be better quality than old code, not worse.   I haven't

actually seen the driver yet, but by that statement I'd be clearly

against a merge.
--

> >  - Neteffect "nes" driver.  It's not terribly clean code but since

 > >    it's a new driver that is completely self-contained, I plan on

 > >    merging it and letting cleanups happen upstream.

 >

 > New code should be better quality than old code, not worse.   I haven't

 > actually seen the driver yet, but by that statement I'd be clearly

 > against a merge.
The driver has been posted a few times; the latest code is in the

"neteffect" branch of my tree:
    git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git neteffect
It's not *that* bad -- certainly there are lots of things that could

be improved (sparse endianness annotation, too many lines that are way

to long, strange indentation of case labeles, etc, etc) but it is a

self-contained hardware driver.  I agree with Linus's position (stated

at the last kernel summit) that we ought to merge hardware drivers

early, so that users get the drivers with as little hassle as

possible.  We lose a little leverage in getting cleanups done, but the

number of people who see the code and are able to clean it up

increases, so I think it's a good trade-off.
 - R.

--

that's a blocker for sure.  No new code that's not sparse clean, please.
--

> > be improved (sparse endianness annotation,
 > that's a blocker for sure.  No new code that's not sparse clean, please.
I have to disagree -- remember how strongly Linus pushed to merge

hardware drivers early?  the code in question will not run unless you

have the hardware it drives, and that hardware is useless without the

code.  And *something* is better for users than nothing.
Anyway I don't think the endianness annotations are that hard so it

will probably show up soon.
 - R.

--

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.
Anyway this is what I have in case the promised cleanups don't turn up

in time...
Signed-off-by: Roland Dreier <rolandd@cisco.com>
diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c

index 7a2f596..365ebaa 100644

--- a/drivers/infiniband/hw/nes/nes.c

+++ b/drivers/infiniband/hw/nes/nes.c

@@ -231,10 +231,10 @@ static int nes_net_event(struct notifier_block *notifier,

 					} else {

 						if (neigh->nud_state & NUD_VALID) {

 							nes_manage_arp_cache(neigh->dev, neigh->ha,

-									ntohl(*(u32 *)neigh->primary_key), NES_ARP_ADD);

+									ntohl(*(__be32 *)neigh->primary_key), NES_ARP_ADD);

 						} else {

 							nes_manage_arp_cache(neigh->dev, neigh->ha,

-									ntohl(*(u32 *)neigh->primary_key), NES_ARP_DELETE);

+									ntohl(*(__be32 *)neigh->primary_k...
[ message continues ]

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,
--

My view is the code should and will be cleaned up based upon

the feedback we've gotten from the community.  It is a priority

for me.
Several cleanup fixes are in the queue and are being worked.

Haven't slipped into complacency at the prospect of the merge.
Glenn

gstreiff@neteffect.com

--

It wasn't clear to me that this issue was ever really nailed. Was the

loop on this closed ?
-- Hal
--

The error that this patches addresses is fairly obvious if you inspect the code.

There's a strong chance that the patch fixes the bug that was reported, but the

last I remember, they had trouble reproducing the crash to see if the patch

would indeed make it go away.
- Sean
--

The bug seems obvious but I'm not sure about the fix. Just my $0.02

worth.
--