Re: [PATCH v4 for-next 07/14] IB/core: GID attribute should be returned from verbs API and cache API

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Thu, May 28, 2015 at 04:50:09PM +0300, Matan Barak wrote:

> The argument for removing the gid_type seems reasonable to me.
> However, I don't think we should be removing net.
> if_index should always come with net - passing only if_index makes
> roce_gid_table's API a bit broken.

Well, get rid of if_index too? Isn't all of that rocev2 stuff? Todays
roce drivers do not need if_index at this point to do gid index lookup.

It isn't clear to me why that should change in a refactoring exercise.

> Patch 8 (the ndev part) is relevant. GID is now related to a ndev and
> we would like
> to expose this information to the user.
> In non rdma-cm applications, how would a user select the gid_index he wants?

I don't mean drop forever, I mean, concentrate on getting this clean
up done, then start discussing UAPI changes separately. Please don't
bury UAPI changes, new features, etc in a cleanup patch series.

> > I'm also not sure about patch 10, that looks like a functional
> > change? It should not be in a cleanup series.
> 
> We drop smac and vlan_id from path and av. Instead, it's taken
> from the netdev in roce_gid_table. That's a crucial part of the new
> architecture.

Again, I didn't say drop it forever, these patches belong in a later
series, perhaps as a precursor series to rocev2.

> It's not worth adding roce_gid_table and not taking the parameters
> based on its information.

It should be worth adding roce_gid_table because it factors duplicate
existing code out of several drivers. Get that merged, then deal with
adding rocev2, adding new UAPIs, etc.

If that isn't true then we have a problem :)

You started with a giagantic series that added new UAPIs, refactored
code, clean ups, added rocev2, and maybe more I haven't noticed. That
is too big to review. Make it smaller:

- Refactor the gid table code from the drivers - as is. Minimize
  changes elsewhere
- Add rocev2 in stages
- New UAPIs for rocev2
- Anything else I missed.

Don't try and do so much at once.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html