Re: [openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
Roland Dreier wrote:
Looking at this list of exports, I do see a couple of things that
could maybe be improved:
> +EXPORT_SYMBOL(rdma_wq);
> +EXPORT_SYMBOL(rdma_translate_ip);
> +EXPORT_SYMBOL(rdma_resolve_ip);
> +EXPORT_SYMBOL(rdma_addr_cancel);
First, rdma_wq seems like a strange internal thing to be exporting.
Sean, why does more than one module need to use the same workqueue?
This is simply an attempt to reduce/combine work queues used by the Infiniband
code. This keeps the threading a little simpler in the rdma_cm, since all
callbacks are invoked using the same work queue. (I'm also using this with the
local SA/multicast code, but that's not ready for merging.)
Second, the naming of rdma_addr_cancel() is not that symmetric with
the rdma_{translate,resolve}_ip() functions. Unfortunately I'm just
clever enough to criticize, not clever enough to come up with a better
suggestion.
rdma_resolve_ip() is the operation actually being canceled if that makes it
easier to come up with a better name.
- Sean
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
> >>Please explain the thinking behind the choice of a non-GPL export. > >>(Yes, we discussed this when inifiniband was first merged, but it > >>doesn't hurt to reiterate). > > The agreement made within the OpenIB community, from where this code > originates, > is that all source code be licensed under a dual license of BSD/GPL. I am > not a > lawyer, so I don't know the implications of changing the exports to be GPL > only, > given the OpenIB license. But my understanding is that makes using those > functions less attractive. no actually ;) but I understood OpenIB to be "GPL when used in the linux kernel, optionally BSD when outside linux". Since EXPORT_SYMBOL is highly linux kernel specific... the _GPL should be just fine ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
Putting this in -mm has generated more discussion than three postings
of the complete patch series to lkml. Andrew, please don't ever turn
your powers to evil ;)
Andrew> What code will use them?
Andrew> Is it planned that this code be merged?
Anyway, there are three main users of these APIs:
- A kernel module that allows userspace to use this IP-based RDMA
connection abstraction. It's my fault that this didn't make it
into -mm: I didn't think it was quite as baked as the kernel side,
and it's also more important to have the userspace interface stable
before merging it upstream, so I didn't put it in my git tree.
However, it should go into -mm, so I'll include it for next time.
- iSER ("iSCSI over IB"). The feeling I get from Christoph et al is
that this is pretty much ready to merge as well, but I'm not sure
if it's being staged anywhere.
- NFS/RDMA. This is not ready to merge yet but it is being worked on
with the idea of going upstream as soon as it's ready.
Looking at this list of exports, I do see a couple of things that
could maybe be improved:
> +EXPORT_SYMBOL(rdma_wq);
> +EXPORT_SYMBOL(rdma_translate_ip);
> +EXPORT_SYMBOL(rdma_resolve_ip);
> +EXPORT_SYMBOL(rdma_addr_cancel);
First, rdma_wq seems like a strange internal thing to be exporting.
Sean, why does more than one module need to use the same workqueue?
Second, the naming of rdma_addr_cancel() is not that symmetric with
the rdma_{translate,resolve}_ip() functions. Unfortunately I'm just
clever enough to criticize, not clever enough to come up with a better
suggestion.
Andrew> Please explain the thinking behind the choice of a non-GPL
Andrew> export. (Yes, we discussed this when inifiniband was
Andrew> first merged, but it doesn't hurt to reiterate).
I don't think there's any deep thought behind it, except that these
exports (for the most part) form a high-level coherent API and there's
no desire by the original author to restrict things.
ip_dev_find() was originally a non-GPL export, then removed for a
while. So it makes sense to just revert the first change.
- R.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
Matthew Frost wrote: To the point. I, insightful betimes, but a non-user of the technology, can grep TFM's and find out what the names could mean, but we're left guessing at what some of these *do*. Translating names falls into the "any idiot can" category of data mining, but if you code for them, we can see context. If you named them more transparently, we might even use them right. Maybe just comment well? Documentation in the form of comments are provided in the header files. I can clarify the calls if needed. To help understand the structure of this patch, three modules were submitted: ib_addr - maps IP addresses to an RDMA device. rdma_cm - Adds connection management over Infiniband using IP addressing. This exports most of the symbols below. rdma_ucm - Exports the rdma_cm functionality to userspace. This uses most of the exported symbols (starting at rdma_create_id and below). +EXPORT_SYMBOL(rdma_wq); Work Queue (do what to it?) This is used by ib_addr and rdma_cm modules to invoke user callbacks. Additional code not yet ready for merging will also make use of this work queue. The intent is to re-use this work queue, rather than each module creating their own. +EXPORT_SYMBOL(rdma_translate_ip); Translate IP Address +EXPORT_SYMBOL(rdma_resolve_ip); Resolve IP Address +EXPORT_SYMBOL(rdma_addr_cancel); Address Cancel (memory?) These exports are from the ib_addr module. The routines are called by the rdma_cm module. The first two map an IP address to a local Infiniband device address. Rdma_resolve_ip is an asynchronous call, so rdma_addr_cancel is used to cancel its operation. 'rdma_cancel_resolve_ip' might have been a clearer name. +EXPORT_SYMBOL(rdma_create_id); Create (?) ID The rdma_cm_id created by this call is required for the calls below. Conceptually, it may help to think of an rdma_cm_id as somewhat like a socket. +EXPORT_SYMBOL(rdma_create_qp); Create Queue Pair (WQ,CQ) +EXPORT_SYMBOL(rdma_destroy_qp); Destroy Queue Pair (WQ,CQ) 'rdma_create_qp' associates a QP with an rdma_cm_id, so that the rdma_cm can perform the QP transitions for the user during connection establishment. +EXPORT_SYMBOL(rdma_init_qp_attr); Set Initial Queue Pair Attributes (?) This initializes the QP attributes for a user that wants to manually perform QP transitions. It is provided mainly for userspace support. +EXPORT_SYMBOL(rdma_destroy_id); Destroy (?) ID +EXPORT_SYMBOL(rdma_listen); Listen (to ... socket, port, pipe, what?) Listens across RDMA devices for connection requests. The listen is on an rdma_cm_id. +EXPORT_SYMBOL(rdma_resolve_route); Resolve Route (datagram path?) In Infiniband terms, this obtains a path record from the subnet manager. The path record specifies the route through the subnet that packets between two connected queue pairs will take. +EXPORT_SYMBOL(rdma_resolve_addr); Resolve Address (memory?) This converts struct sockaddr to RDMA addresses. It ends up calling rdma_translate_ip and rdma_resolve_ip, but performs some additional work to handle device hotplug events. +EXPORT_SYMBOL(rdma_bind_addr); Bind Address (memory?) Associates an rdma_cm_id to a specific struct sockaddr. +EXPORT_SYMBOL(rdma_connect); Connect +EXPORT_SYMBOL(rdma_accept); Accept +EXPORT_SYMBOL(rdma_reject); Reject +EXPORT_SYMBOL(rdma_disconnect); Disconnect Address vs. IP - I know we're talking about a net/dma kluge here, but the twin usage is bugging me. I'm intuiting the _addr as memory addresses, rather than IP addresses, which seem to be _ip, but my poor gray goo suffers pointer overload. Maybe the naming is off here. I used _ip when referring specifically to an IP address, and _addr when using a struct sockaddr. In some cases, such as rdma_bind_addr, the 'address' may be nothing more than a port number. +EXPORT_SYMBOL(ib_get_rmpp_segment); Reliable MultiPacket Protocol This is from a separate patch. It is exported by the ib_mad module, and used by the ib_umad module. There is at least one out of tree module (not yet ready for merging) that will make use of it. +EXPORT_SYMBOL(ib_copy_qp_attr_to_user); Push Queue Pair Attribute +EXPORT_SYMBOL(ib_copy_path_rec_to_user); Push Path Record +EXPORT_SYMBOL(ib_copy_path_rec_from_user); Retrieve Path Record These are used by ib_uverbs, ib_ucm, and rdma_ucm modules. +EXPORT_SYMBOL(ib_modify_qp_is_ok); Yes, Modify Queue Pair, or "QP is OK", or "QP was Modified OK"? This is from a separate patch. It should be exported by ib_verbs, and used by ib_mthca. The call verifies that the settings to modify a QP from one state to the next are valid. I believe that the check used to be part of the ib_mthca driver itself, but additional kernel drivers, such as the ipath driver recently submitted for inclusion, now make use of this routine. +EXPORT_SYMBOL(ip_dev_find); Find IP device (sub(/ip/, "ib")? find the network interface device?) This is the network call in fib_frontend.
[openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
--- Andrew Morton <[EMAIL PROTECTED]> wrote: > "Sean Hefty" <[EMAIL PROTECTED]> wrote: > > > > >I'm not exactly happy that this tree adds tons of RDMA CM > > >EXPORT_SYMBOL's that are neither currently used nor _GPL. > > > > The symbols are used by the kernel component that provides userspace > support. > > > > There's brevity and then there is obscurity. > To the point. I, insightful betimes, but a non-user of the technology, can grep TFM's and find out what the names could mean, but we're left guessing at what some of these *do*. Translating names falls into the "any idiot can" category of data mining, but if you code for them, we can see context. If you named them more transparently, we might even use them right. Maybe just comment well? RDMA +EXPORT_SYMBOL(rdma_wq); Work Queue (do what to it?) +EXPORT_SYMBOL(rdma_translate_ip); Translate IP Address +EXPORT_SYMBOL(rdma_resolve_ip); Resolve IP Address +EXPORT_SYMBOL(rdma_addr_cancel); Address Cancel (memory?) +EXPORT_SYMBOL(rdma_create_id); Create (?) ID +EXPORT_SYMBOL(rdma_create_qp); Create Queue Pair (WQ,CQ) +EXPORT_SYMBOL(rdma_destroy_qp); Destroy Queue Pair (WQ,CQ) +EXPORT_SYMBOL(rdma_init_qp_attr); Set Initial Queue Pair Attributes (?) +EXPORT_SYMBOL(rdma_destroy_id); Destroy (?) ID +EXPORT_SYMBOL(rdma_listen); Listen (to ... socket, port, pipe, what?) +EXPORT_SYMBOL(rdma_resolve_route); Resolve Route (datagram path?) +EXPORT_SYMBOL(rdma_resolve_addr); Resolve Address (memory?) +EXPORT_SYMBOL(rdma_bind_addr); Bind Address (memory?) +EXPORT_SYMBOL(rdma_connect); Connect +EXPORT_SYMBOL(rdma_accept); Accept +EXPORT_SYMBOL(rdma_reject); Reject +EXPORT_SYMBOL(rdma_disconnect); Disconnect Address vs. IP - I know we're talking about a net/dma kluge here, but the twin usage is bugging me. I'm intuiting the _addr as memory addresses, rather than IP addresses, which seem to be _ip, but my poor gray goo suffers pointer overload. INFINIBAND +EXPORT_SYMBOL(ib_get_rmpp_segment); Reliable MultiPacket Protocol +EXPORT_SYMBOL(ib_copy_qp_attr_to_user); Push Queue Pair Attribute +EXPORT_SYMBOL(ib_copy_path_rec_to_user); Push Path Record +EXPORT_SYMBOL(ib_copy_path_rec_from_user); Retrieve Path Record +EXPORT_SYMBOL(ib_modify_qp_is_ok); Yes, Modify Queue Pair, or "QP is OK", or "QP was Modified OK"? +EXPORT_SYMBOL(ip_dev_find); Find IP device (sub(/ip/, "ib")? find the network interface device?) > > Some or all of those exports have no in-tree users. > > What code will use them? > > Is it planned that this code be merged? *Can* work that uses them be merged? Code that requires a non-GPL export? > > Please explain the thinking behind the choice of a non-GPL export. > (Yes, we discussed this when inifiniband was first merged, but it > doesn't hurt to reiterate). > Color me curious ... Frosty > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" > in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
On Sat, 2006-03-18 at 19:44 -0600, Tom Tucker wrote: > I apologize in advance for my ignorance, but I don't understand how > 'new symbols' could pass the test of 'currently used'. > Normally when adding new exports you would submit the code that uses them at the same time. Lee ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
On Sat, 2006-03-18 at 18:25 +0100, Adrian Bunk wrote: > On Sat, Mar 18, 2006 at 04:40:56AM -0800, Andrew Morton wrote: > >... > > Changes since 2.6.16-rc6-mm1: > >... > > git-infiniband.patch > >... > > git trees. > >... > > I'm not exactly happy that this tree adds tons of RDMA CM > EXPORT_SYMBOL's that are neither currently used nor _GPL. I apologize in advance for my ignorance, but I don't understand how 'new symbols' could pass the test of 'currently used'. > cu > Adrian > ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: 2.6.16-rc6-mm2: new RDMA CM EXPORT_SYMBOL's
"Sean Hefty" <[EMAIL PROTECTED]> wrote: > > >I'm not exactly happy that this tree adds tons of RDMA CM > >EXPORT_SYMBOL's that are neither currently used nor _GPL. > > The symbols are used by the kernel component that provides userspace support. > There's brevity and then there is obscurity. +EXPORT_SYMBOL(rdma_wq); +EXPORT_SYMBOL(rdma_translate_ip); +EXPORT_SYMBOL(rdma_resolve_ip); +EXPORT_SYMBOL(rdma_addr_cancel); +EXPORT_SYMBOL(rdma_create_id); +EXPORT_SYMBOL(rdma_create_qp); +EXPORT_SYMBOL(rdma_destroy_qp); +EXPORT_SYMBOL(rdma_init_qp_attr); +EXPORT_SYMBOL(rdma_destroy_id); +EXPORT_SYMBOL(rdma_listen); +EXPORT_SYMBOL(rdma_resolve_route); +EXPORT_SYMBOL(rdma_resolve_addr); +EXPORT_SYMBOL(rdma_bind_addr); +EXPORT_SYMBOL(rdma_connect); +EXPORT_SYMBOL(rdma_accept); +EXPORT_SYMBOL(rdma_reject); +EXPORT_SYMBOL(rdma_disconnect); +EXPORT_SYMBOL(ib_get_rmpp_segment); +EXPORT_SYMBOL(ib_copy_qp_attr_to_user); +EXPORT_SYMBOL(ib_copy_path_rec_to_user); +EXPORT_SYMBOL(ib_copy_path_rec_from_user); +EXPORT_SYMBOL(ib_modify_qp_is_ok); +EXPORT_SYMBOL(ip_dev_find); Some or all of those exports have no in-tree users. What code will use them? Is it planned that this code be merged? Please explain the thinking behind the choice of a non-GPL export. (Yes, we discussed this when inifiniband was first merged, but it doesn't hurt to reiterate). ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
