Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v4 2/9] switchdev: Add SWITCHDEV_PORT_ATTR_SET, SWITCHDEV_PORT_ATTR_GET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

In preparation for allowing switchdev enabled drivers to veto specific
attribute settings from within the context of the caller, introduce a
new switchdev notifier type for port attributes.

Suggested-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/switchdev.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 5e87b54c5dc5..b8becabbef38 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -143,6 +143,9 @@ enum switchdev_notifier_type {
 	SWITCHDEV_VXLAN_FDB_ADD_TO_DEVICE,
 	SWITCHDEV_VXLAN_FDB_DEL_TO_DEVICE,
 	SWITCHDEV_VXLAN_FDB_OFFLOADED,
+
+	SWITCHDEV_PORT_ATTR_SET, /* Blocking. */
+	SWITCHDEV_PORT_ATTR_GET, /* Blocking. */
 };
 
 struct switchdev_notifier_info {
@@ -165,6 +168,13 @@ struct switchdev_notifier_port_obj_info {
 	bool handled;
 };
 
+struct switchdev_notifier_port_attr_info {
+	struct switchdev_notifier_info info; /* must be first */
+	struct switchdev_attr *attr;
+	struct switchdev_trans *trans;
+	bool handled;
+};
+
 static inline struct net_device *
 switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
 {
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 3/9] rocker: Handle SWITCHDEV_PORT_ATTR_GET/SET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.

Prepare rocker to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
rocker_port_attr_{set,get} calls.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/rocker/rocker_main.c | 24 +++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 66f72f8c46e5..9a8fe49e32b6 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2835,6 +2835,27 @@ rocker_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
 	return notifier_from_errno(err);
 }
 
+static int
+rocker_switchdev_port_attr_event(unsigned long event, struct net_device *netdev,
+				 struct switchdev_notifier_port_attr_info
+				 *port_attr_info)
+{
+	int err = -EOPNOTSUPP;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = rocker_port_attr_set(netdev, port_attr_info->attr,
+					   port_attr_info->trans);
+		break;
+	case SWITCHDEV_PORT_ATTR_GET:
+		err = rocker_port_attr_get(netdev, port_attr_info->attr);
+		break;
+	}
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 static int rocker_switchdev_blocking_event(struct notifier_block *unused,
 					   unsigned long event, void *ptr)
 {
@@ -2847,6 +2868,9 @@ static int rocker_switchdev_blocking_event(struct notifier_block *unused,
 	case SWITCHDEV_PORT_OBJ_ADD:
 	case SWITCHDEV_PORT_OBJ_DEL:
 		return rocker_switchdev_port_obj_event(event, dev, ptr);
+	case SWITCHDEV_PORT_ATTR_SET: /* fall through */
+	case SWITCHDEV_PORT_ATTR_GET:
+		return rocker_switchdev_port_attr_event(event, dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 4/9] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_GET/SET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.

Prepare mlxsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
mlxsw_sp_port_attr_{set,get} calls.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../mellanox/mlxsw/spectrum_switchdev.c       | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 95e37de3e48f..88d4994309a7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -3443,6 +3443,26 @@ mlxsw_sp_switchdev_handle_vxlan_obj_del(struct net_device *vxlan_dev,
 	}
 }
 
+static int
+mlxsw_sp_switchdev_port_attr_event(unsigned long event, struct net_device *dev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err = -EOPNOTSUPP;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = mlxsw_sp_port_attr_set(dev, port_attr_info->attr,
+					     port_attr_info->trans);
+		break;
+	case SWITCHDEV_PORT_ATTR_GET:
+		err = mlxsw_sp_port_attr_get(dev, port_attr_info->attr);
+		break;
+	}
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
 					     unsigned long event, void *ptr)
 {
@@ -3466,6 +3486,9 @@ static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
 							mlxsw_sp_port_dev_check,
 							mlxsw_sp_port_obj_del);
 		return notifier_from_errno(err);
+	case SWITCHDEV_PORT_ATTR_SET: /* fall through */
+	case SWITCHDEV_PORT_ATTR_GET:
+		return mlxsw_sp_switchdev_port_attr_event(event, dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 5/9] net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_GET/SET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.

Prepare ocelot to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
ocelot_port_attr_{set,get} calls.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mscc/ocelot.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 195306d05bcd..2708809713ed 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1582,6 +1582,24 @@ struct notifier_block ocelot_netdevice_nb __read_mostly = {
 };
 EXPORT_SYMBOL(ocelot_netdevice_nb);
 
+static int
+ocelot_switchdev_port_attr_event(unsigned long event,
+		struct net_device *netdev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err = -EOPNOTSUPP;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = ocelot_port_attr_set(netdev, port_attr_info->attr,
+					   port_attr_info->trans);
+		break;
+	}
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
 					   unsigned long event, void *ptr)
 {
@@ -1600,6 +1618,9 @@ static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
 						    ocelot_netdevice_dev_check,
 						    ocelot_port_obj_del);
 		return notifier_from_errno(err);
+	case SWITCHDEV_PORT_ATTR_SET: /* fall through */
+	case SWITCHDEV_PORT_ATTR_GET:
+		return ocelot_switchdev_port_attr_event(event, dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 6/9] staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_GET/SET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.

Prepare ethsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
swdev_port_attr_{set,get} calls.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index e559f4c25cf7..7c77fb43233e 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1111,6 +1111,27 @@ ethsw_switchdev_port_obj_event(unsigned long event, struct net_device *netdev,
 	return notifier_from_errno(err);
 }
 
+static int
+ethsw_switchdev_port_attr_event(unsigned long event,
+		struct net_device *netdev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err = -EOPNOTSUPP;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = swdev_port_attr_set(netdev, port_attr_info->attr,
+					  port_attr_info->trans);
+		break;
+	case SWITCHDEV_PORT_ATTR_GET:
+		err = swdev_port_attr_get(netdev, port_attr_info->attr);
+		break;
+	}
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 static int port_switchdev_blocking_event(struct notifier_block *unused,
 					 unsigned long event, void *ptr)
 {
@@ -1123,6 +1144,9 @@ static int port_switchdev_blocking_event(struct notifier_block *unused,
 	case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
 	case SWITCHDEV_PORT_OBJ_DEL:
 		return ethsw_switchdev_port_obj_event(event, dev, ptr);
+	case SWITCHDEV_PORT_ATTR_SET: /* fall through */
+	case SWITCHDEV_PORT_ATTR_GET:
+		return ethsw_switchdev_port_attr_event(event, dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 9/9] net: Remove switchdev_ops
From: Florian Fainelli @ 2019-02-11 19:10 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Now that we have converted all possible callers to using a switchdev
notifier for attributes we do not have a need for implementing
switchdev_ops anymore, and this can be removed from all drivers the
net_device structure.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 12 ------------
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  2 --
 .../mellanox/mlxsw/spectrum_switchdev.c        | 13 -------------
 drivers/net/ethernet/mscc/ocelot.c             |  5 -----
 drivers/net/ethernet/rocker/rocker_main.c      |  6 ------
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c        |  6 ------
 include/linux/netdevice.h                      |  3 ---
 include/net/switchdev.h                        | 18 ------------------
 net/dsa/slave.c                                |  6 ------
 9 files changed, 71 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 7c9745cecbbd..619965abab43 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3220,7 +3220,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	}
 	mlxsw_sp_port->default_vlan = mlxsw_sp_port_vlan;
 
-	mlxsw_sp_port_switchdev_init(mlxsw_sp_port);
 	mlxsw_sp->ports[local_port] = mlxsw_sp_port;
 	err = register_netdev(dev);
 	if (err) {
@@ -3237,7 +3236,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 
 err_register_netdev:
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_destroy(mlxsw_sp_port_vlan);
 err_port_vlan_create:
 err_port_pvid_set:
@@ -3280,7 +3278,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
 	unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_flush(mlxsw_sp_port, true);
 	mlxsw_sp_port_nve_fini(mlxsw_sp_port);
 	mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
@@ -4001,12 +3998,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 		goto err_span_init;
 	}
 
-	err = mlxsw_sp_switchdev_init(mlxsw_sp);
-	if (err) {
-		dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize switchdev\n");
-		goto err_switchdev_init;
-	}
-
 	err = mlxsw_sp_counter_pool_init(mlxsw_sp);
 	if (err) {
 		dev_err(mlxsw_sp->bus_info->dev, "Failed to init counter pool\n");
@@ -4077,8 +4068,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
 err_afa_init:
 	mlxsw_sp_counter_pool_fini(mlxsw_sp);
 err_counter_pool_init:
-	mlxsw_sp_switchdev_fini(mlxsw_sp);
-err_switchdev_init:
 	mlxsw_sp_span_fini(mlxsw_sp);
 err_span_init:
 	mlxsw_sp_lag_fini(mlxsw_sp);
@@ -4141,7 +4130,6 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
 	mlxsw_sp_nve_fini(mlxsw_sp);
 	mlxsw_sp_afa_fini(mlxsw_sp);
 	mlxsw_sp_counter_pool_fini(mlxsw_sp);
-	mlxsw_sp_switchdev_fini(mlxsw_sp);
 	mlxsw_sp_span_fini(mlxsw_sp);
 	mlxsw_sp_lag_fini(mlxsw_sp);
 	mlxsw_sp_buffers_fini(mlxsw_sp);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index ceebc91f4f1d..82e3e6dc81a1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -375,8 +375,6 @@ u32 mlxsw_sp_bytes_cells(const struct mlxsw_sp *mlxsw_sp, u32 bytes);
 /* spectrum_switchdev.c */
 int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp);
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port);
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port);
 int mlxsw_sp_rif_fdb_op(struct mlxsw_sp *mlxsw_sp, const char *mac, u16 fid,
 			bool adding);
 void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 88d4994309a7..3e2ef69a1d22 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1957,11 +1957,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct mlxsw_sp *mlxsw_sp,
 	return NULL;
 }
 
