Netdev List
 help / color / mirror / Atom feed
* [PATCHv2 net-next-2.6 1/2] qlcnic: FW dump support
From: Anirban Chakraborty @ 2011-05-10  1:02 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, davem, Anirban Chakraborty
In-Reply-To: <1304989352-24810-1-git-send-email-anirban.chakraborty@qlogic.com>

Added code to take FW dump.
o Driver queries FW at the init time and gets the dump template
o It takes FW dump as per the dump template
o Level of FW dump (and its size) is configured via dump flag

Singed-off-by: Sritej Velaga <sritej.velaga@qlogic.com>
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h      |  176 +++++++++++++++-
 drivers/net/qlcnic/qlcnic_ctx.c  |   91 ++++++++
 drivers/net/qlcnic/qlcnic_hdr.h  |   40 +++-
 drivers/net/qlcnic/qlcnic_hw.c   |  460 ++++++++++++++++++++++++++++++++++++++
 drivers/net/qlcnic/qlcnic_init.c |    4 +-
 drivers/net/qlcnic/qlcnic_main.c |   13 +-
 6 files changed, 775 insertions(+), 9 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index f729363..689adea 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -411,6 +411,29 @@ struct qlcnic_nic_intr_coalesce {
 	u32	timer_out;
 };
 
+struct qlcnic_dump_template_hdr {
+	__le32	type;
+	__le32	offset;
+	__le32	size;
+	__le32	cap_mask;
+	__le32	num_entries;
+	__le32	version;
+	__le32	timestamp;
+	__le32	checksum;
+	__le32	drv_cap_mask;
+	__le32	sys_info[3];
+	__le32	saved_state[16];
+	__le32	cap_sizes[8];
+	__le32	rsvd[0];
+};
+
+struct qlcnic_fw_dump {
+	u8	clr;	/* flag to indicate if dump is cleared */
+	u32	size;	/* total size of the dump */
+	void	*data;	/* dump data area */
+	struct	qlcnic_dump_template_hdr *tmpl_hdr;
+};
+
 /*
  * One hardware_context{} per adapter
  * contains interrupt info as well shared hardware info.
@@ -431,6 +454,7 @@ struct qlcnic_hardware_context {
 	u16 board_type;
 
 	struct qlcnic_nic_intr_coalesce coal;
+	struct qlcnic_fw_dump fw_dump;
 };
 
 struct qlcnic_adapter_stats {
@@ -574,6 +598,8 @@ struct qlcnic_recv_context {
 #define QLCNIC_CDRP_CMD_GET_ESWITCH_PORT_CONFIG	0x00000029
 #define QLCNIC_CDRP_CMD_GET_ESWITCH_STATS	0x0000002a
 #define QLCNIC_CDRP_CMD_CONFIG_PORT		0x0000002E
+#define QLCNIC_CDRP_CMD_TEMP_SIZE		0x0000002f
+#define QLCNIC_CDRP_CMD_GET_TEMP_HDR		0x00000030
 
 #define QLCNIC_RCODE_SUCCESS		0
 #define QLCNIC_RCODE_NOT_SUPPORTED	9
@@ -1157,6 +1183,152 @@ struct qlcnic_esw_statistics {
 	struct __qlcnic_esw_statistics tx;
 };
 
+struct qlcnic_common_entry_hdr {
+	__le32	type;
+	__le32	offset;
+	__le32	cap_size;
+	u8	mask;
+	u8	rsvd[2];
+	u8	flags;
+} __packed;
+
+struct __crb {
+	__le32	addr;
+	u8	stride;
+	u8	rsvd1[3];
+	__le32	data_size;
+	__le32	no_ops;
+	__le32	rsvd2[4];
+} __packed;
+
+struct __ctrl {
+	__le32	addr;
+	u8	stride;
+	u8	index_a;
+	__le16	timeout;
+	__le32	data_size;
+	__le32	no_ops;
+	u8	opcode;
+	u8	index_v;
+	u8	shl_val;
+	u8	shr_val;
+	__le32	val1;
+	__le32	val2;
+	__le32	val3;
+} __packed;
+
+struct __cache {
+	__le32	addr;
+	u8	stride;
+	u8	rsvd;
+	__le16	init_tag_val;
+	__le32	size;
+	__le32	no_ops;
+	__le32	ctrl_addr;
+	__le32	ctrl_val;
+	__le32	read_addr;
+	u8	read_addr_stride;
+	u8	read_addr_num;
+	u8	rsvd1[2];
+} __packed;
+
+struct __ocm {
+	u8	rsvd[8];
+	__le32	size;
+	__le32	no_ops;
+	u8	rsvd1[8];
+	__le32	read_addr;
+	__le32	read_addr_stride;
+} __packed;
+
+struct __mem {
+	u8	rsvd[24];
+	__le32	addr;
+	__le32	size;
+} __packed;
+
+struct __mux {
+	__le32	addr;
+	u8	rsvd[4];
+	__le32	size;
+	__le32	no_ops;
+	__le32	val;
+	__le32	val_stride;
+	__le32	read_addr;
+	u8	rsvd2[4];
+} __packed;
+
+struct __queue {
+	__le32	sel_addr;
+	__le16	stride;
+	u8	rsvd[2];
+	__le32	size;
+	__le32	no_ops;
+	u8	rsvd2[8];
+	__le32	read_addr;
+	u8	read_addr_stride;
+	u8	read_addr_cnt;
+	u8	rsvd3[2];
+} __packed;
+
+struct qlcnic_dump_entry {
+	struct qlcnic_common_entry_hdr hdr;
+	union {
+		struct __crb	crb;
+		struct __cache	cache;
+		struct __ocm	ocm;
+		struct __mem	mem;
+		struct __mux	mux;
+		struct __queue	que;
+		struct __ctrl	ctrl;
+	} region;
+} __packed;
+
+enum op_codes {
+	QLCNIC_DUMP_NOP		= 0,
+	QLCNIC_DUMP_READ_CRB	= 1,
+	QLCNIC_DUMP_READ_MUX	= 2,
+	QLCNIC_DUMP_QUEUE	= 3,
+	QLCNIC_DUMP_BRD_CONFIG	= 4,
+	QLCNIC_DUMP_READ_OCM	= 6,
+	QLCNIC_DUMP_PEG_REG	= 7,
+	QLCNIC_DUMP_L1_DTAG	= 8,
+	QLCNIC_DUMP_L1_ITAG	= 9,
+	QLCNIC_DUMP_L1_DATA	= 11,
+	QLCNIC_DUMP_L1_INST	= 12,
+	QLCNIC_DUMP_L2_DTAG	= 21,
+	QLCNIC_DUMP_L2_ITAG	= 22,
+	QLCNIC_DUMP_L2_DATA	= 23,
+	QLCNIC_DUMP_L2_INST	= 24,
+	QLCNIC_DUMP_READ_ROM	= 71,
+	QLCNIC_DUMP_READ_MEM	= 72,
+	QLCNIC_DUMP_READ_CTRL	= 98,
+	QLCNIC_DUMP_TLHDR	= 99,
+	QLCNIC_DUMP_RDEND	= 255
+};
+
+#define QLCNIC_DUMP_WCRB	BIT_0
+#define QLCNIC_DUMP_RWCRB	BIT_1
+#define QLCNIC_DUMP_ANDCRB	BIT_2
+#define QLCNIC_DUMP_ORCRB	BIT_3
+#define QLCNIC_DUMP_POLLCRB	BIT_4
+#define QLCNIC_DUMP_RD_SAVE	BIT_5
+#define QLCNIC_DUMP_WRT_SAVED	BIT_6
+#define QLCNIC_DUMP_MOD_SAVE_ST	BIT_7
+#define QLCNIC_DUMP_SKIP	BIT_7
+
+#define QLCNIC_DUMP_MASK_MIN		3
+#define QLCNIC_DUMP_MASK_DEF		0x0f
+#define QLCNIC_DUMP_MASK_MAX		0xff
+#define QLCNIC_FORCE_FW_DUMP_KEY	0xdeadfeed
+
+struct qlcnic_dump_operations {
+	enum op_codes opcode;
+	u32 (*handler)(struct qlcnic_adapter *,
+			struct qlcnic_dump_entry *, u32 *);
+};
+
+int qlcnic_fw_cmd_get_minidump_temp(struct qlcnic_adapter *adapter);
 int qlcnic_fw_cmd_set_port(struct qlcnic_adapter *adapter, u32 config);
 
 u32 qlcnic_hw_read_wx_2M(struct qlcnic_adapter *adapter, ulong off);
@@ -1203,6 +1375,7 @@ int qlcnic_wol_supported(struct qlcnic_adapter *adapter);
 int qlcnic_config_led(struct qlcnic_adapter *adapter, u32 state, u32 rate);
 void qlcnic_prune_lb_filters(struct qlcnic_adapter *adapter);
 void qlcnic_delete_lb_filters(struct qlcnic_adapter *adapter);
+int qlcnic_dump_fw(struct qlcnic_adapter *);
 
 /* Functions from qlcnic_init.c */
 int qlcnic_load_firmware(struct qlcnic_adapter *adapter);
@@ -1213,7 +1386,7 @@ int qlcnic_pinit_from_rom(struct qlcnic_adapter *adapter);
 int qlcnic_setup_idc_param(struct qlcnic_adapter *adapter);
 int qlcnic_check_flash_fw_ver(struct qlcnic_adapter *adapter);
 
-int qlcnic_rom_fast_read(struct qlcnic_adapter *adapter, int addr, int *valp);
+int qlcnic_rom_fast_read(struct qlcnic_adapter *adapter, u32 addr, u32 *valp);
 int qlcnic_rom_fast_read_words(struct qlcnic_adapter *adapter, int addr,
 				u8 *bytes, size_t size);
 int qlcnic_alloc_sw_resources(struct qlcnic_adapter *adapter);
@@ -1265,6 +1438,7 @@ int qlcnic_diag_alloc_res(struct net_device *netdev, int test);
 netdev_tx_t qlcnic_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
 int qlcnic_validate_max_rss(struct net_device *netdev, u8 max_hw, u8 val);
 int qlcnic_set_max_rss(struct qlcnic_adapter *adapter, u8 data);
+void qlcnic_dev_request_reset(struct qlcnic_adapter *);
 
 /* Management functions */
 int qlcnic_get_mac_address(struct qlcnic_adapter *, u8*);
diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
index 3a99886..bab041a 100644
--- a/drivers/net/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/qlcnic/qlcnic_ctx.c
@@ -64,6 +64,97 @@ qlcnic_issue_cmd(struct qlcnic_adapter *adapter,
 	return rcode;
 }
 
+static uint32_t qlcnic_temp_checksum(uint32_t *temp_buffer, u16 temp_size)
+{
+	uint64_t sum = 0;
+	int count = temp_size / sizeof(uint32_t);
+	while (count-- > 0)
+		sum += *temp_buffer++;
+	while (sum >> 32)
+		sum = (sum & 0xFFFFFFFF) + (sum >> 32);
+	return ~sum;
+}
+
+int qlcnic_fw_cmd_get_minidump_temp(struct qlcnic_adapter *adapter)
+{
+	int err, i;
+	u16 temp_size;
+	void *tmp_addr;
+	u32 version, csum, *template, *tmp_buf;
+	struct qlcnic_hardware_context *ahw;
+	struct qlcnic_dump_template_hdr *tmpl_hdr, *tmp_tmpl;
+	dma_addr_t tmp_addr_t = 0;
+
+	ahw = adapter->ahw;
+	err = qlcnic_issue_cmd(adapter,
+			adapter->ahw->pci_func,
+			adapter->fw_hal_version,
+			0,
+			0,
+			0,
+			QLCNIC_CDRP_CMD_TEMP_SIZE);
+	if (err != QLCNIC_RCODE_SUCCESS) {
+		err = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
+		dev_err(&adapter->pdev->dev,
+			"Failed to get template size %d\n", err);
+		err = -EIO;
+		return err;
+	}
+	version = QLCRD32(adapter, QLCNIC_ARG3_CRB_OFFSET);
+	temp_size = QLCRD32(adapter, QLCNIC_ARG2_CRB_OFFSET);
+	if (!temp_size)
+		return -EIO;
+
+	tmp_addr = dma_alloc_coherent(&adapter->pdev->dev, temp_size,
+			&tmp_addr_t, GFP_KERNEL);
+	if (!tmp_addr) {
+		dev_err(&adapter->pdev->dev,
+			"Can't get memory for FW dump template\n");
+		return -ENOMEM;
+	}
+	err = qlcnic_issue_cmd(adapter,
+		adapter->ahw->pci_func,
+		adapter->fw_hal_version,
+		LSD(tmp_addr_t),
+		MSD(tmp_addr_t),
+		temp_size,
+		QLCNIC_CDRP_CMD_GET_TEMP_HDR);
+
+	if (err != QLCNIC_RCODE_SUCCESS) {
+		dev_err(&adapter->pdev->dev,
+			"Failed to get mini dump template header %d\n", err);
+		err = -EIO;
+		goto error;
+	}
+	tmp_tmpl = (struct qlcnic_dump_template_hdr *) tmp_addr;
+	csum = qlcnic_temp_checksum((uint32_t *) tmp_addr, temp_size);
+	if (csum) {
+		dev_err(&adapter->pdev->dev,
+			"Template header checksum validation failed\n");
+		err = -EIO;
+		goto error;
+	}
+	ahw->fw_dump.tmpl_hdr = vzalloc(temp_size);
+	if (!ahw->fw_dump.tmpl_hdr) {
+		err = -EIO;
+		goto error;
+	}
+	tmp_buf = (u32 *) tmp_addr;
+	template = (u32 *) ahw->fw_dump.tmpl_hdr;
+	for (i = 0; i < temp_size/sizeof(u32); i++)
+		*template++ = __le32_to_cpu(*tmp_buf++);
+
+	tmpl_hdr = ahw->fw_dump.tmpl_hdr;
+	if (tmpl_hdr->cap_mask > QLCNIC_DUMP_MASK_DEF &&
+		tmpl_hdr->cap_mask <= QLCNIC_DUMP_MASK_MAX)
+		tmpl_hdr->drv_cap_mask = tmpl_hdr->cap_mask;
+	else
+		tmpl_hdr->drv_cap_mask = QLCNIC_DUMP_MASK_DEF;
+error:
+	dma_free_coherent(&adapter->pdev->dev, temp_size, tmp_addr, tmp_addr_t);
+	return err;
+}
+
 int
 qlcnic_fw_cmd_set_mtu(struct qlcnic_adapter *adapter, int mtu)
 {
diff --git a/drivers/net/qlcnic/qlcnic_hdr.h b/drivers/net/qlcnic/qlcnic_hdr.h
index 726ef55..d14506f 100644
--- a/drivers/net/qlcnic/qlcnic_hdr.h
+++ b/drivers/net/qlcnic/qlcnic_hdr.h
@@ -492,10 +492,10 @@ enum {
 
 #define TEST_AGT_CTRL	(0x00)
 
-#define TA_CTL_START	1
-#define TA_CTL_ENABLE	2
-#define TA_CTL_WRITE	4
-#define TA_CTL_BUSY	8
+#define TA_CTL_START	BIT_0
+#define TA_CTL_ENABLE	BIT_1
+#define TA_CTL_WRITE	BIT_2
+#define TA_CTL_BUSY	BIT_3
 
 /*
  *   Register offsets for MN
@@ -765,6 +765,38 @@ struct qlcnic_legacy_intr_set {
 #define QLCNIC_MAX_PCI_FUNC	8
 #define QLCNIC_MAX_VLAN_FILTERS	64
 
+/* FW dump defines */
+#define MIU_TEST_CTR		0x41000090
+#define MIU_TEST_ADDR_LO	0x41000094
+#define MIU_TEST_ADDR_HI	0x41000098
+#define FLASH_ROM_WINDOW	0x42110030
+#define FLASH_ROM_DATA		0x42150000
+
+static const u32 MIU_TEST_READ_DATA[] = {
+	0x410000A8, 0x410000AC, 0x410000B8, 0x410000BC, };
+
+#define QLCNIC_FW_DUMP_REG1	0x00130060
+#define QLCNIC_FW_DUMP_REG2	0x001e0000
+#define QLCNIC_FLASH_SEM2_LK	0x0013C010
+#define QLCNIC_FLASH_SEM2_ULK	0x0013C014
+#define QLCNIC_FLASH_LOCK_ID	0x001B2100
+
+#define QLCNIC_RD_DUMP_REG(addr, bar0, data) do {			\
+	writel((addr & 0xFFFF0000), (void *) (bar0 +			\
+		QLCNIC_FW_DUMP_REG1));					\
+	readl((void *) (bar0 + QLCNIC_FW_DUMP_REG1));			\
+	*data = readl((void *) (bar0 + QLCNIC_FW_DUMP_REG2 +		\
+		LSW(addr)));						\
+} while (0)
+
+#define QLCNIC_WR_DUMP_REG(addr, bar0, data) do {			\
+	writel((addr & 0xFFFF0000), (void *) (bar0 +			\
+		QLCNIC_FW_DUMP_REG1));					\
+	readl((void *) (bar0 + QLCNIC_FW_DUMP_REG1));			\
+	writel(data, (void *) (bar0 + QLCNIC_FW_DUMP_REG2 + LSW(addr)));\
+	readl((void *) (bar0 + QLCNIC_FW_DUMP_REG2 + LSW(addr)));	\
+} while (0)
+
 /* PCI function operational mode */
 enum {
 	QLCNIC_MGMT_FUNC	= 0,
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index cbb27f2..869f47b 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -9,6 +9,7 @@
 
 #include <linux/slab.h>
 #include <net/ip.h>
+#include <linux/bitops.h>
 
 #define MASK(n) ((1ULL<<(n))-1)
 #define OCM_WIN_P3P(addr) (addr & 0xffc0000)
@@ -1261,3 +1262,462 @@ int qlcnic_config_led(struct qlcnic_adapter *adapter, u32 state, u32 rate)
 
 	return rv;
 }
