Re: [RFC net-next 4/4] ynl: add a sample user for ethtool
From: Jakub Kicinski <kuba@kernel.org>
Date: 2022-08-11 23:31:17
Also in:
linux-doc
On Thu, 11 Aug 2022 15:55:44 -0700 Stanislav Fomichev wrote:
quoted
We could, I guess. To be clear this controls the count, IOW: enum { PREFIX_A_BLA_ATTR = 1, PREFIX_A_ANOTHER_ATTR, PREFIX_A_AND_ONEMORE, __PFREIX_A_CNT, // <--- This thing }; #define PREFIX_A_MAX (__PFREIX_A_CNT - 1) It's not used in the generated code, only if we codegen the uAPI, AFAIR. So we'd need a way to tell the generator of the uAPI about the situation, anyway. I could be misremembering.My worry is that we'll have more hacks like these and it's hard, as a spec reader/writer, to figure out that they exist.. So I was wondering if it's "easier" (from the spec reader/writer pov) to have some c-header-fixup: section where we can have plain c-preprocessor hacks like these (where we need to redefine something to match the old behavior).
Let me think about it some more. My main motivation is people writing new families, I haven't sent too much time worrying about the existing ones with all their quirks. It's entirely possible that the uAPI quirks can just go and we won't generate uAPI for existing families as it doesn't buy us anything.
Coming from stubby/grpc, I was expecting to see words like
message/field/struct. The question is what's more confusing: sticking
with netlink naming or trying to map grpc/thrift concepts on top of
what we have. (I'm assuming more people know about grpc/thrift than
netlink)
messages: # or maybe 'attribute-sets' ?
- name: channels
...Still not convinced about messages, as it makes me think that every "space" is then a definition of a message rather than just container for field definitions with independent ID spaces. Attribute-sets sounds good, happy to rename. Another thought I just had was to call it something like "data-types" or "field-types" or "type-spaces". To indicate the split into "data" and "actions"/"operations"?
operations:
- name: channel_get
message: channels
do:
request:
fields:
- header
- rx_max
Or maybe all we really need is a section in the doc called 'Netlink
for gRPC/Thrift users' where we map these concepts:
- attribute-spaces (attribute-sets?) -> messages
- attributes -> fieldsExcellent idea!
quoted
Dunno, that'd mean that the Python method is called ETHTOOL_MSG_CHANNELS_GET rather than just channels_get. I don't want to force all languages to use the C naming. The C naming just leads to silly copy'n'paste issues like f329a0ebeab.Can we have 'name:' and 'long-name:' or 'c-name:' or 'full-name' ? - name: header attributes: - name: dev_index full-name: ETHTOOL_A_HEADER_DEV_INDEX val: type: Suppose I'm rewriting my c application from uapi to some generated (in the future) python-like channels_get() method. If I can grep for ETHTOOL_MSG_CHANNELS_GET, that would save me a bunch of time figuring out what the new canonical wrapper is. Also, maybe, at some point we'll have: - name: dev_index c-name: ETHTOOL_A_HEADER_DEV_INDEX java-name: headerDevIndex
Herm, looking at my commits where I started going with the C codegen
(which I haven't posted here) is converting the values to the same
format as keys (i.e. YAML/JSON style with dashes). So the codegen does:
c_name = attr['name']
if c_name in c_keywords:
c_name += '_'
c_name = c_name.replace('-', '_')
So the name would be "dev-index", C will make that dev_index, Java will
make that devIndex (or whatever) etc.
I really don't want people to have to prefix the names because that's
creating more work. We can slap a /* header.dev_index */ comment in
the generated uAPI, for the grep? Dunno..
quoted
Good catch, I'm aware. I was planning to add a "header constants" section or some such. A section in "headers" which defines the constants which C code will get from the headers.Define as in 're-define' or define as in 'you need to include some other header for this to work'? const: - name: ALTIFNAMSIZ val: 128
This one. In most cases the constant is defined in the same uAPI header as the proto so we're good. But there's IFNAMSIZ and friends which are shared.
which then does #ifndef #define ALTIFNAMSIZ 128 #else static_assert(ALTIFNAMSIZ == 128) #endif ? or: external-const: - name: ALTIFNAMSIZ header: include/uapi/linux/if.h which then might generate the following: #include <include/uapi/linux/if.h> #ifndef ALTIFNAMSIZ #error "include/uapi/linux/if.h does not define ALTIFNAMSIZ" #endifquoted
For Python it does not matter, as we don't have to size arrays.Hm, I was expecting the situation to be the opposite :-) Because if you really have to know this len in python, how do you resolve ALTIFNAMSIZ?
Why does Python need to know the length of the string tho? On receive if kernel gives you a longer name - great, no problem. On send the kernel will tell you so also meh. In C the struct has a char bla[FIXED_SIZE] so if we get an oversized string we're pooped, that's my point, dunno what other practical use the string sizing has.
The simplest thing to do might be to require these headers to be hermetic (i.e., redefine all consts the spec cares about internally)?
That's what I'm thinking if they are actually needed. But it only C cares we can just slap the right includes and not worry. Dunno if other languages are similarly string-challenged.