-static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
-	.switchdev_port_attr_get	= mlxsw_sp_port_attr_get,
-	.switchdev_port_attr_set	= mlxsw_sp_port_attr_set,
-};
-
 static int
 mlxsw_sp_bridge_8021q_port_join(struct mlxsw_sp_bridge_device *bridge_device,
 				struct mlxsw_sp_bridge_port *bridge_port,
@@ -3576,11 +3571,3 @@ void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp)
 	kfree(mlxsw_sp->bridge);
 }
 
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-	mlxsw_sp_port->dev->switchdev_ops = &mlxsw_sp_port_switchdev_ops;
-}
-
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-}
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 2708809713ed..374636e8fcbe 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1324,10 +1324,6 @@ static int ocelot_port_obj_del(struct net_device *dev,
 	return ret;
 }
 
-static const struct switchdev_ops ocelot_port_switchdev_ops = {
-	.switchdev_port_attr_set	= ocelot_port_attr_set,
-};
-
 static int ocelot_port_bridge_join(struct ocelot_port *ocelot_port,
 				   struct net_device *bridge)
 {
@@ -1654,7 +1650,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 
 	dev->netdev_ops = &ocelot_port_netdev_ops;
 	dev->ethtool_ops = &ocelot_ethtool_ops;
-	dev->switchdev_ops = &ocelot_port_switchdev_ops;
 
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 9a8fe49e32b6..b017543fd196 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2147,11 +2147,6 @@ static int rocker_port_obj_del(struct net_device *dev,
 	return err;
 }
 
-static const struct switchdev_ops rocker_port_switchdev_ops = {
-	.switchdev_port_attr_get	= rocker_port_attr_get,
-	.switchdev_port_attr_set	= rocker_port_attr_set,
-};
-
 struct rocker_fib_event_work {
 	struct work_struct work;
 	union {
@@ -2605,7 +2600,6 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_port_dev_addr_init(rocker_port);
 	dev->netdev_ops = &rocker_port_netdev_ops;
 	dev->ethtool_ops = &rocker_port_ethtool_ops;
-	dev->switchdev_ops = &rocker_port_switchdev_ops;
 	netif_tx_napi_add(dev, &rocker_port->napi_tx, rocker_port_poll_tx,
 			  NAPI_POLL_WEIGHT);
 	netif_napi_add(dev, &rocker_port->napi_rx, rocker_port_poll_rx,
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 7c77fb43233e..f4489976a9ac 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -932,11 +932,6 @@ static int swdev_port_obj_del(struct net_device *netdev,
 	return err;
 }
 
-static const struct switchdev_ops ethsw_port_switchdev_ops = {
-	.switchdev_port_attr_get	= swdev_port_attr_get,
-	.switchdev_port_attr_set	= swdev_port_attr_set,
-};
-
 /* For the moment, only flood setting needs to be updated */
 static int port_bridge_join(struct net_device *netdev,
 			    struct net_device *upper_dev)
@@ -1466,7 +1461,6 @@ static int ethsw_probe_port(struct ethsw_core *ethsw, u16 port_idx)
 	SET_NETDEV_DEV(port_netdev, dev);
 	port_netdev->netdev_ops = &ethsw_port_ops;
 	port_netdev->ethtool_ops = &ethsw_port_ethtool_ops;
-	port_netdev->switchdev_ops = &ethsw_port_switchdev_ops;
 
 	/* Set MTU limits */
 	port_netdev->min_mtu = ETH_MIN_MTU;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1fb733f38a47..a747456b9d23 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1836,9 +1836,6 @@ struct net_device {
 #endif
 	const struct net_device_ops *netdev_ops;
 	const struct ethtool_ops *ethtool_ops;
-#ifdef CONFIG_NET_SWITCHDEV
-	const struct switchdev_ops *switchdev_ops;
-#endif
 #ifdef CONFIG_NET_L3_MASTER_DEV
 	const struct l3mdev_ops	*l3mdev_ops;
 #endif
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index b8becabbef38..e9aa920aba57 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -113,21 +113,6 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans *trans);
 
 typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
 
-/**
- * struct switchdev_ops - switchdev operations
- *
- * @switchdev_port_attr_get: Get a port attribute (see switchdev_attr).
- *
- * @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
- */
-struct switchdev_ops {
-	int	(*switchdev_port_attr_get)(struct net_device *dev,
-					   struct switchdev_attr *attr);
-	int	(*switchdev_port_attr_set)(struct net_device *dev,
-					   const struct switchdev_attr *attr,
-					   struct switchdev_trans *trans);
-};
-
 enum switchdev_notifier_type {
 	SWITCHDEV_FDB_ADD_TO_BRIDGE = 1,
 	SWITCHDEV_FDB_DEL_TO_BRIDGE,
@@ -229,7 +214,6 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
 			int (*del_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj));
 
-#define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
 #else
 
 static inline void switchdev_deferred_process(void)
@@ -322,8 +306,6 @@ switchdev_handle_port_obj_del(struct net_device *dev,
 	return 0;
 }
 
-#define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
-
 #endif
 
 #endif /* _LINUX_SWITCHDEV_H_ */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 66c6c353f4f7..3deab965dad9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1057,11 +1057,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_get_port_parent_id	= dsa_slave_get_port_parent_id,
 };
 
-static const struct switchdev_ops dsa_slave_switchdev_ops = {
-	.switchdev_port_attr_get	= dsa_slave_port_attr_get,
-	.switchdev_port_attr_set	= dsa_slave_port_attr_set,
-};
-
 static struct device_type dsa_type = {
 	.name	= "dsa",
 };
@@ -1321,7 +1316,6 @@ int dsa_slave_create(struct dsa_port *port)
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
 	slave_dev->min_mtu = 0;
 	slave_dev->max_mtu = ETH_MAX_MTU;
 	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 8/9] net: switchdev: Replace port attr get/set SDO with a notification
From: Florian Fainelli @ 2019-02-11 19:10 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Drop switchdev_ops.switchdev_port_attr_get and _set. Drop the uses of
this field from all clients, which were migrated to use switchdev
notification in the previous patches.

Add a new function switchdev_port_attr_notify() that sends the switchdev
notifications SWITCHDEV_PORT_ATTR_GET and _SET.

Update switchdev_port_attr_get() to dispatch to this new function. Drop
__switchdev_port_attr_set() and update switchdev_port_attr_set()
likewise.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/switchdev/switchdev.c | 107 +++++++++++++-------------------------
 1 file changed, 37 insertions(+), 70 deletions(-)

diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 7e1357db33d7..8fc3db2179f5 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -174,81 +174,31 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
 	return 0;
 }
 
-/**
- *	switchdev_port_attr_get - Get port attribute
- *
- *	@dev: port device
- *	@attr: attribute to get
- */
-int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
+static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
+				      struct net_device *dev,
+				      struct switchdev_attr *attr,
+				      struct switchdev_trans *trans)
 {
-	const struct switchdev_ops *ops = dev->switchdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	struct switchdev_attr first = {
-		.id = SWITCHDEV_ATTR_ID_UNDEFINED
-	};
-	int err = -EOPNOTSUPP;
+	int err;
+	int rc;
 
-	if (ops && ops->switchdev_port_attr_get)
-		return ops->switchdev_port_attr_get(dev, attr);
+	struct switchdev_notifier_port_attr_info attr_info = {
+		.attr = attr,
+		.trans = trans,
+		.handled = false,
+	};
 
-	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
+	rc = call_switchdev_blocking_notifiers(nt, dev, &attr_info.info, NULL);
+	err = notifier_to_errno(rc);
+	if (err) {
+		WARN_ON(!attr_info.handled);
 		return err;
-
-	/* Switch device port(s) may be stacked under
-	 * bond/team/vlan dev, so recurse down to get attr on
-	 * each port.  Return -ENODATA if attr values don't
-	 * compare across ports.
-	 */
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = switchdev_port_attr_get(lower_dev, attr);
-		if (err)
-			break;
-		if (first.id == SWITCHDEV_ATTR_ID_UNDEFINED)
-			first = *attr;
-		else if (memcmp(&first, attr, sizeof(*attr)))
-			return -ENODATA;
-	}
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
-
-static int __switchdev_port_attr_set(struct net_device *dev,
-				     const struct switchdev_attr *attr,
-				     struct switchdev_trans *trans)
-{
-	const struct switchdev_ops *ops = dev->switchdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int err = -EOPNOTSUPP;
-
-	if (ops && ops->switchdev_port_attr_set) {
-		err = ops->switchdev_port_attr_set(dev, attr, trans);
-		goto done;
-	}
-
-	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
-		goto done;
-
-	/* Switch device port(s) may be stacked under
-	 * bond/team/vlan dev, so recurse down to set attr on
-	 * each port.
-	 */
-
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = __switchdev_port_attr_set(lower_dev, attr, trans);
-		if (err)
-			break;
 	}
 
-done:
-	if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
-		err = 0;
+	if (!attr_info.handled)
+		return -EOPNOTSUPP;
 
-	return err;
+	return 0;
 }
 
 static int switchdev_port_attr_set_now(struct net_device *dev,
@@ -267,7 +217,9 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = true;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET,
+					 dev, (struct switchdev_attr *)attr,
+					 &trans);
 	if (err) {
 		/* Prepare phase failed: abort the transaction.  Any
 		 * resources reserved in the prepare phase are
@@ -286,7 +238,9 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = false;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET,
+					 dev, (struct switchdev_attr *)attr,
+					 &trans);
 	WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
 	     dev->name, attr->id);
 	switchdev_trans_items_warn_destroy(dev, &trans);
@@ -338,6 +292,19 @@ int switchdev_port_attr_set(struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(switchdev_port_attr_set);
 
+/**
+ *	switchdev_port_attr_get - Get port attribute
+ *
+ *	@dev: port device
+ *	@attr: attribute to get
+ */
+int switchdev_port_attr_get(struct net_device *dev, struct switchdev_attr *attr)
+{
+	return switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_GET, dev,
+					  attr, NULL);
+}
+EXPORT_SYMBOL_GPL(switchdev_port_attr_get);
+
 static size_t switchdev_obj_size(const struct switchdev_obj *obj)
 {
 	switch (obj->id) {
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v4 7/9] net: dsa: Handle SWITCHDEV_PORT_ATTR_GET/SET
From: Florian Fainelli @ 2019-02-11 19:09 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

Following patches will change the way we communicate getting or setting
a port's attribute and use a blocking notifier to perform those tasks.

Prepare DSA to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_GET/SET and simply translate that into the existing
dsa_slave_port_attr_{set,get} calls.

Acked-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/slave.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e5e7c04821b..66c6c353f4f7 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1558,6 +1558,27 @@ dsa_slave_switchdev_port_obj_event(unsigned long event,
 	return notifier_from_errno(err);
 }
 
+static int
+dsa_slave_switchdev_port_attr_event(unsigned long event,
+		struct net_device *netdev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err = -EOPNOTSUPP;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = dsa_slave_port_attr_set(netdev, port_attr_info->attr,
+					      port_attr_info->trans);
+		break;
+	case SWITCHDEV_PORT_ATTR_GET:
+		err = dsa_slave_port_attr_get(netdev, port_attr_info->attr);
+		break;
+	}
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
 					      unsigned long event, void *ptr)
 {
@@ -1570,6 +1591,9 @@ static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
 	case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
 	case SWITCHDEV_PORT_OBJ_DEL:
 		return dsa_slave_switchdev_port_obj_event(event, dev, ptr);
+	case SWITCHDEV_PORT_ATTR_SET: /* fall through */
+	case SWITCHDEV_PORT_ATTR_GET:
+		return dsa_slave_switchdev_port_attr_event(event, dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* Re: Possible bug into DSA2 code.
From: Florian Fainelli @ 2019-02-11 19:13 UTC (permalink / raw)
  To: Rodolfo Giometti, Andrew Lunn; +Cc: Vivien Didelot, David S. Miller, netdev
In-Reply-To: <2651a882-c5ab-ba3b-ef4c-731dca90147d@gmail.com>

On 2/11/19 10:01 AM, Florian Fainelli wrote:
> On 2/11/19 9:51 AM, Rodolfo Giometti wrote:
>> On 11/02/2019 18:28, Florian Fainelli wrote:
>>> On 2/10/19 3:45 AM, Rodolfo Giometti wrote:
>>>> On 09/02/2019 20:34, Andrew Lunn wrote:
>>>>>> So we I see two possible solutions:
>>>>>>
>>>>>> 1) having both ds->slave_mii_bus and ds->ops->phy_read already
>>>>>> defined is an
>>>>>> error, then it must be signaled to the calling code, or
>>>>>
>>>>> I don't think we can do that. mv88e6xxx optionally instantiates the
>>>>> MDIO busses, depending on what is in device tree. If there is no mdio
>>>>> property, we need the DSA core to create an MDIO bus.
>>>>
>>>> OK, but using the following check to know if DSA did such allocation is
>>>> not correct because DSA drivers can allocate it by their own:
>>>>
>>>> static void dsa_switch_teardown(struct dsa_switch *ds)
>>>> {
>>>>          if (ds->slave_mii_bus && ds->ops->phy_read)
>>>>                  mdiobus_unregister(ds->slave_mii_bus);
>>>>
>>>> Maybe can we add a flag to register ds->slave_mii_bus allocation by DSA?
>>>
>>> If drivers allocate the slave_mii_bus, or use it as a pointer to their
>>> bus, then they should not be providing a ds->ops->phy_read() callback
>>> since we assume they would have mii_bus::read and mii_bus::write set to
>>> their driver internal version.
>>
>> I see, so having ds->slave_mii_bus and ds->ops->phy_read both not NULL
>> into dsa_switch_setup() is a potential bug, I suppose... If so why not
>> adding a BUG_ON() call to signal it instead of doing nothing? :-o
> 
> If you have both non NULL, then your driver did allocate
> ds->slave_mii_bus on its own, and also assigned a valid
> ds->ops->phy_read() then things will work, except that
> ds->ops->phy_read() will not be used. And yes, that is going to be
> blowing away when the whole DSA tree gets teardowned.
> 
> If you want to add a check for that condition, that would be a good
> thing, just not a BUG_ON(), propagate an error back to the caller and
> abort the tree/switch probing.

Does that work:

diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..54cf6a5c865d 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -368,6 +368,9 @@ static int dsa_switch_setup(struct dsa_switch *ds)
        if (err)
                return err;

+       if (ds->slave_mii_bus && (ds->ops->phy_read || ds->ops->phy_write))
+               return -EINVAL;
+
        if (!ds->slave_mii_bus && ds->ops->phy_read) {
                ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
                if (!ds->slave_mii_bus)
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index cb42939db776..0796c6213be6 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -176,6 +176,9 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
        if (ret)
                return ret;

+       if (ds->slave_mii_bus && (ops->phy_read || ops->phy_write))
+               return -EINVAL;
+
        if (!ds->slave_mii_bus && ops->phy_read) {
                ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
                if (!ds->slave_mii_bus)

> 


-- 
Florian

^ permalink raw reply related

* Re: [oss-drivers] Re: [RFC 1/3] devlink: add flash update command
From: Jakub Kicinski @ 2019-02-11 19:25 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, netdev, oss-drivers, mkubecek, andrew
In-Reply-To: <20190211164513.GI2314@nanopsycho.orion>

On Mon, 11 Feb 2019 17:45:13 +0100, Jiri Pirko wrote:
> Mon, Feb 11, 2019 at 07:59:20AM CET, jakub.kicinski@netronome.com wrote:
> >Add devlink flash update command. Advanced NICs have firmware
> >stored in flash and often cryptographically secured. Updating
> >that flash is handled by management firmware. Ethtool has a
> >flash update command which served us well, however, it has two
> >shortcomings:
> > - it takes rtnl_lock unnecessarily - really flash update has
> >   nothing to do with networking, so using a networking device
> >   as a handle is suboptimal, which leads us to the second one:
> > - it requires a functioning netdev - in case device enters an
> >   error state and can't spawn a netdev (e.g. communication
> >   with the device fails) there is no netdev to use as a handle
> >   for flashing.
> >
> >Devlink already has the ability to report the firmware versions,
> >now with the ability to update the firmware/flash we will be
> >able to recover devices in bad state.
> >
> >To enable easy interoperability with ethtool add the target
> >partition ID. We may or may not add a different method of
> >identification, but there is no such immediate need.
> >
> >Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> >---
> > include/net/devlink.h        |  2 ++
> > include/uapi/linux/devlink.h |  6 ++++++
> > net/core/devlink.c           | 30 ++++++++++++++++++++++++++++++
> > 3 files changed, 38 insertions(+)
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index 07660fe4c0e3..55b3478b1291 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -529,6 +529,8 @@ struct devlink_ops {
> > 				      struct netlink_ext_ack *extack);
> > 	int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
> > 			struct netlink_ext_ack *extack);
> >+	int (*flash_update)(struct devlink *devlink, const char *path,
> >+			    u32 target, struct netlink_ext_ack *extack);
> > };
> > 
> > static inline void *devlink_priv(struct devlink *devlink)
> >diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> >index 72d9f7c89190..f4417283fd1b 100644
> >--- a/include/uapi/linux/devlink.h
> >+++ b/include/uapi/linux/devlink.h
> >@@ -103,6 +103,8 @@ enum devlink_command {
> > 	DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
> > 	DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
> > 
> >+	DEVLINK_CMD_FLASH_UPDATE,
> >+
> > 	/* add new commands above here */
> > 	__DEVLINK_CMD_MAX,
> > 	DEVLINK_CMD_MAX = __DEVLINK_CMD_MAX - 1
> >@@ -326,6 +328,10 @@ enum devlink_attr {
> > 	DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS,		/* u64 */
> > 	DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD,	/* u64 */
> > 	DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER,	/* u8 */
> >+
> >+	DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME,	/* string */
> >+	DEVLINK_ATTR_FLASH_UPDATE_TARGET_ID,	/* u32 */  
> 
> Do we need to carry this on? I mean, the ef->region is only checked in 4
> drivers against ETHTOOL_FLASH_ALL_REGIONS, which is default.
> There is this bnxt driver which is the only one working with this value.
> I think that for compat, there should be some id-region mapping
> provided by driver which the compat layer would use to translate.
> 
> I also think that this should be in sync with what is returned in
> DEVLINK_ATTR_INFO_VERSION_NAME.
> 
> For example:
> $ devlink dev info pci/0000:82:00.0
> pci/0000:82:00.0:
>   driver nfp
>   serial_number 16240145
>   versions:
>       fixed:
>         board.id AMDA0081-0001
>         board.rev 15
>         board.vendor SMA
>         board.model hydrogen
>       running:
>         fw.mgmt 010181.010181.0101d4
>         fw.cpld 0x1030000
>         fw.app abm-d372b6
>         fw.undi 0.0.2
>         chip.init AMDA-0081-0001  20160318164536
>       stored:
>         fw.mgmt 010181.010181.0101d4
>         fw.app abm-d372b6
>         fw.undi 0.0.2
>         chip.init AMDA-0081-0001  20160318164536
> 
> Now user should be able to use one of the identifiers to flash relevant
> fw, like:
> 
> devlink dev flash pci/0000:82:00.0 XXX fw.mgmt file flash-boot.bin
> 
> I'm not sure about the name of "xxx" attribute. Maybe "id":
> 
> devlink dev flash pci/0000:82:00.0 id fw.mgmt file flash-boot.bin
> devlink dev flash pci/0000:82:00.0 id fw.cpld file some-other.bin

Agreed, that looks good!  TBH in case of Netronome the binary
image contains an identifier so it will update the correct component
automatically.  That's why I say "no immediate need" :)  (How about
"component" instead of "id", BTW?)

I will drop the target ID, I just added it for full backward compat
with ethtool, but it may be confusing, given it would be mostly unused.
I'll drop it in non-RFC, do you want me to add the id/component or leave
it out for now?

^ permalink raw reply

* Re: [net-next PATCH 2/2] net: page_pool: don't use page->private to store dma_addr_t
From: Alexander Duyck @ 2019-02-11 19:31 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Netdev, linux-mm, Toke Høiland-Jørgensen,
	Ilias Apalodimas, Matthew Wilcox, Saeed Mahameed, Andrew Morton,
	Mel Gorman, David S. Miller, Tariq Toukan
In-Reply-To: <154990121192.24530.11128024662816211563.stgit@firesoul>

On Mon, Feb 11, 2019 at 8:07 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> As pointed out by David Miller the current page_pool implementation
> stores dma_addr_t in page->private.
> This won't work on 32-bit platforms with 64-bit DMA addresses since the
> page->private is an unsigned long and the dma_addr_t a u64.
>
> A previous patch is adding dma_addr_t on struct page to accommodate this.
> This patch adapts the page_pool related functions to use the newly added
> struct for storing and retrieving DMA addresses from network drivers.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
>  net/core/page_pool.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 43a932cb609b..897a69a1477e 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -136,7 +136,9 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
>         if (!(pool->p.flags & PP_FLAG_DMA_MAP))
>                 goto skip_dma_map;
>
> -       /* Setup DMA mapping: use page->private for DMA-addr
> +       /* Setup DMA mapping: use 'struct page' area for storing DMA-addr
> +        * since dma_addr_t can be either 32 or 64 bits and does not always fit
> +        * into page private data (i.e 32bit cpu with 64bit DMA caps)
>          * This mapping is kept for lifetime of page, until leaving pool.
>          */
>         dma = dma_map_page(pool->p.dev, page, 0,
> @@ -146,7 +148,7 @@ static struct page *__page_pool_alloc_pages_slow(struct page_pool *pool,
>                 put_page(page);
>                 return NULL;
>         }
> -       set_page_private(page, dma); /* page->private = dma; */
> +       page->dma_addr = dma;
>
>  skip_dma_map:
>         /* When page just alloc'ed is should/must have refcnt 1. */
> @@ -175,13 +177,16 @@ EXPORT_SYMBOL(page_pool_alloc_pages);
>  static void __page_pool_clean_page(struct page_pool *pool,
>                                    struct page *page)
>  {
> +       dma_addr_t dma;
> +
>         if (!(pool->p.flags & PP_FLAG_DMA_MAP))
>                 return;
>
> +       dma = page->dma_addr;
>         /* DMA unmap */
> -       dma_unmap_page(pool->p.dev, page_private(page),
> +       dma_unmap_page(pool->p.dev, dma,
>                        PAGE_SIZE << pool->p.order, pool->p.dma_dir);
> -       set_page_private(page, 0);
> +       page->dma_addr = 0;
>  }
>
>  /* Return a page to the page allocator, cleaning up our state */

This comment is unrelated to this patch specifically, but applies more
generally to the page_pool use of dma_unmap_page.

So just looking at this I am pretty sure the use of just
dma_unmap_page isn't correct here. You should probably be using
dma_unmap_page_attrs and specifically be passing the attribute
DMA_ATTR_SKIP_CPU_SYNC so that you can tear down the mapping without
invalidating the contents of the page.

This is something that will work for most cases but if you run into a
case where this is used with SWIOTLB in bounce buffer mode you would
end up potentially corrupting data on the unmap call.

^ permalink raw reply

* Re: [iproute PATCH] man: ip-link: Describe promisc mode
From: Stephen Hemminger @ 2019-02-11 19:36 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netdev, William Flanagan
In-Reply-To: <20190211091706.20624-1-phil@nwl.cc>

On Mon, 11 Feb 2019 10:17:06 +0100
Phil Sutter <phil@nwl.cc> wrote:

> Briefly explain what it is and where it's typically used.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  man/man8/ip-link.8.in | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index 73d37c190fffa..5c327f01b6b45 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -1780,6 +1780,14 @@ flag on the device. Indicates that address can change when interface goes down (
>  .B NOT
>  used by the Linux).
>  
> +.TP
> +.BR "promisc on " or " promisc off"
> +change the
> +.B PROMISC
> +flag on the device. This requests receipt of all packets arriving at the NIC
> +irrespective of their destination MAC address. It is typically used by traffic
> +sniffers and also set by Linux bridges for their ports.

This added sentence is confusing. The Linux bridge enables it by default,
and if a sniffer wants to enable it then it is best done from the application.
In either case the user should not need to directly set this through ip commands.

Yes, there are a lots of incorrect web pages out there that say you need to
set an interface into promiscious mode (with ifconfig) before adding it to a bridge.
That might have been true 20 years ago, but hasn't been needed since Linux 2.4

Bottom line, adding this to the documentation is not going to be helpful.

^ permalink raw reply

* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Jonathan Lemon @ 2019-02-11 19:48 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: bjorn.topel, ast, daniel, netdev, jakub.kicinski, bjorn.topel,
	qi.z.zhang, brouer, xiaolong.ye
In-Reply-To: <1549631126-29067-1-git-send-email-magnus.karlsson@intel.com>

On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:

> This patch proposes to add AF_XDP support to libbpf. The main reason
> for this is to facilitate writing applications that use AF_XDP by
> offering higher-level APIs that hide many of the details of the AF_XDP
> uapi. This is in the same vein as libbpf facilitates XDP adoption by
> offering easy-to-use higher level interfaces of XDP
> functionality. Hopefully this will facilitate adoption of AF_XDP, make
> applications using it simpler and smaller, and finally also make it
> possible for applications to benefit from optimizations in the AF_XDP
> user space access code. Previously, people just copied and pasted the
> code from the sample application into their application, which is not
> desirable.

I like the idea of encapsulating the boilerplate logic in a library.

I do think there is an important missing piece though - there should be
some code which queries the netdev for how many queues are attached, and
create the appropriate number of umem/AF_XDP sockets.

I ran into this issue when testing the current AF_XDP code - on my test
boxes, the mlx5 card has 55 channels (aka queues), so when the test program
binds only to channel 0, nothing works as expected, since not all traffic
is being intercepted.  While obvious in hindsight, this took a while to
track down.
-- 
Jonathan

^ permalink raw reply

* Re: [PATCH mlx5-next 2/2] net/mlx5: Factor out HCA capabilities functions
From: Jason Gunthorpe @ 2019-02-11 19:50 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Moni Shoua,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190211115608.22677-3-leon@kernel.org>

On Mon, Feb 11, 2019 at 01:56:08PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Combine all HCA capabilities setters under one function
> and compile out the ODP related function in case kernel
> was compiled without ODP support.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  .../net/ethernet/mellanox/mlx5/core/main.c    | 47 +++++++++++++------
>  1 file changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 6d45518edbdc..d7145ab6105d 100644
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -459,6 +459,7 @@ static int handle_hca_cap_atomic(struct mlx5_core_dev *dev)
>  	return err;
>  }
>  
> +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
>  static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
>  {
>  	void *set_hca_cap;
> @@ -502,6 +503,7 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
>  	kfree(set_ctx);
>  	return err;
>  }
> +#endif
>  
>  static int handle_hca_cap(struct mlx5_core_dev *dev)
>  {
> @@ -576,6 +578,35 @@ static int handle_hca_cap(struct mlx5_core_dev *dev)
>  	return err;
>  }
>  
> +static int set_hca_cap(struct mlx5_core_dev *dev)
> +{
> +	struct pci_dev *pdev = dev->pdev;
> +	int err;
> +
> +	err = handle_hca_cap(dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "handle_hca_cap failed\n");
> +		goto out;
> +	}
> +
> +	err = handle_hca_cap_atomic(dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "handle_hca_cap_atomic failed\n");
> +		goto out;
> +	}
> +
> +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> +	err = handle_hca_cap_odp(dev);
> +	if (err) {
> +		dev_err(&pdev->dev, "handle_hca_cap_odp failed\n");
> +		goto out;
> +	}
> +#endif

Adding 
  if (IS_ENABLED..) 
    return 0;

To the top of handle_hca_cap_odp is alot better.

Jason

^ permalink raw reply

* Re: [PATCH v2] xsk: share the mmap_sem for page pinning
From: Daniel Borkmann @ 2019-02-11 19:53 UTC (permalink / raw)
  To: akpm, linux-mm, linux-kernel, David S . Miller, Bjorn Topel,
	Magnus Karlsson, netdev, Davidlohr Bueso
In-Reply-To: <20190211161529.uskq5ca7y3j5522i@linux-r8p5>

On 02/11/2019 05:15 PM, Davidlohr Bueso wrote:
> Holding mmap_sem exclusively for a gup() is an overkill.
> Lets share the lock and replace the gup call for gup_longterm(),
> as it is better suited for the lifetime of the pinning.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Bjorn Topel <bjorn.topel@intel.com>
> Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> CC: netdev@vger.kernel.org
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH mlx5-next 2/2] net/mlx5: Factor out HCA capabilities functions
From: Leon Romanovsky @ 2019-02-11 20:02 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Doug Ledford, RDMA mailing list, Moni Shoua, Saeed Mahameed,
	linux-netdev