+
+/* FW dump related functions */
+static u32
+qlcnic_dump_crb(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
+		u32 *buffer)
+{
+	int i;
+	u32 addr, data;
+	struct __crb *crb = &entry->region.crb;
+	void __iomem *base = adapter->ahw->pci_base0;
+
+	addr = crb->addr;
+
+	for (i = 0; i < crb->no_ops; i++) {
+		QLCNIC_RD_DUMP_REG(addr, base, &data);
+		*buffer++ = cpu_to_le32(addr);
+		*buffer++ = cpu_to_le32(data);
+		addr += crb->stride;
+	}
+	return crb->no_ops * 2 * sizeof(u32);
+}
+
+static u32
+qlcnic_dump_ctrl(struct qlcnic_adapter *adapter,
+	struct qlcnic_dump_entry *entry, u32 *buffer)
+{
+	int i, k, timeout = 0;
+	void __iomem *base = adapter->ahw->pci_base0;
+	u32 addr, data;
+	u8 opcode, no_ops;
+	struct __ctrl *ctr = &entry->region.ctrl;
+	struct qlcnic_dump_template_hdr *t_hdr = adapter->ahw->fw_dump.tmpl_hdr;
+
+	addr = ctr->addr;
+	no_ops = ctr->no_ops;
+
+	for (i = 0; i < no_ops; i++) {
+		k = 0;
+		opcode = 0;
+		for (k = 0; k < 8; k++) {
+			if (!(ctr->opcode & (1 << k)))
+				continue;
+			switch (1 << k) {
+			case QLCNIC_DUMP_WCRB:
+				QLCNIC_WR_DUMP_REG(addr, base, ctr->val1);
+				break;
+			case QLCNIC_DUMP_RWCRB:
+				QLCNIC_RD_DUMP_REG(addr, base, &data);
+				QLCNIC_WR_DUMP_REG(addr, base, data);
+				break;
+			case QLCNIC_DUMP_ANDCRB:
+				QLCNIC_RD_DUMP_REG(addr, base, &data);
+				QLCNIC_WR_DUMP_REG(addr, base,
+					(data & ctr->val2));
+				break;
+			case QLCNIC_DUMP_ORCRB:
+				QLCNIC_RD_DUMP_REG(addr, base, &data);
+				QLCNIC_WR_DUMP_REG(addr, base,
+					(data | ctr->val3));
+				break;
+			case QLCNIC_DUMP_POLLCRB:
+				while (timeout <= ctr->timeout) {
+					QLCNIC_RD_DUMP_REG(addr, base, &data);
+					if ((data & ctr->val2) == ctr->val1)
+						break;
+					msleep(1);
+					timeout++;
+				}
+				if (timeout > ctr->timeout) {
+					dev_info(&adapter->pdev->dev,
+					"Timed out, aborting poll CRB\n");
+					return -EINVAL;
+				}
+				break;
+			case QLCNIC_DUMP_RD_SAVE:
+				if (ctr->index_a)
+					addr = t_hdr->saved_state[ctr->index_a];
+				QLCNIC_RD_DUMP_REG(addr, base, &data);
+				t_hdr->saved_state[ctr->index_v] = data;
+				break;
+			case QLCNIC_DUMP_WRT_SAVED:
+				if (ctr->index_v)
+					data = t_hdr->saved_state[ctr->index_v];
+				else
+					data = ctr->val1;
+				if (ctr->index_a)
+					addr = t_hdr->saved_state[ctr->index_a];
+				QLCNIC_WR_DUMP_REG(addr, base, data);
+				break;
+			case QLCNIC_DUMP_MOD_SAVE_ST:
+				data = t_hdr->saved_state[ctr->index_v];
+				data <<= ctr->shl_val;
+				data >>= ctr->shr_val;
+				if (ctr->val2)
+					data &= ctr->val2;
+				data |= ctr->val3;
+				data += ctr->val1;
+				t_hdr->saved_state[ctr->index_v] = data;
+				break;
+			default:
+				dev_info(&adapter->pdev->dev,
+					"Unknown opcode\n");
+				break;
+			}
+		}
+		addr += ctr->stride;
+	}
+	return 0;
+}
+
+static u32
+qlcnic_dump_mux(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
+	u32 *buffer)
+{
+	int loop;
+	u32 val, data = 0;
+	struct __mux *mux = &entry->region.mux;
+	void __iomem *base = adapter->ahw->pci_base0;
+
+	val = mux->val;
+	for (loop = 0; loop < mux->no_ops; loop++) {
+		QLCNIC_WR_DUMP_REG(mux->addr, base, val);
+		QLCNIC_RD_DUMP_REG(mux->read_addr, base, &data);
+		*buffer++ = cpu_to_le32(val);
+		*buffer++ = cpu_to_le32(data);
+		val += mux->val_stride;
+	}
+	return 2 * mux->no_ops * sizeof(u32);
+}
+
+static u32
+qlcnic_dump_que(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
+	u32 *buffer)
+{
+	int i, loop;
+	u32 cnt, addr, data, que_id = 0;
+	void __iomem *base = adapter->ahw->pci_base0;
+	struct __queue *que = &entry->region.que;
+
+	addr = que->read_addr;
+	cnt = que->read_addr_cnt;
+
+	for (loop = 0; loop < que->no_ops; loop++) {
+		QLCNIC_WR_DUMP_REG(que->sel_addr, base, que_id);
+		for (i = 0; i < cnt; i++) {
+			QLCNIC_RD_DUMP_REG(addr, base, &data);
+			*buffer++ = cpu_to_le32(data);
+			addr += que->read_addr_stride;
+		}
+		que_id += que->stride;
+	}
+	return que->no_ops * cnt * sizeof(u32);
+}
+
+static u32
+qlcnic_dump_ocm(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
+	u32 *buffer)
+{
+	int i;
+	u32 data;
+	void __iomem *addr;
+	struct __ocm *ocm = &entry->region.ocm;
+
+	addr = adapter->ahw->pci_base0 + ocm->read_addr;
+	for (i = 0; i < ocm->no_ops; i++) {
+		data = readl(addr);
+		*buffer++ = cpu_to_le32(data);
+		addr += ocm->read_addr_stride;
+	}
+	return ocm->no_ops * sizeof(u32);
+}
+
+static u32
+qlcnic_read_rom(struct qlcnic_adapter *adapter, struct qlcnic_dump_entry *entry,
+	u32 *buffer)
+{
+	int i, count = 0;
+	u32 fl_addr, size, val, lck_val, addr;
+	struct __mem *rom = &entry->region.mem;
+	void __iomem *base = adapter->ahw->pci_base0;
+
+	fl_addr = rom->addr;
+	size = rom->size/4;
+lock_try:
+	lck_val = readl(base + QLCNIC_FLASH_SEM2_LK);
+	if (!lck_val && count < MAX_CTL_CHECK) {
+		msleep(10);
+		count++;
+		goto lock_try;
+	}
+	writel(adapter->ahw->pci_func, (base + QLCNIC_FLASH_LOCK_ID));
+	for (i = 0; i < size; i++) {
+		addr = fl_addr & 0xFFFF0000;
+		QLCNIC_WR_DUMP_REG(FLASH_ROM_WINDOW, base, addr);
+		addr = LSW(fl_addr) + FLASH_ROM_DATA;
+		QLCNIC_RD_DUMP_REG(addr, base, &val);
+		fl_addr += 4;
+		*buffer++ = cpu_to_le32(val);
+	}
+	readl(base + QLCNIC_FLASH_SEM2_ULK);
+	return rom->size;
+}
+
+static u32
+qlcnic_dump_l1_cache(struct qlcnic_adapter *adapter,
+	struct qlcnic_dump_entry *entry, u32 *buffer)
+{
+	int i;
+	u32 cnt, val, data, addr;
+	void __iomem *base = adapter->ahw->pci_base0;
+	struct __cache *l1 = &entry->region.cache;
+
+	val = l1->init_tag_val;
+
+	for (i = 0; i < l1->no_ops; i++) {
+		QLCNIC_WR_DUMP_REG(l1->addr, base, val);
+		QLCNIC_WR_DUMP_REG(l1->ctrl_addr, base, LSW(l1->ctrl_val));
+		addr = l1->read_addr;
+		cnt = l1->read_addr_num;
+		while (cnt) {
+			QLCNIC_RD_DUMP_REG(addr, base, &data);
+			*buffer++ = cpu_to_le32(data);
+			addr += l1->read_addr_stride;
+			cnt--;
+		}
+		val += l1->stride;
+	}
+	return l1->no_ops * l1->read_addr_num * sizeof(u32);
+}
+
+static u32
+qlcnic_dump_l2_cache(struct qlcnic_adapter *adapter,
+	struct qlcnic_dump_entry *entry, u32 *buffer)
+{
+	int i;
+	u32 cnt, val, data, addr;
+	u8 poll_mask, poll_to, time_out = 0;
+	void __iomem *base = adapter->ahw->pci_base0;
+	struct __cache *l2 = &entry->region.cache;
+
+	val = l2->init_tag_val;
+	poll_mask = LSB(MSW(l2->ctrl_val));
+	poll_to = MSB(MSW(l2->ctrl_val));
+
+	for (i = 0; i < l2->no_ops; i++) {
+		QLCNIC_WR_DUMP_REG(l2->addr, base, val);
+		do {
+			QLCNIC_WR_DUMP_REG(l2->ctrl_addr, base,
+				LSW(l2->ctrl_val));
+			QLCNIC_RD_DUMP_REG(l2->ctrl_addr, base, &data);
+			if (!(data & poll_mask))
+				break;
+			msleep(1);
+			time_out++;
+		} while (time_out <= poll_to);
+		if (time_out > poll_to)
+			return -EINVAL;
+
+		addr = l2->read_addr;
+		cnt = l2->read_addr_num;
+		while (cnt) {
+			QLCNIC_RD_DUMP_REG(addr, base, &data);
+			*buffer++ = cpu_to_le32(data);
+			addr += l2->read_addr_stride;
+			cnt--;
+		}
+		val += l2->stride;
+	}
+	return l2->no_ops * l2->read_addr_num * sizeof(u32);
+}
+
+static u32
+qlcnic_read_memory(struct qlcnic_adapter *adapter,
+	struct qlcnic_dump_entry *entry, u32 *buffer)
+{
+	u32 addr, data, test, ret = 0;
+	int i, reg_read;
+	struct __mem *mem = &entry->region.mem;
+	void __iomem *base = adapter->ahw->pci_base0;
+
+	reg_read = mem->size;
+	addr = mem->addr;
+	/* check for data size of multiple of 16 and 16 byte alignment */
+	if ((addr & 0xf) || (reg_read%16)) {
+		dev_info(&adapter->pdev->dev,
+			"Unaligned memory addr:0x%x size:0x%x\n",
+			addr, reg_read);
+		return -EINVAL;
+	}
+
+	mutex_lock(&adapter->ahw->mem_lock);
+
+	while (reg_read != 0) {
+		QLCNIC_WR_DUMP_REG(MIU_TEST_ADDR_LO, base, addr);
+		QLCNIC_WR_DUMP_REG(MIU_TEST_ADDR_HI, base, 0);
+		QLCNIC_WR_DUMP_REG(MIU_TEST_CTR, base,
+			TA_CTL_ENABLE | TA_CTL_START);
+
+		for (i = 0; i < MAX_CTL_CHECK; i++) {
+			QLCNIC_RD_DUMP_REG(MIU_TEST_CTR, base, &test);
+			if (!(test & TA_CTL_BUSY))
+				break;
+		}
+		if (i == MAX_CTL_CHECK) {
+			if (printk_ratelimit()) {
+				dev_err(&adapter->pdev->dev,
+					"failed to read through agent\n");
+				ret = -EINVAL;
+				goto out;
+			}
+		}
+		for (i = 0; i < 4; i++) {
+			QLCNIC_RD_DUMP_REG(MIU_TEST_READ_DATA[i], base, &data);
+			*buffer++ = cpu_to_le32(data);
+		}
+		addr += 16;
+		reg_read -= 16;
+		ret += 16;
+	}
+out:
+	mutex_unlock(&adapter->ahw->mem_lock);
+	return mem->size;
+}
+
+static u32
+qlcnic_dump_nop(struct qlcnic_adapter *adapter,
+	struct qlcnic_dump_entry *entry, u32 *buffer)
+{
+	entry->hdr.flags |= QLCNIC_DUMP_SKIP;
+	return 0;
+}
+
+struct qlcnic_dump_operations fw_dump_ops[] = {
+	{ QLCNIC_DUMP_NOP, qlcnic_dump_nop },
+	{ QLCNIC_DUMP_READ_CRB, qlcnic_dump_crb },
+	{ QLCNIC_DUMP_READ_MUX, qlcnic_dump_mux },
+	{ QLCNIC_DUMP_QUEUE, qlcnic_dump_que },
+	{ QLCNIC_DUMP_BRD_CONFIG, qlcnic_read_rom },
+	{ QLCNIC_DUMP_READ_OCM, qlcnic_dump_ocm },
+	{ QLCNIC_DUMP_PEG_REG, qlcnic_dump_ctrl },
+	{ QLCNIC_DUMP_L1_DTAG, qlcnic_dump_l1_cache },
+	{ QLCNIC_DUMP_L1_ITAG, qlcnic_dump_l1_cache },
+	{ QLCNIC_DUMP_L1_DATA, qlcnic_dump_l1_cache },
+	{ QLCNIC_DUMP_L1_INST, qlcnic_dump_l1_cache },
+	{ QLCNIC_DUMP_L2_DTAG, qlcnic_dump_l2_cache },
+	{ QLCNIC_DUMP_L2_ITAG, qlcnic_dump_l2_cache },
+	{ QLCNIC_DUMP_L2_DATA, qlcnic_dump_l2_cache },
+	{ QLCNIC_DUMP_L2_INST, qlcnic_dump_l2_cache },
+	{ QLCNIC_DUMP_READ_ROM, qlcnic_read_rom },
+	{ QLCNIC_DUMP_READ_MEM, qlcnic_read_memory },
+	{ QLCNIC_DUMP_READ_CTRL, qlcnic_dump_ctrl },
+	{ QLCNIC_DUMP_TLHDR, qlcnic_dump_nop },
+	{ QLCNIC_DUMP_RDEND, qlcnic_dump_nop },
+};
+
+/* Walk the template and collect dump for each entry in the dump template */
+static int
+qlcnic_valid_dump_entry(struct device *dev, struct qlcnic_dump_entry *entry,
+	u32 size)
+{
+	int ret = 1;
+	if (size != entry->hdr.cap_size) {
+		dev_info(dev,
+		"Invalidate dump, Type:%d\tMask:%d\tSize:%dCap_size:%d\n",
+		entry->hdr.type, entry->hdr.mask, size, entry->hdr.cap_size);
+		dev_info(dev, "Aborting further dump capture\n");
+		ret = 0;
+	}
+	return ret;
+}
+
+int qlcnic_dump_fw(struct qlcnic_adapter *adapter)
+{
+	u32 *buffer;
+	char mesg[64];
+	char *msg[] = {mesg, NULL};
+	int i, k, ops_cnt, ops_index, dump_size = 0;
+	u32 entry_offset, dump, no_entries, buf_offset = 0;
+	struct qlcnic_dump_entry *entry;
+	struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
+	struct qlcnic_dump_template_hdr *tmpl_hdr = fw_dump->tmpl_hdr;
+
+	if (fw_dump->clr) {
+		dev_info(&adapter->pdev->dev,
+			"Previous dump not cleared, not capturing dump\n");
+		return -EIO;
+	}
+	/* Calculate the size for dump data area only */
+	for (i = 2, k = 1; (i & QLCNIC_DUMP_MASK_MAX); i <<= 1, k++)
+		if (i & tmpl_hdr->drv_cap_mask)
+			dump_size += tmpl_hdr->cap_sizes[k];
+	if (!dump_size)
+		return -EIO;
+
+	fw_dump->data = vzalloc(dump_size);
+	if (!fw_dump->data) {
+		dev_info(&adapter->pdev->dev,
+			"Unable to allocate (%d KB) for fw dump\n",
+			dump_size/1024);
+		return -ENOMEM;
+	}
+	buffer = fw_dump->data;
+	fw_dump->size = dump_size;
+	no_entries = tmpl_hdr->num_entries;
+	ops_cnt = ARRAY_SIZE(fw_dump_ops);
+	entry_offset = tmpl_hdr->offset;
+	tmpl_hdr->sys_info[0] = QLCNIC_DRIVER_VERSION;
+	tmpl_hdr->sys_info[1] = adapter->fw_version;
+
+	for (i = 0; i < no_entries; i++) {
+		entry = (struct qlcnic_dump_entry *) ((void *) tmpl_hdr +
+			entry_offset);
+		if (!(entry->hdr.mask & tmpl_hdr->drv_cap_mask)) {
+			entry->hdr.flags |= QLCNIC_DUMP_SKIP;
+			entry_offset += entry->hdr.offset;
+			continue;
+		}
+		/* Find the handler for this entry */
+		ops_index = 0;
+		while (ops_index < ops_cnt) {
+			if (entry->hdr.type == fw_dump_ops[ops_index].opcode)
+				break;
+			ops_index++;
+		}
+		if (ops_index == ops_cnt) {
+			dev_info(&adapter->pdev->dev,
+				"Invalid entry type %d, exiting dump\n",
+				entry->hdr.type);
+			goto error;
+		}
+		/* Collect dump for this entry */
+		dump = fw_dump_ops[ops_index].handler(adapter, entry, buffer);
+		if (dump && !qlcnic_valid_dump_entry(&adapter->pdev->dev, entry,
+			dump))
+			entry->hdr.flags |= QLCNIC_DUMP_SKIP;
+		buf_offset += entry->hdr.cap_size;
+		entry_offset += entry->hdr.offset;
+		buffer = fw_dump->data + buf_offset;
+	}
+	if (dump_size != buf_offset) {
+		dev_info(&adapter->pdev->dev,
+			"Captured(%d) and expected size(%d) do not match\n",
+			buf_offset, dump_size);
+		goto error;
+	} else {
+		fw_dump->clr = 1;
+		snprintf(mesg, sizeof(mesg), "FW dump for device: %d\n",
+			adapter->pdev->devfn);
+		dev_info(&adapter->pdev->dev, "Dump data, %d bytes captured\n",
+			fw_dump->size);
+		/* Send a udev event to notify availability of FW dump */
+		kobject_uevent_env(&adapter->pdev->dev.kobj, KOBJ_CHANGE, msg);
+		return 0;
+	}
+error:
+	vfree(fw_dump->data);
+	fw_dump->size = 0;
+	return -EINVAL;
+}
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index d0f338b..5b8bbcf 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -345,7 +345,7 @@ static int qlcnic_wait_rom_done(struct qlcnic_adapter *adapter)
 }
 
 static int do_rom_fast_read(struct qlcnic_adapter *adapter,
-			    int addr, int *valp)
+			    u32 addr, u32 *valp)
 {
 	QLCWR32(adapter, QLCNIC_ROMUSB_ROM_ADDRESS, addr);
 	QLCWR32(adapter, QLCNIC_ROMUSB_ROM_DUMMY_BYTE_CNT, 0);
@@ -398,7 +398,7 @@ qlcnic_rom_fast_read_words(struct qlcnic_adapter *adapter, int addr,
 	return ret;
 }
 
-int qlcnic_rom_fast_read(struct qlcnic_adapter *adapter, int addr, int *valp)
+int qlcnic_rom_fast_read(struct qlcnic_adapter *adapter, u32 addr, u32 *valp)
 {
 	int ret;
 
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index d6cc4d4..3ab7d2c 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -1343,6 +1343,10 @@ static void qlcnic_free_adapter_resources(struct qlcnic_adapter *adapter)
 	kfree(adapter->recv_ctx);
 	adapter->recv_ctx = NULL;
 
+	if (adapter->ahw->fw_dump.tmpl_hdr) {
+		vfree(adapter->ahw->fw_dump.tmpl_hdr);
+		adapter->ahw->fw_dump.tmpl_hdr = NULL;
+	}
 	kfree(adapter->ahw);
 	adapter->ahw = NULL;
 }
@@ -1586,6 +1590,10 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* This will be reset for mezz cards  */
 	adapter->portnum = adapter->ahw->pci_func;
 
+	/* Get FW dump template and store it */
+	if (adapter->op_mode != QLCNIC_NON_PRIV_FUNC)
+		qlcnic_fw_cmd_get_minidump_temp(adapter);
+
 	err = qlcnic_get_board_info(adapter);
 	if (err) {
 		dev_err(&pdev->dev, "Error getting board config info.\n");
@@ -2825,6 +2833,8 @@ skip_ack_check:
 			set_bit(__QLCNIC_START_FW, &adapter->state);
 			QLCDB(adapter, DRV, "Restarting fw\n");
 			qlcnic_idc_debug_info(adapter, 0);
+			QLCDB(adapter, DRV, "Take FW dump\n");
+			qlcnic_dump_fw(adapter);
 		}
 
 		qlcnic_api_unlock(adapter);
@@ -2923,7 +2933,7 @@ qlcnic_set_npar_non_operational(struct qlcnic_adapter *adapter)
 }
 
 /*Transit to RESET state from READY state only */
-static void
+void
 qlcnic_dev_request_reset(struct qlcnic_adapter *adapter)
 {
 	u32 state;
@@ -3515,7 +3525,6 @@ qlcnic_sysfs_write_mem(struct file *filp, struct kobject *kobj,
 	return size;
 }
 
-
 static struct bin_attribute bin_attr_crb = {
 	.attr = {.name = "crb", .mode = (S_IRUGO | S_IWUSR)},
 	.size = 0,
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2] ethtool: Allow ethtool to take FW dump
From: Anirban Chakraborty @ 2011-05-10  1:02 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, davem, Anirban Chakraborty

Take FW dump via ethtool.

Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 ethtool-copy.h |   28 +++++++++++++-
 ethtool.c      |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 141 insertions(+), 3 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 22215e9..d6115c5 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -76,6 +76,31 @@ struct ethtool_drvinfo {
 	__u32	regdump_len;	/* Size of data from ETHTOOL_GREGS (bytes) */
 };
 
+/**
+ * struct ethtool_dump - used for retrieving, setting device dump
+ * @type: type of operation, get dump settings or data
+ * @version: FW version of the dump
+ * @flag: flag for dump setting
+ * @len: length of dump data
+ * @data: data collected for this command
+ */
+struct ethtool_dump {
+	__u32	cmd;
+	__u32	type;
+	__u32	version;
+	__u32	flag;
+	__u32	len;
+	__u8	data[0];
+};
+
+/*
+ * ethtool_dump_op_type - used to determine type of fetch, flag or data
+ */
+enum ethtool_dump_op_type {
+	ETHTOOL_DUMP_FLAG	= 0,
+	ETHTOOL_DUMP_DATA,
+};
+
 #define SOPASS_MAX	6
 /* wake-on-lan settings */
 struct ethtool_wolinfo {
@@ -654,7 +679,6 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_F_WISH          (1 << ETHTOOL_F_WISH__BIT)
 #define ETHTOOL_F_COMPAT        (1 << ETHTOOL_F_COMPAT__BIT)
 
-
 /* CMDs currently supported */
 #define ETHTOOL_GSET		0x00000001 /* Get settings. */
 #define ETHTOOL_SSET		0x00000002 /* Set settings. */
@@ -717,6 +741,8 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
+#define ETHTOOL_SET_DUMP	0x0000003e /* Set dump settings */
+#define ETHTOOL_GET_DUMP	0x0000003f /* Get dump settings */
 
 #define ETHTOOL_GFEATURES	0x0000003a /* Get device offload settings */
 #define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
diff --git a/ethtool.c b/ethtool.c
index cfdac65..5b91326 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
@@ -110,6 +111,8 @@ static int do_srxntuple(int fd, struct ifreq *ifr);
 static int do_grxntuple(int fd, struct ifreq *ifr);
 static int do_flash(int fd, struct ifreq *ifr);
 static int do_permaddr(int fd, struct ifreq *ifr);
+static int do_getfwdump(int fd, struct ifreq *ifr);
+static int do_setfwdump(int fd, struct ifreq *ifr);
 
 static int send_ioctl(int fd, struct ifreq *ifr);
 
@@ -142,6 +145,8 @@ static enum {
 	MODE_GNTUPLE,
 	MODE_FLASHDEV,
 	MODE_PERMADDR,
+	MODE_SET_DUMP,
+	MODE_GET_DUMP,
 } mode = MODE_GSET;
 
 static struct option {
@@ -263,6 +268,12 @@ static struct option {
 		"Get Rx ntuple filters and actions\n" },
     { "-P", "--show-permaddr", MODE_PERMADDR,
 		"Show permanent hardware address" },
+    { "-w", "--get-dump", MODE_GET_DUMP,
+		"Get dump and options",
+		"		[ data FILENAME ]\n" },
+    { "-W", "--set-dump", MODE_SET_DUMP,
+		"Set dump flag",
+		"		dumpflag" " Dump flag for the device\n" },
     { "-h", "--help", MODE_HELP, "Show this help" },
     { NULL, "--version", MODE_VERSION, "Show version number" },
     {}
@@ -413,6 +424,9 @@ static int flash_region = -1;
 static int msglvl_changed;
 static u32 msglvl_wanted = 0;
 static u32 msglvl_mask = 0;
+static u32 dump_flag;
+static u32 dump_type;
+static char *dump_file = NULL;
 
 static enum {
 	ONLINE=0,
@@ -852,7 +866,9 @@ static void parse_cmdline(int argc, char **argp)
 			    (mode == MODE_GNTUPLE) ||
 			    (mode == MODE_PHYS_ID) ||
 			    (mode == MODE_FLASHDEV) ||
-			    (mode == MODE_PERMADDR)) {
+			    (mode == MODE_PERMADDR) ||
+			    (mode == MODE_SET_DUMP) ||
+			    (mode == MODE_GET_DUMP)) {
 				devname = argp[i];
 				break;
 			}
@@ -874,6 +890,9 @@ static void parse_cmdline(int argc, char **argp)
 				flash_file = argp[i];
 				flash = 1;
 				break;
+			} else if (mode == MODE_SET_DUMP) {
+				dump_flag = get_u32(argp[i], 0);
+				break;
 			}
 			/* fallthrough */
 		default:
@@ -1020,6 +1039,23 @@ static void parse_cmdline(int argc, char **argp)
 				}
 				break;
 			}
+			if (mode == MODE_GET_DUMP) {
+				if (argc != i + 2) {
+					exit_bad_args();
+					break;
+				}
+				if (!strcmp(argp[i], "data"))
+					dump_type = ETHTOOL_DUMP_DATA;
+				else {
+					exit_bad_args();
+					break;
+				}
+				i += 1;
+				dump_file = argp[i];
+				i = argc;
+fprintf(stdout, "file name: %s\n", dump_file);
+				break;
+			}
 			if (mode != MODE_SSET)
 				exit_bad_args();
 			if (!strcmp(argp[i], "speed")) {
@@ -2042,6 +2078,10 @@ static int doit(void)
 		return do_flash(fd, &ifr);
 	} else if (mode == MODE_PERMADDR) {
 		return do_permaddr(fd, &ifr);
+	} else if (mode == MODE_GET_DUMP) {
+		return do_getfwdump(fd, &ifr);
+	} else if (mode == MODE_SET_DUMP) {
+		return do_setfwdump(fd, &ifr);
 	}
 
 	return 69;
@@ -2679,7 +2719,6 @@ static int do_gregs(int fd, struct ifreq *ifr)
 		perror("Cannot get driver information");
 		return 72;
 	}
-
 	regs = calloc(1, sizeof(*regs)+drvinfo.regdump_len);
 	if (!regs) {
 		perror("Cannot allocate memory for register dump");
@@ -3241,6 +3280,79 @@ static int do_grxntuple(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+static void do_writedump(struct ethtool_dump *dump)
+{
+	FILE *f;
+	size_t bytes;
+
+	f = fopen(dump_file, "wb+");
+
+	if (!f) {
+		fprintf(stderr, "Can't open file %s: %s\n",
+			dump_file, strerror(errno));
+		return;
+	}
+	bytes = fwrite(dump->data, 1, dump->len, f);
+	if (fclose(f))
+		fprintf(stderr, "Can't close file %s: %s\n",
+			dump_file, strerror(errno));
+}
+
+static int do_getfwdump(int fd, struct ifreq *ifr)
+{
+	int err;
+	struct ethtool_dump edata;
+	struct ethtool_dump *data;
+
+	edata.cmd = ETHTOOL_GET_DUMP;
+	edata.type = ETHTOOL_DUMP_FLAG;
+
+	ifr->ifr_data = (caddr_t) &edata;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Can not get dump level");
+		return 74;
+	}
+	if (dump_type == ETHTOOL_DUMP_FLAG) {
+		fprintf(stdout, "flag: %u, version: %u length: %u\n",
+			edata.flag, edata.version, edata.len);
+		return 0;
+	}
+	data = calloc(1, offsetof(struct ethtool_dump, data) + edata.len);
+	if (!data) {
+		perror("Can not allocate enough memory");
+		return 0;
+	}
+	data->cmd = ETHTOOL_GET_DUMP;
+	data->type = dump_type;
+	ifr->ifr_data = (caddr_t) data;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Can not get dump data\n");
+		goto free;
+	}
+	do_writedump(data);
+free:
+	free(data);
+	return 0;
+}
+
+static int do_setfwdump(int fd, struct ifreq *ifr)
+{
+	int err;
+	struct ethtool_dump dump;
+
+	dump.cmd = ETHTOOL_SET_DUMP;
+	dump.flag = dump_flag;
+	ifr->ifr_data = (caddr_t)&dump;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Can not set dump level");
+		return 74;
+	}
+	return 0;
+}
+
 static int send_ioctl(int fd, struct ifreq *ifr)
 {
 	return ioctl(fd, SIOCETHTOOL, ifr);
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2 net-next-2.6] ethtool: Added support for FW dump
From: Anirban Chakraborty @ 2011-05-10  1:02 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, davem, Anirban Chakraborty
In-Reply-To: <1304989352-24810-1-git-send-email-anirban.chakraborty@qlogic.com>

Added code to take FW dump via ethtool. A pair of set and get functions are
added to configure dump level and fetch dump data from the driver respectively.

Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 include/linux/ethtool.h |   31 +++++++++++++++++++++++
 net/core/ethtool.c      |   62 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 0 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index bd0b50b..f2cd7e1 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -601,6 +601,31 @@ struct ethtool_flash {
 	char	data[ETHTOOL_FLASH_MAX_FILENAME];
 };
 
+/**
+ * struct ethtool_dump - used for retrieving, setting device dump
+ * @type: type of operation, get dump settings or data
+ * @version: FW version of the dump
+ * @flag: flag for dump setting
+ * @len: length of dump data
+ * @data: data collected for this command
+ */
+struct ethtool_dump {
+	__u32	cmd;
+	__u32	type;
+	__u32	version;
+	__u32	flag;
+	__u32	len;
+	u8	data[0];
+};
+
+/*
+ * ethtool_dump_op_type - used to determine type of fetch, flag or data
+ */
+enum ethtool_dump_op_type {
+	ETHTOOL_DUMP_FLAG	= 0,
+	ETHTOOL_DUMP_DATA,
+};
+
 /* for returning and changing feature sets */
 
 /**
@@ -853,6 +878,8 @@ bool ethtool_invalid_flags(struct net_device *dev, u32 data, u32 supported);
  * @get_channels: Get number of channels.
  * @set_channels: Set number of channels.  Returns a negative error code or
  *	zero.
+ * @get_dump: Get dump flag indicating current dump settings of the device
+ * @set_dump: Set dump specific flags to the device
  *
  * All operations are optional (i.e. the function pointer may be set
  * to %NULL) and callers must take this into account.  Callers must
@@ -927,6 +954,8 @@ struct ethtool_ops {
 				  const struct ethtool_rxfh_indir *);
 	void	(*get_channels)(struct net_device *, struct ethtool_channels *);
 	int	(*set_channels)(struct net_device *, struct ethtool_channels *);
+	int	(*get_dump)(struct net_device *, struct ethtool_dump *, void *);
+	int	(*set_dump)(struct net_device *, struct ethtool_dump *);
 
 };
 #endif /* __KERNEL__ */
@@ -998,6 +1027,8 @@ struct ethtool_ops {
 #define ETHTOOL_SFEATURES	0x0000003b /* Change device offload settings */
 #define ETHTOOL_GCHANNELS	0x0000003c /* Get no of channels */
 #define ETHTOOL_SCHANNELS	0x0000003d /* Set no of channels */
+#define ETHTOOL_SET_DUMP	0x0000003e /* Set dump settings */
+#define ETHTOOL_GET_DUMP	0x0000003f /* Get dump settings */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index b6f4058..3c3af8b 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1823,6 +1823,62 @@ static noinline_for_stack int ethtool_flash_device(struct net_device *dev,
 	return dev->ethtool_ops->flash_device(dev, &efl);
 }
 
