| 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
| ||
|
Message-ID: <20200721103636.6d1054b5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date: Tue, 21 Jul 2020 10:36:36 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, Alice Michael <alice.michael@...el.com>,
netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
jeffrey.t.kirsher@...el.com, Alan Brady <alan.brady@...el.com>,
Phani Burra <phani.r.burra@...el.com>,
Joshua Hay <joshua.a.hay@...el.com>,
Madhu Chittim <madhu.chittim@...el.com>,
Pavan Kumar Linga <pavan.kumar.linga@...el.com>,
Donald Skidmore <donald.c.skidmore@...el.com>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Sridhar Samudrala <sridhar.samudrala@...el.com>
Subject: Re: [net-next v4 02/15] iecm: Add framework set of header files
On Mon, 20 Jul 2020 17:37:57 -0700 Tony Nguyen wrote:
> diff --git a/include/linux/net/intel/iecm_alloc.h b/include/linux/net/intel/iecm_alloc.h
> new file mode 100644
> index 000000000000..fd13bf085663
> --- /dev/null
> +++ b/include/linux/net/intel/iecm_alloc.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2020, Intel Corporation. */
> +
> +#ifndef _IECM_ALLOC_H_
> +#define _IECM_ALLOC_H_
> +
> +/* Memory types */
> +enum iecm_memset_type {
> + IECM_NONDMA_MEM = 0,
> + IECM_DMA_MEM
> +};
> +
> +/* Memcpy types */
> +enum iecm_memcpy_type {
> + IECM_NONDMA_TO_NONDMA = 0,
> + IECM_NONDMA_TO_DMA,
> + IECM_DMA_TO_DMA,
> + IECM_DMA_TO_NONDMA
> +};
These enums are never used. Please remove all dead code in the submission.
(Unused HW interface defines are okay.)
> diff --git a/include/linux/net/intel/iecm.h b/include/linux/net/intel/iecm.h
> new file mode 100644
> index 000000000000..ef6e4d79f51f
> --- /dev/null
> +++ b/include/linux/net/intel/iecm.h
> @@ -0,0 +1,430 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2020 Intel Corporation */
> +
> +#ifndef _IECM_H_
> +#define _IECM_H_
> +
> +#include <linux/aer.h>
> +#include <linux/bitmap.h>
> +#include <linux/compiler.h>
> +#include <linux/cpumask.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/errno.h>
> +#include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> +#include <linux/if_bridge.h>
> +#include <linux/if_vlan.h>
> +#include <linux/interrupt.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/pci.h>
> +#include <linux/pkt_sched.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/sctp.h>
> +#include <linux/skbuff.h>
> +#include <linux/string.h>
> +#include <linux/timer.h>
> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +#include <net/tcp.h>
> +#include <net/ipv6.h>
> +#include <net/ip6_checksum.h>
> +#include <linux/prefetch.h>
> +#include <net/pkt_sched.h>
> +
> +#include <net/gre.h>
> +#include <net/udp_tunnel.h>
> +#include <linux/avf/virtchnl.h>
> +
> +#include <linux/net/intel/iecm_osdep.h>
> +#include <linux/net/intel/iecm_controlq_api.h>
> +#include <linux/net/intel/iecm_lan_txrx.h>
> +
> +#include <linux/net/intel/iecm_txrx.h>
> +#include <linux/net/intel/iecm_type.h>
Please trim this litany of include files. You don't need most of them.
> diff --git a/include/linux/net/intel/iecm_osdep.h b/include/linux/net/intel/iecm_osdep.h
> new file mode 100644
> index 000000000000..33e01582f516
> --- /dev/null
> +++ b/include/linux/net/intel/iecm_osdep.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2020, Intel Corporation. */
> +
> +#ifndef _IECM_OSDEP_H_
> +#define _IECM_OSDEP_H_
OS abstraction layers are not allowed in Linux drivers. Please rename
this to something meaningful or marge the contents to appropriate
headers and remove this one.
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/net/intel/iecm_alloc.h>
None of these headers are required here. Yet you fail to include io.h..
> +#define wr32(a, reg, value) writel((value), ((a)->hw_addr + (reg)))
> +#define rd32(a, reg) readl((a)->hw_addr + (reg))
> +#define wr64(a, reg, value) writeq((value), ((a)->hw_addr + (reg)))
> +#define rd64(a, reg) readq((a)->hw_addr + (reg))
> +
> +struct iecm_dma_mem {
> + void *va;
> + dma_addr_t pa;
> + size_t size;
> +};
> +
> +#define iecm_wmb() wmb() /* memory barrier */
Please remove this define and use wmb() directly.
> +#endif /* _IECM_OSDEP_H_ */
> diff --git a/include/linux/net/intel/iecm_controlq.h b/include/linux/net/intel/iecm_controlq.h
> new file mode 100644
> index 000000000000..4cba637042cd
> --- /dev/null
> +++ b/include/linux/net/intel/iecm_controlq.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2020, Intel Corporation. */
> +
> +#ifndef _IECM_CONTROLQ_H_
> +#define _IECM_CONTROLQ_H_
> +
> +#include <linux/net/intel/iecm_controlq_api.h>
> +
> +/* Maximum buffer lengths for all control queue types */
> +#define IECM_CTLQ_MAX_RING_SIZE 1024
> +#define IECM_CTLQ_MAX_BUF_LEN 4096
> +
> +#define IECM_CTLQ_DESC(R, i) \
> + (&(((struct iecm_ctlq_desc *)((R)->desc_ring.va))[i]))
> +
> +#define IECM_CTLQ_DESC_UNUSED(R) \
> + (u16)((((R)->next_to_clean > (R)->next_to_use) ? 0 : (R)->ring_size) + \
> + (R)->next_to_clean - (R)->next_to_use - 1)
> +
> +/* Data type manipulation macros. */
> +#define IECM_HI_DWORD(x) ((u32)((((x) >> 16) >> 16) & 0xFFFFFFFF))
> +#define IECM_LO_DWORD(x) ((u32)((x) & 0xFFFFFFFF))
Please use lower_32_bits() / upper_32_bits() directly.
> +#define IECM_HI_WORD(x) ((u16)(((x) >> 16) & 0xFFFF))
> +#define IECM_LO_WORD(x) ((u16)((x) & 0xFFFF))
unused
Powered by blists - more mailing lists