Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 05/10] net/mlx4_core: Fix HW2SW_EQ to conform to the firmware spec
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The driver incorrectly assigned an out-mailbox to this command,
and used an opcode modifier = 0, which is a reserved value (it
should use opcode modifier = 1).

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c           |  2 +-
 drivers/net/ethernet/mellanox/mlx4/eq.c            | 26 ++++------------------
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  | 16 ++++---------
 3 files changed, 9 insertions(+), 35 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 2b48932..928b706 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1221,7 +1221,7 @@ static struct mlx4_cmd_info cmd_info[] = {
 	{
 		.opcode = MLX4_CMD_HW2SW_EQ,
 		.has_inbox = false,
-		.has_outbox = true,
+		.has_outbox = false,
 		.out_is_imm = false,
 		.encode_slave_id = true,
 		.verify = NULL,
diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 4df006d..264bc15 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -868,12 +868,10 @@ static int mlx4_SW2HW_EQ(struct mlx4_dev *dev, struct mlx4_cmd_mailbox *mailbox,
 			MLX4_CMD_WRAPPED);
 }
 
-static int mlx4_HW2SW_EQ(struct mlx4_dev *dev, struct mlx4_cmd_mailbox *mailbox,
-			 int eq_num)
+static int mlx4_HW2SW_EQ(struct mlx4_dev *dev,  int eq_num)
 {
-	return mlx4_cmd_box(dev, 0, mailbox->dma, eq_num,
-			    0, MLX4_CMD_HW2SW_EQ, MLX4_CMD_TIME_CLASS_A,
-			    MLX4_CMD_WRAPPED);
+	return mlx4_cmd(dev, 0, eq_num, 1, MLX4_CMD_HW2SW_EQ,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_WRAPPED);
 }
 
 static int mlx4_num_eq_uar(struct mlx4_dev *dev)
@@ -1046,7 +1044,6 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 			 struct mlx4_eq *eq)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
-	struct mlx4_cmd_mailbox *mailbox;
 	int err;
 	int i;
 	/* CX3 is capable of extending the CQE/EQE from 32 to 64 bytes, with
@@ -1054,24 +1051,10 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 	 */
 	int npages = PAGE_ALIGN(dev->caps.eqe_size  * eq->nent) / PAGE_SIZE;
 
-	mailbox = mlx4_alloc_cmd_mailbox(dev);
-	if (IS_ERR(mailbox))
-		return;
-
-	err = mlx4_HW2SW_EQ(dev, mailbox, eq->eqn);
+	err = mlx4_HW2SW_EQ(dev, eq->eqn);
 	if (err)
 		mlx4_warn(dev, "HW2SW_EQ failed (%d)\n", err);
 
-	if (0) {
-		mlx4_dbg(dev, "Dumping EQ context %02x:\n", eq->eqn);
-		for (i = 0; i < sizeof (struct mlx4_eq_context) / 4; ++i) {
-			if (i % 4 == 0)
-				pr_cont("[%02x] ", i * 4);
-			pr_cont(" %08x", be32_to_cpup(mailbox->buf + i * 4));
-			if ((i + 1) % 4 == 0)
-				pr_cont("\n");
-		}
-	}
 	synchronize_irq(eq->irq);
 	tasklet_disable(&eq->tasklet_ctx.task);
 
@@ -1083,7 +1066,6 @@ static void mlx4_free_eq(struct mlx4_dev *dev,
 
 	kfree(eq->page_list);
 	mlx4_bitmap_free(&priv->eq_table.bitmap, eq->eqn, MLX4_USE_RR);
-	mlx4_free_cmd_mailbox(dev, mailbox);
 }
 
 static void mlx4_free_irqs(struct mlx4_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index 3e93879..79feeb6 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -4677,7 +4677,6 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
 	int state;
 	LIST_HEAD(tlist);
 	int eqn;
-	struct mlx4_cmd_mailbox *mailbox;
 
 	err = move_all_busy(dev, slave, RES_EQ);
 	if (err)
@@ -4703,20 +4702,13 @@ static void rem_slave_eqs(struct mlx4_dev *dev, int slave)
 					break;
 
 				case RES_EQ_HW:
-					mailbox = mlx4_alloc_cmd_mailbox(dev);
-					if (IS_ERR(mailbox)) {
-						cond_resched();
-						continue;
-					}
-					err = mlx4_cmd_box(dev, slave, 0,
-							   eqn & 0xff, 0,
-							   MLX4_CMD_HW2SW_EQ,
-							   MLX4_CMD_TIME_CLASS_A,
-							   MLX4_CMD_NATIVE);
+					err = mlx4_cmd(dev, slave, eqn & 0xff,
+						       1, MLX4_CMD_HW2SW_EQ,
+						       MLX4_CMD_TIME_CLASS_A,
+						       MLX4_CMD_NATIVE);
 					if (err)
 						mlx4_dbg(dev, "rem_slave_eqs: failed to move slave %d eqs %d to SW ownership\n",
 							 slave, eqn);
-					mlx4_free_cmd_mailbox(dev, mailbox);
 					atomic_dec(&eq->mtt->ref_count);
 					state = RES_EQ_RESERVED;
 					break;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 04/10] net/mlx4_core: Adjust command timeouts to conform to the firmware spec
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

The firmware spec states that the timeout for all commands should be 60 seconds.

In the past, the spec indicated that there were several classes of timeout
(short, medium, and long).  The driver has these different timeout classes.
We leave the class differentiation in the driver as-is (to protect against any
future spec changes), but set the timeout for all classes to be 60 seconds.

In addition, we fix a few commands which had hard-coded numeric timeouts specified.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c | 19 ++++++++++---------
 drivers/net/ethernet/mellanox/mlx4/mr.c |  4 ++--
 include/linux/mlx4/cmd.h                |  6 +++---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 2aa7c23..0c90d10 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -1774,8 +1774,8 @@ int mlx4_INIT_HCA(struct mlx4_dev *dev, struct mlx4_init_hca_param *param)
 		MLX4_PUT(inbox, parser_params,	INIT_HCA_VXLAN_OFFSET);
 	}
 
