netdev - Re: [PATCH net-next v4 2/3] udp: elide zerocopy operation in hot path

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]

Message-ID: <0799fd4fabead127a44d90db1758f1365e93b777.camel@redhat.com>
Date:   Mon, 03 Dec 2018 13:50:05 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        netdev@...r.kernel.org
Cc:     davem@...emloft.net, Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next v4 2/3] udp: elide zerocopy operation in hot
 path

On Fri, 2018-11-30 at 15:32 -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@...gle.com>
> 
> With MSG_ZEROCOPY, each skb holds a reference to a struct ubuf_info.
> Release of its last reference triggers a completion notification.
> 
> The TCP stack in tcp_sendmsg_locked holds an extra ref independent of
> the skbs, because it can build, send and free skbs within its loop,
> possibly reaching refcount zero and freeing the ubuf_info too soon.
> 
> The UDP stack currently also takes this extra ref, but does not need
> it as all skbs are sent after return from __ip(6)_append_data.
> 
> Avoid the extra refcount_inc and refcount_dec_and_test, and generally
> the sock_zerocopy_put in the common path, by passing the initial
> reference to the first skb.
> 
> This approach is taken instead of initializing the refcount to 0, as
> that would generate error "refcount_t: increment on 0" on the
> next skb_zcopy_set.
> 
> Changes
>   v3 -> v4
>     - Move skb_zcopy_set below the only kfree_skb that might cause
>       a premature uarg destroy before skb_zerocopy_put_abort
>       - Move the entire skb_shinfo assignment block, to keep that
>         cacheline access in one place
> 
> Signed-off-by: Willem de Bruijn <willemb@...gle.com>

I like this solution!

Acked-by: Paolo Abeni <pabeni@...hat.com>