UCP: add support fixed address for mmap #1329

evgeny-leksikov · 2017-02-15T11:46:44Z

Fixes #1273
Do not merge before #1323
@shssf , @brminich pls review, note that several commits (UCT part) are already reviewed in #1323

evgeny-leksikov · 2017-02-16T06:58:03Z

@MattBBaker please share ORNL failures

brminich

Not sure that I checked all relevant commits, but tried to look thru UCP related

brminich · 2017-02-16T10:00:04Z

src/ucp/api/ucp.h

+    UCP_MEM_MAP_ALLOCATE = UCS_BIT(2)  /**< Identify requirement for allocation,
+                                            if passed address is not a null-pointer
+                                            then it will be used as a hint or direct
+                                            address for allocation depend */


looks like incomplete sentence

brminich · 2017-02-16T10:00:38Z

src/ucp/core/ucp_mm.c

@@ -142,14 +143,18 @@ static ucs_status_t ucp_mem_alloc(ucp_context_h context, size_t length,
         * name specified in the configuration.
         */
        num_mds = 0;
-        if (method == UCT_ALLOC_METHOD_MD) {
+        if ((method == UCT_ALLOC_METHOD_MD)) {


no need for extra brackets

brminich · 2017-02-16T10:03:20Z

src/ucp/core/ucp_mm.c

+    return flags;
+}
+
+/* Matrix of behavior


looks cool as a comment!

isn't it better to have this matrix in the ucp.h ?

@alex-mikheev I've added such kind of the table to ucp.h as a markdown table. It's unreadable in the code but should look fine in the browser. I decided to keep this one "as is" for development needs.

You might use @verbatim clause for this type of comments. Please look into uct_def.h

As I understand verbatim does not support hyperlinks, doesn't it? The resulting table includes links to all actual fields and flags.

brminich · 2017-02-16T10:15:08Z

src/ucp/core/ucp_mm.c

+
+    /* Now, lets check all erroneous cases from the matrix */
+    if (params->address == NULL) {
+        if (params->flags == 0) {


these checks can be reduced to smth like this:

if ((params->flags & UCP_MEM_MAP_FIXED) ||
!(params->flags & UCP_MEM_MAP_ALLOCATE)) {
return UCS_ERR_INVALID_PARAM;
}

brminich · 2017-02-16T10:17:09Z

src/ucp/core/ucp_mm.c

+            (params->flags & UCP_MEM_MAP_FIXED)) {
+            return UCS_ERR_INVALID_PARAM;
+        }
+    } else if (!(params->flags & UCP_MEM_MAP_ALLOCATE) &&


why is this an invalid param case?

brminich · 2017-02-16T10:35:41Z

test/gtest/ucp/test_ucp_memheap.cc

@@ -170,6 +173,10 @@ void test_ucp_memheap::test_blocking_xfer(blocking_send_func_t send,
        goto out;
    }

+    if (!malloc_allocate) {


can set it in the check above at line 162 (though flags needs to be initialized before that check)

MattBBaker · 2017-02-16T16:21:45Z

cc1plus: warnings being treated as errors
/var/lib/jenkins/jobs/UCX/workspace/contrib/../test/gtest/ucp/test_ucp_memheap.cc: In member function ‘void test_ucp_memheap::test_blocking_xfer(void (test_ucp_memheap::*)(ucp_test_base::entity*, size_t, void*, ucp_rkey*, std::string&), size_t, int, size_t, bool, bool)’:
/var/lib/jenkins/jobs/UCX/workspace/contrib/../test/gtest/ucp/test_ucp_memheap.cc:151: error: ‘memheap’ may be used uninitialized in this function
make[2]: *** [ucp/gtest-test_ucp_memheap.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: Leaving directory `/var/lib/jenkins/jobs/UCX/workspace/build-tests/test/gtest'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/UCX/workspace/build-tests'
make: *** [all] Error 2

It's because memheap doesn't get set until line 172, but in line 166 there is a 'goto out;' statement that will lead to a code path that passes memheap to free(), so at that point memheap won't be initialized. Setting memheap = NULL at initialization in line 151 will fix this.

evgeny-leksikov · 2017-02-16T17:14:51Z

@MattBBaker Thanks! This is a new error, the same as Mellanox testing shows, It should be fixed now. Yesterday's PR has something else (https://pod9.ccs.ornl.gov/job/UCX/865/)

yosefe · 2017-02-24T11:31:12Z

src/ucp/api/ucp.h

+ * @param [out] params      Defined @ref ucp_mem_map_params_t configurations
+ *                          for the @ref ucp_mem_h "UCP memory handle".
+ */
+ucs_status_t ucp_mem_query(const ucp_mem_h memh, ucp_mem_map_params_t *params);


the query function should return a new structure (not ucp_mem_map_params_t), according to the convention it would be ucp_mem_attr_t.

Do we really need this function ?

add @return

yosefe · 2017-02-24T11:31:55Z

src/ucp/core/ucp_mm.c

+
+static inline int ucp_mem_map_is_allocate(ucp_mem_map_params_t *params)
+{
+    ucs_assert(params != NULL);


don;t think this assert is needed

yosefe · 2017-02-24T11:32:52Z

src/ucp/core/ucp_mm.c

@@ -178,20 +178,96 @@ static ucs_status_t ucp_mem_alloc(ucp_context_h context, size_t length,
 }


+static inline unsigned uct_flags(ucp_mem_map_params_t *params)


pls rename the function

yosefe · 2017-02-24T11:33:04Z

src/ucp/core/ucp_mm.c

+{
+    unsigned flags = 0;
+
+    if (params && (params->field_mask & UCP_MEM_MAP_PARAM_FIELD_FLAGS)) {


no need to check for params!=NULL

yosefe · 2017-02-24T11:34:55Z

src/ucp/core/ucp_mm.c

+ */
+static inline ucs_status_t ucp_mem_map_check_and_adjust_params(ucp_mem_map_params_t *params)
+{
+    if (!params) {


no need to check this

yosefe · 2017-02-24T11:42:05Z

src/ucp/api/ucp.h

+    UCP_MEM_MAP_FIXED    = UCS_BIT(1), /**< Don't interpret addr as a hint:
+                                            place the mapping at exactly that
+                                            address. See man mmap for details */
+    UCP_MEM_MAP_ALLOCATE = UCS_BIT(2)  /**< Identify requirement for allocation,


pls reverse order, ALLOCATE flag should come before FIXED

It is not clear how UCP_MEM_MAP_NONBLOCK interacts with the UCP_MEM_MAP_ALLOCATE. You need to clarify if nonblock is good for mapping (registration), for allocation or for both

yosefe · 2017-02-24T11:45:12Z

src/ucp/api/ucp.h

+                                            address. See man mmap for details */
+    UCP_MEM_MAP_ALLOCATE = UCS_BIT(2)  /**< Identify requirement for allocation,
+                                            if passed address is not a null-pointer
+                                            then it will be used as a hint or direct


IIRC address-as-a-hint is not implemented by UCT today, right?

It's implemented, we added address argument to uct_mem_alloc function for that

yosefe · 2017-02-24T11:45:53Z

src/ucp/core/ucp_mm.c

+
+    /* First of all, define all fields */
+    if (!(params->field_mask & UCP_MEM_MAP_PARAM_FIELD_ADDRESS)) {
+        params->field_mask |= UCP_MEM_MAP_PARAM_FIELD_ADDRESS;


pls avoid changing the structure passed by user. Need to make the parameter to ucp_mem_map() a const*.

Fixed, now the copy user's structure is changing to simplify definition of default and erroneous cases

yosefe · 2017-02-24T11:46:03Z

src/ucp/core/ucp_mm.c

+        }
+    } else if (!(params->flags & UCP_MEM_MAP_ALLOCATE) &&
+               (params->flags & UCP_MEM_MAP_FIXED)) {
+            return UCS_ERR_INVALID_PARAM;


indentation

yosefe · 2017-02-24T11:47:10Z

src/ucp/core/ucp_mm.c

+    }
+
+    params->field_mask = 0;
+    return UCS_ERR_INVALID_PARAM;


this function should not return UCS_ERR_INVALID_PARAM

alex-mikheev · 2017-02-23T15:19:11Z

src/ucp/api/ucp.h

+                                            up-front, and mapping them later when
+                                            they are accessed by communication
+                                            routines. */
+    UCP_MEM_MAP_FIXED    = UCS_BIT(1), /**< Don't interpret addr as a hint:


^addr^address

alex-mikheev · 2017-02-26T09:26:24Z

src/ucp/api/ucp.h

+ * @param [out] params      Defined @ref ucp_mem_map_params_t configurations
+ *                          for the @ref ucp_mem_h "UCP memory handle".
+ */
+ucs_status_t ucp_mem_query(const ucp_mem_h memh, ucp_mem_map_params_t *params);


Do we really need this function ?

alex-mikheev · 2017-02-26T09:27:16Z

src/ucp/api/ucp.h

+ * @brief query mapped memory segment
+ *
+ * This routine returns valid address and length of memory segment mapped with
+ * @ref ucp_mem_map "ucp_mem_map()" routine.


remove 'valid'

alex-mikheev · 2017-02-26T09:28:45Z

src/ucp/api/ucp.h

+ * @param [out] params      Defined @ref ucp_mem_map_params_t configurations
+ *                          for the @ref ucp_mem_h "UCP memory handle".
+ */
+ucs_status_t ucp_mem_query(const ucp_mem_h memh, ucp_mem_map_params_t *params);


add @return

alex-mikheev · 2017-02-26T09:30:39Z

src/ucp/core/ucp_mm.c

+    return flags;
+}
+
+/* Matrix of behavior


isn't it better to have this matrix in the ucp.h ?

alex-mikheev · 2017-02-26T10:00:06Z

src/ucp/api/ucp.h

+    UCP_MEM_MAP_FIXED    = UCS_BIT(1), /**< Don't interpret addr as a hint:
+                                            place the mapping at exactly that
+                                            address. See man mmap for details */
+    UCP_MEM_MAP_ALLOCATE = UCS_BIT(2)  /**< Identify requirement for allocation,


It is not clear how UCP_MEM_MAP_NONBLOCK interacts with the UCP_MEM_MAP_ALLOCATE. You need to clarify if nonblock is good for mapping (registration), for allocation or for both

alex-mikheev · 2017-02-26T12:50:36Z

@evgeny-leksikov @yosefe
I want to clarify a usage pattern:

allocate memory with ALLOCATE flags. In return i qam getting a mem handle
map the memory. I am getting another mem handle

Is it OK to free handles in the reverse order ?

yosefe · 2017-02-26T12:53:11Z

@alex-mikheev in general there is no need to map a memory which was allocated by ucp.
However, if indeed you do it, you cannot release in reverse order. because releasing memh(1) would also release the memory, and you should not release memory before unmapping it [e.g release memh(2)]

alex-mikheev · 2017-02-26T12:57:20Z

The docs in this PR suggest that allocated memory is not mapped

yosefe · 2017-02-26T12:59:22Z

@evgeny-leksikov need to explicitly specify that memory allocated by ucp_mem_map() is available for RMAs and AMOs

evgeny-leksikov · 2017-02-27T12:04:18Z

@alex--m @yosefe it's described in existing docs for ucp_mem_map:

Map or allocate memory for zero-copy operations

and

If a large segment is registered, and then segmented for subsequent use by a user, then the user is responsible for segmentation and subsequent management

alex-mikheev · 2017-02-27T12:28:04Z

@evgeny-leksikov It is not clear if the allocated memory is also mapped. In fact based on the table in the comment one has a reason to think that allocated memory is not mapped/registered

evgeny-leksikov · 2017-02-27T13:59:33Z

@alex-mikheev I see your point. This table is a comment for internal function, not API, so it should not be considered as a user's documentation. Any way, I'll fix alloc -> alloc/reg int the table, that's OK?

yosefe · 2017-02-27T14:02:17Z

@evgeny-leksikov we need to put a more comprehensive explanation in the UCP level, can be a similar table (but something which looks good on doxygen), or something else which clearly explains what are the valid combinations of flags, and what would happen for each.

evgeny-leksikov · 2017-02-27T17:32:15Z

PR is updated to cover the most of comments and especially API, I'm going to update ucp_mem_map docs a bit later

yosefe · 2017-02-28T14:40:50Z

src/tools/perf/libperf.c


    status = ucp_mem_map(perf->ucp.context, &mem_map_params,
                         &perf->ucp.send_memh);
    if (status != UCS_OK) {
        goto err;
    }
-    perf->send_buffer = mem_map_params.address;
+    status = ucp_mem_query(perf->ucp.send_memh, &mem_attr);


pls add space line

yosefe · 2017-02-28T14:41:49Z

src/ucp/core/ucp_mm.c

+ucs_status_t ucp_mem_query(const ucp_mem_h memh, ucp_mem_attr_t *attr)
+{
+    if (memh->address) {
+        attr->field_mask = UCP_MEM_ATTR_FIELD_ADDRESS |


the field mask is an input parameter, specifying which fields should be filled by the structure.

i.e. attr should be in/out parameter?
This looks more flexible but requires a bit more actions from user.
And what the function should return if requested field, for example, is not available? UCS_ERR_UNSUPPORTED?

yes, see ucp_worker_query for example.
if not available - currently it's not handled correctly (good catch)
I guess we need to return ERR_INVALID_PARAM if any unexpected bit is received in attr.field_mask. but let's do it in another PR, and for all ucp_*_query() functions?

Ok, but I like UNSUPPORTED more from perspective that field_flags introduced for backward compatibility. This might be actual for ucp*params functions as well

yosefe · 2017-02-28T14:45:06Z

src/ucp/api/ucp_def.h

+    uint64_t                field_mask;
+
+    /**
+     * A virtual address.


Address of the memory segment

yosefe · 2017-02-28T14:45:43Z

src/ucp/api/ucp_def.h

@@ -115,6 +115,44 @@ typedef struct ucp_mem                   *ucp_mem_h;


 /**
+ * @ingroup UCP_MEM
+ * @brief Attributes of the @ref ucp_mem_h "UCP Memory handle", might be


"might be requested by" -> "filled by"

yosefe · 2017-02-28T14:46:57Z

src/ucp/core/ucp_mm.c

+    /* Now, lets check the rest of erroneous cases from the matrix */
+    if (params->address == NULL) {
+        if (!(params->flags & UCP_MEM_MAP_ALLOCATE)) {
+            return UCS_ERR_INVALID_PARAM;


shall we add error message, as in line 233?

yosefe · 2017-02-28T14:47:47Z

src/ucp/core/ucp_mm.c

+    ucp_mem_h               memh;
+    ucp_mem_map_params_t    mem_params;
+
+    if (!params) {


IMHO no need to check for params==NULL since we don't check it in any other function

yosefe · 2017-02-28T14:48:29Z

src/ucp/core/ucp_mm.c

-    if (params->address == NULL) {
-        status = ucp_mem_alloc(context, params->length, uct_flags, "user allocation", memh);
+    if (ucp_mem_map_is_allocate(&mem_params)) {
+        status = ucp_mem_alloc(context, mem_params.length,


let's add debug message like in line 303

yosefe · 2017-02-28T14:49:05Z

src/ucs/sys/sys.h

@@ -237,6 +237,12 @@ size_t ucs_get_page_size();


 /**
+ * @return 1 if ptr is aligned to page size on the system, otherwise 0
+ */
+int ucs_is_page_aligned(void *ptr);


need to remove

yosefe · 2017-02-28T14:49:53Z

test/gtest/ucp/test_ucp_fence.cc

+        EXPECT_TRUE(mem_attr.field_mask & (UCP_MEM_ATTR_FIELD_ADDRESS |
+                                           UCP_MEM_ATTR_FIELD_LENGTH));
+        EXPECT_LE(memheap_size, mem_attr.length);
+        if (!memheap) {


why not memheap = mem_attr.address unconditionally?

Because if memheap allocated by malloc then mem_attr.address may different in common case, I'm not sure that the test will work fine everywhere :) Are you suggesting to remove the check?

yosefe · 2017-02-28T14:50:54Z

test/gtest/ucp/test_ucp_fence.cc

-        ASSERT_UCS_OK(status);
-
-        free(memheap);
+        if (!(params.flags & UCP_MEM_MAP_ALLOCATE)) {


IMHO would be better to use GetParam().variant than params.flags

yosefe · 2017-02-28T14:52:41Z

@evgeny-leksikov can you please also squash the commits?

mellanox-github · 2017-02-28T17:19:01Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1069/ for details.

mellanox-github · 2017-02-28T17:51:49Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3021/ for details (Mellanox internal link).

evgeny-leksikov · 2017-02-28T19:59:45Z

All comments are addressed. @yosefe , @alex-mikheev , pls review

yosefe · 2017-03-01T09:31:32Z

@shamisp @bbenton @alex-mikheev pls review as well

bbenton · 2017-03-02T15:21:46Z

src/ucp/api/ucp.h

+ *                                         <td>@ref anch_alloc_hint_reg "alloc+register,hint"
+ *                                         <td>@ref anch_err "error"
+ *                                         <td>@ref anch_alloc_fixed_reg "alloc+register,fixed"
+ * </table>


When converted to pdf, this table has some formatting and alignment issues. Maybe using shorter names to print in the references to the parameter flags will help. If it could look more like the ASCII matrix in ucp_mm.c, I think that would help for readability.

@bbenton formatting for PDF is a pain.
The table is too wide without abbreviations even in ASCII and it doesn't support hyperlinks to provide good description and navigation.
I've prepared 3 different variants of the table, please take patch here and choose the best one from your point of view. If you would play with it and provide better formatting, it would be very appreciated!

Thanks for re-working the table and providing the 3 options. I prefer the 2nd option; I think it is much cleaner in the pdf. I have a couple of minor suggested modifications:

change the NONBLOCK column text from
affects only\n registration/mapping\n phase
to
only affects the\n register/map\n phase
this keeps the text from overflowing the column width

for the "address" column, modify the "1" value to "defined", as in table 3.

shamisp · 2017-03-02T15:23:12Z

src/ucp/core/ucp_mm.c

+    if ((params->flags & UCP_MEM_MAP_FIXED) &&
+        (!params->address ||
+         ((uintptr_t)params->address % ucs_get_page_size()))) {
+        ucs_error("UCP_MEM_MAP_FIXED flag requires page aligned address");


The requirement for fix address page alignment has to be explicitly document in UCP API.

shamisp · 2017-03-02T15:26:35Z

src/ucp/core/ucp_mm.c

-        /* TODO: pass addr */
-        status = uct_mem_alloc(NULL, length, uct_flags, &method, 1, mds, num_mds,
-                               name, &mem);
+        status = uct_mem_alloc(memh->address, length, uct_flags, &method, 1, mds,


mmap does not guarantee allocation with user defined address. Do we have an error code to propagate it to UCP later. The fact that we can't guarantee such allocation has to be explicitly mentioned in UCP documentation

@shamisp I'm not sure I see you point
man mmap for MAP_FIXED flag says me a bit different things: it's less portable, overlapped parts will be discarded, etc. but it does. If the addr can't be used, mmap will fail, so ucp_mem_map should return NO_MEMORY.

@shamisp the API doc says "use address as a hint". If the memory is allocated in a different address - user can find this using ucp_mem_query(), it is not considered an error situation.

return NO_MEMORY - answers the question

but the memory allocation did succeed, just not in address provided as hint..

okay...how we notify user about this ? We don't want a user to invoke the query every single allocation.

seems like there are 2 options:

return error code

return success, user does mem_query

I think (2) is better because:

similar behavior to system mmap

error status usually means no side effect happened, but here memory was allocated

user who wants to force specific address can use FIXED flag. otherwise, he has to query anyway to discover the real address.

I agree with the idea of following mmap semantics. If UCP_MEM_MAP_FIXED was specified UCP should fail and return an error.

mellanox-github · 2017-03-04T22:29:34Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1096/ for details.

mellanox-github · 2017-03-04T22:59:25Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3037/ for details (Mellanox internal link).

mellanox-github · 2017-03-05T23:25:33Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1106/ for details.

evgeny-leksikov · 2017-03-06T16:05:42Z

@shamisp looks like all issues are resolved. Do you have other comments?

shamisp · 2017-03-06T19:58:43Z

src/ucp/api/ucp.h

@@ -262,7 +262,8 @@ enum {
                                            address for allocation. */
    UCP_MEM_MAP_FIXED    = UCS_BIT(2)  /**< Don't interpret address as a hint:
                                            place the mapping at exactly that
-                                            address. */
+                                            address. Address must be a multiple


The address

@evgeny-leksikov once you make the chance consider that it is auto-approved by me :) but you have to get approval from @bbenton as well.

shamisp · 2017-03-06T20:22:42Z

@evgeny-leksikov - just one minor comment and we may call it done.

bbenton · 2017-03-07T02:17:50Z

👍 for doc/api

Signed-off-by:Evgeny Leksikov <evgenylek@mellanox.com>

evgeny-leksikov · 2017-03-07T08:09:19Z

Rebased on master to resolve conflicts

mellanox-github · 2017-03-07T08:35:42Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1121/ for details.

mellanox-github · 2017-03-07T09:06:33Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3055/ for details (Mellanox internal link).

evgeny-leksikov · 2017-03-07T10:46:41Z

bot:mlx:retest

mellanox-github · 2017-03-07T11:49:20Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3057/ for details (Mellanox internal link).

yosefe added the WIP-DNM Work in progress / Do not review label Feb 15, 2017

brminich reviewed Feb 16, 2017

View reviewed changes

evgeny-leksikov force-pushed the mmap_fixed_ucp branch from 12969b6 to ef88b59 Compare February 16, 2017 17:09

yosefe added API Feature New feature and removed WIP-DNM Work in progress / Do not review labels Feb 17, 2017

evgeny-leksikov force-pushed the mmap_fixed_ucp branch from ef88b59 to 6119655 Compare February 20, 2017 10:09

yosefe requested changes Feb 24, 2017

View reviewed changes

alex-mikheev suggested changes Feb 26, 2017

View reviewed changes

evgeny-leksikov force-pushed the mmap_fixed_ucp branch from 4200681 to 0ce81f9 Compare February 28, 2017 12:18

yosefe reviewed Feb 28, 2017

View reviewed changes

evgeny-leksikov force-pushed the mmap_fixed_ucp branch from 0ce81f9 to 3e51640 Compare February 28, 2017 16:50

yosefe approved these changes Mar 1, 2017

View reviewed changes

yosefe added the Ready for Review label Mar 1, 2017

alex-mikheev approved these changes Mar 1, 2017

View reviewed changes

bbenton reviewed Mar 2, 2017

View reviewed changes

shamisp requested changes Mar 2, 2017

View reviewed changes

shamisp reviewed Mar 6, 2017

View reviewed changes

evgeny-leksikov added 3 commits March 7, 2017 10:06

UCP/MEM: add mmap fixed support

1c85205

UCP/DOC: doc update for ucp_mem_map

08964f0

Signed-off-by:Evgeny Leksikov <evgenylek@mellanox.com>

UCP/DOC: fix description of UCP_MEM_MAP_FIXED flag

dd323f3

evgeny-leksikov force-pushed the mmap_fixed_ucp branch from f21bde4 to dd323f3 Compare March 7, 2017 08:08

shamisp approved these changes Mar 7, 2017

View reviewed changes

shamisp merged commit ce7508e into openucx:master Mar 7, 2017

evgeny-leksikov deleted the mmap_fixed_ucp branch March 7, 2017 15:24

		@@ -178,20 +178,96 @@ static ucs_status_t ucp_mem_alloc(ucp_context_h context, size_t length,
		}


		static inline unsigned uct_flags(ucp_mem_map_params_t *params)

UCP: add support fixed address for mmap #1329

UCP: add support fixed address for mmap #1329

Conversation

evgeny-leksikov commented Feb 15, 2017 • edited Loading

evgeny-leksikov commented Feb 16, 2017

brminich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MattBBaker commented Feb 16, 2017

evgeny-leksikov commented Feb 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-mikheev commented Feb 26, 2017

yosefe commented Feb 26, 2017 • edited Loading

alex-mikheev commented Feb 26, 2017

yosefe commented Feb 26, 2017

evgeny-leksikov commented Feb 27, 2017

alex-mikheev commented Feb 27, 2017

evgeny-leksikov commented Feb 27, 2017

yosefe commented Feb 27, 2017

evgeny-leksikov commented Feb 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yosefe commented Feb 28, 2017

mellanox-github commented Feb 28, 2017

mellanox-github commented Feb 28, 2017

evgeny-leksikov commented Feb 28, 2017

yosefe commented Mar 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamisp Mar 3, 2017 • edited Loading

Choose a reason for hiding this comment

evgeny-leksikov commented Feb 15, 2017 •

edited

Loading

yosefe commented Feb 26, 2017 •

edited

Loading

shamisp Mar 3, 2017 •

edited

Loading

yosefe Mar 3, 2017 •

edited

Loading