In-Reply-To: <20190211195050.GL24706@mellanox.com>

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

On Mon, Feb 11, 2019 at 07:50:55PM +0000, Jason Gunthorpe wrote:
> On Mon, Feb 11, 2019 at 01:56:08PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Combine all HCA capabilities setters under one function
> > and compile out the ODP related function in case kernel
> > was compiled without ODP support.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  .../net/ethernet/mellanox/mlx5/core/main.c    | 47 +++++++++++++------
> >  1 file changed, 33 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > index 6d45518edbdc..d7145ab6105d 100644
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> > @@ -459,6 +459,7 @@ static int handle_hca_cap_atomic(struct mlx5_core_dev *dev)
> >  	return err;
> >  }
> >
> > +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> >  static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
> >  {
> >  	void *set_hca_cap;
> > @@ -502,6 +503,7 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
> >  	kfree(set_ctx);
> >  	return err;
> >  }
> > +#endif
> >
> >  static int handle_hca_cap(struct mlx5_core_dev *dev)
> >  {
> > @@ -576,6 +578,35 @@ static int handle_hca_cap(struct mlx5_core_dev *dev)
> >  	return err;
> >  }
> >
> > +static int set_hca_cap(struct mlx5_core_dev *dev)
> > +{
> > +	struct pci_dev *pdev = dev->pdev;
> > +	int err;
> > +
> > +	err = handle_hca_cap(dev);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "handle_hca_cap failed\n");
> > +		goto out;
> > +	}
> > +
> > +	err = handle_hca_cap_atomic(dev);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "handle_hca_cap_atomic failed\n");
> > +		goto out;
> > +	}
> > +
> > +#ifdef CONFIG_INFINIBAND_ON_DEMAND_PAGING
> > +	err = handle_hca_cap_odp(dev);
> > +	if (err) {
> > +		dev_err(&pdev->dev, "handle_hca_cap_odp failed\n");
> > +		goto out;
> > +	}
> > +#endif
>
> Adding
>   if (IS_ENABLED..)
>     return 0;
>
> To the top of handle_hca_cap_odp is alot better.

