Skip to content

UCT/PLUGIN: add definition in uct_iface_attr_v2_t for uct_iface_query_v2#11316

Open
fteng-NV wants to merge 6 commits intoopenucx:masterfrom
fteng-NV:feature/uct-api-v2
Open

UCT/PLUGIN: add definition in uct_iface_attr_v2_t for uct_iface_query_v2#11316
fteng-NV wants to merge 6 commits intoopenucx:masterfrom
fteng-NV:feature/uct-api-v2

Conversation

@fteng-NV
Copy link
Copy Markdown

@fteng-NV fteng-NV commented Apr 1, 2026

What?

add definition in uct_iface_attr_v2_t for uct_iface_query_v2.

Why?

The definition is used for following interface implementation. This PR is seperated from PR #11277

@fteng-NV fteng-NV force-pushed the feature/uct-api-v2 branch from e768a2c to a6b8374 Compare April 1, 2026 11:22
* Caller allocates a buffer of @ref uct_iface_attr_v2_t::tx_token_length
* bytes and sets this pointer; callee fills the buffer with the token.
*/
void *tx_token;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra leading space — tx_token and rx_token are indented 5 spaces, but all other struct members use 4.

🤖 Generated by Claude

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified in 47c652c


/** Enables @ref uct_iface_attr_v2_t::tx_token_length (output). */
UCT_IFACE_ATTR_FIELD_TX_TOKEN_LENGTH = UCS_BIT(1),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blank line has trailing whitespace. Same issue on lines 1074, 1100, 1106, 1112, 1120 — will likely fail style/CI checks.

🤖 Generated by Claude

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified in 47c652c

* When set, @ref uct_iface_attr_v2_t::tx_token is input (from sender),
* and @ref uct_iface_attr_v2_t::rx_token is output (derived by receiver).
*/
UCT_IFACE_ATTR_FIELD_RX_TOKEN = UCS_BIT(3)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name is misleading — this single flag enables both tx_token (input) and rx_token (output). Consider UCT_IFACE_ATTR_FIELD_TOKEN_DERIVATION, or split into two separate bits.

🤖 Generated by Claude

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed into two flags UCT_IFACE_ATTR_FIELD_TX_TOKEN and UCT_IFACE_ATTR_FIELD_RX_TOKEN and updated related code comments in 47c652c

* the TX token received from the sender.
* @ref UCT_IFACE_ATTR_FIELD_RX_TOKEN must be set together.
*/
const void *tx_token;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check the CI output and fix the formatting?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modified in 74a60e6

@gleon99 gleon99 requested a review from evgeny-leksikov April 2, 2026 15:49
Comment on lines +1104 to +1108
/**
* Plugin-contributed capability flags (bitmask of UCT_IFACE_FLAG_*).
* Valid when @ref UCT_IFACE_ATTR_FIELD_FLAGS is set.
*/
uint64_t flags;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why "Plugin-contributed"? maybe return all supported capabilities?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modify comment in 0028fd6

/* Reserved for future use */
UCT_IFACE_ATTR_FIELD_RESERVED = 0
/** Enables @ref uct_iface_attr_v2_t::flags (output).
* Returns plugin-contributed capability flags.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why plugin?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in eb549d4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants