linux-kernel - Re: [RFC net-next v02 1/3] net: hinic3: module initialization and tx/rx logic

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: <20241212170256.GA3347301@bhelgaas>
Date: Thu, 12 Dec 2024 11:02:56 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Gur Stavi <gur.stavi@...wei.com>
Cc: gongfan <gongfan1@...wei.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Andrew Lunn <andrew+netdev@...n.ch>, linux-doc@...r.kernel.org,
	Jonathan Corbet <corbet@....net>,
	Cai Huoqing <cai.huoqing@...ux.dev>, Xin Guo <guoxin09@...wei.com>,
	Shen Chenyang <shenchenyang1@...ilicon.com>,
	Zhou Shuai <zhoushuai28@...wei.com>, Wu Like <wulike1@...wei.com>,
	Shi Jing <shijing34@...wei.com>,
	Meny Yossefi <meny.yossefi@...wei.com>
Subject: Re: [RFC net-next v02 1/3] net: hinic3: module initialization and
 tx/rx logic

On Thu, Dec 12, 2024 at 02:04:15PM +0200, Gur Stavi wrote:
> From: gongfan <gongfan1@...wei.com>
> 
> This is [1/3] part of hinic3 Ethernet driver initial submission.
> With this patch hinic3 is a valid kernel module but non-functional driver.

> +++ b/Documentation/networking/device_drivers/ethernet/huawei/hinic3.rst
> @@ -0,0 +1,136 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================================================================
> +Linux kernel driver for Huawei Ethernet Device Driver (hinic3) family
> +=====================================================================
> +
> +Overview
> +========
> +
> +The hinic3 is a network interface card(NIC) for Data Center. It supports

Add space before "(".

> +Prime Physical Function (PPF) is responsible for the management of the
> +whole NIC card. For example, clock synchronization between the NIC and
> +the host.
> +Any PF may serve as a PPF. The PPF is selected dynamically.

Add blank line between paragraphs or reflow into a single paragraph.

Is the PPF selected dynamically by the driver?  By firmware on the
NIC?

> +hinic3_pci_id_tbl.h       Supported device IDs.
> +hinic3_hw_intf.h          Interface between HW and driver.
> +hinic3_queue_common.[ch]  Common structures and methods for NIC queues.
> +hinic3_common.[ch]        Encapsulation of memory operations in Linux.
> +hinic3_csr.h              Register definitions in the BAR.
> +hinic3_hwif.[ch]          Interface for BAR.
> +hinic3_eqs.[ch]           Interface for AEQs and CEQs.
> +hinic3_mbox.[ch]          Interface for mailbox.
> +hinic3_mgmt.[ch]          Management interface based on mailbox and AEQ.
> +hinic3_wq.[ch]            Work queue data structures and interface.
> +hinic3_cmdq.[ch]          Command queue is used to post command to HW.
> +hinic3_hwdev.[ch]         HW structures and methods abstractions.
> +hinic3_lld.[ch]           Auxiliary driver adaptation layer.
> +hinic3_hw_comm.[ch]       Interface for common HW operations.
> +hinic3_mgmt_interface.h   Interface between firmware and driver.
> +hinic3_hw_cfg.[ch]        Interface for HW configuration.
> +hinic3_irq.c              Interrupt request
> +hinic3_netdev_ops.c       Operations registered to Linux kernel stack.
> +hinic3_nic_dev.h          NIC structures and methods abstractions.
> +hinic3_main.c             Main Linux kernel driver.
> +hinic3_nic_cfg.[ch]       NIC service configuration.
> +hinic3_nic_io.[ch]        Management plane interface for TX and RX.
> +hinic3_rss.[ch]           Interface for Receive Side Scaling (RSS).
> +hinic3_rx.[ch]            Interface for transmit.
> +hinic3_tx.[ch]            Interface for receive.
> +hinic3_ethtool.c          Interface for ethtool operations (ops).
> +hinic3_filter.c           Interface for mac address.

Could drop "." at end (or use it consistently).

s/mac/MAC/

> +2 mailbox related events.
> +
> +MailBox

s/MailBox/Mailbox/ since that's how you use it elsewhere.

> +-------
> +
> +Mailbox is a communication mechanism between the hinic3 driver and the HW.

> +The implementation of CEQ is the same as AEQ. It receives completion events
> +form HW over a fixed size descriptor of 32 bits. Every device can have up
> +to 32 CEQs. Every CEQ has a dedicated IRQ. CEQ only receives solicited
> +events that are responses to requests from the driver. CEQ can receive
> +multiple types of events, but in practice the hinic3 driver ignores all
> +events except for HINIC3_CMDQ that represents completion of previously
> +posted commands on a cmdq.

s/form HW/from HW/

> +Work queues are logical arrays of fixed size WQEs. The array may be spread
> +over multiple non-contiguous pages using indirection table.

Add blank line or wrap into single paragraph.

> +Work queues are used by I/O queues and command queues.

> +Every function, PF or VF, has a unique ordinal identification within the device.
> +Many commands to management (mbox or cmdq) contain this ID so HW can apply the
> +command effect to the right function.

Add blank line or wrap into single paragraph.

> +PF is allowed to post management commands to a subordinate VF by specifying the
> +VFs ID. A VF must provide its own ID. Anti-spoofing in the HW will cause
> +command from a VF to fail if it contains the wrong ID.

> +config HINIC3
> +	tristate "Huawei Intelligent Network Interface Card 3rd"
> +	# Fields of HW and management structures are little endian and will not
> +	# be explicitly converted

I guess this comment is here to explain the !CPU_BIG_ENDIAN below?
That's quite an unusual dependency.

> +	depends on 64BIT && !CPU_BIG_ENDIAN

> +++ b/drivers/net/ethernet/huawei/hinic3/Makefile
> @@ -0,0 +1,21 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) Huawei Technologies Co., Ltd. 2024. All rights reserved.
> +
> +obj-$(CONFIG_HINIC3) += hinic3.o
> +
> +hinic3-objs := hinic3_hwdev.o \
> +	       hinic3_lld.o \
> +	       hinic3_common.o \
> +	       hinic3_hwif.o \
> +	       hinic3_hw_cfg.o \
> +	       hinic3_queue_common.o \
> +	       hinic3_mbox.o \
> +	       hinic3_hw_comm.o \
> +	       hinic3_wq.o \
> +	       hinic3_nic_io.o \
> +	       hinic3_nic_cfg.o \
> +	       hinic3_tx.o \
> +	       hinic3_rx.o \
> +	       hinic3_netdev_ops.o \
> +	       hinic3_rss.o \
> +	       hinic3_main.o
> \ No newline at end of file

Add newline.