| Message ID | SN1PR0301MB2127376FC9B3D170C878A47BDE8D0@SN1PR0301MB2127.namprd03.prod.outlook.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Aug 15, 2017 at 06:25:54PM +0000, Suri Shelvapille wrote: > Leon: > > We do have IB_SWITCH and IB_ROUTER functionality and we do use the SWITCH capability provided by the "core" in our product. The "core" has a wrapper function which Identifies the device as a switch to direct smp packets. I am glad that has not been removed. And I hope it won't be removed as it would be a burden to us. Although we prefer to have the IB_SWITCH/IB_ROUTER definitions in the core as well, it is not a big deal to move these definitions inside our driver. > Can you please point me to IN-KERNEL hardware/driver where these values are being set? Because I didn't find any. Thanks
Leon: Our Drivers are not in Kernel (being a small company we don't have resources to make it in-kernel). When I and Hal worked on adding the switch capability into the kernel, we thought it would be useful for the community (as there was interest in a few others besides us). Philosophically, since SWITCH and ROUTER definitions are part of the IB spec, don't you think having these definitions makes the kernel close to spec and hence useful? I know these definitions can be easily added into our drivers. Just treat this as a request, if it is not an onus to the community, please keep the SWITCH and ROUTER (only) definitions. If you have strong reasons I have no objections to your patches. Thanks, Suri -----Original Message----- From: Leon Romanovsky [mailto:leon@kernel.org] Sent: Wednesday, August 16, 2017 1:38 AM To: Suri Shelvapille <suri@baymicrosystems.com> Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org Subject: Re: [rdma-next v2 07/23] RDMA/core: Remove unimplemented node_types and node transport On Tue, Aug 15, 2017 at 06:25:54PM +0000, Suri Shelvapille wrote: > Leon: > > We do have IB_SWITCH and IB_ROUTER functionality and we do use the SWITCH capability provided by the "core" in our product. The "core" has a wrapper function which Identifies the device as a switch to direct smp packets. I am glad that has not been removed. And I hope it won't be removed as it would be a burden to us. Although we prefer to have the IB_SWITCH/IB_ROUTER definitions in the core as well, it is not a big deal to move these definitions inside our driver. > Can you please point me to IN-KERNEL hardware/driver where these values are being set? Because I didn't find any. Thanks This correspondence, and any attachments or files transmitted with this correspondence, contains information which may be confidential and privileged and is intended solely for the use of the addressee. Unless you are the addressee or are authorized to receive messages for the addressee, you may not use, copy, disseminate, or disclose this correspondence or any information contained in this correspondence to any third party. If you have received this correspondence in error, please notify the sender immediately and delete this correspondence and any attachments or files transmitted with this correspondence from your system, and destroy any and all copies thereof, electronic or otherwise. Your cooperation and understanding are greatly appreciated. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Leon, We probably should be okay to retain SWITCH and ROUTER devices, lets say if someone wants to port entire linux kernel IB stack on a IB-switch or Router. Probably portability was the motivation to add these since the birth of kernel IB stack. On Wed, Aug 16, 2017 at 8:00 PM, Suri Shelvapille <suri@baymicrosystems.com> wrote: > Leon: > Our Drivers are not in Kernel (being a small company we don't have resources to make it in-kernel). When I and Hal worked on adding the switch capability into the kernel, we thought it would be useful for the community (as there was interest in a few others besides us). Philosophically, since SWITCH and ROUTER definitions are part of the IB spec, don't you think having these definitions makes the kernel close to spec and hence useful? I know these definitions can be easily added into our drivers. > Just treat this as a request, if it is not an onus to the community, please keep the SWITCH and ROUTER (only) definitions. If you have strong reasons I have no objections to your patches. > > Thanks, > Suri > > -----Original Message----- > From: Leon Romanovsky [mailto:leon@kernel.org] > Sent: Wednesday, August 16, 2017 1:38 AM > To: Suri Shelvapille <suri@baymicrosystems.com> > Cc: Doug Ledford <dledford@redhat.com>; linux-rdma@vger.kernel.org > Subject: Re: [rdma-next v2 07/23] RDMA/core: Remove unimplemented node_types and node transport > > On Tue, Aug 15, 2017 at 06:25:54PM +0000, Suri Shelvapille wrote: >> Leon: >> >> We do have IB_SWITCH and IB_ROUTER functionality and we do use the SWITCH capability provided by the "core" in our product. The "core" has a wrapper function which Identifies the device as a switch to direct smp packets. I am glad that has not been removed. And I hope it won't be removed as it would be a burden to us. Although we prefer to have the IB_SWITCH/IB_ROUTER definitions in the core as well, it is not a big deal to move these definitions inside our driver. >> > > Can you please point me to IN-KERNEL hardware/driver where these values are being set? Because I didn't find any. > > Thanks > > This correspondence, and any attachments or files transmitted with this correspondence, contains information which may be confidential and privileged and is intended solely for the use of the addressee. Unless you are the addressee or are authorized to receive messages for the addressee, you may not use, copy, disseminate, or disclose this correspondence or any information contained in this correspondence to any third party. If you have received this correspondence in error, please notify the sender immediately and delete this correspondence and any attachments or files transmitted with this correspondence from your system, and destroy any and all copies thereof, electronic or otherwise. Your cooperation and understanding are greatly appreciated. > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 16, 2017 at 02:30:05PM +0000, Suri Shelvapille wrote: > Leon: > Our Drivers are not in Kernel (being a small company we don't have resources to make it in-kernel). When I and Hal worked on adding the switch capability into the kernel, we thought it would be useful for the community (as there was interest in a few others besides us). Philosophically, since SWITCH and ROUTER definitions are part of the IB spec, don't you think having these definitions makes the kernel close to spec and hence useful? I know these definitions can be easily added into our drivers. > Just treat this as a request, if it is not an onus to the community, please keep the SWITCH and ROUTER (only) definitions. If you have strong reasons I have no objections to your patches. It looks like I need to explain the rationale why I did this patch. RDMAtool presents various information from the kernel, one of such info is the node_type. In order to correctly present it, I was asked to expose possible node_types through UAPI files. These files are done with extra care and my goal was to provide the minimal set and the most cleanest exposure, so I cleaned everything in those paths, from the lowest possible layer to highest possible layer. One of such cleanups were removal of node_types fields, which don't have in-kernel users and against software development principles - don't leave dead code. So for now, I'm leaving this patch as is. It is far below my lowest quality bar to leave those fields in place, but if members of RDMA community prefer to go such low, they should speak and explain publicly why RDMA subsystem is different from the rest of the kernel. Thanks
On Wed, Aug 16, 2017 at 09:33:18PM +0530, Devesh Sharma wrote: > Hi Leon, > > We probably should be okay to retain SWITCH and ROUTER devices, lets say if > someone wants to port entire linux kernel IB stack on a IB-switch or > Router. Probably portability was the motivation to add these since the > birth of kernel IB stack. > It was in the happy days of coding of the IBTA spec, in 2017, the reality is different. I don't want to support people who doesn't contribute their code back. Thanks
On Wed, Aug 16, 2017 at 07:21:03PM +0300, Leon Romanovsky wrote: > On Wed, Aug 16, 2017 at 02:30:05PM +0000, Suri Shelvapille wrote: > > Leon: > > Our Drivers are not in Kernel (being a small company we don't have resources to make it in-kernel). When I and Hal worked on adding the switch capability into the kernel, we thought it would be useful for the community (as there was interest in a few others besides us). Philosophically, since SWITCH and ROUTER definitions are part of the IB spec, don't you think having these definitions makes the kernel close to spec and hence useful? I know these definitions can be easily added into our drivers. > > Just treat this as a request, if it is not an onus to the community, please keep the SWITCH and ROUTER (only) definitions. If you have strong reasons I have no objections to your patches. > > It looks like I need to explain the rationale why I did this patch. > > RDMAtool presents various information from the kernel, one of such info > is the node_type. In order to correctly present it, I was asked to expose > possible node_types through UAPI files. > > These files are done with extra care and my goal was to provide the minimal > set and the most cleanest exposure, so I cleaned everything in those paths, > from the lowest possible layer to highest possible layer. > > One of such cleanups were removal of node_types fields, which don't have > in-kernel users and against software development principles - don't > leave dead code. > > So for now, I'm leaving this patch as is. > > It is far below my lowest quality bar to leave those fields in place, > but if members of RDMA community prefer to go such low, they should > speak and explain publicly why RDMA subsystem is different from the > rest of the kernel. I found the elegant way how to code the RDMAtool without dependency on those UAPI files. I'll post tomorrow morning new version of the tool, and we will end with exposing of two enums only - rdma_dev_cap and rdma_port_cap. Thanks > > Thanks
diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index abc5ab581f82..c43280f8d5b3 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -1146,10 +1146,7 @@ static ssize_t show_node_type(struct device *device, switch (dev->node_type) { case RDMA_NODE_IB_CA: return sprintf(buf, "%d: CA\n", dev->node_type); case RDMA_NODE_RNIC: return sprintf(buf, "%d: RNIC\n", dev->node_type); -case RDMA_NODE_USNIC: return sprintf(buf, "%d: usNIC\n", dev->node_type); case RDMA_NODE_USNIC_UDP: return sprintf(buf, "%d: usNIC UDP\n", dev->node_type); -case RDMA_NODE_IB_SWITCH: return sprintf(buf, "%d: switch\n", dev->node_type); -case RDMA_NODE_IB_ROUTER: return sprintf(buf, "%d: router\n", dev->node_type); default: return sprintf(buf, "%d: <unknown>\n", dev->node_type); } } diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c index 3d1de62de839..21fef6b6d4f3 100644 --- a/drivers/infiniband/core/verbs.c +++ b/drivers/infiniband/core/verbs.c @@ -182,13 +182,9 @@ rdma_node_get_transport(enum rdma_node_type node_type) { switch (node_type) { case RDMA_NODE_IB_CA: -case RDMA_NODE_IB_SWITCH: -case RDMA_NODE_IB_ROUTER: return RDMA_TRANSPORT_IB; case RDMA_NODE_RNIC: return RDMA_TRANSPORT_IWARP; -case RDMA_NODE_USNIC: -return RDMA_TRANSPORT_USNIC; case RDMA_NODE_USNIC_UDP: return RDMA_TRANSPORT_USNIC_UDP; default: @@ -207,7 +203,6 @@ enum rdma_link_layer rdma_port_get_link_layer(struct ib_device *device, u8 port_ case RDMA_TRANSPORT_IB: return IB_LINK_LAYER_INFINIBAND; case RDMA_TRANSPORT_IWARP: -case RDMA_TRANSPORT_USNIC: case RDMA_TRANSPORT_USNIC_UDP: return IB_LINK_LAYER_ETHERNET; default: diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 4ce188128aa9..6aec3971628b 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -95,12 +95,9 @@ struct ib_gid_attr { enum rdma_node_type { /* IB values map to NodeInfo:NodeType. */ -RDMA_NODE_IB_CA = 1, -RDMA_NODE_IB_SWITCH, -RDMA_NODE_IB_ROUTER, -RDMA_NODE_RNIC, -RDMA_NODE_USNIC, -RDMA_NODE_USNIC_UDP, +RDMA_NODE_IB_CA= 1, +RDMA_NODE_RNIC= 4, +RDMA_NODE_USNIC_UDP= 6, }; enum { @@ -109,10 +106,9 @@ enum { }; enum rdma_transport_type { -RDMA_TRANSPORT_IB, -RDMA_TRANSPORT_IWARP, -RDMA_TRANSPORT_USNIC, -RDMA_TRANSPORT_USNIC_UDP +RDMA_TRANSPORT_IB= 0, +RDMA_TRANSPORT_IWARP= 1, +RDMA_TRANSPORT_USNIC_UDP= 3, }; enum rdma_protocol_type {