Hi Moni, Please find my response inline:
> -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf > Of Moni Shoua > Sent: Tuesday, December 30, 2014 9:10 PM > To: Somnath Kotur > Cc: [email protected]; linux-rdma; Devesh Sharma > Subject: Re: [PATCH 1/6] IB/Core: Changes to the IB Core infrastructure for > RoCEv2 support > > > Although you follow the spec here, 3 types of RDMA_NETWORK are not > > really required. Maybe we can get rid of this Maybe we can get rid of > > this duplication > > > > [SOM]: Not sure i understood the duplication here or why it's not required? > > We now have a new 'network/L3' layer on top of L2 - that was the reason, > does it not make sense? > > > Once you know pair (type of RoCE and GID value) you know what is the > network type And once you know network type you know the pair. > So, it is not really necessary to keep them all stored. > My idea is to get rid if the type that describes network type > > > [SOM]: Well, partly the use of get_port_type() was motivated by the > > SPEC(Query HCA - 17.5.x.x IIRC) talking about the need to have a > > port_type attribute as part of RoCEV2 that would indicate if a HW > device/port supported RoCEV2 or not. It was also serving another purpose in > cma_acqure_dev() as you can see above in the patch where it was helping > the use case of devices that only support V2 and not V1. > > Still feel it doesn't make sense? > > Not sure how/where did you want point 2 coming from - > sysfs/proc/debugfs? > > I'd prefer to have that in the next stage of the patchset > > Spec is confusing. Under the section QUERY HCA it describes changes to the > port attr. > Anyway, I see that spec points to the ability of querying capabilities and > comparing them against decisions but not as a method to take decisions. > What I would do is to add 2 flags to ib_port_cap_flags, IB_PORT_ROCE_SUP > and IB_PORT_ROCE_V2_SUP [DS]: In the ib_port_cap_flags such flags is added, however it needs a name change as per your Suggestion. @@ -265,7 +329,8 @@ enum ib_port_cap_flags { IB_PORT_BOOT_MGMT_SUP = 1 << 23, IB_PORT_LINK_LATENCY_SUP = 1 << 24, IB_PORT_CLIENT_REG_SUP = 1 << 25, - IB_PORT_IP_BASED_GIDS = 1 << 26 + IB_PORT_IP_BASED_GIDS = 1 << 26, + IB_PORT_RoCEV2_BASED_GIDS = 1 << 27 }; On the other hand; The motive to have rdma_get_port_type() is to query port type in cma_acquire_dev() even if port_num = 0. Ib_query_port would fail to report the ib_port_cap_flags if application have not specified the device port number explicitly. The failure is due to port number range check. However, I think it's also okay to call ib_query_port() and skip the status check for this call. Makes sense? . > > I think that the main difference between approaches is how to decide about > the RoCE type to use for a session. > This is also why I think that we should not postpone the change in > cma_acquire_dev to later. > > thanks > Moni
