From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Wang Subject: Re: [PATCH v4 14/19] IB/core: Add IB_DEVICE_OPA_MAD_SUPPORT device cap flag Date: Wed, 25 Mar 2015 11:30:32 +0100 Message-ID: <55128E48.1080406@profitbricks.com> References: <550C2514.5070001@profitbricks.com> <6A3D3202-0128-4F33-B596-D7A76AB66DF8@gmail.com> <20150320235748.GA22703@phlsvsds.ph.intel.com> <20150321000541.GA24717@phlsvsds.ph.intel.com> <1828884A29C6694DAF28B7E6B8A8237399E82E58@ORSMSX109.amr.corp.intel.com> <55115D61.9040201@profitbricks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55115D61.9040201-EIkl63zCoXaH+58JC4qpiA@public.gmane.org> Sender: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Hefty, Sean" , "Weiny, Ira" Cc: Doug Ledford , "roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Hal Rosenstock List-Id: linux-rdma@vger.kernel.org Hi, Sean On 03/24/2015 01:49 PM, Michael Wang wrote: > On 03/23/2015 05:31 PM, Hefty, Sean wrote: >>> [snip] >> To restate my suggesting, I was thinking of defining something like >> this: >> >> enum { >> IB_MGMT_PROTO_SM = (1 << 0), /* supports IB SMPs */ >> IB_MGMT_PROTO_SA = (1 << 1), /* supports IB SA MADs */ >> IB_MGMT_PROTO_GS = (1 << 2), /* supports IB GSI MADs (e.g. PM, >> ...) */ >> IB_MGMT_PROTO_CM = (1 << 3), /* IB CM called out separately */ >> IB_MGMT_PROTO_IW_CM = (1 << 4),/* iWarp CM */ >> /* OPA can define new values here */ >> }; >> >> struct ib_port_attr { >> ... >> u32 mgmt_proto; /* bitmask of supported protocols */ >> }; > > Thanks for the restate, Sean :) seems like your proposal is also to > ask vendor > setup 'mgmt_proto' during ib_query_port(), correct? I think we got one problem here, if we rely on ib_query_port() to setup mgmt flags each time, the performance may suffered, since some implementation of query_port() is really expensive, like hardware communication (mlx4/5) and mutex protection (usnic)... And also I found that the current implementation doesn't match the idea very well, for example CM, I haven't found any scene to check whether a specified port support CM or not (correct me please), mostly only check the device rather than it's port, SM is checking the port, however since we already verified transport layer at beginning, just check link layer sounds not that bad... Thus I think using flags may not very helpful to the current implementation, may be use some helper to refine the code is more applicable? I'll send out the patch later with just helpers, we can discuss in that thread see if there is any better solutions ;-) Regards, Michael Wang > >> >> I am not familiar enough with RoCE (IBoE) to know off the top of my >> head if this breakdown works as I defined it, or if IB_MGMT_PROTO_GS >> needs to be separated into more mgmt classes. (Hal or Ira might.) I >> separated out the CM class, as the rdma cm has checks where it wants >> to distinguish between which CM protocol to execute (IB or iWarp). > > Maybe we can apply this thought to CM part firstly? >> >> This change would be limited to management checks only. There may >> still be places in the code where the link and transport checks would >> continue to exist. Again, this is just a suggestion. Without >> actually implementing the patch, I don't know if this would simplify >> things. The checks in the rdma cm, in particular, are messy. > I think it's time to make a formal patch now and discuss the problem in > a separate thread, I'll still use the mechanism in draft and apply these > flags, let's see if it satisfied peoples ;-) > > Regards, > Michael Wang > >> - Sean > -- 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