+static int ethtool_set_dump(struct net_device *dev,
+			void __user *useraddr)
+{
+	struct ethtool_dump dump;
+
+	if (!dev->ethtool_ops->set_dump)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&dump, useraddr, sizeof(dump)))
+		return -EFAULT;
+
+	return dev->ethtool_ops->set_dump(dev, &dump);
+}
+
+static int ethtool_get_dump(struct net_device *dev,
+				void __user *useraddr)
+{
+	int ret;
+	void *data = NULL;
+	struct ethtool_dump dump;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	enum ethtool_dump_op_type type;
+
+	if (!dev->ethtool_ops->get_dump)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&dump, useraddr, sizeof(dump)))
+		return -EFAULT;
+
+	type = dump.type;
+	dump.type = ETHTOOL_DUMP_FLAG;
+	ret = ops->get_dump(dev, &dump, data);
+	if (ret)
+		return ret;
+	dump.type = type;
+	if (copy_to_user(useraddr, &dump, sizeof(dump)))
+		return -EFAULT;
+	if (type != ETHTOOL_DUMP_DATA)
+		return 0;
+
+	data = vzalloc(dump.len);
+	if (!data)
+		return -ENOMEM;
+	ret = ops->get_dump(dev, &dump, data);
+	if (ret) {
+		ret = -EFAULT;
+		goto out;
+	}
+	useraddr += offsetof(struct ethtool_dump, data);
+	if (copy_to_user(useraddr, data, dump.len))
+		ret = -EFAULT;
+out:
+	vfree(data);
+	return ret;
+}
+
 /* The main entry point in this file.  Called from net/core/dev.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -2039,6 +2095,12 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SCHANNELS:
 		rc = ethtool_set_channels(dev, useraddr);
 		break;
+	case ETHTOOL_SET_DUMP:
+		rc = ethtool_set_dump(dev, useraddr);
+		break;
+	case ETHTOOL_GET_DUMP:
+		rc = ethtool_get_dump(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2 net-next-2.6 2/2] qlcnic: Take FW dump via ethtool
From: Anirban Chakraborty @ 2011-05-10  1:02 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, davem, Anirban Chakraborty
In-Reply-To: <1304989352-24810-1-git-send-email-anirban.chakraborty@qlogic.com>

Driver checks if the previous dump has been cleared before taking the dump.
It doesn't take the dump if it is not cleared.

Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 drivers/net/qlcnic/qlcnic_ethtool.c |   60 +++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index c541461..1237449 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -965,6 +965,64 @@ static void qlcnic_set_msglevel(struct net_device *netdev, u32 msglvl)
 	adapter->msg_enable = msglvl;
 }
 
+static int
+qlcnic_get_dump(struct net_device *netdev, struct ethtool_dump *dump,
+		void *buffer)
+{
+	int i, copy_sz;
+	u32 *hdr_ptr, *data;
+	struct qlcnic_adapter *adapter = netdev_priv(netdev);
+	struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
+
+	if (dump->type == ETHTOOL_DUMP_FLAG) {
+		dump->len = fw_dump->tmpl_hdr->size + fw_dump->size;
+		dump->flag = fw_dump->tmpl_hdr->drv_cap_mask;
+		return 0;
+	}
+	if (!fw_dump->clr) {
+		netdev_info(netdev, "Dump not available\n");
+		return -EINVAL;
+	}
+	copy_sz = fw_dump->tmpl_hdr->size;
+	/* Copy template header first */
+	hdr_ptr = (u32 *) fw_dump->tmpl_hdr;
+	data = (u32 *) buffer;
+	for (i = 0; i < copy_sz/sizeof(u32); i++)
+		*data++ = cpu_to_le32(*hdr_ptr++);
+	/* Copy captured dump data */
+	memcpy(buffer + copy_sz, fw_dump->data, fw_dump->size);
+	dump->len = copy_sz + fw_dump->size;
+	dump->flag = fw_dump->tmpl_hdr->drv_cap_mask;
+	/* free dump area once the whoel dump data has been captured */
+	vfree(fw_dump->data);
+	fw_dump->size = 0;
+	fw_dump->data = NULL;
+	fw_dump->clr = 0;
+	return 0;
+}
+
+static int
+qlcnic_set_dump(struct net_device *netdev, struct ethtool_dump *val)
+{
+	struct qlcnic_adapter *adapter = netdev_priv(netdev);
+	struct qlcnic_fw_dump *fw_dump = &adapter->ahw->fw_dump;
+	if (val->flag == QLCNIC_FORCE_FW_DUMP_KEY) {
+		netdev_info(netdev, "Forcing a FW dump\n");
+		qlcnic_dev_request_reset(adapter);
+	} else {
+		if (val->flag > QLCNIC_DUMP_MASK_MAX ||
+			val->flag < QLCNIC_DUMP_MASK_MIN) {
+				netdev_info(netdev,
+				"Invalid dump level: 0x%x\n", val->flag);
+				return -EINVAL;
+		}
+		fw_dump->tmpl_hdr->drv_cap_mask = val->flag & 0xff;
+		netdev_info(netdev, "Driver mask changed to: 0x%x\n",
+			fw_dump->tmpl_hdr->drv_cap_mask);
+	}
+	return 0;
+}
+
 const struct ethtool_ops qlcnic_ethtool_ops = {
 	.get_settings = qlcnic_get_settings,
 	.set_settings = qlcnic_set_settings,
@@ -991,4 +1049,6 @@ const struct ethtool_ops qlcnic_ethtool_ops = {
 	.set_phys_id = qlcnic_set_led,
 	.set_msglevel = qlcnic_set_msglevel,
 	.get_msglevel = qlcnic_get_msglevel,
+	.get_dump = qlcnic_get_dump,
+	.set_dump = qlcnic_set_dump,
 };
-- 
1.7.4.1


^ permalink raw reply related

* Re: [v2 000/115] faster tree-based sysctl implementation
From: Lucian Adrian Grijincu @ 2011-05-10  1:06 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: linux-kernel, netdev, Alexey Dobriyan, Octavian Purdila,
	David S . Miller
In-Reply-To: <m1ei48zhyz.fsf@fess.ebiederm.org>

On Mon, May 9, 2011 at 5:11 AM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> With respect to review and merging.  The priorities should be:
> 1) Figure out what the users really need to do, before we change
>   everything so we only need one sweeping pass through the users
>   that hopefully simplifies them.


The users of the API should not rely on the .child field of 'struct
ctl_table' because we don't use that to encode the tree structure any
more.
That's absolutely the only thing that's changed in the API.

Most of the sysctl users in the tree don't need any change and will
work with the new sysctl just fine.

For the few that do, the first path of the series (the patches with
"no-child" in the name) get's rid of the .child field by:
a) replacing register_sysctl_table with register_sysctl_paths (very
straight forward change and the new code is cleaner)
b) registering files in multiple directories separately



That part of the patch series does not disturb the current
functionality and does not depend of the later patches; it uses the
current sysctl API to achieve the same goal.



> 2) Ensure you have a version of the new interfaces ready at the
>   start of the patchset, so the sweeping changes can be made
>   incrementally.
>
> 3) Clean up the users.
>
> 4) Make simplifications because you don't have any old users left.


Hmm. I'm not sure if I understand you correctly.

You want this so that the cleanup patches get in through the affected
subsystems, and not in a big series that would possibly create merge
problems with those trees?

This wouldn't be too hard to do and wouldn't uglify the new interface
too much, but I have limited time available to do this in the near
future (exams and school projects). I'll see how I can squeeze this
in.

-- 
 .
..: Lucian

^ permalink raw reply

* 2.6.39-rc6-mmotm0506 - another lockdep splat (networking this time)
From: Valdis.Kletnieks @ 2011-05-10  1:05 UTC (permalink / raw)
  To: Andrew Morton, David S. Miller; +Cc: linux-kernel, netdev

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

Seen during boot this afternoon:

[   43.309549] 
[   43.309550] ===================================================
[   43.309553] [ INFO: suspicious rcu_dereference_check() usage. ]
[   43.309555] ---------------------------------------------------
[   43.309558] include/linux/inetdevice.h:226 invoked rcu_dereference_check() without protection!
[   43.309560] 
[   43.309561] other info that might help us debug this:
[   43.309561] 
[   43.309563] 
[   43.309564] rcu_scheduler_active = 1, debug_locks = 1
[   43.309567] 2 locks held by ip/1193:
[   43.309568]  #0:  (nlk->cb_mutex){+.+...}, at: [<ffffffff81426c1d>] netlink_dump+0x45/0x1d0
[   43.309579]  #1:  (rcu_read_lock){.+.+..}, at: [<ffffffff8141b9b9>] rtnl_dump_ifinfo+0x0/0x156
[   43.309587] 
[   43.309587] stack backtrace:
[   43.309590] Pid: 1193, comm: ip Not tainted 2.6.39-rc6-mmotm0506 #1
[   43.309592] Call Trace:
[   43.309599]  [<ffffffff81066b6e>] lockdep_rcu_dereference+0x9a/0xa2
[   43.309604]  [<ffffffff81468d41>] __in_dev_get_rtnl+0x3c/0x47
[   43.309607]  [<ffffffff81468ef0>] inet_fill_link_af+0x12/0x5b
[   43.309611]  [<ffffffff8141b8fc>] rtnl_fill_ifinfo+0x665/0x710
[   43.309616]  [<ffffffff8141ba76>] rtnl_dump_ifinfo+0xbd/0x156
[   43.309620]  [<ffffffff81426c37>] netlink_dump+0x5f/0x1d0
[   43.309624]  [<ffffffff81404e3e>] ? consume_skb+0x8a/0x8f
[   43.309628]  [<ffffffff81426fc8>] netlink_recvmsg+0x1dd/0x31d
[   43.309632]  [<ffffffff813ff21f>] ? rcu_read_unlock+0x21/0x23
[   43.309636]  [<ffffffff813fab57>] sock_recvmsg+0xed/0x112
[   43.309641]  [<ffffffff810d1ebf>] ? might_fault+0x4e/0x9e
[   43.309645]  [<ffffffff81068816>] ? __lock_release+0x93/0x9c
[   43.309649]  [<ffffffff810d1ebf>] ? might_fault+0x4e/0x9e
[   43.309652]  [<ffffffff810d1f08>] ? might_fault+0x97/0x9e
[   43.309656]  [<ffffffff814065f8>] ? copy_from_user+0x3c/0x44
[   43.309660]  [<ffffffff813fc047>] __sys_recvmsg+0x16c/0x224
[   43.309665]  [<ffffffff81059240>] ? up_read+0x23/0x27
[   43.309669]  [<ffffffff81065165>] ? arch_local_irq_save+0x9/0xc
[   43.309673]  [<ffffffff810f736b>] ? fcheck_files+0xb4/0xeb
[   43.309676]  [<ffffffff810f79f3>] ? fget_light+0x35/0x96
[   43.309680]  [<ffffffff813fdded>] sys_recvmsg+0x3d/0x5b
[   43.309686]  [<ffffffff815705bb>] system_call_fastpath+0x16/0x1b
[   44.437679] e1000e 0000:00:19.0: irq 46 for MSI/MSI-X
[   44.488363] e1000e 0000:00:19.0: irq 46 for MSI/MSI-X
[   44.494680] ADDRCONF(NETDEV_UP): eth0: link is not ready



[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

^ permalink raw reply

* [PATCH net-next] vmxnet3: Use single tx queue when CONFIG_PCI_MSI not defined
From: Shreyas Bhatewara @ 2011-05-10  0:39 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: pv-drivers


Limit number of Tx queues to 1 if MSI/MSI-X support is not configured in 
the kernel. This will make number of tx and rx queues equal when MSI/X is 
not configured thus providing better performance.

Signed-off-by: Shreyas N Bhatewara <sbhatewara@vmware.com>

---

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 7a494f7..537e387 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2890,10 +2890,12 @@ vmxnet3_probe_device(struct pci_dev *pdev,
 #endif
 		num_rx_queues = 1;
 
+#ifdef CONFIG_PCI_MSI
 	if (enable_mq)
 		num_tx_queues = min(VMXNET3_DEVICE_MAX_TX_QUEUES,
 				    (int)num_online_cpus());
 	else
+#endif
 		num_tx_queues = 1;
 
 	netdev = alloc_etherdev_mq(sizeof(struct vmxnet3_adapter),
diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index 8ba7b5f..f50d36f 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -68,10 +68,10 @@
 /*
  * Version numbers
  */
-#define VMXNET3_DRIVER_VERSION_STRING   "1.0.25.0-k"
+#define VMXNET3_DRIVER_VERSION_STRING   "1.1.9.0-k"
 
 /* a 32-bit int, each byte encode a verion number in VMXNET3_DRIVER_VERSION */
-#define VMXNET3_DRIVER_VERSION_NUM      0x01001900
+#define VMXNET3_DRIVER_VERSION_NUM      0x01010900
 
 #if defined(CONFIG_PCI_MSI)
 	/* RSS only makes sense if MSI-X is supported. */

^ permalink raw reply related

* [PATCH 2/2] net/dl2k: Don't reconfigure link @100Mbps when disabling autoneg @1Gbps
From: David Decotigny @ 2011-05-10  0:19 UTC (permalink / raw)
  To: Giuseppe Cavallaro, David S. Miller, Joe Perches,
	Stanislaw Gruszka, netdev
  Cc: David Decotigny
In-Reply-To: <1304986748-15809-1-git-send-email-decot@google.com>

The initial version of the driver used to force the link to 100Mbps
when auto-negociation was disabled on a 1Gbps link, ignoring the
requested link speed. Instead, this change refuses to change anything
when it is asked to configure the link speed at 1Gbps without
auto-negociation, but acts as requested in all the other cases.

IMPORTANT: Previously, the return value from mii_set_media() was
           ignored. This patch uses it for its own return value.

Tested: module compiling, NOT tested on real hardware.
Signed-off-by: David Decotigny <decot@google.com>
---
 drivers/net/dl2k.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/dl2k.c b/drivers/net/dl2k.c
index c445457..1a4856b 100644
--- a/drivers/net/dl2k.c
+++ b/drivers/net/dl2k.c
@@ -1211,24 +1211,17 @@ static int rio_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 	if (cmd->autoneg == AUTONEG_ENABLE) {
 		if (np->an_enable)
 			return 0;
-		else {
-			np->an_enable = 1;
-			mii_set_media(dev);
-			return 0;
-		}
+
+		np->an_enable = 1;
 	} else {
-		np->an_enable = 0;
-		if (np->speed == 1000) {
-			ethtool_cmd_speed_set(cmd, SPEED_100);
-			cmd->duplex = DUPLEX_FULL;
-			printk("Warning!! Can't disable Auto negotiation in 1000Mbps, change to Manual 100Mbps, Full duplex.\n");
-		}
 		switch (ethtool_cmd_speed(cmd)) {
 		case SPEED_10:
+			np->an_enable = 0;
 			np->speed = 10;
 			np->full_duplex = (cmd->duplex == DUPLEX_FULL);
 			break;
 		case SPEED_100:
+			np->an_enable = 0;
 			np->speed = 100;
 			np->full_duplex = (cmd->duplex == DUPLEX_FULL);
 			break;
@@ -1236,9 +1229,9 @@ static int rio_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		default:
 			return -EINVAL;
 		}
-		mii_set_media(dev);
 	}
-	return 0;
+
+	return mii_set_media(dev);
 }
 
 static u32 rio_get_link(struct net_device *dev)
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 1/2] net/stmmac: don't go through ethtool to start autonegociation
From: David Decotigny @ 2011-05-10  0:19 UTC (permalink / raw)
  To: Giuseppe Cavallaro, David S. Miller, Joe Perches,
	Stanislaw Gruszka, netdev
  Cc: David Decotigny
In-Reply-To: <1304986748-15809-1-git-send-email-decot@google.com>

The driver used to call phy's ethtool configuration routine to start
autonegociation. This change has it call directly phy's routine to
start autonegociation.

IMPORTANT: initial version was hiding phy_start_aneg() return value,
           this patch returns it (<0 upon error).

Tested: module compiles, NOT tested on real hardware.
Signed-off-by: David Decotigny <decot@google.com>
---
 drivers/net/stmmac/stmmac_ethtool.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_ethtool.c b/drivers/net/stmmac/stmmac_ethtool.c
index 6f5aaeb..9c05cf0 100644
--- a/drivers/net/stmmac/stmmac_ethtool.c
+++ b/drivers/net/stmmac/stmmac_ethtool.c
@@ -236,17 +236,8 @@ stmmac_set_pauseparam(struct net_device *netdev,
 	priv->flow_ctrl = new_pause;
 
 	if (phy->autoneg) {
-		if (netif_running(netdev)) {
-			struct ethtool_cmd cmd = { .cmd = ETHTOOL_SSET };
-			/* auto-negotiation automatically restarted */
-			cmd.supported = phy->supported;
-			cmd.advertising = phy->advertising;
-			cmd.autoneg = phy->autoneg;
-			ethtool_cmd_speed_set(&cmd, phy->speed);
-			cmd.duplex = phy->duplex;
-			cmd.phy_address = phy->addr;
-			ret = phy_ethtool_sset(phy, &cmd);
-		}
+		if (netif_running(netdev))
+			ret = phy_start_aneg(phy);
 	} else
 		priv->hw->mac->flow_ctrl(priv->ioaddr, phy->duplex,
 					 priv->flow_ctrl, priv->pause);
-- 
1.7.3.1

^ permalink raw reply related

* [PATCH 0/2] Minor autonegociation changes, testers welcome
From: David Decotigny @ 2011-05-10  0:19 UTC (permalink / raw)
  To: Giuseppe Cavallaro, David S. Miller, Joe Perches,
	Stanislaw Gruszka, netdev
  Cc: David Decotigny

The following 2 patches fix a few minor issues noticed by Ben and that
I tried to address here. The code compiles and "looks" good but I
don't have the required hardware to test. If you have access to the
affected NICs, I would really appreciate your help testing the
patches:
 - stmmac: ethtool -a ethX
           ethtool -A ethX [same params except:] autoneg off
 - dl2k: ethtool -s ethX speed 10 autoneg off # should work
         likewise with 100, but not with 1000 (ethtool error)

IMHO, the main differences that could be experienced relate to the
return value of ethtool, which should now report errors more
frequently than before (most were ignored).

David Decotigny (2):
  net/stmmac: don't go through ethtool to start autonegociation
  net/dl2k: Don't reconfigure link @100Mbps when disabling autoneg
    @1Gbps

 drivers/net/dl2k.c                  |   19 ++++++-------------
 drivers/net/stmmac/stmmac_ethtool.c |   13 ++-----------
 2 files changed, 8 insertions(+), 24 deletions(-)

-- 
1.7.3.1

^ permalink raw reply

* Re: [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Joe Perches @ 2011-05-10  0:15 UTC (permalink / raw)
  To: aquini
  Cc: kernel-janitors, David Miller, Jay Vosburgh, Andy Gospodarek,
	shemminger, netdev, Nicolas Kaiser
In-Reply-To: <20110510000758.GA12728@x61.tchesoft.com>

On Mon, 2011-05-09 at 21:08 -0300, Rafael Aquini wrote:
>  drivers/net/bonding/bond_3ad.c |  886 +++++++++++++++++++++++-----------------
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> @@ -411,36 +407,37 @@ static inline void __initialize_port_locks(struct port *port)
>   */
>  static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
>  {
> -	u16 retval = 0; /* to silence the compiler */
> +	u16 uninitialized_var(retval);

uninitialized_var isn't necessary.  Every case will set retval.

>  	switch (timer_type) {
[]
> +	default:
> +		/* Exception: unexpected timer_type received */
> +		pr_debug("Unexpected AD timer_type: %d\n", timer_type);
> +		retval = 0;
> +		break;
>  	}
>  	return retval;
>  }



^ permalink raw reply

* [PATCH] net/bonding: adjust codingstyle for bond_3ad files
From: Rafael Aquini @ 2011-05-10  0:08 UTC (permalink / raw)
  To: kernel-janitors
  Cc: David Miller, Joe Perches, Jay Vosburgh, Andy Gospodarek,
	shemminger, netdev, Nicolas Kaiser

Howdy,

While I was studying what bond_3ad has under its hood, I realized its coding
style did not follow all Documentation/CodingStyle recommendations. As a tiny
collaboration I did some mods there, in an attempt to make that code stick as
closely as possible with Kernel's coding style. Also, Nicolas Kaiser has kindly
suggested some conditional simplifications integrated in this patch.
Modifications:
        * switched all comments from C99-style to C89-style;
        * replaced MAC_ADDRESS_COMPARE macro for compare_ether_addr();
	* use pr_debug to print info on unexpected status checkings;
        * simplify conditionals:
		(a || (!a && !b)) => (a || !b)
		(!(!a && b)) => (a || !b)

Signed-off-by: Rafael Aquini <aquini@linux.com>
Signed-off-by: Nicolas Kaiser <nikai@nikai.net>
---
 drivers/net/bonding/bond_3ad.c |  886 +++++++++++++++++++++++-----------------
 drivers/net/bonding/bond_3ad.h |  195 +++++-----
 2 files changed, 618 insertions(+), 463 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 31912f1..af3fc20 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -34,14 +34,14 @@
 #include "bonding.h"
 #include "bond_3ad.h"
 
-// General definitions
+/* General definitions */
 #define AD_SHORT_TIMEOUT           1
 #define AD_LONG_TIMEOUT            0
 #define AD_STANDBY                 0x2
 #define AD_MAX_TX_IN_SECOND        3
 #define AD_COLLECTOR_MAX_DELAY     0
 
-// Timer definitions(43.4.4 in the 802.3ad standard)
+/* Timer definitions (43.4.4 in the 802.3ad standard) */
 #define AD_FAST_PERIODIC_TIME      1
 #define AD_SLOW_PERIODIC_TIME      30
 #define AD_SHORT_TIMEOUT_TIME      (3*AD_FAST_PERIODIC_TIME)
@@ -49,7 +49,7 @@
 #define AD_CHURN_DETECTION_TIME    60
 #define AD_AGGREGATE_WAIT_TIME     2
 
-// Port state definitions(43.4.2.2 in the 802.3ad standard)
+/* Port state definitions (43.4.2.2 in the 802.3ad standard) */
 #define AD_STATE_LACP_ACTIVITY   0x1
 #define AD_STATE_LACP_TIMEOUT    0x2
 #define AD_STATE_AGGREGATION     0x4
@@ -59,7 +59,9 @@
 #define AD_STATE_DEFAULTED       0x40
 #define AD_STATE_EXPIRED         0x80
 
-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
+/* Port Variables definitions used by the State Machines
+ * (43.4.7 in the 802.3ad standard)
+ */
 #define AD_PORT_BEGIN           0x1
 #define AD_PORT_LACP_ENABLED    0x2
 #define AD_PORT_ACTOR_CHURN     0x4
@@ -71,27 +73,23 @@
 #define AD_PORT_SELECTED        0x100
 #define AD_PORT_MOVED           0x200
 
-// Port Key definitions
-// key is determined according to the link speed, duplex and
-// user key(which is yet not supported)
-//              ------------------------------------------------------------
-// Port key :   | User key                       |      Speed       |Duplex|
-//              ------------------------------------------------------------
-//              16                               6               1 0
+/* Port Key definitions:
+ *  key is determined according to the link speed, duplex and
+ *  user key (which is yet not supported)
+ *             ------------------------------------------------------------
+ *  Port key:  | User key                       |      Speed       |Duplex|
+ *             ------------------------------------------------------------
+ *             16                               6                  1      0
+ */
 #define  AD_DUPLEX_KEY_BITS    0x1
 #define  AD_SPEED_KEY_BITS     0x3E
 #define  AD_USER_KEY_BITS      0xFFC0
 
-//dalloun
 #define     AD_LINK_SPEED_BITMASK_1MBPS       0x1
 #define     AD_LINK_SPEED_BITMASK_10MBPS      0x2
 #define     AD_LINK_SPEED_BITMASK_100MBPS     0x4
 #define     AD_LINK_SPEED_BITMASK_1000MBPS    0x8
 #define     AD_LINK_SPEED_BITMASK_10000MBPS   0x10
-//endalloun
-
-// compare MAC addresses
-#define MAC_ADDRESS_COMPARE(A, B) memcmp(A, B, ETH_ALEN)
 
 static struct mac_addr null_mac_addr = { { 0, 0, 0, 0, 0, 0 } };
 static u16 ad_ticks_per_sec;
@@ -99,7 +97,7 @@ static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
 static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
 
-// ================= main 802.3ad protocol functions ==================
+/* ================= main 802.3ad protocol functions ================== */
 static int ad_lacpdu_send(struct port *port);
 static int ad_marker_send(struct port *port, struct bond_marker *marker);
 static void ad_mux_machine(struct port *port);
@@ -113,14 +111,12 @@ static void ad_initialize_agg(struct aggregator *aggregator);
 static void ad_initialize_port(struct port *port, int lacp_fast);
 static void ad_enable_collecting_distributing(struct port *port);
 static void ad_disable_collecting_distributing(struct port *port);
-static void ad_marker_info_received(struct bond_marker *marker_info, struct port *port);
-static void ad_marker_response_received(struct bond_marker *marker, struct port *port);
-
-
-/////////////////////////////////////////////////////////////////////////////////
-// ================= api to bonding and kernel code ==================
-/////////////////////////////////////////////////////////////////////////////////
+static void ad_marker_info_received(struct bond_marker *marker_info,
+				    struct port *port);
+static void ad_marker_response_received(struct bond_marker *marker,
+					struct port *port);
 
+/* ================= api to bonding and kernel code ================== */
 /**
  * __get_bond_by_port - get the port's bonding struct
  * @port: the port we're looking at
@@ -161,7 +157,7 @@ static inline struct port *__get_next_port(struct port *port)
 	struct bonding *bond = __get_bond_by_port(port);
 	struct slave *slave = port->slave;
 
-	// If there's no bond for this port, or this is the last slave
+	/* If there's no bond for this port, or this is the last slave */
 	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
 
@@ -179,7 +175,7 @@ static inline struct aggregator *__get_first_agg(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
 
-	// If there's no bond for this port, or bond has no slaves
+	/* If there's no bond for this port, or bond has no slaves */
 	if ((bond == NULL) || (bond->slave_cnt == 0))
 		return NULL;
 
@@ -198,7 +194,7 @@ static inline struct aggregator *__get_next_agg(struct aggregator *aggregator)
 	struct slave *slave = aggregator->slave;
 	struct bonding *bond = bond_get_bond_by_slave(slave);
 
-	// If there's no bond for this aggregator, or this is the last slave
+	/* If there's no bond for this aggregator, or this is the last slave */
 	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
 
@@ -316,10 +312,9 @@ static u16 __get_link_speed(struct port *port)
 	struct slave *slave = port->slave;
 	u16 speed;
 
-	/* this if covers only a special case: when the configuration starts with
-	 * link down, it sets the speed to 0.
-	 * This is done in spite of the fact that the e100 driver reports 0 to be
-	 * compatible with MVT in the future.*/
+	/* handling a special case:
+	 * when the configuration starts with link down, it sets the speed to 0
+	 */
 	if (slave->link != BOND_LINK_UP)
 		speed = 0;
 	else {
@@ -341,7 +336,9 @@ static u16 __get_link_speed(struct port *port)
 			break;
 
 		default:
-			speed = 0; // unknown speed value from ethtool. shouldn't happen
+			/* unknown speed value from ethtool. shouldn't happen */
+			pr_debug("Unexpected slave->speed: %d\n", slave->speed);
+			speed = 0;
 			break;
 		}
 	}
@@ -362,13 +359,13 @@ static u16 __get_link_speed(struct port *port)
 static u8 __get_duplex(struct port *port)
 {
 	struct slave *slave = port->slave;
+	u8 retval = 0x0;
 
-	u8 retval;
-
-	//  handling a special case: when the configuration starts with
-	// link down, it sets the duplex to 0.
+	/* handling a special case:
+	 * when the configuration starts with link down, it sets the duplex to 0
+	 */
 	if (slave->link != BOND_LINK_UP)
-		retval = 0x0;
+		goto out;
 	else {
 		switch (slave->duplex) {
 		case DUPLEX_FULL:
@@ -384,6 +381,7 @@ static u8 __get_duplex(struct port *port)
 			break;
 		}
 	}
+out:
 	return retval;
 }
 
@@ -394,12 +392,10 @@ static u8 __get_duplex(struct port *port)
  */
 static inline void __initialize_port_locks(struct port *port)
 {
-	// make sure it isn't called twice
+	/* make sure it isn't called twice */
 	spin_lock_init(&(SLAVE_AD_INFO(port->slave).state_machine_lock));
 }
 
-//conversions
-
 /**
  * __ad_timer_to_ticks - convert a given timer type to AD module ticks
  * @timer_type:	which timer to operate
@@ -411,36 +407,37 @@ static inline void __initialize_port_locks(struct port *port)
  */
 static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 {
-	u16 retval = 0; /* to silence the compiler */
+	u16 uninitialized_var(retval);
 
 	switch (timer_type) {
-	case AD_CURRENT_WHILE_TIMER:   // for rx machine usage
+	case AD_CURRENT_WHILE_TIMER:	/* for rx machine usage */
 		if (par)
-			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec); // short timeout
+			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec);
 		else
-			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec); // long timeout
+			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec);
 		break;
-	case AD_ACTOR_CHURN_TIMER:	    // for local churn machine
+	case AD_ACTOR_CHURN_TIMER:	/* for local churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_PERIODIC_TIMER:	    // for periodic machine
-		retval = (par*ad_ticks_per_sec); // long timeout
+	case AD_PERIODIC_TIMER:		/* for periodic machine */
+		retval = (par*ad_ticks_per_sec);
 		break;
-	case AD_PARTNER_CHURN_TIMER:   // for remote churn machine
+	case AD_PARTNER_CHURN_TIMER:	/* for remote churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_WAIT_WHILE_TIMER:	    // for selection machine
+	case AD_WAIT_WHILE_TIMER:	/* for selection machine */
 		retval = (AD_AGGREGATE_WAIT_TIME*ad_ticks_per_sec);
 		break;
+	default:
+		/* Exception: unexpected timer_type received */
+		pr_debug("Unexpected AD timer_type: %d\n", timer_type);
+		retval = 0;
+		break;
 	}
 	return retval;
 }
 
