On 1/30/2022 5:59 AM, Ansuel Smith wrote:

quoted

Hi,
sorry for the delay in sending v8, it's ready but I'm far from home and
I still need to check some mdio improvement with pointer handling.

Anyway I have some concern aboutall the skb alloc.
I wonder if that part can be improved at the cost of some additional
space used.

The idea Is to use the cache stuff also for the eth skb (or duplicate
it?) And use something like build_skb and recycle the skb space
everytime...
This comes from the fact that packet size is ALWAYS the same and it
seems stupid to allocate and free it everytime. Considering we also
enforce a one way transaction (we send packet and we wait for response)
this makes the allocation process even more stupid.

So I wonder if we would have some perf improvement/less load by
declaring the mgmt eth space and build an skb that always use that
preallocate space and just modify data.

I would really love some feedback considering qca8k is also used in very
low spec ath79 device where we need to reduce the load in every way
possible. Also if anyone have more ideas on how to improve this to make
it less heavy cpu side, feel free to point it out even if it would
mean that my implemenation is complete sh*t.

(The use of caching the address would permit us to reduce the write to
this preallocated space even more or ideally to send the same skb)

I would say first things first: get this patch series included since it 
is very close from being suitable for inclusion in net-next. Then you 
can profile the I/O accesses over the management Ethernet frames and 
devise a strategy to optimize them to make as little CPU cycles 
intensive as possible.

build_skb() is not exactly a magic bullet that will solve all 
performance problems, you still need the non-data portion of the skb to 
be allocated, and also keep in mind that you need tail room at the end 
of the data buffer in order for struct skb_shared_info to be written. 
This means that the hardware is not allowed to write at the end of the 
data buffer, or you must reduce the maximum RX length accordingly to 
prevent that. Your frames are small enough here this is unlikely to be 
an issue.

Since the MDIO layer does not really allow more than one outstanding 
transaction per MDIO device at a time, you might be just fine with just 
have a front and back skb set of buffers and alternating between these two.
-- 
Florian