Re: [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases

[Geliang Tang <geliang.tang@suse.com>]

Hi Matt,

On Wed, Feb 09, 2022 at 12:13:00PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 09/02/2022 11:55, Geliang Tang wrote:
> > On Wed, Feb 09, 2022 at 10:23:52AM +0100, Matthieu Baerts wrote:
> >> On 09/02/2022 09:57, Geliang Tang wrote:
> >>> +fail_tests()
> >>> +{
> >>> +	# multiple subflows
> >>> +	reset_with_fail 2
> >>> +	pm_nl_set_limits $ns1 0 2
> >>> +	pm_nl_set_limits $ns2 0 2
> >>> +	pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
> >>> +	pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
> >>> +	run_tests $ns1 $ns2 10.0.1.1 3
> >>> +	pedit_action_happened 2
> >>> +	chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
> >>> +		$pedit_action $pedit_action
> >>
> >> Related to my comment in the v2: we should not use $pedit_action in the
> >> args we need to verify I think. Instead we should probably then have:
> >>
> >>   chk_join_nr "(...)" 2 2 2 1 1
> > 
> > Thanks for your review and suggestions. I updated them in v5.
> > 
> > We will get this failure in v5 in rare cases, about one percent
> > probability
> 
> Do you know why?
> 
> We invert a byte so it should always cause an issue, no? (we can always
> have 2 same hashes for two different sources but it is a very very rare
> case we don't have to consider I think, especially when the content is
> so close, no?)
> 
> Or maybe we need to improve the IPTables and TC rules?
> 

I think we should go back to the old version of this patch. We shouldn't
always expect getting 1 MP_FAIL. If no checksum failure occur, we should
expect no MP_FAIL. I just sent a squash-to patch to fix this.

Thanks,
-Geliang

> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>