Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> writes: > On Fri, Jun 26, 2020 at 07:51:41AM -0500, Eric W. Biederman wrote: >> >> Asking for people to fix their bugs in this user mode driver code has >> been remarkably unproductive. So here are my bug fixes. >> >> I have tested them by booting with the code compiled in and >> by killing "bpfilter_umh" and running iptables -vnL to restart >> the userspace driver. >> >> I have split the changes into small enough pieces so they should be >> easily readable and testable. >> >> The changes lean into the preexisting interfaces in the kernel and >> remove special cases for user mode driver code in favor of solutions >> that don't need special cases. This results in smaller code with >> fewer bugs. >> >> At a practical level this removes the maintenance burden of the >> user mode drivers from the user mode helper code and from exec as >> the special cases are removed. >> >> Similarly the LSM interaction bugs are fixed by not having unnecessary >> special cases for user mode drivers. >> >> Please let me know if you see any bugs. Once the code review is >> finished I plan to take this through my tree. > > I did a quick look and I like the cleanup. Thanks! Good then we have a path forward. > The end result looks good. > The only problem that you keep breaking the build between patches, > so series will not be bisectable. Keep breaking? There is an issue with patch 5/14 where the build breaks when bpfilter is not enabled. Do you see any others? I know I tested each patch individually. But I was only testing with CONFIG_BPFILTER enabled so I missed one. So there should not be things that break but things slip through occassionally. I will resend this shortly with the fix and any others that I can find. > blob_to_mnt is a great idea. Much better than embedding fs you advocated earlier. I was lazy and not overdesigning but I still suspect the blob will benefit from becoming a cpio in the future. > I'm swamped with other stuff today and will test the set Sunday/Monday > with other patches that I'm working on. > I'm not sure why you want to rename the interface. Seems > pointless. But fine. For maintainability I think the code very much benefits from a clear separation between the user mode driver code from the user mode helper code. > As far as routing trees. Do you mind I'll take it via bpf-next ? > As I said countless times we're working on bpf_iter using fork_blob. > If you take this set via your tree we would need to wait the whole kernel release. > Which is 8+ weeks before we can use the interface (due to renaming and overall changes). > I'd really like to avoid this huge delay. > Unless you can land it into 5.8-rc2 or rc3. I also want to build upon this code. How about when the review is done I post a frozen branch based on v5.8-rc1 that you can merge into the bpf-next tree, and I can merge into my branch. That way we both can build upon this code. That is the way conflicts like this are usually handled. Further I will leave any further enhancements to the user mode driver infrastructure that people have suggested to you. I will probably replace do_execve with a kernel_execve that doesn't need set_fs() to copy the command line argument. I haven't seen Christoph Hellwig address that yet, and it looks pretty straight foward at this point. Eric
