From:   xu xin <xu.xin.sc@gmail.com>
To:     willemdebruijn.kernel@gmail.com
Cc:     davem@davemloft.net, edumazet@google.com, jiang.xuexin@zte.com.cn,
        kuba@kernel.org, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, netdev@vger.kernel.org,
        pabeni@redhat.com, shuah@kernel.org, xu.xin16@zte.com.cn,
        yang.yang29@zte.com.cn, zhang.yunkai@zte.com.cn
Subject: RE: [PATCH linux-next v2] selftests: net: udpgso_bench_tx: Add test for IP fragmentation of UDP packets
Date:   Mon,  6 Mar 2023 07:31:36 +0000
Message-Id: <20230306073136.155697-1-xu.xin16@zte.com.cn>
In-Reply-To: <6401f7889e959_3f6dc82084b@willemb.c.googlers.com.notmuch>
References: <6401f7889e959_3f6dc82084b@willemb.c.googlers.com.notmuch>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

>> >     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.

>
>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'.

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