netdev - Re: [PATCH net-next v6 03/12] net: homa: create shared Homa header files

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: <08c42b4a-6eed-4814-8bf8-fad40de6f2ed@redhat.com>
Date: Thu, 23 Jan 2025 12:01:08 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: John Ousterhout <ouster@...stanford.edu>, netdev@...r.kernel.org
Cc: edumazet@...gle.com, horms@...nel.org, kuba@...nel.org
Subject: Re: [PATCH net-next v6 03/12] net: homa: create shared Homa header
 files

On 1/15/25 7:59 PM, John Ousterhout wrote:
[...]
> +/**
> + * union sockaddr_in_union - Holds either an IPv4 or IPv6 address (smaller
> + * and easier to use than sockaddr_storage).
> + */
> +union sockaddr_in_union {
> +	/** @sa: Used to access as a generic sockaddr. */
> +	struct sockaddr sa;
> +
> +	/** @in4: Used to access as IPv4 socket. */
> +	struct sockaddr_in in4;
> +
> +	/** @in6: Used to access as IPv6 socket.  */
> +	struct sockaddr_in6 in6;
> +};

There are other protocol using the same struct with a different name
(sctp) or  a very similar struct (mptcp). It would be nice to move this
in a shared header and allow re-use.

[...]
> +	/**
> +	 * @core: Core on which @thread was executing when it registered
> +	 * its interest.  Used for load balancing (see balance.txt).
> +	 */
> +	int core;

I don't see a 'balance.txt' file in this submission, possibly stray
reference?

[...]
> +	/**
> +	 * @pacer_wake_time: time (in sched_clock units) when the pacer last
> +	 * woke up (if the pacer is running) or 0 if the pacer is sleeping.
> +	 */
> +	__u64 pacer_wake_time;

why do you use the '__' variant here? this is not uapi, you should use
the plain u64/u32 (more occurrences below).

[...]
> +	/**
> +	 * @prev_default_port: The most recent port number assigned from
> +	 * the range of default ports.
> +	 */
> +	__u16 prev_default_port __aligned(L1_CACHE_BYTES);

I think the idiomatic way to express the above is to use:

	u16 prev_default_port ____cacheline_aligned;

or

	u16 prev_default_port ____cacheline_aligned_in_smp;

more similar occourrences below.

/P