On Thu, Jul 14, 2022 at 7:36 AM Daniel Müller <deso@xxxxxxxxxx> wrote: > > On Wed, Jul 13, 2022 at 09:48:32PM -0700, Andrii Nakryiko wrote: > > On Tue, Jul 12, 2022 at 4:01 PM Daniel Müller <deso@xxxxxxxxxx> wrote: > > > > > > On Tue, Jul 12, 2022 at 03:33:26PM -0700, sdf@xxxxxxxxxx wrote: > > > > On 07/12, Daniel M�ller wrote: > > > > > On Tue, Jul 12, 2022 at 02:27:47PM -0700, Alexei Starovoitov wrote: > > > > > > On Tue, Jul 12, 2022 at 2:21 PM Daniel M�ller <deso@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > This change integrates the libbpf maintained configurations and > > > > > > > black/white lists [0] into the repository, co-located with the BPF > > > > > > > selftests themselves. The only differences from the source is that we > > > > > > > replaced the terms blacklist & whitelist with denylist and allowlist, > > > > > > > respectively. > > > > > > > > > > > > > > [0] https://github.com/libbpf/libbpf/tree/20f03302350a4143825cedcbd210c4d7112c1898/travis-ci/vmtest/configs > > > > > > > > > > > > > > Signed-off-by: Daniel M�ller <deso@xxxxxxxxxx> > > > > > > > --- > > > > > > > .../bpf/configs/allowlist/ALLOWLIST-4.9.0 | 8 + > > > > > > > .../bpf/configs/allowlist/ALLOWLIST-5.5.0 | 55 + > > > > > > > .../selftests/bpf/configs/config-latest.s390x | 2711 +++++++++++++++ > > > > > > > .../bpf/configs/config-latest.x86_64 | 3073 > > > > > +++++++++++++++++ > > > > > > > > > > > > Instead of checking in the full config please trim it to > > > > > > relevant dependencies like existing selftests/bpf/config. > > > > > > Otherwise every update/addition would trigger massive patches. > > > > > > > > > Thanks for taking a look. Sure. Do we have some kind of tooling for that > > > > > or are > > > > > there any suggestions on the best approach to minimize? > > > > > > > > I would be interested to know as well if somebody knows some tricks on > > > > how to deal with kconfig. I've spent some time yesterday manually > > > > crafting various minimal bpf configs (for build tests), running make > > > > olddefconfig and then verifying that all my options are still present in > > > > the final config file. > > > > > > > > It seems like kconfig tool can resolve some of the dependencies, > > > > but there is a lot of if/endif that can break in non-obvious ways. > > > > For example, putting CONFIG_TRACING=y and doing 'make olddefconfig' > > > > won't get you CONFIG_TRACING=y in the final .config > > > > > > > > So the only thing, for me, that helped, was to manually go through > > > > the kconfig files trying to see what the dependencies are. > > > > I've tried scripts/kconfig/merge_config.sh, but it doesn't > > > > seem to bring anything new to the table.. > > > > > > > > So here is what I ended up with, I don't think it will help you that > > > > much, but at least can highlight the moving parts (I was thinking that > > > > maybe we can eventually put them in the CI as well to make sure all weird > > > > configurations are build-tested?): > > > > > > [...] > > > > > > I *think* that make savedefconfig [0] is the way to go, at least for my use > > > case. That cuts down the config file to <350 lines. However, it does change some > > > configurations from 'm' to 'y', which I can't say I quite understand or would > > > have expected (but perhaps minimal implies no modules or similar; I haven't > > > investigated). > > > I am still verifying that the result is working as expected, though. > > > > I think ideally we'd do defconfig first, then append whatever is in > > selftests/bpf/config, do olddefconfig to fill in all the unspecified > > options, and then use the result as the config. Yes, that requires > > that selftests/bpf/config has some of the dependent values specified, > > which is an annoying mostly one-time thing, but I think it's > > beneficial to all the new BPF users, because it *really* shows what > > needs to be added to kernel config to make everything work. We can > > also split it into BPF-specific and non-BPF (dependencies) configs, if > > that is cleaner. > > Agreed, we should do that eventually. But let's not put everything into > this patch set, which was never intended to rework everything we have, > okay? It contains a few steps towards where we want to head. > > If we start diverging massively now, while also moving configurations > between multiple repositories, we end up with a mess of a history that > will be hard to follow. > > So unless there exist very strong arguments forcing us to do that here > and now (such as us regressing on one front, which I don't see here), > I'd rather we go about it at a later point after other check boxes have > been ticked. What do you think? > You mean the part about checking in huge Kconfigs for x86 and s390x? I don't think we should do that as a first step. Yes it's an annoying (but also very important) part to figure out the minimal set of added configs on top of default config, but I think we should do that from the beginning instead of polluting Git history with massive configs. It will also keep selftests/bpf/config "honest" instead of putting it on new users to figure out other missed or dependent configs by themselves. With s390x config, at least, I hope that Ilya can ease the pain, especially that he was the one who came up with that config in the first place (cc'ed Ilya). > > Also, I don't think we should move 4.9.0 and 5.5.0 lists here, let's > > keep them in libbpf CI, they are very specific there. Here we should > > only maintain the latest per-arch configs and allow/deny lists only. > > Sounds good, will remove them. > > Thanks, > Daniel
