netdev - Re: [PATCH v1 09/16] net-next/yunsilicon: Init net device

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: <51fa5341-4e7b-4c76-8fd5-9ca1f4b57de7@lunn.ch>
Date: Wed, 18 Dec 2024 19:28:32 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Xin Tian <tianx@...silicon.com>
Cc: netdev@...r.kernel.org, andrew+netdev@...n.ch, kuba@...nel.org,
	pabeni@...hat.com, edumazet@...gle.com, davem@...emloft.net,
	jeff.johnson@....qualcomm.com, przemyslaw.kitszel@...el.com,
	weihg@...silicon.com, wanry@...silicon.com
Subject: Re: [PATCH v1 09/16] net-next/yunsilicon: Init net device

> +static int xsc_attach_netdev(struct xsc_adapter *adapter)
> +{
> +	int err = -1;
> +
> +	err = xsc_eth_nic_enable(adapter);
> +	if (err)
> +		return err;
> +
> +	xsc_core_info(adapter->xdev, "%s ok\n", __func__);

...

> +static int xsc_eth_attach(struct xsc_core_device *xdev, struct xsc_adapter *adapter)
> +{
> +	int err = -1;
> +
> +	if (netif_device_present(adapter->netdev))
> +		return 0;
> +
> +	err = xsc_attach_netdev(adapter);
> +	if (err)
> +		return err;
> +
> +	xsc_core_info(adapter->xdev, "%s ok\n", __func__);

Don't spam the log like this. _dbg() or nothing.

> +	err = xsc_eth_nic_init(adapter, rep_priv, num_chl, num_tc);
> +	if (err) {
> +		xsc_core_warn(xdev, "xsc_nic_init failed, num_ch=%d, num_tc=%d, err=%d\n",
> +			      num_chl, num_tc, err);
> +		goto err_free_netdev;
> +	}
> +
> +	err = xsc_eth_attach(xdev, adapter);
> +	if (err) {
> +		xsc_core_warn(xdev, "xsc_eth_attach failed, err=%d\n", err);
> +		goto err_cleanup_netdev;
> +	}
> +
>  	err = register_netdev(netdev);
>  	if (err) {
>  		xsc_core_warn(xdev, "register_netdev failed, err=%d\n", err);
> -		goto err_free_netdev;
> +		goto err_detach;
>  	}
>  
>  	xdev->netdev = (void *)netdev;

Before register_netdev() returns, the device is live and sending
packets, especially if you are using NFS root. What will happen if
xdev->netdev is NULL with those first few packets?

And why the void * cast? 

> +/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
> +/*
> + * Copyright (C) 2021-2025, Shanghai Yunsilicon Technology Co., Ltd.
> + * Copyright (c) 2015-2016, Mellanox Technologies. All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */

The /* SPDX-License-Identifier: line replaces all such license
boilerplate. Please delete this.

	Andrew