Re: [net-next,v17,08/14] net: homa: create homa_rpc.h and homa_rpc.c

[Paolo Abeni <pabeni@redhat.com>]

On 3/23/26 11:43 PM, John Ousterhout wrote:
> On Wed, Mar 18, 2026 at 12:21 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> This approach would allow lockless RCU lookups and faster reclamation. Paolo
>> referenced tcp established socket lookup as an example:
>>
>>   "Have a look at tcp established socket lookup and the SLAB_TYPESAFE_BY_RCU
>>    flag usage for slab-based allocations. A combo of such flag for RPC
>>    allocation (using a dedicated kmem_cache) and RCU lookup should improve
>>    consistently the performances, with a consolidate code layout and no
>>    unmanageable problems with large number of objects waiting for the grace
>>    period."
>>
>> The current code still uses plain kzalloc/kfree without a dedicated cache.
>> Would it make sense to implement this optimization?
>>
>> Reference: https://lore.kernel.org/netdev/9209dfbb-ca3a-4fb7-a2fb-0567394f8cda@redhat.com/
> 
> I tried using a kmem_cache, but it made performance slightly worse:
> average allocation time for a struct homa_rpc increased from about 380
> cycles to about 400 cycles in my benchmarks. Since there is no
> performance advantage to using kmem_cache I removed it.

So kzalloc/kfree is faster than kmem_cache?!? That is quite unexpected,
possibly the kmem_cache pointer ends up on a contended cacheline?!? I
guess it would be useful to understand the extra overhead, as kmalloc is
basically another layer on top of kmem_cache.

> I'm surprised how long allocation takes: 400 cycles seems like a *lot*
> if there are per-cpu caches of the right size. Presumably no
> synchronization is needed and all the function has to do in the normal
> case is pull the top entry off a list.
> 
>>
>>> +     if (!srpc) {
>>> +             err = -ENOMEM;
>>> +             goto error;
>>> +     }
>>
>> [ ... ]
>>
>>> +error:
>>> +     homa_bucket_unlock(bucket, id);
>>> +     if (srpc && srpc->peer)
>>> +             homa_peer_release(srpc->peer);
>>> +     kfree(srpc);
>>> +     return ERR_PTR(err);
>>> +}
>>
>> [ ... ]
>>
>> In the v6 review thread, Paolo and Eric raised concerns about SKB freeing
>> costs and suggested optimizations. Eric noted that proper software GRO
>> significantly reduces SKB count using napi_get_frags() and napi_reuse_skb().
> 
> Unfortunately napi_reuse_skb is not currently exported. My
> understanding is that I need to wait until Homa GRO is upstreamed
> before I can request that it be exported. Once that happens I'll be
> happy to use napi_reuse_skb.

Ok

>> Paolo also suggested using deferred SKB freeing (skb_attempt_defer_free) to
>> reduce the cost. While you mentioned that GRO is implemented in the full
>> Homa but was left out of the initial patch series, would it be beneficial to
>> include at least some of these optimizations in this initial submission?
>>
>> Reference: https://lore.kernel.org/netdev/9209dfbb-ca3a-4fb7-a2fb-0567394f8cda@redhat.com/
> 
> When I asked ChatGPT about homa_attempt_defer_free it said that it
> only works if the skb has no dst, no destructor, and no skb_nfct. I'm
> a bit unsure how to proceed. Is homa_attempt_defer_free only intended
> for incoming packets? If so, then presumably there is no dst? But what
> about the other fields? Do I need to do something to clean them up
> before calling homa_attempt_defer_free?
skb_attempt_defer_free() usage is intended for the RX path only.

Incoming packets get state (dst, extensions, ct...) attached in the rx
path before reaching the socket. You should free/release the state, if
possible, before inserting the skb into the receive queue, so that you
could defer free them after read.

/P