Message-ID: <2ea1e869-5f28-6beb-9fb1-15ae5a396077@solarflare.com>
Date:   Wed, 7 Nov 2018 19:29:31 +0000
From:   Edward Cree <ecree@...arflare.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Martin Lau <kafai@...com>, Yonghong Song <yhs@...com>,
        Alexei Starovoitov <ast@...com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and
 BTF_KIND_FUNC_PROTO

On 07/11/18 00:59, Alexei Starovoitov wrote:
> Function name and function argument names are part of the same debug info.
> Splitting them makes no sense.
... except where combining them involves creating pain elsewhere.
Sure the function name *could* go in the type record, but there
 still needs to be a separate function record in a functions
 table (because types are not instances), and that being the
 case the latter _may as well_ be where the name lives so that
 multiple functions (and pointers to them) can all share the
 same type record when the param names etc. all match.

> struct name and struct field names live in the same BTF record.
No, the struct _type_ name lives in the same record as the field
 names, but an _instance_ name doesn't.  I.e. in
    struct foo {int x;} bar;
 the BTF type record holds the names 'foo' and 'x', but not 'bar'
 because that's not the name of the _type_.  Indeed there isn't
 room in the record for both 'foo' and 'bar' because there's only
 one name_off field for the type.
And I argue that the name of a function is more like 'bar' than
 'foo' here, not least from the point of view of which C namespace
 they occupy.

> Similarly function name and function argument names should be
> in the same BTF record, so we can reuse most of the BTF validation
> and BTF parsing logic by doing so.
I think it's incredibly short-sighted to focus on 'what can most
 easily be done with the existing implementation' when designing a
 file format which is intended to have 'long legs'.

> assembler is not a high level language.
I never said it was.
> I believe it's a proper trade-off to make C easier to use
> in expense of some ugliness in your ebpf_asm.
Please respond to the arguments I make, rather than unrelated
 arguments that you might imagine me making.  Asm is merely a
cause of my present interest in BTF, it is not the lens
 through which I see the whole thing.

> Let's keep 'nasty hack' claims out of this discussion.
> I find the current BTF design and KIND_FUNC addition to be elegant
> and appropriate.
Whereas I don't, and I don't feel like my core criticisms have
 been addressed _at all_.  The only answer I get to "BTF should
 store type and instance information in separate records" is
 "it's a debuginfo", no indication of why that's a meaningful
 noun let alone why it implies they should be conflated in the
 format.
And please explain what's "elegant" about how map types are
 currently handled.

> BTF is not *type* only format. It's debug info format.
> Trying to make BTF into type only is not going to work.
> It's already more than type only as I showed earlier.
Again, as I have *repeatedly* said, I am not trying to remove
 non-type information from BTF.  I am just trying to organise
 BTF to consist of separate _parts_ for types and instances,
 rather than forcing both into the same Procrustean bed.

(I don't feel like we're making progress in understanding one
 another here; maybe we should have a telephone discussion?
 Sadly I'm not going to Plumbers, else that would be the
 perfect place to thrash this out.)

-Ed

Powered by blists - more mailing lists