net: VMware virtual Ethernet NIC driver: vmxnet3
in [Kernel]
|
Prev: WARNING: at net/ipv4/af_inet.c:154 inet_sock_destruct
Next: tracing: Fix infinite loop in ftrace_update_pid_func()
From: Chris Wright on 29 Sep 2009 05:00 * Shreyas Bhatewara (sbhatewara(a)vmware.com) wrote: > Some of the features of vmxnet3 are : > PCIe 2.0 compliant PCI device: Vendor ID 0x15ad, Device ID 0x07b0 > INTx, MSI, MSI-X (25 vectors) interrupts > 16 Rx queues, 8 Tx queues Driver doesn't appear to actually support more than a single MSI-X interrupt. What is your plan for doing real multiqueue? > Offloads: TCP/UDP checksum, TSO over IPv4/IPv6, > 802.1q VLAN tag insertion, filtering, stripping > Multicast filtering, Jumbo Frames How about GRO conversion? > Wake-on-LAN, PCI Power Management D0-D3 states > PXE-ROM for boot support > Whole thing appears to be space indented, and is fairly noisy w/ printk. Also, heavy use of BUG_ON() (counted 51 of them), are you sure that none of them can be triggered by guest or remote (esp. the ones that happen in interrupt context)? Some initial thoughts below. <snip> > diff --git a/drivers/net/vmxnet3/upt1_defs.h b/drivers/net/vmxnet3/upt1_defs.h > new file mode 100644 > index 0000000..b50f91b > --- /dev/null > +++ b/drivers/net/vmxnet3/upt1_defs.h > @@ -0,0 +1,104 @@ > +/* > + * Linux driver for VMware's vmxnet3 ethernet NIC. > + * > + * Copyright (C) 2008-2009, VMware, Inc. All Rights Reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; version 2 of the License and no later version. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. > + * > + * The full GNU General Public License is included in this distribution in > + * the file called "COPYING". > + * > + * Maintained by: Shreyas Bhatewara <pv-drivers(a)vmware.com> > + * > + */ > + > +/* upt1_defs.h > + * > + * Definitions for Uniform Pass Through. > + */ Most of the source files have this format (some include -- after file name). Could just keep it all w/in the same comment block. Since you went to the trouble of saying what the file does, something a tad more descriptive would be welcome. > + > +#ifndef _UPT1_DEFS_H > +#define _UPT1_DEFS_H > + > +#define UPT1_MAX_TX_QUEUES 64 > +#define UPT1_MAX_RX_QUEUES 64 This is different than the 16/8 described above (and seemingly all moot since it becomes a single queue device). > + > +/* interrupt moderation level */ > +#define UPT1_IML_NONE 0 /* no interrupt moderation */ > +#define UPT1_IML_HIGHEST 7 /* least intr generated */ > +#define UPT1_IML_ADAPTIVE 8 /* adpative intr moderation */ enum? also only appears to support adaptive mode? > +/* values for UPT1_RSSConf.hashFunc */ > +enum { > + UPT1_RSS_HASH_TYPE_NONE = 0x0, > + UPT1_RSS_HASH_TYPE_IPV4 = 0x01, > + UPT1_RSS_HASH_TYPE_TCP_IPV4 = 0x02, > + UPT1_RSS_HASH_TYPE_IPV6 = 0x04, > + UPT1_RSS_HASH_TYPE_TCP_IPV6 = 0x08, > +}; > + > +enum { > + UPT1_RSS_HASH_FUNC_NONE = 0x0, > + UPT1_RSS_HASH_FUNC_TOEPLITZ = 0x01, > +}; > + > +#define UPT1_RSS_MAX_KEY_SIZE 40 > +#define UPT1_RSS_MAX_IND_TABLE_SIZE 128 > + > +struct UPT1_RSSConf { > + uint16_t hashType; > + uint16_t hashFunc; > + uint16_t hashKeySize; > + uint16_t indTableSize; > + uint8_t hashKey[UPT1_RSS_MAX_KEY_SIZE]; > + uint8_t indTable[UPT1_RSS_MAX_IND_TABLE_SIZE]; > +}; > + > +/* features */ > +enum { > + UPT1_F_RXCSUM = 0x0001, /* rx csum verification */ > + UPT1_F_RSS = 0x0002, > + UPT1_F_RXVLAN = 0x0004, /* VLAN tag stripping */ > + UPT1_F_LRO = 0x0008, > +}; > +#endif > diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h > new file mode 100644 > index 0000000..a33a90b > --- /dev/null > +++ b/drivers/net/vmxnet3/vmxnet3_defs.h > @@ -0,0 +1,534 @@ > +/* > + * Linux driver for VMware's vmxnet3 ethernet NIC. > + * > + * Copyright (C) 2008-2009, VMware, Inc. All Rights Reserved. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; version 2 of the License and no later version. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or > + * NON INFRINGEMENT. See the GNU General Public License for more > + * details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. > + * > + * The full GNU General Public License is included in this distribution in > + * the file called "COPYING". > + * > + * Maintained by: Shreyas Bhatewara <pv-drivers(a)vmware.com> > + * > + */ > + > +/* > + * vmxnet3_defs.h -- Not particularly useful ;-) > + */ > + > +#ifndef _VMXNET3_DEFS_H_ > +#define _VMXNET3_DEFS_H_ > + > +#include "upt1_defs.h" > + > +/* all registers are 32 bit wide */ > +/* BAR 1 */ > +enum { > + VMXNET3_REG_VRRS = 0x0, /* Vmxnet3 Revision Report Selection */ > + VMXNET3_REG_UVRS = 0x8, /* UPT Version Report Selection */ > + VMXNET3_REG_DSAL = 0x10, /* Driver Shared Address Low */ > + VMXNET3_REG_DSAH = 0x18, /* Driver Shared Address High */ > + VMXNET3_REG_CMD = 0x20, /* Command */ > + VMXNET3_REG_MACL = 0x28, /* MAC Address Low */ > + VMXNET3_REG_MACH = 0x30, /* MAC Address High */ > + VMXNET3_REG_ICR = 0x38, /* Interrupt Cause Register */ > + VMXNET3_REG_ECR = 0x40 /* Event Cause Register */ > +}; > + > +/* BAR 0 */ > +enum { > + VMXNET3_REG_IMR = 0x0, /* Interrupt Mask Register */ > + VMXNET3_REG_TXPROD = 0x600, /* Tx Producer Index */ > + VMXNET3_REG_RXPROD = 0x800, /* Rx Producer Index for ring 1 */ > + VMXNET3_REG_RXPROD2 = 0xA00 /* Rx Producer Index for ring 2 */ > +}; > + > +#define VMXNET3_PT_REG_SIZE 4096 /* BAR 0 */ > +#define VMXNET3_VD_REG_SIZE 4096 /* BAR 1 */ > + > +#define VMXNET3_REG_ALIGN 8 /* All registers are 8-byte aligned. */ > +#define VMXNET3_REG_ALIGN_MASK 0x7 > + > +/* I/O Mapped access to registers */ > +#define VMXNET3_IO_TYPE_PT 0 > +#define VMXNET3_IO_TYPE_VD 1 > +#define VMXNET3_IO_ADDR(type, reg) (((type) << 24) | ((reg) & 0xFFFFFF)) > +#define VMXNET3_IO_TYPE(addr) ((addr) >> 24) > +#define VMXNET3_IO_REG(addr) ((addr) & 0xFFFFFF) > + > +enum { > + VMXNET3_CMD_FIRST_SET = 0xCAFE0000, > + VMXNET3_CMD_ACTIVATE_DEV = VMXNET3_CMD_FIRST_SET, > + VMXNET3_CMD_QUIESCE_DEV, > + VMXNET3_CMD_RESET_DEV, > + VMXNET3_CMD_UPDATE_RX_MODE, > + VMXNET3_CMD_UPDATE_MAC_FILTERS, > + VMXNET3_CMD_UPDATE_VLAN_FILTERS, > + VMXNET3_CMD_UPDATE_RSSIDT, > + VMXNET3_CMD_UPDATE_IML, > + VMXNET3_CMD_UPDATE_PMCFG, > + VMXNET3_CMD_UPDATE_FEATURE, > + VMXNET3_CMD_LOAD_PLUGIN, > + > + VMXNET3_CMD_FIRST_GET = 0xF00D0000, > + VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET, > + VMXNET3_CMD_GET_STATS, > + VMXNET3_CMD_GET_LINK, > + VMXNET3_CMD_GET_PERM_MAC_LO, > + VMXNET3_CMD_GET_PERM_MAC_HI, > + VMXNET3_CMD_GET_DID_LO, > + VMXNET3_CMD_GET_DID_HI, > + VMXNET3_CMD_GET_DEV_EXTRA_INFO, > + VMXNET3_CMD_GET_CONF_INTR > +}; > + > +struct Vmxnet3_TxDesc { > + uint64_t addr; > + > + uint32_t len:14; > + uint32_t gen:1; /* generation bit */ > + uint32_t rsvd:1; > + uint32_t dtype:1; /* descriptor type */ > + uint32_t ext1:1; > + uint32_t msscof:14; /* MSS, checksum offset, flags */ > + > + uint32_t hlen:10; /* header len */ > + uint32_t om:2; /* offload mode */ > + uint32_t eop:1; /* End Of Packet */ > + uint32_t cq:1; /* completion request */ > + uint32_t ext2:1; > + uint32_t ti:1; /* VLAN Tag Insertion */ > + uint32_t tci:16; /* Tag to Insert */ > +}; > + > +/* TxDesc.OM values */ > +#define VMXNET3_OM_NONE 0 > +#define VMXNET3_OM_CSUM 2 > +#define VMXNET3_OM_TSO 3 > + > +/* fields in TxDesc we access w/o using bit fields */ > +#define VMXNET3_TXD_EOP_SHIFT 12 > +#define VMXNET3_TXD_CQ_SHIFT 13 > +#define VMXNET3_TXD_GEN_SHIFT 14 > + > +#define VMXNET3_TXD_CQ (1 << VMXNET3_TXD_CQ_SHIFT) > +#define VMXNET3_TXD_EOP (1 << VMXNET3_TXD_EOP_SHIFT) > +#define VMXNET3_TXD_GEN (1 << VMXNET3_TXD_GEN_SHIFT) > + > +#define VMXNET3_HDR_COPY_SIZE 128 > + > + > +struct Vmxnet3_TxDataDesc { > + uint8_t data[VMXNET3_HDR_COPY_SIZE]; > +}; > + > + > +struct Vmxnet3_TxCompDesc { > + uint32_t txdIdx:12; /* Index of the EOP TxDesc */ > + uint32_t ext1:20; > + > + uint32_t ext2; > + uint32_t ext3; > + > + uint32_t rsvd:24; > + uint32_t type:7; /* completion type */ > + uint32_t gen:1; /* generation bit */ > +}; > + > + > +struct Vmxnet3_RxDesc { > + uint64_t addr; > + > + uint32_t len:14; > + uint32_t btype:1; /* Buffer Type */ > + uint32_t dtype:1; /* Descriptor type */ > + uint32_t rsvd:15; > + uint32_t gen:1; /* Generation bit */ > + > + uint32_t ext1; > +}; > + > +/* values of RXD.BTYPE */ > +#define VMXNET3_RXD_BTYPE_HEAD 0 /* head only */ > +#define VMXNET3_RXD_BTYPE_BODY 1 /* body only */ > + > +/* fields in RxDesc we access w/o using bit fields */ > +#define VMXNET3_RXD_BTYPE_SHIFT 14 > +#define VMXNET3_RXD_GEN_SHIFT 31 > + > + > +struct Vmxnet3_RxCompDesc { > + uint32_t rxdIdx:12; /* Index of the RxDesc */ > + uint32_t ext1:2; > + uint32_t eop:1; /* End of Packet */ > + uint32_t sop:1; /* Start of Packet */ > + uint32_t rqID:10; /* rx queue/ring ID */ > + uint32_t rssType:4; /* RSS hash type used */ > + uint32_t cnc:1; /* Checksum Not Calculated */ > + uint32_t ext2:1; > + > + uint32_t rssHash; /* RSS hash value */ > + > + uint32_t len:14; /* data length */ > + uint32_t err:1; /* Error */ > + uint32_t ts:1; /* Tag is stripped */ > + uint32_t tci:16; /* Tag stripped */ > + > + uint32_t csum:16; > + uint32_t tuc:1; /* TCP/UDP Checksum Correct */ > + uint32_t udp:1; /* UDP packet */ > + uint32_t tcp:1; /* TCP packet */ > + uint32_t ipc:1; /* IP Checksum Correct */ > + uint32_t v6:1; /* IPv6 */ > + uint32_t v4:1; /* IPv4 */ > + uint32_t frg:1; /* IP Fragment */ > + uint32_t fcs:1; /* Frame CRC correct */ > + uint32_t type:7; /* completion type */ > + uint32_t gen:1; /* generation bit */ > +}; > + > +/* fields in RxCompDesc we access via Vmxnet3_GenericDesc.dword[3] */ > +#define VMXNET3_RCD_TUC_SHIFT 16 > +#define VMXNET3_RCD_IPC_SHIFT 19 > + > +/* fields in RxCompDesc we access via Vmxnet3_GenericDesc.qword[1] */ > +#define VMXNET3_RCD_TYPE_SHIFT 56 > +#define VMXNET3_RCD_GEN_SHIFT 63 > + > +/* csum OK for TCP/UDP pkts over IP */ > +#define VMXNET3_RCD_CSUM_OK (1 << VMXNET3_RCD_TUC_SHIFT | \ > + 1 << VMXNET3_RCD_IPC_SHIFT) > + > +/* value of RxCompDesc.rssType */ > +enum { > + VMXNET3_RCD_RSS_TYPE_NONE = 0, > + VMXNET3_RCD_RSS_TYPE_IPV4 = 1, > + VMXNET3_RCD_RSS_TYPE_TCPIPV4 = 2, > + VMXNET3_RCD_RSS_TYPE_IPV6 = 3, > + VMXNET3_RCD_RSS_TYPE_TCPIPV6 = 4, > +}; > + > +/* a union for accessing all cmd/completion descriptors */ > +union Vmxnet3_GenericDesc { > + uint64_t qword[2]; > + uint32_t dword[4]; > + uint16_t word[8]; > + struct Vmxnet3_TxDesc txd; > + struct Vmxnet3_RxDesc rxd; > + struct Vmxnet3_TxCompDesc tcd; > + struct Vmxnet3_RxCompDesc rcd; > +}; > + > +#define VMXNET3_INIT_GEN 1 > + > +/* Max size of a single tx buffer */ > +#define VMXNET3_MAX_TX_BUF_SIZE (1 << 14) > + > +/* # of tx desc needed for a tx buffer size */ > +#define VMXNET3_TXD_NEEDED(size) (((size) + VMXNET3_MAX_TX_BUF_SIZE - 1) / \ > + VMXNET3_MAX_TX_BUF_SIZE) > + > +/* max # of tx descs for a non-tso pkt */ > +#define VMXNET3_MAX_TXD_PER_PKT 16 > + > +/* Max size of a single rx buffer */ > +#define VMXNET3_MAX_RX_BUF_SIZE ((1 << 14) - 1) > +/* Minimum size of a type 0 buffer */ > +#define VMXNET3_MIN_T0_BUF_SIZE 128 > +#define VMXNET3_MAX_CSUM_OFFSET 1024 > + > +/* Ring base address alignment */ > +#define VMXNET3_RING_BA_ALIGN 512 > +#define VMXNET3_RING_BA_MASK (VMXNET3_RING_BA_ALIGN - 1) > + > +/* Ring size must be a multiple of 32 */ > +#define VMXNET3_RING_SIZE_ALIGN 32 > +#define VMXNET3_RING_SIZE_MASK (VMXNET3_RING_SIZE_ALIGN - 1) > + > +/* Max ring size */ > +#define VMXNET3_TX_RING_MAX_SIZE 4096 > +#define VMXNET3_TC_RING_MAX_SIZE 4096 > +#define VMXNET3_RX_RING_MAX_SIZE 4096 > +#define VMXNET3_RC_RING_MAX_SIZE 8192 > + > +/* a list of reasons for queue stop */ > + > +enum { > + VMXNET3_ERR_NOEOP = 0x80000000, /* cannot find the EOP desc of a pkt */ > + VMXNET3_ERR_TXD_REUSE = 0x80000001, /* reuse TxDesc before tx completion */ > + VMXNET3_ERR_BIG_PKT = 0x80000002, /* too many TxDesc for a pkt */ > + VMXNET3_ERR_DESC_NOT_SPT = 0x80000003, /* descriptor type not supported */ > + VMXNET3_ERR_SMALL_BUF = 0x80000004, /* type 0 buffer too small */ > + VMXNET3_ERR_STRESS = 0x80000005, /* stress option firing in vmkernel */ > + VMXNET3_ERR_SWITCH = 0x80000006, /* mode switch failure */ > + VMXNET3_ERR_TXD_INVALID = 0x80000007, /* invalid TxDesc */ > +}; > + > +/* completion descriptor types */ > +#define VMXNET3_CDTYPE_TXCOMP 0 /* Tx Completion Descriptor */ > +#define VMXNET3_CDTYPE_RXCOMP 3 /* Rx Completion Descriptor */ > + > +enum { > + VMXNET3_GOS_BITS_UNK = 0, /* unknown */ > + VMXNET3_GOS_BITS_32 = 1, > + VMXNET3_GOS_BITS_64 = 2, > +}; > + > +#define VMXNET3_GOS_TYPE_LINUX 1 > + > +/* All structures in DriverShared are padded to multiples of 8 bytes */ > + > + > +struct Vmxnet3_GOSInfo { > + uint32_t gosBits:2; /* 32-bit or 64-bit? */ > + uint32_t gosType:4; /* which guest */ > + uint32_t gosVer:16; /* gos version */ > + uint32_t gosMisc:10; /* other info about gos */ > +}; > + > + > +struct Vmxnet3_DriverInfo { > + uint32_t version; /* driver version */ > + struct Vmxnet3_GOSInfo gos; > + uint32_t vmxnet3RevSpt; /* vmxnet3 revision supported */ > + uint32_t uptVerSpt; /* upt version supported */ > +}; > + > +#define VMXNET3_REV1_MAGIC 0xbabefee1 > > + > +/* > + * QueueDescPA must be 128 bytes aligned. It points to an array of > + * Vmxnet3_TxQueueDesc followed by an array of Vmxnet3_RxQueueDesc. > + * The number of Vmxnet3_TxQueueDesc/Vmxnet3_RxQueueDesc are specified by > + * Vmxnet3_MiscConf.numTxQueues/numRxQueues, respectively. > + */ > +#define VMXNET3_QUEUE_DESC_ALIGN 128 Lot of inconsistent spacing between types and names in the structure def'ns > +struct Vmxnet3_MiscConf { > + struct Vmxnet3_DriverInfo driverInfo; > + uint64_t uptFeatures; > + uint64_t ddPA; /* driver data PA */ > + uint64_t queueDescPA; /* queue descriptor table PA */ > + uint32_t ddLen; /* driver data len */ > + uint32_t queueDescLen; /* queue desc. table len in bytes */ > + uint32_t mtu; > + uint16_t maxNumRxSG; > + uint8_t numTxQueues; > + uint8_t numRxQueues; > + uint32_t reserved[4]; > +}; should this be packed (or others that are shared w/ device)? i assume you've already done 32 vs 64 here > +struct Vmxnet3_TxQueueConf { > + uint64_t txRingBasePA; > + uint64_t dataRingBasePA; > + uint64_t compRingBasePA; > + uint64_t ddPA; /* driver data */ > + uint64_t reserved; > + uint32_t txRingSize; /* # of tx desc */ > + uint32_t dataRingSize; /* # of data desc */ > + uint32_t compRingSize; /* # of comp desc */ > + uint32_t ddLen; /* size of driver data */ > + uint8_t intrIdx; > + uint8_t _pad[7]; > +}; > + > + > +struct Vmxnet3_RxQueueConf { > + uint64_t rxRingBasePA[2]; > + uint64_t compRingBasePA; > + uint64_t ddPA; /* driver data */ > + uint64_t reserved; > + uint32_t rxRingSize[2]; /* # of rx desc */ > + uint32_t compRingSize; /* # of rx comp desc */ > + uint32_t ddLen; /* size of driver data */ > + uint8_t intrIdx; > + uint8_t _pad[7]; > +}; > + > +enum vmxnet3_intr_mask_mode { > + VMXNET3_IMM_AUTO = 0, > + VMXNET3_IMM_ACTIVE = 1, > + VMXNET3_IMM_LAZY = 2 > +}; > + > +enum vmxnet3_intr_type { > + VMXNET3_IT_AUTO = 0, > + VMXNET3_IT_INTX = 1, > + VMXNET3_IT_MSI = 2, > + VMXNET3_IT_MSIX = 3 > +}; > + > +#define VMXNET3_MAX_TX_QUEUES 8 > +#define VMXNET3_MAX_RX_QUEUES 16 different to UPT, I must've missed some layering here > +/* addition 1 for events */ > +#define VMXNET3_MAX_INTRS 25 > + > + <snip> > --- /dev/null > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c > @@ -0,0 +1,2608 @@ > +/* > + * Linux driver for VMware's vmxnet3 ethernet NIC. <snip> > +/* > + * vmxnet3_drv.c -- > + * > + * Linux driver for VMware's vmxnet3 NIC > + */ Not useful > +static void > +vmxnet3_enable_intr(struct vmxnet3_adapter *adapter, unsigned intr_idx) > +{ > + VMXNET3_WRITE_BAR0_REG(adapter, VMXNET3_REG_IMR + intr_idx * 8, 0); writel(0, adapter->hw_addr0 + VMXNET3_REG_IMR + intr_idx * 8) seems just as clear to me. > +vmxnet3_enable_all_intrs(struct vmxnet3_adapter *adapter) > +{ > + int i; > + > + for (i = 0; i < adapter->intr.num_intrs; i++) > + vmxnet3_enable_intr(adapter, i); > +} > + > +static void > +vmxnet3_disable_all_intrs(struct vmxnet3_adapter *adapter) > +{ > + int i; > + > + for (i = 0; i < adapter->intr.num_intrs; i++) > + vmxnet3_disable_intr(adapter, i); > +} only ever num_intrs=1, so there's some plan to bump this up and make these wrappers useful? > +static void > +vmxnet3_ack_events(struct vmxnet3_adapter *adapter, u32 events) > +{ > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_ECR, events); > +} > + > + > +static bool > +vmxnet3_tq_stopped(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter) > +{ > + return netif_queue_stopped(adapter->netdev); > +} > + > + > +static void > +vmxnet3_tq_start(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter *adapter) > +{ > + tq->stopped = false; is tq->stopped used besides just toggling back and forth? > + netif_start_queue(adapter->netdev); > +} > +static void > +vmxnet3_process_events(struct vmxnet3_adapter *adapter) Should be trivial to break out to it's own MSI-X vector, basically set up to do that already. > +{ > + u32 events = adapter->shared->ecr; > + if (!events) > + return; > + > + vmxnet3_ack_events(adapter, events); > + > + /* Check if link state has changed */ > + if (events & VMXNET3_ECR_LINK) > + vmxnet3_check_link(adapter); > + > + /* Check if there is an error on xmit/recv queues */ > + if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) { > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, > + VMXNET3_CMD_GET_QUEUE_STATUS); > + > + if (adapter->tqd_start->status.stopped) { > + printk(KERN_ERR "%s: tq error 0x%x\n", > + adapter->netdev->name, > + adapter->tqd_start->status.error); > + } > + if (adapter->rqd_start->status.stopped) { > + printk(KERN_ERR "%s: rq error 0x%x\n", > + adapter->netdev->name, > + adapter->rqd_start->status.error); > + } > + > + schedule_work(&adapter->work); > + } > +} <snip> > + > + tq->buf_info = kcalloc(sizeof(tq->buf_info[0]), tq->tx_ring.size, > + GFP_KERNEL); kcalloc args look backwards <snip> > +static int > +vmxnet3_alloc_pci_resources(struct vmxnet3_adapter *adapter, bool *dma64) > +{ > + int err; > + unsigned long mmio_start, mmio_len; > + struct pci_dev *pdev = adapter->pdev; > + > + err = pci_enable_device(pdev); looks ioport free, can be pci_enable_device_mem()... > + if (err) { > + printk(KERN_ERR "Failed to enable adapter %s: error %d\n", > + pci_name(pdev), err); > + return err; > + } > + > + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) == 0) { > + if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64)) != 0) { > + printk(KERN_ERR "pci_set_consistent_dma_mask failed " > + "for adapter %s\n", pci_name(pdev)); > + err = -EIO; > + goto err_set_mask; > + } > + *dma64 = true; > + } else { > + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(32)) != 0) { > + printk(KERN_ERR "pci_set_dma_mask failed for adapter " > + "%s\n", pci_name(pdev)); > + err = -EIO; > + goto err_set_mask; > + } > + *dma64 = false; > + } > + > + err = pci_request_regions(pdev, vmxnet3_driver_name); ....pci_request_selected_regions() > + if (err) { > + printk(KERN_ERR "Failed to request region for adapter %s: " > + "error %d\n", pci_name(pdev), err); > + goto err_set_mask; > + } > + > + pci_set_master(pdev); > + > + mmio_start = pci_resource_start(pdev, 0); > + mmio_len = pci_resource_len(pdev, 0); > + adapter->hw_addr0 = ioremap(mmio_start, mmio_len); > + if (!adapter->hw_addr0) { > + printk(KERN_ERR "Failed to map bar0 for adapter %s\n", > + pci_name(pdev)); > + err = -EIO; > + goto err_ioremap; > + } > + > + mmio_start = pci_resource_start(pdev, 1); > + mmio_len = pci_resource_len(pdev, 1); > + adapter->hw_addr1 = ioremap(mmio_start, mmio_len); > + if (!adapter->hw_addr1) { > + printk(KERN_ERR "Failed to map bar1 for adapter %s\n", > + pci_name(pdev)); > + err = -EIO; > + goto err_bar1; > + } > + return 0; > + > +err_bar1: > + iounmap(adapter->hw_addr0); > +err_ioremap: > + pci_release_regions(pdev); ....and pci_release_selected_regions() > +err_set_mask: > + pci_disable_device(pdev); > + return err; > +} > + <snip> > +vmxnet3_declare_features(struct vmxnet3_adapter *adapter, bool dma64) > +{ > + struct net_device *netdev = adapter->netdev; > + > + netdev->features = NETIF_F_SG | > + NETIF_F_HW_CSUM | > + NETIF_F_HW_VLAN_TX | > + NETIF_F_HW_VLAN_RX | > + NETIF_F_HW_VLAN_FILTER | > + NETIF_F_TSO | > + NETIF_F_TSO6; > + > + printk(KERN_INFO "features: sg csum vlan jf tso tsoIPv6"); > + > + adapter->rxcsum = true; > + adapter->jumbo_frame = true; > + > + if (!disable_lro) { > + adapter->lro = true; > + printk(" lro"); > + } Plan to switch to GRO? > + if (dma64) { > + netdev->features |= NETIF_F_HIGHDMA; > + printk(" highDMA"); > + } > + > + netdev->vlan_features = netdev->features; > + printk("\n"); > +} > + > +static int __devinit > +vmxnet3_probe_device(struct pci_dev *pdev, > + const struct pci_device_id *id) > +{ > + static const struct net_device_ops vmxnet3_netdev_ops = { > + .ndo_open = vmxnet3_open, > + .ndo_stop = vmxnet3_close, > + .ndo_start_xmit = vmxnet3_xmit_frame, > + .ndo_set_mac_address = vmxnet3_set_mac_addr, > + .ndo_change_mtu = vmxnet3_change_mtu, > + .ndo_get_stats = vmxnet3_get_stats, > + .ndo_tx_timeout = vmxnet3_tx_timeout, > + .ndo_set_multicast_list = vmxnet3_set_mc, > + .ndo_vlan_rx_register = vmxnet3_vlan_rx_register, > + .ndo_vlan_rx_add_vid = vmxnet3_vlan_rx_add_vid, > + .ndo_vlan_rx_kill_vid = vmxnet3_vlan_rx_kill_vid, > +# ifdef CONFIG_NET_POLL_CONTROLLER > + .ndo_poll_controller = vmxnet3_netpoll, > +# endif #ifdef #endif is more typical style here > + }; > + int err; > + bool dma64 = false; /* stupid gcc */ > + u32 ver; > + struct net_device *netdev; > + struct vmxnet3_adapter *adapter; > + u8 mac[ETH_ALEN]; extra space between type and name > + > + netdev = alloc_etherdev(sizeof(struct vmxnet3_adapter)); > + if (!netdev) { > + printk(KERN_ERR "Failed to alloc ethernet device for adapter " > + "%s\n", pci_name(pdev)); > + return -ENOMEM; > + } > + > + pci_set_drvdata(pdev, netdev); > + adapter = netdev_priv(netdev); > + adapter->netdev = netdev; > + adapter->pdev = pdev; > + > + adapter->shared = pci_alloc_consistent(adapter->pdev, > + sizeof(struct Vmxnet3_DriverShared), > + &adapter->shared_pa); > + if (!adapter->shared) { > + printk(KERN_ERR "Failed to allocate memory for %s\n", > + pci_name(pdev)); > + err = -ENOMEM; > + goto err_alloc_shared; > + } > + > + adapter->tqd_start = pci_alloc_consistent(adapter->pdev, extra space before = > diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c b/drivers/net/vmxnet3/vmxnet3_ethtool.c > new file mode 100644 > index 0000000..490577f > --- /dev/null > +++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c > +#include "vmxnet3_int.h" > + > +struct vmxnet3_stat_desc { > + char desc[ETH_GSTRING_LEN]; > + int offset; > +}; > + > + > +static u32 > +vmxnet3_get_rx_csum(struct net_device *netdev) > +{ > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > + return adapter->rxcsum; > +} > + > + > +static int > +vmxnet3_set_rx_csum(struct net_device *netdev, u32 val) > +{ > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > + > + if (adapter->rxcsum != val) { > + adapter->rxcsum = val; > + if (netif_running(netdev)) { > + if (val) > + adapter->shared->devRead.misc.uptFeatures |= > + UPT1_F_RXCSUM; > + else > + adapter->shared->devRead.misc.uptFeatures &= > + ~UPT1_F_RXCSUM; > + > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, > + VMXNET3_CMD_UPDATE_FEATURE); > + } > + } > + return 0; > +} > + > + > +static u32 > +vmxnet3_get_tx_csum(struct net_device *netdev) > +{ > + return (netdev->features & NETIF_F_HW_CSUM) != 0; > +} Not needed > +static int > +vmxnet3_set_tx_csum(struct net_device *netdev, u32 val) > +{ > + if (val) > + netdev->features |= NETIF_F_HW_CSUM; > + else > + netdev->features &= ~NETIF_F_HW_CSUM; > + > + return 0; > +} This is just ethtool_op_set_tx_hw_csum() > +static int > +vmxnet3_set_sg(struct net_device *netdev, u32 val) > +{ > + ethtool_op_set_sg(netdev, val); > + return 0; > +} Useless wrapper > +static int > +vmxnet3_set_tso(struct net_device *netdev, u32 val) > +{ > + ethtool_op_set_tso(netdev, val); > + return 0; > +} Useless wrapper > +struct net_device_stats* > +vmxnet3_get_stats(struct net_device *netdev) > +{ > + struct vmxnet3_adapter *adapter; > + struct vmxnet3_tq_driver_stats *drvTxStats; > + struct vmxnet3_rq_driver_stats *drvRxStats; > + struct UPT1_TxStats *devTxStats; > + struct UPT1_RxStats *devRxStats; > + > + adapter = netdev_priv(netdev); > + > + /* Collect the dev stats into the shared area */ > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, VMXNET3_CMD_GET_STATS); > + > + /* Assuming that we have a single queue device */ > + devTxStats = &adapter->tqd_start->stats; > + devRxStats = &adapter->rqd_start->stats; Another single queue assumption > + > + /* Get access to the driver stats per queue */ > + drvTxStats = &adapter->tx_queue.stats; > + drvRxStats = &adapter->rx_queue.stats; > + > + memset(&adapter->net_stats, 0, sizeof(adapter->net_stats)); > + > + adapter->net_stats.rx_packets = devRxStats->ucastPktsRxOK + > + devRxStats->mcastPktsRxOK + > + devRxStats->bcastPktsRxOK; > + > + adapter->net_stats.tx_packets = devTxStats->ucastPktsTxOK + > + devTxStats->mcastPktsTxOK + > + devTxStats->bcastPktsTxOK; > + > + adapter->net_stats.rx_bytes = devRxStats->ucastBytesRxOK + > + devRxStats->mcastBytesRxOK + > + devRxStats->bcastBytesRxOK; > + > + adapter->net_stats.tx_bytes = devTxStats->ucastBytesTxOK + > + devTxStats->mcastBytesTxOK + > + devTxStats->bcastBytesTxOK; > + > + adapter->net_stats.rx_errors = devRxStats->pktsRxError; > + adapter->net_stats.tx_errors = devTxStats->pktsTxError; > + adapter->net_stats.rx_dropped = drvRxStats->drop_total; > + adapter->net_stats.tx_dropped = drvTxStats->drop_total; > + adapter->net_stats.multicast = devRxStats->mcastPktsRxOK; > + > + return &adapter->net_stats; > +} > + > +static int > +vmxnet3_get_stats_count(struct net_device *netdev) > +{ > + return ARRAY_SIZE(vmxnet3_tq_dev_stats) + > + ARRAY_SIZE(vmxnet3_tq_driver_stats) + > + ARRAY_SIZE(vmxnet3_rq_dev_stats) + > + ARRAY_SIZE(vmxnet3_rq_driver_stats) + > + ARRAY_SIZE(vmxnet3_global_stats); > +} > + > + > +static int > +vmxnet3_get_regs_len(struct net_device *netdev) > +{ > + return 20 * sizeof(u32); > +} > + > + > +static void > +vmxnet3_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo) > +{ > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > + > + strncpy(drvinfo->driver, vmxnet3_driver_name, sizeof(drvinfo->driver)); > + drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0'; > + > + strncpy(drvinfo->version, VMXNET3_DRIVER_VERSION_REPORT, > + sizeof(drvinfo->version)); > + drvinfo->driver[sizeof(drvinfo->version) - 1] = '\0'; > + > + strncpy(drvinfo->fw_version, "N/A", sizeof(drvinfo->fw_version)); > + drvinfo->fw_version[sizeof(drvinfo->fw_version) - 1] = '\0'; > + > + strncpy(drvinfo->bus_info, pci_name(adapter->pdev), > + ETHTOOL_BUSINFO_LEN); simplify all these to strlcpy > + drvinfo->n_stats = vmxnet3_get_stats_count(netdev); > + drvinfo->testinfo_len = 0; > + drvinfo->eedump_len = 0; > + drvinfo->regdump_len = vmxnet3_get_regs_len(netdev); > +} > +static int > +vmxnet3_set_ringparam(struct net_device *netdev, > + struct ethtool_ringparam *param) > +{ > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > + u32 new_tx_ring_size, new_rx_ring_size; > + u32 sz; > + int err = 0; > + > + if (param->tx_pending == 0 || param->tx_pending > > + VMXNET3_TX_RING_MAX_SIZE) { > + printk(KERN_ERR "%s: invalid tx ring size %u\n", netdev->name, > + param->tx_pending); Seems noisy > + return -EINVAL; > + } > + if (param->rx_pending == 0 || param->rx_pending > > + VMXNET3_RX_RING_MAX_SIZE) { > + printk(KERN_ERR "%s: invalid rx ring size %u\n", netdev->name, > + param->rx_pending); Same here > + return -EINVAL; > + } > + > + /* round it up to a multiple of VMXNET3_RING_SIZE_ALIGN */ > + new_tx_ring_size = (param->tx_pending + VMXNET3_RING_SIZE_MASK) & > + ~VMXNET3_RING_SIZE_MASK; > + new_tx_ring_size = min_t(u32, new_tx_ring_size, > + VMXNET3_TX_RING_MAX_SIZE); > + BUG_ON(new_tx_ring_size > VMXNET3_TX_RING_MAX_SIZE); > + BUG_ON(new_tx_ring_size % VMXNET3_RING_SIZE_ALIGN != 0); Don't use BUG_ON for validating user input > + > + /* ring0 has to be a multiple of > + * rx_buf_per_pkt * VMXNET3_RING_SIZE_ALIGN > + */ > + sz = adapter->rx_buf_per_pkt * VMXNET3_RING_SIZE_ALIGN; > + new_rx_ring_size = (param->rx_pending + sz - 1) / sz * sz; > + new_rx_ring_size = min_t(u32, new_rx_ring_size, > + VMXNET3_RX_RING_MAX_SIZE / sz * sz); > + BUG_ON(new_rx_ring_size > VMXNET3_RX_RING_MAX_SIZE); > + BUG_ON(new_rx_ring_size % sz != 0); > + > + if (new_tx_ring_size == adapter->tx_queue.tx_ring.size && > + new_rx_ring_size == adapter->rx_queue.rx_ring[0].size) { > + return 0; > + } > + > + /* > + * Reset_work may be in the middle of resetting the device, wait for its > + * completion. > + */ > + while (test_and_set_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state)) > + msleep(1); > + > + if (netif_running(netdev)) { > + vmxnet3_quiesce_dev(adapter); > + vmxnet3_reset_dev(adapter); > + > + /* recreate the rx queue and the tx queue based on the > + * new sizes */ > + vmxnet3_tq_destroy(&adapter->tx_queue, adapter); > + vmxnet3_rq_destroy(&adapter->rx_queue, adapter); > + > + err = vmxnet3_create_queues(adapter, new_tx_ring_size, > + new_rx_ring_size, VMXNET3_DEF_RX_RING_SIZE); > + if (err) { > + /* failed, most likely because of OOM, try default > + * size */ > + printk(KERN_ERR "%s: failed to apply new sizes, try the" > + " default ones\n", netdev->name); > + err = vmxnet3_create_queues(adapter, > + VMXNET3_DEF_TX_RING_SIZE, > + VMXNET3_DEF_RX_RING_SIZE, > + VMXNET3_DEF_RX_RING_SIZE); > + if (err) { > + printk(KERN_ERR "%s: failed to create queues " > + "with default sizes. Closing it\n", > + netdev->name); > + goto out; > + } > + } > + > + err = vmxnet3_activate_dev(adapter); > + if (err) { > + printk(KERN_ERR "%s: failed to re-activate, error %d." > + " Closing it\n", netdev->name, err); > + goto out; Going to out: anyway... > + } > + } > + > +out: > + clear_bit(VMXNET3_STATE_BIT_RESETTING, &adapter->state); > + if (err) > + vmxnet3_force_close(adapter); > + > + return err; > +} > + > + > +static struct ethtool_ops vmxnet3_ethtool_ops = { > + .get_settings = vmxnet3_get_settings, > + .get_drvinfo = vmxnet3_get_drvinfo, > + .get_regs_len = vmxnet3_get_regs_len, > + .get_regs = vmxnet3_get_regs, > + .get_wol = vmxnet3_get_wol, > + .set_wol = vmxnet3_set_wol, > + .get_link = ethtool_op_get_link, > + .get_rx_csum = vmxnet3_get_rx_csum, > + .set_rx_csum = vmxnet3_set_rx_csum, > + .get_tx_csum = vmxnet3_get_tx_csum, > + .set_tx_csum = vmxnet3_set_tx_csum, > + .get_sg = ethtool_op_get_sg, > + .set_sg = vmxnet3_set_sg, > + .get_tso = ethtool_op_get_tso, > + .set_tso = vmxnet3_set_tso, > + .get_strings = vmxnet3_get_strings, > + .get_stats_count = vmxnet3_get_stats_count, use get_sset_count instead > + .get_ethtool_stats = vmxnet3_get_ethtool_stats, > + .get_ringparam = vmxnet3_get_ringparam, > + .set_ringparam = vmxnet3_set_ringparam, > +}; > + > +void vmxnet3_set_ethtool_ops(struct net_device *netdev) > +{ > + SET_ETHTOOL_OPS(netdev, &vmxnet3_ethtool_ops); > +} <snip> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Shreyas Bhatewara on 29 Sep 2009 17:00
Chris, Thanks for the review. Bhavesh responded to some queries. I will attempt the answer the rest. I am working on rebasing the code to v2.6.32-rc1. Will send out a patch with the changes you suggested after that. ->Shreyas > -----Original Message----- > From: Chris Wright [mailto:chrisw(a)sous-sol.org] > Sent: Tuesday, September 29, 2009 1:54 AM > To: Shreyas Bhatewara > Cc: linux-kernel(a)vger.kernel.org; netdev(a)vger.kernel.org; Stephen > Hemminger; David S. Miller; Jeff Garzik; Anthony Liguori; Chris Wright; > Greg Kroah-Hartman; Andrew Morton; virtualization; pv- > drivers(a)vmware.com > Subject: Re: [PATCH 2.6.31-rc9] net: VMware virtual Ethernet NIC > driver: vmxnet3 > > > > Wake-on-LAN, PCI Power Management D0-D3 states > > PXE-ROM for boot support > > > > Whole thing appears to be space indented, and is fairly noisy w/ > printk. The code written is tab indented. Is there a specific line / function which you find space indented ? If not, may be my email client did not preserve the tabs while sending. I will take care while posting next patch. Some of the printks are debug only. Others have been marked as INFO or ERR appropriately. I will remove a few. > Also, heavy use of BUG_ON() (counted 51 of them), are you sure that > none > of them can be triggered by guest or remote (esp. the ones that happen > in interrupt context)? Some initial thoughts below. > Like you said below, I will remove the user input dependent ones. > > + * the file called "COPYING". > > + * > > + * Maintained by: Shreyas Bhatewara <pv-drivers(a)vmware.com> > > + * > > + */ > > + > > +/* upt1_defs.h > > + * > > + * Definitions for Uniform Pass Through. > > + */ > > Most of the source files have this format (some include -- after file > name). Could just keep it all w/in the same comment block. Since you > went to the trouble of saying what the file does, something a tad more > descriptive would be welcome. > Yes, I will merge the two blocks and elaborate on the description. > > + > > +#ifndef _UPT1_DEFS_H > > +#define _UPT1_DEFS_H > > + > > +#define UPT1_MAX_TX_QUEUES 64 > > +#define UPT1_MAX_RX_QUEUES 64 > > This is different than the 16/8 described above (and seemingly all moot > since it becomes a single queue device). > > > + > > +/* interrupt moderation level */ > > +#define UPT1_IML_NONE 0 /* no interrupt moderation */ > > +#define UPT1_IML_HIGHEST 7 /* least intr generated */ > > +#define UPT1_IML_ADAPTIVE 8 /* adpative intr moderation */ > > enum? also only appears to support adaptive mode? > > + > > +/* > > + * QueueDescPA must be 128 bytes aligned. It points to an array of > > + * Vmxnet3_TxQueueDesc followed by an array of Vmxnet3_RxQueueDesc. > > + * The number of Vmxnet3_TxQueueDesc/Vmxnet3_RxQueueDesc are > specified by > > + * Vmxnet3_MiscConf.numTxQueues/numRxQueues, respectively. > > + */ > > +#define VMXNET3_QUEUE_DESC_ALIGN 128 > > Lot of inconsistent spacing between types and names in the structure > def'ns Okay, I will try to make it uniform. > > > +struct Vmxnet3_MiscConf { > > + struct Vmxnet3_DriverInfo driverInfo; > > + uint64_t uptFeatures; > > + uint64_t ddPA; /* driver data PA */ > > + uint64_t queueDescPA; /* queue descriptor table > PA */ > > + uint32_t ddLen; /* driver data len */ > > + uint32_t queueDescLen; /* queue desc. table len > in bytes */ > > + uint32_t mtu; > > + uint16_t maxNumRxSG; > > + uint8_t numTxQueues; > > + uint8_t numRxQueues; > > + uint32_t reserved[4]; > > +}; > > should this be packed (or others that are shared w/ device)? i assume > you've already done 32 vs 64 here > > > +struct Vmxnet3_TxQueueConf { > > + uint64_t txRingBasePA; > > + uint64_t dataRingBasePA; > > + uint64_t compRingBasePA; > > + uint64_t ddPA; /* driver data */ > > + uint64_t reserved; > > + uint32_t txRingSize; /* # of tx desc */ > > + uint32_t dataRingSize; /* # of data desc */ > > + uint32_t compRingSize; /* # of comp desc */ > > + uint32_t ddLen; /* size of driver data */ > > + uint8_t intrIdx; > > + uint8_t _pad[7]; > > +}; > > + > > + > > +struct Vmxnet3_RxQueueConf { > > + uint64_t rxRingBasePA[2]; > > + uint64_t compRingBasePA; > > + uint64_t ddPA; /* driver data */ > > + uint64_t reserved; > > + uint32_t rxRingSize[2]; /* # of rx desc */ > > + uint32_t compRingSize; /* # of rx comp desc */ > > + uint32_t ddLen; /* size of driver data */ > > + uint8_t intrIdx; > > + uint8_t _pad[7]; > > +}; > > + > > +enum vmxnet3_intr_mask_mode { > > + VMXNET3_IMM_AUTO = 0, > > + VMXNET3_IMM_ACTIVE = 1, > > + VMXNET3_IMM_LAZY = 2 > > +}; > > + > > +enum vmxnet3_intr_type { > > + VMXNET3_IT_AUTO = 0, > > + VMXNET3_IT_INTX = 1, > > + VMXNET3_IT_MSI = 2, > > + VMXNET3_IT_MSIX = 3 > > +}; > > + > > +#define VMXNET3_MAX_TX_QUEUES 8 > > +#define VMXNET3_MAX_RX_QUEUES 16 > > different to UPT, I must've missed some layering here There are the right ones, I will remove the other definitions. > > > +/* addition 1 for events */ > > +#define VMXNET3_MAX_INTRS 25 > > + > > + > <snip> > > > --- /dev/null > > +++ b/drivers/net/vmxnet3/vmxnet3_drv.c > > @@ -0,0 +1,2608 @@ > > +/* > > + * Linux driver for VMware's vmxnet3 ethernet NIC. > <snip> > > +/* > > + * vmxnet3_drv.c -- > > + * > > + * Linux driver for VMware's vmxnet3 NIC > > + */ > > Not useful > > > +static void > > +vmxnet3_enable_intr(struct vmxnet3_adapter *adapter, unsigned > intr_idx) > > +{ > > + VMXNET3_WRITE_BAR0_REG(adapter, VMXNET3_REG_IMR + intr_idx * > 8, 0); > > writel(0, adapter->hw_addr0 + VMXNET3_REG_IMR + intr_idx * 8) > > seems just as clear to me. The intention is to differentiate bar0 and bar1 writes. hw_addr0/1 doesn't seem to convey that instantly. > > > +vmxnet3_enable_all_intrs(struct vmxnet3_adapter *adapter) > > +{ > > + int i; > > + > > + for (i = 0; i < adapter->intr.num_intrs; i++) > > + vmxnet3_enable_intr(adapter, i); > > +} > > + > > +static void > > +vmxnet3_disable_all_intrs(struct vmxnet3_adapter *adapter) > > +{ > > + int i; > > + > > + for (i = 0; i < adapter->intr.num_intrs; i++) > > + vmxnet3_disable_intr(adapter, i); > > +} > > only ever num_intrs=1, so there's some plan to bump this up and make > these wrappers useful? > > > +static void > > +vmxnet3_ack_events(struct vmxnet3_adapter *adapter, u32 events) > > +{ > > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_ECR, events); > > +} > > + > > + > > +static bool > > +vmxnet3_tq_stopped(struct vmxnet3_tx_queue *tq, struct > vmxnet3_adapter *adapter) > > +{ > > + return netif_queue_stopped(adapter->netdev); > > +} > > + > > + > > +static void > > +vmxnet3_tq_start(struct vmxnet3_tx_queue *tq, struct vmxnet3_adapter > *adapter) > > +{ > > + tq->stopped = false; > > is tq->stopped used besides just toggling back and forth? It is used in ethtool ops. > > > + netif_start_queue(adapter->netdev); > > +} > > > +static void > > +vmxnet3_process_events(struct vmxnet3_adapter *adapter) > > Should be trivial to break out to it's own MSI-X vector, basically set > up to do that already. > > > +{ > > + u32 events = adapter->shared->ecr; > > + if (!events) > > + return; > > + > > + vmxnet3_ack_events(adapter, events); > > + > > + /* Check if link state has changed */ > > + if (events & VMXNET3_ECR_LINK) > > + vmxnet3_check_link(adapter); > > + > > + /* Check if there is an error on xmit/recv queues */ > > + if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) { > > + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD, > > + VMXNET3_CMD_GET_QUEUE_STATUS); > > + > > + if (adapter->tqd_start->status.stopped) { > > + printk(KERN_ERR "%s: tq error 0x%x\n", > > + adapter->netdev->name, > > + adapter->tqd_start->status.error); > > + } > > + if (adapter->rqd_start->status.stopped) { > > + printk(KERN_ERR "%s: rq error 0x%x\n", > > + adapter->netdev->name, > > + adapter->rqd_start->status.error); > > + } > > + > > + schedule_work(&adapter->work); > > + } > > +} > <snip> > > > + > > + tq->buf_info = kcalloc(sizeof(tq->buf_info[0]), tq- > >tx_ring.size, > > + GFP_KERNEL); > > kcalloc args look backwards > > <snip> > > +static int > > +vmxnet3_alloc_pci_resources(struct vmxnet3_adapter *adapter, bool > *dma64) > > +{ > > + int err; > > + unsigned long mmio_start, mmio_len; > > + struct pci_dev *pdev = adapter->pdev; > > + > > + err = pci_enable_device(pdev); > > looks ioport free, can be pci_enable_device_mem()... Yes, will do that. > > > + if (err) { > > + printk(KERN_ERR "Failed to enable adapter %s: error > %d\n", > > + pci_name(pdev), err); > > + return err; > > + } > > + > > + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(64)) == 0) { > > + if (pci_set_consistent_dma_mask(pdev, > DMA_BIT_MASK(64)) != 0) { > > + printk(KERN_ERR "pci_set_consistent_dma_mask > failed " > > + "for adapter %s\n", pci_name(pdev)); > > + err = -EIO; > > + goto err_set_mask; > > + } > > + *dma64 = true; > > + } else { > > + if (pci_set_dma_mask(pdev, DMA_BIT_MASK(32)) != 0) { > > + printk(KERN_ERR "pci_set_dma_mask failed for > adapter " > > + "%s\n", pci_name(pdev)); > > + err = -EIO; > > + goto err_set_mask; > > + } > > + *dma64 = false; > > + } > > + > > + err = pci_request_regions(pdev, vmxnet3_driver_name); > > ...pci_request_selected_regions() Okay. > > > + if (err) { > > + printk(KERN_ERR "Failed to request region for adapter > %s: " > > + "error %d\n", pci_name(pdev), err); > > + goto err_set_mask; > > + } > > + > > + pci_set_master(pdev); > > + > > + mmio_start = pci_resource_start(pdev, 0); > > + mmio_len = pci_resource_len(pdev, 0); > > + adapter->hw_addr0 = ioremap(mmio_start, mmio_len); > > + if (!adapter->hw_addr0) { > > + printk(KERN_ERR "Failed to map bar0 for adapter > %s\n", > > + pci_name(pdev)); > > + err = -EIO; > > + goto err_ioremap; > > + } > > + > > + mmio_start = pci_resource_start(pdev, 1); > > + mmio_len = pci_resource_len(pdev, 1); > > + adapter->hw_addr1 = ioremap(mmio_start, mmio_len); > > + if (!adapter->hw_addr1) { > > + printk(KERN_ERR "Failed to map bar1 for adapter > %s\n", > > + pci_name(pdev)); > > + err = -EIO; > > + goto err_bar1; > > + } > > + return 0; > > + > > +err_bar1: > > + iounmap(adapter->hw_addr0); > > +err_ioremap: > > + pci_release_regions(pdev); > > ...and pci_release_selected_regions() > > > +err_set_mask: > > + pci_disable_device(pdev); > > + return err; > > +} > > + > > <snip> > > +vmxnet3_declare_features(struct vmxnet3_adapter *adapter, bool > dma64) > > +{ > > + struct net_device *netdev = adapter->netdev; > > + > > + netdev->features = NETIF_F_SG | > > + NETIF_F_HW_CSUM | > > + NETIF_F_HW_VLAN_TX | > > + NETIF_F_HW_VLAN_RX | > > + NETIF_F_HW_VLAN_FILTER | > > + NETIF_F_TSO | > > + NETIF_F_TSO6; > > + > > + printk(KERN_INFO "features: sg csum vlan jf tso tsoIPv6"); > > + > > + adapter->rxcsum = true; > > + adapter->jumbo_frame = true; > > + > > + if (!disable_lro) { > > + adapter->lro = true; > > + printk(" lro"); > > + } > > Plan to switch to GRO? > > > + if (dma64) { > > + netdev->features |= NETIF_F_HIGHDMA; > > + printk(" highDMA"); > > + } > > + > > + netdev->vlan_features = netdev->features; > > + printk("\n"); > > +} > > + > > +static int __devinit > > +vmxnet3_probe_device(struct pci_dev *pdev, > > + const struct pci_device_id *id) > > +{ > > + static const struct net_device_ops vmxnet3_netdev_ops = { > > + .ndo_open = vmxnet3_open, > > + .ndo_stop = vmxnet3_close, > > + .ndo_start_xmit = vmxnet3_xmit_frame, > > + .ndo_set_mac_address = vmxnet3_set_mac_addr, > > + .ndo_change_mtu = vmxnet3_change_mtu, > > + .ndo_get_stats = vmxnet3_get_stats, > > + .ndo_tx_timeout = vmxnet3_tx_timeout, > > + .ndo_set_multicast_list = vmxnet3_set_mc, > > + .ndo_vlan_rx_register = vmxnet3_vlan_rx_register, > > + .ndo_vlan_rx_add_vid = vmxnet3_vlan_rx_add_vid, > > + .ndo_vlan_rx_kill_vid = vmxnet3_vlan_rx_kill_vid, > > +# ifdef CONFIG_NET_POLL_CONTROLLER > > + .ndo_poll_controller = vmxnet3_netpoll, > > +# endif > > #ifdef > #endif > > is more typical style here > > > + }; > > + int err; > > + bool dma64 = false; /* stupid gcc */ > > + u32 ver; > > + struct net_device *netdev; > > + struct vmxnet3_adapter *adapter; > > + u8 mac[ETH_ALEN]; > > extra space between type and name > > > + > > + netdev = alloc_etherdev(sizeof(struct vmxnet3_adapter)); > > + if (!netdev) { > > + printk(KERN_ERR "Failed to alloc ethernet device for > adapter " > > + "%s\n", pci_name(pdev)); > > + return -ENOMEM; > > + } > > + > > + pci_set_drvdata(pdev, netdev); > > + adapter = netdev_priv(netdev); > > + adapter->netdev = netdev; > > + adapter->pdev = pdev; > > + > > + adapter->shared = pci_alloc_consistent(adapter->pdev, > > + sizeof(struct Vmxnet3_DriverShared), > > + &adapter->shared_pa); > > + if (!adapter->shared) { > > + printk(KERN_ERR "Failed to allocate memory for %s\n", > > + pci_name(pdev)); > > + err = -ENOMEM; > > + goto err_alloc_shared; > > + } > > + > > + adapter->tqd_start = pci_alloc_consistent(adapter->pdev, > > extra space before = > > > diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c > b/drivers/net/vmxnet3/vmxnet3_ethtool.c > > new file mode 100644 > > index 0000000..490577f > > --- /dev/null > > +++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c > > +#include "vmxnet3_int.h" > > + > > +struct vmxnet3_stat_desc { > > + char desc[ETH_GSTRING_LEN]; > > + int offset; > > +}; > > + > > + > > +static u32 > > +vmxnet3_get_rx_csum(struct net_device *netdev) > > +{ > > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > > + return adapter->rxcsum; > > +} > > + > > + > > +static int > > +vmxnet3_set_rx_csum(struct net_device *netdev, u32 val) > > +{ > > + struct vmxnet3_adapter *adapter = netdev_priv(netdev); > > + > > + if (adapter->rxcsum != val) { > > + adapter->rxcsum = val; > > + if (netif_running(netdev)) { > > + if (val) > > + adapter->shared- > >devRead.misc.uptFeatures |= > > + > UPT1_F_RXCSUM; > > + else > > + adapter->shared- > >devRead.misc.uptFeatures &= > > + > ~UPT1_F_RXCSUM; > > + > > + VMXNET3_WRITE_BAR1_REG(adapter, > VMXNET3_REG_CMD, > > + > VMXNET3_CMD_UPDATE_FEATURE); > > + } > > + } > > + return 0; > > +} > > + > > + > > +static u32 > > +vmxnet3_get_tx_csum(struct net_device *netdev) > > +{ > > + return (netdev->features & NETIF_F_HW_CSUM) != 0; > > +} > > Not needed > > > +static int > > +vmxnet3_set_tx_csum(struct net_device *netdev, u32 val) > > +{ > > + if (val) > > + netdev->features |= NETIF_F_HW_CSUM; > > + else > > + netdev->features &= ~NETIF_F_HW_CSUM; > > + > > + return 0; > > +} > > This is just ethtool_op_set_tx_hw_csum() > > > +static int > > +vmxnet3_set_sg(struct net_device *netdev, u32 val) > > +{ > > + ethtool_op_set_sg(netdev, val); > > + return 0; > > +} > > Useless wrapper > > > +static int > > +vmxnet3_set_tso(struct net_device *netdev, u32 val) > > +{ > > + ethtool_op_set_tso(netdev, val); > > + return 0; > > +} > > Useless wrapper > I will remove the unwanted wrappers functions, spaces and get back with a patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |