netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: "Ramsay, Lincoln" <Lincoln.Ramsay@digi.com>
Cc: Florian Westphal <fw@strlen.de>,
	Igor Russkikh <irusskikh@marvell.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Dmitry Bogdanov <dbogdanov@marvell.com>
Subject: Re: [PATCH v4] aquantia: Remove the build_skb path
Date: Fri, 20 Nov 2020 01:17:05 +0100	[thread overview]
Message-ID: <20201120001705.GL15137@breakpoint.cc> (raw)
In-Reply-To: <CY4PR1001MB2311844FE8390F00A3363DEEE8E00@CY4PR1001MB2311.namprd10.prod.outlook.com>

Ramsay, Lincoln <Lincoln.Ramsay@digi.com> wrote:

[ patch looks good to me, I have no further comments ]

> > For build_skb path to work the buffer scheme would need to be changed
> > to reserve headroom, so yes, I think that the proposed patch is the
> > most convenient solution.
> 
> I don't know about benefits/feasibility, but I did wonder if (in the event that the "fast path" is possible), the dma_mapping could use an offset? The page would include the skb header but the dma mapping would not. If that was done though, only 1 RX frame would fit into the page (at least on my system, where the RX frame seems to be 2k and the page is 4k). Also, there's a possibility to set the "order" variable, so that multiple pages are created at once and I'm not sure if this would work in that case.

Yes, this is what some drivers do, they allocate a page, pass
pageaddr + headroom_offset everywhere, except build_skb() which gets the
pageaddr followed by skb_reserve(skb, headroom_offset).

> > This only copies the initial part and then the rest is added as a frag.
> 
> Oh yeah. That's not as bad as I had thought then :)
> 
> I wonder though... if the "fast path" is possible, could the whole packet (including header) be added as a frag, avoiding the header copy? Or is that not how SKBs work?

No, you can either have skb->head point to the page (build_skb), or
skb->head needs to be kmalloc'd (napi_alloc_skb for example).

Both can have page frags. In the second case, at least L2 header
needs to be in skb->head (and ip stack would pull in L3 header as well
later on anyway).

  reply	other threads:[~2020-11-20  0:17 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18  1:52 [PATCH] aquantia: Reserve space when allocating an SKB Ramsay, Lincoln
2020-11-18 14:02 ` [EXT] " Igor Russkikh
2020-11-19  0:14   ` Ramsay, Lincoln
2020-11-19  5:19     ` Ramsay, Lincoln
2020-11-19 22:01       ` [PATCH] aquantia: Remove the build_skb path Ramsay, Lincoln
2020-11-19 22:07         ` [PATCH v2] " Ramsay, Lincoln
2020-11-19 22:15           ` Florian Westphal
2020-11-19 22:24             ` Ramsay, Lincoln
2020-11-19 22:28               ` Florian Westphal
2020-11-19 22:34                 ` [PATCH v3] " Ramsay, Lincoln
2020-11-19 22:49                   ` Maciej Fijalkowski
2020-11-20  8:18                     ` [EXT] " Igor Russkikh
2020-11-23 19:28                       ` Maciej Fijalkowski
2020-11-24 15:26                         ` Igor Russkikh
2020-11-19 22:58                   ` Florian Westphal
2020-11-19 23:52                     ` [PATCH v4] " Ramsay, Lincoln
2020-11-20  0:17                       ` Florian Westphal [this message]
2020-11-20  0:23                         ` Ramsay, Lincoln
2020-11-21 21:22                       ` Jakub Kicinski
2020-11-21 21:23                         ` Jakub Kicinski
2020-11-22 22:36                           ` Ramsay, Lincoln
2020-11-23 16:42                             ` Jakub Kicinski
2020-11-23 21:40                               ` [PATCH net v5] " Ramsay, Lincoln
2020-11-24 19:02                                 ` Jakub Kicinski
2020-11-22 21:55                         ` [PATCH v4] " Ramsay, Lincoln
2020-11-20  7:52         ` [EXT] [PATCH] " Igor Russkikh
2020-11-23  4:20           ` Ramsay, Lincoln
2020-11-24 14:29             ` Igor Russkikh

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=20201120001705.GL15137@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=Lincoln.Ramsay@digi.com \
    --cc=davem@davemloft.net \
    --cc=dbogdanov@marvell.com \
    --cc=irusskikh@marvell.com \
    --cc=kuba@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).