Re: [PATCH for-7.2 v3 3/3] rtl8139: honor large send MSS value

[Stefan Hajnoczi]

On Thu, Apr 13, 2023 at 04:38:52PM +0100, Peter Maydell wrote:
> On Thu, 17 Nov 2022 at 16:58, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > The Large-Send Task Offload Tx Descriptor (9.2.1 Transmit) has a
> > Large-Send MSS value where the driver specifies the MSS. See the
> > datasheet here:
> > http://realtek.info/pdf/rtl8139cp.pdf
> >
> > The code ignores this value and uses a hardcoded MSS of 1500 bytes
> > instead. When the MTU is less than 1500 bytes the hardcoded value
> > results in IP fragmentation and poor performance.
> >
> > Use the Large-Send MSS value to correctly size Large-Send packets.
> >
> > Jason Wang <jasowang@redhat.com> noticed that the Large-Send MSS value
> > mask was incorrect so it is adjusted to match the datasheet and Linux
> > 8139cp driver.
> 
> Hi Stefan -- in v2 of this patch
> 
> https://lore.kernel.org/qemu-devel/20221116154122.1705399-1-stefanha@redhat.com/
> 
> there was a check for "is the specified large_send_mss value
> too small?":
> 
> +                /* MSS too small? */
> +                if (tcp_hlen + hlen >= large_send_mss) {
> +                    goto skip_offload;
> +                }
> 
> but it isn't present in this final version of the patch which
> went into git. Was that deliberately dropped?
> 
> I ask because the fuzzers have discovered that if you feed this
> device a descriptor where the large_send_mss value is 0, then
> we will now do a division by zero and crash:
> https://gitlab.com/qemu-project/qemu/-/issues/1582
> 
> (The datasheet, naturally, says nothing at all about what
> happens if the descriptor contains a bogus MSS value.)

Yes, I dropped that deliberately and forgot to add a large_send_mss == 0
check. Sorry!

I have sent a patch:
20230413171946.2865726-1-stefanha@redhat.com/">https://patchew.org/QEMU/20230413171946.2865726-1-stefanha@redhat.com/

Thanks,
Stefan

signature.asc
Description: PGP signature