Re: [PATCH net-next 2/4] tools: ynl: allow setting recv() size

[Donald Hunter <donald.hunter@gmail.com>]

Jakub Kicinski <kuba@kernel.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@kernel.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: