netdev - RE: [PATCH linux-next v2] selftests: net: udpgso_bench_tx: Add test for IP fragmentation of UDP packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 06 Mar 2023 09:35:22 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     xu xin <xu.xin.sc@...il.com>, willemdebruijn.kernel@...il.com
Cc:     davem@...emloft.net, edumazet@...gle.com, jiang.xuexin@....com.cn,
        kuba@...nel.org, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
        pabeni@...hat.com, shuah@...nel.org, xu.xin16@....com.cn,
        yang.yang29@....com.cn, zhang.yunkai@....com.cn
Subject: RE: [PATCH linux-next v2] selftests: net: udpgso_bench_tx: Add test
 for IP fragmentation of UDP packets

xu xin wrote:
> >> >     IP_PMTUDISC_DONT: turn off pmtu detection.
> >> >     IP_PMTUDISC_OMIT: the same as DONT, but in some scenarios, DF will
> >> > be ignored. I did not construct such a scene, presumably when forwarding.
> >> > Any way, in this test, is the same as DONT.
> >
> >My points was not to compare IP_PMTUDISC_OMIT to .._DONT but to .._DO,
> >which is what the existing UDP GSO test is setting.
> 
> Yeah, we got your point, but the result was as the patch showed, which hadn't
> changed much (patch v2 V.S patch v1), because the fragmentation option of 'patch v1'
> used the default PMTU discovery strategy(IP_PMTUDISC_DONT, because the code didn't
> setting PMTU explicitly by setsockopt() when use './udpgso_bench_tx -f' ), which is
> not much different from the 'patch v2' using IP_PMTUDISC_OMIT.

Or IP_PMTUDISC_WANT unless sysctl_ip_no_pmtu_disc is set.
But fair point. Explicitly disabling pmtu is not needed.
 
> >
> >USO should generate segments that meet MTU rules. The test forces
> >the DF bit (IP_PMTUDISC_DO).
> >
> >UFO instead requires local fragmentation, must enter the path for this
> >in ip_output.c. It should fail if IP_PMTUDISC_DO is set:
> >
> >        /* Unless user demanded real pmtu discovery (IP_PMTUDISC_DO), we allow
> >         * to fragment the frame generated here. No matter, what transforms
> >         * how transforms change size of the packet, it will come out.
> >         */
> >        skb->ignore_df = ip_sk_ignore_df(sk);
> >
> >        /* DF bit is set when we want to see DF on outgoing frames.
> >         * If ignore_df is set too, we still allow to fragment this frame
> >         * locally. */
> >        if (inet->pmtudisc == IP_PMTUDISC_DO ||
> >            inet->pmtudisc == IP_PMTUDISC_PROBE ||
> >            (skb->len <= dst_mtu(&rt->dst) &&
> >             ip_dont_fragment(sk, &rt->dst)))
> >                df = htons(IP_DF);
> > 
> >> >
> >> > We have a question, what is the point of this test if it is not compared to
> >> > UDP GSO and IP fragmentation. No user or tool will segment in user mode,
> >
> >Are you saying no process will use UDP_SEGMENT?
> >
> No, we are saying "user-space payload splitting", in other words, use ./udpgso_bench_tx
> without '-f' or '-S'.

I see. I guess you heard the arguments why the test does not compare
udp segmentation with udp fragmentation:

- fragmentation is particularly expensive on the receiver side
- fragmentation cannot be offloaded, while segmentation can

> Sincerely.
> 
> >The local protocol stack removed UFO in series d9d30adf5677.
> >USO can be offloaded to hardware by quite a few devices (NETIF_F_GSO_UDP_L4).
> >> > UDP GSO should compare performance with IP fragmentation.
> >> 
> >> I think it is misleading to think the cost of IP fragmentation matters