Message-ID: <a109d5c6-76d6-47c5-834d-9f263f254b5c@lunn.ch>
Date: Wed, 30 Oct 2024 02:03:36 +0100
From: Andrew Lunn <andrew@...n.ch>
To: John Ousterhout <ouster@...stanford.edu>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next 11/12] net: homa: create homa_plumbing.c
 homa_utils.c

> +static int __init homa_load(void)
> +{
> +	int status;
> +
> +	pr_notice("Homa module loading\n");
> +	pr_notice("Homa structure sizes: data_header %u, seg_header %u, ack %u, peer %u, ip_hdr %u flowi %u ipv6_hdr %u, flowi6 %u tcp_sock %u homa_rpc %u sk_buff %u rcvmsg_control %u union sockaddr_in_union %u HOMA_MAX_BPAGES %u NR_CPUS %u nr_cpu_ids %u, MAX_NUMNODES %d\n",
> +		  sizeof32(struct data_header),
> +		  sizeof32(struct seg_header),
> +		  sizeof32(struct homa_ack),
> +		  sizeof32(struct homa_peer),
> +		  sizeof32(struct iphdr),
> +		  sizeof32(struct flowi),
> +		  sizeof32(struct ipv6hdr),
> +		  sizeof32(struct flowi6),
> +		  sizeof32(struct tcp_sock),
> +		  sizeof32(struct homa_rpc),
> +		  sizeof32(struct sk_buff),
> +		  sizeof32(struct homa_recvmsg_args),
> +		  sizeof32(union sockaddr_in_union),
> +		  HOMA_MAX_BPAGES,
> +		  NR_CPUS,
> +		  nr_cpu_ids,
> +		  MAX_NUMNODES);

We generally don't want kernel modules spamming the log. I would
suggest dropping these the pr_dbg(), or removing them altogether.

> +	status = proto_register(&homa_prot, 1);
> +	if (status != 0) {
> +		pr_err("proto_register failed for homa_prot: %d\n", status);
> +		goto out;
> +	}
> +	status = proto_register(&homav6_prot, 1);
> +	if (status != 0) {
> +		pr_err("proto_register failed for homav6_prot: %d\n", status);
> +		goto out;
> +	}
> +	inet_register_protosw(&homa_protosw);
> +	inet6_register_protosw(&homav6_protosw);
> +	status = inet_add_protocol(&homa_protocol, IPPROTO_HOMA);
> +	if (status != 0) {
> +		pr_err("inet_add_protocol failed in %s: %d\n", __func__,
> +		       status);
> +		goto out_cleanup;
> +	}
> +	status = inet6_add_protocol(&homav6_protocol, IPPROTO_HOMA);
> +	if (status != 0) {
> +		pr_err("inet6_add_protocol failed in %s: %d\n",  __func__,
> +		       status);
> +		goto out_cleanup;
> +	}
> +
> +	status = homa_init(homa);
> +	if (status)
> +		goto out_cleanup;
> +
> +	timer_kthread = kthread_run(homa_timer_main, homa, "homa_timer");
> +	if (IS_ERR(timer_kthread)) {
> +		status = PTR_ERR(timer_kthread);
> +		pr_err("couldn't create homa pacer thread: error %d\n",
> +		       status);
> +		timer_kthread = NULL;
> +		goto out_cleanup;
> +	}
> +
> +	return 0;
> +
> +out_cleanup:
> +	homa_destroy(homa);
> +	inet_del_protocol(&homa_protocol, IPPROTO_HOMA);
> +	inet_unregister_protosw(&homa_protosw);
> +	inet6_del_protocol(&homav6_protocol, IPPROTO_HOMA);
> +	inet6_unregister_protosw(&homav6_protosw);
> +	proto_unregister(&homa_prot);
> +	proto_unregister(&homav6_prot);
> +out:
> +	return status;
> +}
> +
> +/**
> + * homa_unload() - invoked when this module is unloaded from the Linux kernel.
> + */
> +static void __exit homa_unload(void)
> +{
> +	pr_notice("Homa module unloading\n");
> +	exiting = true;
> +
> +	if (timer_kthread)
> +		wake_up_process(timer_kthread);
> +	wait_for_completion(&timer_thread_done);
> +	homa_destroy(homa);
> +	inet_del_protocol(&homa_protocol, IPPROTO_HOMA);
> +	inet_unregister_protosw(&homa_protosw);
> +	inet6_del_protocol(&homav6_protocol, IPPROTO_HOMA);
> +	inet6_unregister_protosw(&homav6_protosw);
> +	proto_unregister(&homa_prot);
> +	proto_unregister(&homav6_prot);
> +}
> +
> +module_init(homa_load);
> +module_exit(homa_unload);
> +
> +/**
> + * homa_bind() - Implements the bind system call for Homa sockets: associates
> + * a well-known service port with a socket. Unlike other AF_INET6 protocols,
> + * there is no need to invoke this system call for sockets that are only
> + * used as clients.
> + * @sock:     Socket on which the system call was invoked.
> + * @addr:    Contains the desired port number.
> + * @addr_len: Number of bytes in uaddr.
> + * Return:    0 on success, otherwise a negative errno.
> + */
> +int homa_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
> +{
> +	struct homa_sock *hsk = homa_sk(sock->sk);
> +	union sockaddr_in_union *addr_in = (union sockaddr_in_union *)addr;
> +	int port = 0;
> +
> +	if (unlikely(addr->sa_family != sock->sk->sk_family))
> +		return -EAFNOSUPPORT;
> +	if (addr_in->in6.sin6_family == AF_INET6) {
> +		if (addr_len < sizeof(struct sockaddr_in6))
> +			return -EINVAL;
> +		port = ntohs(addr_in->in4.sin_port);
> +	} else if (addr_in->in4.sin_family == AF_INET) {
> +		if (addr_len < sizeof(struct sockaddr_in))
> +			return -EINVAL;
> +		port = ntohs(addr_in->in6.sin6_port);
> +	}
> +	return homa_sock_bind(homa->port_map, hsk, port);
> +}
> +
> +/**
> + * homa_close() - Invoked when close system call is invoked on a Homa socket.
> + * @sk:      Socket being closed
> + * @timeout: ??
> + */
> +void homa_close(struct sock *sk, long timeout)
> +{
> +	struct homa_sock *hsk = homa_sk(sk);
> +
> +	homa_sock_destroy(hsk);
> +	sk_common_release(sk);
> +}
> +
> +/**
> + * homa_shutdown() - Implements the shutdown system call for Homa sockets.
> + * @sock:    Socket to shut down.
> + * @how:     Ignored: for other sockets, can independently shut down
> + *           sending and receiving, but for Homa any shutdown will
> + *           shut down everything.
> + *
> + * Return: 0 on success, otherwise a negative errno.
> + */
> +int homa_shutdown(struct socket *sock, int how)
> +{
> +	homa_sock_shutdown(homa_sk(sock->sk));
> +	return 0;
> +}
> +
> +/**
> + * homa_disconnect() - Invoked when disconnect system call is invoked on a
> + * Homa socket.
> + * @sk:    Socket to disconnect
> + * @flags: ??
> + *
> + * Return: 0 on success, otherwise a negative errno.
> + */
> +int homa_disconnect(struct sock *sk, int flags)
> +{
> +	pr_warn("unimplemented disconnect invoked on Homa socket\n");
> +	return -EINVAL;

Can this be used to DoS the system, spam the log? At minimum rate
limit the message, but it might be better to just remove the message.

> +}
> +
> +/**
> + * homa_ioctl() - Implements the ioctl system call for Homa sockets.
> + * @sk:    Socket on which the system call was invoked.
> + * @cmd:   Identifier for a particular ioctl operation.
> + * @karg:  Operation-specific argument; typically the address of a block
> + *         of data in user address space.
> + *
> + * Return: 0 on success, otherwise a negative errno.
> + */
> +int homa_ioctl(struct sock *sk, int cmd, int *karg)
> +{
> +	return -EINVAL;
> +}

Is this actually needed? Core code will generally test to see if the
op is not NULL before calling it.

> +/**
> + * homa_getsockopt() - Implements the getsockopt system call for Homa sockets.
> + * @sk:      Socket on which the system call was invoked.
> + * @level:   ??
> + * @optname: Identifies a particular setsockopt operation.
> + * @optval:  Address in user space where the option's value should be stored.
> + * @option:  ??.
> + * Return:   0 on success, otherwise a negative errno.
> + */
> +int homa_getsockopt(struct sock *sk, int level, int optname,
> +		    char __user *optval, int __user *option)
> +{
> +	pr_warn("unimplemented getsockopt invoked on Homa socket: level %d, optname %d\n",
> +		level, optname);

Another way to spam the log.

	Andrew

Powered by blists - more mailing lists