netdev - Re: [PATCH net-next v4 10/10] tsnep: Support XDP BPF program setup

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] [day] [month] [year] [list]

Date:   Wed, 11 Jan 2023 12:06:06 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Gerhard Engleder <gerhard@...leder-embedded.com>
Cc:     Alexander H Duyck <alexander.duyck@...il.com>,
        netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com
Subject: Re: [PATCH net-next v4 10/10] tsnep: Support XDP BPF program setup

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!