Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue: 4223310 VMA support for kernel 6.10 #1100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tomerdbz
Copy link
Collaborator

@tomerdbz tomerdbz commented Dec 23, 2024

Kernel 6.10 netlink has breaked VMA functionality.
Transitioned to libnl - an abstraction that wraps netlink.

This both solves the issue and makes us more robust.

What

Build routing and rules table using libnl instead of netlink.
removed protocol field in rule_val as:

  1. it wasn't in use.
  2. the api to populate it in libnl - rtnl_rule_get_protocol isn't available in sles on the CI.

Why ?

solves https://redmine.mellanox.com/issues/4223310

How ?

Tested on kernel 5.21 and kernel 6.10.

Change type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Check list

  • Code follows the style de facto guidelines of this project
  • Comments have been inserted in hard to understand places
  • Documentation has been updated (if necessary)
  • Test has been added (if possible)

@tomerdbz tomerdbz requested a review from galnoam December 23, 2024 12:28
@tomerdbz tomerdbz changed the title VMA support for kernel 6.10 issue: 4223310 VMA support for kernel 6.10 Dec 23, 2024
@tomerdbz tomerdbz requested a review from igor-ivanov December 23, 2024 16:21
@tomerdbz tomerdbz force-pushed the route_to_libnl branch 2 times, most recently from 55fa392 to 7a2cf7f Compare December 23, 2024 16:23
__log_err("NL socket Connection: ");
nl_socket_free(m_sock);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

formatting

Copy link
Collaborator

@igor-ivanov igor-ivanov Dec 23, 2024

Choose a reason for hiding this comment

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

please consider m_sock = nullptr; and following

	if (nl_connect(m_sock, NETLINK_ROUTE) < 0) {
		__log_err("NL socket Connection: ");
                nl_socket_free(m_sock);
               m_sock = nullptr;
              return;
    }

to be consistent and return at the end just in case good flow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

super right. fixed. Thanks :)

*p_ent_num = entry_cnt;
}, &iterator_context);

m_tab.entries_num = entry_cnt;
Copy link
Collaborator

Choose a reason for hiding this comment

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

using this approach probably it is better insert sanity check inside this function

if (m_tab.entries_num >= MAX_TABLE_SIZE) {
		__log_warn("reached the maximum route table size");
	}

so that initialization and verification are performed in one place

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good idea. fixed

char *oif_name = rtnl_rule_get_oif(rule);
if (oif_name) {
p_val->set_oif_name(oif_name);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

please consider adapting your IDE for product format style.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! didn't have clang format applied for this repo. not a native dev here :D
went through all the files and applied formatting

Kernel 6.10 netlink has breaked VMA functionality.
Transitioned to libnl - an abstraction that wraps netlink.

Signed-off-by: Tomer Cabouly <[email protected]>
@tomerdbz
Copy link
Collaborator Author

tomerdbz commented Jan 5, 2025

git patch of this PR:
kernel_6_10.patch

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.

2 participants