Saeed gave comment that he prefers code to be compiled-out in case
config is not set. In you suggestion, the code will exist and only
with some optimizations enabled it will be thrown.

Thanks

>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH 0/9] perf annotation of BPF programs
From: Song Liu @ 2019-02-11 20:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	ast@kernel.org, daniel@iogearbox.net, Kernel Team,
	peterz@infradead.org, Jiri Olsa, Namhyung Kim, Andi Kleen,
	Stephane Eranian, Arnaldo Carvalho de Melo
In-Reply-To: <20190211185400.GA2084@redhat.com>



> On Feb 11, 2019, at 10:54 AM, Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> 
> Em Fri, Feb 08, 2019 at 05:16:56PM -0800, Song Liu escreveu:
>> This series enables annotation of BPF programs in perf.
>> 
>> perf tool gathers information via sys_bpf and (optionally) stores them in
>> perf.data as headers.
> 
> Jiri, Stephane, this is the patchkit I mentioned in the context of doing
> away with perf.data headers and instead package everything as userspace
> PERF_RECORD_ metadata events.
> 
> Song, please add Jiri and Namhyung in future perf patchkits, they are
> listed as perf tools reviewers in MAINTAINANERS and Jiri also is working
> on something directly related.
> 
> Thanks,
> 
> - Arnaldo

