iface_attr.cap.flags value on ib/ud

[keisukefukuda]

I'm creating this issue instead of #2816 .

I think UCT_IFACE_FLAG_CONNECT_TO_EP and UCT_IFACE_FLAG_CONNECT_TO_IFACE of iface_attr.cap.flags are mutually exclusive.

However, in my environment, which has Mellanox Infiniband FDR HCA, both of the flags of ib/ud/mlx4_0:1 are ON as shown below.

Here's a program to demonstrate the problem, which is a shorter version of test/examples/uct_hello_world.c.
https://gist.github.com/keisukefukuda/5ab5b36d63cdb8a5acdd7428cd380f2a#file-uct_hello_world-c-L467

It checks the value of if_info.attr.cap.flags and prints if UCT_IFACE_FLAG_CONNECT_TO_EP and UCT_IFACE_FLAG_CONNECT_TO_IFACE are ON.

In my environment,

$ gcc -o uct_hello_world   uct_hello_world.c   -luct -lucs -I$HOME/ucx/install/include -L$HOME/ucx/install/lib -Wl,-rpath,$HOME/ucx/install/lib -pedantic
$ ./uct_hello_world -b -t ud -d mlx4_0:1
Using mlx4_0:1 with ud.
UCT_IFACE_FLAG_CONNECT_TO_IFACE
UCT_IFACE_FLAG_CONNECT_TO_EP

UCX version is

$ git show
commit 493445d62a875d6035d48824eeda145448630bed
Merge: 87d06c5 4cb5047
Author: Yossi Itigin <yosefe@mellanox.com>
Date:   Wed Aug 22 11:00:09 2018 +0300

Thanks.

[shamisp]

Thanks for bringing up this issue. FYI for code contribution we require sign contributor agreement which can be found here http://www.openucx.org/license/

You can email it to me.

[yosefe]

this is intentional, UD supports both connection modes

[shamisp]

@yosefe if user gets confused about this we have to document this explicitly. The flags are not mutually exclusive and some interfaces may support both. @keisukefukuda what kinda error it causes for you application ?

[keisukefukuda]

I wrote code like this: https://github.com/keisukefukuda/flucx/blob/master/include/flucx/communicator/communicator.hpp#L82

