netdev - Re: [PATCH net-next 1/3] net: core: add helper tcp_v6_gso_csum

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: <f349d932-582c-96f9-fa7a-b76c615adb96@gmail.com>
Date:   Tue, 18 Feb 2020 19:53:48 +0100
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        Jay Cliburn <jcliburn@...il.com>,
        Chris Snook <chris.snook@...il.com>,
        Rasesh Mody <rmody@...vell.com>,
        Sudarsana Kalluru <skalluru@...vell.com>,
        GR-Linux-NIC-Dev@...vell.com,
        Christian Benvenuti <benve@...co.com>,
        Govindarajulu Varadarajan <_govind@....com>,
        Parvi Kaustubhi <pkaustub@...co.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Guo-Fu Tseng <cooldavid@...ldavid.org>,
        Shannon Nelson <snelson@...sando.io>,
        Pensando Drivers <drivers@...sando.io>,
        Timur Tabi <timur@...nel.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Sasha Levin <sashal@...nel.org>,
        Ronak Doshi <doshir@...are.com>,
        "VMware, Inc." <pv-drivers@...are.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        linux-hyperv@...r.kernel.org,
        Linux USB Mailing List <linux-usb@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] net: core: add helper tcp_v6_gso_csum_prep

On 18.02.2020 19:25, Alexander Duyck wrote:
> On Mon, Feb 17, 2020 at 1:41 PM Heiner Kallweit <hkallweit1@...il.com> wrote:
>>
>> Several network drivers for chips that support TSO6 share the same code
>> for preparing the TCP header. A difference is that some reset the
>> payload_len whilst others don't do this. Let's factor out this common
>> code to a new helper.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>> ---
>>  include/net/ip6_checksum.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
>> index 7bec95df4..ef0130023 100644
>> --- a/include/net/ip6_checksum.h
>> +++ b/include/net/ip6_checksum.h
>> @@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
>>         }
>>  }
>>
>> +static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
>> +                                       bool clear_payload_len)
>> +{
>> +       struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>> +       struct tcphdr *th = tcp_hdr(skb);
>> +
>> +       if (clear_payload_len)
>> +               ipv6h->payload_len = 0;
>> +
>> +       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
>> +}
>> +
>>  #if IS_ENABLED(CONFIG_IPV6)
>>  static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
>>  {
> 
> So functionally I believe this is correct. The only piece I have a
> question about is if we should just force the clear_payload_len as
> always being the case since the value should either be
> ignored/overwritten in any GSO case anyway.
> 
I also thought about this and just wasn't sure whether this functional
change may break any driver. But yes, then let's change it this way.
Breaking down the series into smaller patches makes it easier to
revert an individual patch in case of a problem.

> Reviewed-by: Alexander Duyck <alexander.h.duyck@...ux.intel.com>
> 
Thanks for the review.