Netdev Archive on
help / color / mirror / Atom feed
From: Casey Schaufler <>
To: Paolo Abeni <>,
Cc: "David S. Miller" <>,
	Jakub Kicinski <>, Florian Westphal <>,
	Eric Dumazet <>,,,
	Casey Schaufler <>
Subject: Re: [PATCH RFC 0/9] sk_buff: optimize layout for GRO
Date: Thu, 22 Jul 2021 09:04:32 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 7/22/2021 12:10 AM, Paolo Abeni wrote:
> Hello,
> On Wed, 2021-07-21 at 11:15 -0700, Casey Schaufler wrote:
>> On 7/21/2021 9:44 AM, Paolo Abeni wrote:
>>> This is a very early draft - in a different world would be
>>> replaced by hallway discussion at in-person conference - aimed at
>>> outlining some ideas and collect feedback on the overall outlook.
>>> There are still bugs to be fixed, more test and benchmark need, etc.
>>> There are 3 main goals:
>>> - [try to] avoid the overhead for uncommon conditions at GRO time
>>>   (patches 1-4)
>>> - enable backpressure for the veth GRO path (patches 5-6)
>>> - reduce the number of cacheline used by the sk_buff lifecycle
>>>   from 4 to 3, at least in some common scenarios (patches 1,7-9).
>>>   The idea here is avoid the initialization of some fields and
>>>   control their validity with a bitmask, as presented by at least
>>>   Florian and Jesper in the past.
>> If I understand correctly, you're creating an optimized case
>> which excludes ct, secmark, vlan and UDP tunnel. Is this correct,
>> and if so, why those particular fields? What impact will this have
>> in the non-optimal (with any of the excluded fields) case?
> Thank you for the feedback.

You're most welcome. You did request comments.

> There are 2 different relevant points:
> - the GRO stage.
>   packets carring any of CT, dst, sk or skb_ext will do 2 additional
> conditionals per gro_receive WRT the current code. My understanding is
> that having any of such field set at GRO receive time is quite
> exceptional for real nic. All others packet will do 4 or 5 less
> conditionals, and will traverse a little less code.
> - sk_buff lifecycle
>   * packets carrying vlan and UDP will not see any differences: sk_buff
> lifecycle will stil use 4 cachelines, as currently does, and no
> additional conditional is introduced.
>   * packets carring nfct or secmark will see an additional conditional
> every time such field is accessed. The number of cacheline used will
> still be 4, as in the current code. My understanding is that when such
> access happens, there is already a relevant amount of "additional" code
> to be executed, the conditional overhead should not be measurable.

I'm responsible for some of that "additonal" code. If the secmark
is considered to be outside the performance critical data there are
changes I would like to make that will substantially improve the
performance of that "additional" code that would include a u64
secmark. If use of a secmark is considered indicative of a "slow"
path, the rationale for restricting it to u32, that it might impact
the "usual" case performance, seems specious. I can't say that I
understand all the nuances and implications involved. It does
appear that the changes you've suggested could negate the classic
argument that requires the u32 secmark.

> Cheers,
> Paolo

  reply	other threads:[~2021-07-22 16:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 16:44 [PATCH RFC 0/9] sk_buff: optimize layout for GRO Paolo Abeni
2021-07-21 18:15 ` Casey Schaufler
2021-07-22  7:10   ` Paolo Abeni
2021-07-22 16:04     ` Casey Schaufler [this message]
2021-07-22 16:57       ` Paolo Abeni
2021-07-22 18:41         ` Paul Moore
2021-07-24 18:51           ` Florian Westphal
2021-07-25 14:57             ` Paul Moore
2021-07-25 16:25               ` Florian Westphal
2021-07-25 21:53                 ` Casey Schaufler
2021-07-25 22:52                   ` Florian Westphal
2021-07-26 15:13                     ` Casey Schaufler
2021-07-27  2:51                       ` Paul Moore
2021-07-28 16:21                         ` Paolo Abeni

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \

* 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).