[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m2le6yjkga.fsf@gmail.com>
Date: Mon, 04 Mar 2024 11:38:29 +0000
From: Donald Hunter <donald.hunter@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
pabeni@...hat.com, jiri@...nulli.us
Subject: Re: [PATCH net-next 2/4] tools: ynl: allow setting recv() size
Jakub Kicinski <kuba@...nel.org> writes:
> Make the size of the buffer we use for recv() configurable.
> The details of the buffer sizing in netlink are somewhat
> arcane, we could spend a lot of time polishing this API.
> Let's just leave some hopefully helpful comments for now.
> This is a for-developers-only feature, anyway.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> tools/net/ynl/lib/ynl.py | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py
> index 92ade9105f31..bc5a526dbb99 100644
> --- a/tools/net/ynl/lib/ynl.py
> +++ b/tools/net/ynl/lib/ynl.py
> @@ -84,6 +84,10 @@ from .nlspec import SpecFamily
> return f"Netlink error: {os.strerror(-self.nl_msg.error)}\n{self.nl_msg}"
>
>
> +class ConfigError(Exception):
> + pass
> +
> +
> class NlAttr:
> ScalarFormat = namedtuple('ScalarFormat', ['native', 'big', 'little'])
> type_formats = {
> @@ -400,7 +404,8 @@ genl_family_name_to_id = None
>
>
> class YnlFamily(SpecFamily):
> - def __init__(self, def_path, schema=None, process_unknown=False):
> + def __init__(self, def_path, schema=None, process_unknown=False,
> + recv_size=131072):
> super().__init__(def_path, schema)
>
> self.include_raw = False
> @@ -423,6 +428,16 @@ genl_family_name_to_id = None
> self.async_msg_ids = set()
> self.async_msg_queue = []
>
> + # Note that netlink will use conservative (min) message size for
> + # the first dump recv() on the socket, our setting will only matter
> + # from the second recv() on.
> + self._recv_size = recv_size
> + # Netlink will always allocate at least PAGE_SIZE - sizeof(skb_shinfo)
> + # for a message, so smaller receive sizes will lead to truncation.
> + # Note that the min size for other families may be larger than 4k!
> + if self._recv_size < 4000:
> + raise ConfigError()
Nit: You've added this between the declaration of async_msg_ids and
where it gets populated. Otherwise LGTM.
> +
> for msg in self.msgs.values():
> if msg.is_async:
> self.async_msg_ids.add(msg.rsp_value)
> @@ -799,7 +814,7 @@ genl_family_name_to_id = None
> def check_ntf(self):
> while True:
> try:
> - reply = self.sock.recv(128 * 1024, socket.MSG_DONTWAIT)
> + reply = self.sock.recv(self._recv_size, socket.MSG_DONTWAIT)
> except BlockingIOError:
> return
>
> @@ -854,7 +869,7 @@ genl_family_name_to_id = None
> done = False
> rsp = []
> while not done:
> - reply = self.sock.recv(128 * 1024)
> + reply = self.sock.recv(self._recv_size)
> nms = NlMsgs(reply, attr_space=op.attr_set)
> for nl_msg in nms:
> if nl_msg.extack:
Powered by blists - more mailing lists