Message-ID: <20171126074323.GA2023@nanopsycho>
Date:   Sun, 26 Nov 2017 08:43:23 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        netdev@...r.kernel.org, oss-drivers@...ronome.com,
        daniel@...earbox.net
Subject: Re: [RFC net-next 0/6] xdp: make stack perform remove and tests

Sun, Nov 26, 2017 at 02:43:04AM CET, jakub.kicinski@...ronome.com wrote:
>On Sat, 25 Nov 2017 17:02:09 -0800, Alexei Starovoitov wrote:
>> On Thu, Nov 23, 2017 at 06:36:07PM -0800, Jakub Kicinski 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
>> > 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
>> > ref counts.  The only slightly unusual thing is that the BPF code
>> > sleeps for a bit on remove in the netdev notifier.  
>> 
>> I like the direction.
>> Patch 3 in particular with auto prog_put is a great idea!
>> Patch 4 - if you want to do dual gpl+bsd. It's ok and really your call as an author.

Sure. I was just curious if it is really necessary. That's all.


>> Patch 5 - I'm only not excited about debugfs. Can you do the same with tracepoints?
>
>I would prefer to stick to DebugFS, if you don't mind.  Some of the
>knobs are writeable.  I look at DebugFS as a back door.  We throw
>standard API commands at the device and want to inspect its internal
>state.  And cross check the standard reporting APIs.  As such DebugFS
>is completely inappropriate but for API-test-drivers.

I agree. This is one of few exceptions where debugfs is good to be used.


>
>On that topic - is there a way to trigger and wait for an rcu barrier
>from user space?  Quentin pointed out that selftest is shaky unless run
>with RCU debug enabled.  To wait for deletion of programs I need a
>barrier :S
>
>> Patch 6 - python 3 is a requirement? or can work with 2.7 ? I don't mind if it's 3+ only.
>> Just trying to understand the dependencies.
>
>Am a python noob so I defaulted to version 3 and proceeded to look
>things I needed up in the python 3 docs.  Which perhaps resulted in
>using more python3 only features than necessary...  
>
>Upon a quick inspection, the only python3 feature I seem to be using 
>is the Popen.args member.
>
>> and thanks for adding it to selftests.
>
>Thanks for the comments!

Powered by blists - more mailing lists