All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Jonathan Lemon <jonathan.lemon@gmail.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	Network Development <netdev@vger.kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH 0/9 v1 RFC] Generic zcopy_* functions
Date: Sat, 19 Dec 2020 14:00:55 -0500	[thread overview]
Message-ID: <CA+FuTSfcxCncqzUsQh22A5Kdha_+wXmE=tqPk4SiJ3+CEui_Vw@mail.gmail.com> (raw)
In-Reply-To: <20201218211648.rh5ktnkm333sw4hf@bsd-mbp.dhcp.thefacebook.com>

On Fri, Dec 18, 2020 at 4:27 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>
> On Fri, Dec 18, 2020 at 03:49:44PM -0500, Willem de Bruijn wrote:
> > On Fri, Dec 18, 2020 at 3:23 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> > >
> > > From: Jonathan Lemon <bsd@fb.com>
> > >
> > > This is set of cleanup patches for zerocopy which are intended
> > > to allow a introduction of a different zerocopy implementation.
> >
> > Can you describe in more detail what exactly is lacking in the current
> > zerocopy interface for this this different implementation? Or point to
> > a github tree with the feature patches attached, perhaps.
>
> I'll get the zctap features up into a github tree.
>
> Essentially, I need different behavior from ubuf_info:
>   - no refcounts on RX packets (static ubuf)

That is already the case for vhost and tpacket zerocopy use cases.

>   - access to the skb on RX skb free (for page handling)

To refers only to patch 9, moving the callback earlier in
skb_release_data, right?

>   - no page pinning on TX/tx completion

That is not part of the skb zerocopy infrastructure?

>   - marking the skb data as inaccessible so skb_condense()
>     and skb_zeroocopy_clone() leave it alone.

Yep. Skipping content access on the Rx path will be interesting. I
wonder if that should be a separate opaque skb feature, independent
from whether the data is owned by userspace, peripheral memory, the
page cache or anything else.

> > I think it's good to split into multiple smaller patchsets, starting
> > with core stack support. But find it hard to understand which of these
> > changes are truly needed to support a new use case.
>
> Agree - kind of hard to see why this is done without a use case.
> These patches are purely restructuring, and don't introduce any
> new features.
>
>
> > If anything, eating up the last 8 bits in skb_shared_info should be last resort.
>
> I would like to add 2 more bits in the future, which is why I
> moved them.  Is there a compelling reason to leave the bits alone?

Opportunity cost.

We cannot grow skb_shared_info due to colocation with MTU sized linear
skbuff's in half a page.

It took me quite some effort to free up a few bytes in commit
4d276eb6a478 ("net: remove deprecated syststamp timestamp").

If we are very frugal, we could shadow some bits to have different
meaning in different paths. SKBTX_IN_PROGRESS is transmit only, I
think. But otherwise we'll have to just dedicate the byte to more
flags. Yours are likely not to be the last anyway.

  reply	other threads:[~2020-12-19 19:02 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-18 20:16 [PATCH 0/9 v1 RFC] Generic zcopy_* functions Jonathan Lemon
2020-12-18 20:16 ` [PATCH 1/9 v1 RFC] net: group skb_shinfo zerocopy related bits together Jonathan Lemon
2020-12-18 20:16 ` [PATCH 2/9 v1 RFC] skbuff: remove unused skb_zcopy_abort function Jonathan Lemon
2020-12-18 20:16 ` [PATCH 3/9 v1 RFC] skbuff: replace sock_zerocopy_put() with skb_zcopy_put() Jonathan Lemon
2020-12-19 18:46   ` Willem de Bruijn
2020-12-21 19:18     ` Jonathan Lemon
2020-12-21 22:49       ` Willem de Bruijn
2020-12-18 20:16 ` [PATCH 4/9 v1 RFC] skbuff: replace sock_zerocopy_get with skb_zcopy_get Jonathan Lemon
2020-12-18 20:16 ` [PATCH 5/9 v1 RFC] skbuff: Add skb parameter to the ubuf zerocopy callback Jonathan Lemon
2020-12-18 20:16 ` [PATCH 6/9 v1 RFC] skbuff: Call sock_zerocopy_put_abort from skb_zcopy_put_abort Jonathan Lemon
2020-12-18 20:16 ` [PATCH 7/9 v1 RFC] skbuff: add zc_flags to ubuf_info for ubuf setup Jonathan Lemon
2020-12-18 20:16 ` [PATCH 8/9 v1 RFC] tap/tun: use skb_zcopy_set() instead of open coded assignment Jonathan Lemon
2020-12-18 20:16 ` [PATCH 9/9 v1 RFC] skbuff: Call skb_zcopy_clear() before unref'ing fragments Jonathan Lemon
2020-12-18 20:49 ` [PATCH 0/9 v1 RFC] Generic zcopy_* functions Willem de Bruijn
2020-12-18 21:16   ` Jonathan Lemon
2020-12-19 19:00     ` Willem de Bruijn [this message]
2020-12-21 19:50       ` Jonathan Lemon
2020-12-21 22:52         ` Willem de Bruijn
2020-12-22  0:07           ` Jonathan Lemon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+FuTSfcxCncqzUsQh22A5Kdha_+wXmE=tqPk4SiJ3+CEui_Vw@mail.gmail.com' \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=edumazet@google.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.