On 1 November 2016 at 03:54, Hannes Frederic Sowa <han...@stressinduktion.org> wrote: > I do fear the complexity and debugability introduced by this patch set > quite a bit.
What is the complexity concern? This is pretty straight forward. I agree on debugability. This is being worked on separately as Alexei mentioned, to address this for all BPF integrations. > I wonder if architecturally it would be more feasible to add a generic > (bfp-)hook into into dst_output(_sk) and allow arbitrary metadata to be > added into the dsts. > > BPF could then be able to access parts of the metadata in the attached > metadata dst entries and performing the matching this way? If I understand you correctly then a single BPF program would be loaded which then applies to all dst_output() calls? This has a huge drawback, instead of multiple small BPF programs which do exactly what is required per dst, a large BPF program is needed which matches on metadata. That's way slower and renders one of the biggest advantages of BPF invalid, the ability to generate a a small program tailored to a particular use. See Cilium. > The reason why I would prefer an approach like this: irregardless of the > routing lookup we would process the skb with bpf or not. This gives a > single point to debug, where instead in your approach we first must > figure out the corresponding bpf program and then check for it specifically. Not sure I see what kind of advantage this actually provides. You can dump the routes and see which programs get invoked and which section. If it's based on metadata then you need to know the program logic and associate it with the metadata in the dst. It actually doesn't get much easier than to debug one of the samples, they are completely static once compiled and it's very simple to verify if they do what they are supposed to do. If you like the single program approach, feel free to load the same program for every dst. Perfectly acceptable but I don't see why we should force everybody to use that model.