Thanks Arnaldo! I will keep Jiri and Namhyung in the loop. 

Song




>> Patch 1/9 fixes a minor issue in kernel;
>> Patch 2/9 to 4/9 introduce new helper functions and use them in perf and
>>     bpftool;
>> Patch 5/9 and 6/9 saves information of bpf program in perf_env;
>> Patch 7/9 adds --bpf-event options to perf-top;
>> Patch 8/9 enables annotation of bpf programs based on information gathered
>>     in 5/9 and 6/9;
>> Patch 9/9 handles information of short living BPF program that are loaded
>>     during perf-record or perf-top.
>> 
>> Commands tested during developments are perf-top, perf-record, perf-report,
>> and perf-annotate.
>> 
>> ===================== Note on patch dependency  ========================
>> This set has dependency in both bpf-next tree and tip/perf/core. Current
>> version is developed on bpf-next tree with the following commits
>> cherry-picked from tip/perf/core:
>> 
>> (from 1/10 to 10/10)
>> commit 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
>> commit d764ac646491 ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
>> commit 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
>> commit df063c83aa2c ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
>> commit 9aa0bfa370b2 ("perf tools: Handle PERF_RECORD_KSYMBOL")
>> commit 45178a928a4b ("perf tools: Handle PERF_RECORD_BPF_EVENT")
>> commit 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
>> commit a40b95bcd30c ("perf top: Synthesize BPF events for pre-existing loaded BPF programs")
>> commit 6934058d9fb6 ("bpf: Add module name [bpf] to ksymbols for bpf programs")
>> commit 811184fb6977 ("perf bpf: Fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT")
>> ========================================================================
>> 
>> Song Liu (9):
>>  perf, bpf: consider events with attr.bpf_event as side-band events
>>  bpf: libbpf: introduce bpf_program__get_prog_info_linear()
>>  bpf: bpftool: use bpf_program__get_prog_info_linear() in
>>    prog.c:do_dump()
>>  perf, bpf: synthesize bpf events with
>>    bpf_program__get_prog_info_linear()
>>  perf, bpf: save bpf_prog_info in a rbtree in perf_env
>>  perf, bpf: save btf in a rbtree in perf_env
>>  perf-top: add option --bpf-event
>>  perf, bpf: enable annotation of bpf program
>>  perf, bpf: save information about short living bpf programs
>> 
>> kernel/events/core.c        |   3 +-
>> tools/bpf/bpftool/prog.c    | 266 ++++++---------------------
>> tools/lib/bpf/libbpf.c      | 251 ++++++++++++++++++++++++++
>> tools/lib/bpf/libbpf.h      |  63 +++++++
>> tools/lib/bpf/libbpf.map    |   3 +
>> tools/perf/Makefile.config  |   2 +-
>> tools/perf/builtin-record.c |  15 +-
>> tools/perf/builtin-top.c    |  15 +-
>> tools/perf/util/annotate.c  | 149 ++++++++++++++-
>> tools/perf/util/bpf-event.c | 351 +++++++++++++++++++++++++++---------
>> tools/perf/util/bpf-event.h |  48 ++++-
>> tools/perf/util/dso.c       |   1 +
>> tools/perf/util/dso.h       |  33 ++--
>> tools/perf/util/env.c       | 148 +++++++++++++++
>> tools/perf/util/env.h       |  12 ++
>> tools/perf/util/evlist.c    |  20 ++
>> tools/perf/util/evlist.h    |   2 +
>> tools/perf/util/header.c    | 231 +++++++++++++++++++++++-
>> tools/perf/util/header.h    |   2 +
>> tools/perf/util/symbol.c    |   1 +
>> 20 files changed, 1304 insertions(+), 312 deletions(-)
>> 
>> --
>> 2.17.1


^ permalink raw reply

* Re: [PATCH bpf] bpf: only adjust gso_size on bytestream protocols
From: Willem de Bruijn @ 2019-02-11 20:11 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Network Development, Alexei Starovoitov,
	Peter Oskolkov, Daniel Axtens, Willem de Bruijn
In-Reply-To: <25e33f72-b04a-3229-ce8e-2320d29b8ee6@iogearbox.net>

