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

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

On Mon, Feb 07, 2022 at 11:32:05AM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 07/02/2022 05:13, Geliang Tang wrote:
> > Hi Matt,
> > 
> > Matthieu Baerts <matthieu.baerts@tessares.net> 于2022年1月29日周六 01:04写道：
> >>
> >> Hi Mat, Geliang,
> >>
> >> On 27/01/2022 01:07, Mat Martineau wrote:
> >>> On Tue, 25 Jan 2022, Geliang Tang wrote:
> >>>
> >>>> Added the test cases for MP_FAIL, use 'tc' commands to trigger the
> >>>> checksum failures.
> >>>>
> >>>
> >>> This patch is working well with the inverted byte detection, thanks!
> >>
> >> The public CI seems quite happy with the new version, good work :)
> >>
> >> Here is its report:
> >>
> >> ====== 8< ======
> >> Our CI did some validations and here is its report:
> >>
> >> - KVM Validation: normal:
> >>   - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
> >>   - Task: https://cirrus-ci.com/task/4993794666921984
> >>   - Summary:
> >> https://api.cirrus-ci.com/v1/artifact/task/4993794666921984/summary/summary.txt
> >>
> >> - KVM Validation: debug:
> >>   - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴:
> >>   - Task: https://cirrus-ci.com/task/4682233209421824
> >>   - Summary:
> >> https://api.cirrus-ci.com/v1/artifact/task/4682233209421824/summary/summary.txt
> >>
> >> Initiator: Patchew Applier
> >> Commits:
> >> https://github.com/multipath-tcp/mptcp_net-next/commits/c52e51136372
> >> ====== 8< ======
> >>
> >> But if you look at the logs [1], there are probably 3 last things to
> >> improve:
> >>
> >> [1] https://api.cirrus-ci.com/v1/task/4993794666921984/logs/test.log
> >>
> >> This line is repeated 90+ times:
> >>
> >>   tc action pedit offset 162 out of bounds
> >>
> >> (we talked about this one at the last meeting with ideas on how to
> >> reduce/get rid of them)
> >>
> > 
> > Dose 'tc + iptables' work for our case?
> > 
> > Use iptables to select the packets with enough data. Then use tc to
> > edit the selected packets only. (I don't know how to do it yet.)
> > 
> > Here's a link about this:
> > 
> > https://wiki.archlinux.org/title/advanced_traffic_control#Using_tc_+_iptables
> > .
> > '''
> > Using tc + iptables
> > 
> > Iptables has a method called fwmark that can be used to mark packets
> > across interfaces.
> > 
> > First, this makes packets marked with 6, to be processed by the 1:30 class
> > 
> > # tc filter add dev eth0 protocol ip parent 1: prio 1 handle 6 fw flowid 1:30
> > 
> > This sets that mark 6, using iptables
> > 
> > # iptables -A PREROUTING -t mangle -i eth0 -j MARK --set-mark 6
> > 
> > You can then use iptables normally to match packets and then mark them
> > with fwmark.
> > '''
> 
> Good idea, probably simpler!
> 
> I guess we can mark some packets in POSTROUTING and modify the egress TC
> rule.
> 
> Do you need a hand for that?
> 

Thanks, Matt, I really need your help :)

> >> But also this one now, repeated 40+ times with different offsets:
> >>
> >>   file received by server has inverted byte at 106565
> >>
> > 
> > These inverted byte logs will not be there if we add the code to drop
> > the bad data in future. (I haven't figured out how to implement this
> > yet.) Right now, just let them be there.
> 
> I see. I guess later, when the code to drop the bad data in the future
> will be ready, this "warning" will become an "error" we will no longer
> ignore.
> 
> >> And one last thing, we can see that now we have more than 99 tests, the
> >> alignement is no longer OK :)
> >>
> >>
> >>   # 101 1 MP_FAIL, single subflow            syn[ ok ] - synack[ ok ]...
> >>   #                                         sum[ ok ] - csum  [ ok ]
> >>   #                                         ftx[ ok ] - frx   [ ok ]
> >>   #                                         itx[ ok ] - irx   [ ok ]
> >>
> >> A very small detail, up to you to fix it or not ;)
> >>
> > 
> > I'll update this in the next version.
> 
> Thanks!
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>