netdev - Re: [PATCH net-next v2 1/2] net: remove init_dummy

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: <95b33055-5cb4-4ba0-880f-f39a565c963b@kernel.org>
Date: Mon, 13 Jan 2025 17:16:23 +0100
From: Matthieu Baerts <matttbe@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 martineau@...nel.org, geliang@...nel.org, steffen.klassert@...unet.com,
 herbert@...dor.apana.org.au, mptcp@...ts.linux.dev
Subject: Re: [PATCH net-next v2 1/2] net: remove init_dummy_netdev()

Hi Jakub,

On 13/01/2025 01:34, Jakub Kicinski wrote:
> init_dummy_netdev() can initialize statically declared or embedded
> net_devices. Such netdevs did not come from alloc_netdev_mqs().
> After recent work by Breno, there are the only two cases where
> we have do that.
> 
> Switch those cases to alloc_netdev_mqs() and delete init_dummy_netdev().
> Dealing with static netdevs is not worth the maintenance burden.

Thank you for the patch!

(...)

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 1b2e7cbb577f..c44c89ecaca6 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -47,7 +47,7 @@ static void __mptcp_destroy_sock(struct sock *sk);
>  static void mptcp_check_send_data_fin(struct sock *sk);
>  
>  DEFINE_PER_CPU(struct mptcp_delegated_action, mptcp_delegated_actions);
> -static struct net_device mptcp_napi_dev;
> +static struct net_device *mptcp_napi_dev;
>  
>  /* Returns end sequence number of the receiver's advertised window */
>  static u64 mptcp_wnd_end(const struct mptcp_sock *msk)
> @@ -4147,11 +4147,13 @@ void __init mptcp_proto_init(void)
>  	if (percpu_counter_init(&mptcp_sockets_allocated, 0, GFP_KERNEL))
>  		panic("Failed to allocate MPTCP pcpu counter\n");
>  
> -	init_dummy_netdev(&mptcp_napi_dev);
> +	mptcp_napi_dev = alloc_netdev_dummy(0);

This seems to initialise quite a few things that are not needed for the
MPTCP case. But it doesn't seem to hurt, and the dummy net device
doesn't seem to be exposed to the userspace (net devices, sysfs). So if
it helps with the maintenance, it looks fine to do that.

For the modifications in MPTCP code:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>

> +	if (!mptcp_napi_dev)
> +		panic("Failed to allocate MPTCP dummy netdev\n");
>  	for_each_possible_cpu(cpu) {
>  		delegated = per_cpu_ptr(&mptcp_delegated_actions, cpu);
>  		INIT_LIST_HEAD(&delegated->head);
> -		netif_napi_add_tx(&mptcp_napi_dev, &delegated->napi,
> +		netif_napi_add_tx(mptcp_napi_dev, &delegated->napi,
>  				  mptcp_napi_poll);
>  		napi_enable(&delegated->napi);
>  	}

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.