On Wed, Jan 4, 2023 at 11:44 AM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > On Wed, Jan 04, 2023 at 10:43:37AM -0800, Andrii Nakryiko wrote: > > > extern bool bpf_dynptr_is_null(const struct bpf_dynptr *p) __ksym; > > > > > > will likely work with both gcc and clang. > > > And if it doesn't we can fix it. > > > > > > While when gcc folks saw helpers: > > > > > > static bool (*bpf_dynptr_is_null)(const struct bpf_dynptr *p) = (void *) 777; > > > > > > they realized that it is a hack that abuses compiler optimizations. > > > They even invented attr(kernel_helper) to workaround this issue. > > > After a bunch of arguing gcc added support for this hack without attr, > > > but it's going to be around forever... in gcc, in clang and in kernel. > > > It's something that we could have fixed if it wasn't for uapi. > > > Just one more example of unfixable mistake that causing issues > > > to multiple projects. > > > That's the core issue of kernel uapi rules: inability to fix mistakes. > > > > This is BPF ISA defining `call #N;` to call helper with ID N, which > > you agree that it (ISA) has to be stable, documented and standardized, > > right? > > > > Everything else is just how we expose those constants into C code and > > how libbpf deals with them. Libbpf could support new attribute or even > > extern-based convention, if necessary. > > > > But it wasn't necessary for years and only was brought up during GCC's > > attempt to invent a new convention here. And they successfully dealt > > with this challenge. > > 'dealt with this challenge'? You mean didn't, right? > gcc doesn't guarantee that '= (void *) 777;' will work even with optimization on. I don't use gcc-bpf, but given they dropped kernel_helper attribute, and given you said "After a bunch of arguing gcc added support for this hack without attr but it's going to be around forever..." I assumed it does work. Are you saying it doesn't? > In clang we cannot guarantee that either. It works today, if it ever regresses there will be a lot of noise and this regression will be fixed. So maybe technically it's not guaranteed, but in practice it will keep working. We had a `const volatile` case recently, variables were not being put into .rodata section properly. GCC was changed to do it the same way as Clang so that all the existing apps can keep working. > Nothing requires a compiler to do constant propagation. > > > > > Yes, we won't change existing helpers, but we can add new ones if we > > need to extend them. That's how APIs work. Yes, they need careful > > considerations when designing and implementing new APIs. Yes, mistakes > > do happen, that's just fact of life and par for the course of software > > development. Yes, we have to live with those mistakes. Nothing changed > > about that. > > > > But somehow libraries and kernel still produce stable APIs and > > maintain them because they clearly provide benefits to end users. > > Did you 'live with mistakes done in libbpf 0.x' ? No. for a long time yes. And it's not apples to apples comparison, with library it is possible to deprecate APIs, which is what we did. With lots of work and gradual transition, but did it. If we couldn't pull this through, yeah, I would live with whatever APIs are there. And added new ones as a better replacement. As is always done for APIs, nothing new here. Within 0.x and 1.x APIs are stable and we live with them. This API stability fear doesn't paralyze libbpf development, we still add new stable APIs, if they are considered useful and thought through enough. > You've introduced libbpf 1.0 with incompatible api and some users suffereed. By "suffered" you mean a few systemd folks being grumpy about this? And having to do 100 lines of code changes ([0]) to support two incompatible major versions of libbpf *simultaneously*? On the other hand we got a library with saner error propagation behavior and various API normalizations and additions. Not too bad of a trade off. Sure, deprecation is not easy or free, there was a lot of prep work, and some users had to adjust their code to use new APIs. But this is quite a tangent. [0] https://github.com/systemd/systemd/pull/24511/ > > > We'll get the same amount of flame when we try to change kfunc that's > > widely adopted. > > Of course. That's why we need to define a stability and deperecation > plan for them. Lots of things that need to be defined and figured out, but we are already quick to freeze BPF helpers.
