netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
To: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Cc: Linux Network Development Mailing List <netdev@vger.kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net v2] neighbour: add RTNL_FLAG_DUMP_SPLIT_NLM_DONE to RTM_GETNEIGH
Date: Sun, 16 Jun 2024 10:09:07 +0200	[thread overview]
Message-ID: <CANP3RGeENFk0RFD2m1kBuOJxdAhKEjR=9caokkKah35py5kXbg@mail.gmail.com> (raw)
In-Reply-To: <20240615113224.4141608-1-maze@google.com>

For the other patch, I've tracked down:
  32affa5578f0 ("fib: rules: no longer hold RTNL in fib_nl_dumprule()")
which causes half the regression.

But... I haven't figured out what causes the final half (or third
depending on how you look at it).

I've also spent quite a while trying to figure out what exactly is
going wrong in the python netlink parsing code.
The code leaves a *lot* to be desired...

Turns out it doesn't honour the nlmsghdr.length field of NLMSG_DONE
messages, so it only reads the header (16 bytes) instead of the kernel
generated 20=16+4 NULL bytes.  I'm not sure why those extra 4 bytes
are there, but they are... (anyone know?)
This results in a leftover 4 bytes, which then fail to parse as
another nlmsghdr (because it also effectively ignores that it's a DONE
and continues parsing).
Which explains the failure:
  TypeError: NLMsgHdr requires a bytes object of length 16, got 4

Fixing the parsing, results in things hanging, because we ignore the DONE.

Fixing that... causes more issues (or I'm still confused about how the
rest works, it's hard to follow, complicated by python's lack of types
and some apparently dead code).

Ultimately I think the right answer is to simply fix the horribly
broken netlink parser, which only ever worked by (more-or-less)
chance.  We have plenty of time (months) to fix it in time for the
next release of Android after 15/V, which will be the first one to
support a kernel newer than 6.6 LTS anyway.

Furthermore, the python netlink parser is only used in the test
framework, while the non-test code itself uses C++& java netlink
parsers (that I have not yet looked at) but is likely to either work
or contain entirely different classes of bugs ;-)

  reply	other threads:[~2024-06-16  8:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-15 11:32 [PATCH net v2] neighbour: add RTNL_FLAG_DUMP_SPLIT_NLM_DONE to RTM_GETNEIGH Maciej Żenczykowski
2024-06-16  8:09 ` Maciej Żenczykowski [this message]
2024-06-17 16:36   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANP3RGeENFk0RFD2m1kBuOJxdAhKEjR=9caokkKah35py5kXbg@mail.gmail.com' \
    --to=zenczykowski@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).