-
-/////////////////////////////////////////////////////////////////////////////////
-// ================= ad_rx_machine helper functions ==================
-/////////////////////////////////////////////////////////////////////////////////
-
+/* ================= ad_rx_machine helper functions ================== */
 /**
  * __choose_matched - update a port's matched variable from a received lacpdu
  * @lacpdu: the lacpdu we've received
@@ -466,17 +463,17 @@ static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
  */
 static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
 {
-	// check if all parameters are alike
+	/* check if all parameters are alike */
 	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
 	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
-	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
+	     !compare_ether_addr((const u8 *)&(lacpdu->partner_system), (const u8 *)&(port->actor_system)) &&
 	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
 	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
 	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
-	    // or this is individual link(aggregation == FALSE)
+	    /* or this is individual link(aggregation == FALSE) */
 	    ((lacpdu->actor_state & AD_STATE_AGGREGATION) == 0)
 		) {
-		// update the state machine Matched variable
+		/* update the state machine Matched variable */
 		port->sm_vars |= AD_PORT_MATCHED;
 	} else {
 		port->sm_vars &= ~AD_PORT_MATCHED;
@@ -498,7 +495,7 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		struct port_params *partner = &port->partner_oper;
 
 		__choose_matched(lacpdu, port);
-		// record the new parameter values for the partner operational
+		/* record the new values for the operational partner */
 		partner->port_number = ntohs(lacpdu->actor_port);
 		partner->port_priority = ntohs(lacpdu->actor_port_priority);
 		partner->system = lacpdu->actor_system;
@@ -506,12 +503,14 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		partner->key = ntohs(lacpdu->actor_key);
 		partner->port_state = lacpdu->actor_state;
 
-		// set actor_oper_port_state.defaulted to FALSE
+		/* set actor_oper_port_state.defaulted to FALSE */
 		port->actor_oper_port_state &= ~AD_STATE_DEFAULTED;
 
-		// set the partner sync. to on if the partner is sync. and the port is matched
-		if ((port->sm_vars & AD_PORT_MATCHED)
-		    && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
+		/* set the partner sync to on if the partner is sync
+		 * and the port is matched.
+		 */
+		if ((port->sm_vars & AD_PORT_MATCHED) &&
+		    (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
 		else
 			partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
@@ -529,11 +528,8 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 static void __record_default(struct port *port)
 {
 	if (port) {
-		// record the partner admin parameters
 		memcpy(&port->partner_oper, &port->partner_admin,
 		       sizeof(struct port_params));
-
-		// set actor_oper_port_state.defaulted to true
 		port->actor_oper_port_state |= AD_STATE_DEFAULTED;
 	}
 }
@@ -556,14 +552,14 @@ static void __update_selected(struct lacpdu *lacpdu, struct port *port)
 	if (lacpdu && port) {
 		const struct port_params *partner = &port->partner_oper;
 
-		// check if any parameter is different
+		/* check if any parameter is different */
 		if (ntohs(lacpdu->actor_port) != partner->port_number ||
 		    ntohs(lacpdu->actor_port_priority) != partner->port_priority ||
-		    MAC_ADDRESS_COMPARE(&lacpdu->actor_system, &partner->system) ||
+		    compare_ether_addr((const u8 *)&lacpdu->actor_system, (const u8 *)&partner->system) ||
 		    ntohs(lacpdu->actor_system_priority) != partner->system_priority ||
 		    ntohs(lacpdu->actor_key) != partner->key ||
 		    (lacpdu->actor_state & AD_STATE_AGGREGATION) != (partner->port_state & AD_STATE_AGGREGATION)) {
-			// update the state machine Selected variable
+			/* update the state machine Selected variable */
 			port->sm_vars &= ~AD_PORT_SELECTED;
 		}
 	}
@@ -587,15 +583,15 @@ static void __update_default_selected(struct port *port)
 		const struct port_params *admin = &port->partner_admin;
 		const struct port_params *oper = &port->partner_oper;
 
-		// check if any parameter is different
 		if (admin->port_number != oper->port_number ||
 		    admin->port_priority != oper->port_priority ||
-		    MAC_ADDRESS_COMPARE(&admin->system, &oper->system) ||
+		    compare_ether_addr((const u8 *)&admin->system,
+					 (const u8 *)&oper->system) ||
 		    admin->system_priority != oper->system_priority ||
 		    admin->key != oper->key ||
 		    (admin->port_state & AD_STATE_AGGREGATION)
 			!= (oper->port_state & AD_STATE_AGGREGATION)) {
-			// update the state machine Selected variable
+			/* update the state machine Selected variable */
 			port->sm_vars &= ~AD_PORT_SELECTED;
 		}
 	}
@@ -615,12 +611,12 @@ static void __update_default_selected(struct port *port)
  */
 static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 {
-	// validate lacpdu and port
+	/* validate lacpdu and port */
 	if (lacpdu && port) {
-		// check if any parameter is different
+		/* check if any parameter is different */
 		if ((ntohs(lacpdu->partner_port) != port->actor_port_number) ||
 		    (ntohs(lacpdu->partner_port_priority) != port->actor_port_priority) ||
-		    MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) ||
+		    compare_ether_addr((const u8 *)&(lacpdu->partner_system), (const u8 *)&(port->actor_system)) ||
 		    (ntohs(lacpdu->partner_system_priority) != port->actor_system_priority) ||
 		    (ntohs(lacpdu->partner_key) != port->actor_oper_port_key) ||
 		    ((lacpdu->partner_state & AD_STATE_LACP_ACTIVITY) != (port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY)) ||
@@ -628,7 +624,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 		    ((lacpdu->partner_state & AD_STATE_SYNCHRONIZATION) != (port->actor_oper_port_state & AD_STATE_SYNCHRONIZATION)) ||
 		    ((lacpdu->partner_state & AD_STATE_AGGREGATION) != (port->actor_oper_port_state & AD_STATE_AGGREGATION))
 		   ) {
-
+			/* set port ntt */
 			port->ntt = true;
 		}
 	}
@@ -644,9 +640,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
  */
 static void __attach_bond_to_agg(struct port *port)
 {
-	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing since the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	port = NULL;
 }
 
 /**
@@ -659,9 +653,7 @@ static void __attach_bond_to_agg(struct port *port)
  */
 static void __detach_bond_from_agg(struct port *port)
 {
-	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing sience the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	port = NULL;
 }
 
 /**
@@ -675,7 +667,9 @@ static int __agg_ports_are_ready(struct aggregator *aggregator)
 	int retval = 1;
 
 	if (aggregator) {
-		// scan all ports in this aggregator to verfy if they are all ready
+		/* scan all ports in this aggregator
+		 * to verify if they are all ready.
+		 */
 		for (port = aggregator->lag_ports;
 		     port;
 		     port = port->next_port_in_aggregator) {
@@ -715,8 +709,8 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
  */
 static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 {
-	u32 bandwidth = 0;
-	u32 basic_speed;
+	u32 uninitialized_var(bandwidth);
+	u32 uninitialized_var(basic_speed);
 
 	if (aggregator->num_of_ports) {
 		basic_speed = __get_link_speed(aggregator->lag_ports);
@@ -737,7 +731,11 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 			bandwidth = aggregator->num_of_ports * 10000;
 			break;
 		default:
-			bandwidth = 0; /*to silence the compiler ....*/
+			/* Exception: unexpected basic_speed returned */
+			pr_debug("Unexpected aggregator basic_speed: %d\n",
+				 basic_speed);
+			bandwidth = 0;
+			break;
 		}
 	}
 	return bandwidth;
@@ -809,10 +807,7 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	 */
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= main 802.3ad protocol code ======================================
-//////////////////////////////////////////////////////////////////////////////////////
-
+/* ================= main 802.3ad protocol code ================= */
 /**
  * ad_lacpdu_send - send out a lacpdu packet on a given port
  * @port: the port we're looking at
@@ -841,11 +836,12 @@ static int ad_lacpdu_send(struct port *port)
 
 	memcpy(lacpdu_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
 	/* Note: source address is set to be the member's PERMANENT address,
-	   because we use it to identify loopback lacpdus in receive. */
+	 * because we use it to identify loopback lacpdus in receive.
+	 */
 	memcpy(lacpdu_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	lacpdu_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	lacpdu_header->lacpdu = port->lacpdu; // struct copy
+	lacpdu_header->lacpdu = port->lacpdu;
 
 	dev_queue_xmit(skb);
 
@@ -886,7 +882,7 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 	memcpy(marker_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	marker_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	marker_header->marker = *marker; // struct copy
+	marker_header->marker = *marker;
 
 	dev_queue_xmit(skb);
 
@@ -902,80 +898,104 @@ static void ad_mux_machine(struct port *port)
 {
 	mux_states_t last_state;
 
-	// keep current State Machine state to compare later if it was changed
 	last_state = port->sm_mux_state;
 
 	if (port->sm_vars & AD_PORT_BEGIN) {
-		port->sm_mux_state = AD_MUX_DETACHED;		 // next state
+		port->sm_mux_state = AD_MUX_DETACHED;
 	} else {
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
-			if ((port->sm_vars & AD_PORT_SELECTED)
-			    || (port->sm_vars & AD_PORT_STANDBY))
+			if ((port->sm_vars & AD_PORT_SELECTED) ||
+			    (port->sm_vars & AD_PORT_STANDBY))
 				/* if SELECTED or STANDBY */
-				port->sm_mux_state = AD_MUX_WAITING; // next state
+				port->sm_mux_state = AD_MUX_WAITING;
 			break;
 		case AD_MUX_WAITING:
-			// if SELECTED == FALSE return to DETACH state
-			if (!(port->sm_vars & AD_PORT_SELECTED)) { // if UNSELECTED
+			/* if SELECTED == FALSE return to DETACH state */
+			if (!(port->sm_vars & AD_PORT_SELECTED)) {
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the Selection Logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
-				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;	 // next state
+				/* in order to withhold the Selection Logic
+				 * to check all ports READY_N value at every
+				 * callback cycle to update ready variable,
+				 * we check READY_N and update READY here
+				 */
+				__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+				port->sm_mux_state = AD_MUX_DETACHED;
 				break;
 			}
 
-			// check if the wait_while_timer expired
-			if (port->sm_mux_timer_counter
-			    && !(--port->sm_mux_timer_counter))
+			/* check if the wait_while_timer expired */
+			if (port->sm_mux_timer_counter &&
+			    !(--port->sm_mux_timer_counter))
 				port->sm_vars |= AD_PORT_READY_N;
 
-			// in order to withhold the selection logic to check all ports READY_N value
-			// every callback cycle to update ready variable, we check READY_N and update READY here
-			__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-
-			// if the wait_while_timer expired, and the port is in READY state, move to ATTACHED state
-			if ((port->sm_vars & AD_PORT_READY)
-			    && !port->sm_mux_timer_counter)
-				port->sm_mux_state = AD_MUX_ATTACHED;	 // next state
+			/* in order to withhold the selection logic
+			 * to check all ports READY_N value at every
+			 * callback cycle to update ready variable,
+			 * we check READY_N and update READY here
+			 */
+			__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+
+			/* if the wait_while_timer expired,  and the port
+			 * is in READY state, move to ATTACHED state
+			 */
+			if ((port->sm_vars & AD_PORT_READY) &&
+			    !port->sm_mux_timer_counter)
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			break;
 		case AD_MUX_ATTACHED:
-			// check also if agg_select_timer expired(so the edable port will take place only after this timer)
-			if ((port->sm_vars & AD_PORT_SELECTED) && (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) && !__check_agg_selection_timer(port)) {
-				port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;// next state
-			} else if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY)) {	  // if UNSELECTED or STANDBY
+			/* check also if agg_select_timer expired
+			 * so the enable port will take place
+			 * only after this timer
+			 */
+			if ((port->sm_vars & AD_PORT_SELECTED) &&
+			    (port->partner_oper.port_state &
+			     AD_STATE_SYNCHRONIZATION) &&
+			    !__check_agg_selection_timer(port)) {
+				port->sm_mux_state =
+					AD_MUX_COLLECTING_DISTRIBUTING;
+			} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
+				   (port->sm_vars & AD_PORT_STANDBY)) {
+				/* if UNSELECTED or STANDBY */
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the selection logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
-				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;// next state
+				/* in order to withhold the Selection Logic
+				 * to check all ports READY_N value at every
+				 * callback cycle to update ready variable,
+				 * we check READY_N and update READY here
+				 */
+				__set_agg_ports_ready(port->aggregator,
+				       __agg_ports_are_ready(port->aggregator));
+				port->sm_mux_state = AD_MUX_DETACHED;
 			}
 			break;
 		case AD_MUX_COLLECTING_DISTRIBUTING:
-			if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY) ||
-			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)
-			   ) {
-				port->sm_mux_state = AD_MUX_ATTACHED;// next state
-
+			if (!(port->sm_vars & AD_PORT_SELECTED) ||
+			    (port->sm_vars & AD_PORT_STANDBY) ||
+			    !(port->partner_oper.port_state &
+			     AD_STATE_SYNCHRONIZATION)) {
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			} else {
-				// if port state hasn't changed make
-				// sure that a collecting distributing
-				// port in an active aggregator is enabled
+				/* if port state hasn't changed make
+				 * sure that a collecting distributing
+				 * port in an active aggregator is enabled
+				 */
 				if (port->aggregator &&
 				    port->aggregator->is_active &&
-				    !__port_is_enabled(port)) {
-
+				    !__port_is_enabled(port))
 					__enable_port(port);
-				}
 			}
 			break;
-		default:    //to silence the compiler
+		default:
+			/* Exception: unexpected port->sm_mux_state */
+			pr_debug("Unexpected port->sm_mux_state: %d\n",
+				 port->sm_mux_state);
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_mux_state != last_state) {
 		pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -983,14 +1003,16 @@ static void ad_mux_machine(struct port *port)
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			__detach_bond_from_agg(port);
-			port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
+			port->actor_oper_port_state &=
+					~AD_STATE_SYNCHRONIZATION;
 			ad_disable_collecting_distributing(port);
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
 			port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
 			port->ntt = true;
 			break;
 		case AD_MUX_WAITING:
-			port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
+			port->sm_mux_timer_counter =
+				__ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
 			break;
 		case AD_MUX_ATTACHED:
 			__attach_bond_to_agg(port);
@@ -1006,7 +1028,10 @@ static void ad_mux_machine(struct port *port)
 			ad_enable_collecting_distributing(port);
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
+			/* Exception: unexpected port->sm_mux_state */
+			pr_debug("Unexpected port->sm_mux_state: %d\n",
+				 port->sm_mux_state);
 			break;
 		}
 	}
@@ -1023,61 +1048,67 @@ static void ad_mux_machine(struct port *port)
  */
 static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 {
-	rx_states_t last_state;
-
-	// keep current State Machine state to compare later if it was changed
-	last_state = port->sm_rx_state;
+	rx_states_t last_state = port->sm_rx_state;
 
-	// check if state machine should change state
-	// first, check if port was reinitialized
+	/* check if state machine should change state
+	 * first, check if port was reinitialized
+	 */
 	if (port->sm_vars & AD_PORT_BEGIN)
 		/* next state */
 		port->sm_rx_state = AD_RX_INITIALIZE;
-	// check if port is not enabled
-	else if (!(port->sm_vars & AD_PORT_BEGIN)
-		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
+	/* check if port is not enabled */
+	else if (!(port->sm_vars & AD_PORT_BEGIN) &&
+		 !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
 		/* next state */
 		port->sm_rx_state = AD_RX_PORT_DISABLED;
-	// check if new lacpdu arrived
-	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) || (port->sm_rx_state == AD_RX_DEFAULTED) || (port->sm_rx_state == AD_RX_CURRENT))) {
-		port->sm_rx_timer_counter = 0; // zero timer
+	/* check if new lacpdu arrived */
+	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) ||
+		 (port->sm_rx_state == AD_RX_DEFAULTED) ||
+		 (port->sm_rx_state == AD_RX_CURRENT))) {
+		port->sm_rx_timer_counter = 0; /* zero timer */
 		port->sm_rx_state = AD_RX_CURRENT;
 	} else {
-		// if timer is on, and if it is expired
-		if (port->sm_rx_timer_counter && !(--port->sm_rx_timer_counter)) {
+		/* if timer is on, and if it is expired */
+		if (port->sm_rx_timer_counter &&
+		    !(--port->sm_rx_timer_counter)) {
 			switch (port->sm_rx_state) {
 			case AD_RX_EXPIRED:
-				port->sm_rx_state = AD_RX_DEFAULTED;		// next state
+				port->sm_rx_state = AD_RX_DEFAULTED;
 				break;
 			case AD_RX_CURRENT:
-				port->sm_rx_state = AD_RX_EXPIRED;	    // next state
+				port->sm_rx_state = AD_RX_EXPIRED;
 				break;
-			default:    //to silence the compiler
+			default:
+				/* Exception: unexpected port->sm_rx_state */
+				pr_debug("Unexpected port->sm_rx_state: %d\n",
+					 port->sm_rx_state);
 				break;
 			}
 		} else {
-			// if no lacpdu arrived and no timer is on
+			/* if no lacpdu arrived and no timer is on */
 			switch (port->sm_rx_state) {
 			case AD_RX_PORT_DISABLED:
 				if (port->sm_vars & AD_PORT_MOVED)
-					port->sm_rx_state = AD_RX_INITIALIZE;	    // next state
-				else if (port->is_enabled
-					 && (port->sm_vars
-					     & AD_PORT_LACP_ENABLED))
-					port->sm_rx_state = AD_RX_EXPIRED;	// next state
-				else if (port->is_enabled
-					 && ((port->sm_vars
-					      & AD_PORT_LACP_ENABLED) == 0))
-					port->sm_rx_state = AD_RX_LACP_DISABLED;    // next state
+					port->sm_rx_state = AD_RX_INITIALIZE;
+				else if (port->is_enabled &&
+					 (port->sm_vars &
+					     AD_PORT_LACP_ENABLED))
+					port->sm_rx_state = AD_RX_EXPIRED;
+				else if (port->is_enabled &&
+					 ((port->sm_vars &
+					      AD_PORT_LACP_ENABLED) == 0))
+					port->sm_rx_state = AD_RX_LACP_DISABLED;
 				break;
-			default:    //to silence the compiler
+			default:
+				/* Exception: unexpected port->sm_rx_state */
+				pr_debug("Unexpected port->sm_rx_state: %d\n",
+					 port->sm_rx_state);
 				break;
-
 			}
 		}
 	}
 
-	// check if the State machine was changed or new lacpdu arrived
+	/* check if the State machine was changed or new lacpdu arrived */
 	if ((port->sm_rx_state != last_state) || (lacpdu)) {
 		pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -1092,7 +1123,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			__record_default(port);
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			port->sm_vars &= ~AD_PORT_MOVED;
-			port->sm_rx_state = AD_RX_PORT_DISABLED;	// next state
+			port->sm_rx_state = AD_RX_PORT_DISABLED;
 
 			/*- Fall Through -*/
 
@@ -1107,14 +1138,20 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
 		case AD_RX_EXPIRED:
-			//Reset of the Synchronization flag. (Standard 43.4.12)
-			//This reset cause to disable this port in the COLLECTING_DISTRIBUTING state of the
-			//mux machine in case of EXPIRED even if LINK_DOWN didn't arrive for the port.
-			port->partner_oper.port_state &= ~AD_STATE_SYNCHRONIZATION;
+			/* Reset of the Synchronization flag. (Standard 43.4.12)
+			 * This reset cause to disable this port in the
+			 * COLLECTING_DISTRIBUTING state of the mux machine
+			 * in case of EXPIRED even if LINK_DOWN didn't arrive
+			 * for the port.
+			 */
+			port->partner_oper.port_state &=
+						~AD_STATE_SYNCHRONIZATION;
 			port->sm_vars &= ~AD_PORT_MATCHED;
 			port->partner_oper.port_state |=
 				AD_STATE_LACP_ACTIVITY;
-			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(AD_SHORT_TIMEOUT));
+			port->sm_rx_timer_counter =
+				__ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER,
+						(u16)(AD_SHORT_TIMEOUT));
 			port->actor_oper_port_state |= AD_STATE_EXPIRED;
 			break;
 		case AD_RX_DEFAULTED:
@@ -1124,12 +1161,13 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
 		case AD_RX_CURRENT:
-			// detect loopback situation
-			if (!MAC_ADDRESS_COMPARE(&(lacpdu->actor_system), &(port->actor_system))) {
-				// INFO_RECEIVED_LOOPBACK_FRAMES
+			/* detect loopback situation */
+			if (!compare_ether_addr((const u8 *)&(lacpdu->actor_system), (const u8 *)&(port->actor_system))) {
+				/* INFO_RECEIVED_LOOPBACK_FRAMES */
 				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
 				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->dev->master->name, port->slave->dev->name);
+				       port->slave->dev->master->name,
+				       port->slave->dev->name);
 				return;
 			}
 			__update_selected(lacpdu, port);