On Mon, Feb 11, 2019 at 9:58 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Hi Willem,
>
> On 02/11/2019 05:00 AM, Alexei Starovoitov wrote:
> > On Thu, Feb 07, 2019 at 02:54:16PM -0500, Willem de Bruijn wrote:
> >> From: Willem de Bruijn <willemb@google.com>
> >>
> >> bpf_skb_change_proto and bpf_skb_adjust_room change skb header length.
> >> For GSO packets they adjust gso_size to maintain the same MTU.
> >>
> >> The gso size can only be safely adjusted on bytestream protocols.
> >> Commit d02f51cbcf12 ("bpf: fix bpf_skb_adjust_net/bpf_skb_proto_xlat
> >> to deal with gso sctp skbs") excluded SKB_GSO_SCTP.
> >>
> >> Since then type SKB_GSO_UDP_L4 has been added, whose contents are one
> >> gso_size unit per datagram. Also exclude these.
> >>
> >> Move from a blacklist to a whitelist check to future proof against
> >> additional such new GSO types, e.g., for fraglist based GRO.
> >>
> >> Fixes: bec1f6f69736 ("udp: generate gso with UDP_SEGMENT")
> >> Signed-off-by: Willem de Bruijn <willemb@google.com>
> >
> > Applied to bpf tree.
> > I agree that whitelist approach is the most appropriate.
>
> What would be needed to get UDP GSO working with nat64 work above? I don't
> really mind about SCTP, but sucks that this doesn't guarantee full support
> for TCP *and* UDP at least. :/

The easy part is shrinking headers in bpf_skb_net_shrink and
bpf_skb_proto_6_to_4. Those are safe if they adjust gso_size only if
skb_is_gso_tcp(skb).

Growing headers, whether with nat64 or in-review BPF_LWT_ENCAP_IP, is
fine if the original gso_size was chosen sufficiently below MSS to
account for the possible transformation.

Though this is not so cheap to verify here. But the same MTU concern
exists for non-GSO packets. Those are also adjusted unconditionally,
as far as I can tell. We do not need to add an MTU check solely for GSO.

For both GSO and non-GSO, for egress transformation, an admin
inserting such BPF programs can at least add an explicit route mtu to
force processes to limit the size they generate. Analogous to how tunnel
devices derive their mtu from their destination device minus encap headers.

^ permalink raw reply

* Re: [net-next PATCH 1/2] mm: add dma_addr_t to struct page
From: Andrew Morton @ 2019-02-11 20:16 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: netdev, linux-mm, Toke Høiland-Jørgensen,
	Ilias Apalodimas, willy, Saeed Mahameed, mgorman, David S. Miller,
	Tariq Toukan
In-Reply-To: <154990120685.24530.15350136329514629029.stgit@firesoul>

On Mon, 11 Feb 2019 17:06:46 +0100 Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> The page_pool API is using page->private to store DMA addresses.
> As pointed out by David Miller we can't use that on 32-bit architectures
> with 64-bit DMA
> 
> This patch adds a new dma_addr_t struct to allow storing DMA addresses
> 
> ..
>
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -95,6 +95,14 @@ struct page {
>  			 */
>  			unsigned long private;
>  		};
> +		struct {	/* page_pool used by netstack */
> +			/**
> +			 * @dma_addr: Page_pool need to store DMA-addr, and
> +			 * cannot use @private, as DMA-mappings can be 64-bit
> +			 * even on 32-bit Architectures.
> +			 */

This comment is a bit awkward.  The discussion about why it doesn't use
->private is uninteresting going forward and is more material for a
changelog.

How about

			/**
			 * @dma_addr: page_pool requires a 64-bit value even on
			 * 32-bit architectures.
			 */

Otherwise,

Acked-by: Andrew Morton <akpm@linux-foundation.org>

^ permalink raw reply

* [PATCH 0/3] Add gup fast + longterm and use it in HFI1
From: ira.weiny @ 2019-02-11 20:16 UTC (permalink / raw)
  To: linux-rdma, linux-kernel, linux-mm, Daniel Borkmann,
	Davidlohr Bueso, netdev
  Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford,
	Jason Gunthorpe, Andrew Morton, Kirill A. Shutemov, Dan Williams,
	Ira Weiny

From: Ira Weiny <ira.weiny@intel.com>

NOTE: This series depends on my clean up patch to remove the write parameter
from gup_fast_permitted()[1]

HFI1 uses get_user_pages_fast() due to it performance advantages.  Like RDMA,
HFI1 pages can be held for a significant time.  But get_user_pages_fast() does
not protect against mapping of FS DAX pages.

Introduce a get_user_pages_fast_longterm() which retains the performance while
also adding the FS DAX checks.  XDP has also shown interest in using this
functionality.[2]

[1] https://lkml.org/lkml/2019/2/11/237
[2] https://lkml.org/lkml/2019/2/11/1789

Ira Weiny (3):
  mm/gup: Change "write" parameter to flags
  mm/gup: Introduce get_user_pages_fast_longterm()
  IB/HFI1: Use new get_user_pages_fast_longterm()

 drivers/infiniband/hw/hfi1/user_pages.c |   2 +-
 include/linux/mm.h                      |   8 ++
 mm/gup.c                                | 152 ++++++++++++++++--------
 3 files changed, 114 insertions(+), 48 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 3/3] IB/HFI1: Use new get_user_pages_fast_longterm()
From: ira.weiny @ 2019-02-11 20:16 UTC (permalink / raw)
  To: linux-rdma, linux-kernel, linux-mm, Daniel Borkmann,
	Davidlohr Bueso, netdev
  Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford,
	Jason Gunthorpe, Andrew Morton, Kirill A. Shutemov, Dan Williams,
	Ira Weiny
In-Reply-To: <20190211201643.7599-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

Use the new get_user_pages_fast_longterm() call to protect against
FS DAX pages being mapped.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/infiniband/hw/hfi1/user_pages.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hfi1/user_pages.c b/drivers/infiniband/hw/hfi1/user_pages.c
index 24b592c6522e..b94ab5385a09 100644
--- a/drivers/infiniband/hw/hfi1/user_pages.c
+++ b/drivers/infiniband/hw/hfi1/user_pages.c
@@ -105,7 +105,7 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
 {
 	int ret;
 
-	ret = get_user_pages_fast(vaddr, npages, writable, pages);
+	ret = get_user_pages_fast_longterm(vaddr, npages, writable, pages);
 	if (ret < 0)
 		return ret;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/3] mm/gup: Introduce get_user_pages_fast_longterm()
From: ira.weiny @ 2019-02-11 20:16 UTC (permalink / raw)
  To: linux-rdma, linux-kernel, linux-mm, Daniel Borkmann,
	Davidlohr Bueso, netdev
  Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford,
	Jason Gunthorpe, Andrew Morton, Kirill A. Shutemov, Dan Williams,
	Ira Weiny
In-Reply-To: <20190211201643.7599-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

Users of get_user_pages_fast are not protected against mapping
pages within FS DAX.  Introduce a call which protects them.

We do this by checking for DEVMAP pages during the fast walk and
falling back to the longterm gup call to check for FS DAX if needed.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 include/linux/mm.h |   8 ++++
 mm/gup.c           | 102 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 80bb6408fe73..8f831c823630 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1540,6 +1540,8 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
 long get_user_pages_longterm(unsigned long start, unsigned long nr_pages,
 			    unsigned int gup_flags, struct page **pages,
 			    struct vm_area_struct **vmas);
+int get_user_pages_fast_longterm(unsigned long start, int nr_pages, bool write,
+				 struct page **pages);
 #else
 static inline long get_user_pages_longterm(unsigned long start,
 		unsigned long nr_pages, unsigned int gup_flags,
@@ -1547,6 +1549,11 @@ static inline long get_user_pages_longterm(unsigned long start,
 {
 	return get_user_pages(start, nr_pages, gup_flags, pages, vmas);
 }
+static inline int get_user_pages_fast_longterm(unsigned long start, int nr_pages,
+					       bool write, struct page **pages)
+{
+	return get_user_pages_fast(start, nr_pages, write, pages);
+}
 #endif /* CONFIG_FS_DAX */
 
 int get_user_pages_fast(unsigned long start, int nr_pages, int write,
@@ -2615,6 +2622,7 @@ struct page *follow_page(struct vm_area_struct *vma, unsigned long address,
 #define FOLL_REMOTE	0x2000	/* we are working on non-current tsk/mm */
 #define FOLL_COW	0x4000	/* internal GUP flag */
 #define FOLL_ANON	0x8000	/* don't do file mappings */
+#define FOLL_LONGTERM	0x10000	/* mapping is intended for a long term pin */
 
 static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
 {
diff --git a/mm/gup.c b/mm/gup.c
index 894ab014bd1e..f7d86a304405 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1190,6 +1190,21 @@ long get_user_pages_longterm(unsigned long start, unsigned long nr_pages,
 EXPORT_SYMBOL(get_user_pages_longterm);
 #endif /* CONFIG_FS_DAX */
 
+static long get_user_pages_longterm_unlocked(unsigned long start,
+					     unsigned long nr_pages,
+					     struct page **pages,
+					     unsigned int gup_flags)
+{
+	struct mm_struct *mm = current->mm;
+	long ret;
+
+	down_read(&mm->mmap_sem);
+	ret = get_user_pages_longterm(start, nr_pages, gup_flags, pages, NULL);
+	up_read(&mm->mmap_sem);
+
+	return ret;
+}
+
 /**
  * populate_vma_page_range() -  populate a range of pages in the vma.
  * @vma:   target vma
@@ -1417,6 +1432,9 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
 			goto pte_unmap;
 
 		if (pte_devmap(pte)) {
+			if (flags & FOLL_LONGTERM)
+				goto pte_unmap;
+
 			pgmap = get_dev_pagemap(pte_pfn(pte), pgmap);
 			if (unlikely(!pgmap)) {
 				undo_dev_pagemap(nr, nr_start, pages);
@@ -1556,8 +1574,12 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
 	if (!pmd_access_permitted(orig, flags & FOLL_WRITE))
 		return 0;
 
-	if (pmd_devmap(orig))
+	if (pmd_devmap(orig)) {
+		if (flags & FOLL_LONGTERM)
+			return 0;
+
 		return __gup_device_huge_pmd(orig, pmdp, addr, end, pages, nr);
+	}
 
 	refs = 0;
 	page = pmd_page(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
@@ -1837,24 +1859,9 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	return nr;
 }
 
-/**
- * get_user_pages_fast() - pin user pages in memory
- * @start:	starting user address
- * @nr_pages:	number of pages from start to pin
- * @write:	whether pages will be written to
- * @pages:	array that receives pointers to the pages pinned.
- *		Should be at least nr_pages long.
- *
- * Attempt to pin user pages in memory without taking mm->mmap_sem.
- * If not successful, it will fall back to taking the lock and
- * calling get_user_pages().
- *
- * Returns number of pages pinned. This may be fewer than the number
- * requested. If nr_pages is 0 or negative, returns 0. If no pages
- * were pinned, returns -errno.
- */
-int get_user_pages_fast(unsigned long start, int nr_pages, int write,
-			struct page **pages)
+static int __get_user_pages_fast_flags(unsigned long start, int nr_pages,
+				       unsigned int gup_flags,
+				       struct page **pages)
 {
 	unsigned long addr, len, end;
 	int nr = 0, ret = 0;
@@ -1872,7 +1879,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 
 	if (gup_fast_permitted(start, nr_pages)) {
 		local_irq_disable();
-		gup_pgd_range(addr, end, write ? FOLL_WRITE : 0, pages, &nr);
+		gup_pgd_range(addr, end, gup_flags, pages, &nr);
 		local_irq_enable();
 		ret = nr;
 	}
@@ -1882,8 +1889,14 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 		start += nr << PAGE_SHIFT;
 		pages += nr;
 
-		ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
-				write ? FOLL_WRITE : 0);
+		if (gup_flags & FOLL_LONGTERM)
+			ret = get_user_pages_longterm_unlocked(start,
+							       nr_pages - nr,
+							       pages,
+							       gup_flags);
+		else
+			ret = get_user_pages_unlocked(start, nr_pages - nr,
+						      pages, gup_flags);
 
 		/* Have to be a bit careful with return values */
 		if (nr > 0) {
@@ -1897,4 +1910,49 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 	return ret;
 }
 
+/**
+ * get_user_pages_fast() - pin user pages in memory
+ * @start:	starting user address
+ * @nr_pages:	number of pages from start to pin
+ * @write:	whether pages will be written to
+ * @pages:	array that receives pointers to the pages pinned.
+ *		Should be at least nr_pages long.
+ *
+ * Attempt to pin user pages in memory without taking mm->mmap_sem.
+ * If not successful, it will fall back to taking the lock and
+ * calling get_user_pages().
+ *
+ * Returns number of pages pinned. This may be fewer than the number
+ * requested. If nr_pages is 0 or negative, returns 0. If no pages
+ * were pinned, returns -errno.
+ */
+int get_user_pages_fast(unsigned long start, int nr_pages, int write,
+			struct page **pages)
+{
+	return __get_user_pages_fast_flags(start, nr_pages,
+					   write ? FOLL_WRITE : 0,
+					   pages);
+}
+
+#ifdef CONFIG_FS_DAX
+/**
+ * get_user_pages_fast_longterm() - pin user pages in memory
+ *
+ * Exactly the same semantics as get_user_pages_fast() except fails mappings
+ * device mapped pages (such as DAX pages) which then fall back to checking for
+ * FS DAX pages with get_user_pages_longterm().
+ */
+int get_user_pages_fast_longterm(unsigned long start, int nr_pages, bool write,
+				 struct page **pages)
+{
+	unsigned int gup_flags = FOLL_LONGTERM;
+
+	if (write)
+		gup_flags |= FOLL_WRITE;
+
+	return __get_user_pages_fast_flags(start, nr_pages, gup_flags, pages);
+}
+EXPORT_SYMBOL(get_user_pages_fast_longterm);
+#endif /* CONFIG_FS_DAX */
+
 #endif /* CONFIG_HAVE_GENERIC_GUP */
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/3] mm/gup: Change "write" parameter to flags
From: ira.weiny @ 2019-02-11 20:16 UTC (permalink / raw)
  To: linux-rdma, linux-kernel, linux-mm, Daniel Borkmann,
	Davidlohr Bueso, netdev
  Cc: Mike Marciniszyn, Dennis Dalessandro, Doug Ledford,
	Jason Gunthorpe, Andrew Morton, Kirill A. Shutemov, Dan Williams,
	Ira Weiny
In-Reply-To: <20190211201643.7599-1-ira.weiny@intel.com>

From: Ira Weiny <ira.weiny@intel.com>

In order to support more options in the GUP fast walk, change the
write parameter to flags throughout the call stack.

This patch does not change functionality and passes FOLL_WRITE
where write was previously used.

Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 mm/gup.c | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index b63e88eca31b..894ab014bd1e 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1395,7 +1395,7 @@ static void undo_dev_pagemap(int *nr, int nr_start, struct page **pages)
 
 #ifdef CONFIG_ARCH_HAS_PTE_SPECIAL
 static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
-			 int write, struct page **pages, int *nr)
+			 unsigned int flags, struct page **pages, int *nr)
 {
 	struct dev_pagemap *pgmap = NULL;
 	int nr_start = *nr, ret = 0;
@@ -1413,7 +1413,7 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
 		if (pte_protnone(pte))
 			goto pte_unmap;
 
-		if (!pte_access_permitted(pte, write))
+		if (!pte_access_permitted(pte, flags & FOLL_WRITE))
 			goto pte_unmap;
 
 		if (pte_devmap(pte)) {
@@ -1465,7 +1465,7 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
  * useful to have gup_huge_pmd even if we can't operate on ptes.
  */
 static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
-			 int write, struct page **pages, int *nr)
+			 unsigned int flags, struct page **pages, int *nr)
 {
 	return 0;
 }
@@ -1548,12 +1548,12 @@ static int __gup_device_huge_pud(pud_t pud, pud_t *pudp, unsigned long addr,
 #endif
 
 static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
-		unsigned long end, int write, struct page **pages, int *nr)
+		unsigned long end, unsigned int flags, struct page **pages, int *nr)
 {
 	struct page *head, *page;
 	int refs;
 
-	if (!pmd_access_permitted(orig, write))
+	if (!pmd_access_permitted(orig, flags & FOLL_WRITE))
 		return 0;
 
 	if (pmd_devmap(orig))
@@ -1586,12 +1586,12 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
 }
 
 static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
-		unsigned long end, int write, struct page **pages, int *nr)
+		unsigned long end, unsigned int flags, struct page **pages, int *nr)
 {
 	struct page *head, *page;
 	int refs;
 
-	if (!pud_access_permitted(orig, write))
+	if (!pud_access_permitted(orig, flags & FOLL_WRITE))
 		return 0;
 
 	if (pud_devmap(orig))
@@ -1624,13 +1624,13 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
 }
 
 static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
-			unsigned long end, int write,
+			unsigned long end, unsigned int flags,
 			struct page **pages, int *nr)
 {
 	int refs;
 	struct page *head, *page;
 
-	if (!pgd_access_permitted(orig, write))
+	if (!pgd_access_permitted(orig, flags & FOLL_WRITE))
 		return 0;
 
 	BUILD_BUG_ON(pgd_devmap(orig));
@@ -1661,7 +1661,7 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
 }
 
 static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
