Mike Pattrick <m...@redhat.com> writes:
> On Wed, 2021-12-01 at 14:46 +0100, Eelco Chaudron wrote:
>>
>> On 29 Nov 2021, at 22:02, Aaron Conole wrote:
>>
>> > This reverts commit c645550bb249 ("odp-util: Always report
>> > ODP_FIT_TOO_LITTLE for IGMP.")
>> >
>> > Always forcing a slow path action can result in some over-broad
>> > flows which swallow all traffic and force them to userspace, as reported
>> > in the thread at
>> > https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387706.html
>> > and at
>> > https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387689.html
>> >
>> > Revert the ODP_FIT_TOO_LITTLE return for IGMP specifically.
>> > Additionally, remove the userspace wc mask to prevent revalidator from
>> > cycling flows.
>> >
>> > Fixes: c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for
>> > IGMP.")
>> > Signed-off-by: Aaron Conole <acon...@redhat.com>
>> > Acked-by: Eelco Chaudron <echau...@redhat.com>
>>
>> Thanks Aaron for adding the tests!
>>
>> Acked-by: Eelco Chaudron <echau...@redhat.com>
>
> Hello Aaron,
>
> Looks good but I'm having some issues with the "igmp flood for non-snoop
> enabled" test.
>
> I find that it fails for me a small percentage of the time, The
> following snippet will reproduce this issue:
>
> for i in $(seq 1 100); do make check TESTSUITEFLAGS="2379"; if [[ $?
> -ne 0 ]]; then echo "Test $i failed"; break; fi; done
>
>
> The diff shows:
>
> -recirc_id(0),in_port(1),eth(src=aa:55:aa:55:00:01,dst=01:01:00:0c:29:a0),eth_type(0x0800),ipv4(frag=no),
> actions:100,2
> -recirc_id(0),in_port(2),eth(src=01:01:00:0c:29:a0,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(frag=no),
> actions:1
> +recirc_id(0),in_port(2),eth(src=aa:55:aa:55:00:01,dst=01:01:00:0c:29:a0),eth_type(0x0800),ipv4(frag=no),
> actions:100,1
> +recirc_id(0),in_port(1),eth(src=01:01:00:0c:29:a0,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(frag=no),
> actions:2
>
>
> And the logs show:
>> 2021-12-06T20:32:20.519Z|00053|bridge|INFO|bridge br0: added interface p1 on
>> port 2
>> 2021-12-06T20:32:20.519Z|00054|bridge|INFO|bridge br0: added interface p0 on
>> port 1
>
>
> So it seems like the proper port numbers are being assigned, but the
> flow seems to have incorrect port numbers. Not sure what's going on
> there, but I also found that splitting the the "ovs-vsctl add-port"
> invocations into two commands fixed the issue.
I've dug into this a bit more and want to update the list.
I can reproduce this on multiple systems, with multiple compilers - in
order to do so, I needed to use '-O2' when compiling. I also see
similar problems with other tests in the mcast-snooping.at file (and I
haven't tried other test suites to confirm). For example:
mcast-snooping.at:165 mcast - delete the port mdb when port destroyed
Also fails in this same way.
I have done a bit of messing around, and it appears that if I drop ICMP
messages from processing, I don't see these issues any longer. It's
difficult to hook up -fsanitize=undefined to try and do a quick trim
because there are a number of "false" positives there re: pointer
casting for the HMAP_FOR_EACH(), et. al. container functions.
I would say it might be a good idea to apply this patch anyway, and we
can dig into the issue in parallel - after all, it exists whether or not
we have this patch applied AFAIK.
> Cheers,
> M
>
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
