Hi Fab,

On the last year the work with openfabrics svn takes a very long time, so I 
guess that we should either fix that or find another place to hold the joined 
code.

As for the current series of patches, almost all of them have been extremely 
small, (probably less than 5 lines). Many of them are only removing an assert 
or changing a print.
The only one of them that is bigger is the memory allocation patch that we did 
not check in. We are still thinking of the best way to continue and are facing 
a few different requirements:

1)      We have already found a bug in this area.

2)      We can fix the immediate bug, but we want to be sure that we fix all of 
the bugs in this area.

3)      The simple solution that we find is not accepted by the community.

4)      The only advice that we hear from the community is that we should find 
all bugs. (how comes we didn't think of this ourselves?)

As for checking in big chunk of codes all together. I must say that many times 
we have a demand from business not to expose products too early.
What is your suggestion for us when we have to develop a ULP for some big 
company but the project is under NDA?

And last comment, if any one fills that one of the checkins done lately is 
wrong, please let us know.

Thanks
Tzachi

From: Fab Tillier [mailto:[email protected]]
Sent: Thursday, September 02, 2010 7:21 PM
To: Tzachi Dar; Leonid Keller; [email protected]
Subject: RE: patch: [IBAL] remove unnecessary ASSERT

Ugh, seriously?  Thanks for giving time to discuss this.

The switch statement handles all possible pd_types.  Are you saying that your 
unit tests passes in arbitrary values for PD types?

It seems like Mellanox is on some quest to just sync the OFED source with 
whatever you have in-house, regardless of the feedback.  While that's a nice 
idea, you shouldn't just ram whatever your changes are down the OFED 
community's throat.  A better model would be to do all your work in the OFED 
tree, for everyone to see and comment on while you do it.

This has come up over and over again over many years: you do a bunch of coding 
behind closed doors, and then dump everything into the OFED source tree - too 
much code to be reviewed, too much time invested on your part to bother 
addressing the community's comments when there are some.  Please start treating 
the community as partners, not subordinates.

-Fab

From: Tzachi Dar [mailto:[email protected]]
Sent: Thursday, September 02, 2010 12:23 AM
To: Leonid Keller; Fab Tillier; [email protected]
Subject: RE: patch: [IBAL] remove unnecessary ASSERT

Applied on 2887.

Thanks
Tzachi

From: Leonid Keller
Sent: Wednesday, September 01, 2010 12:54 PM
To: Fab Tillier; Tzachi Dar; [email protected]
Subject: RE: patch: [IBAL] remove unnecessary ASSERT

We hit it in our test program, which - in error flow - creates PD with all 
possible pd_types.
The test expects to get IB_INVALID_PARAMETER, which it gets.
We believe, that the error status that is returned in this case is enough to 
spot an error.

From: [email protected] 
[mailto:[email protected]] On Behalf Of Fab Tillier
Sent: Tuesday, August 31, 2010 7:56 PM
To: Tzachi Dar; [email protected]
Subject: Re: [ofw] patch: [IBAL] remove unnecessary ASSERT

The assert here is to trap a bad call to alloc_pd.  Why is it being removed?  
Did you have code that hit it?  Did you fix that code so it wouldn't hit the 
assertion?  Or was the assertion incorrect?  Basically, the assert has been in 
the code for years now - what suddenly makes it unnecessary?

-Fab

From: [email protected] 
[mailto:[email protected]] On Behalf Of Tzachi Dar
Sent: Tuesday, August 31, 2010 8:29 AM
To: [email protected]
Subject: [ofw] patch: [IBAL] remove unnecessary ASSERT

Signed off by: Leonid

Mlnx 5599

Index: B:/users/tzachid/projinf9/trunk/core/al/al_pd.c
===================================================================
--- B:/users/tzachid/projinf9/trunk/core/al/al_pd.c         (revision 5598)
+++ B:/users/tzachid/projinf9/trunk/core/al/al_pd.c      (revision 5599)
@@ -137,7 +137,6 @@
                                break;

                default:
-                              CL_ASSERT( h_pd->type == IB_PDT_ALIAS || 
h_pd->type == IB_PDT_NORMAL );
                                status = IB_INVALID_PARAMETER;
                }

_______________________________________________
ofw mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw

Reply via email to