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

On 09.12.22 01:53, Saeed Mahameed wrote:

On 08 Dec 21:32, Gerhard Engleder wrote:

quoted

On 08.12.22 13:59, Maciej Fijalkowski wrote:

quoted

On Thu, Dec 08, 2022 at 06:40:44AM +0100, Gerhard Engleder wrote:

quoted

Register xdp_rxq_info with page_pool memory model. This is needed for
XDP buffer handling.

Signed-off-by: Gerhard Engleder <redacted>
---
 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?

a rule of thumb, organize the structure in the same order they are
being accessed in the data path.. but this doesn't go without saying you
need to do some layout testing via pahole for example..
It's up to you and the maintainer of this driver to decide how critical 
this
is.

Reviewed-by: Saeed Mahameed <saeed@kernel.org>

Thanks for the clarification, I will think about it.

I wrote the driver and I'm responsible to keep it working. Is this equal
to being the maintainer?

Thanks for the review!

Gerhard