netdev - Re: [PATCH net-next v9 09/14] xsc: 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: <20250325051934.002db3cd@kernel.org>
Date: Tue, 25 Mar 2025 05:19:34 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: "Xin Tian" <tianx@...silicon.com>
Cc: <netdev@...r.kernel.org>, <leon@...nel.org>, <andrew+netdev@...n.ch>,
 <pabeni@...hat.com>, <edumazet@...gle.com>, <davem@...emloft.net>,
 <jeff.johnson@....qualcomm.com>, <przemyslaw.kitszel@...el.com>,
 <weihg@...silicon.com>, <wanry@...silicon.com>, <jacky@...silicon.com>,
 <horms@...nel.org>, <parthiban.veerasooran@...rochip.com>,
 <masahiroy@...nel.org>, <kalesh-anakkur.purayil@...adcom.com>,
 <geert+renesas@...der.be>, <geert@...ux-m68k.org>
Subject: Re: [PATCH net-next v9 09/14] xsc: Init net device

On Tue, 18 Mar 2025 23:15:11 +0800 Xin Tian wrote:
> +static int xsc_eth_set_hw_mtu(struct xsc_core_device *xdev,
> +			      u16 mtu, u16 rx_buf_sz)
> +{
> +	struct xsc_set_mtu_mbox_out out;
> +	struct xsc_set_mtu_mbox_in in;
> +	int ret;
> +
> +	memset(&in, 0, sizeof(struct xsc_set_mtu_mbox_in));
> +	memset(&out, 0, sizeof(struct xsc_set_mtu_mbox_out));
> +
> +	in.hdr.opcode = cpu_to_be16(XSC_CMD_OP_SET_MTU);
> +	in.mtu = cpu_to_be16(mtu);
> +	in.rx_buf_sz_min = cpu_to_be16(rx_buf_sz);
> +	in.mac_port = xdev->mac_port;
> +
> +	ret = xsc_cmd_exec(xdev, &in, sizeof(struct xsc_set_mtu_mbox_in), &out,
> +			   sizeof(struct xsc_set_mtu_mbox_out));
> +	if (ret || out.hdr.status) {
> +		netdev_err(((struct xsc_adapter *)xdev->eth_priv)->netdev,

Please use temporary variable or define a local print macro.
The cast is too ugly.

> +			   "failed to set hw_mtu=%u rx_buf_sz=%u, err=%d, status=%d\n",
> +			   mtu, rx_buf_sz, ret, out.hdr.status);
> +		ret = -ENOEXEC;

Why are you overwriting the ret code from xsc_cmd_exec() ?
And why with such an unusual errno ?

> +static int xsc_eth_get_mac(struct xsc_core_device *xdev, char *mac)
> +{
> +	struct xsc_query_eth_mac_mbox_out *out;
> +	struct xsc_query_eth_mac_mbox_in in;
> +	int err = 0;
> +
> +	out = kzalloc(sizeof(*out), GFP_KERNEL);
> +	if (!out)
> +		return -ENOMEM;
> +
> +	memset(&in, 0, sizeof(in));
> +	in.hdr.opcode = cpu_to_be16(XSC_CMD_OP_QUERY_ETH_MAC);
> +
> +	err = xsc_cmd_exec(xdev, &in, sizeof(in), out, sizeof(*out));
> +	if (err || out->hdr.status) {
> +		netdev_err(((struct xsc_adapter *)xdev->eth_priv)->netdev,
> +			   "get mac failed! err=%d, out.status=%u\n",
> +			   err, out->hdr.status);
> +		err = -ENOEXEC;
> +		goto err_free;
> +	}
> +
> +	memcpy(mac, out->mac, 6);

6 -> ETH_ALEN or ether_addr_copy()

> +
> +err_free:
> +	kfree(out);
> +	return err;
> +}
> +
> +static void xsc_eth_l2_addr_init(struct xsc_adapter *adapter)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +	char mac[6] = {0};
> +	int ret = 0;
> +
> +	ret = xsc_eth_get_mac(adapter->xdev, mac);
> +	if (ret) {
> +		netdev_err(netdev, "get mac failed %d, generate random mac...",
> +			   ret);
> +		eth_random_addr(mac);

eth_hw_addr_random()

> +	}
> +	dev_addr_mod(netdev, 0, mac, 6);

Why not dev_addr_set() ?!