netdev - Re: [PATCH net-next v2 5/6] tsnep: Add RX queue info for XDP support

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

Message-ID: <45d4de61-9a6e-be55-5a00-9e7ff464f4be@engleder-embedded.com>
Date:   Thu, 8 Dec 2022 21:32:23 +0100
From:   Gerhard Engleder <gerhard@...leder-embedded.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
        kuba@...nel.org, edumazet@...gle.com, pabeni@...hat.com,
        ast@...nel.org, daniel@...earbox.net, hawk@...nel.org,
        john.fastabend@...il.com
Subject: Re: [PATCH net-next v2 5/6] tsnep: Add RX queue info for XDP support

On 08.12.22 13:59, Maciej Fijalkowski wrote:
> On Thu, Dec 08, 2022 at 06:40:44AM +0100, Gerhard Engleder wrote:
>> Register xdp_rxq_info with page_pool memory model. This is needed for
>> XDP buffer handling.
>>
>> Signed-off-by: Gerhard Engleder <gerhard@...leder-embedded.com>
>> ---
>>   drivers/net/ethernet/engleder/tsnep.h      |  5 ++--
>>   drivers/net/ethernet/engleder/tsnep_main.c | 34 +++++++++++++++++-----
>>   2 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h
>> index 0e7fc36a64e1..70bc133d4a9d 100644
>> --- a/drivers/net/ethernet/engleder/tsnep.h
>> +++ b/drivers/net/ethernet/engleder/tsnep.h
>> @@ -127,6 +127,7 @@ struct tsnep_rx {
>>   	u32 owner_counter;
>>   	int increment_owner_counter;
>>   	struct page_pool *page_pool;
>> +	struct xdp_rxq_info xdp_rxq;
> 
> this occupies full cacheline, did you make sure that you don't break
> tsnep_rx layout with having xdp_rxq_info in the middle of the way?

Actually I did no cacheline optimisation for this structure so far.
I saw that igb/igc put xdp_rxq_info to the end. Is this best practice
to prevent other variables in the same cacheline of xdp_rxq?

>>   
>>   	u32 packets;
>>   	u32 bytes;
>> @@ -139,11 +140,11 @@ struct tsnep_queue {
>>   	struct tsnep_adapter *adapter;
>>   	char name[IFNAMSIZ + 9];
>>   
>> +	struct napi_struct napi;
>> +
>>   	struct tsnep_tx *tx;
>>   	struct tsnep_rx *rx;
>>   
>> -	struct napi_struct napi;
> 
> why this move?

I reordered it because napi is now initialised before tx/rx. A cosmetic
move.

Gerhard