(Although it was more than a week ago and I don't remember well...)
When I wrote the code first, the order the if blocks were CONNECT_TO_IFACE -> CONNECT_TO_EP, and I got an error UCS_ERR_NO_RESOURCE = -2 from uct_ep_am_zcopy(). Then I looked into the UCX internal and found that uct_ud_ep_is_connected() returned 0 at https://github.com/openucx/ucx/blob/master/src/uct/ib/ud/base/ud_inl.h#L73 .

After the finding, I tired changing the order of the if blocks to CONNECT_TO_EP -> CONNECT_TO_IFACE, which is the same order as test/examples/uct_hello_world.c does, and it worked.

Although my code is my own C++ wrapper of uct and it's a bit long, it is easy to check the issue by just changing uct_hello_world.c .

The original uct_hello_world

$ ./uct_hello_world -b -t ud -d mlx4_0:1 &
[1] 14083
INFO: UCT_HELLO_WORLD AM function = uct_ep_am_bcopy server = (null) port = 13337
 Using mlx4_0:1 with ud.
Waiting for connection...

$ ./uct_hello_world -b -t ud -d mlx4_0:1 -n localhost -p 13337
INFO: UCT_HELLO_WORLD AM function = uct_ep_am_bcopy server = localhost port = 13337
Using mlx4_0:1 with ud.


----- UCT TEST SUCCESS ----

[callback] uct_ep_am_bcopy sent QVIJJLUGZXFUXOS

---------------------------


----- UCT TEST SUCCESS ----

[main] uct_ep_am_bcopy sent QVIJJLUGZXFUXOS

---------------------------
[2]  + done       ./uct_hello_world -b -t ud -d mlx4_0:1

Good.

Then change the code at https://github.com/openucx/ucx/blob/master/test/examples/uct_hello_world.c#L550 .

    if (if_info.attr.cap.flags & UCT_IFACE_FLAG_CONNECT_TO_IFACE) {
        /* Create an endpoint which is connected to a remote interface */
        status = uct_ep_create_connected(if_info.iface, peer_dev, peer_iface, &ep);
    } else if (if_info.attr.cap.flags & UCT_IFACE_FLAG_CONNECT_TO_EP) {
        own_ep = (uct_ep_addr_t*)calloc(1, if_info.attr.ep_addr_len);
        CHKERR_JUMP(NULL == own_ep, "allocate memory for ep addrs", out_free_if_addrs);

        /* Create new endpoint */
        status = uct_ep_create(if_info.iface, &ep);
        CHKERR_JUMP(UCS_OK != status, "create endpoint", out_free_ep_addrs);
        ....

(just change the order of if blocks so CONNECT_TO_IFACE comes first) then the result is

INFO: UCT_HELLO_WORLD AM function = uct_ep_am_bcopy server = localhost port = 13337
Using mlx4_0:1 with ud.
Failed to send active msg

If ib/ud can accept both of CONNECT_TO_EP and CONNECT_TO_IFACE as @yosefe says, I think the code should work regardless of the order of if blocks. It looks to me that the error happened becauseib/ud does not support CONNECT_TO_IFACE.

[keisukefukuda]

@shamisp BTW, about CLA, I didn't find your email address. Do you mind telling me your address via twitter DM (twitter/@keisukefukuda) or [same account name]@gmail.com to me?

[yosefe]

so the code of https://github.com/openucx/ucx/blob/master/test/examples/uct_hello_world.c#L164 is not entirely correct, UCS_ERR_NO_RESOURCE is actually a valid error code which means the caller has to do some progress and then retry the operation. I guess uct_ep_connect_to_ep() just avoids this situation in the first place.
@evgeny-leksikov can you pls fix the uct_hello_world example to check for UCS_ERR_NO_RESOURCE?

[shamisp]

@keisukefukuda it is pasharesearch at gmail

[shamisp]

I checked the documentation and we do not state anywhere that UCT_IFACE_FLAG_CONNECT_TO_EP and UCT_IFACE_FLAG_CONNECT_TO_IFACE are mutually exclusive. As @yosefe mentioned we should fix the example

[keisukefukuda]

Thanks for the replies, but I'm afraid I'm still confused on this.

Now I understand that UCT_IFACE_FLAG_CONNECT_TO_EP and UCT_IFACE_FLAG_CONNECT_TO_IFACE are NOT mutually exclusive and ib/ud supports both of them.

Then, however, I believe that uct_hello_world should work regardless of the if block order (as I mentioned previously). I checked it again using the #2820.
https://github.com/keisukefukuda/ucx/blob/check-uct-hello-world/test/examples/uct_hello_world.c#L550 (the branch on my repo is derived from #2820).
Alternatively, you can even remove the UCT_IFACE_FLAG_CONNECT_TO_EP branch entirely. It should still work , shouldn't it?

So the problems are:

(1) The modified code still doesn't work with zcopy. After uct_ep_am_zcopy got UCS_ERR_NO_RESOURCE, the it enters an infinite loop. (i.e. uct_worker_progress() does not actually progress anything)

(2) The original problem happens for bcopy as well. The same error UCS_ERR_NO_RESOURCE is returned from uct_ep_am_bcopy() and the program fails with an error message Failed to send active msg.
(Actually, in my comment above, the program option were -b (bcopy), after talking about zcopy. I didn't explicitly mention it and it was confusing. sorry.)

So I'm afraid #2820 is not a solution yet.

Keisuke

[evgeny-leksikov]

@keisukefukuda thanks for details I'll continue to work with #2820 to have all related fixes in single PR.

[shamisp]

@keisukefukuda please let me know if your problem was resolved

[keisukefukuda]

I'm afraid the problem is not solved.

My understanding is that ib/ud supports both of CONNECT_TO_EP and CONNECT_TO_IFACE, so this code https://gist.github.com/keisukefukuda/da884c53610a6bb989472977b690170d#file-uct_hello_world-c-L564 should work.
The code is basically the same as uct_hello_world.c with a slight modification. In short, it does not use CONNECT_TO_EP for connection. I think it is expected to work with ib/ud, but does not in my environment (with Mellanox FDR).

I would like to know if the code works for you guys in your environment. If it's just an environment-spepcific issue, I will investigate on my own (or just avoid the issue) in my code.

The actual diff from the master original is

diff --git a/test/examples/uct_hello_world.c b/test/examples/uct_hello_world.c
index 18fc291..fa264e4 100644
--- a/test/examples/uct_hello_world.c
+++ b/test/examples/uct_hello_world.c
@@ -547,26 +547,7 @@ int main(int argc, char **argv)
         CHKERR_JUMP(0 != status, "ifaces exchange", out_free_if_addrs);
     }

-    if (if_info.attr.cap.flags & UCT_IFACE_FLAG_CONNECT_TO_EP) {
-        own_ep = (uct_ep_addr_t*)calloc(1, if_info.attr.ep_addr_len);
-        CHKERR_JUMP(NULL == own_ep, "allocate memory for ep addrs", out_free_if_addrs);
-
-        /* Create new endpoint */
-        status = uct_ep_create(if_info.iface, &ep);
-        CHKERR_JUMP(UCS_OK != status, "create endpoint", out_free_ep_addrs);
-
-        /* Get endpoint address */
-        status = uct_ep_get_address(ep, own_ep);
-        CHKERR_JUMP(UCS_OK != status, "get endpoint address", out_free_ep);
-
-        status = sendrecv(oob_sock, own_ep, if_info.attr.ep_addr_len,
-                          (void **)&peer_ep);
-        CHKERR_JUMP(0 != status, "EPs exchange", out_free_ep);
-
-        /* Connect endpoint to a remote endpoint */
-        status = uct_ep_connect_to_ep(ep, peer_dev, peer_ep);
-        barrier(oob_sock);
-    } else if (if_info.attr.cap.flags & UCT_IFACE_FLAG_CONNECT_TO_IFACE) {
+    if (if_info.attr.cap.flags & UCT_IFACE_FLAG_CONNECT_TO_IFACE) {
         /* Create an endpoint which is connected to a remote interface */
         status = uct_ep_create_connected(if_info.iface, peer_dev, peer_iface, &ep);
     } else {

[evgeny-leksikov]

@keisukefukuda please check if your repo is up to date against master. I have checked your patch applied to master and it works fine on my setup with all AM APIs (short/bcopy/zcopy) and UD transport.
(https://gist.github.com/keisukefukuda/da884c53610a6bb989472977b690170d#file-uct_hello_world-c-L564 is not updated)
If it's still not working by some reason, pls share how you run the test and output of ibv_devinfo command on your servers.

[keisukefukuda]

Oops, my local master was not updated. my apologies.
I tried and it worked!! Thanks!