netdev - Re: [PATCH net-next 01/12] net: homa: define user-visible API for 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <174d72f1-6636-538a-72d2-fd20f9c4cbd0@gmail.com>
Date: Thu, 7 Nov 2024 21:58:11 +0000
From: Edward Cree <ecree.xilinx@...il.com>
To: John Ousterhout <ouster@...stanford.edu>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 01/12] net: homa: define user-visible API for
 Homa

On 28/10/2024 21:35, John Ousterhout wrote:
> Note: for man pages, see the Homa Wiki at:
> https://homa-transport.atlassian.net/wiki/spaces/HOMA/overview
> 
> Signed-off-by: John Ousterhout <ouster@...stanford.edu>
...
> +/**
> + * Holds either an IPv4 or IPv6 address (smaller and easier to use than
> + * sockaddr_storage).
> + */
> +union sockaddr_in_union {
> +	struct sockaddr sa;
> +	struct sockaddr_in in4;
> +	struct sockaddr_in6 in6;
> +};

Are there fundamental reasons why Homa can only run over IP and not
 other L3 networks?  Or performance measurements showing that the
 cost of using sockaddr_storage is excessive?
Otherwise, baking this into the uAPI seems unwise.

> +	/**
> +	 * @error_addr: the address of the peer is stored here when available.
> +	 * This field is different from the msg_name field in struct msghdr
> +	 * in that the msg_name field isn't set after errors. This field will
> +	 * always be set when peer information is available, which includes
> +	 * some error cases.
> +	 */
> +	union sockaddr_in_union peer_addr;

Member name (peer_addr) doesn't match the kerneldoc (@error_addr).

> +int     homa_send(int sockfd, const void *message_buf,
> +		  size_t length, const union sockaddr_in_union *dest_addr,
> +		  uint64_t *id, uint64_t completion_cookie);
> +int     homa_sendv(int sockfd, const struct iovec *iov,
> +		   int iovcnt, const union sockaddr_in_union *dest_addr,
> +		   uint64_t *id, uint64_t completion_cookie);
> +ssize_t homa_reply(int sockfd, const void *message_buf,
> +		   size_t length, const union sockaddr_in_union *dest_addr,
> +		   uint64_t id);
> +ssize_t homa_replyv(int sockfd, const struct iovec *iov,
> +		    int iovcnt, const union sockaddr_in_union *dest_addr,
> +		    uint64_t id);

I don't think these belong in here.  They seem to be userland
 library functions which wrap the sendmsg syscall, and as far as
 I can tell the definitions corresponding to these prototypes do
 not appear in the patch series.