Date:   Mon, 06 Mar 2023 09:35:22 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To:     xu xin <xu.xin.sc@gmail.com>, 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
Message-ID: <6405fa2a80577_bb2242089c@willemb.c.googlers.com.notmuch>
In-Reply-To: <20230306073136.155697-1-xu.xin16@zte.com.cn>
References: <6401f7889e959_3f6dc82084b@willemb.c.googlers.com.notmuch>
 <20230306073136.155697-1-xu.xin16@zte.com.cn>
Subject: RE: [PATCH linux-next v2] selftests: net: udpgso_bench_tx: Add test
 for IP fragmentation of UDP packets
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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