-		int write, struct page **pages, int *nr)
+		unsigned int flags, struct page **pages, int *nr)
 {
 	unsigned long next;
 	pmd_t *pmdp;
@@ -1683,7 +1683,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
 			if (pmd_protnone(pmd))
 				return 0;
 
-			if (!gup_huge_pmd(pmd, pmdp, addr, next, write,
+			if (!gup_huge_pmd(pmd, pmdp, addr, next, flags,
 				pages, nr))
 				return 0;
 
@@ -1693,9 +1693,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
 			 * pmd format and THP pmd format
 			 */
 			if (!gup_huge_pd(__hugepd(pmd_val(pmd)), addr,
-					 PMD_SHIFT, next, write, pages, nr))
+					 PMD_SHIFT, next, flags, pages, nr))
 				return 0;
-		} else if (!gup_pte_range(pmd, addr, next, write, pages, nr))
+		} else if (!gup_pte_range(pmd, addr, next, flags, pages, nr))
 			return 0;
 	} while (pmdp++, addr = next, addr != end);
 
@@ -1703,7 +1703,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, unsigned long end,
 }
 
 static int gup_pud_range(p4d_t p4d, unsigned long addr, unsigned long end,
-			 int write, struct page **pages, int *nr)
+			 unsigned int flags, struct page **pages, int *nr)
 {
 	unsigned long next;
 	pud_t *pudp;
@@ -1716,14 +1716,14 @@ static int gup_pud_range(p4d_t p4d, unsigned long addr, unsigned long end,
 		if (pud_none(pud))
 			return 0;
 		if (unlikely(pud_huge(pud))) {
-			if (!gup_huge_pud(pud, pudp, addr, next, write,
+			if (!gup_huge_pud(pud, pudp, addr, next, flags,
 					  pages, nr))
 				return 0;
 		} else if (unlikely(is_hugepd(__hugepd(pud_val(pud))))) {
 			if (!gup_huge_pd(__hugepd(pud_val(pud)), addr,
-					 PUD_SHIFT, next, write, pages, nr))
+					 PUD_SHIFT, next, flags, pages, nr))
 				return 0;
-		} else if (!gup_pmd_range(pud, addr, next, write, pages, nr))
+		} else if (!gup_pmd_range(pud, addr, next, flags, pages, nr))
 			return 0;
 	} while (pudp++, addr = next, addr != end);
 
