netdev - Re: [PATCH net-next 02/10] udp: add gso

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Apr 2018 13:48:21 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
Cc:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Dimitris Michailidis <dmichail@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next 02/10] udp: add gso

Sent a v2 with all but the below suggestion incorporated.

>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index ff49e352deea..c647cfe114e0 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -4940,6 +4940,8 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
>>                 thlen = tcp_hdrlen(skb);
>>         } else if (unlikely(skb_is_gso_sctp(skb))) {
>>                 thlen = sizeof(struct sctphdr);
>> +       } else if (shinfo->gso_type & SKB_GSO_UDP_L4) {
>> +               thlen = sizeof(struct udphdr);
>>         }
>>         /* UFO sets gso_size to the size of the fragmentation
>>          * payload, i.e. the size of the L4 (UDP) header is already
>
> It might make more sense to look at converting this over to a switch
> statement based off of shinfo(skb)->gso_type & GSO_TRANSPORT_MASK,
> where the transport mask consists of the 4 bits that are supported.

I decided to skip this. The types SKB_GSO_{TCPV4, TCPV6, SCTP, UDP_L4}
are far apart in the enum namespace and the tests have to use & instead
of direct comparison. I did not see an obvious way to have the compiler
convert this into a jump table.

Doing so is also a bit out of scope of the feature, so even if feasible
without too much gymnastics I suggest doing so in a separate patch.