Re: [RFC net-next 0/6] xdp: make stack perform remove and tests

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Thu, Nov 23, 2017 at 11:45 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Nov 24, 2017 at 03:36:07AM CET, jakub.kicinski@netronome.com wrote:
>>Hi!
>>
>>The purpose of this series is to add a software model of BPF offloads
>>to make it easier for everyone to test them and make some of the more
>>arcane rules and assumptions more clear.
>>
>>The series starts with 3 patches aiming to make XDP handling in the
>>drivers less error prone.  Currently driver authors have to remember
>>to free XDP programs if XDP is active during unregister.  With this
>>series the core will disable XDP on its own.  It will take place
>>after close, drivers are not expected to perform reconfiguration
>>when disabling XDP on a downed device.
>>
>>Next two patches add the software netdev driver.  Last but not least
>
> I wonder if for this it is needed to split the driver into multiple
> files. I think that a single file would be better as I don't expect the
> driver would get big.

I was hoping other offloads will be added to their separate files, to
make it easier for people to find "all code relevant when implementing
X" easier.

Sort of related to your comment on the license, I'm hoping to be able
to use SPDX one-line header to lower the overhead of many files.  Has
anyone managed to get an OK to do that?

>>there is a python test which exercises all the corner cases which
>>came to my mind.
>>
>>Test needs to be run as root.  It will print basic information to
>>stdout, but can also create a more detailed log of all commands
>>when --log option is passed.  Log is in Emacs Org-mode format.
>>
>>  ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
>>
>>Something I'm still battling with, and would appreciate help of
>>wiser people is that occasionally during the test something makes
>>the refcount of init_net drop to 0 :S  I tried to create a simple
>>reproducer, but seems like just running the script in the loop is
>>the easiest way to go...  Could it have something to do with the
>>recent TC work?  The driver is pretty simple and never touches
>
> I don't see how...

To be clear I meant the changes made to destruction of filters, not
your work. The BPF code doesn't touch ref counts and cls exts do seem
to hold a ref on the net...  but perhaps that's just pointing the
finger unnecessarily :)  I will try to investigate again tomorrow.

>>ref counts.  The only slightly unusual thing is that the BPF code
>>sleeps for a bit on remove in the netdev notifier.
>>
>>
>>Jakub Kicinski (6):
>>  net: xdp: avoid output parameters when querying XDP prog
>>  net: xdp: report flags program was installed with on query
>>  net: xdp: make the stack take care of the tear down
>>  netdevsim: add software driver for testing offloads
>>  netdevsim: add bpf offload support
>>  selftests/bpf: add offload test based on netdevsim
>
> Patchset looks fine to me.
> Thanks for this!

Thanks!