[ovs-discuss] New test case - it fails

[Joe Stringer]

Hi Tony,

On 10 June 2015 at 01:50, Tony van der Peet <tony.vanderpeet at gmail.com> wrote:
> * git clone carried out Wednesday 10/Jun/2015 (NZ time)

Great, we always develop against the tip of master.

> * test case suggested by a failing oftest case
> * is this in the correct test file?

Yes, this is an appropriate file. I've also alternatively used
tests/dpif-netdev.at for testing userspace datapath behaviour like
this.

> * is the test written to the correct standard?
> * is it a valid test?

There is no end-quote on the netdev-dummy line, which suggests that
this is probably not identical to the test that you were running
locally. A common problem is that your email client is messing with
the patch. git format-patch (and git send-email) do a great job of
ensuring that a patch you've committed locally is sent in the correct
format.

Typically if I want to determine whether a datapath flow was installed
correctly, I will do something like the following:

- Increase log level at the beginning of the test (after vswitchd start)
AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])

- use FILTER_FLOW_INSTALL
AT_CHECK([cat ovs-vswitchd.log | grep 'in_port=[[1]]' |
FILTER_FLOW_INSTALL | STRIP_XOUT], [0], [dnl
<flow>
]

This alleviates some issues with test script race conditions when
running the tests on a highly-loaded system (or highly-parallel). You
can see some examples in the same test file; the macro is defined in
tests/dpif-netdev.at.

> * what other test cases suggest themselves? (the fix is going to involve
> *all* IPv6 protocols, not just ICMPv6)

I suspect if we'd thought of particular additional cases, we would
have added them. If there are more problems that oftest is showing up,
fixing those bugs and adding tests to show that the fix works would be
appropriate. Generally the CI we use (eg travis) doesn't run the
oftest framework, so this would be a way to ensure that there aren't
regressions in areas like this.

> * how do I proceed from here?
> * if you apply the patch it should be test #912

I noticed that if you apply something similar to the patch below on
branch-2.3 (complete with truncated packet), the datapath flow that's
generated still includes the icmp code (and seems fine). Yet on the
latest master that doesn't seem to be the case. Is that what you've
noticed? That seems like potentially a regression or change in the
behaviour since v2.3. Perhaps because the truncated packet doesn't
include a valid ICMPv6 option, so the newer OVS doesn't even parse the
type and/or code. (I haven't looked particularly close at the packet
you're using; I cheated with an online hexdump decoder)