<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 8/30/21 9:14 AM, James Y Knight
wrote:<br>
</div>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">In my opinion, this sounds like a hack to work
around a problem that might be better solved outside the
compiler. Or, at the very least, I think some more justification
as to why this is the best solution would be helpful.
<div>
<div><br>
</div>
<div>Restating the problem:</div>
<div>
<div>You have a tool which generates a "vmlinux.h" header
file based on the currently-running kernel (e.g. by
parsing <span style="font-variant-ligatures:no-common-ligatures;color:rgb(0,0,0);font-family:Menlo;font-size:11px">/sys/kernel/btf/vmlinux</span>). This
allows users to avoid needing a copy of the kernel header
files, which may not be readily available. However, the
program cannot extract #defines -- only types -- because
defines aren't recorded in the kernel BTF debug info. So,
despite having an easy way to generate vmlinux.h, users do
still want to include the actual kernel headers sometimes,
in order to access these constants. You want to allow
that.</div>
</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>Correct.</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div>Your proposed solution is:</div>
<div>Modify the compiler so that it can ignore the duplicate
struct definitions, in order to allow users to include both
the kernel headers and the generated vmlinux.h header, even
though they don't cooperate with each-other.</div>
<div><br>
</div>
<div>But -- is that really necessary? It seems a bit odd to
invent a compiler feature to work around non-cooperating
headers. Even more so when all the headers are part of the
same project.</div>
</div>
</div>
</blockquote>
<p><br>
</p>
<p>We knew about this limitation from day 1 (and couldn't do much
about it with the language features we have at our disposal), and
it is brought up pretty regularly by various users, so it's a
pretty annoying problem which we haven't found *any* way to solve
satisfactory, except with extending Clang to help with it.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div><br>
</div>
<div>Here's some ideas of possible alternatives. You may
well have already thought of these and rejected them, but if
so, it would be nice to hear why.</div>
<div><br>
</div>
<div>1) Modify the original headers to have appropriate
#ifndefs, in order to avoid conflicting.<br>
</div>
</div>
</div>
</blockquote>
<p>Kernel community won't allow adding random #ifndefs into kernel
headers (neither UAPI nor internal, don't even know which one
would get more opposition), for the sake of BPF user applications.
It's also unclear what #ifndefs you mean. Is the proposal to have,
for each kernel type, a corresponding #define, e.g., #define
__task_struct__defined for struct task_struct, and add
#ifndef/#endif block for each such type? If yes, I don't see how
that will fly with kernel community (and also consider that
typical kernel configuration defines >80000 types).<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div><br>
</div>
<div>
<div>2) Tell users that they can either include either the
autogenerated "vmlinux.h", OR set the pragma and include
normal kernel headers. But not both. AFAICT, the vmlinux.h
file is not really version-independent -- except that it
uses the "preserve_access_index" pragma in order to allow
field accesses to be adjusted at load-time to match the
actual struct layout. Doesn't putting the same pragma
around the normal kernel headers work, too?</div>
</div>
</div>
</div>
</blockquote>
<p>That's what we are telling users right now. And that's what users
are complaining about. vmlinux.h is sufficiently
version-independent with BPF CO-RE, unless some fields are
removed/renamed, or you need the very latest new field added in
more recent kernel than the one you used to generate vmlinux.h.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div><br>
</div>
<div>3) Automatically extract #defines from kernel headers
(e.g. using -dD to print all macro-expansions) to create a
new header with those values. (Or defines and inline
functions. Or everything but struct definitions. Or
something along those lines). Have people include that,
instead.<br>
</div>
</div>
<div><br>
</div>
<div>
<div>...Although, IIUC, you want use the kernel-internal
headers e.g. "include/linux/socket.h", not the stable
"uapi" headers, e.g. "include/uapi/linux/socket.h". But
can't those defines change arbitrarily? Doesn't that ruin
the version-independence aspect? So, maybe you want:</div>
</div>
</div>
</div>
</blockquote>
<p>Yes, users often need to work with types defined in
kernel-internal headers. You are right that internal #defines are
"unstable", so users are usually interested in stable #defines in
UAPI headers. The problem is that vmlinux.h is all or nothing
approach, currently. If you use vmlinux.h (because you need
internal types, not just UAPI ones), you can't include UAPI
headers (for those #defines) anymore due to type conflicts. That's
exactly what Yonghong is trying to solve here.<br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div><br>
</div>
</div>
<div>4) Enhance BPF/CO:RE to support defines as
compile-time-unknown/runtime-constants [at least some useful
subset of them]. Then they too can be fixed up at BPF
load-time, instead of hoping that the values never change.<br>
</div>
</div>
</div>
</blockquote>
<p>#defines are not relocatable at runtime by any means, because
they are just arbitrary text substituted by compiler. Once Clang
compiled BPF program into object file, there is nothing you can do
about #defines used. I don't see any way for BPF CO-RE to do
"relocatable" #defines.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>Though, the example you give is "TCP" (IPPROTO_TCP?) which <i>is</i> in
the uapi. Maybe you do need only the stable uapi defines? In
which case,<br>
</div>
<div><br>
</div>
<div>
<div>5) Create a set of modified uapi headers for use with
bpf, which omits struct definitions, and instead has
#include vmlinux.h at the top instead. Distribute with
libbpf-dev, perhaps?<br>
</div>
</div>
</div>
</blockquote>
<p>That's not really a solution, rather a maintenance nightmare.
It's never a good idea to maintain a separate copy of something
that's developed by thousands of developers, and have hope to keep
up with all the changes. If that's the only possible solution,
then going back to status quo (making this user's problem and
telling them no to use vmlinux.h at all) is a less painful way.<br>
</p>
<p><br>
</p>
<blockquote type="cite" cite="mid:CAA2zVHq-uMrf8OHFjWzu6fxJHBsYv3WDbf02KNc26wAgWTfwfg@mail.gmail.com">
<div dir="ltr">
<div>
<div><br>
</div>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Mon, Aug 23, 2021 at 7:17
PM Y Song via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">cfe-dev@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
This is a BPF use case. We would like to check whether<br>
__attribute__((weak)) support for non-primitive types would be<br>
possible in clang or not. For example, if we have two types
like<br>
struct t { int a; } __attribute__((weak));<br>
struct t { int a; };<br>
<br>
typedef unsigned __u32;<br>
typedef unsigned __u32 __attribute__((weak));<br>
the compilation should not fail. It should just ignore weak
definitions.<br>
<br>
In BPF, to support CO-RE (compile once and run everywhere), we<br>
generate a *generic* vmlinux.h<br>
(<a href="https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html" rel="noreferrer" target="_blank" moz-do-not-send="true">https://facebookmicrosites.github.io/bpf/blog/2020/02/19/bpf-portability-and-co-re.html</a>)<br>
which is included in the bpf program. The bpf loader (libbpf)
will<br>
adjust field offsets properly based on host record layout so
the same<br>
program can run on different kernels which may have different
record<br>
layouts.<br>
<br>
But vmlinux.h does not support macros (and simple static
inline helper<br>
functions in kernel header files). Currently, users need to
define<br>
these macros and static inline functions explicitly in their
bpf<br>
program. What we are thinking is to permit users to include
kernel<br>
header files containing these macros/static inline functions.
But this<br>
may introduce duplicated types as these types have been
defined in<br>
vmlinux.h.<br>
<br>
For example, we may have:<br>
socket.h:<br>
struct socket { ... };<br>
#define TCP 6<br>
vmlinux.h:<br>
struct socket { ... };<br>
bpf.c:<br>
#include <socket.h><br>
#include <vmlinux.h><br>
... TCP ...<br>
<br>
In this case, struct "socket" is defined twice. If we can have
something like<br>
bpf.c:<br>
#pragma clang attribute push (__attribute__((weak)),
apply_to = record)<br>
#include <socket.h><br>
#pragma clang attribute pop<br>
#include <vmlinux.h><br>
... TCP ...<br>
<br>
<br>
Then bpf program can get header file macro definitions without
copying<br>
explicitly.<br>
<br>
We need support for "apply_to = typedef" in the above pragma,
so we<br>
can deal with typedef types as well.<br>
<br>
Another issue is related to what if two types are not
compatible. For example,<br>
struct t { int a; } __attribute__((weak));<br>
struct t { int a; int b; };<br>
I think we could have a flag to control whether we allow
incompatible<br>
weak type or not.<br>
<br>
It would be good if I can get some suggestions/directions on
whether<br>
such a feature is possible or not for clang frontend.<br>
<br>
Thanks,<br>
<br>
Yonghong<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote>
</div>
</blockquote>
</body>
</html>