All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Donald Hunter <donald.hunter@gmail.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, Stanislav Fomichev <sdf@google.com>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>,
	donald.hunter@redhat.com
Subject: Re: [PATCH net-next v2 06/10] tools/net/ynl: Add support for netlink-raw families
Date: Thu, 17 Aug 2023 18:18:28 -0700	[thread overview]
Message-ID: <20230817181828.76ac2c11@kernel.org> (raw)
In-Reply-To: <m2cyzmhw50.fsf@gmail.com>

On Thu, 17 Aug 2023 10:10:35 +0100 Donald Hunter wrote:
> > Looks good, but do we also need some extra plumbing to decode extack
> > for classic netlink correctly?  Basically shouldn't _decode_extack()
> > also move to proto? Or we can parameterize it? All we really need there
> > is to teach it how much of fixed headers parser needs to skip to get to
> > attributes, really (which, BTW is already kinda buggy for genl families
> > with fixed headers).  
> 
> I have been working on the assumption that extack responses don't
> include any fixed headers. I have seen extack messages decoded correctly
> for classic netlink, here with RTM_NEWROUTE:
> 
> lib.ynl.NlError: Netlink error: Invalid argument
> nl_len = 80 (64) nl_flags = 0x300 nl_type = 2
>   error: -22  extack: {'msg': 'Invalid prefix for given prefix length'}
> 
> Is there something I am missing?

I'm thinking of extack messages carrying offsets in addition to the 
textual error message. NLMSGERR_ATTR_OFFS or NLMSGERR_ATTR_MISS_NEST.

In that case ynl will try to re-parse its own message via
_decode_extack_path() to resolve from the offset to what attribute
was there. See the commit message on a552bfa16:

    lib.ynl.NlError: Netlink error: Numerical result out of range
    nl_len = 108 (92) nl_flags = 0x300 nl_type = 2
            error: -34      extack: {'msg': 'integer out of range',...
                                     'bad-attr': '.ifindex'}

I mean the "bad-attr" thing.

I think it works out of sheer luck here, we happen to skip over 
the fixed header because it looks like a 0-length attribute?

  reply	other threads:[~2023-08-18  1:19 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-15 19:42 [PATCH net-next v2 00/10] tools/net/ynl: Add support for netlink-raw families Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 01/10] doc/netlink: Add a schema " Donald Hunter
2023-08-16  8:41   ` Simon Horman
2023-08-16 13:19     ` Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 02/10] doc/netlink: Document the genetlink-legacy schema extensions Donald Hunter
2023-08-16  2:49   ` Jakub Kicinski
2023-08-16  8:25     ` Donald Hunter
2023-08-16 13:16       ` Donald Hunter
2023-08-16 15:00         ` Jakub Kicinski
2023-08-15 19:42 ` [PATCH net-next v2 03/10] doc/netlink: Document the netlink-raw " Donald Hunter
2023-08-16  2:52   ` Jakub Kicinski
2023-08-16  8:32     ` Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 04/10] tools/ynl: Add mcast-group schema parsing to ynl Donald Hunter
2023-08-16 15:12   ` Jakub Kicinski
2023-08-15 19:42 ` [PATCH net-next v2 05/10] tools/net/ynl: Refactor decode_fixed_header into NlMsg Donald Hunter
2023-08-16 15:20   ` Jakub Kicinski
2023-08-17 15:14     ` Donald Hunter
2023-08-18  1:37       ` Jakub Kicinski
2023-08-18 10:21         ` Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 06/10] tools/net/ynl: Add support for netlink-raw families Donald Hunter
2023-08-16 15:29   ` Jakub Kicinski
2023-08-17  9:10     ` Donald Hunter
2023-08-18  1:18       ` Jakub Kicinski [this message]
2023-08-21 14:00         ` Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 07/10] tools/net/ynl: Implement nlattr array-nest decoding in ynl Donald Hunter
2023-08-16 15:31   ` Jakub Kicinski
2023-08-15 19:42 ` [PATCH net-next v2 08/10] doc/netlink: Add spec for rt addr messages Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 09/10] doc/netlink: Add spec for rt link messages Donald Hunter
2023-08-15 19:42 ` [PATCH net-next v2 10/10] doc/netlink: Add spec for rt route messages Donald Hunter

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=20230817181828.76ac2c11@kernel.org \
    --to=kuba@kernel.org \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=donald.hunter@redhat.com \
    --cc=edumazet@google.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.