All of lore.kernel.org
 help / color / mirror / Atom feed
From: Donald Hunter <donald.hunter@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org,  "David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	 Paolo Abeni <pabeni@redhat.com>,
	donald.hunter@redhat.com
Subject: Re: [RFC net-next v1] tools: ynl: Add an strace rendering mode to ynl-gen
Date: Fri, 16 Jun 2023 11:17:25 +0100	[thread overview]
Message-ID: <m2o7lfhft6.fsf@gmail.com> (raw)
In-Reply-To: <20230615200036.393179ae@kernel.org> (Jakub Kicinski's message of "Thu, 15 Jun 2023 20:00:36 -0700")

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 15 Jun 2023 16:13:36 +0100 Donald Hunter wrote:
>> Add --mode strace to ynl-gen-c.py to generate source files for strace
>> that teach it to understand how to decode genetlink messages defined
>> in the spec. I successfully used this to add openvswitch message
>> decoding to strace as I described in:
>> 
>> https://donaldh.wtf/2023/06/teaching-strace-new-tricks/
>> 
>> It successfully generated ovs_datapath and ovs_vport but ovs_flow
>> needed manual fixes to fix code ordering and forward declarations.
>> 
>> Limitations:
>> 
>> - Uses a crude mechanism to try and emit functions in the right order
>>   which fails for ovs_flow
>
> What's the dependency? I pushed some stuff recently to try to order
> types more intelligently but for normal C netlink it still won't deal
> with cycles :(

For strace I need to emit attr decoder functions before referencing them
in dispatch tables. The crude mechanism I used was to emit decoders for
nested attributes first, which worked okay for e.g. ovs_vport. But
ovs_flow has I think at least 1 cycle.

> Actually I think that you're using raw family info rather than the
> codegen-focused structs, maybe that's why?

Yes, that's a fair point. I'm just walking through the declared
attribute-sets in the order defined in the schema. I can take a look at
what the codegen-focused structs provide.

>> - Outputs all strace sources to stdout or a single file
>> - Does not use the right semantic strace decoders for e.g. IP or MAC
>>   addresses because there is no schema information to say what the
>>   domain type is.
>
> The interpretation depends on another attribute or we expose things 
> as binary with no machine-readable indication if its IP addr or MAC etc?

Yeah, it's the lack of machine-readable indication. I'd suggest adding
something like 'format: ipv4-address' to the schema.

>> This seems like a useful tool to have as part of the ynl suite since
>> it lowers the cost of getting good strace support for new netlink
>> families. But I realise that the generated format is dependent on an
>> out of tree project. If there is interest in having this in-tree then
>> I can clean it up and address some of the limitations before
>> submission.
>
> I think it's fine, we'll have to cross this bridge sooner or later.
> I suspect we'll need to split ynl-gen-c once again (like the
> tools/net/ynl/lib/nlspec.py, maybe we need another layer for code 
> generators? nlcodegen or some such?) before we add codegen for more
> languages. I'm not sure you actually need that yet, maybe the strace
> generator needs just nlspec.py and it can be a separate script?

The strace generator uses CodeWriter and makes partial use of the Type*
classes as well. If we split those out of ynl-gen-c then it could be a
separate script. A first step could be to move all but main() into a
lib?

  reply	other threads:[~2023-06-16 10:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-15 15:13 [RFC net-next v1] tools: ynl: Add an strace rendering mode to ynl-gen Donald Hunter
2023-06-15 17:19 ` Simon Horman
2023-06-16  3:00 ` Jakub Kicinski
2023-06-16 10:17   ` Donald Hunter [this message]
2023-06-16 18:11     ` Jakub Kicinski
2023-06-19 10:04       ` Donald Hunter
2023-06-19 19:00         ` Jakub Kicinski
2023-06-23 12:04           ` Donald Hunter
2023-06-23 15:28             ` 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=m2o7lfhft6.fsf@gmail.com \
    --to=donald.hunter@gmail.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@redhat.com \
    --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 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.