@@ -1731,7 +1731,7 @@ static int gup_pud_range(p4d_t p4d, unsigned long addr, unsigned long end,
 }
 
 static int gup_p4d_range(pgd_t pgd, unsigned long addr, unsigned long end,
-			 int write, struct page **pages, int *nr)
+			 unsigned int flags, struct page **pages, int *nr)
 {
 	unsigned long next;
 	p4d_t *p4dp;
@@ -1746,9 +1746,9 @@ static int gup_p4d_range(pgd_t pgd, unsigned long addr, unsigned long end,
 		BUILD_BUG_ON(p4d_huge(p4d));
 		if (unlikely(is_hugepd(__hugepd(p4d_val(p4d))))) {
 			if (!gup_huge_pd(__hugepd(p4d_val(p4d)), addr,
-					 P4D_SHIFT, next, write, pages, nr))
+					 P4D_SHIFT, next, flags, pages, nr))
 				return 0;
-		} else if (!gup_pud_range(p4d, addr, next, write, pages, nr))
+		} else if (!gup_pud_range(p4d, addr, next, flags, pages, nr))
 			return 0;
 	} while (p4dp++, addr = next, addr != end);
 
@@ -1756,7 +1756,7 @@ static int gup_p4d_range(pgd_t pgd, unsigned long addr, unsigned long end,
 }
 
 static void gup_pgd_range(unsigned long addr, unsigned long end,
-		int write, struct page **pages, int *nr)
+		unsigned int flags, struct page **pages, int *nr)
 {
 	unsigned long next;
 	pgd_t *pgdp;
@@ -1769,14 +1769,14 @@ static void gup_pgd_range(unsigned long addr, unsigned long end,
 		if (pgd_none(pgd))
 			return;
 		if (unlikely(pgd_huge(pgd))) {
-			if (!gup_huge_pgd(pgd, pgdp, addr, next, write,
+			if (!gup_huge_pgd(pgd, pgdp, addr, next, flags,
 					  pages, nr))
 				return;
 		} else if (unlikely(is_hugepd(__hugepd(pgd_val(pgd))))) {
 			if (!gup_huge_pd(__hugepd(pgd_val(pgd)), addr,
-					 PGDIR_SHIFT, next, write, pages, nr))
+					 PGDIR_SHIFT, next, flags, pages, nr))
 				return;
-		} else if (!gup_p4d_range(pgd, addr, next, write, pages, nr))
+		} else if (!gup_p4d_range(pgd, addr, next, flags, pages, nr))
 			return;
 	} while (pgdp++, addr = next, addr != end);
 }
@@ -1830,7 +1830,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
 
 	if (gup_fast_permitted(start, nr_pages)) {
 		local_irq_save(flags);
-		gup_pgd_range(start, end, write, pages, &nr);
+		gup_pgd_range(start, end, write ? FOLL_WRITE : 0, pages, &nr);
 		local_irq_restore(flags);
 	}
 
@@ -1872,7 +1872,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
 
 	if (gup_fast_permitted(start, nr_pages)) {
 		local_irq_disable();
-		gup_pgd_range(addr, end, write, pages, &nr);
+		gup_pgd_range(addr, end, write ? FOLL_WRITE : 0, pages, &nr);
 		local_irq_enable();
 		ret = nr;
 	}
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v4 0/9] net: Remove switchdev_ops
From: David Miller @ 2019-02-11 20:16 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, idosch, linux-kernel, devel, bridge, jiri, andrew,
	vivien.didelot
In-Reply-To: <20190211191001.8623-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 11 Feb 2019 11:09:52 -0800

> David, I would like to get Ido's feedback on this to make sure I did not
> miss something, thank you!

Ok, Ido please look at this when you can.

^ permalink raw reply

* [PATCH] Revert: "p54: Use skb_peek_tail() instead of direct head pointer accesses"
From: Matthew Whitehead @ 2019-02-11 20:20 UTC (permalink / raw)
  To: whiteheadm, netdev, davem; +Cc: Matthew Whitehead

Commit e3554197fc8fbb9656f62c18f9c9edd396394e16 causes a null pointer error.

kernel: p54pci 0000:07:00.0: enabling device (0000 -> 0002)
kernel: ieee80211 phy1: p54 detected a LM86 firmware
kernel: p54: rx_mtu reduced from 3240 to 2376
kernel: ieee80211 phy1: FW rev 2.13.1.0 - Softmac protocol 5.5
kernel: ieee80211 phy1: cryptographic accelerator WEP:YES, TKIP:YES, CCMP:YES
kernel: BUG: unable to handle kernel NULL pointer dereference at 00000000
kernel: *pde = 00000000
kernel: Oops: 0000 [#1] PREEMPT SMP
kernel: CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted 4.19.0.bisect-14.#871
kernel: Hardware name: IBM 2378RVU/2378RVU, BIOS 1RETDKWW (3.16 ) 04/19/2005
kernel: Workqueue: events request_firmware_work_func
kernel: EIP: p54_tx_pending+0xff/0x128 [p54common]
kernel: Code: 8b 4d dc 89 7e 30 89 56 34 0f b6 53 56 01 d7 89 79 04 8b 96 a0 00 00 00 f6 42 01 80 75 0c 80 7a 28 00 75 06 89 bb d4 01 00 00 <8b> 10 89 46 04 89 16 89 30 8b 45 ec 89 72 04 8b 55 e8 ff 43 2c e8
kernel: EAX: 00000000 EBX: ec6a2d60 ECX: ed4de568 EDX: ed4de568
kernel: ESI: ec4e0980 EDI: 00020264 EBP: c0071eb8 ESP: c0071e94
kernel: DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010082
kernel: CR0: 80050033 CR2: 00000000 CR3: 2f715000 CR4: 00000690
kernel: Call Trace:
kernel:  p54_tx+0x1a/0x1d [p54common]
kernel:  p54_download_eeprom+0xa6/0xfb [p54common]
kernel:  p54_read_eeprom+0x5c/0x99 [p54common]
kernel:  p54p_firmware_step2+0x50/0xcd [p54pci]
kernel:  request_firmware_work_func+0x2a/0x51
kernel:  process_one_work+0x16b/0x28e
kernel:  worker_thread+0x180/0x222
kernel:  kthread+0xce/0xd0
kernel:  ? cancel_delayed_work+0x5e/0x5e
kernel:  ? kthread_create_worker_on_cpu+0x1c/0x1c
kernel:  ret_from_fork+0x19/0x24
kernel: Modules linked in: p54pci p54common crc_ccitt mac80211 ipw2200 libipw lib80211 cfg80211 uhci_hcd pcmcia ehci_pci yenta_socket ehci_hcd rfkill i2c_i801 pcmcia_rsrc e1000 usbcore i2c_core pcmcia_core lpc_ich usb_common mfd_core floppy autofs4
kernel: CR2: 0000000000000000
kernel: ---[ end trace ddc1a265fd4f4bc6 ]---
kernel: EIP: p54_tx_pending+0xff/0x128 [p54common]
kernel: Code: 8b 4d dc 89 7e 30 89 56 34 0f b6 53 56 01 d7 89 79 04 8b 96 a0 00 00 00 f6 42 01 80 75 0c 80 7a 28 00 75 06 89 bb d4 01 00 00 <8b> 10 89 46 04 89 16 89 30 8b 45 ec 89 72 04 8b 55 e8 ff 43 2c e8
kernel: EAX: 00000000 EBX: ec6a2d60 ECX: ed4de568 EDX: ed4de568
kernel: ESI: ec4e0980 EDI: 00020264 EBP: c0071eb8 ESP: c16252e8
kernel: DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010082
kernel: CR0: 80050033 CR2: 00000000 CR3: 2f715000 CR4: 00000690
kernel: note: kworker/0:0[5] exited with preempt_count 1

Reverting the patch fixes the problem.

Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
---
 drivers/net/wireless/intersil/p54/txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/txrx.c b/drivers/net/wireless/intersil/p54/txrx.c
index 79078456..3a4214d 100644
--- a/drivers/net/wireless/intersil/p54/txrx.c
+++ b/drivers/net/wireless/intersil/p54/txrx.c
@@ -121,8 +121,8 @@ static int p54_assign_address(struct p54_common *priv, struct sk_buff *skb)
 	}
 	if (unlikely(!target_skb)) {
 		if (priv->rx_end - last_addr >= len) {
-			target_skb = skb_peek_tail(&priv->tx_queue);
-			if (target_skb) {
+			target_skb = priv->tx_queue.prev;
+			if (!skb_queue_empty(&priv->tx_queue)) {
 				info = IEEE80211_SKB_CB(target_skb);
 				range = (void *)info->rate_driver_data;
 				target_addr = range->end_addr;
-- 
1.8.3.1


^ permalink raw reply related


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