Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 10:09 PM, Doug Ledford wrote: [snip] >> >> I had asked for better kdocs for the new helpers so new people can >> understand when and where to use them. >> >> I've not looked at the series at all for the past few postings. > > Michael, please work up an incremental patch to address the kdocs issue. > I've picked up the v8 patchset, and there is no need to respin it, but I > would like to have that kdoc patch before the 4.2 merge window opens. Sure, now these helpers are settled down, it's time for document, I'll send out the RFC ASAP :-) Regards, Michael Wang > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 10:09 PM, Doug Ledford wrote: [snip] I had asked for better kdocs for the new helpers so new people can understand when and where to use them. I've not looked at the series at all for the past few postings. Michael, please work up an incremental patch to address the kdocs issue. I've picked up the v8 patchset, and there is no need to respin it, but I would like to have that kdoc patch before the 4.2 merge window opens. Sure, now these helpers are settled down, it's time for document, I'll send out the RFC ASAP :-) Regards, Michael Wang -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Tue, 2015-05-12 at 12:09 -0600, Jason Gunthorpe wrote: > On Mon, May 11, 2015 at 08:27:00PM -0400, Doug Ledford wrote: > > On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: > > > I have run with this series and the only issue I have found is not with > > > this > > > patch set directly. > > > > > > This patch: > > > > > > > IB/Verbs: Use management helper rdma_cap_ib_mad() > > > > > > causes an error when you actually use the port passed from the ib_umad > > > module. > > > I have a patch to fix that which I found while trying to build on this > > > series > > > for the use of a bit mask. > > > > > > Doug, I don't know what you would like to do for this fix. I am > > > submitting it > > > shortly with a new version of the core capability bit patches. If you > > > want to > > > just add it after this series or force Michael to respin with the fix? > > > > As I recall, there was a comment from Or requesting to squash some of > > the individual patches down, but I no longer have that email in my Inbox > > to double check. And it seemed like there was one other review comment > > not yet addressed. Do I have that right Michael? And if so, are you > > working on a v9? > > I had asked for better kdocs for the new helpers so new people can > understand when and where to use them. > > I've not looked at the series at all for the past few postings. Michael, please work up an incremental patch to address the kdocs issue. I've picked up the v8 patchset, and there is no need to respin it, but I would like to have that kdoc patch before the 4.2 merge window opens. -- Doug Ledford GPG KeyID: 0E572FDD signature.asc Description: This is a digitally signed message part
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Mon, May 11, 2015 at 08:27:00PM -0400, Doug Ledford wrote: > On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: > > I have run with this series and the only issue I have found is not with this > > patch set directly. > > > > This patch: > > > > > IB/Verbs: Use management helper rdma_cap_ib_mad() > > > > causes an error when you actually use the port passed from the ib_umad > > module. > > I have a patch to fix that which I found while trying to build on this > > series > > for the use of a bit mask. > > > > Doug, I don't know what you would like to do for this fix. I am submitting > > it > > shortly with a new version of the core capability bit patches. If you want > > to > > just add it after this series or force Michael to respin with the fix? > > As I recall, there was a comment from Or requesting to squash some of > the individual patches down, but I no longer have that email in my Inbox > to double check. And it seemed like there was one other review comment > not yet addressed. Do I have that right Michael? And if so, are you > working on a v9? I had asked for better kdocs for the new helpers so new people can understand when and where to use them. I've not looked at the series at all for the past few postings. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 04:24 PM, Doug Ledford wrote: [snip] >> >> AFAIK Or was asking to merge the #15~23, and want to reserve the changelog >> meanwhile reply the cover of prev version (I'm still confused on that...), >> I've replied but get no respond yet. >> >> I can make a v9 to merge the #15~#23 if that could benefit the >> maintainability, >> please let me know your opinion :-) > > I don't think it would make a significant difference. I've pulled the > v8 patchset out of patchworks and I'll throw a new branch with it > included up to my github repo sometime today. Got it :-) Regards, Michael Wang > >> About the Bug, if it was not introduced in this series, maybe including the >> fix in next series would be better? >> >> Regards, >> Michael Wang >> >>> Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny Tested-by: Ira Weiny -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: > Since v7: > * Thanks to Doug, Ira, Devesh for the testing :-) > * Thanks for the comments from or, Doug, Ira, Jason :-) > Please remind me if anything missed :-P > * Use rdma_cap_XX() instead of cap_XX() for readability > * Remove CC list in git log for maintainability > * Use bool as return value > * Updated github repository to v8 > > There are plenty of lengthy code to check the transport type of IB device, > or the link layer type of it's port, but actually we are just speculating > whether a particular management/feature is supported by the device/port. > > Thus instead of inferring, we should have our own mechanism for IB > management > capability/protocol/feature checking, several proposals below. > > This patch set will introduce query_protocol() to check management > requirement > instead of inferring from transport and link layer respectively, along > with > the new enum on protocol type. > > Mapping List: > node-type link-layer transport protocol > nes RNICETH IWARP IWARP > amso1100 RNICETH IWARP IWARP > cxgb3 RNICETH IWARP IWARP > cxgb4 RNICETH IWARP IWARP > usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP > ocrdmaIB_CA ETH IB IBOE > mlx4 IB_CA IB/ETH IB IB/IBOE > mlx5 IB_CA IB IB IB > ehca IB_CA IB IB IB > ipath IB_CA IB IB IB > mthca IB_CA IB IB IB > qib IB_CA IB IB IB > > For example: > if (transport == IB) && (link-layer == ETH) > will now become: > if (query_protocol() == IBOE) > > Thus we will be able to get rid of the respective transport and link-layer > checking, and it will help us to add new protocol/Technology (like OPA) > more > easier, also with the introduced management helpers, IB management logical > will be more clear and easier for extending. > > Highlights: > The long CC list in each patches was complained consider about the > maintainability, it was suggested folks to provide their reviewed-by > or > Acked-by instead, so for those who used to be on the CC list, please > provide your signature voluntarily :-) > > The 'mgmt-helpers' branch of > 'g...@github.com:ywang-pb/infiniband-wy.git' > contain this series based on the latest 'infiniband/for-next' > > Patch 1#~14# included all the logical reform, 15#~23# introduced the > management helpers. > > Doug suggested the bitmask mechanism: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html > which could be the plan for future reforming, we prefer that to be > another > series which focus on semantic and performance. > > This patch-set is somewhat 'bloated' now and it may be a good timing > for > staging, I'd like to suggest we focus on improving existed helpers > and push > all the further reforms into next series ;-) > > Proposals: > Sean: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html > Doug: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html >
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Tue, 2015-05-12 at 09:57 +0200, Michael Wang wrote: > > On 05/12/2015 02:27 AM, Doug Ledford wrote: > > On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: > >> I have run with this series and the only issue I have found is not with > >> this > >> patch set directly. > >> > >> This patch: > >> > >>> IB/Verbs: Use management helper rdma_cap_ib_mad() > >> > >> causes an error when you actually use the port passed from the ib_umad > >> module. > >> I have a patch to fix that which I found while trying to build on this > >> series > >> for the use of a bit mask. > >> > >> Doug, I don't know what you would like to do for this fix. I am > >> submitting it > >> shortly with a new version of the core capability bit patches. If you > >> want to > >> just add it after this series or force Michael to respin with the fix? > > > > As I recall, there was a comment from Or requesting to squash some of > > the individual patches down, but I no longer have that email in my Inbox > > to double check. And it seemed like there was one other review comment > > not yet addressed. Do I have that right Michael? And if so, are you > > working on a v9? > > AFAIK Or was asking to merge the #15~23, and want to reserve the changelog > meanwhile reply the cover of prev version (I'm still confused on that...), > I've replied but get no respond yet. > > I can make a v9 to merge the #15~#23 if that could benefit the > maintainability, > please let me know your opinion :-) I don't think it would make a significant difference. I've pulled the v8 patchset out of patchworks and I'll throw a new branch with it included up to my github repo sometime today. > About the Bug, if it was not introduced in this series, maybe including the > fix in next series would be better? > > Regards, > Michael Wang > > > > >> Frankly > >> I vote for the former because as it stands this series does not break > >> directly. > >> It was only after I changed the implementation of rdma_cap_ib_mad that it > >> broke. > >> > >> > >> For the rest of the series. > >> > >> Reviewed-by: Ira Weiny > >> Tested-by: Ira Weiny > >>-- Limited to mlx4, qib, and OPA (with additional patches.) > >> > >> > >> On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: > >>> Since v7: > >>> * Thanks to Doug, Ira, Devesh for the testing :-) > >>> * Thanks for the comments from or, Doug, Ira, Jason :-) > >>> Please remind me if anything missed :-P > >>> * Use rdma_cap_XX() instead of cap_XX() for readability > >>> * Remove CC list in git log for maintainability > >>> * Use bool as return value > >>> * Updated github repository to v8 > >>> > >>> There are plenty of lengthy code to check the transport type of IB device, > >>> or the link layer type of it's port, but actually we are just speculating > >>> whether a particular management/feature is supported by the device/port. > >>> > >>> Thus instead of inferring, we should have our own mechanism for IB > >>> management > >>> capability/protocol/feature checking, several proposals below. > >>> > >>> This patch set will introduce query_protocol() to check management > >>> requirement > >>> instead of inferring from transport and link layer respectively, along > >>> with > >>> the new enum on protocol type. > >>> > >>> Mapping List: > >>> node-type link-layer transport protocol > >>> nes RNICETH IWARP IWARP > >>> amso1100 RNICETH IWARP IWARP > >>> cxgb3 RNICETH IWARP IWARP > >>> cxgb4 RNICETH IWARP IWARP > >>> usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP > >>> ocrdmaIB_CA ETH IB IBOE > >>> mlx4 IB_CA IB/ETH IB IB/IBOE > >>> mlx5 IB_CA IB IB IB > >>> ehca IB_CA IB IB IB > >>> ipath IB_CA IB IB IB > >>> mthca IB_CA IB IB IB > >>> qib IB_CA IB IB IB > >>> > >>> For example: > >>> if (transport == IB) && (link-layer == ETH) > >>> will now become: > >>> if (query_protocol() == IBOE) > >>> > >>> Thus we will be able to get rid of the respective transport and link-layer > >>> checking, and it will help us to add new protocol/Technology (like OPA) > >>> more > >>> easier, also with the introduced management helpers, IB management logical > >>> will be more clear and easier for extending. > >>> > >>> Highlights: > >>> The long CC list in each patches was complained consider about the > >>> maintainability, it was suggested folks to provide their reviewed-by > >>> or > >>> Acked-by instead, so for those who used to be on the CC list, please > >>> provide your signature voluntarily :-) > >>> >
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 01:49 AM, ira.weiny wrote: > I have run with this series and the only issue I have found is not with this > patch set directly. > > This patch: > >> IB/Verbs: Use management helper rdma_cap_ib_mad() > > causes an error when you actually use the port passed from the ib_umad module. > I have a patch to fix that which I found while trying to build on this series > for the use of a bit mask. > > Doug, I don't know what you would like to do for this fix. I am submitting it > shortly with a new version of the core capability bit patches. If you want to > just add it after this series or force Michael to respin with the fix? > Frankly > I vote for the former because as it stands this series does not break > directly. > It was only after I changed the implementation of rdma_cap_ib_mad that it > broke. Agree, it sounds more reasonable to include the fix in the series introduced it :-P > > > For the rest of the series. > > Reviewed-by: Ira Weiny > Tested-by: Ira Weiny > -- Limited to mlx4, qib, and OPA (with additional patches.) Thanks for the review and testing :-) Regards, Michael Wang > > > On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: >> Since v7: >> * Thanks to Doug, Ira, Devesh for the testing :-) >> * Thanks for the comments from or, Doug, Ira, Jason :-) >> Please remind me if anything missed :-P >> * Use rdma_cap_XX() instead of cap_XX() for readability >> * Remove CC list in git log for maintainability >> * Use bool as return value >> * Updated github repository to v8 >> >> There are plenty of lengthy code to check the transport type of IB device, >> or the link layer type of it's port, but actually we are just speculating >> whether a particular management/feature is supported by the device/port. >> >> Thus instead of inferring, we should have our own mechanism for IB management >> capability/protocol/feature checking, several proposals below. >> >> This patch set will introduce query_protocol() to check management >> requirement >> instead of inferring from transport and link layer respectively, along with >> the new enum on protocol type. >> >> Mapping List: >> node-type link-layer transport protocol >> nes RNICETH IWARP IWARP >> amso1100 RNICETH IWARP IWARP >> cxgb3RNICETH IWARP IWARP >> cxgb4RNICETH IWARP IWARP >> usnicUSNIC_UDP ETH USNIC_UDP USNIC_UDP >> ocrdma IB_CA ETH IB IBOE >> mlx4 IB_CA IB/ETH IB IB/IBOE >> mlx5 IB_CA IB IB IB >> ehca IB_CA IB IB IB >> ipathIB_CA IB IB IB >> mthcaIB_CA IB IB IB >> qib IB_CA IB IB IB >> >> For example: >> if (transport == IB) && (link-layer == ETH) >> will now become: >> if (query_protocol() == IBOE) >> >> Thus we will be able to get rid of the respective transport and link-layer >> checking, and it will help us to add new protocol/Technology (like OPA) more >> easier, also with the introduced management helpers, IB management logical >> will be more clear and easier for extending. >> >> Highlights: >> The long CC list in each patches was complained consider about the >> maintainability, it was suggested folks to provide their reviewed-by or >> Acked-by instead, so for those who used to be on the CC list, please >> provide your signature voluntarily :-) >> >> The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' >> contain this series based on the latest 'infiniband/for-next' >> >> Patch 1#~14# included all the logical reform, 15#~23# introduced the >> management helpers. >> >> Doug suggested the bitmask mechanism: >> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html >> which could be the plan for future reforming, we prefer that to be >> another >> series which focus on semantic and performance. >> >> This patch-set is somewhat 'bloated' now and it may be a good timing for >> staging, I'd like to suggest we focus on improving existed helpers and >> push >> all the further reforms into next series ;-) >> >> Proposals: >> Sean: >> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html >> Doug: >> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html >> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html >> Jason: >> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html >> >> Michael Wang (23): >> IB/Verbs: Implement new callback query_protocol() >> IB/Verbs:
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 02:27 AM, Doug Ledford wrote: > On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: >> I have run with this series and the only issue I have found is not with this >> patch set directly. >> >> This patch: >> >>> IB/Verbs: Use management helper rdma_cap_ib_mad() >> >> causes an error when you actually use the port passed from the ib_umad >> module. >> I have a patch to fix that which I found while trying to build on this series >> for the use of a bit mask. >> >> Doug, I don't know what you would like to do for this fix. I am submitting >> it >> shortly with a new version of the core capability bit patches. If you want >> to >> just add it after this series or force Michael to respin with the fix? > > As I recall, there was a comment from Or requesting to squash some of > the individual patches down, but I no longer have that email in my Inbox > to double check. And it seemed like there was one other review comment > not yet addressed. Do I have that right Michael? And if so, are you > working on a v9? AFAIK Or was asking to merge the #15~23, and want to reserve the changelog meanwhile reply the cover of prev version (I'm still confused on that...), I've replied but get no respond yet. I can make a v9 to merge the #15~#23 if that could benefit the maintainability, please let me know your opinion :-) About the Bug, if it was not introduced in this series, maybe including the fix in next series would be better? Regards, Michael Wang > >> Frankly >> I vote for the former because as it stands this series does not break >> directly. >> It was only after I changed the implementation of rdma_cap_ib_mad that it >> broke. >> >> >> For the rest of the series. >> >> Reviewed-by: Ira Weiny >> Tested-by: Ira Weiny >> -- Limited to mlx4, qib, and OPA (with additional patches.) >> >> >> On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: >>> Since v7: >>> * Thanks to Doug, Ira, Devesh for the testing :-) >>> * Thanks for the comments from or, Doug, Ira, Jason :-) >>> Please remind me if anything missed :-P >>> * Use rdma_cap_XX() instead of cap_XX() for readability >>> * Remove CC list in git log for maintainability >>> * Use bool as return value >>> * Updated github repository to v8 >>> >>> There are plenty of lengthy code to check the transport type of IB device, >>> or the link layer type of it's port, but actually we are just speculating >>> whether a particular management/feature is supported by the device/port. >>> >>> Thus instead of inferring, we should have our own mechanism for IB >>> management >>> capability/protocol/feature checking, several proposals below. >>> >>> This patch set will introduce query_protocol() to check management >>> requirement >>> instead of inferring from transport and link layer respectively, along with >>> the new enum on protocol type. >>> >>> Mapping List: >>> node-type link-layer transport protocol >>> nes RNICETH IWARP IWARP >>> amso1100RNICETH IWARP IWARP >>> cxgb3 RNICETH IWARP IWARP >>> cxgb4 RNICETH IWARP IWARP >>> usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP >>> ocrdma IB_CA ETH IB IBOE >>> mlx4IB_CA IB/ETH IB IB/IBOE >>> mlx5IB_CA IB IB IB >>> ehcaIB_CA IB IB IB >>> ipath IB_CA IB IB IB >>> mthca IB_CA IB IB IB >>> qib IB_CA IB IB IB >>> >>> For example: >>> if (transport == IB) && (link-layer == ETH) >>> will now become: >>> if (query_protocol() == IBOE) >>> >>> Thus we will be able to get rid of the respective transport and link-layer >>> checking, and it will help us to add new protocol/Technology (like OPA) more >>> easier, also with the introduced management helpers, IB management logical >>> will be more clear and easier for extending. >>> >>> Highlights: >>> The long CC list in each patches was complained consider about the >>> maintainability, it was suggested folks to provide their reviewed-by or >>> Acked-by instead, so for those who used to be on the CC list, please >>> provide your signature voluntarily :-) >>> >>> The 'mgmt-helpers' branch of >>> 'g...@github.com:ywang-pb/infiniband-wy.git' >>> contain this series based on the latest 'infiniband/for-next' >>> >>> Patch 1#~14# included all the logical reform, 15#~23# introduced the >>> management helpers. >>> >>> Doug suggested the bitmask mechanism: >>> https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html >>> which could be the plan for future reforming, we prefer that to
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 02:27 AM, Doug Ledford wrote: On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? AFAIK Or was asking to merge the #15~23, and want to reserve the changelog meanwhile reply the cover of prev version (I'm still confused on that...), I've replied but get no respond yet. I can make a v9 to merge the #15~#23 if that could benefit the maintainability, please let me know your opinion :-) About the Bug, if it was not introduced in this series, maybe including the fix in next series would be better? Regards, Michael Wang Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdma IB_CA ETH IB IBOE mlx4IB_CA IB/ETH IB IB/IBOE mlx5IB_CA IB IB IB ehcaIB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 01:49 AM, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. Agree, it sounds more reasonable to include the fix in the series introduced it :-P For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) Thanks for the review and testing :-) Regards, Michael Wang On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100 RNICETH IWARP IWARP cxgb3RNICETH IWARP IWARP cxgb4RNICETH IWARP IWARP usnicUSNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdma IB_CA ETH IB IBOE mlx4 IB_CA IB/ETH IB IB/IBOE mlx5 IB_CA IB IB IB ehca IB_CA IB IB IB ipathIB_CA IB IB IB mthcaIB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html Michael Wang (23): IB/Verbs: Implement new callback query_protocol() IB/Verbs: Implement raw management helpers IB/Verbs: Reform IB-core mad/agent/user_mad IB/Verbs: Reform IB-core cm IB/Verbs: Reform IB-core sa_query
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Mon, May 11, 2015 at 08:27:00PM -0400, Doug Ledford wrote: On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? I had asked for better kdocs for the new helpers so new people can understand when and where to use them. I've not looked at the series at all for the past few postings. Jason -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Tue, 2015-05-12 at 12:09 -0600, Jason Gunthorpe wrote: On Mon, May 11, 2015 at 08:27:00PM -0400, Doug Ledford wrote: On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? I had asked for better kdocs for the new helpers so new people can understand when and where to use them. I've not looked at the series at all for the past few postings. Michael, please work up an incremental patch to address the kdocs issue. I've picked up the v8 patchset, and there is no need to respin it, but I would like to have that kdoc patch before the 4.2 merge window opens. -- Doug Ledford dledf...@redhat.com GPG KeyID: 0E572FDD signature.asc Description: This is a digitally signed message part
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Tue, 2015-05-12 at 09:57 +0200, Michael Wang wrote: On 05/12/2015 02:27 AM, Doug Ledford wrote: On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? AFAIK Or was asking to merge the #15~23, and want to reserve the changelog meanwhile reply the cover of prev version (I'm still confused on that...), I've replied but get no respond yet. I can make a v9 to merge the #15~#23 if that could benefit the maintainability, please let me know your opinion :-) I don't think it would make a significant difference. I've pulled the v8 patchset out of patchworks and I'll throw a new branch with it included up to my github repo sometime today. About the Bug, if it was not introduced in this series, maybe including the fix in next series would be better? Regards, Michael Wang Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100 RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdmaIB_CA ETH IB IBOE mlx4 IB_CA IB/ETH IB IB/IBOE mlx5 IB_CA IB IB IB ehca IB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism:
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/12/2015 04:24 PM, Doug Ledford wrote: [snip] AFAIK Or was asking to merge the #15~23, and want to reserve the changelog meanwhile reply the cover of prev version (I'm still confused on that...), I've replied but get no respond yet. I can make a v9 to merge the #15~#23 if that could benefit the maintainability, please let me know your opinion :-) I don't think it would make a significant difference. I've pulled the v8 patchset out of patchworks and I'll throw a new branch with it included up to my github repo sometime today. Got it :-) Regards, Michael Wang About the Bug, if it was not introduced in this series, maybe including the fix in next series would be better? Regards, Michael Wang Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100 RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdmaIB_CA ETH IB IBOE mlx4 IB_CA IB/ETH IB IB/IBOE mlx5 IB_CA IB IB IB ehca IB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html Michael Wang (23): IB/Verbs: Implement new callback query_protocol() IB/Verbs: Implement raw management helpers IB/Verbs: Reform IB-core mad/agent/user_mad IB/Verbs: Reform IB-core cm IB/Verbs: Reform IB-core sa_query IB/Verbs: Reform IB-core multicast IB/Verbs: Reform IB-ulp ipoib IB/Verbs: Reform IB-ulp xprtrdma
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: > I have run with this series and the only issue I have found is not with this > patch set directly. > > This patch: > > > IB/Verbs: Use management helper rdma_cap_ib_mad() > > causes an error when you actually use the port passed from the ib_umad module. > I have a patch to fix that which I found while trying to build on this series > for the use of a bit mask. > > Doug, I don't know what you would like to do for this fix. I am submitting it > shortly with a new version of the core capability bit patches. If you want to > just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? > Frankly > I vote for the former because as it stands this series does not break > directly. > It was only after I changed the implementation of rdma_cap_ib_mad that it > broke. > > > For the rest of the series. > > Reviewed-by: Ira Weiny > Tested-by: Ira Weiny > -- Limited to mlx4, qib, and OPA (with additional patches.) > > > On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: > > Since v7: > > * Thanks to Doug, Ira, Devesh for the testing :-) > > * Thanks for the comments from or, Doug, Ira, Jason :-) > > Please remind me if anything missed :-P > > * Use rdma_cap_XX() instead of cap_XX() for readability > > * Remove CC list in git log for maintainability > > * Use bool as return value > > * Updated github repository to v8 > > > > There are plenty of lengthy code to check the transport type of IB device, > > or the link layer type of it's port, but actually we are just speculating > > whether a particular management/feature is supported by the device/port. > > > > Thus instead of inferring, we should have our own mechanism for IB > > management > > capability/protocol/feature checking, several proposals below. > > > > This patch set will introduce query_protocol() to check management > > requirement > > instead of inferring from transport and link layer respectively, along with > > the new enum on protocol type. > > > > Mapping List: > > node-type link-layer transport protocol > > nes RNICETH IWARP IWARP > > amso1100RNICETH IWARP IWARP > > cxgb3 RNICETH IWARP IWARP > > cxgb4 RNICETH IWARP IWARP > > usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP > > ocrdma IB_CA ETH IB IBOE > > mlx4IB_CA IB/ETH IB IB/IBOE > > mlx5IB_CA IB IB IB > > ehcaIB_CA IB IB IB > > ipath IB_CA IB IB IB > > mthca IB_CA IB IB IB > > qib IB_CA IB IB IB > > > > For example: > > if (transport == IB) && (link-layer == ETH) > > will now become: > > if (query_protocol() == IBOE) > > > > Thus we will be able to get rid of the respective transport and link-layer > > checking, and it will help us to add new protocol/Technology (like OPA) more > > easier, also with the introduced management helpers, IB management logical > > will be more clear and easier for extending. > > > > Highlights: > > The long CC list in each patches was complained consider about the > > maintainability, it was suggested folks to provide their reviewed-by or > > Acked-by instead, so for those who used to be on the CC list, please > > provide your signature voluntarily :-) > > > > The 'mgmt-helpers' branch of > > 'g...@github.com:ywang-pb/infiniband-wy.git' > > contain this series based on the latest 'infiniband/for-next' > > > > Patch 1#~14# included all the logical reform, 15#~23# introduced the > > management helpers. > > > > Doug suggested the bitmask mechanism: > > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html > > which could be the plan for future reforming, we prefer that to be > > another > > series which focus on semantic and performance. > > > > This patch-set is somewhat 'bloated' now and it may be a good timing for > > staging, I'd like to suggest we focus on improving existed helpers and > > push > > all the further reforms into next series ;-) > > > > Proposals: > > Sean: > > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html > > Doug: > > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html > >
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
I have run with this series and the only issue I have found is not with this patch set directly. This patch: > IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny Tested-by: Ira Weiny -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: > Since v7: > * Thanks to Doug, Ira, Devesh for the testing :-) > * Thanks for the comments from or, Doug, Ira, Jason :-) > Please remind me if anything missed :-P > * Use rdma_cap_XX() instead of cap_XX() for readability > * Remove CC list in git log for maintainability > * Use bool as return value > * Updated github repository to v8 > > There are plenty of lengthy code to check the transport type of IB device, > or the link layer type of it's port, but actually we are just speculating > whether a particular management/feature is supported by the device/port. > > Thus instead of inferring, we should have our own mechanism for IB management > capability/protocol/feature checking, several proposals below. > > This patch set will introduce query_protocol() to check management requirement > instead of inferring from transport and link layer respectively, along with > the new enum on protocol type. > > Mapping List: > node-type link-layer transport protocol > nes RNICETH IWARP IWARP > amso1100 RNICETH IWARP IWARP > cxgb3 RNICETH IWARP IWARP > cxgb4 RNICETH IWARP IWARP > usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP > ocrdmaIB_CA ETH IB IBOE > mlx4 IB_CA IB/ETH IB IB/IBOE > mlx5 IB_CA IB IB IB > ehca IB_CA IB IB IB > ipath IB_CA IB IB IB > mthca IB_CA IB IB IB > qib IB_CA IB IB IB > > For example: > if (transport == IB) && (link-layer == ETH) > will now become: > if (query_protocol() == IBOE) > > Thus we will be able to get rid of the respective transport and link-layer > checking, and it will help us to add new protocol/Technology (like OPA) more > easier, also with the introduced management helpers, IB management logical > will be more clear and easier for extending. > > Highlights: > The long CC list in each patches was complained consider about the > maintainability, it was suggested folks to provide their reviewed-by or > Acked-by instead, so for those who used to be on the CC list, please > provide your signature voluntarily :-) > > The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' > contain this series based on the latest 'infiniband/for-next' > > Patch 1#~14# included all the logical reform, 15#~23# introduced the > management helpers. > > Doug suggested the bitmask mechanism: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html > which could be the plan for future reforming, we prefer that to be another > series which focus on semantic and performance. > > This patch-set is somewhat 'bloated' now and it may be a good timing for > staging, I'd like to suggest we focus on improving existed helpers and > push > all the further reforms into next series ;-) > > Proposals: > Sean: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html > Doug: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html > Jason: > https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html > > Michael Wang (23): > IB/Verbs: Implement new callback query_protocol() > IB/Verbs: Implement raw management helpers > IB/Verbs: Reform IB-core mad/agent/user_mad > IB/Verbs: Reform IB-core cm > IB/Verbs: Reform IB-core sa_query > IB/Verbs: Reform IB-core multicast > IB/Verbs: Reform IB-ulp ipoib > IB/Verbs: Reform IB-ulp xprtrdma > IB/Verbs: Reform IB-core verbs >
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On Mon, 2015-05-11 at 19:49 -0400, ira.weiny wrote: I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? As I recall, there was a comment from Or requesting to squash some of the individual patches down, but I no longer have that email in my Inbox to double check. And it seemed like there was one other review comment not yet addressed. Do I have that right Michael? And if so, are you working on a v9? Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdma IB_CA ETH IB IBOE mlx4IB_CA IB/ETH IB IB/IBOE mlx5IB_CA IB IB IB ehcaIB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
I have run with this series and the only issue I have found is not with this patch set directly. This patch: IB/Verbs: Use management helper rdma_cap_ib_mad() causes an error when you actually use the port passed from the ib_umad module. I have a patch to fix that which I found while trying to build on this series for the use of a bit mask. Doug, I don't know what you would like to do for this fix. I am submitting it shortly with a new version of the core capability bit patches. If you want to just add it after this series or force Michael to respin with the fix? Frankly I vote for the former because as it stands this series does not break directly. It was only after I changed the implementation of rdma_cap_ib_mad that it broke. For the rest of the series. Reviewed-by: Ira Weiny ira.we...@intel.com Tested-by: Ira Weiny ira.we...@intel.com -- Limited to mlx4, qib, and OPA (with additional patches.) On Tue, May 05, 2015 at 02:50:17PM +0200, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100 RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdmaIB_CA ETH IB IBOE mlx4 IB_CA IB/ETH IB IB/IBOE mlx5 IB_CA IB IB IB ehca IB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html Michael Wang (23): IB/Verbs: Implement new callback query_protocol() IB/Verbs: Implement raw management helpers IB/Verbs: Reform IB-core mad/agent/user_mad IB/Verbs: Reform IB-core cm IB/Verbs: Reform IB-core sa_query IB/Verbs: Reform IB-core multicast IB/Verbs: Reform IB-ulp ipoib IB/Verbs: Reform IB-ulp xprtrdma IB/Verbs: Reform IB-core verbs IB/Verbs: Reform cm related part in IB-core cma/ucm
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
Reviewed-by: Devesh Sharma On Tue, May 5, 2015 at 8:10 PM, Michael Wang wrote: > > > On 05/05/2015 04:16 PM, Or Gerlitz wrote: >> On 5/5/2015 3:50 PM, Michael Wang wrote: >>> Since v7: >>>* Thanks to Doug, Ira, Devesh for the testing:-) >>>* Thanks for the comments from or, Doug, Ira, Jason:-) >>> Please remind me if anything missed:-P >>>* Use rdma_cap_XX() instead of cap_XX() for readability >>>* Remove CC list in git log for maintainability >>>* Use bool as return value >>>* Updated github repository to v8 >> >> Didn't you see that patches 15~23 will be squashed into one? > > I missed the conversation on that, you mentioned to remove the > CC list but don't want to merge the patches, correct? > >> >> >> Also, when you post version N you need not only to list the changes since >> version N-1 but rather also to keep the full changes since Vx for x=1...N-2, >> reviewers needs not chase your previous cover letters and see if/whatwent >> wrong, specifically with a sensitive series like this one. > > That maybe too long and with some outdated info, I can reserve > the log since this version if it was preferred by folks. > >> >> So if/when there's V9, use that practice, and for the time being, reply on >> the V8 cover letter with the full listing of changes > > I'm using git send-mail which reply on the cover of each version, > not sure how to reply on a prev cover? and that won't start a new > thread for the new version, isn't it? > > Regards, > Michael Wang > >> >> Or. >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- -Regards Devesh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
Reviewed-by: Devesh Sharma devesh.sha...@avagotech.com On Tue, May 5, 2015 at 8:10 PM, Michael Wang yun.w...@profitbricks.com wrote: On 05/05/2015 04:16 PM, Or Gerlitz wrote: On 5/5/2015 3:50 PM, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing:-) * Thanks for the comments from or, Doug, Ira, Jason:-) Please remind me if anything missed:-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 Didn't you see that patches 15~23 will be squashed into one? I missed the conversation on that, you mentioned to remove the CC list but don't want to merge the patches, correct? Also, when you post version N you need not only to list the changes since version N-1 but rather also to keep the full changes since Vx for x=1...N-2, reviewers needs not chase your previous cover letters and see if/whatwent wrong, specifically with a sensitive series like this one. That maybe too long and with some outdated info, I can reserve the log since this version if it was preferred by folks. So if/when there's V9, use that practice, and for the time being, reply on the V8 cover letter with the full listing of changes I'm using git send-mail which reply on the cover of each version, not sure how to reply on a prev cover? and that won't start a new thread for the new version, isn't it? Regards, Michael Wang Or. -- To unsubscribe from this list: send the line unsubscribe linux-rdma in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- -Regards Devesh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 5/5/2015 3:50 PM, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing:-) * Thanks for the comments from or, Doug, Ira, Jason:-) Please remind me if anything missed:-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 Didn't you see that patches 15~23 will be squashed into one? Also, when you post version N you need not only to list the changes since version N-1 but rather also to keep the full changes since Vx for x=1...N-2, reviewers needs not chase your previous cover letters and see if/whatwent wrong, specifically with a sensitive series like this one. So if/when there's V9, use that practice, and for the time being, reply on the V8 cover letter with the full listing of changes Or. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/05/2015 04:16 PM, Or Gerlitz wrote: > On 5/5/2015 3:50 PM, Michael Wang wrote: >> Since v7: >>* Thanks to Doug, Ira, Devesh for the testing:-) >>* Thanks for the comments from or, Doug, Ira, Jason:-) >> Please remind me if anything missed:-P >>* Use rdma_cap_XX() instead of cap_XX() for readability >>* Remove CC list in git log for maintainability >>* Use bool as return value >>* Updated github repository to v8 > > Didn't you see that patches 15~23 will be squashed into one? I missed the conversation on that, you mentioned to remove the CC list but don't want to merge the patches, correct? > > > Also, when you post version N you need not only to list the changes since > version N-1 but rather also to keep the full changes since Vx for x=1...N-2, > reviewers needs not chase your previous cover letters and see if/whatwent > wrong, specifically with a sensitive series like this one. That maybe too long and with some outdated info, I can reserve the log since this version if it was preferred by folks. > > So if/when there's V9, use that practice, and for the time being, reply on > the V8 cover letter with the full listing of changes I'm using git send-mail which reply on the cover of each version, not sure how to reply on a prev cover? and that won't start a new thread for the new version, isn't it? Regards, Michael Wang > > Or. > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 00/23] IB/Verbs: IB Management Helpers
Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdma IB_CA ETH IB IBOE mlx4IB_CA IB/ETH IB IB/IBOE mlx5IB_CA IB IB IB ehcaIB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) && (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html Michael Wang (23): IB/Verbs: Implement new callback query_protocol() IB/Verbs: Implement raw management helpers IB/Verbs: Reform IB-core mad/agent/user_mad IB/Verbs: Reform IB-core cm IB/Verbs: Reform IB-core sa_query IB/Verbs: Reform IB-core multicast IB/Verbs: Reform IB-ulp ipoib IB/Verbs: Reform IB-ulp xprtrdma IB/Verbs: Reform IB-core verbs IB/Verbs: Reform cm related part in IB-core cma/ucm IB/Verbs: Reform route related part in IB-core cma IB/Verbs: Reform mcast related part in IB-core cma IB/Verbs: Reform cma_acquire_dev() IB/Verbs: Reform rest part in IB-core cma IB/Verbs: Use management helper rdma_cap_ib_mad() IB/Verbs: Use management helper rdma_cap_ib_smi() IB/Verbs: Use management helper rdma_cap_ib_cm() IB/Verbs: Use management helper rdma_cap_iw_cm() IB/Verbs: Use management helper rdma_cap_ib_sa() IB/Verbs: Use management helper rdma_cap_ib_mcast() IB/Verbs: Use management helper rdma_cap_read_multi_sge() IB/Verbs: Use management helper rdma_cap_af_ib() IB/Verbs: Use management helper rdma_cap_eth_ah() drivers/infiniband/core/agent.c | 2 +- drivers/infiniband/core/cm.c | 20 ++- drivers/infiniband/core/cma.c| 257 +++ drivers/infiniband/core/device.c | 1 + drivers/infiniband/core/mad.c| 43 +++-- drivers/infiniband/core/multicast.c | 12 +-
[PATCH v8 00/23] IB/Verbs: IB Management Helpers
Since v7: * Thanks to Doug, Ira, Devesh for the testing :-) * Thanks for the comments from or, Doug, Ira, Jason :-) Please remind me if anything missed :-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 There are plenty of lengthy code to check the transport type of IB device, or the link layer type of it's port, but actually we are just speculating whether a particular management/feature is supported by the device/port. Thus instead of inferring, we should have our own mechanism for IB management capability/protocol/feature checking, several proposals below. This patch set will introduce query_protocol() to check management requirement instead of inferring from transport and link layer respectively, along with the new enum on protocol type. Mapping List: node-type link-layer transport protocol nes RNICETH IWARP IWARP amso1100RNICETH IWARP IWARP cxgb3 RNICETH IWARP IWARP cxgb4 RNICETH IWARP IWARP usnic USNIC_UDP ETH USNIC_UDP USNIC_UDP ocrdma IB_CA ETH IB IBOE mlx4IB_CA IB/ETH IB IB/IBOE mlx5IB_CA IB IB IB ehcaIB_CA IB IB IB ipath IB_CA IB IB IB mthca IB_CA IB IB IB qib IB_CA IB IB IB For example: if (transport == IB) (link-layer == ETH) will now become: if (query_protocol() == IBOE) Thus we will be able to get rid of the respective transport and link-layer checking, and it will help us to add new protocol/Technology (like OPA) more easier, also with the introduced management helpers, IB management logical will be more clear and easier for extending. Highlights: The long CC list in each patches was complained consider about the maintainability, it was suggested folks to provide their reviewed-by or Acked-by instead, so for those who used to be on the CC list, please provide your signature voluntarily :-) The 'mgmt-helpers' branch of 'g...@github.com:ywang-pb/infiniband-wy.git' contain this series based on the latest 'infiniband/for-next' Patch 1#~14# included all the logical reform, 15#~23# introduced the management helpers. Doug suggested the bitmask mechanism: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html which could be the plan for future reforming, we prefer that to be another series which focus on semantic and performance. This patch-set is somewhat 'bloated' now and it may be a good timing for staging, I'd like to suggest we focus on improving existed helpers and push all the further reforms into next series ;-) Proposals: Sean: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23339.html Doug: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23418.html https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23765.html Jason: https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg23425.html Michael Wang (23): IB/Verbs: Implement new callback query_protocol() IB/Verbs: Implement raw management helpers IB/Verbs: Reform IB-core mad/agent/user_mad IB/Verbs: Reform IB-core cm IB/Verbs: Reform IB-core sa_query IB/Verbs: Reform IB-core multicast IB/Verbs: Reform IB-ulp ipoib IB/Verbs: Reform IB-ulp xprtrdma IB/Verbs: Reform IB-core verbs IB/Verbs: Reform cm related part in IB-core cma/ucm IB/Verbs: Reform route related part in IB-core cma IB/Verbs: Reform mcast related part in IB-core cma IB/Verbs: Reform cma_acquire_dev() IB/Verbs: Reform rest part in IB-core cma IB/Verbs: Use management helper rdma_cap_ib_mad() IB/Verbs: Use management helper rdma_cap_ib_smi() IB/Verbs: Use management helper rdma_cap_ib_cm() IB/Verbs: Use management helper rdma_cap_iw_cm() IB/Verbs: Use management helper rdma_cap_ib_sa() IB/Verbs: Use management helper rdma_cap_ib_mcast() IB/Verbs: Use management helper rdma_cap_read_multi_sge() IB/Verbs: Use management helper rdma_cap_af_ib() IB/Verbs: Use management helper rdma_cap_eth_ah() drivers/infiniband/core/agent.c | 2 +- drivers/infiniband/core/cm.c | 20 ++- drivers/infiniband/core/cma.c| 257 +++ drivers/infiniband/core/device.c | 1 + drivers/infiniband/core/mad.c| 43 +++-- drivers/infiniband/core/multicast.c | 12 +-
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 05/05/2015 04:16 PM, Or Gerlitz wrote: On 5/5/2015 3:50 PM, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing:-) * Thanks for the comments from or, Doug, Ira, Jason:-) Please remind me if anything missed:-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 Didn't you see that patches 15~23 will be squashed into one? I missed the conversation on that, you mentioned to remove the CC list but don't want to merge the patches, correct? Also, when you post version N you need not only to list the changes since version N-1 but rather also to keep the full changes since Vx for x=1...N-2, reviewers needs not chase your previous cover letters and see if/whatwent wrong, specifically with a sensitive series like this one. That maybe too long and with some outdated info, I can reserve the log since this version if it was preferred by folks. So if/when there's V9, use that practice, and for the time being, reply on the V8 cover letter with the full listing of changes I'm using git send-mail which reply on the cover of each version, not sure how to reply on a prev cover? and that won't start a new thread for the new version, isn't it? Regards, Michael Wang Or. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 00/23] IB/Verbs: IB Management Helpers
On 5/5/2015 3:50 PM, Michael Wang wrote: Since v7: * Thanks to Doug, Ira, Devesh for the testing:-) * Thanks for the comments from or, Doug, Ira, Jason:-) Please remind me if anything missed:-P * Use rdma_cap_XX() instead of cap_XX() for readability * Remove CC list in git log for maintainability * Use bool as return value * Updated github repository to v8 Didn't you see that patches 15~23 will be squashed into one? Also, when you post version N you need not only to list the changes since version N-1 but rather also to keep the full changes since Vx for x=1...N-2, reviewers needs not chase your previous cover letters and see if/whatwent wrong, specifically with a sensitive series like this one. So if/when there's V9, use that practice, and for the time being, reply on the V8 cover letter with the full listing of changes Or. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