@@ -1137,15 +1175,22 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			__record_pdu(lacpdu, port);
 			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT));
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
-			// verify that if the aggregator is enabled, the port is enabled too.
-			//(because if the link goes down for a short time, the 802.3ad will not
-			// catch it, and the port will continue to be disabled)
+
+			/* verify that if the aggregator is enabled,
+			 * so the port is enabled too.
+			 * because if the link goes down for a short time,
+			 * the 802.3ad will not catch it,
+			 * and the port will continue to be disabled
+			 */
 			if (port->aggregator
 			    && port->aggregator->is_active
 			    && !__port_is_enabled(port))
 				__enable_port(port);
 			break;
-		default:    //to silence the compiler
+		default:
+			/* Exception: unexpected port->sm_rx_state */
+			pr_debug("Unexpected port->sm_rx_state: %d\n",
+				 port->sm_rx_state);
 			break;
 		}
 	}
@@ -1158,9 +1203,11 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
  */
 static void ad_tx_machine(struct port *port)
 {
-	// check if tx timer expired, to verify that we do not send more than 3 packets per second
+	/* check if tx timer has expired,
+	 * to verify that we do not send more than 3 packets per second
+	 */
 	if (port->sm_tx_timer_counter && !(--port->sm_tx_timer_counter)) {
-		// check if there is something to send
+		/* check if there is something to send */
 		if (port->ntt && (port->sm_vars & AD_PORT_LACP_ENABLED)) {
 			__update_lacpdu_from_port(port);
 
@@ -1168,14 +1215,17 @@ static void ad_tx_machine(struct port *port)
 				pr_debug("Sent LACPDU on port %d\n",
 					 port->actor_port_number);
 
-				/* mark ntt as false, so it will not be sent again until
-				   demanded */
+				/* mark ntt as false,
+				 * so it won't be sent again until demanded
+				 */
 				port->ntt = false;
 			}
 		}
-		// restart tx timer(to verify that we will not exceed AD_MAX_TX_IN_SECOND
+		/* restart tx timer
+		 * to verify that we won't exceed AD_MAX_TX_IN_SECOND
+		 */
 		port->sm_tx_timer_counter =
-			ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
+				ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 	}
 }
 
@@ -1189,76 +1239,96 @@ static void ad_periodic_machine(struct port *port)
 {
 	periodic_states_t last_state;
 
-	// keep current state machine state to compare later if it was changed
+	/* keep current state machine state to compare later if it was changed*/
 	last_state = port->sm_periodic_state;
 
-	// check if port was reinitialized
-	if (((port->sm_vars & AD_PORT_BEGIN) || !(port->sm_vars & AD_PORT_LACP_ENABLED) || !port->is_enabled) ||
-	    (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))
-	   ) {
-		port->sm_periodic_state = AD_NO_PERIODIC;	     // next state
+	/* check if port was reinitialized */
+	if (((port->sm_vars & AD_PORT_BEGIN) ||
+	    !(port->sm_vars & AD_PORT_LACP_ENABLED) ||
+	    !port->is_enabled) ||
+	    (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) &&
+	     !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))) {
+		port->sm_periodic_state = AD_NO_PERIODIC; /* next state */
 	}
