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] [day] [month] [year] [list]

Message-Id: <d4ae26f7-e7c8-4332-ae2f-0e51ecdbf05f@yunsilicon.com>
Date: Fri, 20 Dec 2024 18:43:17 +0800
From: "tianx" <tianx@...silicon.com>
To: "Andrew Lunn" <andrew@...n.ch>
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

On 2024/12/19 2:28, Andrew Lunn wrote:
>> +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.
OK, will fix in v2.
>> +	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?

It's OK because xdev->netdev is never used before register_netdev() returns.

>
> And why the void * cast?

unnecessary cast, will fix that

>> +/* 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
will delete that in v2