Date:   Fri, 03 Mar 2023 08:35:04 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To:     Eric Dumazet <edumazet@google.com>, yang.yang29@zte.com.cn
Cc:     willemdebruijn.kernel@gmail.com, davem@davemloft.net,
        kuba@kernel.org, pabeni@redhat.com, shuah@kernel.org,
        netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org, zhang.yunkai@zte.com.cn,
        xu.xin16@zte.com.cn, jiang.xuexin@zte.com.cn
Message-ID: <6401f7889e959_3f6dc82084b@willemb.c.googlers.com.notmuch>
In-Reply-To: <CANn89iK3frwxddhSbbi5cvmuSjr2TqPbD_mTgBa3k4ESDQnrHA@mail.gmail.com>
References: <6400bd699f568_20743e2082b@willemb.c.googlers.com.notmuch>
 <202303031900454292466@zte.com.cn>
 <CANn89iK3frwxddhSbbi5cvmuSjr2TqPbD_mTgBa3k4ESDQnrHA@mail.gmail.com>
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: quoted-printable
Precedence: bulk

Eric Dumazet wrote:
> On Fri, Mar 3, 2023 at 12:03=E2=80=AFPM <yang.yang29@zte.com.cn> wrote:=

> >
> > > Did you actually observe a difference in behavior with this change?=

> >
> > The test of UDP only cares about sending, and does not much need to
> > consider the problem of PMTU, we configure it to IP_PMTUDISC_DONT.
> >     IP_PMTUDISC_DONT: turn off pmtu detection.
> >     IP_PMTUDISC_OMIT: the same as DONT, but in some scenarios, DF wil=
l
> > be ignored. I did not construct such a scene, presumably when forward=
ing.
> > 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.

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 transfor=
ms
         * how transforms change size of the packet, it will come out.
         */
        skb->ignore_df =3D 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 =3D=3D IP_PMTUDISC_DO ||
            inet->pmtudisc =3D=3D IP_PMTUDISC_PROBE ||
            (skb->len <=3D dst_mtu(&rt->dst) &&
             ip_dont_fragment(sk, &rt->dst)))
                df =3D htons(IP_DF);
 =

> >
> > We have a question, what is the point of this test if it is not compa=
red to
> > UDP GSO and IP fragmentation. No user or tool will segment in user mo=
de,

Are you saying no process will use UDP_SEGMENT?

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
> at the sender side.
> =

> Major issue is the receiving side, with many implications of memory
> and cpu costs,
> not counting amplifications of potential packet losses.
> =

> So your patch would make sense if you also change
> tools/testing/selftests/net/udpgso_bench_rx.c accordingly.
> =

> If you send UDP packets to a receiver, then you should not receive
> ICMP errors, unless a reassembly error occured.
> =

> About ICMP packets being disruptive, you can always ignore errors at
> sendmsg() time and retry the syscall.