Skip to content

Conversation

amastbaum
Copy link
Contributor

What?

Iteratively receive netlink messages until 'NLMSG_DONE' flag is set.

Why?

https://redmine.mellanox.com/issues/4607309
In some cases, when many routing rules exist (many interfaces with multiple tables), the kernel may return those rules in chunks rather than in a single response.
When NLM_F_DUMP flag is set in the netlink request, it is actually necessary to perform multiple recv calls until an NLMSG_DONE message is received.

@amastbaum amastbaum requested review from yosefe and gleon99 September 3, 2025 11:57
@amastbaum amastbaum self-assigned this Sep 3, 2025
@amastbaum amastbaum removed the request for review from yosefe September 3, 2025 11:58
@amastbaum amastbaum force-pushed the vrf_ipv6_reachability branch from 1a829d3 to 19620b5 Compare September 9, 2025 12:30
@amastbaum amastbaum requested a review from gleon99 September 9, 2025 12:30
@amastbaum amastbaum requested a review from yosefe September 16, 2025 07:47

while ((status == UCS_INPROGRESS) && NLMSG_OK(nlh, msg_len) &&
(nlh->nlmsg_type != NLMSG_DONE)) {
(!is_dump || !(*is_done_p = UCS_NETLINK_IS_MESSAGE_DONE(nlh)))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we avoid passing int *is_done_p and is_dump to this function, and keep the logic of calling ucs_netlink_parse_msg several times in the upper function, ucs_netlink_send_request?

@amastbaum amastbaum requested a review from yosefe September 28, 2025 08:19
rtm.rtm_table = RT_TABLE_MAIN;
ucs_netlink_send_request(NETLINK_ROUTE, RTM_GETROUTE, NLM_F_DUMP, &rtm,
sizeof(rtm), ucs_netlink_parse_rt_entry_cb,
NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

why removed RT_TABLE_MAIN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because now we define it as RT_TABLE_UNSPEC (value 0) in order to fetch all the tables

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe set it explicitly to RT_TABLE_UNSPEC and comment the reason?

@amastbaum amastbaum requested a review from yosefe September 28, 2025 12:16
int netlink_fd = -1;
int is_message_done = 0;
int is_dump = nlmsg_flags & NLM_F_DUMP;
size_t recv_msg_len = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to ini

struct rtmsg rtm = {0};
ucs_netlink_route_info_t info;

rtm.rtm_table = RT_TABLE_UNSPEC; /* fetch all the tables */
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move it inside UCS_INIT_ONCE block? as it used to be

@amastbaum amastbaum requested a review from yosefe September 28, 2025 13:25
@amastbaum amastbaum force-pushed the vrf_ipv6_reachability branch from 5add2ce to 6d96bec Compare September 28, 2025 15:21
@yosefe yosefe enabled auto-merge September 29, 2025 05:44
@yosefe yosefe merged commit a0f89a0 into openucx:master Sep 29, 2025
141 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants