ucx icon indicating copy to clipboard operation
ucx copied to clipboard

UCT/API: fixup for uct_ep_connect_to_ep_v2

Open hoopoepg opened this issue 3 years ago • 6 comments

What

Fixed backward compatibility issue

How ?

  • due to compatibility issue updated uct_ep_connect_to_ep_v2 routine parameters
  • added uct_ep_connect_to_ep_param_field_t enum to add ep_address length
  • iface address will not be used in ep_connect_to_ep routine

hoopoepg avatar Sep 12 '22 13:09 hoopoepg

@yosefe ok to merge?

hoopoepg avatar Sep 19 '22 06:09 hoopoepg

@shamisp @tonycurtis can you pls review?

yosefe avatar Sep 19 '22 07:09 yosefe

Can you please provide additional details about backward compatibility issue ?

shamisp avatar Sep 20 '22 19:09 shamisp

Can you please provide additional details about backward compatibility issue ?

this is issue in wire compatibility. we were going to extend rc_x iface address flags by new bits, but we found that in older versions of UCX there is exact comparing of flags with some values and adding non-zero bits to address bytes breaks compatibility

hoopoepg avatar Sep 20 '22 21:09 hoopoepg

https://github.com/openucx/ucx/pull/8352 - this is regression PR

hoopoepg avatar Sep 21 '22 10:09 hoopoepg

It appeared that we can't extend iface address, like it is done in #8352 without breaking wire compatibility. So the new approach is to introduce new API which also accepts address length, so that the transport can determine (in compatible way) whether the address contains an extension

brminich avatar Sep 21 '22 14:09 brminich

/azp run

yosefe avatar Sep 25 '22 09:09 yosefe

Azure Pipelines successfully started running 3 pipeline(s).

azure-pipelines[bot] avatar Sep 25 '22 09:09 azure-pipelines[bot]