-	err = mlx4_cmd(dev, mailbox->dma, 0, 0, MLX4_CMD_INIT_HCA, 10000,
-		       MLX4_CMD_NATIVE);
+	err = mlx4_cmd(dev, mailbox->dma, 0, 0, MLX4_CMD_INIT_HCA,
+		       MLX4_CMD_TIME_CLASS_C, MLX4_CMD_NATIVE);
 
 	if (err)
 		mlx4_err(dev, "INIT_HCA returns %d\n", err);
@@ -2029,7 +2029,7 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 	if (dev->caps.port_mask[port] != MLX4_PORT_TYPE_IB) {
 		if (priv->mfunc.master.init_port_ref[port] == 1) {
 			err = mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
-				       1000, MLX4_CMD_NATIVE);
+				       MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 			if (err)
 				return err;
 		}
@@ -2040,7 +2040,7 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 			if (!priv->mfunc.master.qp0_state[port].qp0_active &&
 			    priv->mfunc.master.qp0_state[port].port_active) {
 				err = mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
-					       1000, MLX4_CMD_NATIVE);
+					       MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 				if (err)
 					return err;
 				priv->mfunc.master.slave_state[slave].init_port_mask &= ~(1 << port);
@@ -2055,15 +2055,15 @@ int mlx4_CLOSE_PORT_wrapper(struct mlx4_dev *dev, int slave,
 
 int mlx4_CLOSE_PORT(struct mlx4_dev *dev, int port)
 {
-	return mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT, 1000,
-			MLX4_CMD_WRAPPED);
+	return mlx4_cmd(dev, 0, port, 0, MLX4_CMD_CLOSE_PORT,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_WRAPPED);
 }
 EXPORT_SYMBOL_GPL(mlx4_CLOSE_PORT);
 
 int mlx4_CLOSE_HCA(struct mlx4_dev *dev, int panic)
 {
-	return mlx4_cmd(dev, 0, 0, panic, MLX4_CMD_CLOSE_HCA, 1000,
-			MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0, panic, MLX4_CMD_CLOSE_HCA,
+			MLX4_CMD_TIME_CLASS_C, MLX4_CMD_NATIVE);
 }
 
 struct mlx4_config_dev {
@@ -2202,7 +2202,8 @@ int mlx4_SET_ICM_SIZE(struct mlx4_dev *dev, u64 icm_size, u64 *aux_pages)
 int mlx4_NOP(struct mlx4_dev *dev)
 {
 	/* Input modifier of 0x1f means "finish as soon as possible." */
-	return mlx4_cmd(dev, 0, 0x1f, 0, MLX4_CMD_NOP, 100, MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0x1f, 0, MLX4_CMD_NOP, MLX4_CMD_TIME_CLASS_A,
+			MLX4_CMD_NATIVE);
 }
 
 int mlx4_get_phys_port_id(struct mlx4_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 8dbdf1d..d21e884 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -1155,7 +1155,7 @@ EXPORT_SYMBOL_GPL(mlx4_fmr_free);
 
 int mlx4_SYNC_TPT(struct mlx4_dev *dev)
 {
-	return mlx4_cmd(dev, 0, 0, 0, MLX4_CMD_SYNC_TPT, 1000,
-			MLX4_CMD_NATIVE);
+	return mlx4_cmd(dev, 0, 0, 0, MLX4_CMD_SYNC_TPT,
+			MLX4_CMD_TIME_CLASS_A, MLX4_CMD_NATIVE);
 }
 EXPORT_SYMBOL_GPL(mlx4_SYNC_TPT);
diff --git a/include/linux/mlx4/cmd.h b/include/linux/mlx4/cmd.h
index c989442..ae95adc 100644
--- a/include/linux/mlx4/cmd.h
+++ b/include/linux/mlx4/cmd.h
@@ -165,9 +165,9 @@ enum {
 };
 
 enum {
-	MLX4_CMD_TIME_CLASS_A	= 10000,
-	MLX4_CMD_TIME_CLASS_B	= 10000,
-	MLX4_CMD_TIME_CLASS_C	= 10000,
+	MLX4_CMD_TIME_CLASS_A	= 60000,
+	MLX4_CMD_TIME_CLASS_B	= 60000,
+	MLX4_CMD_TIME_CLASS_C	= 60000,
 };
 
 enum {
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 10/10] net/mlx4_core: Fix device capabilities dumping
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Or Gerlitz <ogerlitz@mellanox.com>

We are dumping device capabilities which are supported both by the
firmware and the driver. Align the array that holds the capability
strings with this practice.

Reported-by: Yuval Shaia <yuval.shaia@oracle.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/fw.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/fw.c b/drivers/net/ethernet/mellanox/mlx4/fw.c
index 0c90d10..0ab81a9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx4/fw.c
@@ -84,13 +84,10 @@ static void dump_dev_cap_flags(struct mlx4_dev *dev, u64 flags)
 		[ 1] = "UC transport",
 		[ 2] = "UD transport",
 		[ 3] = "XRC transport",
-		[ 4] = "reliable multicast",
-		[ 5] = "FCoIB support",
 		[ 6] = "SRQ support",
 		[ 7] = "IPoIB checksum offload",
 		[ 8] = "P_Key violation counter",
 		[ 9] = "Q_Key violation counter",
-		[10] = "VMM",
 		[12] = "Dual Port Different Protocol (DPDP) support",
 		[15] = "Big LSO headers",
 		[16] = "MW support",
@@ -99,12 +96,11 @@ static void dump_dev_cap_flags(struct mlx4_dev *dev, u64 flags)
 		[19] = "Raw multicast support",
 		[20] = "Address vector port checking support",
 		[21] = "UD multicast support",
-		[24] = "Demand paging support",
-		[25] = "Router support",
 		[30] = "IBoE support",
 		[32] = "Unicast loopback support",
 		[34] = "FCS header control",
-		[38] = "Wake On LAN support",
+		[37] = "Wake On LAN (port1) support",
+		[38] = "Wake On LAN (port2) support",
 		[40] = "UDP RSS support",
 		[41] = "Unicast VEP steering support",
 		[42] = "Multicast VEP steering support",
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 09/10] net/mlx4: Fix memory corruption in mlx4_MAD_IFC_wrapper
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin, Matan Barak
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Matan Barak <matanb@mellanox.com>

Fix a memory corruption at mlx4_MAD_IFC_wrapper.

A table of size dev->caps.pkey_table_len[port]*sizeof(*table)
was allocated, but get_full_pkey_table() assumes that the number
of entries in the table is a multiplication of 32 (which isn't always
correct).

Fixes: 0a9a018 ('mlx4: MAD_IFC paravirtualization')
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 928b706..154effb 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -901,7 +901,9 @@ static int mlx4_MAD_IFC_wrapper(struct mlx4_dev *dev, int slave,
 				index = be32_to_cpu(smp->attr_mod);
 				if (port < 1 || port > dev->caps.num_ports)
 					return -EINVAL;
-				table = kcalloc(dev->caps.pkey_table_len[port], sizeof *table, GFP_KERNEL);
+				table = kcalloc((dev->caps.pkey_table_len[port] / 32) + 1,
+						sizeof(*table) * 32, GFP_KERNEL);
+
 				if (!table)
 					return -ENOMEM;
 				/* need to get the full pkey table because the paravirtualized
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Struct mlx4_vhcr was implicitly padded by the gcc compiler.
This commit makes that padding explicit, to prevent issues with
changing compilers. Note that we added the padding dword (rather
than simply packing the structure) in order to maintain compatibility
with previous kernels.


Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
index 096a81c..595e18a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
@@ -196,13 +196,14 @@ struct mlx4_vhcr {
 struct mlx4_vhcr_cmd {
 	__be64 in_param;
 	__be32 in_modifier;
+	u32 reserved1;
 	__be64 out_param;
 	__be16 token;
 	u16 reserved;
 	u8 status;
 	u8 flags;
 	__be16 opcode;
-};
+} __packed;
 
 struct mlx4_cmd_info {
 	u16 opcode;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 07/10] net/mlx4_core: Remove duplicate code line from procedure mlx4_bf_alloc
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

mlx4_bf_alloc had an unnecessary/duplicate code line. Did no harm,
but not good practice.

Reported by the Mellanox Beijing team.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/pd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/pd.c b/drivers/net/ethernet/mellanox/mlx4/pd.c
index a42b4c0..609c59d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/pd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/pd.c
@@ -214,7 +214,6 @@ int mlx4_bf_alloc(struct mlx4_dev *dev, struct mlx4_bf *bf, int node)
 		list_add(&uar->bf_list, &priv->bf_list);
 	}
 
-	bf->uar = uar;
 	idx = ffz(uar->free_bf_bmap);
 	uar->free_bf_bmap |= 1 << idx;
 	bf->uar = uar;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 08/10] net/mlx4_en: Use ethtool cmd->autoneg as a hint for ethtool set settings
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin, Saeed Mahameed
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>

Use cmd->autoneg as a user hint to decide what to set in ethtool set settings callback.
When cmd->autoneg == AUTONEG_ENABLE set according to ethtool->advertise otherwise,
set according to ethtool->speed.

Usage:
	- ethtool -s eth<x> speed 56000 autoneg off
	- ethtool -s eth<x> advertise 0x800000 autoneg on

While we're here:
	- Move proto_admin masking outcome check to be adjacent to the operation.
	- Move en_warn("port reset..") print to "port reset" block.

Fixes: 312df74 ("net/mlx4_en: mlx4_en_set_settings() always fails when autoneg is set")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index 569eda9..a7b58ba 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -770,22 +770,20 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		return 0;
 	}
 
-	proto_admin = cpu_to_be32(ptys_adv);
-	if (speed >= 0 && speed != priv->port_state.link_speed)
-		/* If speed was set then speed decides :-) */
-		proto_admin = speed_set_ptys_admin(priv, speed,
-						   ptys_reg.eth_proto_cap);
+	proto_admin = cmd->autoneg == AUTONEG_ENABLE ?
+		cpu_to_be32(ptys_adv) :
+		speed_set_ptys_admin(priv, speed,
+				     ptys_reg.eth_proto_cap);
 
 	proto_admin &= ptys_reg.eth_proto_cap;
-
-	if (proto_admin == ptys_reg.eth_proto_admin)
-		return 0; /* Nothing to change */
-
 	if (!proto_admin) {
 		en_warn(priv, "Not supported link mode(s) requested, check supported link modes.\n");
 		return -EINVAL; /* nothing to change due to bad input */
 	}
 
+	if (proto_admin == ptys_reg.eth_proto_admin)
+		return 0; /* Nothing to change */
+
 	en_dbg(DRV, priv, "mlx4_ACCESS_PTYS_REG SET: ptys_reg.eth_proto_admin = 0x%x\n",
 	       be32_to_cpu(proto_admin));
 
@@ -798,9 +796,9 @@ static int mlx4_en_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 		return ret;
 	}
 
-	en_warn(priv, "Port link mode changed, restarting port...\n");
 	mutex_lock(&priv->mdev->state_lock);
 	if (priv->port_up) {
+		en_warn(priv, "Port link mode changed, restarting port...\n");
 		mlx4_en_stop_port(dev, 1);
 		if (mlx4_en_start_port(dev))
 			en_err(priv, "Failed restarting port %d\n", priv->port);
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 03/10] net/mlx4_core: Fix mem leak in SRIOV mlx4_init_one error flow
From: Amir Vadai @ 2015-01-27  9:32 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Structs allocated for the resource tracker must be freed in
the error flow.


Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 46531c2..ab990f9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2980,8 +2980,10 @@ err_free_eq:
 	mlx4_free_eq_table(dev);
 
 err_master_mfunc:
-	if (mlx4_is_master(dev))
+	if (mlx4_is_master(dev)) {
+		mlx4_free_resource_tracker(dev, RES_TR_FREE_STRUCTS_ONLY);
 		mlx4_multi_func_cleanup(dev);
+	}
 
 	if (mlx4_is_slave(dev)) {
 		kfree(dev->caps.qp0_qkey);
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]
From: Alexander Aring @ 2015-01-27  9:34 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: netdev, davem, dmitry.tarnyagin, arvid.brodin, linux-wpan
In-Reply-To: <1422307694-10079-1-git-send-email-nicolas.dichtel@6wind.com>

Hi,

On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
> 
> When one of these attributes is set, the netdevice is created into the netns
> pointed by IFLA_NET_NS_[PID|FD] (see the call to rtnl_create_link() in
> rtnl_newlink()). Let's call this netns the dest_net. After this creation, if the
> newlink handler exists, it is called with a netns argument that points to the
> netns where the netlink message has been received (called src_net in the code)
> which is the link netns.
> Hence, with one of these attributes, it's possible to create a x-netns
> netdevice.
> 
> Here is the result of my code review:
> - all ip tunnels (sit, ipip, ip6_tunnels, gre[tap][v6], ip_vti[6]) does not
>   really allows to use this feature: the netdevice is created in the dest_net
>   and the src_net is completely ignored in the newlink handler.
> - VLAN properly handles this x-netns creation.
> - bridge ignores src_net, which seems fine (NETIF_F_NETNS_LOCAL is set).
> - CAIF subsystem is not clear for me (I don't know how it works), but it seems
>   to wrongly use src_net. Patch #1 tries to fix this, but it was done only by
>   code review (and only compile-tested), so please carefully review it. I may
>   miss something.
> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>   the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>   supports x-netns? If not, the newlink handler should use the dest_net instead
>   of src_net, I can provide the patch.
> - ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
>   question: does this netdevice really supports x-netns?

I am not sure if I understand exactly what you mean. First of all, I
didn't test anything about net namespaces for the ieee802154 branch.
In 802.15.4 branch we have two interfaces: wpan and 6LoWPAN.

After running "grep -r "src_net" net" I found this is used in:

net/ieee802154/6lowpan/core.c [0]

This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.

To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
By calling "ip link add link wpan0 name lowpan0 type lowpan" the
lowpan_newlink function will be called and we need to find the wpan interface
(returned as real_dev in this case).

Namespace setting in wpan interface:

Currently we don't use any net namespace settings there, also we don't
change the net namespace. The default net namespace for a wpan shoule be
"init_net".

So this line could be also written as (I found also some others code which search
the wpan interface in &init_net):

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 9dbe0d69..495c6ad 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
        if (!tb[IFLA_LINK])
                return -EINVAL;
        /* find and hold real wpan device */
-       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
        if (!real_dev)
                return -ENODEV;
        if (real_dev->type != ARPHRD_IEEE802154) {



The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
used. This interface will be created inside function "newlink".

Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
Don't know if this information is really necessary.

Should I set now the NETIF_F_NETNS_LOCAL for both interface types?

- Alex

[0] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/ieee802154/6lowpan/core.c#n154

^ permalink raw reply related

* RE: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: David Laight @ 2015-01-27  9:43 UTC (permalink / raw)
  To: 'Amir Vadai', David S. Miller
  Cc: netdev@vger.kernel.org, Or Gerlitz, Yevgeny Petrilin,
	Jack Morgenstein
In-Reply-To: <1422351179-27284-7-git-send-email-amirv@mellanox.com>

From: Amir Vadai
> From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> 
> Struct mlx4_vhcr was implicitly padded by the gcc compiler.
> This commit makes that padding explicit, to prevent issues with
> changing compilers. Note that we added the padding dword (rather
> than simply packing the structure) in order to maintain compatibility
> with previous kernels.

It isn't a 'compiler' option, but depends on the architecture.

> Reported-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/mlx4.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> index 096a81c..595e18a 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h
> @@ -196,13 +196,14 @@ struct mlx4_vhcr {
>  struct mlx4_vhcr_cmd {
>  	__be64 in_param;
>  	__be32 in_modifier;
> +	u32 reserved1;

Adding a pad here changes the alignment on most 32bit architectures (eg i386)
where 64bit integers are only aligned to 4 byte boundaries.
So you are not 'maintaining compatibility with previous kernels'.

>  	__be64 out_param;
>  	__be16 token;
>  	u16 reserved;
>  	u8 status;
>  	u8 flags;
>  	__be16 opcode;
> -};
> +} __packed;

Don't add '__packed' unless you expect the structure to be misaligned
in memory.
On systems that fault mis-aligned memory requests you've requested the
compiler generate code to read/write everything using byte sized memory
accesses and a lot of shifting and masking.

	David

^ permalink raw reply

* Re: [RFC PATCH] net: ipv6: Make address flushing on ifdown optional
From: Hannes Frederic Sowa @ 2015-01-27  9:44 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: Stephen Hemminger, David Ahern, netdev
In-Reply-To: <20150127045653.GB13164@gospo.home.greyhouse.net>

Hi,

On Mo, 2015-01-26 at 23:56 -0500, Andy Gospodarek wrote:
> On Fri, Jan 23, 2015 at 01:22:17PM +0100, Hannes Frederic Sowa wrote:
> > On Do, 2015-01-22 at 22:40 -0800, Stephen Hemminger wrote:
> > > On Wed, 14 Jan 2015 12:17:19 -0700
> > > David Ahern <dsahern@gmail.com> wrote:
> > > 
> > > > Currently, ipv6 addresses are flushed when the interface is configured down:
> > > > 
> > > > [root@f20 ~]# ip -6 addr add dev eth1 2000:11:1:1::1/64
> > > > [root@f20 ~]# ip addr show dev eth1
> > > > 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
> > > >     link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> > > >     inet6 2000:11:1:1::1/64 scope global tentative
> > > >        valid_lft forever preferred_lft forever
> > > > [root@f20 ~]# ip link set dev eth1 up
> > > > [root@f20 ~]# ip link set dev eth1 down
> > > > [root@f20 ~]# ip addr show dev eth1
> > > > 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> > > >     link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> > > > 
> > > > Add a new sysctl to make this behavior optional. Setting defaults to flush
> > > > addresses to maintain backwards compatibility. When reset flushing is bypassed:
> > > > 
> > > > [root@f20 ~]# echo 0 > /proc/sys/net/ipv6/conf/eth1/flush_addr_on_down
> > > > [root@f20 ~]# ip -6 addr add dev eth1 2000:11:1:1::1/64
> > > > [root@f20 ~]# ip addr show dev eth1
> > > > 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> > > >     link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> > > >     inet6 2000:11:1:1::1/64 scope global tentative
> > > >        valid_lft forever preferred_lft forever
> > > > [root@f20 ~]#  ip link set dev eth1 up
> > > > [root@f20 ~]#  ip link set dev eth1 down
> > > > [root@f20 ~]# ip addr show dev eth1
> > > > 3: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN group default qlen 1000
> > > >     link/ether 02:04:11:22:33:01 brd ff:ff:ff:ff:ff:ff
> > > >     inet6 2000:11:1:1::1/64 scope global
> > > >        valid_lft forever preferred_lft forever
> > > >     inet6 fe80::4:11ff:fe22:3301/64 scope link
> > > >        valid_lft forever preferred_lft forever
> > > > 
> > > > Suggested-by: Hannes Frederic Sowa <hannes@redhat.com>
> > > > Signed-off-by: David Ahern <dsahern@gmail.com>
> > > > Cc: Hannes Frederic Sowa <hannes@redhat.com>
> > > 
> > > Would this break existing application expecting a particular semantic
> > > by listening to netlink?  What happens to packets received with the static
> > > address when interface is down? With IPv4 Linux is mostly a weak host
> > > model, and IPv6 somewhere in between.
> > 
> > IPv6 is mostly a weak end model, too, but IFA_LINK addresses are used
> > much more. So yes, it is somewhere in between.
> > 
> > Addresses bound to interfaces which are currently down will work with
> > IPv6 (in contrast to IPv4).
> > 
> > > For vendors that control the application stack or have limited number
> > > of services this would work fine, but what about RHEL?
> > 
> > The new model is only enabled if the sysctl is set. I don't expect a lot
> > of vendors or distributions switching anytime soon.
> 
> You are probably correct that many will not switch, but this sysctl
> gives those who want to switch a nice option without having to carry
> extra kernel patches.  I like it. 

I don't see any problem with the patch in general but DAD should
definitely happen on reenabling interfaces. Current behavior does not
seem fine to me. It's ok if this patch doesn't change this behavior now
but a follow-up one will be needed then.

David, will you look after the DAD logic and do you plan a follow-up
patch?

> I have been pondering a few different changes to interface address and
> route behavior on both interface and link (gasp!) down and would like to
> use sysctls to make those options available to those who are interested
> without changing the current model.

Can you be more specific? In the last year we added some per interface
flags to e.g. handle the creation of on-link routes in case of an
address gets added to the interface. Maybe this is the better approach,
because quite a lot of stuff happens interface local in IPv6.

Thanks,
Hannes

^ permalink raw reply

* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Thomas Graf @ 2015-01-27  9:45 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, David.Laight, ying.xue, kaber, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150126233100.GA31108@gondor.apana.org.au>

On 01/27/15 at 10:31am, Herbert Xu wrote:
> On Tue, Jan 27, 2015 at 09:42:16AM +1100, Herbert Xu wrote:
> > 
> > As otherwise we'd be looking at fixing the existing users (in
> > particular, nft_hash) properly which might take a lot more time.
> 
> I take that back.  Looks like my walkers would be no good for
> netfilter anyway since it wants to do nested walks, meaning no
> locks can be taken.
> 
> So I think I'll just have to tackle netfilter first in which case
> the walk function would be no use anyway since I could just fix
> netlink by itself.

OK. I'll wait for your netfilter approach before posting NLM_F_INTR
patches.

^ permalink raw reply

* RE: [PATCHv3 ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: David Laight @ 2015-01-27  9:46 UTC (permalink / raw)
  To: 'Fan Du', steffen.klassert@secunet.com
  Cc: herbert@gondor.apana.org.au, davem@davemloft.net,
	netdev@vger.kernel.org, fengyuleidian0615@gmail.com
In-Reply-To: <1422349230-17394-1-git-send-email-fan.du@intel.com>

From: Fan Du
> structure like xfrm_usersa_info or xfrm_userpolicy_info
> has different sizeof when compiled as 32bits and 64bits
> due to not appending pack attribute in their definition.

Don't 'pack' the structure, just ensure that all the fields
are fixed sized and on their natural boundary.

Possibly add a compile-time check that the structure is
of the expected size.

	David

^ permalink raw reply

* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-27  9:52 UTC (permalink / raw)
  To: Joe Perches
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	abrestic-F7+t8E8rja9g9hUCZPvPmw
In-Reply-To: <1422301185.18650.4.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>


On 26/01/15 19:39, Joe Perches wrote:
> This header file is the public API for the driver.
> Should it not live under the 'include' directory?
> Several other drivers seem to follow that convention.
> It depends on how public is public.
>
> If it's _really_ public, it should be in uapi.
> If it's kinda public, then _maybe_ it should be
> in include/linux, but how likely is it another
> driver will use it?
>
>
It is intended for user space code that needs
to use the driver as it defines the netlink messages and
attributes that the driver understands. So I guess
uapi/linux would be the place for it.

Thanks,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Herbert Xu @ 2015-01-27  9:54 UTC (permalink / raw)
  To: Thomas Graf
  Cc: David Miller, David.Laight, ying.xue, kaber, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150127094541.GC22262@casper.infradead.org>

On Tue, Jan 27, 2015 at 09:45:41AM +0000, Thomas Graf wrote:
>
> OK. I'll wait for your netfilter approach before posting NLM_F_INTR
> patches.

Why would we still need the INTR stuff given that Dave has asked
for this to be done outside of rhashtable?

What I'm planning on doing right now is to divide rhashtable users
into two camps, lockless insert/removal vs. locked insert/removal.
netfilter will obviously fall into the latter camp.

For those with locked insert/removal I will make resizing/rehashing
synchronous since there is zero benefit from doing it in a separate
thread.

Once that is done you can then easily walk the table by just holding
the lock, which is what netfilter does today.

I'll probably make netlink locked too since it's much simpler that
way.  Only really high performance users like TCP/UDP sockets need
lockless insert/removal and the complexity that comes with it.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* RE: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: David Laight @ 2015-01-27 10:09 UTC (permalink / raw)
  To: 'Herbert Xu'
  Cc: Thomas Graf, Ying Xue, davem@davemloft.net, kaber@trash.net,
	paulmck@linux.vnet.ibm.com, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org
In-Reply-To: <20150126222259.GC30116@gondor.apana.org.au>

From: Herbert Xu 
> On Mon, Jan 26, 2015 at 10:09:24AM +0000, David Laight wrote:
> >
> > That doesn't look right to me.
> > Surely you shouldn't be calling rcu_read_lock() when the mutex
> > request is interrupted.
> >
> > So maybe:
> > 	err = mutex_lock_interruptible(&ht->mutex);
> > 	if (err)
> > 		return err;
> > 	rcu_read_lock();
> 
> No, we need to grab the RCU read lock while holding the mutex
> in order to prevent future resizes from happening once we release
> the mutex.

But if err is non-zero you don't hold the mutex.
Presumably the calling code also errors out immediately,
so the RCU lock isn't needed at all.

	David

^ permalink raw reply

* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Herbert Xu @ 2015-01-27 10:12 UTC (permalink / raw)
  To: David Laight
  Cc: Thomas Graf, Ying Xue, davem@davemloft.net, kaber@trash.net,
	paulmck@linux.vnet.ibm.com, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B6C@AcuExch.aculab.com>

On Tue, Jan 27, 2015 at 10:09:18AM +0000, David Laight wrote:
>
> But if err is non-zero you don't hold the mutex.
> Presumably the calling code also errors out immediately,
> so the RCU lock isn't needed at all.

No the calling code expects the RCU lock to be held because it
will always call stop.  Look at how seq_file works.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH net-next v2] rtnetlink: pass link_net to the newlink handler
From: Nicolas Dichtel @ 2015-01-27 10:13 UTC (permalink / raw)
  To: netdev; +Cc: davem, Nicolas Dichtel
In-Reply-To: <54C61ED1.5040501@6wind.com>

When IFLA_LINK_NETNSID is used, the netdevice should be built in this link netns
and moved at the end to another netns (pointed by the socket netns or
IFLA_NET_NS_[PID|FD]).

Existing user of the newlink handler will use the netns argument (src_net) to
find a link netdevice or to check some other information into the link netns.
For example, to find a netdevice, two information are required: an ifindex
(usually from IFLA_LINK) and a netns (this link netns).

Note: when using IFLA_LINK_NETNSID and IFLA_NET_NS_[PID|FD], a user may create a
netdevice that stands in netnsX and with its link part in netnsY, by sending a
rtnl message from netnsZ.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---

v2: instead of patching each subsystem, pass the right netns to
    the newlink handler

 net/core/rtnetlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 07447d1665e6..fedd7ab4085a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2148,7 +2148,7 @@ replay:
 		dev->ifindex = ifm->ifi_index;
 
 		if (ops->newlink) {
-			err = ops->newlink(net, dev, tb, data);
+			err = ops->newlink(link_net ? : net, dev, tb, data);
 			/* Drivers should call free_netdev() in ->destructor
 			 * and unregister it on failure after registration
 			 * so that device could be finally freed in rtnl_unlock.
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Thomas Graf @ 2015-01-27 10:15 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David Miller, David.Laight, ying.xue, kaber, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150127095454.GA2695@gondor.apana.org.au>

On 01/27/15 at 08:54pm, Herbert Xu wrote:
> On Tue, Jan 27, 2015 at 09:45:41AM +0000, Thomas Graf wrote:
> >
> > OK. I'll wait for your netfilter approach before posting NLM_F_INTR
> > patches.
> 
> Why would we still need the INTR stuff given that Dave has asked
> for this to be done outside of rhashtable?

Because libraries like libnl already handle this transparently for
various user space apps. Those apps are not even aware of the problem
and always get a consistent dump if the kernel implements INTR.

> What I'm planning on doing right now is to divide rhashtable users
> into two camps, lockless insert/removal vs. locked insert/removal.
> netfilter will obviously fall into the latter camp.
> 
> For those with locked insert/removal I will make resizing/rehashing
> synchronous since there is zero benefit from doing it in a separate
> thread.

Because making it synchronous does not help the Netlink dump problem
at all, see below.

Also, a sync resize causes a random inserts/removal to take a lot
longer. This leads to less predictable insert/removal times. I have
no objection to offering both sync and async resizing as long as the
choice of balance is configurable.

> Once that is done you can then easily walk the table by just holding
> the lock, which is what netfilter does today.

How? Do you want to hold the mutex while the user is reading? I don't
see how to achieve consistency with locking without introducing a ton
of complexity and attackable behaviour like user space being able to
delay inserts. If at all this only seems acceptable for privileged
users.

My vote is to rely on the proven INTR flag and optionally restart the
dump if user space did not indicate that INTR is supported as Patrick
suggested.

^ permalink raw reply

* Re: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: Herbert Xu @ 2015-01-27 10:24 UTC (permalink / raw)
  To: Thomas Graf
  Cc: David Miller, David.Laight, ying.xue, kaber, paulmck, netdev,
	netfilter-devel
In-Reply-To: <20150127101512.GE22262@casper.infradead.org>

On Tue, Jan 27, 2015 at 10:15:12AM +0000, Thomas Graf wrote:
>
> Because libraries like libnl already handle this transparently for
> various user space apps. Those apps are not even aware of the problem
> and always get a consistent dump if the kernel implements INTR.

Did you see Dave's message? He said those that dump need to create
their own data structure so there is no point in proceeding with
any of this work within rhashtable.
 
> Because making it synchronous does not help the Netlink dump problem
> at all, see below.
> 
> Also, a sync resize causes a random inserts/removal to take a lot
> longer. This leads to less predictable insert/removal times. I have
> no objection to offering both sync and async resizing as long as the
> choice of balance is configurable.

So what?  They can switch over to lockless mode if they care.

The point is that for netfilter as it stands it makes no sense
to go lockless because its modus operandi is to hold a single
lock over everything, including walking/dumping.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]
From: Nicolas Dichtel @ 2015-01-27 10:32 UTC (permalink / raw)
  To: Alexander Aring; +Cc: netdev, davem, dmitry.tarnyagin, arvid.brodin, linux-wpan
In-Reply-To: <20150127093425.GA2698@omega>

Le 27/01/2015 10:34, Alexander Aring a écrit :
> Hi,
>
> On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
>>
[snip]
>> - ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
>>    question: does this netdevice really supports x-netns?
>
> I am not sure if I understand exactly what you mean. First of all, I
> didn't test anything about net namespaces for the ieee802154 branch.
> In 802.15.4 branch we have two interfaces: wpan and 6LoWPAN.
>
> After running "grep -r "src_net" net" I found this is used in:
>
> net/ieee802154/6lowpan/core.c [0]
Yes, I was talking about this.

>
> This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
> IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.
>
> To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
> By calling "ip link add link wpan0 name lowpan0 type lowpan" the
> lowpan_newlink function will be called and we need to find the wpan interface
> (returned as real_dev in this case).
>
> Namespace setting in wpan interface:
>
> Currently we don't use any net namespace settings there, also we don't
> change the net namespace. The default net namespace for a wpan shoule be
> "init_net".
Ok. After grepping for init_net, it seems to be used a lot in net/ieee802154/.

>
> So this line could be also written as (I found also some others code which search
> the wpan interface in &init_net):
>
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 9dbe0d69..495c6ad 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
>          if (!tb[IFLA_LINK])
>                  return -EINVAL;
>          /* find and hold real wpan device */
> -       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> +       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
>          if (!real_dev)
>                  return -ENODEV;
>          if (real_dev->type != ARPHRD_IEEE802154) {
>
>
>
> The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
> For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
> used. This interface will be created inside function "newlink".
>
> Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
> Don't know if this information is really necessary.
>
> Should I set now the NETIF_F_NETNS_LOCAL for both interface types?
I think yes. If it's not set, a user may do:
$ ip link add link wpan0 name lowpan0 type lowpan
$ ip netns add foo
$ ip link set lowpan0 netns foo

The flag forbids the last command.

Instead of your patch, what about this one:

 From d9a9cd22d5e1db1417b3ffb53cc020481dc761b2 Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 27 Jan 2015 11:26:20 +0100
Subject: [PATCH] ieee802154: forbid to create an iface in a netns != init_net

6LoWPAN currently doesn't supports netns.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
  net/ieee802154/6lowpan/core.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 055fbb71ba6f..fe8fd022042e 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
  	dev->header_ops		= &lowpan_header_ops;
  	dev->ml_priv		= &lowpan_mlme;
  	dev->destructor		= free_netdev;
+	dev->features		|= NETIF_F_NETNS_LOCAL;
  }

  static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
@@ -148,7 +149,9 @@ static int lowpan_newlink(struct net *src_net, struct 
net_device *dev,

  	pr_debug("adding new link\n");

-	if (!tb[IFLA_LINK])
+	if (!tb[IFLA_LINK] ||
+	    !net_eq(src_net, &init_net) ||
+	    !net_eq(dev_net(dev), &init_net))
  		return -EINVAL;
  	/* find and hold real wpan device */
  	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
-- 
2.2.2

^ permalink raw reply related

* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-27 10:51 UTC (permalink / raw)
  To: Florian Fainelli, Stathis Voukelatos, netdev, linux-kernel,
	devicetree
  Cc: abrestic
In-Reply-To: <54C6BFEA.5020101@gmail.com>

Hi Florian,

On 26/01/15 22:30, Florian Fainelli wrote:
> On 23/01/15 02:07, Stathis Voukelatos wrote:
>> This patch adds support the Ethernet Packet Sniffer H/W module
>> developed by Linn Products Ltd and found in the IMG Pistachio SoC.
>> The module allows Ethernet packets to be parsed, matched against
>> a user-defined pattern and timestamped. It sits between a 100M
>> Ethernet MAC and PHY and is completely passive with respect to
>> Ethernet frames.
> Is there any latency penalty involved in capturing (or not) packets as
> opposed to having this capture HW unused?

There is no additional latency introduced by the sniffer at the H/W level,
if that is what you mean. Only the S/W overhead for handling the
sniffer interrupt for each match event.

>> Matched packet bytes and timestamp values are returned through a
>> FIFO. Timestamps are provided to the module through an externally
>> generated Gray-encoded counter.
>>
>> The command pattern for packet matching is stored in module RAM
>> and consists of a sequence of 16-bit entries. Each entry includes
>> an 8-bit command code and and 8-bit data value. Valid command
>> codes are:
>> 0 - Don't care
>> 1 - Match: packet data must match command string byte
>> 2 - Copy: packet data will be copied to FIFO
>> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>>                   is copied into the FIFO
>> 4 - Copy/Done: packet data will be copied into the FIFO.
>>                 This command terminates the command string.
>>
>> The driver consists of two modules:
>> - Core: it provides an API to user space using the Generic Netlink
>>          framework. Specific backend implementations, like the
>>          Ethernet Packet Sniffer, register one or more channels
>>          with the Core. For each channel a Genl family is created.
>>          User space can access a channel by sending Genl messages
>>          to the Genl family associated with the channel. Packet
>>          matching events are multicast.
> Instead of having this new generic netlink family to control sniffing,
> could we imagine registering a netdevice which does not nothing but
> still allows for tools like tcpdump, af_packet and other capture tools
> to work transparently and just leverage the HW capture?
Thanks, I will work on that change. It has been suggested by a previous
reviewer too and it makes sense to go down that route.

Stathis

^ permalink raw reply

* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Mark Rutland @ 2015-01-27 10:53 UTC (permalink / raw)
  To: Stathis Voukelatos
  Cc: Stathis Voukelatos, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	abrestic@chromium.org
In-Reply-To: <54C613F2.5090507@linn.co.uk>

On Mon, Jan 26, 2015 at 10:16:18AM +0000, Stathis Voukelatos wrote:
> 
> On 23/01/15 10:51, Mark Rutland wrote:
> >
> > diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> > new file mode 100644
> > index 0000000..6b6e105
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> > @@ -0,0 +1,27 @@
> > +* Linn Products Ethernet Packet Sniffer
> > +
> > +Required properties:
> > +- compatible : must be "linn,eth-sniffer"
> > +- reg : physical addresses and sizes of registers. Must contain 3 entries:
> > +          first entry: registers memory space
> > +          second entry: TX command memory
> > +          third entry: RX command memory
> > Just to check: are those memories are part of the packet sniffer device,
> > or are carveouts from other memory?
> Yes, the 3 memory areas are part of the packet sniffer module.
> >> +- fifo-block-words : number of words in one data FIFO entry
> >> +
> >> +Example:
> >> +
> >> +sniffer@1814a000 {
> >> +        compatible = "linn,eth-sniffer";
> >> +        reg = <0x1814a000 0x100>, <0x1814a400 0x400>, <0x1814a800 0x400>;
> >> +        reg-names = "regs", "tx-ram", "rx-ram";
> >> +        interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
> >> +        interrupt-names = "eth-sniffer-irq";
> >> +        clocks = <&system_clk>;
> >> +        clock-names = "sys";
> >> +        fifo-block-words = <4>;
> >> +    };
> > Surely the relationship between the sniffer, MAC, and PHY should be
> > described, so we know which interface the sniffer is related to?
> >
> The packet sniffer sits between the MAC and the PHY and monitors TX or RX
> packets, or both. Will add a description in the binding doc.

I understood that. However the binding does not explicitly refer to
either of those, so I don't see how you'd associate this sniffer
instance with the relevant PHY+MAC instances.

That should be made explicit in the binding.

Mark.

^ permalink raw reply

* Re: [PATCHv3 ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: Florian Westphal @ 2015-01-27 11:04 UTC (permalink / raw)
  To: David Laight
  Cc: 'Fan Du', steffen.klassert@secunet.com,
	herbert@gondor.apana.org.au, davem@davemloft.net,
	netdev@vger.kernel.org, fengyuleidian0615@gmail.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B2B@AcuExch.aculab.com>

David Laight <David.Laight@ACULAB.COM> wrote:
> From: Fan Du
> > structure like xfrm_usersa_info or xfrm_userpolicy_info
> > has different sizeof when compiled as 32bits and 64bits
> > due to not appending pack attribute in their definition.
> 
> Don't 'pack' the structure, just ensure that all the fields
> are fixed sized and on their natural boundary.

How do you propose to do this without breaking ABI?

> Possibly add a compile-time check that the structure is
> of the expected size.

Uh, what?

^ permalink raw reply

* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-27 11:15 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <54C612A5.2000208@redhat.com>

Hi Daniel,

On 26/01/15 10:10, Daniel Borkmann wrote:
>> Hello Daniel. Thank you for your feedback.
>> Packet sockets could also be used for the driver interface to
>> user space, however I think that both approaches would require the same
>> amount of maintenance. We need to maintain a protocol consisting of
>> a set of messages or commands that user space can use to communicate
>> with the driver in order to configure the H/W and retrieve results.
>> We could use packet sockets to send those messages  too, but I thought
>> netlink already provides a message exchange framework that we could
>> make use of.
>
> When using packet sockets and your driver as a backend feeding them,
> users can see that there's an extra capturing/monitoring netdev present,
> all libpcap-based tools such as tcpdump et al would work out of the box
> w/o adapting any code, and as an admin you can also see what users/tools
> are making of use of the device through packet sockets. I couldn't parse
> the exact motivation from the commit message of why avoiding all this is
> better?
>
> Thanks,
> Daniel
>
>
Just wanted to clarify some implementation details for your approach.
- The driver would need to create and register two net_device instances.
One for sniffing Ethernet TX packets and one for RX.
- Would the control interface for the sniffer in that case need to be
through private socket ioctls (ie SIOCDEVPRIVATE + x ioctl ids)?
- For each ethernet packet that matches the command string the sniffer
returns some data bytes and optionally a timestamp (depending on the
command string). Would a new protocol need to be added in
<linux/if_ether.h> in order to deliver that data to user space through
a packet socket?

Thanks,
Stathis

^ 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