-	// check if state machine should change state
+	/* check if state machine should change state */
 	else if (port->sm_periodic_timer_counter) {
-		// check if periodic state machine expired
+		/* check if periodic state machine expired */
 		if (!(--port->sm_periodic_timer_counter)) {
-			// if expired then do tx
-			port->sm_periodic_state = AD_PERIODIC_TX;    // next state
+			/* if expired then do tx, next state */
+			port->sm_periodic_state = AD_PERIODIC_TX;
 		} else {
-			// If not expired, check if there is some new timeout parameter from the partner state
+			/* If not expired, check if there is some
+			 * new timeout parameter from the partner state
+			 */
 			switch (port->sm_periodic_state) {
 			case AD_FAST_PERIODIC:
 				if (!(port->partner_oper.port_state
 				      & AD_STATE_LACP_TIMEOUT))
-					port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+					port->sm_periodic_state = AD_SLOW_PERIODIC;
 				break;
 			case AD_SLOW_PERIODIC:
-				if ((port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
-					// stop current timer
+				if ((port->partner_oper.port_state &
+				     AD_STATE_LACP_TIMEOUT)) {
+					/* stop current timer */
 					port->sm_periodic_timer_counter = 0;
-					port->sm_periodic_state = AD_PERIODIC_TX;	 // next state
+					port->sm_periodic_state = AD_PERIODIC_TX;
 				}
 				break;
-			default:    //to silence the compiler
+			default:
+				/* unexpected port->sm_sm_periodic_state */
+				pr_debug("Unexpected port->sm_periodic_state: %d\n",
+					 port->sm_periodic_state);
 				break;
 			}
 		}
 	} else {
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_state = AD_FAST_PERIODIC;	 // next state
+			port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
 		case AD_PERIODIC_TX:
-			if (!(port->partner_oper.port_state
-			      & AD_STATE_LACP_TIMEOUT))
-				port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+			if (!(port->partner_oper.port_state &
+			      AD_STATE_LACP_TIMEOUT))
+				port->sm_periodic_state = AD_SLOW_PERIODIC;
 			else
-				port->sm_periodic_state = AD_FAST_PERIODIC;  // next state
+				port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
-		default:    //to silence the compiler
+		default:
+			/* unexpected port->sm_sm_periodic_state */
+			pr_debug("Unexpected port->sm_periodic_state: %d\n",
+				 port->sm_periodic_state);
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_periodic_state != last_state) {
 		pr_debug("Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
 			 port->sm_periodic_state);
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_timer_counter = 0;	   // zero timer
+			port->sm_periodic_timer_counter = 0;
 			break;
 		case AD_FAST_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_FAST_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter =
+					__ad_timer_to_ticks(AD_PERIODIC_TIMER,
+						(u16)(AD_FAST_PERIODIC_TIME))-1;
 			break;
 		case AD_SLOW_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter =
+					__ad_timer_to_ticks(AD_PERIODIC_TIMER,
+						(u16)(AD_SLOW_PERIODIC_TIME))-1;
 			break;
 		case AD_PERIODIC_TX:
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
+			/* unexpected port->sm_sm_periodic_state */
+			pr_debug("Unexpected port->sm_periodic_state: %d\n",
+				 port->sm_periodic_state);
 			break;
 		}
 	}
@@ -1274,46 +1344,58 @@ static void ad_periodic_machine(struct port *port)
  */
 static void ad_port_selection_logic(struct port *port)
 {
-	struct aggregator *aggregator, *free_aggregator = NULL, *temp_aggregator;
+	struct aggregator *aggregator, *temp_aggregator;
+	struct aggregator *free_aggregator = NULL;
 	struct port *last_port = NULL, *curr_port;
 	int found = 0;
 
-	// if the port is already Selected, do nothing
+	/* if the port is already Selected, do nothing */
 	if (port->sm_vars & AD_PORT_SELECTED)
 		return;
 
-	// if the port is connected to other aggregator, detach it
+	/* if the port is connected to other aggregator, detach it */
 	if (port->aggregator) {
-		// detach the port from its former aggregator
+		/* detach the port from its former aggregator */
 		temp_aggregator = port->aggregator;
 		for (curr_port = temp_aggregator->lag_ports; curr_port;
 		     last_port = curr_port,
 			     curr_port = curr_port->next_port_in_aggregator) {
 			if (curr_port == port) {
 				temp_aggregator->num_of_ports--;
-				if (!last_port) {// if it is the first port attached to the aggregator
+				if (!last_port) {
+					/* if it is the first port attached
+					   to the aggregator */
 					temp_aggregator->lag_ports =
 						port->next_port_in_aggregator;
-				} else {// not the first port attached to the aggregator
+				} else {
+					/* not the first port attached
+					   to the aggregator */
 					last_port->next_port_in_aggregator =
 						port->next_port_in_aggregator;
 				}
 
-				// clear the port's relations to this aggregator
+				/* clear the port's relations
+				   to this aggregator */
 				port->aggregator = NULL;
 				port->next_port_in_aggregator = NULL;
 				port->actor_port_aggregator_identifier = 0;
 
 				pr_debug("Port %d left LAG %d\n",
-					 port->actor_port_number,
-					 temp_aggregator->aggregator_identifier);
-				// if the aggregator is empty, clear its parameters, and set it ready to be attached
+					port->actor_port_number,
+					temp_aggregator->aggregator_identifier);
+				/* if the aggregator is empty,
+				 * clear its parameters, and set it ready
+				 * to be attached
+				 */
 				if (!temp_aggregator->lag_ports)
 					ad_clear_agg(temp_aggregator);
 				break;
 			}
 		}
-		if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
+		if (!curr_port) {
+			/* meaning: the port was related to an aggregator
+			 * but was not on the aggregator port list.
+			 */
 			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
 				   port->slave->dev->master->name,
 				   port->actor_port_number,
@@ -1321,27 +1403,34 @@ static void ad_port_selection_logic(struct port *port)
 				   port->aggregator->aggregator_identifier);
 		}
 	}
-	// search on all aggregators for a suitable aggregator for this port
+	/* search on all aggregators for a suitable aggregator for this port */
 	for (aggregator = __get_first_agg(port); aggregator;
 	     aggregator = __get_next_agg(aggregator)) {
 
-		// keep a free aggregator for later use(if needed)
+		/* keep a free aggregator for later use (if needed) */
 		if (!aggregator->lag_ports) {
 			if (!free_aggregator)
 				free_aggregator = aggregator;
 			continue;
 		}
-		// check if current aggregator suits us
-		if (((aggregator->actor_oper_aggregator_key == port->actor_oper_port_key) && // if all parameters match AND
-		     !MAC_ADDRESS_COMPARE(&(aggregator->partner_system), &(port->partner_oper.system)) &&
+
+		/* check if current aggregator suits us
+		 * a suitable aggregator must fit the following requirements,
+		 * tested here:
+		 *   i. match all parameters;
+		 *  ii. has partner answers;
+		 * iii. it is not individual
+		 */
+		if (((aggregator->actor_oper_aggregator_key == port->actor_oper_port_key) &&
+		     !compare_ether_addr((const u8 *)&(aggregator->partner_system), (const u8 *)&(port->partner_oper.system)) &&
 		     (aggregator->partner_system_priority == port->partner_oper.system_priority) &&
 		     (aggregator->partner_oper_aggregator_key == port->partner_oper.key)
 		    ) &&
-		    ((MAC_ADDRESS_COMPARE(&(port->partner_oper.system), &(null_mac_addr)) && // partner answers
-		      !aggregator->is_individual)  // but is not individual OR
+		    ((compare_ether_addr((const u8 *)&(port->partner_oper.system), (const u8 *)&(null_mac_addr)) &&
+		      !aggregator->is_individual)
 		    )
 		   ) {
-			// attach to the founded aggregator
+			/* attach to the founded aggregator */
 			port->aggregator = aggregator;
 			port->actor_port_aggregator_identifier =
 				port->aggregator->aggregator_identifier;
@@ -1352,42 +1441,45 @@ static void ad_port_selection_logic(struct port *port)
 				 port->actor_port_number,
 				 port->aggregator->aggregator_identifier);
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 			found = 1;
 			break;
 		}
 	}
 
-	// the port couldn't find an aggregator - attach it to a new aggregator
+	/* the port couldn't find an aggregator, attach it to a new aggregator*/
 	if (!found) {
 		if (free_aggregator) {
-			// assign port a new aggregator
+			/* assign port a new aggregator */
 			port->aggregator = free_aggregator;
 			port->actor_port_aggregator_identifier =
 				port->aggregator->aggregator_identifier;
 
-			// update the new aggregator's parameters
-			// if port was responsed from the end-user
+			/* update the new aggregator's parameters
+			   if port was replied from the end-user */
 			if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)
 				/* if port is full duplex */
 				port->aggregator->is_individual = false;
 			else
 				port->aggregator->is_individual = true;
 
-			port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
-			port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
+			port->aggregator->actor_admin_aggregator_key =
+						port->actor_admin_port_key;
+			port->aggregator->actor_oper_aggregator_key =
+						port->actor_oper_port_key;
 			port->aggregator->partner_system =
-				port->partner_oper.system;
+						port->partner_oper.system;
 			port->aggregator->partner_system_priority =
-				port->partner_oper.system_priority;
-			port->aggregator->partner_oper_aggregator_key = port->partner_oper.key;
+					port->partner_oper.system_priority;
+			port->aggregator->partner_oper_aggregator_key =
+						port->partner_oper.key;
 			port->aggregator->receive_state = 1;
 			port->aggregator->transmit_state = 1;
 			port->aggregator->lag_ports = port;
 			port->aggregator->num_of_ports++;
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 
 			pr_debug("Port %d joined LAG %d(new LAG)\n",
@@ -1399,16 +1491,18 @@ static void ad_port_selection_logic(struct port *port)
 			       port->actor_port_number, port->slave->dev->name);
 		}
 	}
-	// if all aggregator's ports are READY_N == TRUE, set ready=TRUE in all aggregator's ports
-	// else set ready=FALSE in all aggregator's ports
-	__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
+	/* if all aggregator's ports are READY_N == TRUE,
+	 * set ready=TRUE in all aggregator's ports
+	 * else set ready=FALSE in all aggregator's ports
+	 */
+	__set_agg_ports_ready(port->aggregator,
+				__agg_ports_are_ready(port->aggregator));
 
 	aggregator = __get_first_agg(port);
 	ad_agg_selection_logic(aggregator);
 }
 
-/*
- * Decide if "agg" is a better choice for the new active aggregator that
+/* Decide if "agg" is a better choice for the new active aggregator that
  * the current best, according to the ad_select policy.
  */
 static struct aggregator *ad_agg_selection_test(struct aggregator *best,
@@ -1533,18 +1627,16 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 
 	if (best &&
 	    __get_agg_selection_mode(best->lag_ports) == BOND_AD_STABLE) {
-		/*
-		 * For the STABLE policy, don't replace the old active
-		 * aggregator if it's still active (it has an answering
-		 * partner) or if both the best and active don't have an
-		 * answering partner.
+
+		/* For the STABLE policy, don't replace the old active
+		 * aggregator if it's still active (it has an answering partner)
+		 * or if both the best and active don't have answering partners
 		 */
 		if (active && active->lag_ports &&
 		    active->lag_ports->is_enabled &&
-		    (__agg_has_partner(active) ||
-		     (!__agg_has_partner(active) && !__agg_has_partner(best)))) {
-			if (!(!active->actor_oper_aggregator_key &&
-			      best->actor_oper_aggregator_key)) {
+		    (__agg_has_partner(active) || !__agg_has_partner(best))) {
+			if (active->actor_oper_aggregator_key ||
+			    !best->actor_oper_aggregator_key) {
 				best = NULL;
 				active->is_active = 1;
 			}
@@ -1556,7 +1648,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		active->is_active = 1;
 	}
 
-	// if there is new best aggregator, activate it
+	/* if there is new best aggregator, activate it */
 	if (best) {
 		pr_debug("best Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
 			 best->aggregator_identifier, best->num_of_ports,
@@ -1577,7 +1669,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 				 agg->is_individual, agg->is_active);
 		}
 
-		// check if any partner replys
+		/* check if any partner replies */
 		if (best->is_individual) {
 			pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
 				   best->slave ? best->slave->dev->master->name : "NULL");
@@ -1592,7 +1684,9 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 			 best->partner_oper_aggregator_key,
 			 best->is_individual, best->is_active);
 
-		// disable the ports that were related to the former active_aggregator
+		/* disable the ports that were related to
+		 * the former active_aggregator
+		 */
 		if (active) {
 			for (port = active->lag_ports; port;
 			     port = port->next_port_in_aggregator) {
@@ -1601,8 +1695,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		}
 	}
 
-	/*
-	 * if the selected aggregator is of join individuals
+	/* if the selected aggregator is of join individuals
 	 * (partner_system is NULL), enable their ports
 	 */
 	active = __get_active_agg(origin);
@@ -1701,8 +1794,10 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		port->ntt = false;
 		port->actor_admin_port_key = 1;
 		port->actor_oper_port_key  = 1;
-		port->actor_admin_port_state = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
-		port->actor_oper_port_state  = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
+		port->actor_admin_port_state =
+				AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
+		port->actor_oper_port_state  =
+				AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
 
 		if (lacp_fast)
 			port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
@@ -1711,7 +1806,7 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
 
 		port->is_enabled = true;
-		// ****** private parameters ******
+		/* ****** private parameters ****** */
 		port->sm_vars = 0x3;
 		port->sm_rx_state = 0;
 		port->sm_rx_timer_counter = 0;
@@ -1753,7 +1848,9 @@ static void ad_enable_collecting_distributing(struct port *port)
  */
 static void ad_disable_collecting_distributing(struct port *port)
 {
-	if (port->aggregator && MAC_ADDRESS_COMPARE(&(port->aggregator->partner_system), &(null_mac_addr))) {
+	if (port->aggregator &&
+	    compare_ether_addr((const u8 *)&(port->aggregator->partner_system),
+	     (const u8 *)&(null_mac_addr))) {
 		pr_debug("Disabling port %d(LAG %d)\n",
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
@@ -1775,27 +1872,28 @@ static void ad_marker_info_send(struct port *port)
 	struct bond_marker marker;
 	u16 index;
 
-	// fill the marker PDU with the appropriate values
+	/* fill the marker PDU with the appropriate values */
 	marker.subtype = 0x02;
 	marker.version_number = 0x01;
 	marker.tlv_type = AD_MARKER_INFORMATION_SUBTYPE;
 	marker.marker_length = 0x16;
-	// convert requester_port to Big Endian
-	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |((u16)(port->actor_port_number & 0xFF00) >> 8));
+	/* convert requester_port to Big Endian */
+	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |
+				((u16)(port->actor_port_number & 0xFF00) >> 8));
 	marker.requester_system = port->actor_system;
-	// convert requester_port(u32) to Big Endian
+	/* convert requester_port(u32) to Big Endian */
 	marker.requester_transaction_id =
-		(((++port->transaction_id & 0xFF) << 24)
-		 | ((port->transaction_id & 0xFF00) << 8)
-		 | ((port->transaction_id & 0xFF0000) >> 8)
-		 | ((port->transaction_id & 0xFF000000) >> 24));
+		(((++port->transaction_id & 0xFF) << 24) |
+		  ((port->transaction_id & 0xFF00) << 8) |
+		  ((port->transaction_id & 0xFF0000) >> 8) |
+		  ((port->transaction_id & 0xFF000000) >> 24));
 	marker.pad = 0;
 	marker.tlv_type_terminator = 0x00;
 	marker.terminator_length = 0x00;
 	for (index = 0; index < 90; index++)
 		marker.reserved_90[index] = 0;
 
-	// send the marker information
+	/* send the marker information */
 	if (ad_marker_send(port, &marker) >= 0) {
 		pr_debug("Sent Marker Information on port %d\n",
 			 port->actor_port_number);
@@ -1814,12 +1912,13 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 {
 	struct bond_marker marker;
 
-	// copy the received marker data to the response marker
-	//marker = *marker_info;
+	/* copy the received marker data to the response marker
+	 * marker = *marker_info;
+	 */
 	memcpy(&marker, marker_info, sizeof(struct bond_marker));
-	// change the marker subtype to marker response
+	/* change the marker subtype to marker response */
 	marker.tlv_type = AD_MARKER_RESPONSE_SUBTYPE;
-	// send the marker response
+	/* send the marker response */
 
 	if (ad_marker_send(port, &marker) >= 0) {
 		pr_debug("Sent Marker Response on port %d\n",
@@ -1839,16 +1938,13 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 static void ad_marker_response_received(struct bond_marker *marker,
 	struct port *port)
 {
-	marker = NULL; /* just to satisfy the compiler */
-	port = NULL;  /* just to satisfy the compiler */
-	// DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW
+	marker = NULL;
+	port = NULL;
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= AD exported functions to the main bonding code ==================
-//////////////////////////////////////////////////////////////////////////////////////
+/* ============= AD exported functions to the main bonding code ============ */
 
-// Check aggregators status in team every T seconds
+/* Check aggregators status in team every T seconds */
 #define AD_AGGREGATOR_SELECTION_TIMER  8
 
 /*
@@ -1876,17 +1972,20 @@ static u16 aggregator_identifier;
  */
 void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast)
 {
-	// check that the bond is not initialized yet
-	if (MAC_ADDRESS_COMPARE(&(BOND_AD_INFO(bond).system.sys_mac_addr),
+	/* check that the bond is not initialized yet */
+	if (compare_ether_addr((const u8 *)&(BOND_AD_INFO(bond).system.sys_mac_addr),
 				bond->dev->dev_addr)) {
 
 		aggregator_identifier = 0;
 
 		BOND_AD_INFO(bond).lacp_fast = lacp_fast;
 		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
+		BOND_AD_INFO(bond).system.sys_mac_addr =
+				      *((struct mac_addr *)bond->dev->dev_addr);
 
-		// initialize how many times this module is called in one second(should be about every 100ms)
+		/* initialize how many times this module is
+		 * called in one second(should be about every 100ms)
+		 */
 		ad_ticks_per_sec = tick_resolution;
 
 		bond_3ad_initiate_agg_selection(bond,
@@ -1914,31 +2013,37 @@ int bond_3ad_bind_slave(struct slave *slave)
 		return -1;
 	}
 
-	//check that the slave has not been initialized yet.
+	/* check that the slave has not been initialized yet. */
 	if (SLAVE_AD_INFO(slave).port.slave != slave) {
 
-		// port initialization
+		/* port initialization */
 		port = &(SLAVE_AD_INFO(slave).port);
 
 		ad_initialize_port(port, BOND_AD_INFO(bond).lacp_fast);
 
 		port->slave = slave;
 		port->actor_port_number = SLAVE_AD_INFO(slave).id;
-		// key is determined according to the link speed, duplex and user key(which is yet not supported)
-		//              ------------------------------------------------------------
-		// Port key :   | User key                       |      Speed       |Duplex|
-		//              ------------------------------------------------------------
-		//              16                               6               1 0
-		port->actor_admin_port_key = 0;	// initialize this parameter
+		/* key is determined according to the link speed,
+		 * duplex and user key(which is yet not supported)
+		 * Port key:
+		 * ------------------------------------------------------------
+		 * | User key                       |      Speed       |Duplex|
+		 * ------------------------------------------------------------
+		 * 16                               6                  1      0
+		 */
+		port->actor_admin_port_key = 0;	/* initialize this parameter */
 		port->actor_admin_port_key |= __get_duplex(port);
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
-		// if the port is not full duplex, then the port should be not lacp Enabled
+		/* if the port is not full duplex,
+		 * then the port should be not lacp Enabled
+		 */
 		if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS))
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
-		// actor system is the bond's system
+		/* actor system is the bond's system */
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
-		// tx timer(to verify that no more than MAX_TX_IN_SECOND lacpdu's are sent in one second)
+
+		/* certify that no more than MAX_TX_IN_SECOND lacpdu sent/sec */
 		port->sm_tx_timer_counter = ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 		port->aggregator = NULL;
 		port->next_port_in_aggregator = NULL;
@@ -1947,12 +2052,13 @@ int bond_3ad_bind_slave(struct slave *slave)
 		__initialize_port_locks(port);
 
 
-		// aggregator initialization
+		/* aggregator initialization */
 		aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
 		ad_initialize_agg(aggregator);
 
-		aggregator->aggregator_mac_address = *((struct mac_addr *)bond->dev->dev_addr);
+		aggregator->aggregator_mac_address =
+				      *((struct mac_addr *)bond->dev->dev_addr);
 		aggregator->aggregator_identifier = (++aggregator_identifier);
 		aggregator->slave = slave;
 		aggregator->is_active = 0;
@@ -1976,13 +2082,13 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	struct aggregator *aggregator, *new_aggregator, *temp_aggregator;
 	int select_new_active_agg = 0;
 
-	// find the aggregator related to this slave
+	/* find the aggregator related to this slave */
 	aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
-	// find the port related to this slave
+	/* find the port related to this slave */
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
 		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
 			   slave->dev->master->name, slave->dev->name);
@@ -1997,32 +2103,43 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	__update_lacpdu_from_port(port);
 	ad_lacpdu_send(port);
 
-	// check if this aggregator is occupied
+	/* check if this aggregator is occupied */
 	if (aggregator->lag_ports) {
-		// check if there are other ports related to this aggregator except
-		// the port related to this slave(thats ensure us that there is a
-		// reason to search for new aggregator, and that we will find one
-		if ((aggregator->lag_ports != port) || (aggregator->lag_ports->next_port_in_aggregator)) {
-			// find new aggregator for the related port(s)
+		/* check if there are other ports related to this aggregator
+		 * except the port related to this slave
+		 * (it ensures us that there is a reason to search for
+		 *  new aggregator, and that we will find one)
+		 */
+		if ((aggregator->lag_ports != port) ||
+		    (aggregator->lag_ports->next_port_in_aggregator)) {
+			/* find new aggregator for the related port(s) */
 			new_aggregator = __get_first_agg(port);
-			for (; new_aggregator; new_aggregator = __get_next_agg(new_aggregator)) {
-				// if the new aggregator is empty, or it is connected to our port only
-				if (!new_aggregator->lag_ports
-				    || ((new_aggregator->lag_ports == port)
-					&& !new_aggregator->lag_ports->next_port_in_aggregator))
+			for (; new_aggregator;
+			     new_aggregator = __get_next_agg(new_aggregator)) {
+				/* if the new aggregator is empty,
+				   or it is connected to our port only */
+				if (!new_aggregator->lag_ports ||
+				    ((new_aggregator->lag_ports == port) &&
+				     !new_aggregator->lag_ports->next_port_in_aggregator))
 					break;
 			}
-			// if new aggregator found, copy the aggregator's parameters
-			// and connect the related lag_ports to the new aggregator
-			if ((new_aggregator) && ((!new_aggregator->lag_ports) || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator))) {
-				pr_debug("Some port(s) related to LAG %d - replaceing with LAG %d\n",
+			/* if new aggregator found, copy the aggregator's
+			 * parameters and connect the related lag_ports to the
+			 * new aggregator
+			 */
+			if ((new_aggregator) &&
+			    ((!new_aggregator->lag_ports) ||
+			     ((new_aggregator->lag_ports == port) &&
+			      !new_aggregator->lag_ports->next_port_in_aggregator))) {
+				pr_debug("Some port(s) related to LAG %d - replacing with LAG %d\n",
 					 aggregator->aggregator_identifier,
 					 new_aggregator->aggregator_identifier);
 
-				if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
+				if ((new_aggregator->lag_ports == port) &&
+				     new_aggregator->is_active) {
 					pr_info("%s: Removing an active aggregator\n",
 						aggregator->slave->dev->master->name);
-					// select new active aggregator
+					/* select new active aggregator */
 					 select_new_active_agg = 1;
 				}
 
@@ -2038,14 +2155,15 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				new_aggregator->is_active = aggregator->is_active;
 				new_aggregator->num_of_ports = aggregator->num_of_ports;
 
-				// update the information that is written on the ports about the aggregator
-				for (temp_port = aggregator->lag_ports; temp_port;
-				     temp_port = temp_port->next_port_in_aggregator) {
+				/* update the information that is written on
+				 * the ports about the aggregator
+				 */
+				for (temp_port = aggregator->lag_ports; temp_port; temp_port = temp_port->next_port_in_aggregator) {
 					temp_port->aggregator = new_aggregator;
 					temp_port->actor_port_aggregator_identifier = new_aggregator->aggregator_identifier;
 				}
 
-				// clear the aggregator
+				/* clear the aggregator */
 				ad_clear_agg(aggregator);
 
 				if (select_new_active_agg)
@@ -2054,42 +2172,50 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
 					   slave->dev->master->name);
 			}
-		} else { // in case that the only port related to this aggregator is the one we want to remove
+		} else {
+			 /* in case that the only port related to this
+			  * aggregator is the one we want to remove
+			  */
 			select_new_active_agg = aggregator->is_active;
-			// clear the aggregator
+			/* clear the aggregator */
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
 				pr_info("%s: Removing an active aggregator\n",
 					slave->dev->master->name);
-				// select new active aggregator
+				/* select new active aggregator */
 				ad_agg_selection_logic(__get_first_agg(port));
 			}
 		}
 	}
 
 	pr_debug("Unbinding port %d\n", port->actor_port_number);
-	// find the aggregator that this port is connected to
+	/* find the aggregator that this port is connected to */
 	temp_aggregator = __get_first_agg(port);
-	for (; temp_aggregator; temp_aggregator = __get_next_agg(temp_aggregator)) {
+	for (; temp_aggregator;
+	     temp_aggregator = __get_next_agg(temp_aggregator)) {
 		prev_port = NULL;
-		// search the port in the aggregator's related ports
+		/* search the port in the aggregator's related ports */
 		for (temp_port = temp_aggregator->lag_ports; temp_port;
 		     prev_port = temp_port,
 			     temp_port = temp_port->next_port_in_aggregator) {
-			if (temp_port == port) { // the aggregator found - detach the port from this aggregator
+			if (temp_port == port) {
+				/* the aggregator found
+				   detach the port from this aggregator */
 				if (prev_port)
-					prev_port->next_port_in_aggregator = temp_port->next_port_in_aggregator;
+					prev_port->next_port_in_aggregator =
+					     temp_port->next_port_in_aggregator;
 				else
-					temp_aggregator->lag_ports = temp_port->next_port_in_aggregator;
+					temp_aggregator->lag_ports =
+					     temp_port->next_port_in_aggregator;
 				temp_aggregator->num_of_ports--;
 				if (temp_aggregator->num_of_ports == 0) {
 					select_new_active_agg = temp_aggregator->is_active;
-					// clear the aggregator
+					/* clear the aggregator */
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
 						pr_info("%s: Removing an active aggregator\n",
 							slave->dev->master->name);
-						// select new active aggregator
+						/* select new active aggreg */
 						ad_agg_selection_logic(__get_first_agg(port));
 					}
 				}
@@ -2125,13 +2251,14 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	if (bond->kill_timers)
 		goto out;
 
-	//check if there are any slaves
+	/* check if there are any slaves */
 	if (bond->slave_cnt == 0)
 		goto re_arm;
 
-	// check if agg_select_timer timer after initialize is timed out
-	if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
-		// select the active aggregator for the bond
+	/* check if agg_select_timer timer after initialize is timed out */
+	if (BOND_AD_INFO(bond).agg_select_timer &&
+	    !(--BOND_AD_INFO(bond).agg_select_timer)) {
+		/* select the active aggregator for the bond */
 		if ((port = __get_first_port(bond))) {
 			if (!port->slave) {
 				pr_warning("%s: Warning: bond's first port is uninitialized\n",
@@ -2145,17 +2272,18 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		bond_3ad_set_carrier(bond);
 	}
 
-	// for each port run the state machines
-	for (port = __get_first_port(bond); port; port = __get_next_port(port)) {
+	/* for each port run the state machines */
+	for (port = __get_first_port(bond); port;
+	     port = __get_next_port(port)) {
 		if (!port->slave) {
 			pr_warning("%s: Warning: Found an uninitialized port\n",
 				   bond->dev->name);
 			goto re_arm;
 		}
 
-		/* Lock around state machines to protect data accessed
-		 * by all (e.g., port->sm_vars).  ad_rx_machine may run
-		 * concurrently due to incoming LACPDU.
+		/* Lock around state machines to protect data accessed by all
+		 * (e.g., port->sm_vars).
+		 * ad_rx_machine may run concurrently due to incoming LACPDU.
 		 */
 		__get_state_machine_lock(port);
 
@@ -2165,7 +2293,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		ad_mux_machine(port);
 		ad_tx_machine(port);
 
-		// turn off the BEGIN bit, since we already handled it
+		/* turn off the BEGIN bit, since we already handled it */
 		if (port->sm_vars & AD_PORT_BEGIN)
 			port->sm_vars &= ~AD_PORT_BEGIN;
 
@@ -2198,7 +2326,8 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
 
 		if (!port->slave) {
 			pr_warning("%s: Warning: port of slave %s is uninitialized\n",
-				   slave->dev->name, slave->dev->master->name);
+				   slave->dev->name,
+				   slave->dev->master->name);
 			return;
 		}
 
@@ -2213,7 +2342,9 @@ static void bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u
 			break;
 
 		case AD_TYPE_MARKER:
-			// No need to convert fields to Little Endian since we don't use the marker's fields.
+			/* No need to convert fields to Little Endian
+			 *  since we don't use the marker's fields.
+			 */
 
 			switch (((struct bond_marker *)lacpdu)->tlv_type) {
 			case AD_MARKER_INFORMATION_SUBTYPE:
@@ -2248,19 +2379,22 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
 		pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->dev->master->name,
+			   slave->dev->name);
 		return;
 	}
 
 	port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS;
 	port->actor_oper_port_key = port->actor_admin_port_key |=
 		(__get_link_speed(port) << 1);
+
 	pr_debug("Port %d changed speed\n", port->actor_port_number);
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+	/* there is no need to reselect a new aggregator,
+	 * just signal the state machines to reinitialize
+	 */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2276,7 +2410,7 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
 		pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
 			   slave->dev->master->name, slave->dev->name);
@@ -2286,9 +2420,11 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 	port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
 	port->actor_oper_port_key = port->actor_admin_port_key |=
 		__get_duplex(port);
+
 	pr_debug("Port %d changed duplex\n", port->actor_port_number);
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+	/* there is no need to reselect a new aggregator,
+	 * just signal the state machines to reinitialize
+	 */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2305,15 +2441,18 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
 		pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
 			   slave->dev->master->name, slave->dev->name);
 		return;
 	}
 
-	// on link down we are zeroing duplex and speed since some of the adaptors(ce1000.lan) report full duplex/speed instead of N/A(duplex) / 0(speed)
-	// on link up we are forcing recheck on the duplex and speed since some of he adaptors(ce1000.lan) report
+	/* on link down we are zeroing duplex and speed
+	 * since some of the adapters (ce1000.lan) report full duplex/speed
+	 * instead of N/A (duplex) / 0(speed)
+	 * on link up we are forcing recheck on the duplex and speed
+	 */
 	if (link == BOND_LINK_UP) {
 		port->is_enabled = true;
 		port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
@@ -2329,9 +2468,15 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 		port->actor_oper_port_key = (port->actor_admin_port_key &=
 					     ~AD_SPEED_KEY_BITS);
 	}
-	//BOND_PRINT_DBG(("Port %d changed link status to %s", port->actor_port_number, ((link == BOND_LINK_UP)?"UP":"DOWN")));
-	// there is no need to reselect a new aggregator, just signal the
-	// state machines to reinitialize
+
+	/* BOND_PRINT_DBG(("Port %d changed link status to %s",
+	 *		  port->actor_port_number,
+	 *		  ((link == BOND_LINK_UP)?"UP":"DOWN")));
+	 */
+
+	/* there is no need to reselect a new aggregator,
+	 * just signal the state machines to reinitialize
+	 */
 	port->sm_vars |= AD_PORT_BEGIN;
 }
 
@@ -2387,7 +2532,8 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 		ad_info->ports = aggregator->num_of_ports;
 		ad_info->actor_key = aggregator->actor_oper_aggregator_key;
 		ad_info->partner_key = aggregator->partner_oper_aggregator_key;
-		memcpy(ad_info->partner_system, aggregator->partner_system.mac_addr_value, ETH_ALEN);
+		memcpy(ad_info->partner_system,
+			 aggregator->partner_system.mac_addr_value, ETH_ALEN);
 		return 0;
 	}
 
@@ -2405,9 +2551,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	struct ad_info ad_info;
 	int res = 1;
 
-	/* make sure that the slaves list will
-	 * not change during tx
-	 */
+	/* make sure that the slaves list will not change during tx */
 	read_lock(&bond->lock);
 
 	if (!BOND_IS_OK(bond))
@@ -2442,7 +2586,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 
 	if (slave_agg_no >= 0) {
 		pr_err("%s: Error: Couldn't find a slave to tx on for aggregator ID %d\n",
-		       dev->name, agg_id);
+			dev->name, agg_id);
 		goto out;
 	}
 
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index b28baff..f12e054 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -28,7 +28,7 @@
 #include <linux/netdevice.h>
 #include <linux/if_ether.h>
 
-// General definitions
+/* General definitions */
 #define PKT_TYPE_LACPDU         cpu_to_be16(ETH_P_SLOW)
 #define AD_TIMER_INTERVAL       100 /*msec*/
 
@@ -47,54 +47,54 @@ enum {
 	BOND_AD_COUNT = 2,
 };
 
-// rx machine states(43.4.11 in the 802.3ad standard)
+/* rx machine states (43.4.11 in the 802.3ad standard) */
 typedef enum {
 	AD_RX_DUMMY,
-	AD_RX_INITIALIZE,     // rx Machine
-	AD_RX_PORT_DISABLED,  // rx Machine
-	AD_RX_LACP_DISABLED,  // rx Machine
-	AD_RX_EXPIRED,	      // rx Machine
-	AD_RX_DEFAULTED,      // rx Machine
-	AD_RX_CURRENT	      // rx Machine
+	AD_RX_INITIALIZE,
+	AD_RX_PORT_DISABLED,
+	AD_RX_LACP_DISABLED,
+	AD_RX_EXPIRED,
+	AD_RX_DEFAULTED,
+	AD_RX_CURRENT
 } rx_states_t;
 
-// periodic machine states(43.4.12 in the 802.3ad standard)
+/* periodic machine states (43.4.12 in the 802.3ad standard) */
 typedef enum {
 	AD_PERIODIC_DUMMY,
-	AD_NO_PERIODIC,	       // periodic machine
-	AD_FAST_PERIODIC,      // periodic machine
-	AD_SLOW_PERIODIC,      // periodic machine
-	AD_PERIODIC_TX	   // periodic machine
+	AD_NO_PERIODIC,
+	AD_FAST_PERIODIC,
+	AD_SLOW_PERIODIC,
+	AD_PERIODIC_TX
 } periodic_states_t;
 
-// mux machine states(43.4.13 in the 802.3ad standard)
+/* mux machine states (43.4.13 in the 802.3ad standard) */
 typedef enum {
 	AD_MUX_DUMMY,
-	AD_MUX_DETACHED,       // mux machine
-	AD_MUX_WAITING,	       // mux machine
-	AD_MUX_ATTACHED,       // mux machine
-	AD_MUX_COLLECTING_DISTRIBUTING // mux machine
+	AD_MUX_DETACHED,
+	AD_MUX_WAITING,
+	AD_MUX_ATTACHED,
+	AD_MUX_COLLECTING_DISTRIBUTING
 } mux_states_t;
 
-// tx machine states(43.4.15 in the 802.3ad standard)
+/* tx machine states (43.4.15 in the 802.3ad standard) */
 typedef enum {
 	AD_TX_DUMMY,
-	AD_TRANSMIT	   // tx Machine
+	AD_TRANSMIT
 } tx_states_t;
 
-// rx indication types
+/* rx indication types */
 typedef enum {
-	AD_TYPE_LACPDU = 1,    // type lacpdu
-	AD_TYPE_MARKER	   // type marker
+	AD_TYPE_LACPDU = 1,
+	AD_TYPE_MARKER
 } pdu_type_t;
 
-// rx marker indication types
+/* rx marker indication types */
 typedef enum {
-	AD_MARKER_INFORMATION_SUBTYPE = 1, // marker imformation subtype
-	AD_MARKER_RESPONSE_SUBTYPE     // marker response subtype
+	AD_MARKER_INFORMATION_SUBTYPE = 1,
+	AD_MARKER_RESPONSE_SUBTYPE
 } bond_marker_subtype_t;
 
-// timers types(43.4.9 in the 802.3ad standard)
+/* timers types (43.4.9 in the 802.3ad standard) */
 typedef enum {
 	AD_CURRENT_WHILE_TIMER,
 	AD_ACTOR_CHURN_TIMER,
@@ -105,35 +105,37 @@ typedef enum {
 
 #pragma pack(1)
 
-// Link Aggregation Control Protocol(LACP) data unit structure(43.4.2.2 in the 802.3ad standard)
+/* Link Aggregation Control Protocol (LACP) data unit structure
+ * (43.4.2.2 in the 802.3ad standard)
+ */
 typedef struct lacpdu {
-	u8 subtype;		     // = LACP(= 0x01)
+	u8 subtype;		          /* = LACP(= 0x01) */
 	u8 version_number;
-	u8 tlv_type_actor_info;	      // = actor information(type/length/value)
-	u8 actor_information_length; // = 20
+	u8 tlv_type_actor_info;	          /* = actor info(type/length/value)*/
+	u8 actor_information_length;      /* = 20 */
 	__be16 actor_system_priority;
 	struct mac_addr actor_system;
 	__be16 actor_key;
 	__be16 actor_port_priority;
 	__be16 actor_port;
 	u8 actor_state;
-	u8 reserved_3_1[3];	     // = 0
-	u8 tlv_type_partner_info;     // = partner information
-	u8 partner_information_length;	 // = 20
+	u8 reserved_3_1[3];	          /* = 0 */
+	u8 tlv_type_partner_info;         /* = partner information */
+	u8 partner_information_length;	  /* = 20 */
 	__be16 partner_system_priority;
 	struct mac_addr partner_system;
 	__be16 partner_key;
 	__be16 partner_port_priority;
 	__be16 partner_port;
 	u8 partner_state;
-	u8 reserved_3_2[3];	     // = 0
-	u8 tlv_type_collector_info;	  // = collector information
-	u8 collector_information_length; // = 16
+	u8 reserved_3_2[3];	          /* = 0 */
+	u8 tlv_type_collector_info;	  /* = collector information */
+	u8 collector_information_length;  /* = 16 */
 	__be16 collector_max_delay;
 	u8 reserved_12[12];
-	u8 tlv_type_terminator;	     // = terminator
-	u8 terminator_length;	     // = 0
-	u8 reserved_50[50];	     // = 0
+	u8 tlv_type_terminator;	          /* = terminator */
+	u8 terminator_length;	          /* = 0 */
+	u8 reserved_50[50];	          /* = 0 */
 } lacpdu_t;
 
 typedef struct lacpdu_header {
@@ -141,20 +143,22 @@ typedef struct lacpdu_header {
 	struct lacpdu lacpdu;
 } lacpdu_header_t;
 
-// Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
+/* Marker Protocol Data Unit(PDU) structure
+ * (43.5.3.2 in the 802.3ad standard)
+ */
 typedef struct bond_marker {
-	u8 subtype;		 //  = 0x02  (marker PDU)
-	u8 version_number;	 //  = 0x01
-	u8 tlv_type;		 //  = 0x01  (marker information)
-	//  = 0x02  (marker response information)
-	u8 marker_length;	 //  = 0x16
-	u16 requester_port;	 //   The number assigned to the port by the requester
-	struct mac_addr requester_system;      //   The requester's system id
-	u32 requester_transaction_id;	//   The transaction id allocated by the requester,
-	u16 pad;		 //  = 0
-	u8 tlv_type_terminator;	     //  = 0x00
-	u8 terminator_length;	     //  = 0x00
-	u8 reserved_90[90];	     //  = 0
+	u8 subtype;			  /* = 0x02 (marker PDU) */
+	u8 version_number;		  /* = 0x01 */
+	u8 tlv_type;			  /* = 0x01 (marker information)
+					   * = 0x02  (marker response info */
+	u8 marker_length;		  /* = 0x16 */
+	u16 requester_port;
+	struct mac_addr requester_system; /* The requester's system id */
+	u32 requester_transaction_id;
+	u16 pad;			  /* = 0 */
+	u8 tlv_type_terminator;		  /* = 0x00 */
+	u8 terminator_length;		  /* = 0x00 */
+	u8 reserved_90[90];		  /* = 0 */
 } bond_marker_t;
 
 typedef struct bond_marker_header {
@@ -173,7 +177,7 @@ struct port;
 #pragma pack(8)
 #endif
 
-// aggregator structure(43.4.5 in the 802.3ad standard)
+/* aggregator structure (43.4.5 in the 802.3ad standard) */
 typedef struct aggregator {
 	struct mac_addr aggregator_mac_address;
 	u16 aggregator_identifier;
@@ -183,12 +187,13 @@ typedef struct aggregator {
 	struct mac_addr partner_system;
 	u16 partner_system_priority;
 	u16 partner_oper_aggregator_key;
-	u16 receive_state;		// BOOLEAN
-	u16 transmit_state;		// BOOLEAN
+	u16 receive_state;		/* BOOLEAN */
+	u16 transmit_state;		/* BOOLEAN */
 	struct port *lag_ports;
-	// ****** PRIVATE PARAMETERS ******
-	struct slave *slave;	    // pointer to the bond slave that this aggregator belongs to
-	u16 is_active;	    // BOOLEAN. Indicates if this aggregator is active
+	/* ****** PRIVATE PARAMETERS ****** */
+	struct slave *slave; /* pointer to the bond slave
+				that this aggregator belongs to */
+	u16 is_active;	     /* BOOLEAN. Indicates if the aggregator is active*/
 	u16 num_of_ports;
 } aggregator_t;
 
@@ -201,12 +206,18 @@ struct port_params {
 	u16 port_state;
 };
 
-// port structure(43.4.6 in the 802.3ad standard)
+/* port structure (43.4.6 in the 802.3ad standard) */
 typedef struct port {
 	u16 actor_port_number;
 	u16 actor_port_priority;
-	struct mac_addr actor_system;	       // This parameter is added here although it is not specified in the standard, just for simplification
-	u16 actor_system_priority;	 // This parameter is added here although it is not specified in the standard, just for simplification
+
+	/* in a attempt to simplify operations the
+	 * following two elements were included here
+	 * despite they are not specified in the standard
+	 */
+	struct mac_addr actor_system;
+	u16 actor_system_priority;
+
 	u16 actor_port_aggregator_identifier;
 	bool ntt;
 	u16 actor_admin_port_key;
@@ -219,21 +230,21 @@ typedef struct port {
 
 	bool is_enabled;
 
-	// ****** PRIVATE PARAMETERS ******
-	u16 sm_vars;	      // all state machines variables for this port
-	rx_states_t sm_rx_state;	// state machine rx state
-	u16 sm_rx_timer_counter;    // state machine rx timer counter
-	periodic_states_t sm_periodic_state;// state machine periodic state
-	u16 sm_periodic_timer_counter;	// state machine periodic timer counter
-	mux_states_t sm_mux_state;	// state machine mux state
-	u16 sm_mux_timer_counter;   // state machine mux timer counter
-	tx_states_t sm_tx_state;	// state machine tx state
-	u16 sm_tx_timer_counter;    // state machine tx timer counter(allways on - enter to transmit state 3 time per second)
-	struct slave *slave;	    // pointer to the bond slave that this port belongs to
-	struct aggregator *aggregator;	   // pointer to an aggregator that this port related to
-	struct port *next_port_in_aggregator; // Next port on the linked list of the parent aggregator
-	u32 transaction_id;	    // continuous number for identification of Marker PDU's;
-	struct lacpdu lacpdu;	       // the lacpdu that will be sent for this port
+	/* ****** PRIVATE PARAMETERS ****** */
+	u16 sm_vars;
+	rx_states_t sm_rx_state;
+	u16 sm_rx_timer_counter;
+	periodic_states_t sm_periodic_state;
+	u16 sm_periodic_timer_counter;
+	mux_states_t sm_mux_state;
+	u16 sm_mux_timer_counter;
+	tx_states_t sm_tx_state;
+	u16 sm_tx_timer_counter;
+	struct slave *slave;
+	struct aggregator *aggregator;
+	struct port *next_port_in_aggregator;
+	u32 transaction_id;
+	struct lacpdu lacpdu;
 } port_t;
 
 // system structure
@@ -246,41 +257,41 @@ struct ad_system {
 #pragma pack()
 #endif
 
-// ================= AD Exported structures to the main bonding code ==================
+/* =========== AD Exported structures to the main bonding code ============ */
 #define BOND_AD_INFO(bond)   ((bond)->ad_info)
 #define SLAVE_AD_INFO(slave) ((slave)->ad_info)
 
 struct ad_bond_info {
 	struct ad_system system;	    /* 802.3ad system structure */
-	u32 agg_select_timer;	    // Timer to select aggregator after all adapter's hand shakes
-	u32 agg_select_mode;	    // Mode of selection of active aggregator(bandwidth/count)
-	int lacp_fast;		/* whether fast periodic tx should be
-				 * requested
-				 */
+	u32 agg_select_timer;	            /* aggregator's selected timer */
+	u32 agg_select_mode;	            /* aggregator's selected mode */
+	int lacp_fast;
 	struct timer_list ad_timer;
 	struct packet_type ad_pkt_type;
 };
 
 struct ad_slave_info {
-	struct aggregator aggregator;	    // 802.3ad aggregator structure
-	struct port port;		    // 802.3ad port structure
-	spinlock_t state_machine_lock; /* mutex state machines vs.
-					  incoming LACPDU */
+	struct aggregator aggregator;	    /* 802.3ad aggregator structure */
+	struct port port;		    /* 802.3ad port structure */
+	spinlock_t state_machine_lock;	    /* mutex state machines vs.
+					     * incoming LACPDU */
 	u16 id;
 };
 
-// ================= AD Exported functions to the main bonding code ==================
-void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast);
-int  bond_3ad_bind_slave(struct slave *slave);
+/* ========= AD Exported functions to the main bonding code ========== */
+void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution,
+			 int lacp_fast);
+int bond_3ad_bind_slave(struct slave *slave);
 void bond_3ad_unbind_slave(struct slave *slave);
 void bond_3ad_state_machine_handler(struct work_struct *);
 void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout);
 void bond_3ad_adapter_speed_changed(struct slave *slave);
 void bond_3ad_adapter_duplex_changed(struct slave *slave);
 void bond_3ad_handle_link_change(struct slave *slave, char link);
-int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
+int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
 int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev, struct packet_type* ptype, struct net_device *orig_dev);
+int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct net_device *dev,
+			struct packet_type* ptype, struct net_device *orig_dev);
 int bond_3ad_set_carrier(struct bonding *bond);
 #endif //__BOND_3AD_H__
 
-- 
1.7.4.4


-- 
Rafael Aquini <aquini@linux.com>

^ permalink raw reply related

* RE: [RFC V2 PATCH] rtnetlink: Add method to calculate dump info data size
From: Rose, Gregory V @ 2011-05-09 22:49 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev@vger.kernel.org, bhutchings@solarflare.com,
	davem@davemloft.net
In-Reply-To: <20110509154527.28a72365@nehalam>

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Monday, May 09, 2011 3:45 PM
> To: Rose, Gregory V
> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com; davem@davemloft.net
> Subject: Re: [RFC V2 PATCH] rtnetlink: Add method to calculate dump info
> data size
> 
> On Mon, 09 May 2011 15:26:29 -0700
> Greg Rose <gregory.v.rose@intel.com> wrote:
> 
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -56,6 +56,7 @@
> >  struct rtnl_link {
> >  	rtnl_doit_func		doit;
> >  	rtnl_dumpit_func	dumpit;
> > +	rtnl_calcit_func calcit;
> >  };
> 
> Minor nit, why not line up the columns like the other two?

Haste I suppose.

If this RFC gets the go ahead I'll clean it up.  There's other check patch failures I need to fix anyways but I didn't want to spend a lot of time on something I wasn't sure would fly.

Thanks,

- Greg


^ permalink raw reply

* Re: [RFC V2 PATCH] rtnetlink: Add method to calculate dump info data size
From: Stephen Hemminger @ 2011-05-09 22:45 UTC (permalink / raw)
  To: Greg Rose; +Cc: netdev, bhutchings, davem
In-Reply-To: <20110509222629.8689.77365.stgit@gitlad.jf.intel.com>

On Mon, 09 May 2011 15:26:29 -0700
Greg Rose <gregory.v.rose@intel.com> wrote:

> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -56,6 +56,7 @@
>  struct rtnl_link {
>  	rtnl_doit_func		doit;
>  	rtnl_dumpit_func	dumpit;
> +	rtnl_calcit_func calcit;
>  };

Minor nit, why not line up the columns like the other two?

-- 

^ permalink raw reply

* [RFC V2 PATCH] rtnetlink: Add method to calculate dump info data size
From: Greg Rose @ 2011-05-09 22:26 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings, davem

The message size allocated for rtnl info dumps was limited to a single
page.  This is not enough for additional interface info available with
devices that support SR-IOV.  Calculate the amount of data required so
the dump can allocate enough data to satisfy the request.

V2 of this patch adds a new argument to the rtnl_register service that
allows for a new method to calculate the amount of data required to
complete the info dump request.  So far the method is only implemented
for the RTM_GETLINK slot.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
---

 include/linux/netlink.h              |    6 ++-
 include/net/rtnetlink.h              |    5 +-
 net/bridge/br_netlink.c              |   15 ++++---
 net/core/fib_rules.c                 |    6 +--
 net/core/neighbour.c                 |   10 ++--
 net/core/rtnetlink.c                 |   76 ++++++++++++++++++++++++++++++----
 net/dcb/dcbnl.c                      |    4 +-
 net/decnet/dn_dev.c                  |    6 +--
 net/decnet/dn_fib.c                  |    4 +-
 net/decnet/dn_route.c                |    5 +-
 net/ipv4/devinet.c                   |    6 +--
 net/ipv4/fib_frontend.c              |    6 +--
 net/ipv4/inet_diag.c                 |    2 -
 net/ipv4/ipmr.c                      |    3 +
 net/ipv4/route.c                     |    2 -
 net/ipv6/addrconf.c                  |   12 +++--
 net/ipv6/addrlabel.c                 |    6 +--
 net/ipv6/ip6_fib.c                   |    2 -
 net/ipv6/ip6mr.c                     |    2 -
 net/ipv6/route.c                     |    6 +--
 net/netfilter/ipset/ip_set_core.c    |    2 -
 net/netfilter/nf_conntrack_netlink.c |    4 +-
 net/netlink/af_netlink.c             |   17 +++++---
 net/netlink/genetlink.c              |    2 -
 net/phonet/pn_netlink.c              |   12 +++--
 net/sched/act_api.c                  |    6 +--
 net/sched/cls_api.c                  |    6 +--
 net/sched/sch_api.c                  |   12 +++--
 net/xfrm/xfrm_user.c                 |    2 -
 29 files changed, 159 insertions(+), 88 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 4c4ac3f..8b8dfb8 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -220,7 +220,8 @@ struct netlink_callback {
 	int			(*dump)(struct sk_buff * skb,
 					struct netlink_callback *cb);
 	int			(*done)(struct netlink_callback *cb);
-	int			family;
+	u16			family;
+	u16			min_dump_alloc;
 	long			args[6];
 };
 
@@ -258,7 +259,8 @@ __nlmsg_put(struct sk_buff *skb, u32 pid, u32 seq, int type, int len, int flags)
 extern int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      int (*dump)(struct sk_buff *skb, struct netlink_callback*),
-			      int (*done)(struct netlink_callback*));
+			      int (*done)(struct netlink_callback*),
+			      u16 min_dump_alloc);
 
 
 #define NL_NONROOT_RECV 0x1
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 4093ca7..d1ac642 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -6,11 +6,12 @@
 
 typedef int (*rtnl_doit_func)(struct sk_buff *, struct nlmsghdr *, void *);
 typedef int (*rtnl_dumpit_func)(struct sk_buff *, struct netlink_callback *);
+typedef u16 (*rtnl_calcit_func)(struct sk_buff *);
 
 extern int	__rtnl_register(int protocol, int msgtype,
-				rtnl_doit_func, rtnl_dumpit_func);
+				rtnl_doit_func, rtnl_dumpit_func, rtnl_calcit_func);
 extern void	rtnl_register(int protocol, int msgtype,
-			      rtnl_doit_func, rtnl_dumpit_func);
+			      rtnl_doit_func, rtnl_dumpit_func, rtnl_calcit_func);
 extern int	rtnl_unregister(int protocol, int msgtype);
 extern void	rtnl_unregister_all(int protocol);
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index ffb0dc4..6814083 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -218,19 +218,24 @@ int __init br_netlink_init(void)
 	if (err < 0)
 		goto err1;
 
-	err = __rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL, br_dump_ifinfo);
+	err = __rtnl_register(PF_BRIDGE, RTM_GETLINK, NULL,
+			      br_dump_ifinfo, NULL);
 	if (err)
 		goto err2;
-	err = __rtnl_register(PF_BRIDGE, RTM_SETLINK, br_rtm_setlink, NULL);
+	err = __rtnl_register(PF_BRIDGE, RTM_SETLINK,
+			      br_rtm_setlink, NULL, NULL);
 	if (err)
 		goto err3;
-	err = __rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, br_fdb_add, NULL);
+	err = __rtnl_register(PF_BRIDGE, RTM_NEWNEIGH,
+			      br_fdb_add, NULL, NULL);
 	if (err)
 		goto err3;
-	err = __rtnl_register(PF_BRIDGE, RTM_DELNEIGH, br_fdb_delete, NULL);
+	err = __rtnl_register(PF_BRIDGE, RTM_DELNEIGH,
+			      br_fdb_delete, NULL, NULL);
 	if (err)
 		goto err3;
-	err = __rtnl_register(PF_BRIDGE, RTM_GETNEIGH, NULL, br_fdb_dump);
+	err = __rtnl_register(PF_BRIDGE, RTM_GETNEIGH,
+			      NULL, br_fdb_dump, NULL);
 	if (err)
 		goto err3;
 
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 3911586..56e6fc8 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -739,9 +739,9 @@ static struct pernet_operations fib_rules_net_ops = {
 static int __init fib_rules_init(void)
 {
 	int err;
-	rtnl_register(PF_UNSPEC, RTM_NEWRULE, fib_nl_newrule, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELRULE, fib_nl_delrule, NULL);
-	rtnl_register(PF_UNSPEC, RTM_GETRULE, NULL, fib_nl_dumprule);
+	rtnl_register(PF_UNSPEC, RTM_NEWRULE, fib_nl_newrule, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELRULE, fib_nl_delrule, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETRULE, NULL, fib_nl_dumprule, NULL);
 
 	err = register_pernet_subsys(&fib_rules_net_ops);
 	if (err < 0)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 799f06e..a880b83 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2909,12 +2909,12 @@ EXPORT_SYMBOL(neigh_sysctl_unregister);
 
 static int __init neigh_init(void)
 {
-	rtnl_register(PF_UNSPEC, RTM_NEWNEIGH, neigh_add, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELNEIGH, neigh_delete, NULL);
-	rtnl_register(PF_UNSPEC, RTM_GETNEIGH, NULL, neigh_dump_info);
+	rtnl_register(PF_UNSPEC, RTM_NEWNEIGH, neigh_add, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELNEIGH, neigh_delete, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETNEIGH, NULL, neigh_dump_info, NULL);
 
-	rtnl_register(PF_UNSPEC, RTM_GETNEIGHTBL, NULL, neightbl_dump_info);
-	rtnl_register(PF_UNSPEC, RTM_SETNEIGHTBL, neightbl_set, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETNEIGHTBL, NULL, neightbl_dump_info, NULL);
+	rtnl_register(PF_UNSPEC, RTM_SETNEIGHTBL, neightbl_set, NULL, NULL);
 
 	return 0;
 }
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2963312..77b7f4a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -56,6 +56,7 @@
 struct rtnl_link {
 	rtnl_doit_func		doit;
 	rtnl_dumpit_func	dumpit;
+	rtnl_calcit_func calcit;
 };
 
 static DEFINE_MUTEX(rtnl_mutex);
@@ -144,12 +145,28 @@ static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex)
 	return tab ? tab[msgindex].dumpit : NULL;
 }
 
+static rtnl_calcit_func rtnl_get_calcit(int protocol, int msgindex)
+{
+	struct rtnl_link *tab;
+
+	if (protocol <= RTNL_FAMILY_MAX)
+		tab = rtnl_msg_handlers[protocol];
+	else
+		tab = NULL;
+
+	if (tab == NULL || tab[msgindex].calcit == NULL)
+		tab = rtnl_msg_handlers[PF_UNSPEC];
+
+	return tab ? tab[msgindex].calcit : NULL;
+}
+
 /**
  * __rtnl_register - Register a rtnetlink message type
  * @protocol: Protocol family or PF_UNSPEC
  * @msgtype: rtnetlink message type
  * @doit: Function pointer called for each request message
  * @dumpit: Function pointer called for each dump request (NLM_F_DUMP) message
+ * @calcit: Function pointer to calc size of dump message
  *
  * Registers the specified function pointers (at least one of them has
  * to be non-NULL) to be called whenever a request message for the
@@ -162,7 +179,8 @@ static rtnl_dumpit_func rtnl_get_dumpit(int protocol, int msgindex)
  * Returns 0 on success or a negative error code.
  */
 int __rtnl_register(int protocol, int msgtype,
-		    rtnl_doit_func doit, rtnl_dumpit_func dumpit)
+		    rtnl_doit_func doit, rtnl_dumpit_func dumpit,
+		    rtnl_calcit_func calcit)
 {
 	struct rtnl_link *tab;
 	int msgindex;
@@ -185,6 +203,9 @@ int __rtnl_register(int protocol, int msgtype,
 	if (dumpit)
 		tab[msgindex].dumpit = dumpit;
 
+	if (calcit)
+		tab[msgindex].calcit = calcit;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__rtnl_register);
@@ -199,9 +220,10 @@ EXPORT_SYMBOL_GPL(__rtnl_register);
  * of memory implies no sense in continuing.
  */
 void rtnl_register(int protocol, int msgtype,
-		   rtnl_doit_func doit, rtnl_dumpit_func dumpit)
+		   rtnl_doit_func doit, rtnl_dumpit_func dumpit,
+		   rtnl_calcit_func calcit)
 {
-	if (__rtnl_register(protocol, msgtype, doit, dumpit) < 0)
+	if (__rtnl_register(protocol, msgtype, doit, dumpit, calcit) < 0)
 		panic("Unable to register rtnetlink message handler, "
 		      "protocol = %d, message type = %d\n",
 		      protocol, msgtype);
@@ -1819,6 +1841,32 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 	return err;
 }
 
+static u16 rtnl_calcit(struct sk_buff *skb)
+{
+	struct net *net = sock_net(skb->sk);
+	int h;
+	int idx = 0, s_idx;
+	struct net_device *dev;
+	struct hlist_head *head;
+	struct hlist_node *node;
+	u16 alloc_size = 0;
+
+	for (h = 0; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+		idx = 0;
+		head = &net->dev_index_head[h];
+		hlist_for_each_entry(dev, node, head, index_hlist) {
+			if (idx < s_idx) {
+				idx++;
+				continue;
+			}
+			alloc_size = (u16)if_nlmsg_size(dev);
+			break;
+		}
+	}
+
+	return alloc_size;
+}
+
 static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	int idx;
@@ -1902,13 +1950,21 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
 		struct sock *rtnl;
 		rtnl_dumpit_func dumpit;
+		rtnl_calcit_func calcit;
+		u16 min_dump_alloc = 0;
 
 		dumpit = rtnl_get_dumpit(family, type);
 		if (dumpit == NULL)
 			return -EOPNOTSUPP;
+		calcit = rtnl_get_calcit(family, type);
+		if (calcit) {
+			min_dump_alloc = calcit(skb);
+			printk("Minimum Dump Alloc Size = %d\n", min_dump_alloc);
+		}
 
 		rtnl = net->rtnl;
-		return netlink_dump_start(rtnl, skb, nlh, dumpit, NULL);
+		return netlink_dump_start(rtnl, skb, nlh, dumpit,
+					  NULL, min_dump_alloc);
 	}
 
 	memset(rta_buf, 0, (rtattr_max * sizeof(struct rtattr *)));
@@ -2014,12 +2070,12 @@ void __init rtnetlink_init(void)
 	netlink_set_nonroot(NETLINK_ROUTE, NL_NONROOT_RECV);
 	register_netdevice_notifier(&rtnetlink_dev_notifier);
 
-	rtnl_register(PF_UNSPEC, RTM_GETLINK, rtnl_getlink, rtnl_dump_ifinfo);
-	rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL);
-	rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETLINK, rtnl_getlink, rtnl_dump_ifinfo, rtnl_calcit);
+	rtnl_register(PF_UNSPEC, RTM_SETLINK, rtnl_setlink, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_NEWLINK, rtnl_newlink, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELLINK, rtnl_dellink, NULL, NULL);
 
-	rtnl_register(PF_UNSPEC, RTM_GETADDR, NULL, rtnl_dump_all);
-	rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all);
+	rtnl_register(PF_UNSPEC, RTM_GETADDR, NULL, rtnl_dump_all, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, NULL);
 }
 
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index 3609eac..ed1bb8c 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -1819,8 +1819,8 @@ static int __init dcbnl_init(void)
 {
 	INIT_LIST_HEAD(&dcb_app_list);
 
-	rtnl_register(PF_UNSPEC, RTM_GETDCB, dcb_doit, NULL);
-	rtnl_register(PF_UNSPEC, RTM_SETDCB, dcb_doit, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETDCB, dcb_doit, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_SETDCB, dcb_doit, NULL, NULL);
 
 	return 0;
 }
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 404fa15..0011eba 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -1419,9 +1419,9 @@ void __init dn_dev_init(void)
 
 	dn_dev_devices_on();
 
-	rtnl_register(PF_DECnet, RTM_NEWADDR, dn_nl_newaddr, NULL);
-	rtnl_register(PF_DECnet, RTM_DELADDR, dn_nl_deladdr, NULL);
-	rtnl_register(PF_DECnet, RTM_GETADDR, NULL, dn_nl_dump_ifaddr);
+	rtnl_register(PF_DECnet, RTM_NEWADDR, dn_nl_newaddr, NULL, NULL);
+	rtnl_register(PF_DECnet, RTM_DELADDR, dn_nl_deladdr, NULL, NULL);
+	rtnl_register(PF_DECnet, RTM_GETADDR, NULL, dn_nl_dump_ifaddr, NULL);
 
 	proc_net_fops_create(&init_net, "decnet_dev", S_IRUGO, &dn_dev_seq_fops);
 
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index 1c74ed3..104324d 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -763,8 +763,8 @@ void __init dn_fib_init(void)
 
 	register_dnaddr_notifier(&dn_fib_dnaddr_notifier);
 
-	rtnl_register(PF_DECnet, RTM_NEWROUTE, dn_fib_rtm_newroute, NULL);
-	rtnl_register(PF_DECnet, RTM_DELROUTE, dn_fib_rtm_delroute, NULL);
+	rtnl_register(PF_DECnet, RTM_NEWROUTE, dn_fib_rtm_newroute, NULL, NULL);
+	rtnl_register(PF_DECnet, RTM_DELROUTE, dn_fib_rtm_delroute, NULL, NULL);
 }
 
 
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 74544bc..2949ca4 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1841,10 +1841,11 @@ void __init dn_route_init(void)
 	proc_net_fops_create(&init_net, "decnet_cache", S_IRUGO, &dn_rt_cache_seq_fops);
 
 #ifdef CONFIG_DECNET_ROUTER
-	rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute, dn_fib_dump);
+	rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute,
+		      dn_fib_dump, NULL);
 #else
 	rtnl_register(PF_DECnet, RTM_GETROUTE, dn_cache_getroute,
-		      dn_cache_dump);
+		      dn_cache_dump, NULL);
 #endif
 }
 
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 5345b0b..fee7550 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1833,8 +1833,8 @@ void __init devinet_init(void)
 
 	rtnl_af_register(&inet_af_ops);
 
