Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Pablo Neira Ayuso]

On Thu, Dec 01, 2016 at 04:05:07PM +0100, Arturo Borrero Gonzalez wrote:
> On 1 December 2016 at 11:45, Pablo Neira Ayuso  wrote:
> > I mean, it would be good if you place as much common code as possible
> > in the runner script, so individual unit tests don't result in too
> > much copy and paste.
> >
> 
> Ok, I understand.
> 
> Actually, as you know I'm just experimenting with this.
>
> Anyway the problem I see is that we could end losing a lot of flexibility.
> The current py testsuite is only able to perform one kind of tests
> because of this approach.
> In the other hand, the shell testsuite is able to perform almost any
> kind of tests because it only executes arbitrary binaries.

Flexibility is good indeed. Regarding the comments on the existing
testsuites, I think both complement each other, so they are both
useful.

> So perhaps we could take an intermediate approach:
>  * scapy tests are executed by the shell testsuite runner (they are
> standalone scripts)
>  * we develop a common lib of functions inside
> tests/shell/testcases/scapy/ (for example common.py)
>  * then, each scapy test load that common lib which includes most of
> the factorised code

That's good, I like this balance. Look, my only concern is
maintainability: If we end up using the copy & paste pattern too much
on these scripts, we will have to patch them all if we find a bug on
them.

But don't worry too much on trying to generalize things upfront given
there's experimentation on this going on. We can introduce
abstractions incrementally, as we start seeing that code duplicates.

> Common functions would be something like this:
> 
> * configure(): we do the scapy configuration, network config, or whatever
> * load_ruleset): we pass a nft ruleset (a string) and load it using nft -f
> * check_result(): we grep the ruleset counters, or whatever
> 
> I'm thinking of some tests we could have using this approach:
> 
> * atomic replacement of ruleset during a network transfer
> * conntrack modifications (using the conntrack-tools binaries)
> * packet mangling, NAT, etc
> 
> In any case, I think we should retain the ability to load nft rules,
> send/recv scapy packets and check for nft counters at any time during
> the execution.

Sure, go ahead.

Thanks Arturo.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Vadim Kochan]

On Thu, Dec 1, 2016 at 12:45 PM, Pablo Neira Ayuso  wrote:
> On Thu, Dec 01, 2016 at 09:10:53AM +0100, Arturo Borrero Gonzalez wrote:
>> On 30 November 2016 at 19:28, Pablo Neira Ayuso  wrote:
>> >> * You can probably augment this at some pointer to rely on the new
>> >>   nf_tables tracing infrastructure.
>> >>
>>
>> That would be rather complex.
>
> OK, let's start with this something simple.
>
>> > Only one more question left: Do you think you can slightly generalize
>> > this so we decouple test files from the script? Similar to what we
>> > have for nft-tests.py.
>>
>> What do you mean?
>>
>> This testcase script is decoupled enough that you can even call it
>> this way (and it works):
>>
>>  % NFT=%(which nft) testcases/scapy/0001_ip_ttl_0
>>
>> Do you mean you would like to have a different testsuite (with a
>> different runner script) for these datapath tests?
>
> I mean, it would be good if you place as much common code as possible
> in the runner script, so individual unit tests don't result in too
> much copy and paste.
>
> Thanks Arturo.
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sorry for the noise, may PTF framework help for you ? Usually it is
used for datapath testing
on switches, but may be it can cover your cases.

Regards,
Vadim Kochan
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Pablo Neira Ayuso]

On Thu, Dec 01, 2016 at 09:10:53AM +0100, Arturo Borrero Gonzalez wrote:
> On 30 November 2016 at 19:28, Pablo Neira Ayuso  wrote:
> >> * You can probably augment this at some pointer to rely on the new
> >>   nf_tables tracing infrastructure.
> >>
> 
> That would be rather complex.

OK, let's start with this something simple.

> > Only one more question left: Do you think you can slightly generalize
> > this so we decouple test files from the script? Similar to what we
> > have for nft-tests.py.
> 
> What do you mean?
> 
> This testcase script is decoupled enough that you can even call it
> this way (and it works):
> 
>  % NFT=%(which nft) testcases/scapy/0001_ip_ttl_0
> 
> Do you mean you would like to have a different testsuite (with a
> different runner script) for these datapath tests?

I mean, it would be good if you place as much common code as possible
in the runner script, so individual unit tests don't result in too
much copy and paste.

Thanks Arturo.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Arturo Borrero Gonzalez]

On 30 November 2016 at 19:28, Pablo Neira Ayuso  wrote:
>> * You can probably augment this at some pointer to rely on the new
>>   nf_tables tracing infrastructure.
>>

That would be rather complex.

>
> Only one more question left: Do you think you can slightly generalize
> this so we decouple test files from the script? Similar to what we
> have for nft-tests.py.

What do you mean?

This testcase script is decoupled enough that you can even call it
this way (and it works):

 % NFT=%(which nft) testcases/scapy/0001_ip_ttl_0

Do you mean you would like to have a different testsuite (with a
different runner script) for these datapath tests?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Pablo Neira Ayuso]

On Wed, Nov 30, 2016 at 07:27:04PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Nov 30, 2016 at 10:39:06AM +0100, Arturo Borrero Gonzalez wrote:
> > From: Arturo Borrero Gonzalez 
> > 
> > This test uses scapy to send a packet and test our packet/data path.
> > We grep the 'nft list ruleset' output for a counter increment.
> > 
> > If we like this approach, then we could easily add more testcases
> > following the pattern in this patch.
> 
> I think it's been several netfilter workshops already talking on this,
> but it never happens because nobody pushed this forward.
> 
> If you can make this happen, it would great. Testing the datapath is
> something that we always wanted to have.
> 
> Several ideas:
> 
> * Check if you can use the dummy interface, so we make sure no other
>   packets interfer with the tests.
> 
> * You can probably augment this at some pointer to rely on the new
>   nf_tables tracing infrastructure.
> 
> Anyway, I agree that starting with something simple is good enough.

Only one more question left: Do you think you can slightly generalize
this so we decouple test files from the script? Similar to what we
have for nft-tests.py.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC nft PATCH] tests: shell: add a basic scapy test

[Pablo Neira Ayuso]

On Wed, Nov 30, 2016 at 10:39:06AM +0100, Arturo Borrero Gonzalez wrote:
> From: Arturo Borrero Gonzalez 
> 
> This test uses scapy to send a packet and test our packet/data path.
> We grep the 'nft list ruleset' output for a counter increment.
> 
> If we like this approach, then we could easily add more testcases
> following the pattern in this patch.

I think it's been several netfilter workshops already talking on this,
but it never happens because nobody pushed this forward.

If you can make this happen, it would great. Testing the datapath is
something that we always wanted to have.

Several ideas:

* Check if you can use the dummy interface, so we make sure no other
  packets interfer with the tests.

* You can probably augment this at some pointer to rely on the new
  nf_tables tracing infrastructure.

Anyway, I agree that starting with something simple is good enough.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html