Re: [PATCH for-next V5 00/12] Move RoCE GID management to IB/Core

Jason Gunthorpe
2015-06-11T05:38:23Z

On Thu, Jun 11, 2015 at 12:49:59AM -0400, Doug Ledford wrote:

> fact that the mlx4 driver and the ocrdma driver had their own gid
> management code, there were some distinct differences between the two.
> The gid at index 0 never matched up in my testing for example. One
> supported bonding, the other didn't. Even if you tried to limit things
> to a cleanup, you would still end up altering behavior of both drivers
> just because the cleanup would have to merge the two implementations and
> the result would be different than either one of them.

This is very interesting detail, I would be very happy to see the user
visible functional changes to ocrdma listed in it's commit message.

I always understood that ocrdma would have some change, at least from
bonding, my 'no functional change' litmus test was that mlx4 operation
doesn't have a change. My expectation is the current v5 achieves that?

> I get the impression that a lot of the extra is scalability changes for
> perceived need (probably due to upcoming namespace/container additions).
> But, just to be fair, the entire mlx4 gid code was -500 lines and
> included bonding support. The base gid code addition was +500 for the
> gid table, +600 for the table population functions, +170 for default
> gids, +290 for bonding support, and +140 lines to hook it into the
> existing cache routines. While part of this might be would seem to be
> the over-designed locking, part of it is probably exactly as Matan said,
> code growth due to generalization and standardization.

Line count is such a funny metric.. It is very, very hard to write
succinct code, and I have no magic wand to shrink this particular
patch set down further. I suspect it could be made smaller, but that
would require more study than I care to employ :)

I feel line count is a hopeless avenue to argue, but also a red flag.

Jason