> -----Original Message-----
> From: Ilya Maximets <[email protected]>
> Sent: Friday 5 August 2022 19:41
> To: Aaron Conole <[email protected]>
> Cc: [email protected]; [email protected]; Mike Pattrick
> <[email protected]>; Salem Sol <[email protected]>; Phelan, Michael
> <[email protected]>
> Subject: Re: [PATCH] system-traffic: Fix incorrect neigh entry in ipv6 header
> modification test.
> 
> On 8/5/22 20:32, Aaron Conole wrote:
> > Ilya Maximets <[email protected]> writes:
> >
> >> The permanent neighbor entry for fc00::1 is added into a wrong
> >> namespace, so in order to reply to a ping from at_ns1, the address of
> >> fc00::1 has to be discovered.
> >
> > This is strange - how did it end up in wrong namespace?  Is it a race
> > with the veth setup?  I guess we could possibly fix that?
> 
> We have:
> NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::1 lladdr e4:11:22:33:44:55
> dev p0])
> 
> But it should be:
> NS_CHECK_EXEC([at_ns1], [ip -6 neigh add fc00::1 lladdr e4:11:22:33:44:55
> dev p1])
> 
> because fc00::1 is an IP of p0 in at_ns0.  There is no point adding the entry 
> to
> the namespace where it is directly accessible.
> 
> We could just fix that, but I re-worked the test a little bit instead.
> 
> >
> > Also, there are some other conntrack tests that seem to have a similar
> > if/address setup pattern, allowing ND in only one direction - have we
> > seen them fail as well?
> >
> >> Interfaces are attached
> >> to OVS and we're removing flows that can forward ND requests after
> >> initial setup.  In case ND request wasn't sent and replied before
> >> that, at_ns1 will not be able to discover fc00:1 and won't reply to
> >> pings.
> >>
> >> It's hard to catch this condition while running tests locally, but
> >> for some reason our CI is failing consistently.
> >>
> >> Fix the issue by removing all the unnecessary permanent entries and
> >> just allowing all the normal traffic to flow through the low priority
> >> OVS flow, so all addresses can be discovered.
> >>
> >> Also adding one more wait to avoid occasional drops of the very first
> >> packet.
> >>
> >> Fixes: 2ff43c78c685 ("packets: Re-calculate IPv6 checksum only for
> >> first frag upon modify.")
> >> Signed-off-by: Ilya Maximets <[email protected]>
> >> ---
> >>  tests/system-traffic.at | 14 +++++++++-----
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at index
> >> 53ae80f4e..33108c5ab 100644
> >> --- a/tests/system-traffic.at
> >> +++ b/tests/system-traffic.at
> >> @@ -237,17 +237,21 @@ ADD_NAMESPACES(at_ns0, at_ns1)
> ADD_VETH(p0,
> >> at_ns0, br0, "fc00::1/96", e4:11:22:33:44:55)  ADD_VETH(p1, at_ns1,
> >> br0, "fc00::2/96", e4:11:22:33:44:54)  NS_CHECK_EXEC([at_ns0], [ip -6
> >> neigh add fc00::3 lladdr e4:11:22:33:44:54 dev p0])
> >> -NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::2 lladdr
> >> e4:11:22:33:44:54 dev p0]) -NS_CHECK_EXEC([at_ns0], [ip -6 neigh add
> >> fc00::1 lladdr e4:11:22:33:44:55 dev p0])
> >>
> >>  dnl Linux seems to take a little time to get its IPv6 stack in
> >> order. Without  dnl waiting, we get occasional failures due to the 
> >> following
> error:
> >>  dnl "connect: Cannot assign requested address"
> >>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
> >> +OVS_WAIT_UNTIL([ip netns exec at_ns1 ping6 -c 1 fc00::1])
> >>
> >> -AT_CHECK([ovs-ofctl del-flows -OOpenFlow15 br0])
> >> -AT_CHECK([ovs-ofctl add-flow -OOpenFlow15 br0
> >> in_port=ovs-p0,ipv6,ipv6_dst=fc00::3,ipv6_src=fc00::1,actions=set_fie
> >> ld:fc00::2-\>ipv6_dst,ovs-p1]) -AT_CHECK([ovs-ofctl add-flow
> >> -OOpenFlow15 br0
> >> in_port=ovs-p1,ipv6,ipv6_dst=fc00::1,ipv6_src=fc00::2,actions=set_fie
> >> ld:fc00::3-\>ipv6_src,ovs-p0])
> >> +AT_DATA([flows.txt], [dnl
> >> +priority=100,in_port=ovs-p0,ipv6,ipv6_src=fc00::1,ipv6_dst=fc00::3,a
> >> +ctions=set_field:fc00::2->ipv6_dst,ovs-p1
> >> +priority=100,in_port=ovs-p1,ipv6,ipv6_src=fc00::2,ipv6_dst=fc00::1,a
> >> +ctions=set_field:fc00::3->ipv6_src,ovs-p0
> >> +priority=0,actions=NORMAL
> >> +])
> >> +
> >> +AT_CHECK([ovs-ofctl del-flows br0])
> >> +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> >>
> >>  NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::3 |
> >> FORMAT_PING], [0], [dnl
> >>  3 packets transmitted, 3 received, 0% packet loss, time 0ms
> >
Hey Ilya,
Thanks for the patch. I ran the system userspace test a few hundred times with 
various configurations and didn't encounter any failures so the patch looks 
good to me.

Acked-by: Michael Phelan <[email protected]>

Thanks,
Michael.



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to