-	rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL);
-	rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL);
-	rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr);
+	rtnl_register(PF_INET, RTM_NEWADDR, inet_rtm_newaddr, NULL, NULL);
+	rtnl_register(PF_INET, RTM_DELADDR, inet_rtm_deladdr, NULL, NULL);
+	rtnl_register(PF_INET, RTM_GETADDR, NULL, inet_dump_ifaddr, NULL);
 }
 
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 2252471..92fc5f6 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1124,9 +1124,9 @@ static struct pernet_operations fib_net_ops = {
 
 void __init ip_fib_init(void)
 {
-	rtnl_register(PF_INET, RTM_NEWROUTE, inet_rtm_newroute, NULL);
-	rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL);
-	rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib);
+	rtnl_register(PF_INET, RTM_NEWROUTE, inet_rtm_newroute, NULL, NULL);
+	rtnl_register(PF_INET, RTM_DELROUTE, inet_rtm_delroute, NULL, NULL);
+	rtnl_register(PF_INET, RTM_GETROUTE, NULL, inet_dump_fib, NULL);
 
 	register_pernet_subsys(&fib_net_ops);
 	register_netdevice_notifier(&fib_netdev_notifier);
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 6ffe94c..5ff4765 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -871,7 +871,7 @@ static int inet_diag_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		}
 
 		return netlink_dump_start(idiagnl, skb, nlh,
-					  inet_diag_dump, NULL);
+					  inet_diag_dump, NULL, 0);
 	}
 
 	return inet_diag_get_exact(skb, nlh);
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 30a7763..aae2bd8 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2544,7 +2544,8 @@ int __init ip_mr_init(void)
 		goto add_proto_fail;
 	}
 #endif
-	rtnl_register(RTNL_FAMILY_IPMR, RTM_GETROUTE, NULL, ipmr_rtm_dumproute);
+	rtnl_register(RTNL_FAMILY_IPMR, RTM_GETROUTE,
+		      NULL, ipmr_rtm_dumproute, NULL);
 	return 0;
 
 #ifdef CONFIG_IP_PIMSM_V2
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6a83840..eec0caa 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3312,7 +3312,7 @@ int __init ip_rt_init(void)
 	xfrm_init();
 	xfrm4_init(ip_rt_max_size);
 #endif
-	rtnl_register(PF_INET, RTM_GETROUTE, inet_rtm_getroute, NULL);
+	rtnl_register(PF_INET, RTM_GETROUTE, inet_rtm_getroute, NULL, NULL);
 
 #ifdef CONFIG_SYSCTL
 	register_pernet_subsys(&sysctl_route_ops);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c663a3b..3629ae7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4704,16 +4704,16 @@ int __init addrconf_init(void)
 	if (err < 0)
 		goto errout_af;
 
-	err = __rtnl_register(PF_INET6, RTM_GETLINK, NULL, inet6_dump_ifinfo);
+	err = __rtnl_register(PF_INET6, RTM_GETLINK, NULL, inet6_dump_ifinfo, NULL);
 	if (err < 0)
 		goto errout;
 
 	/* Only the first call to __rtnl_register can fail */
-	__rtnl_register(PF_INET6, RTM_NEWADDR, inet6_rtm_newaddr, NULL);
-	__rtnl_register(PF_INET6, RTM_DELADDR, inet6_rtm_deladdr, NULL);
-	__rtnl_register(PF_INET6, RTM_GETADDR, inet6_rtm_getaddr, inet6_dump_ifaddr);
-	__rtnl_register(PF_INET6, RTM_GETMULTICAST, NULL, inet6_dump_ifmcaddr);
-	__rtnl_register(PF_INET6, RTM_GETANYCAST, NULL, inet6_dump_ifacaddr);
+	__rtnl_register(PF_INET6, RTM_NEWADDR, inet6_rtm_newaddr, NULL, NULL);
+	__rtnl_register(PF_INET6, RTM_DELADDR, inet6_rtm_deladdr, NULL, NULL);
+	__rtnl_register(PF_INET6, RTM_GETADDR, inet6_rtm_getaddr, inet6_dump_ifaddr, NULL);
+	__rtnl_register(PF_INET6, RTM_GETMULTICAST, NULL, inet6_dump_ifmcaddr, NULL);
+	__rtnl_register(PF_INET6, RTM_GETANYCAST, NULL, inet6_dump_ifacaddr, NULL);
 
 	ipv6_addr_label_rtnl_register();
 
diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index c8993e5..f3aa749 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -592,8 +592,8 @@ out:
 
 void __init ipv6_addr_label_rtnl_register(void)
 {
-	__rtnl_register(PF_INET6, RTM_NEWADDRLABEL, ip6addrlbl_newdel, NULL);
-	__rtnl_register(PF_INET6, RTM_DELADDRLABEL, ip6addrlbl_newdel, NULL);
-	__rtnl_register(PF_INET6, RTM_GETADDRLABEL, ip6addrlbl_get, ip6addrlbl_dump);
+	__rtnl_register(PF_INET6, RTM_NEWADDRLABEL, ip6addrlbl_newdel, NULL, NULL);
+	__rtnl_register(PF_INET6, RTM_DELADDRLABEL, ip6addrlbl_newdel, NULL, NULL);
+	__rtnl_register(PF_INET6, RTM_GETADDRLABEL, ip6addrlbl_get, ip6addrlbl_dump, NULL);
 }
 
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 4076a0b..9b257da 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1586,7 +1586,7 @@ int __init fib6_init(void)
 	if (ret)
 		goto out_kmem_cache_create;
 
-	ret = __rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib);
+	ret = __rtnl_register(PF_INET6, RTM_GETROUTE, NULL, inet6_dump_fib, NULL);
 	if (ret)
 		goto out_unregister_subsys;
 out:
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 82a8099..1edfcc9 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1354,7 +1354,7 @@ int __init ip6_mr_init(void)
 		goto add_proto_fail;
 	}
 #endif
-	rtnl_register(RTNL_FAMILY_IP6MR, RTM_GETROUTE, NULL, ip6mr_rtm_dumproute);
+	rtnl_register(RTNL_FAMILY_IP6MR, RTM_GETROUTE, NULL, ip6mr_rtm_dumproute, NULL);
 	return 0;
 #ifdef CONFIG_IPV6_PIMSM_V2
 add_proto_fail:
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f1be5c5..1c49165 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2924,9 +2924,9 @@ int __init ip6_route_init(void)
 		goto xfrm6_init;
 
 	ret = -ENOBUFS;
-	if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL) ||
-	    __rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL) ||
-	    __rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL))
+	if (__rtnl_register(PF_INET6, RTM_NEWROUTE, inet6_rtm_newroute, NULL, NULL) ||
+	    __rtnl_register(PF_INET6, RTM_DELROUTE, inet6_rtm_delroute, NULL, NULL) ||
+	    __rtnl_register(PF_INET6, RTM_GETROUTE, inet6_rtm_getroute, NULL, NULL))
 		goto fib6_rules_init;
 
 	ret = register_netdevice_notifier(&ip6_route_dev_notifier);
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 72d1ac6..dc1528c 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1120,7 +1120,7 @@ ip_set_dump(struct sock *ctnl, struct sk_buff *skb,
 
 	return netlink_dump_start(ctnl, skb, nlh,
 				  ip_set_dump_start,
-				  ip_set_dump_done);
+				  ip_set_dump_done, 0);
 }
 
 /* Add, del and test */
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 30bf8a1..e3e8d4c 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -970,7 +970,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP)
 		return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
-					  ctnetlink_done);
+					  ctnetlink_done, 0);
 
 	err = ctnetlink_parse_zone(cda[CTA_ZONE], &zone);
 	if (err < 0)
@@ -1836,7 +1836,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
 		return netlink_dump_start(ctnl, skb, nlh,
 					  ctnetlink_exp_dump_table,
-					  ctnetlink_exp_done);
+					  ctnetlink_exp_done, 0);
 	}
 
 	err = ctnetlink_parse_zone(cda[CTA_EXPECT_ZONE], &zone);
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index c8f35b5..063bee9 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1665,13 +1665,10 @@ static int netlink_dump(struct sock *sk)
 {
 	struct netlink_sock *nlk = nlk_sk(sk);
 	struct netlink_callback *cb;
-	struct sk_buff *skb;
+	struct sk_buff *skb = NULL;
 	struct nlmsghdr *nlh;
 	int len, err = -ENOBUFS;
-
-	skb = sock_rmalloc(sk, NLMSG_GOODSIZE, 0, GFP_KERNEL);
-	if (!skb)
-		goto errout;
+	int alloc_size;
 
 	mutex_lock(nlk->cb_mutex);
 
@@ -1681,6 +1678,12 @@ static int netlink_dump(struct sock *sk)
 		goto errout_skb;
 	}
 
