netdev - Re: [net] net: feature check mandating HW

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKgT0UfKENJg6QtORLc-nQD+p9-s7c9_Fw2R5g3iPsmSZfT+jQ@mail.gmail.com>
Date:   Fri, 15 Jan 2021 08:59:16 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     rohit maheshwari <rohitm@...lsio.com>
Cc:     Tariq Toukan <ttoukan.linux@...il.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>, secdev@...lsio.com,
        Alexei Starovoitov <ast@...nel.org>,
        Björn Töpel <bjorn.topel@...el.com>,
        Daniel Borkmann <daniel@...earbox.net>, andriin@...com,
        Eric Dumazet <edumazet@...gle.com>,
        Cong Wang <xiyou.wangcong@...il.com>, ap420073@...il.com,
        Jiri Pirko <jiri@...lanox.com>, borisp@...dia.com,
        Tariq Toukan <tariqt@...dia.com>
Subject: Re: [net] net: feature check mandating HW_CSUM is wrong

On Thu, Jan 14, 2021 at 9:39 PM rohit maheshwari <rohitm@...lsio.com> wrote:
>
>
> On 13/01/21 10:37 PM, Tariq Toukan wrote:
> >
> >
> > On 1/13/2021 5:35 AM, Alexander Duyck wrote:
> >> On Tue, Jan 12, 2021 at 6:43 PM rohit maheshwari <rohitm@...lsio.com>
> >> wrote:
> >>>
> >>>
> >>> On 07/01/21 12:47 AM, Jakub Kicinski wrote:
> >>>> On Wed,  6 Jan 2021 23:23:27 +0530 Rohit Maheshwari wrote:
> >>>>> Mandating NETIF_F_HW_CSUM to enable TLS offload feature is wrong.
> >>>>> And it broke tls offload feature for the drivers, which are still
> >>>>> using NETIF_F_IP_CSUM or NETIF_F_IPV6_CSUM. We should use
> >>>>> NETIF_F_CSUM_MASK instead.
> >>>>>
> >>>>> Fixes: ae0b04b238e2 ("net: Disable NETIF_F_HW_TLS_TX when HW_CSUM
> >>>>> is disabled")
> >>>>> Signed-off-by: Rohit Maheshwari <rohitm@...lsio.com>
> >>>> Please use Tariq's suggestion.
> >>> HW_TLS_TX feature is for both IPv4/v6. And If one device is limited to
> >>> support only IPv4 checksum offload, TLS offload should be allowed for
> >>> that too. So I think putting a check of CSUM_MASK is enough.
> >>
> >> The issue is that there is no good software fallback if the packet
> >> arrives at the NIC and it cannot handle the IPv6 TLS offload.
> >>
> >> The problem with the earlier patch you had is that it was just
> >> dropping frames if you couldn't handle the offload and "hoping" the
> >> other end would catch it. That isn't a good practice for any offload.
> >> If you have it enabled you have to have a software fallback and in
> >> this case it sounds like you don't have that.
> >>
> >> In order to do that you would have to alter the fast path to have to
> >> check if the device is capable per packet which is really an
> >> undesirable setup as it would add considerable overhead and is open to
> >> race conditions.
> >>
> >
> > +1
> >
> > Are there really such modern devices with missing IPv6 csum
> > capabilities, or it's just a missing SW implementation in the device
> > driver?
> >
> > IMO, it sounds reasonable for this modern TLS device offload to asks
> > for a basic requirement such as IPv6 csum offload capability, to save
> > the overhead.
> >
> I agree with you, all modern devices support V6 csum, but still if we think
> logically, we can't limit TLS offload to work only if both IPV4_CSUM  and
> IPV6_CSUM are configured/supported. If there is no dependency of IPV6
> in running TLS offload with IPv4  and vice-versa, then why should there
> be any restriction as such.

The requirement is to have a software fallback for any offload that
cannot be performed by the hardware if you are going to advertise it.
So if we were to disable IPv6 checksum offload and then request TLS
offload there isn't a software fallback for the combination since the
L4 header checksum cannot be performed on the packet if we are
expecting the hardware to handle the encryption. So in such a case the
TLS offload will need to occur before software can offload the header
checksum and so the hardware offload is disabled.

The basic idea is any offload combination should be functional so that
it doesn't mangle frames or cause the receiving end to drop packets.