netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Gerhard Engleder <gerhard@engleder-embedded.com>
Cc: Alexander H Duyck <alexander.duyck@gmail.com>,
	netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
	pabeni@redhat.com
Subject: Re: [PATCH net-next v4 10/10] tsnep: Support XDP BPF program setup
Date: Wed, 11 Jan 2023 12:06:06 -0800	[thread overview]
Message-ID: <20230111120606.6a209472@kernel.org> (raw)
In-Reply-To: <8e6fae4d-83f4-e31c-2274-208e27e7b156@engleder-embedded.com>

On Wed, 11 Jan 2023 20:11:44 +0100 Gerhard Engleder wrote:
> I agree with you that this pattern is bad. Most XDP BPF program setup do
> it like that, but this is of course no valid argument.
> 
> In the last review round I made the following suggestion (but got no
> reply so far):
> 
> What about always using 'XDP_PACKET_HEADROOM' as offset in the RX
> buffer? The offset 'NET_SKB_PAD + NET_IP_ALIGN' would not even be used
> if XDP is not enabled. Changing this offset is the only task to be done
> at the first XDP BFP prog setup call. By always using this offset
> no
> 
> 	close()
> 	change config
> 	open()
> 
> pattern is needed. As a result no handling for failed open() is needed
> and __TSNEP_DOWN is not needed. Simpler code with less problems in my
> opinion.
> 
> The only problem could be that NET_IP_ALIGN is not used, but
> NET_IP_ALIGN is 0 anyway on the two platforms (x86, arm64) where this
> driver is used.

You can add NET_IP_ALIGN as well, AFAIU XDP_PACKET_HEADROOM is more 
of a minimum headroom than an exact one.

The other thing off the top of my head is that you'll always need to
DMA map the Rx buffers BIDIR.

If those are acceptable for your platform / applications then indeed
seems like an easy way out of the problem!

      reply	other threads:[~2023-01-11 20:08 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09 19:15 [PATCH net-next v4 00/10] tsnep: XDP support Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 01/10] tsnep: Use spin_lock_bh for TX Gerhard Engleder
2023-01-10 15:49   ` Alexander H Duyck
2023-01-10 19:44     ` Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 02/10] tsnep: Forward NAPI budget to napi_consume_skb() Gerhard Engleder
2023-01-10 15:50   ` Alexander H Duyck
2023-01-09 19:15 ` [PATCH net-next v4 03/10] tsnep: Do not print DMA mapping error Gerhard Engleder
2023-01-10 15:53   ` Alexander H Duyck
2023-01-10 19:47     ` Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 04/10] tsnep: Add adapter down state Gerhard Engleder
2023-01-10 16:05   ` Alexander H Duyck
2023-01-10 20:16     ` Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 05/10] tsnep: Add XDP TX support Gerhard Engleder
2023-01-10 16:56   ` Alexander H Duyck
2023-01-10 21:07     ` Gerhard Engleder
2023-01-10 22:38       ` Alexander Duyck
2023-01-11 18:51         ` Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 06/10] tsnep: Substract TSNEP_RX_INLINE_METADATA_SIZE once Gerhard Engleder
2023-01-10 17:05   ` Alexander H Duyck
2023-01-09 19:15 ` [PATCH net-next v4 07/10] tsnep: Prepare RX buffer for XDP support Gerhard Engleder
2023-01-09 19:15 ` [PATCH net-next v4 08/10] tsnep: Add RX queue info " Gerhard Engleder
2023-01-10 17:29   ` Alexander H Duyck
2023-01-10 21:21     ` Gerhard Engleder
2023-01-10 22:27       ` Alexander Duyck
2023-01-09 19:15 ` [PATCH net-next v4 09/10] tsnep: Add XDP RX support Gerhard Engleder
2023-01-10 17:40   ` Alexander H Duyck
2023-01-10 21:28     ` Gerhard Engleder
2023-01-10 22:30       ` Alexander Duyck
2023-01-09 19:15 ` [PATCH net-next v4 10/10] tsnep: Support XDP BPF program setup Gerhard Engleder
2023-01-10 17:33   ` Alexander H Duyck
2023-01-10 21:38     ` Gerhard Engleder
2023-01-10 23:00       ` Alexander H Duyck
2023-01-11 18:58         ` Gerhard Engleder
2023-01-11  0:12       ` Jakub Kicinski
2023-01-11 19:11         ` Gerhard Engleder
2023-01-11 20:06           ` Jakub Kicinski [this message]

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=20230111120606.6a209472@kernel.org \
    --to=kuba@kernel.org \
    --cc=alexander.duyck@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gerhard@engleder-embedded.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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).