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]
Message-ID: <CAF=yD-+LNSA3c+FdGtXc9YFQyt9Y77MQHZdw4gDZShwHgkb0CQ@mail.gmail.com>
Date:   Mon, 2 Jul 2018 10:45:29 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     borisp@...lanox.com
Cc:     Alexander Duyck <alexander.duyck@...il.com>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>, ogerlitz@...lanox.com,
        yossiku@...lanox.com
Subject: Re: [net-next 01/12] net/mlx5e: Add UDP GSO support

On Mon, Jul 2, 2018 at 9:34 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> On Mon, Jul 2, 2018 at 1:30 AM Boris Pismenny <borisp@...lanox.com> wrote:
> >
> >
> >
> > On 7/2/2018 4:45 AM, Willem de Bruijn wrote:
> > >>> I've noticed that we could get cleaner code in our driver if we remove
> > >>> these two lines from net/ipv4/udp_offload.c:
> > >>> if (skb_is_gso(segs))
> > >>>                mss *= skb_shinfo(segs)->gso_segs;
> > >>>
> > >>> I think that this is correct in case of GSO_PARTIAL segmentation for the
> > >>> following reasons:
> > >>> 1. After this change the UDP payload field is consistent with the IP
> > >>> header payload length field. Currently, IPv4 length is 1500 and UDP
> > >>> total length is the full unsegmented length.
> > >
> > > How does this simplify the driver? Does it currently have to
> > > change the udph->length field to the mss on the wire, because the
> > > device only splits + replicates the headers + computes the csum?
> >
> > Yes, this is the code I have at the moment.
> >
> > The device's limitation is more subtle than this. It could adjust the
> > length, but then the checksum would be wrong.
>
> I see. We do have to keep in mind other devices. Alexander's ixgbe
> RFC patch does not have this logic, so that device must update the
> field directly.
>
>   https://patchwork.ozlabs.org/patch/908396/

To be clear, I think it's fine to remove these two lines if it does not cause
problems for the ixgbe. It's quite possible that that device sets the udp
length field unconditionally, ignoring the previous value. In which case both
devices will work after this change without additional driver logic.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