[ofw] patch: [IBAL] remove unnecessary ASSERT

[Tzachi Dar]

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:ftillier at microsoft.com]
Sent: Thursday, September 02, 2010 7:21 PM
To: Tzachi Dar; Leonid Keller; ofw at lists.openfabrics.org
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:tzachid at mellanox.co.il]
Sent: Thursday, September 02, 2010 12:23 AM
To: Leonid Keller; Fab Tillier; ofw at lists.openfabrics.org
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; ofw at lists.openfabrics.org
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: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Fab Tillier
Sent: Tuesday, August 31, 2010 7:56 PM
To: Tzachi Dar; ofw at lists.openfabrics.org
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: ofw-bounces at lists.openfabrics.org [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
Sent: Tuesday, August 31, 2010 8:29 AM
To: ofw at lists.openfabrics.org
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;
                }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/ofw/attachments/20100902/3a962dcf/attachment.html>