+	alloc_size = max_t(int, cb->min_dump_alloc, NLMSG_GOODSIZE);
+
+	skb = sock_rmalloc(sk, alloc_size, 0, GFP_KERNEL);
+	if (!skb)
+		goto errout;
+
 	len = cb->dump(skb, cb);
 
 	if (len > 0) {
@@ -1727,7 +1730,8 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 		       const struct nlmsghdr *nlh,
 		       int (*dump)(struct sk_buff *skb,
 				   struct netlink_callback *),
-		       int (*done)(struct netlink_callback *))
+		       int (*done)(struct netlink_callback *),
+		       u16 min_dump_alloc)
 {
 	struct netlink_callback *cb;
 	struct sock *sk;
@@ -1741,6 +1745,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 	cb->dump = dump;
 	cb->done = done;
 	cb->nlh = nlh;
+	cb->min_dump_alloc = min_dump_alloc;
 	atomic_inc(&skb->users);
 	cb->skb = skb;
 
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 1781d99..482fa57 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -525,7 +525,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 		genl_unlock();
 		err = netlink_dump_start(net->genl_sock, skb, nlh,
-					 ops->dumpit, ops->done);
+					 ops->dumpit, ops->done, 0);
 		genl_lock();
 		return err;
 	}
diff --git a/net/phonet/pn_netlink.c b/net/phonet/pn_netlink.c
index 438accb..4ad4bb9 100644
--- a/net/phonet/pn_netlink.c
+++ b/net/phonet/pn_netlink.c
@@ -289,15 +289,15 @@ out:
 
 int __init phonet_netlink_register(void)
 {
-	int err = __rtnl_register(PF_PHONET, RTM_NEWADDR, addr_doit, NULL);
+	int err = __rtnl_register(PF_PHONET, RTM_NEWADDR, addr_doit, NULL, NULL);
 	if (err)
 		return err;
 
 	/* Further __rtnl_register() cannot fail */
-	__rtnl_register(PF_PHONET, RTM_DELADDR, addr_doit, NULL);
-	__rtnl_register(PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit);
-	__rtnl_register(PF_PHONET, RTM_NEWROUTE, route_doit, NULL);
-	__rtnl_register(PF_PHONET, RTM_DELROUTE, route_doit, NULL);
-	__rtnl_register(PF_PHONET, RTM_GETROUTE, NULL, route_dumpit);
+	__rtnl_register(PF_PHONET, RTM_DELADDR, addr_doit, NULL, NULL);
+	__rtnl_register(PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, NULL);
+	__rtnl_register(PF_PHONET, RTM_NEWROUTE, route_doit, NULL, NULL);
+	__rtnl_register(PF_PHONET, RTM_DELROUTE, route_doit, NULL, NULL);
+	__rtnl_register(PF_PHONET, RTM_GETROUTE, NULL, route_dumpit, NULL);
 	return 0;
 }
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 14b42f4..c857763 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1120,9 +1120,9 @@ nlmsg_failure:
 
 static int __init tc_action_init(void)
 {
-	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL);
-	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action);
+	rtnl_register(PF_UNSPEC, RTM_NEWACTION, tc_ctl_action, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELACTION, tc_ctl_action, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETACTION, tc_ctl_action, tc_dump_action, NULL);
 
 	return 0;
 }
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index bb2c523..9563887 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -610,10 +610,10 @@ EXPORT_SYMBOL(tcf_exts_dump_stats);
 
 static int __init tc_filter_init(void)
 {
-	rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL);
+	rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_ctl_tfilter, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_ctl_tfilter, NULL, NULL);
 	rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_ctl_tfilter,
-						 tc_dump_tfilter);
+		      tc_dump_tfilter, NULL);
 
 	return 0;
 }
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 7490f3f..7870a92 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1794,12 +1794,12 @@ static int __init pktsched_init(void)
 	register_qdisc(&pfifo_head_drop_qdisc_ops);
 	register_qdisc(&mq_qdisc_ops);
 
-	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL);
-	rtnl_register(PF_UNSPEC, RTM_GETQDISC, tc_get_qdisc, tc_dump_qdisc);
-	rtnl_register(PF_UNSPEC, RTM_NEWTCLASS, tc_ctl_tclass, NULL);
-	rtnl_register(PF_UNSPEC, RTM_DELTCLASS, tc_ctl_tclass, NULL);
-	rtnl_register(PF_UNSPEC, RTM_GETTCLASS, tc_ctl_tclass, tc_dump_tclass);
+	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETQDISC, tc_get_qdisc, tc_dump_qdisc, NULL);
+	rtnl_register(PF_UNSPEC, RTM_NEWTCLASS, tc_ctl_tclass, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_DELTCLASS, tc_ctl_tclass, NULL, NULL);
+	rtnl_register(PF_UNSPEC, RTM_GETTCLASS, tc_ctl_tclass, tc_dump_tclass, NULL);
 
 	return 0;
 }
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 5d1d60d..d31e914 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2296,7 +2296,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		if (link->dump == NULL)
 			return -EINVAL;
 
-		return netlink_dump_start(net->xfrm.nlsk, skb, nlh, link->dump, link->done);
+		return netlink_dump_start(net->xfrm.nlsk, skb, nlh, link->dump, link->done, 0);
 	}
 
 	err = nlmsg_parse(nlh, xfrm_msg_min[type], attrs, XFRMA_MAX,


^ permalink raw reply related

* Re: Scalability of interface creation and deletion
From: Octavian Purdila @ 2011-05-09 21:46 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Alex Bligh, netdev
In-Reply-To: <1304783684.9216.2.camel@edumazet-laptop>

On Sat, May 7, 2011 at 6:54 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:

>
> synchronize_rcu() calls are not consuming cpu, they just _wait_
> rcu grace period.
>
> I suggest you read Documentation/RCU files if you really want to :)
>
> If you want to check how expensive it is, its quite easy:
> add a trace in synchronize_net()
>
<snip>

I proposed adding a "wait" software counter to perf [1] a while ago,
which would allow people identify sync_rcu hotspots:

http://marc.info/?l=linux-kernel&m=129188584110162

I don't know how much visibility it got, so given this context, I
thought of bringing it up again :)

^ permalink raw reply

* [PATCH v2] net: group FCoE related feature flags
From: Yi Zou @ 2011-05-09 21:53 UTC (permalink / raw)
  To: netdev; +Cc: mirq-linux, jeffrey.t.kirsher, devel

Michał Mirosław's patch (http://patchwork.ozlabs.org/patch/94421/) fixes the
issue (http://patchwork.ozlabs.org/patch/94188/) about not populating FCoE related
flags correctly on vlan devices. However, only NETIF_F_FCOE_CRC is part of the
NETIF_F_ALL_TX_OFFLOADS right now, where weed NETIF_F_FCOE_MTU and NETIF_F_FSO
as well.

Therefore, add NETIF_F_ALL_FCOE to indicate feature flags used by FCoE TX offloads.
These include NETIF_F_FCOE_CRC, NETIF_F_FCOE_MTU, and NETIF_F_FSO and add them to
be part of NETIF_F_ALL_TX_OFFLOADS. This would eventually make sure all FCoE needed
flags are populated properly to vlan devices.

Signed-off-by: Yi Zou <yi.zou@intel.com>
---

 include/linux/netdevice.h |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e7244ed..00d650c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1097,10 +1097,14 @@ struct net_device {
 
 #define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
 
+#define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
+				 NETIF_F_FSO)
+
 #define NETIF_F_ALL_TX_OFFLOADS	(NETIF_F_ALL_CSUM | NETIF_F_SG | \
 				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
 				 NETIF_F_HIGHDMA | \
-				 NETIF_F_SCTP_CSUM | NETIF_F_FCOE_CRC)
+				 NETIF_F_SCTP_CSUM | \
+				 NETIF_F_ALL_FCOE)
 
 	/*
 	 * If one device supports one of these features, then enable them


^ permalink raw reply related

* Re: [PATCH] pci, e1000e: Add and use __pci_disable_link_state
From: Jesse Barnes @ 2011-05-09 21:35 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, PJ Waskiewicz, Alex Duyck, John Ronciak,
	Rafael J. Wysocki, Kenji Kaneshige, Matthew Garrett,
	Naga Chumbalkar, e1000-devel, netdev, linux-kernel, linux-pci,
	Andrew Morton
In-Reply-To: <4DC6E6E8.9040306@kernel.org>

On Sun, 08 May 2011 11:54:32 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> 
> Need to use it in _e1000e_disable_aspm.
> when aer happens,
> pci_walk_bus already have down_read(&pci_bus_sem)...
> then report_slot_reset
>         ==> e1000_io_slot_reset
>                 ==> e1000e_disable_aspm
>                         ==> pci_disable_link_state...
> 
> We can not use pci_disable_link_state, and it will try to hold pci_bus_sem again.
> 
> Try to have __pci_disable_link_state that will not need to hold pci_bus_sem.

What about the other callers of e1000e_disable_aspm?  Do they already
have the lock held or is it just reset that needs the already locked
version?

> +extern void __pci_disable_link_state(struct pci_dev *pdev, int state);
>  extern void pcie_clear_aspm(void);
>  extern void pcie_no_aspm(void);
>  #else

pci_disable_link_state_locked would be more descriptive and would match
other usage in the kernel.

Thanks,

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: 8390 drivers (was: Re: Debian kernel 2.6.38-5)
From: Christian T. Steigies @ 2011-05-09 21:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Finn Thain, Thorsten Glaser, linux-m68k, Stephen Hemminger,
	David S. Miller, netdev
In-Reply-To: <BANLkTimG+3jqLiLaA_NVEpGg3LFtvK4bQQ@mail.gmail.com>

On Mon, May 09, 2011 at 10:38:18PM +0200, Geert Uytterhoeven wrote:
> On Mon, May 9, 2011 at 22:25, Christian T. Steigies <cts@debian.org> wrote:
> > On Mon, May 09, 2011 at 09:16:16AM +0200, Geert Uytterhoeven wrote:
> >> On Mon, May 9, 2011 at 01:14, Finn Thain <fthain@telegraphics.com.au> wrote:
> >> > On Sun, 8 May 2011, Christian T. Steigies wrote:
> >> >> PS 2.6.28 did not boot: kernel too old. When was TLS introduced? I'll try to
> >> >> apply the patch you mentioned in your other message.
> >> >
> >> > I sometimes test network cards with busybox. It can be built without
> >> > linking in glibc and doesn't need TLS.
> >> >
> >> >
> >> >> [  130.870000] eth0: trigger_send() called with the transmitter busy.
> >> >> [  132.240000] ------------[ cut here ]------------
> >> >> [  132.240000] WARNING: at net/sched/sch_generic.c:256 dev_watchdog+0x1ac/0x1cc()
> >> >> [  132.250000] NETDEV WATCHDOG: eth0 (): transmit queue 0 timed out
> >> >
> >> > Looks a lot like this problem:
> >> >
> >> > http://patchwork.ozlabs.org/patch/27774/
> >>
> >> Yeah, that's a very likely culprit. Thx!
> >
> > I applied the following patch and it works! apt-get update that is, there is
> > no sshd on this freshly installed system yet...
> >
> > I also changed this line to have 4 underscores at the beginning:
> >
> >        { 0xec940559, "____alloc_ei_netdev" },
> 
> In which file is that?

zorro8390.mod.c
 
> > it does not show up in git diff, so I am not sure if this is a leftover from
> > a previous checkout, or if it is also needed. And if it is needed, if it
> > should be two underscores or four or none, and if so why or why not...
> > Looking at the other drivers, there does not seem to be a lot of consistency?
> 
> From this, I guess hydra and ne-h8300 are also affected, as they
> include lib8390.c
> theirselves and are linked with 8390.o?
> 
> > diff --git a/drivers/net/zorro8390.c b/drivers/net/zorro8390.c
> > index b78a38d9..8c7c522 100644
> > --- a/drivers/net/zorro8390.c
> > +++ b/drivers/net/zorro8390.c
> > @@ -126,7 +126,7 @@ static int __devinit zorro8390_init_one(struct zorro_dev *z,
> >
> >     board = z->resource.start;
> >     ioaddr = board+cards[i].offset;
> > -    dev = alloc_ei_netdev();
> > +    dev = ____alloc_ei_netdev(0);
> >     if (!dev)
> >        return -ENOMEM;
> >     if (!request_mem_region(ioaddr, NE_IO_EXTENT*2, DRV_NAME)) {
> > @@ -146,15 +146,15 @@ static int __devinit zorro8390_init_one(struct zorro_dev *z,
> >  static const struct net_device_ops zorro8390_netdev_ops = {
> >        .ndo_open               = zorro8390_open,
> >        .ndo_stop               = zorro8390_close,
> > -       .ndo_start_xmit         = ei_start_xmit,
> > -       .ndo_tx_timeout         = ei_tx_timeout,
> > -       .ndo_get_stats          = ei_get_stats,
> > -       .ndo_set_multicast_list = ei_set_multicast_list,
> > +       .ndo_start_xmit         = __ei_start_xmit,
> > +       .ndo_tx_timeout         = __ei_tx_timeout,
> > +       .ndo_get_stats          = __ei_get_stats,
> > +       .ndo_set_multicast_list = __ei_set_multicast_list,
> >        .ndo_validate_addr      = eth_validate_addr,
> >        .ndo_set_mac_address    = eth_mac_addr,
> >        .ndo_change_mtu         = eth_change_mtu,
> >  #ifdef CONFIG_NET_POLL_CONTROLLER
> > -       .ndo_poll_controller    = ei_poll,
> > +       .ndo_poll_controller    = __ei_poll,
> >  #endif
> >  };
> 
> And 8390.o can be removed from drivers/net/Makefile?

I did not change that and it still works, modprobe zorro8390 did not load
8390. The speed is rather slow, though, 10kbit/sec seems to be the max
speed. I have not been able to log in via ssh yet, maybe after it has
finished downloading packages.

Christian

^ permalink raw reply

* RE: [PATCH] net: group FCoE related feature flags
From: Zou, Yi @ 2011-05-09 21:15 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev@vger.kernel.org, mirq-linux@rere.qmqm.pl,
	Kirsher, Jeffrey T, devel@open-fcoe.org
In-Reply-To: <1304972279.2888.106.camel@bwh-desktop>

> 
> On Mon, 2011-05-09 at 12:24 -0700, Yi Zou wrote:
> > Michał Mirosław's patch (http://patchwork.ozlabs.org/patch/94421/)
> fixes the
> > issue (http://patchwork.ozlabs.org/patch/94188/) about not populating
> FCoE related
> > flags correctly on vlan devices. However, only NETIF_F_FCOE_CRC is part
> of the
> > NETIF_F_ALL_TX_OFFLOADS right now, where weed NETIF_F_FCOE_MTU and
> NETIF_F_FSO
> > as well.
> >
> > Therefore, add NETIF_F_ALL_FCOE to indicate feature flags used by FCoE
> TX offloads.
> > These include NETIF_F_FCOE_CRC, NETIF_F_FCOE_MTU, and NETIF_F_FSO. They
> are not part
> > of the NETIF_F_ALL_TX_OFFLOADS. This would eventually make sure all
> FCoE needed
> > flags are populated properly to vlan devices.
> >
> > Signed-off-by: Yi Zou <yi.zou@intel.com>
> > ---
> >
> >  include/linux/netdevice.h |    6 +++++-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index e7244ed..40b3df8 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -1097,10 +1097,14 @@ struct net_device {
> >
> >  #define NETIF_F_ALL_TSO 	(NETIF_F_TSO | NETIF_F_TSO6 |
> NETIF_F_TSO_ECN)
> >
> > +#define NETIF_F_ALL_FCOE	(NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
> > +				 NETIF_F_UFO)
> 
> UFO?  Surely FSO, like you wrote in the commit message...
> 
> Ben.
Yikes...how I missed that...thanks, will resend.

yi

> 
> >  #define NETIF_F_ALL_TX_OFFLOADS	(NETIF_F_ALL_CSUM | NETIF_F_SG | \
> >  				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
> >  				 NETIF_F_HIGHDMA | \
> > -				 NETIF_F_SCTP_CSUM | NETIF_F_FCOE_CRC)
> > +				 NETIF_F_SCTP_CSUM | \
> > +				 NETIF_F_ALL_FCOE)
> >
> >  	/*
> >  	 * If one device supports one of these features, then enable them
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: Testing interface removal speedup patches from Eric Dumazet.
From: Ben Greear @ 2011-05-09 21:03 UTC (permalink / raw)
  To: Alex Bligh; +Cc: netdev, Eric Dumazet
In-Reply-To: <39CEA2B6BDF36C1FA4B1EF64@nimrod.local>

On 05/09/2011 01:24 PM, Alex Bligh wrote:
> Ben,
>
> --On 9 May 2011 12:42:39 -0700 Ben Greear <greearb@candelatech.com> wrote:
>
>> I use HZ of 1000, btw.
>>
>> I killed udev, haldaemon..seemed to be just my stuff running.
>>
>> I don't see any 'ifup' running with these things dead...
>>
>> If you want to post your script, I can run it on my
>> machine...
>
> See below - run (on ubuntu) by
> service udev stop
> unshare -n bash
> ./ifseq -n 500
>
> The unshare seems sufficient to stop any remaining udev listeners getting
> wind of interface creation/deletion.

You were not adding the 'up' flag
on creation...and my script was.  I changed my script to not do
up and it reports similar numbers to yours...

Also, deleting with IPs added is likely to be a lot more work
than deleting w/out, as your script does.

Created 500 veth in 12.954143 seconds (0.025908286 per interface).
Added IP addresses in 28.35679 seconds (0.05671358 per addr).
Deleted 500 veth in 21.503877 seconds. (0.043007754 per interface)

Created 100 veth in 1.06471 seconds (0.0106471 per interface).
Added IP addresses in 2.006418 seconds (0.02006418 per addr).
Deleted 100 veth in 2.983303 seconds. (0.02983303 per interface)


 From your script:

[root@lec2010-ath9k-1 lanforge]# /mnt/b32/greearb/tmp/testifs.bash -n 500
Mon May 9 13:53:31 PDT 2011 creating 500 interfaces
Mon May 9 13:53:44 PDT 2011 done

real	0m12.756s
user	0m0.732s
sys	0m11.943s
Mon May 9 13:53:44 PDT 2011 deleting 500 interfaces
Mon May 9 13:54:02 PDT 2011 done

real	0m18.071s
user	0m1.002s
sys	0m8.376s
[root@lec2010-ath9k-1 lanforge]# /mnt/b32/greearb/tmp/testifs.bash -n 100
Mon May 9 13:54:33 PDT 2011 creating 100 interfaces
Mon May 9 13:54:34 PDT 2011 done

real	0m1.048s
user	0m0.092s
sys	0m0.943s
Mon May 9 13:54:34 PDT 2011 deleting 100 interfaces
Mon May 9 13:54:36 PDT 2011 done

real	0m2.219s
user	0m0.134s
sys	0m0.794s

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply

* Re: [PATCH 0/7] Network namespace manipulation with file descriptors
From: David Miller @ 2011-05-09 20:55 UTC (permalink / raw)
  To: ebiederm
  Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, hadi,
	daniel.lezcano, containers, renatowestphal
In-Reply-To: <m1bozbtx0y.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 09 May 2011 13:54:37 -0700

> Do you know if there are any plans to support sendmmsg on parisc for
> 2.6.40?  That could get sticky as the parisc tree has a bunch of syscall
> catchup support they are doing now.

If you would like, you could send me a patch for net-next-2.6 that adds
the sendmmsg syscall entries and hooks for all the platforms not taken
care of right now.

^ permalink raw reply

* Re: [PATCH 0/7] Network namespace manipulation with file descriptors
From: Eric W. Biederman @ 2011-05-09 20:54 UTC (permalink / raw)
  To: David Miller
  Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, hadi,
	daniel.lezcano, containers, renatowestphal
In-Reply-To: <20110509.134007.116389415.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Mon, 09 May 2011 12:59:55 -0700
>
>> David Miller <davem@davemloft.net> writes:
>> 
>>> The networking bits look OK to me:
>>>
>>> Acked-by: David S. Miller <davem@davemloft.net>
>> 
>> Are you merging sendmmsg through the netdev tree?
>
> Yes.
>
>> The conflicts on syscall syscall numbers are an unfortunate pain.
>
> The way we've solved this before is the tree that cares pulls in
> the net-next-2.6 tree to resolve the conflict.

If that is the precedent that then that makes sense.  You are good
at not rewinding net-next so I don't expect there will be a problem
there.

Do you know if there are any plans to support sendmmsg on parisc for
2.6.40?  That could get sticky as the parisc tree has a bunch of syscall
catchup support they are doing now.

Eric

^ permalink raw reply

* Re: [PATCH 0/7] Network namespace manipulation with file descriptors
From: David Miller @ 2011-05-09 20:40 UTC (permalink / raw)
  To: ebiederm
  Cc: linux-arch, linux-kernel, netdev, linux-fsdevel, hadi,
	daniel.lezcano, containers, renatowestphal
In-Reply-To: <m1oc3btzk4.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 09 May 2011 12:59:55 -0700

> David Miller <davem@davemloft.net> writes:
> 
>> The networking bits look OK to me:
>>
>> Acked-by: David S. Miller <davem@davemloft.net>
> 
> Are you merging sendmmsg through the netdev tree?

Yes.

> The conflicts on syscall syscall numbers are an unfortunate pain.

The way we've solved this before is the tree that cares pulls in
the net-next-2.6 tree to resolve the conflict.

^ permalink raw reply

* 8390 drivers (was: Re: Debian kernel 2.6.38-5)
From: Geert Uytterhoeven @ 2011-05-09 20:38 UTC (permalink / raw)
  To: Christian T. Steigies
  Cc: Finn Thain, Thorsten Glaser, linux-m68k, Stephen Hemminger,
	David S. Miller, netdev

On Mon, May 9, 2011 at 22:25, Christian T. Steigies <cts@debian.org> wrote:
> On Mon, May 09, 2011 at 09:16:16AM +0200, Geert Uytterhoeven wrote:
>> On Mon, May 9, 2011 at 01:14, Finn Thain <fthain@telegraphics.com.au> wrote:
>> > On Sun, 8 May 2011, Christian T. Steigies wrote:
>> >> PS 2.6.28 did not boot: kernel too old. When was TLS introduced? I'll try to
>> >> apply the patch you mentioned in your other message.
>> >
>> > I sometimes test network cards with busybox. It can be built without
>> > linking in glibc and doesn't need TLS.
>> >
>> >
>> >> [  130.870000] eth0: trigger_send() called with the transmitter busy.
>> >> [  132.240000] ------------[ cut here ]------------
>> >> [  132.240000] WARNING: at net/sched/sch_generic.c:256 dev_watchdog+0x1ac/0x1cc()
>> >> [  132.250000] NETDEV WATCHDOG: eth0 (): transmit queue 0 timed out
>> >
>> > Looks a lot like this problem:
>> >
>> > http://patchwork.ozlabs.org/patch/27774/
>>
>> Yeah, that's a very likely culprit. Thx!
>
> I applied the following patch and it works! apt-get update that is, there is
> no sshd on this freshly installed system yet...
>
> I also changed this line to have 4 underscores at the beginning:
>
>        { 0xec940559, "____alloc_ei_netdev" },

In which file is that?

> it does not show up in git diff, so I am not sure if this is a leftover from
> a previous checkout, or if it is also needed. And if it is needed, if it
> should be two underscores or four or none, and if so why or why not...
> Looking at the other drivers, there does not seem to be a lot of consistency?

From this, I guess hydra and ne-h8300 are also affected, as they
include lib8390.c
theirselves and are linked with 8390.o?

> diff --git a/drivers/net/zorro8390.c b/drivers/net/zorro8390.c
> index b78a38d9..8c7c522 100644
> --- a/drivers/net/zorro8390.c
> +++ b/drivers/net/zorro8390.c
> @@ -126,7 +126,7 @@ static int __devinit zorro8390_init_one(struct zorro_dev *z,
>
>     board = z->resource.start;
>     ioaddr = board+cards[i].offset;
> -    dev = alloc_ei_netdev();
> +    dev = ____alloc_ei_netdev(0);
>     if (!dev)
>        return -ENOMEM;
>     if (!request_mem_region(ioaddr, NE_IO_EXTENT*2, DRV_NAME)) {
> @@ -146,15 +146,15 @@ static int __devinit zorro8390_init_one(struct zorro_dev *z,
>  static const struct net_device_ops zorro8390_netdev_ops = {
>        .ndo_open               = zorro8390_open,
>        .ndo_stop               = zorro8390_close,
> -       .ndo_start_xmit         = ei_start_xmit,
> -       .ndo_tx_timeout         = ei_tx_timeout,
> -       .ndo_get_stats          = ei_get_stats,
> -       .ndo_set_multicast_list = ei_set_multicast_list,
> +       .ndo_start_xmit         = __ei_start_xmit,
> +       .ndo_tx_timeout         = __ei_tx_timeout,
> +       .ndo_get_stats          = __ei_get_stats,
> +       .ndo_set_multicast_list = __ei_set_multicast_list,
>        .ndo_validate_addr      = eth_validate_addr,
>        .ndo_set_mac_address    = eth_mac_addr,
>        .ndo_change_mtu         = eth_change_mtu,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
> -       .ndo_poll_controller    = ei_poll,
> +       .ndo_poll_controller    = __ei_poll,
>  #endif
>  };

And 8390.o can be removed from drivers/net/Makefile?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox