netdev - Re: [PATCH net-next v9 08/15] net: homa: create homa_pacer.h and homa

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250613144340.GL414686@horms.kernel.org>
Date: Fri, 13 Jun 2025 15:43:40 +0100
From: Simon Horman <horms@...nel.org>
To: John Ousterhout <ouster@...stanford.edu>
Cc: netdev@...r.kernel.org, pabeni@...hat.com, edumazet@...gle.com,
	kuba@...nel.org
Subject: Re: [PATCH net-next v9 08/15] net: homa: create homa_pacer.h and
 homa_pacer.c

On Mon, Jun 09, 2025 at 08:40:41AM -0700, John Ousterhout wrote:
> These files provide facilities to pace packet output in order to prevent
> queue buildup in the NIC. This functionality is needed to implement SRPT
> on output, so short messages don't get stuck in long NIC queues. Note: the
> pacer eventually needs to be replaced with a Homa-specific qdisc, which can
> better manage simultaneous transmissions by Homa and TCP. The current
> implementation can coexist with TCP and doesn't harm TCP, but
> Homa's latency suffers when TCP runs concurrently.
> 
> Signed-off-by: John Ousterhout <ouster@...stanford.edu>
> 
> ---
> Changes for v9:
> * Add support for homa_net objects
> * Use new homa_clock abstraction layer
> * Various name improvements (e.g. use "alloc" instead of "new" for functions
>   that allocate memory)
> 
> Changes for v8:
> * This file is new in v8 (functionality extracted from other files)
> ---
>  net/homa/homa_impl.h  |   1 +
>  net/homa/homa_pacer.c | 316 ++++++++++++++++++++++++++++++++++++++++++
>  net/homa/homa_pacer.h | 190 +++++++++++++++++++++++++
>  3 files changed, 507 insertions(+)
>  create mode 100644 net/homa/homa_pacer.c
>  create mode 100644 net/homa/homa_pacer.h
> 

> diff --git a/net/homa/homa_impl.h b/net/homa/homa_impl.h

...

> +/**
> + * homa_pacer_manage_rpc() - Arrange for the pacer to transmit packets
> + * from this RPC (make sure that an RPC is on the throttled list and wake up
> + * the pacer thread if necessary).
> + * @rpc:     RPC with outbound packets that have been granted but can't be
> + *           sent because of NIC queue restrictions. Must be locked by caller.
> + */
> +void homa_pacer_manage_rpc(struct homa_rpc *rpc)
> +	__must_hold(rpc_bucket_lock)
> +{
> +	struct homa_pacer *pacer = rpc->hsk->homa->pacer;
> +	struct homa_rpc *candidate;
> +	int bytes_left;
> +	int checks = 0;

Checks is set but otherwise unused in this function.
Probably it can be removed.

Flagged by Clang 20.1.4 as:

  .../homa_pacer.c:252:6: warning: variable 'checks' set but not used [-Wunused-but-set-variable]
    252 |         int checks = 0;
        |             ^
> +
> +	if (!list_empty(&rpc->throttled_links))
> +		return;
> +	bytes_left = rpc->msgout.length - rpc->msgout.next_xmit_offset;
> +	homa_pacer_throttle_lock(pacer);
> +	list_for_each_entry(candidate, &pacer->throttled_rpcs,
> +			    throttled_links) {
> +		int bytes_left_cand;
> +
> +		checks++;
> +
> +		/* Watch out: the pacer might have just transmitted the last
> +		 * packet from candidate.
> +		 */
> +		bytes_left_cand = candidate->msgout.length -
> +				candidate->msgout.next_xmit_offset;
> +		if (bytes_left_cand > bytes_left) {
> +			list_add_tail(&rpc->throttled_links,
> +				      &candidate->throttled_links);
> +			goto done;
> +		}
> +	}
> +	list_add_tail(&rpc->throttled_links, &pacer->throttled_rpcs);
> +done:
> +	homa_pacer_throttle_unlock(pacer);
> +	wake_up(&pacer->wait_queue);
> +}

...