On Mon, Sep 24, 2018 at 09:19:06PM -0600, David Ahern wrote: > On top of net-next I am see a dmesg error: > > netlink: 16 bytes leftover after parsing attributes in process `ip'. > > I traced it to address lists and commit: > > commit 6ecf4c37eb3e89b0832c9616089a5cdca3747da7 > Author: Christian Brauner <christ...@brauner.io> > Date: Tue Sep 4 21:53:50 2018 +0200 > > ipv6: enable IFA_TARGET_NETNSID for RTM_GETADDR > > Per the commit you are trying to guess whether the ancillary header is > an ifinfomsg or a ifaddrmsg. I am guessing you are guessing wrong. :-)
Well, I currently don't guess at all. :) I'm parsing with struct ifaddrmsg as assumed header size but ignore parsing errors when that fails. You don't get the niceties of the new property if you don't pack it up nicely in an ifaddrmsg struct. :) > > I don't have time to take this to ground, but address listing is not the > only area subject to iproute2's SNAFU of infomsg everywhere on dumps. I > have thought about this for route dumps, but its solution does not work > here. You'll need to find something because the current warning on every > address dump is not acceptable. Two points before I propose a migitation: 1. The burded of seeing pr_warn_ratelimited() messages in dmesg when userspace is doing something wrong is imho justifiable. Actually, I would argue that we should not hide the problem from userspace at all. The rate-limited (so no logging DOS afaict) warning messages are a perfect indicator that a tool is doing something wrong *without* introducing any regressions. The rtnetlink manpage clearly indicates that ifaddrmsg is supposed to be used too. Additionally, userspace stuffs an ifinfomsg in there but expects to receive ifaddrmsg. They should be warned loudly. :) So I actually like the warning messages. 2. Userspace should be fixed. Especially such an important standard tool as iproute2 that is maintained on git.kernel.org (glibc is already doing the right.). So if people really want to hide this issue as much as we can then we can play the guessing game. I could send a patch that roughly does the following: if (nlmsg_len(cb->nlh) < sizeof(struct ifinfomsg)) guessed_header_len = sizeof(struct ifaddrmsg); else guessed_header_len = sizeof(struct ifinfomsg); This will work since sizeof(ifaddrmsg) == 8 and sizeof(ifinfomsg) == 16. The only valid property for RTM_GETADDR requests is IFA_TARGET_NETNSID. This propert is a __s32 which should bring the message up to 12 bytes (not sure about alignment requiremnts and where we might wend up ten) which is still less than the 16 bytes without that property from ifinfomsg. That's a hacky hacky hack-hack and will likely work but will break when ifaddrmsg grows a new member or we introduce another property that is valid in RTM_GETADDR requests. It also will not work cleanly when users stuff additional properties in there that are valid for the address family but are not used int RTM_GETADDR requests. I would like to hear what other people and davem think we should do. Patch it away or print the warning. Christian
signature.asc
Description: PGP signature