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

fix payload size of Netlink message in route_get() #99

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

Conversation

luznicky
Copy link

The len parameter of NLMSG_LENGTH was size of Netlink header instead of RtNetlink header.

Also make it more clear that the parameter of the macro is the payload size, which is RtNetlink message in this case.

The `len` parameter of `NLMSG_LENGTH`  was size of Netlink header instead of RtNetlink header.

Also make it more clear that the parameter of the macro is the payload size, which is RtNetlink message in this case.
@ofalk ofalk self-assigned this Jun 18, 2024
@ofalk
Copy link
Owner

ofalk commented Jun 18, 2024

Did you see any errors with the previous code? I'm not aware of any. Let me know why you believe the size is wrong please.

@luznicky
Copy link
Author

The description for NLMSG_LENGTH() is Given the payload length, len, this macro returns the aligned length to store in the nlmsg_len field of the nlmsghdr. In this case the payload is RtNetlink message. Therefore I expect that the len shoud be the size of RtNetlink header and RtNetlink payload size.

The Netlink message buffer contains [Netlink_hdr, Netlink_payload], where Netlink_payload is [RtNetlink hdr, RtNetlink_payload].

Also from the source code

#define 	NLMSG_LENGTH(len)   ((len)+NLMSG_ALIGN(NLMSG_HDRLEN))

In the code the macro expands to something like nlmsg_len = (2*NLMSG_HDRLEN) + RTA_LENGTH(alen). Which doesn't make much sense to me.

Hope it's understandable at lest from one of the tree explanation attempts.

Did you see any errors with the previous code?

No. Because because the size of Netlink header is greater than Rtnetlink header.

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