* [PATCH v3 11/17] qca: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-10 10:17 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, David S. Miller, Stefan Wahren,
Michael Heimpold, Yangtao Li
In-Reply-To: <20190810101732.26612-1-gregkh@linuxfoundation.org>
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Michael Heimpold <michael.heimpold@i2se.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/qualcomm/qca_debug.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index bcb890b18a94..702aa217a27a 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -131,17 +131,10 @@ DEFINE_SHOW_ATTRIBUTE(qcaspi_info);
void
qcaspi_init_device_debugfs(struct qcaspi *qca)
{
- struct dentry *device_root;
+ qca->device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev),
+ NULL);
- device_root = debugfs_create_dir(dev_name(&qca->net_dev->dev), NULL);
- qca->device_root = device_root;
-
- if (IS_ERR(device_root) || !device_root) {
- pr_warn("failed to create debugfs directory for %s\n",
- dev_name(&qca->net_dev->dev));
- return;
- }
- debugfs_create_file("info", S_IFREG | 0444, device_root, qca,
+ debugfs_create_file("info", S_IFREG | 0444, qca->device_root, qca,
&qcaspi_info_fops);
}
--
2.22.0
^ permalink raw reply related
* [PATCH v3 10/17] dpaa2: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-10 10:17 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Ioana Radulescu, David S. Miller
In-Reply-To: <20190810101732.26612-1-gregkh@linuxfoundation.org>
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Because we don't care about the individual files, we can remove the
stored dentry for the files, as they are not needed to be kept track of
at all.
Cc: Ioana Radulescu <ruxandra.radulescu@nxp.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../freescale/dpaa2/dpaa2-eth-debugfs.c | 54 +++----------------
.../freescale/dpaa2/dpaa2-eth-debugfs.h | 3 --
2 files changed, 7 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
index a027f4a9d0cc..a9afe46b837f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.c
@@ -164,70 +164,30 @@ static const struct file_operations dpaa2_dbg_ch_ops = {
void dpaa2_dbg_add(struct dpaa2_eth_priv *priv)
{
- if (!dpaa2_dbg_root)
- return;
+ struct dentry *dir;
/* Create a directory for the interface */
- priv->dbg.dir = debugfs_create_dir(priv->net_dev->name,
- dpaa2_dbg_root);
- if (!priv->dbg.dir) {
- netdev_err(priv->net_dev, "debugfs_create_dir() failed\n");
- return;
- }
+ dir = debugfs_create_dir(priv->net_dev->name, dpaa2_dbg_root);
+ priv->dbg.dir = dir;
/* per-cpu stats file */
- priv->dbg.cpu_stats = debugfs_create_file("cpu_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_cpu_ops);
- if (!priv->dbg.cpu_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_cpu_stats;
- }
+ debugfs_create_file("cpu_stats", 0444, dir, priv, &dpaa2_dbg_cpu_ops);
/* per-fq stats file */
- priv->dbg.fq_stats = debugfs_create_file("fq_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_fq_ops);
- if (!priv->dbg.fq_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_fq_stats;
- }
+ debugfs_create_file("fq_stats", 0444, dir, priv, &dpaa2_dbg_fq_ops);
/* per-fq stats file */
- priv->dbg.ch_stats = debugfs_create_file("ch_stats", 0444,
- priv->dbg.dir, priv,
- &dpaa2_dbg_ch_ops);
- if (!priv->dbg.fq_stats) {
- netdev_err(priv->net_dev, "debugfs_create_file() failed\n");
- goto err_ch_stats;
- }
-
- return;
-
-err_ch_stats:
- debugfs_remove(priv->dbg.fq_stats);
-err_fq_stats:
- debugfs_remove(priv->dbg.cpu_stats);
-err_cpu_stats:
- debugfs_remove(priv->dbg.dir);
+ debugfs_create_file("ch_stats", 0444, dir, priv, &dpaa2_dbg_ch_ops);
}
void dpaa2_dbg_remove(struct dpaa2_eth_priv *priv)
{
- debugfs_remove(priv->dbg.fq_stats);
- debugfs_remove(priv->dbg.ch_stats);
- debugfs_remove(priv->dbg.cpu_stats);
- debugfs_remove(priv->dbg.dir);
+ debugfs_remove_recursive(priv->dbg.dir);
}
void dpaa2_eth_dbg_init(void)
{
dpaa2_dbg_root = debugfs_create_dir(DPAA2_ETH_DBG_ROOT, NULL);
- if (!dpaa2_dbg_root) {
- pr_err("DPAA2-ETH: debugfs create failed\n");
- return;
- }
-
pr_debug("DPAA2-ETH: debugfs created\n");
}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
index 4f63de997a26..15598b28f03b 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-debugfs.h
@@ -11,9 +11,6 @@ struct dpaa2_eth_priv;
struct dpaa2_debugfs {
struct dentry *dir;
- struct dentry *fq_stats;
- struct dentry *ch_stats;
- struct dentry *cpu_stats;
};
#ifdef CONFIG_DEBUG_FS
--
2.22.0
^ permalink raw reply related
* [PATCH v3 00/17] Networking driver debugfs cleanups
From: Greg Kroah-Hartman @ 2019-08-10 10:17 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman
There is no need to test the result of any debugfs call anymore. The
debugfs core warns the user if something fails, and the return value of
a debugfs call can always be fed back into another debugfs call with no
problems.
Also, debugfs is for debugging, so if there are problems with debugfs
(i.e. the system is out of memory) the rest of the kernel should not
change behavior, so testing for debugfs calls is pointless and not the
goal of debugfs at all.
This series cleans up a lot of networking drivers and some wimax code
that was calling debugfs and trying to do something with the return
value that it didn't need to. Removing this logic makes the code
smaller, easier to understand, and use less run-time memory in some
cases, all good things.
The series is against net-next, and have no dependancies between any of
them if they want to go through any random tree/order. Or, if wanted,
I can take them through my driver-core tree where other debugfs cleanups
are being slowly fed during major merge windows.
thanks,
greg k-h
v3: fix build warning in i2400m, I thought I had caught them all :(
add acks from some reviewers
v2: fix up build warnings, it's as if I never even built these. Ugh, so
sorry for wasting people's time with the v1 series. I need to stop
relying on 0-day as it isn't working well anymore :(
Greg Kroah-Hartman (17):
wimax: no need to check return value of debugfs_create functions
bonding: no need to print a message if debugfs_create_dir() fails
mlx5: no need to check return value of debugfs_create functions
xgbe: no need to check return value of debugfs_create functions
bnxt: no need to check return value of debugfs_create functions
cxgb4: no need to check return value of debugfs_create functions
hns3: no need to check return value of debugfs_create functions
nfp: no need to check return value of debugfs_create functions
stmmac: no need to check return value of debugfs_create functions
dpaa2: no need to check return value of debugfs_create functions
qca: no need to check return value of debugfs_create functions
skge: no need to check return value of debugfs_create functions
mvpp2: no need to check return value of debugfs_create functions
fm10k: no need to check return value of debugfs_create functions
i40e: no need to check return value of debugfs_create functions
ixgbe: no need to check return value of debugfs_create functions
ieee802154: no need to check return value of debugfs_create functions
drivers/net/bonding/bond_debugfs.c | 5 -
drivers/net/ethernet/amd/xgbe/xgbe-debugfs.c | 107 ++++---------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 -
.../net/ethernet/broadcom/bnxt/bnxt_debugfs.c | 39 ++---
.../ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 5 +-
.../net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 -
.../ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 21 +--
.../freescale/dpaa2/dpaa2-eth-debugfs.c | 54 +------
.../freescale/dpaa2/dpaa2-eth-debugfs.h | 3 -
.../ethernet/hisilicon/hns3/hns3_debugfs.c | 17 +-
.../net/ethernet/intel/fm10k/fm10k_debugfs.c | 2 -
.../net/ethernet/intel/i40e/i40e_debugfs.c | 22 +--
.../net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 22 +--
.../ethernet/marvell/mvpp2/mvpp2_debugfs.c | 19 +--
drivers/net/ethernet/marvell/skge.c | 39 ++---
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 +-----
.../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++----------
drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
.../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
.../ethernet/netronome/nfp/nfp_net_debugfs.c | 17 +-
drivers/net/ethernet/qualcomm/qca_debug.c | 13 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 -
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 52 +-----
drivers/net/ieee802154/adf7242.c | 13 +-
drivers/net/ieee802154/at86rf230.c | 20 +--
drivers/net/ieee802154/ca8210.c | 9 +-
drivers/net/wimax/i2400m/debugfs.c | 150 +++---------------
drivers/net/wimax/i2400m/driver.c | 7 +-
drivers/net/wimax/i2400m/i2400m.h | 7 +-
drivers/net/wimax/i2400m/usb.c | 64 ++------
include/linux/mlx5/driver.h | 12 +-
include/linux/wimax/debug.h | 20 +--
net/wimax/debugfs.c | 42 +----
net/wimax/stack.c | 11 +-
net/wimax/wimax-internal.h | 7 +-
37 files changed, 175 insertions(+), 805 deletions(-)
--
2.22.0
^ permalink raw reply
* [PATCH v3 01/17] wimax: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-10 10:17 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Inaky Perez-Gonzalez, linux-wimax
In-Reply-To: <20190810101732.26612-1-gregkh@linuxfoundation.org>
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
This cleans up a lot of unneeded code and logic around the debugfs wimax
files, making all of this much simpler and easier to understand.
Cc: Inaky Perez-Gonzalez <inaky.perez-gonzalez@intel.com>
Cc: linux-wimax@intel.com
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/wimax/i2400m/debugfs.c | 150 +++++------------------------
drivers/net/wimax/i2400m/driver.c | 7 +-
drivers/net/wimax/i2400m/i2400m.h | 7 +-
drivers/net/wimax/i2400m/usb.c | 64 +++---------
include/linux/wimax/debug.h | 20 +---
net/wimax/debugfs.c | 42 ++------
net/wimax/stack.c | 11 +--
net/wimax/wimax-internal.h | 7 +-
8 files changed, 51 insertions(+), 257 deletions(-)
diff --git a/drivers/net/wimax/i2400m/debugfs.c b/drivers/net/wimax/i2400m/debugfs.c
index 6544ac9df047..73f5892ce6c1 100644
--- a/drivers/net/wimax/i2400m/debugfs.c
+++ b/drivers/net/wimax/i2400m/debugfs.c
@@ -30,15 +30,6 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_netdev_queue_stopped,
debugfs_netdev_queue_stopped_get,
NULL, "%llu\n");
-
-static
-struct dentry *debugfs_create_netdev_queue_stopped(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
-{
- return debugfs_create_file(name, 0400, parent, i2400m,
- &fops_netdev_queue_stopped);
-}
-
/*
* We don't allow partial reads of this file, as then the reader would
* get weirdly confused data as it is updated.
@@ -167,15 +158,6 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_i2400m_suspend,
NULL, debugfs_i2400m_suspend_set,
"%llu\n");
-static
-struct dentry *debugfs_create_i2400m_suspend(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
-{
- return debugfs_create_file(name, 0200, parent, i2400m,
- &fops_i2400m_suspend);
-}
-
-
/*
* Reset the device
*
@@ -205,73 +187,25 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_i2400m_reset,
NULL, debugfs_i2400m_reset_set,
"%llu\n");
-static
-struct dentry *debugfs_create_i2400m_reset(
- const char *name, struct dentry *parent, struct i2400m *i2400m)
+void i2400m_debugfs_add(struct i2400m *i2400m)
{
- return debugfs_create_file(name, 0200, parent, i2400m,
- &fops_i2400m_reset);
-}
-
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
-int i2400m_debugfs_add(struct i2400m *i2400m)
-{
- int result;
- struct device *dev = i2400m_dev(i2400m);
struct dentry *dentry = i2400m->wimax_dev.debugfs_dentry;
- struct dentry *fd;
dentry = debugfs_create_dir("i2400m", dentry);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- goto error;
- }
i2400m->debugfs_dentry = dentry;
- __debugfs_register("dl_", control, dentry);
- __debugfs_register("dl_", driver, dentry);
- __debugfs_register("dl_", debugfs, dentry);
- __debugfs_register("dl_", fw, dentry);
- __debugfs_register("dl_", netdev, dentry);
- __debugfs_register("dl_", rfkill, dentry);
- __debugfs_register("dl_", rx, dentry);
- __debugfs_register("dl_", tx, dentry);
-
- fd = debugfs_create_size_t("tx_in", 0400, dentry,
- &i2400m->tx_in);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_in: %d\n", result);
- goto error;
- }
- fd = debugfs_create_size_t("tx_out", 0400, dentry,
- &i2400m->tx_out);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_out: %d\n", result);
- goto error;
- }
+ d_level_register_debugfs("dl_", control, dentry);
+ d_level_register_debugfs("dl_", driver, dentry);
+ d_level_register_debugfs("dl_", debugfs, dentry);
+ d_level_register_debugfs("dl_", fw, dentry);
+ d_level_register_debugfs("dl_", netdev, dentry);
+ d_level_register_debugfs("dl_", rfkill, dentry);
+ d_level_register_debugfs("dl_", rx, dentry);
+ d_level_register_debugfs("dl_", tx, dentry);
- fd = debugfs_create_u32("state", 0600, dentry,
- &i2400m->state);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "state: %d\n", result);
- goto error;
- }
+ debugfs_create_size_t("tx_in", 0400, dentry, &i2400m->tx_in);
+ debugfs_create_size_t("tx_out", 0400, dentry, &i2400m->tx_out);
+ debugfs_create_u32("state", 0600, dentry, &i2400m->state);
/*
* Trace received messages from user space
@@ -295,60 +229,22 @@ int i2400m_debugfs_add(struct i2400m *i2400m)
* It is not really very atomic, but it is also not too
* critical.
*/
- fd = debugfs_create_u8("trace_msg_from_user", 0600, dentry,
- &i2400m->trace_msg_from_user);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "trace_msg_from_user: %d\n", result);
- goto error;
- }
+ debugfs_create_u8("trace_msg_from_user", 0600, dentry,
+ &i2400m->trace_msg_from_user);
- fd = debugfs_create_netdev_queue_stopped("netdev_queue_stopped",
- dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "netdev_queue_stopped: %d\n", result);
- goto error;
- }
+ debugfs_create_file("netdev_queue_stopped", 0400, dentry, i2400m,
+ &fops_netdev_queue_stopped);
- fd = debugfs_create_file("rx_stats", 0600, dentry, i2400m,
- &i2400m_rx_stats_fops);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_stats: %d\n", result);
- goto error;
- }
+ debugfs_create_file("rx_stats", 0600, dentry, i2400m,
+ &i2400m_rx_stats_fops);
- fd = debugfs_create_file("tx_stats", 0600, dentry, i2400m,
- &i2400m_tx_stats_fops);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "tx_stats: %d\n", result);
- goto error;
- }
+ debugfs_create_file("tx_stats", 0600, dentry, i2400m,
+ &i2400m_tx_stats_fops);
- fd = debugfs_create_i2400m_suspend("suspend", dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry suspend: %d\n",
- result);
- goto error;
- }
+ debugfs_create_file("suspend", 0200, dentry, i2400m,
+ &fops_i2400m_suspend);
- fd = debugfs_create_i2400m_reset("reset", dentry, i2400m);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry reset: %d\n", result);
- goto error;
- }
-
- result = 0;
-error:
- return result;
+ debugfs_create_file("reset", 0200, dentry, i2400m, &fops_i2400m_reset);
}
void i2400m_debugfs_rm(struct i2400m *i2400m)
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 0a29222a1bf9..f66c0f8f6f4a 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -905,11 +905,7 @@ int i2400m_setup(struct i2400m *i2400m, enum i2400m_bri bm_flags)
goto error_sysfs_setup;
}
- result = i2400m_debugfs_add(i2400m);
- if (result < 0) {
- dev_err(dev, "cannot setup i2400m's debugfs: %d\n", result);
- goto error_debugfs_setup;
- }
+ i2400m_debugfs_add(i2400m);
result = i2400m_dev_start(i2400m, bm_flags);
if (result < 0)
@@ -919,7 +915,6 @@ int i2400m_setup(struct i2400m *i2400m, enum i2400m_bri bm_flags)
error_dev_start:
i2400m_debugfs_rm(i2400m);
-error_debugfs_setup:
sysfs_remove_group(&i2400m->wimax_dev.net_dev->dev.kobj,
&i2400m_dev_attr_group);
error_sysfs_setup:
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 5a34e72bab9a..a3733a6d14f5 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -812,13 +812,10 @@ enum i2400m_pt;
int i2400m_tx(struct i2400m *, const void *, size_t, enum i2400m_pt);
#ifdef CONFIG_DEBUG_FS
-int i2400m_debugfs_add(struct i2400m *);
+void i2400m_debugfs_add(struct i2400m *);
void i2400m_debugfs_rm(struct i2400m *);
#else
-static inline int i2400m_debugfs_add(struct i2400m *i2400m)
-{
- return 0;
-}
+static inline void i2400m_debugfs_add(struct i2400m *i2400m) {}
static inline void i2400m_debugfs_rm(struct i2400m *i2400m) {}
#endif
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index 2075e7b1fff6..6953f904232f 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -366,61 +366,25 @@ struct d_level D_LEVEL[] = {
};
size_t D_LEVEL_SIZE = ARRAY_SIZE(D_LEVEL);
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
static
-int i2400mu_debugfs_add(struct i2400mu *i2400mu)
+void i2400mu_debugfs_add(struct i2400mu *i2400mu)
{
- int result;
- struct device *dev = &i2400mu->usb_iface->dev;
struct dentry *dentry = i2400mu->i2400m.wimax_dev.debugfs_dentry;
- struct dentry *fd;
dentry = debugfs_create_dir("i2400m-usb", dentry);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- goto error;
- }
i2400mu->debugfs_dentry = dentry;
- __debugfs_register("dl_", usb, dentry);
- __debugfs_register("dl_", fw, dentry);
- __debugfs_register("dl_", notif, dentry);
- __debugfs_register("dl_", rx, dentry);
- __debugfs_register("dl_", tx, dentry);
- /* Don't touch these if you don't know what you are doing */
- fd = debugfs_create_u8("rx_size_auto_shrink", 0600, dentry,
- &i2400mu->rx_size_auto_shrink);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_size_auto_shrink: %d\n", result);
- goto error;
- }
+ d_level_register_debugfs("dl_", usb, dentry);
+ d_level_register_debugfs("dl_", fw, dentry);
+ d_level_register_debugfs("dl_", notif, dentry);
+ d_level_register_debugfs("dl_", rx, dentry);
+ d_level_register_debugfs("dl_", tx, dentry);
- fd = debugfs_create_size_t("rx_size", 0600, dentry,
- &i2400mu->rx_size);
- result = PTR_ERR(fd);
- if (IS_ERR(fd) && result != -ENODEV) {
- dev_err(dev, "Can't create debugfs entry "
- "rx_size: %d\n", result);
- goto error;
- }
-
- return 0;
+ /* Don't touch these if you don't know what you are doing */
+ debugfs_create_u8("rx_size_auto_shrink", 0600, dentry,
+ &i2400mu->rx_size_auto_shrink);
-error:
- debugfs_remove_recursive(i2400mu->debugfs_dentry);
- return result;
+ debugfs_create_size_t("rx_size", 0600, dentry, &i2400mu->rx_size);
}
@@ -534,15 +498,9 @@ int i2400mu_probe(struct usb_interface *iface,
dev_err(dev, "cannot setup device: %d\n", result);
goto error_setup;
}
- result = i2400mu_debugfs_add(i2400mu);
- if (result < 0) {
- dev_err(dev, "Can't register i2400mu's debugfs: %d\n", result);
- goto error_debugfs_add;
- }
+ i2400mu_debugfs_add(i2400mu);
return 0;
-error_debugfs_add:
- i2400m_release(i2400m);
error_setup:
usb_set_intfdata(iface, NULL);
usb_put_dev(i2400mu->usb_dev);
diff --git a/include/linux/wimax/debug.h b/include/linux/wimax/debug.h
index 7cb63e4ec0ae..4dd2c1cea6a9 100644
--- a/include/linux/wimax/debug.h
+++ b/include/linux/wimax/debug.h
@@ -98,9 +98,7 @@
* To manipulate from user space the levels, create a debugfs dentry
* and then register each submodule with:
*
- * result = d_level_register_debugfs("PREFIX_", submodule_X, parent);
- * if (result < 0)
- * goto error;
+ * d_level_register_debugfs("PREFIX_", submodule_X, parent);
*
* Where PREFIX_ is a name of your chosing. This will create debugfs
* file with a single numeric value that can be use to tweak it. To
@@ -408,25 +406,13 @@ do { \
* @submodule: name of submodule (not a string, just the name)
* @dentry: debugfs parent dentry
*
- * Returns: 0 if ok, < 0 errno on error.
- *
* For removing, just use debugfs_remove_recursive() on the parent.
*/
#define d_level_register_debugfs(prefix, name, parent) \
({ \
- int rc; \
- struct dentry *fd; \
- struct dentry *verify_parent_type = parent; \
- fd = debugfs_create_u8( \
- prefix #name, 0600, verify_parent_type, \
+ debugfs_create_u8( \
+ prefix #name, 0600, parent, \
&(D_LEVEL[__D_SUBMODULE_ ## name].level)); \
- rc = PTR_ERR(fd); \
- if (IS_ERR(fd) && rc != -ENODEV) \
- printk(KERN_ERR "%s: Can't create debugfs entry %s: " \
- "%d\n", __func__, prefix #name, rc); \
- else \
- rc = 0; \
- rc; \
})
diff --git a/net/wimax/debugfs.c b/net/wimax/debugfs.c
index 1af56df30276..3c54bb6b925a 100644
--- a/net/wimax/debugfs.c
+++ b/net/wimax/debugfs.c
@@ -13,49 +13,23 @@
#define D_SUBMODULE debugfs
#include "debug-levels.h"
-
-#define __debugfs_register(prefix, name, parent) \
-do { \
- result = d_level_register_debugfs(prefix, name, parent); \
- if (result < 0) \
- goto error; \
-} while (0)
-
-
-int wimax_debugfs_add(struct wimax_dev *wimax_dev)
+void wimax_debugfs_add(struct wimax_dev *wimax_dev)
{
- int result;
struct net_device *net_dev = wimax_dev->net_dev;
- struct device *dev = net_dev->dev.parent;
struct dentry *dentry;
char buf[128];
snprintf(buf, sizeof(buf), "wimax:%s", net_dev->name);
dentry = debugfs_create_dir(buf, NULL);
- result = PTR_ERR(dentry);
- if (IS_ERR(dentry)) {
- if (result == -ENODEV)
- result = 0; /* No debugfs support */
- else
- dev_err(dev, "Can't create debugfs dentry: %d\n",
- result);
- goto out;
- }
wimax_dev->debugfs_dentry = dentry;
- __debugfs_register("wimax_dl_", debugfs, dentry);
- __debugfs_register("wimax_dl_", id_table, dentry);
- __debugfs_register("wimax_dl_", op_msg, dentry);
- __debugfs_register("wimax_dl_", op_reset, dentry);
- __debugfs_register("wimax_dl_", op_rfkill, dentry);
- __debugfs_register("wimax_dl_", op_state_get, dentry);
- __debugfs_register("wimax_dl_", stack, dentry);
- result = 0;
-out:
- return result;
-error:
- debugfs_remove_recursive(wimax_dev->debugfs_dentry);
- return result;
+ d_level_register_debugfs("wimax_dl_", debugfs, dentry);
+ d_level_register_debugfs("wimax_dl_", id_table, dentry);
+ d_level_register_debugfs("wimax_dl_", op_msg, dentry);
+ d_level_register_debugfs("wimax_dl_", op_reset, dentry);
+ d_level_register_debugfs("wimax_dl_", op_rfkill, dentry);
+ d_level_register_debugfs("wimax_dl_", op_state_get, dentry);
+ d_level_register_debugfs("wimax_dl_", stack, dentry);
}
void wimax_debugfs_rm(struct wimax_dev *wimax_dev)
diff --git a/net/wimax/stack.c b/net/wimax/stack.c
index 1ba99d65feca..4b9b1c5e8f3a 100644
--- a/net/wimax/stack.c
+++ b/net/wimax/stack.c
@@ -481,12 +481,7 @@ int wimax_dev_add(struct wimax_dev *wimax_dev, struct net_device *net_dev)
/* Set up user-space interaction */
mutex_lock(&wimax_dev->mutex);
wimax_id_table_add(wimax_dev);
- result = wimax_debugfs_add(wimax_dev);
- if (result < 0) {
- dev_err(dev, "cannot initialize debugfs: %d\n",
- result);
- goto error_debugfs_add;
- }
+ wimax_debugfs_add(wimax_dev);
__wimax_state_set(wimax_dev, WIMAX_ST_DOWN);
mutex_unlock(&wimax_dev->mutex);
@@ -498,10 +493,6 @@ int wimax_dev_add(struct wimax_dev *wimax_dev, struct net_device *net_dev)
d_fnend(3, dev, "(wimax_dev %p net_dev %p) = 0\n", wimax_dev, net_dev);
return 0;
-error_debugfs_add:
- wimax_id_table_rm(wimax_dev);
- mutex_unlock(&wimax_dev->mutex);
- wimax_rfkill_rm(wimax_dev);
error_rfkill_add:
d_fnend(3, dev, "(wimax_dev %p net_dev %p) = %d\n",
wimax_dev, net_dev, result);
diff --git a/net/wimax/wimax-internal.h b/net/wimax/wimax-internal.h
index e819a09337ee..40751207296c 100644
--- a/net/wimax/wimax-internal.h
+++ b/net/wimax/wimax-internal.h
@@ -57,13 +57,10 @@ void __wimax_state_set(struct wimax_dev *wimax_dev, enum wimax_st state)
void __wimax_state_change(struct wimax_dev *, enum wimax_st);
#ifdef CONFIG_DEBUG_FS
-int wimax_debugfs_add(struct wimax_dev *);
+void wimax_debugfs_add(struct wimax_dev *);
void wimax_debugfs_rm(struct wimax_dev *);
#else
-static inline int wimax_debugfs_add(struct wimax_dev *wimax_dev)
-{
- return 0;
-}
+static inline void wimax_debugfs_add(struct wimax_dev *wimax_dev) {}
static inline void wimax_debugfs_rm(struct wimax_dev *wimax_dev) {}
#endif
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v2 00/17] Networking driver debugfs cleanups
From: Greg KH @ 2019-08-10 10:17 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190809.112054.1126098316584513793.davem@davemloft.net>
On Fri, Aug 09, 2019 at 11:20:54AM -0700, David Miller wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Fri, 9 Aug 2019 14:30:51 +0200
>
> > v2: fix up build warnings, it's as if I never even built these. Ugh, so
> > sorry for wasting people's time with the v1 series. I need to stop
> > relying on 0-day as it isn't working well anymore :(
>
> One more try Greg:
>
> drivers/net/wimax/i2400m/debugfs.c: In function ‘i2400m_debugfs_add’:
> drivers/net/wimax/i2400m/debugfs.c:192:17: warning: unused variable ‘dev’ [-Wunused-variable]
> struct device *dev = i2400m_dev(i2400m);
> ^~~
It's as if I don't even know how to use a compiler anymore. Ugh :(
v3 coming soon, sorry for the noise.
greg k-h
^ permalink raw reply
* Re: [PATCH 2/2] Fix a NULL-ptr-deref bug in ath10k_usb_alloc_urb_from_pipe
From: Greg KH @ 2019-08-10 10:13 UTC (permalink / raw)
To: Hui Peng
Cc: kvalo, davem, Mathias Payer, ath10k, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190804003101.11541-1-benquike@gmail.com>
On Sat, Aug 03, 2019 at 08:31:01PM -0400, Hui Peng wrote:
> The `ar_usb` field of `ath10k_usb_pipe_usb_pipe` objects
> are initialized to point to the containing `ath10k_usb` object
> according to endpoint descriptors read from the device side, as shown
> below in `ath10k_usb_setup_pipe_resources`:
>
> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> endpoint = &iface_desc->endpoint[i].desc;
>
> // get the address from endpoint descriptor
> pipe_num = ath10k_usb_get_logical_pipe_num(ar_usb,
> endpoint->bEndpointAddress,
> &urbcount);
> ......
> // select the pipe object
> pipe = &ar_usb->pipes[pipe_num];
>
> // initialize the ar_usb field
> pipe->ar_usb = ar_usb;
> }
>
> The driver assumes that the addresses reported in endpoint
> descriptors from device side to be complete. If a device is
> malicious and does not report complete addresses, it may trigger
> NULL-ptr-deref `ath10k_usb_alloc_urb_from_pipe` and
> `ath10k_usb_free_urb_to_pipe`.
>
> This patch fixes the bug by preventing potential NULL-ptr-deref.
>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [PATCH 1/2] Fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe
From: Greg KH @ 2019-08-10 10:13 UTC (permalink / raw)
To: Hui Peng
Cc: kvalo, davem, Mathias Payer, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190804002905.11292-1-benquike@gmail.com>
On Sat, Aug 03, 2019 at 08:29:04PM -0400, Hui Peng wrote:
> The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects
> are initialized to point to the containing `ath6kl_usb` object
> according to endpoint descriptors read from the device side, as shown
> below in `ath6kl_usb_setup_pipe_resources`:
>
> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> endpoint = &iface_desc->endpoint[i].desc;
>
> // get the address from endpoint descriptor
> pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb,
> endpoint->bEndpointAddress,
> &urbcount);
> ......
> // select the pipe object
> pipe = &ar_usb->pipes[pipe_num];
>
> // initialize the ar_usb field
> pipe->ar_usb = ar_usb;
> }
>
> The driver assumes that the addresses reported in endpoint
> descriptors from device side to be complete. If a device is
> malicious and does not report complete addresses, it may trigger
> NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and
> `ath6kl_usb_free_urb_to_pipe`.
>
> This patch fixes the bug by preventing potential NULL-ptr-deref.
>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: general protection fault in tls_write_space
From: syzbot @ 2019-08-10 8:23 UTC (permalink / raw)
To: aviadye, borisp, daniel, davejwatson, davem, jakub.kicinski,
john.fastabend, linux-kernel, netdev, oss-drivers, syzkaller-bugs,
willemb
In-Reply-To: <000000000000f5d619058faea744@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: ca497fb6 taprio: remove unused variable 'entry_list_policy'
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=109f3802600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d4cf1ffb87d590d7
dashboard link: https://syzkaller.appspot.com/bug?extid=dcdc9deefaec44785f32
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11c78cd2600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+dcdc9deefaec44785f32@syzkaller.appspotmail.com
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 5.3.0-rc3+ #125
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
RIP: 0010:tls_write_space+0x51/0x170 net/tls/tls_main.c:239
Code: c1 ea 03 80 3c 02 00 0f 85 26 01 00 00 49 8b 9c 24 b0 06 00 00 48 b8
00 00 00 00 00 fc ff df 48 8d 7b 6a 48 89 fa 48 c1 ea 03 <0f> b6 04 02 48
89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 df 00 00 00
RSP: 0018:ffff8880a98b74c8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff860a27a2
RDX: 000000000000000d RSI: ffffffff862c86c1 RDI: 000000000000006a
RBP: ffff8880a98b74e0 R08: ffff8880a98a2240 R09: fffffbfff167c289
R10: fffffbfff167c288 R11: ffffffff8b3e1447 R12: ffff8880a4de41c0
R13: ffff8880a4de45b8 R14: 000000000000000a R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000008c9d1000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
tcp_new_space net/ipv4/tcp_input.c:5151 [inline]
tcp_check_space+0x191/0x760 net/ipv4/tcp_input.c:5162
tcp_data_snd_check net/ipv4/tcp_input.c:5172 [inline]
tcp_rcv_state_process+0xe24/0x4e48 net/ipv4/tcp_input.c:6303
tcp_v6_do_rcv+0x7d7/0x12c0 net/ipv6/tcp_ipv6.c:1381
tcp_v6_rcv+0x31f1/0x3500 net/ipv6/tcp_ipv6.c:1588
ip6_protocol_deliver_rcu+0x2fe/0x1660 net/ipv6/ip6_input.c:397
ip6_input_finish+0x84/0x170 net/ipv6/ip6_input.c:438
NF_HOOK include/linux/netfilter.h:305 [inline]
NF_HOOK include/linux/netfilter.h:299 [inline]
ip6_input+0xe4/0x3f0 net/ipv6/ip6_input.c:447
dst_input include/net/dst.h:442 [inline]
ip6_rcv_finish+0x1de/0x2f0 net/ipv6/ip6_input.c:76
NF_HOOK include/linux/netfilter.h:305 [inline]
NF_HOOK include/linux/netfilter.h:299 [inline]
ipv6_rcv+0x10e/0x420 net/ipv6/ip6_input.c:272
__netif_receive_skb_one_core+0x113/0x1a0 net/core/dev.c:5006
__netif_receive_skb+0x2c/0x1d0 net/core/dev.c:5120
process_backlog+0x206/0x750 net/core/dev.c:5951
napi_poll net/core/dev.c:6388 [inline]
net_rx_action+0x4d6/0x1080 net/core/dev.c:6456
__do_softirq+0x262/0x98c kernel/softirq.c:292
run_ksoftirqd kernel/softirq.c:603 [inline]
run_ksoftirqd+0x8e/0x110 kernel/softirq.c:595
smpboot_thread_fn+0x6a3/0xa40 kernel/smpboot.c:165
kthread+0x361/0x430 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace c21a83505707bb9d ]---
RIP: 0010:tls_write_space+0x51/0x170 net/tls/tls_main.c:239
Code: c1 ea 03 80 3c 02 00 0f 85 26 01 00 00 49 8b 9c 24 b0 06 00 00 48 b8
00 00 00 00 00 fc ff df 48 8d 7b 6a 48 89 fa 48 c1 ea 03 <0f> b6 04 02 48
89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 df 00 00 00
RSP: 0018:ffff8880a98b74c8 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff860a27a2
RDX: 000000000000000d RSI: ffffffff862c86c1 RDI: 000000000000006a
RBP: ffff8880a98b74e0 R08: ffff8880a98a2240 R09: fffffbfff167c289
R10: fffffbfff167c288 R11: ffffffff8b3e1447 R12: ffff8880a4de41c0
R13: ffff8880a4de45b8 R14: 000000000000000a R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff8880ae800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000008c9d1000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
^ permalink raw reply
* Re: [PATCH] vhost: do not reference a file that does not exist
From: Christoph Hellwig @ 2019-08-10 8:15 UTC (permalink / raw)
To: egranata
Cc: linux-kernel, mst, jasowang, kvm, virtualization, netdev, trivial,
egranata
In-Reply-To: <20190808005255.106299-1-egranata@chromium.org>
On Wed, Aug 07, 2019 at 05:52:55PM -0700, egranata@chromium.org wrote:
> From: Enrico Granata <egranata@google.com>
>
> lguest was removed from the mainline kernel in late 2017.
>
> Signed-off-by: Enrico Granata <egranata@google.com>
But this particular file even has an override in the script looking
for dead references, which together with the content of the overal
contents makes me thing the dangling reference is somewhat intentional.
^ permalink raw reply
* [PATCH] ARM: module: Avoid W and X mappings at the beginning
From: zhe.he @ 2019-08-10 8:09 UTC (permalink / raw)
To: linux, ast, daniel, kafai, songliubraving, yhs, matthias.schiffer,
info, gregkh, tglx, linux-arm-kernel, linux-kernel, netdev, bpf,
zhe.he
From: He Zhe <zhe.he@windriver.com>
It is more secure to map module memory as not-execute at the beginning.
Memory sections that need to be executable will be turned to executable
later in complete_formation.
This is a corresponding change for ARM to the following commit
commit f2c65fb3221a ("x86/modules: Avoid breaking W^X while loading modules")
Tested with test_bpf:
test_bpf: Summary: 378 PASSED, 0 FAILED, [0/366 JIT'ed]
Signed-off-by: He Zhe <zhe.he@windriver.com>
---
arch/arm/kernel/module.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index deef17f..197b3b9 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -45,12 +45,12 @@ void *module_alloc(unsigned long size)
gfp_mask |= __GFP_NOWARN;
p = __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
- gfp_mask, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
+ gfp_mask, PAGE_KERNEL, 0, NUMA_NO_NODE,
__builtin_return_address(0));
if (!IS_ENABLED(CONFIG_ARM_MODULE_PLTS) || p)
return p;
return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
- GFP_KERNEL, PAGE_KERNEL_EXEC, 0, NUMA_NO_NODE,
+ GFP_KERNEL, PAGE_KERNEL, 0, NUMA_NO_NODE,
__builtin_return_address(0));
}
#endif
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: openvswitch: free vport unless register_netdevice() succeeds
From: Pravin Shelar @ 2019-08-10 7:34 UTC (permalink / raw)
To: Hillf Danton
Cc: ovs dev, David S. Miller, linux-kernel,
Linux Kernel Network Developers, syzkaller-bugs, syzbot,
Taehee Yoo, Greg Rose, Eric Dumazet, Marcelo Ricardo Leitner,
Ying Xue, Andrey Konovalov
In-Reply-To: <20190809035515.13968-1-hdanton@sina.com>
On Thu, Aug 8, 2019 at 8:55 PM Hillf Danton <hdanton@sina.com> wrote:
>
>
> syzbot found the following crash on:
>
> HEAD commit: 1e78030e Merge tag 'mmc-v5.3-rc1' of git://git.kernel.org/..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=148d3d1a600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=30cef20daf3e9977
> dashboard link: https://syzkaller.appspot.com/bug?extid=13210896153522fe1ee5
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=136aa8c4600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=109ba792600000
>
> =====================================================================
> BUG: memory leak
> unreferenced object 0xffff8881207e4100 (size 128):
> comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s)
> hex dump (first 32 bytes):
> 00 70 16 18 81 88 ff ff 80 af 8c 22 81 88 ff ff .p........."....
> 00 b6 23 17 81 88 ff ff 00 00 00 00 00 00 00 00 ..#.............
> backtrace:
> [<000000000eb78212>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
> [<000000000eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline]
> [<000000000eb78212>] slab_alloc mm/slab.c:3319 [inline]
> [<000000000eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
> [<00000000006ea6c6>] kmalloc include/linux/slab.h:552 [inline]
> [<00000000006ea6c6>] kzalloc include/linux/slab.h:748 [inline]
> [<00000000006ea6c6>] ovs_vport_alloc+0x37/0xf0 net/openvswitch/vport.c:130
> [<00000000f9a04a7d>] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164
> [<0000000056ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199
> [<000000005434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> [<00000000b7b253f1>] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614
> [<00000000e0988518>] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629
> [<00000000d0cc9347>] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> [<000000006694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477
> [<0000000088381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> [<00000000dad42a47>] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
> [<00000000dad42a47>] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328
> [<0000000067e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917
> [<00000000aab08a47>] sock_sendmsg_nosec net/socket.c:637 [inline]
> [<00000000aab08a47>] sock_sendmsg+0x54/0x70 net/socket.c:657
> [<000000004cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> [<00000000c4901c63>] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
> [<00000000c10abb2d>] __do_sys_sendmsg net/socket.c:2365 [inline]
> [<00000000c10abb2d>] __se_sys_sendmsg net/socket.c:2363 [inline]
> [<00000000c10abb2d>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363
>
> BUG: memory leak
> unreferenced object 0xffff88811723b600 (size 64):
> comm "syz-executor032", pid 7014, jiffies 4294944027 (age 13.830s)
> hex dump (first 32 bytes):
> 01 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 02 00 00 00 05 35 82 c1 .............5..
> backtrace:
> [<00000000352f46d8>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
> [<00000000352f46d8>] slab_post_alloc_hook mm/slab.h:522 [inline]
> [<00000000352f46d8>] slab_alloc mm/slab.c:3319 [inline]
> [<00000000352f46d8>] __do_kmalloc mm/slab.c:3653 [inline]
> [<00000000352f46d8>] __kmalloc+0x169/0x300 mm/slab.c:3664
> [<000000008e48f3d1>] kmalloc include/linux/slab.h:557 [inline]
> [<000000008e48f3d1>] ovs_vport_set_upcall_portids+0x54/0xd0 net/openvswitch/vport.c:343
> [<00000000541e4f4a>] ovs_vport_alloc+0x7f/0xf0 net/openvswitch/vport.c:139
> [<00000000f9a04a7d>] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164
> [<0000000056ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199
> [<000000005434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> [<00000000b7b253f1>] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614
> [<00000000e0988518>] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629
> [<00000000d0cc9347>] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> [<000000006694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477
> [<0000000088381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> [<00000000dad42a47>] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
> [<00000000dad42a47>] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328
> [<0000000067e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917
> [<00000000aab08a47>] sock_sendmsg_nosec net/socket.c:637 [inline]
> [<00000000aab08a47>] sock_sendmsg+0x54/0x70 net/socket.c:657
> [<000000004cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> [<00000000c4901c63>] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
>
> BUG: memory leak
> unreferenced object 0xffff8881228ca500 (size 128):
> comm "syz-executor032", pid 7015, jiffies 4294944622 (age 7.880s)
> hex dump (first 32 bytes):
> 00 f0 27 18 81 88 ff ff 80 ac 8c 22 81 88 ff ff ..'........"....
> 40 b7 23 17 81 88 ff ff 00 00 00 00 00 00 00 00 @.#.............
> backtrace:
> [<000000000eb78212>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
> [<000000000eb78212>] slab_post_alloc_hook mm/slab.h:522 [inline]
> [<000000000eb78212>] slab_alloc mm/slab.c:3319 [inline]
> [<000000000eb78212>] kmem_cache_alloc_trace+0x145/0x2c0 mm/slab.c:3548
> [<00000000006ea6c6>] kmalloc include/linux/slab.h:552 [inline]
> [<00000000006ea6c6>] kzalloc include/linux/slab.h:748 [inline]
> [<00000000006ea6c6>] ovs_vport_alloc+0x37/0xf0 net/openvswitch/vport.c:130
> [<00000000f9a04a7d>] internal_dev_create+0x24/0x1d0 net/openvswitch/vport-internal_dev.c:164
> [<0000000056ee7c13>] ovs_vport_add+0x81/0x190 net/openvswitch/vport.c:199
> [<000000005434efc7>] new_vport+0x19/0x80 net/openvswitch/datapath.c:194
> [<00000000b7b253f1>] ovs_dp_cmd_new+0x22f/0x410 net/openvswitch/datapath.c:1614
> [<00000000e0988518>] genl_family_rcv_msg+0x2ab/0x5b0 net/netlink/genetlink.c:629
> [<00000000d0cc9347>] genl_rcv_msg+0x54/0x9c net/netlink/genetlink.c:654
> [<000000006694b647>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477
> [<0000000088381f37>] genl_rcv+0x29/0x40 net/netlink/genetlink.c:665
> [<00000000dad42a47>] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
> [<00000000dad42a47>] netlink_unicast+0x1ec/0x2d0 net/netlink/af_netlink.c:1328
> [<0000000067e6b079>] netlink_sendmsg+0x270/0x480 net/netlink/af_netlink.c:1917
> [<00000000aab08a47>] sock_sendmsg_nosec net/socket.c:637 [inline]
> [<00000000aab08a47>] sock_sendmsg+0x54/0x70 net/socket.c:657
> [<000000004cb7c11d>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2311
> [<00000000c4901c63>] __sys_sendmsg+0x80/0xf0 net/socket.c:2356
> [<00000000c10abb2d>] __do_sys_sendmsg net/socket.c:2365 [inline]
> [<00000000c10abb2d>] __se_sys_sendmsg net/socket.c:2363 [inline]
> [<00000000c10abb2d>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2363
> =====================================================================
>
> The function in net core, register_netdevice(), may fail with vport's
> destruction callback either invoked or not. After commit 309b66970ee2,
> the duty to destroy vport is offloaded from the driver OTOH, which ends
> up in the memory leak reported.
>
> It is fixed by releasing vport unless device is registered successfully.
> To do that, the callback assignment is defered until device is registered.
>
> Reported-by: syzbot+13210896153522fe1ee5@syzkaller.appspotmail.com
> Fixes: 309b66970ee2 ("net: openvswitch: do not free vport if register_netdevice() is failed.")
> Cc: Taehee Yoo <ap420073@gmail.com>
> Cc: Greg Rose <gvrose8192@gmail.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
>
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -137,7 +137,7 @@ static void do_setup(struct net_device *
> netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
> IFF_NO_QUEUE;
> netdev->needs_free_netdev = true;
> - netdev->priv_destructor = internal_dev_destructor;
> + netdev->priv_destructor = NULL;
> netdev->ethtool_ops = &internal_dev_ethtool_ops;
> netdev->rtnl_link_ops = &internal_dev_link_ops;
>
> @@ -159,7 +159,6 @@ static struct vport *internal_dev_create
> struct internal_dev *internal_dev;
> struct net_device *dev;
> int err;
> - bool free_vport = true;
>
> vport = ovs_vport_alloc(0, &ovs_internal_vport_ops, parms);
> if (IS_ERR(vport)) {
> @@ -190,10 +189,9 @@ static struct vport *internal_dev_create
>
> rtnl_lock();
> err = register_netdevice(vport->dev);
> - if (err) {
> - free_vport = false;
> + if (err)
> goto error_unlock;
> - }
> + vport->dev->priv_destructor = internal_dev_destructor;
>
Looks good.
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Thanks,
Pravin.
^ permalink raw reply
* Re: [PATCH 00/16] ARM: remove ks8695 and w90x900 platforms
From: Greg Kroah-Hartman @ 2019-08-10 7:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, Wanzongshun (Vincent), Greg Ungerer, linux-serial,
Dmitry Torokhov, linux-input, Linus Walleij, linux-gpio,
David S. Miller, netdev, Guenter Roeck, Mark Brown, alsa-devel,
linux-spi, Bartlomiej Zolnierkiewicz, linux-fbdev, Miquel Raynal,
linux-mtd, linux-arm-kernel, linux-kernel
In-Reply-To: <20190809202749.742267-1-arnd@arndb.de>
On Fri, Aug 09, 2019 at 10:27:28PM +0200, Arnd Bergmann wrote:
> As discussed previously, these two ARM platforms have no
> known remaining users, let's remove them completely.
>
> Subsystem maintainers: feel free to take the driver removals
> through your respective trees, they are all independent of
> one another. We can merge any remaining patches through the
> soc tree.
Serial and USB host controller driver patches applied, thanks!
greg k-h
^ permalink raw reply
* [PATCH v4 2/2] perf machine: arm/arm64: Improve completeness for kernel address space
From: Leo Yan @ 2019-08-10 7:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li,
Adrian Hunter, Mark Drayton, Tzvetomir Stoyanov (VMware),
linux-kernel, netdev, bpf, clang-built-linux, Mathieu Poirier
Cc: Leo Yan, Peter Zijlstra, Suzuki Poulouse, coresight,
linux-arm-kernel
In-Reply-To: <20190810072135.27072-1-leo.yan@linaro.org>
Arm and arm64 architecture reserve some memory regions prior to the
symbol '_stext' and these memory regions later will be used by device
module and BPF jit. The current code misses to consider these memory
regions thus any address in the regions will be taken as user space
mode, but perf cannot find the corresponding dso with the wrong CPU
mode so we misses to generate samples for device module and BPF
related trace data.
This patch parse the link scripts to get the memory size prior to start
address and reduce this size from 'machine>->kernel_start', then can
get a fixed up kernel start address which contain memory regions for
device module and BPF. Finally, machine__get_kernel_start() can reflect
more complete kernel memory regions and perf can successfully generate
samples.
The reason for parsing the link scripts is Arm architecture changes text
offset dependent on different platforms, which define multiple text
offsets in $kernel/arch/arm/Makefile. This offset is decided when build
kernel and the final value is extended in the link script, so we can
extract the used value from the link script. We use the same way to
parse arm64 link script as well. If fail to find the link script, the
pre start memory size is assumed as zero, in this case it has no any
change caused with this patch.
Below is detailed info for testing this patch:
- Install or build LLVM/Clang;
- Configure perf with ~/.perfconfig:
root@debian:~# cat ~/.perfconfig
# this file is auto-generated.
[llvm]
clang-path = /mnt/build/llvm-build/build/install/bin/clang
kbuild-dir = /mnt/linux-kernel/linux-cs-dev/
clang-opt = "-g"
dump-obj = true
[trace]
show_zeros = yes
show_duration = no
no_inherit = yes
show_timestamp = no
show_arg_names = no
args_alignment = 40
show_prefix = yes
- Run 'perf trace' command with eBPF event:
root@debian:~# perf trace -e string \
-e $kernel/tools/perf/examples/bpf/augmented_raw_syscalls.c
- Read eBPF program memory mapping in kernel:
root@debian:~# echo 1 > /proc/sys/net/core/bpf_jit_kallsyms
root@debian:~# cat /proc/kallsyms | grep -E "bpf_prog_.+_sys_[enter|exit]"
ffff00000008a0d0 t bpf_prog_e470211b846088d5_sys_enter [bpf]
ffff00000008c6a4 t bpf_prog_29c7ae234d79bd5c_sys_exit [bpf]
- Launch any program which accesses file system frequently so can hit
the system calls trace flow with eBPF event;
- Capture CoreSight trace data with filtering eBPF program:
root@debian:~# perf record -e cs_etm/@tmc_etr0/ \
--filter 'filter 0xffff00000008a0d0/0x800' -a sleep 5s
- Decode the eBPF program symbol 'bpf_prog_f173133dc38ccf87_sys_enter':
root@debian:~# perf script -F,ip,sym
Frame deformatter: Found 4 FSYNCS
0 [unknown]
ffff00000008a1ac bpf_prog_e470211b846088d5_sys_enter
ffff00000008a250 bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a124 bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a14c bpf_prog_e470211b846088d5_sys_enter
ffff00000008a13c bpf_prog_e470211b846088d5_sys_enter
ffff00000008a14c bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a180 bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a1ac bpf_prog_e470211b846088d5_sys_enter
ffff00000008a190 bpf_prog_e470211b846088d5_sys_enter
ffff00000008a1ac bpf_prog_e470211b846088d5_sys_enter
ffff00000008a250 bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a124 bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a14c bpf_prog_e470211b846088d5_sys_enter
0 [unknown]
ffff00000008a180 bpf_prog_e470211b846088d5_sys_enter
[...]
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/Makefile.config | 22 ++++++++++++++++++++++
tools/perf/arch/arm/util/Build | 2 ++
tools/perf/arch/arm/util/machine.c | 17 +++++++++++++++++
tools/perf/arch/arm64/util/Build | 1 +
tools/perf/arch/arm64/util/machine.c | 17 +++++++++++++++++
5 files changed, 59 insertions(+)
create mode 100644 tools/perf/arch/arm/util/machine.c
create mode 100644 tools/perf/arch/arm64/util/machine.c
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index e4988f49ea79..76e0ad0b4fd2 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -51,6 +51,17 @@ endif
ifeq ($(SRCARCH),arm)
NO_PERF_REGS := 0
LIBUNWIND_LIBS = -lunwind -lunwind-arm
+ PRE_START_SIZE := 0
+ ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
+ # Extract info from lds:
+ # . = ((0xC0000000)) + 0x00208000;
+ # PRE_START_SIZE := 0x00208000
+ PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
+ $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
+ sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
+ awk -F' ' '{printf "0x%x", $$2}' 2>/dev/null)
+ endif
+ CFLAGS += -DARM_PRE_START_SIZE=$(PRE_START_SIZE)
endif
ifeq ($(SRCARCH),arm64)
@@ -58,6 +69,17 @@ ifeq ($(SRCARCH),arm64)
NO_SYSCALL_TABLE := 0
CFLAGS += -I$(OUTPUT)arch/arm64/include/generated
LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
+ PRE_START_SIZE := 0
+ ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
+ # Extract info from lds:
+ # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
+ # PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000) = 0x10080000
+ PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
+ $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
+ sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
+ awk -F' ' '{printf "0x%x", $$6+$$7+$$8}' 2>/dev/null)
+ endif
+ CFLAGS += -DARM_PRE_START_SIZE=$(PRE_START_SIZE)
endif
ifeq ($(SRCARCH),csky)
diff --git a/tools/perf/arch/arm/util/Build b/tools/perf/arch/arm/util/Build
index 296f0eac5e18..efa6b768218a 100644
--- a/tools/perf/arch/arm/util/Build
+++ b/tools/perf/arch/arm/util/Build
@@ -1,3 +1,5 @@
+perf-y += machine.o
+
perf-$(CONFIG_DWARF) += dwarf-regs.o
perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/arm/util/machine.c b/tools/perf/arch/arm/util/machine.c
new file mode 100644
index 000000000000..db172894e4ea
--- /dev/null
+++ b/tools/perf/arch/arm/util/machine.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/types.h>
+#include <linux/string.h>
+#include <stdlib.h>
+
+#include "../../util/machine.h"
+
+void arch__fix_kernel_text_start(u64 *start)
+{
+ /*
+ * On arm, the 16MB virtual memory space prior to 'kernel_start' is
+ * allocated to device modules, a PMD table if CONFIG_HIGHMEM is
+ * enabled and a PGD table. To reflect the complete kernel address
+ * space, compensate the pre-defined regions for kernel start address.
+ */
+ *start = *start - ARM_PRE_START_SIZE;
+}
diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index 3cde540d2fcf..8081fb8a7b3d 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -1,4 +1,5 @@
perf-y += header.o
+perf-y += machine.o
perf-y += sym-handling.o
perf-$(CONFIG_DWARF) += dwarf-regs.o
perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/arm64/util/machine.c b/tools/perf/arch/arm64/util/machine.c
new file mode 100644
index 000000000000..61058dca8c5a
--- /dev/null
+++ b/tools/perf/arch/arm64/util/machine.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/types.h>
+#include <linux/string.h>
+#include <stdlib.h>
+
+#include "../../util/machine.h"
+
+void arch__fix_kernel_text_start(u64 *start)
+{
+ /*
+ * On arm64, the root PGD table, device module memory region and
+ * BPF jit region are prior to 'kernel_start'. To reflect the
+ * complete kernel address space, compensate these pre-defined
+ * regions for kernel start address.
+ */
+ *start = *start - ARM_PRE_START_SIZE;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 1/2] perf machine: Support arch's specific kernel start address
From: Leo Yan @ 2019-08-10 7:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li,
Adrian Hunter, Mark Drayton, Tzvetomir Stoyanov (VMware),
linux-kernel, netdev, bpf, clang-built-linux, Mathieu Poirier
Cc: Leo Yan
In-Reply-To: <20190810072135.27072-1-leo.yan@linaro.org>
machine__get_kernel_start() gives out the kernel start address; some
architectures need to tweak the start address so that can reflect the
kernel start address correctly. This is not only for x86_64 arch, but
it is also required by other architectures, e.g. arm/arm64 needs to
tweak the kernel start address so can include the kernel memory regions
which are used before the '_stext' symbol.
This patch refactors machine__get_kernel_start() by adding a weak
arch__fix_kernel_text_start(), any architecture can implement it to
tweak its specific start address; this also allows the arch specific
code to be placed into 'arch' folder.
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/arch/x86/util/machine.c | 10 ++++++++++
tools/perf/util/machine.c | 13 +++++++------
tools/perf/util/machine.h | 2 ++
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
index 1e9ec783b9a1..9f012131534a 100644
--- a/tools/perf/arch/x86/util/machine.c
+++ b/tools/perf/arch/x86/util/machine.c
@@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
return ret;
}
+void arch__fix_kernel_text_start(u64 *start)
+{
+ /*
+ * On x86_64, PTI entry trampolines are less than the
+ * start of kernel text, but still above 2^63. So leave
+ * kernel_start = 1ULL << 63 for x86_64.
+ */
+ *start = 1ULL << 63;
+}
+
#endif
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f6ee7fbad3e4..603518835692 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2671,6 +2671,10 @@ int machine__nr_cpus_avail(struct machine *machine)
return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
}
+void __weak arch__fix_kernel_text_start(u64 *start __maybe_unused)
+{
+}
+
int machine__get_kernel_start(struct machine *machine)
{
struct map *map = machine__kernel_map(machine);
@@ -2687,14 +2691,11 @@ int machine__get_kernel_start(struct machine *machine)
machine->kernel_start = 1ULL << 63;
if (map) {
err = map__load(map);
- /*
- * On x86_64, PTI entry trampolines are less than the
- * start of kernel text, but still above 2^63. So leave
- * kernel_start = 1ULL << 63 for x86_64.
- */
- if (!err && !machine__is(machine, "x86_64"))
+ if (!err)
machine->kernel_start = map->start;
}
+
+ arch__fix_kernel_text_start(&machine->kernel_start);
return err;
}
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index ef803f08ae12..9cb459f4bfbc 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -278,6 +278,8 @@ void machine__get_kallsyms_filename(struct machine *machine, char *buf,
int machine__create_extra_kernel_maps(struct machine *machine,
struct dso *kernel);
+void arch__fix_kernel_text_start(u64 *start);
+
/* Kernel-space maps for symbols that are outside the main kernel map and module maps */
struct extra_kernel_map {
u64 start;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/2] perf: arm/arm64: Improve completeness for kernel address space
From: Leo Yan @ 2019-08-10 7:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, David Miller,
Milian Wolff, Donald Yandt, Davidlohr Bueso, Wei Li,
Adrian Hunter, Mark Drayton, Tzvetomir Stoyanov (VMware),
linux-kernel, netdev, bpf, clang-built-linux, Mathieu Poirier
Cc: Leo Yan
This patch set is to improve completeness for kernel address space for
arm/arm64; it adds architecture specific tweaking for the kernel start
address, thus can include the memory regions which are prior to '_stext'
symbol. With this change, we can see the eBPF program can be parsed
properly on arm64.
This patch set is a following up version for the old patch "perf cs-etm:
Improve completeness for kernel address space" [1]; the old patch was
only to fix the issue for CoreSight ETM event; but the kernel address space
issue is not only limited to CoreSight event, it should be a common issue
for other events (e.g. PMU events), clock events for profiling eBPF
program. So this patch set tries to resolve it as a common issue for
arm/arm64 archs.
When implemented related code, I tried to use the API
machine__create_extra_kernel_maps(); but I found the 'perf script' tool
directly calls machine__get_kernel_start() instead of running into
the flow for machine__create_extra_kernel_maps(); this is the reason I
don't use machine__create_extra_kernel_maps() for tweaking kernel start
address and refactor machine__get_kernel_start() alternatively.
If there have better method to resolve this issue, any suggestions and
comments are very welcome!
[1] https://lkml.org/lkml/2019/6/19/1057
Leo Yan (2):
perf machine: Support arch's specific kernel start address
perf machine: arm/arm64: Improve completeness for kernel address space
tools/perf/Makefile.config | 22 ++++++++++++++++++++++
tools/perf/arch/arm/util/Build | 2 ++
tools/perf/arch/arm/util/machine.c | 17 +++++++++++++++++
tools/perf/arch/arm64/util/Build | 1 +
tools/perf/arch/arm64/util/machine.c | 17 +++++++++++++++++
tools/perf/arch/x86/util/machine.c | 10 ++++++++++
tools/perf/util/machine.c | 13 +++++++------
tools/perf/util/machine.h | 2 ++
8 files changed, 78 insertions(+), 6 deletions(-)
create mode 100644 tools/perf/arch/arm/util/machine.c
create mode 100644 tools/perf/arch/arm64/util/machine.c
--
2.17.1
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Jiri Pirko @ 2019-08-10 6:32 UTC (permalink / raw)
To: Roopa Prabhu
Cc: netdev, David Miller, Jakub Kicinski, Stephen Hemminger,
David Ahern, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
In-Reply-To: <CAJieiUj7nzHdRUjBpnfL5bKPszJL0b_hKjxpjM0RGd9ocF3EoA@mail.gmail.com>
Fri, Aug 09, 2019 at 05:40:25PM CEST, roopa@cumulusnetworks.com wrote:
>On Thu, Aug 8, 2019 at 11:25 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Fri, Aug 09, 2019 at 06:11:30AM CEST, roopa@cumulusnetworks.com wrote:
>> >On Fri, Jul 19, 2019 at 4:00 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >>
>> >> Add two commands to add and delete alternative ifnames for net device.
>> >> Each net device can have multiple alternative names.
>> >>
>> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> >> ---
>> >> include/linux/netdevice.h | 4 ++
>> >> include/uapi/linux/if.h | 1 +
>> >> include/uapi/linux/if_link.h | 1 +
>> >> include/uapi/linux/rtnetlink.h | 7 +++
>> >> net/core/dev.c | 58 ++++++++++++++++++-
>> >> net/core/rtnetlink.c | 102 +++++++++++++++++++++++++++++++++
>> >> security/selinux/nlmsgtab.c | 4 +-
>> >> 7 files changed, 175 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> >> index 74f99f127b0e..6922fdb483ca 100644
>> >> --- a/include/linux/netdevice.h
>> >> +++ b/include/linux/netdevice.h
>> >> @@ -920,10 +920,14 @@ struct tlsdev_ops;
>> >>
>> >> struct netdev_name_node {
>> >> struct hlist_node hlist;
>> >> + struct list_head list;
>> >> struct net_device *dev;
>> >> char *name;
>> >> };
>> >>
>> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name);
>> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name);
>> >> +
>> >> /*
>> >> * This structure defines the management hooks for network devices.
>> >> * The following hooks can be defined; unless noted otherwise, they are
>> >> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
>> >> index 7fea0fd7d6f5..4bf33344aab1 100644
>> >> --- a/include/uapi/linux/if.h
>> >> +++ b/include/uapi/linux/if.h
>> >> @@ -33,6 +33,7 @@
>> >> #define IFNAMSIZ 16
>> >> #endif /* __UAPI_DEF_IF_IFNAMSIZ */
>> >> #define IFALIASZ 256
>> >> +#define ALTIFNAMSIZ 128
>> >> #include <linux/hdlc/ioctl.h>
>> >>
>> >> /* For glibc compatibility. An empty enum does not compile. */
>> >> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>> >> index 4a8c02cafa9a..92268946e04a 100644
>> >> --- a/include/uapi/linux/if_link.h
>> >> +++ b/include/uapi/linux/if_link.h
>> >> @@ -167,6 +167,7 @@ enum {
>> >> IFLA_NEW_IFINDEX,
>> >> IFLA_MIN_MTU,
>> >> IFLA_MAX_MTU,
>> >> + IFLA_ALT_IFNAME_MOD, /* Alternative ifname to add/delete */
>> >> __IFLA_MAX
>> >> };
>> >>
>> >> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> >> index ce2a623abb75..b36cfd83eb76 100644
>> >> --- a/include/uapi/linux/rtnetlink.h
>> >> +++ b/include/uapi/linux/rtnetlink.h
>> >> @@ -164,6 +164,13 @@ enum {
>> >> RTM_GETNEXTHOP,
>> >> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>> >>
>> >> + RTM_NEWALTIFNAME = 108,
>> >> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>> >> + RTM_DELALTIFNAME,
>> >> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>> >> + RTM_GETALTIFNAME,
>> >> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>> >> +
>> >
>> >I might have missed the prior discussion, why do we need new commands
>> >?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>> >set alternate names ?
>>
>> How? This is to add/remove. How do you suggest to to add/remove by
>> setlink?
>
>to that point, I am also not sure why we have a new API For multiple
>names. I mean why support more than two names (existing old name and
Please see the previous discussion in the rfc. The point is, udev can
provide multiple names according to multiple naming scheme (mac,
pciaddr, etc).
>a new name to remove the length limitation) ?
>
>Your patch series addresses a very important problem (we run into this
>limitation all the time and its hard to explain it to network
>operators) and
> its already unfortunate that we have to have more than one name
>because we cannot resize the existing one.
>
>The best we can do for simpler transition/management from user-space
>is to keep the api simple..
>ie keep it close to the management of existing link attributes. Hence
>the question.
>
>I assumed this would be like alias. A single new field that can be
>referenced in lieu of the old one.
>
>Your series is very useful to many of us...but when i think about
>changing our network manager to accommodate this, I am worried about
>how many apps will have to change.
>I agree they have to change regardless but now they will have to
>listen to yet another notification and msg format for names ?
>
>(apologies for joining the thread late and if i missed prior discussion on this)
>
>
>>
>>
>> >
>> >
>> >
>> >> __RTM_MAX,
>> >> #define RTM_MAX (((__RTM_MAX + 3) & ~3) - 1)
>> >> };
>> >> diff --git a/net/core/dev.c b/net/core/dev.c
>> >> index ad0d42fbdeee..2a3be2b279d3 100644
>> >> --- a/net/core/dev.c
>> >> +++ b/net/core/dev.c
>> >> @@ -244,7 +244,13 @@ static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>> >> static struct netdev_name_node *
>> >> netdev_name_node_head_alloc(struct net_device *dev)
>> >> {
>> >> - return netdev_name_node_alloc(dev, dev->name);
>> >> + struct netdev_name_node *name_node;
>> >> +
>> >> + name_node = netdev_name_node_alloc(dev, dev->name);
>> >> + if (!name_node)
>> >> + return NULL;
>> >> + INIT_LIST_HEAD(&name_node->list);
>> >> + return name_node;
>> >> }
>> >>
>> >> static void netdev_name_node_free(struct netdev_name_node *name_node)
>> >> @@ -288,6 +294,55 @@ static struct netdev_name_node *netdev_name_node_lookup_rcu(struct net *net,
>> >> return NULL;
>> >> }
>> >>
>> >> +int netdev_name_node_alt_create(struct net_device *dev, char *name)
>> >> +{
>> >> + struct netdev_name_node *name_node;
>> >> + struct net *net = dev_net(dev);
>> >> +
>> >> + name_node = netdev_name_node_lookup(net, name);
>> >> + if (name_node)
>> >> + return -EEXIST;
>> >> + name_node = netdev_name_node_alloc(dev, name);
>> >> + if (!name_node)
>> >> + return -ENOMEM;
>> >> + netdev_name_node_add(net, name_node);
>> >> + /* The node that holds dev->name acts as a head of per-device list. */
>> >> + list_add_tail(&name_node->list, &dev->name_node->list);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +EXPORT_SYMBOL(netdev_name_node_alt_create);
>> >> +
>> >> +static void __netdev_name_node_alt_destroy(struct netdev_name_node *name_node)
>> >> +{
>> >> + list_del(&name_node->list);
>> >> + netdev_name_node_del(name_node);
>> >> + kfree(name_node->name);
>> >> + netdev_name_node_free(name_node);
>> >> +}
>> >> +
>> >> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
>> >> +{
>> >> + struct netdev_name_node *name_node;
>> >> + struct net *net = dev_net(dev);
>> >> +
>> >> + name_node = netdev_name_node_lookup(net, name);
>> >> + if (!name_node)
>> >> + return -ENOENT;
>> >> + __netdev_name_node_alt_destroy(name_node);
>> >> +
>> >> + return 0;
>> >> +}
>> >> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
>> >> +
>> >> +static void netdev_name_node_alt_flush(struct net_device *dev)
>> >> +{
>> >> + struct netdev_name_node *name_node, *tmp;
>> >> +
>> >> + list_for_each_entry_safe(name_node, tmp, &dev->name_node->list, list)
>> >> + __netdev_name_node_alt_destroy(name_node);
>> >> +}
>> >> +
>> >> /* Device list insertion */
>> >> static void list_netdevice(struct net_device *dev)
>> >> {
>> >> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
>> >> dev_uc_flush(dev);
>> >> dev_mc_flush(dev);
>> >>
>> >> + netdev_name_node_alt_flush(dev);
>> >> netdev_name_node_free(dev->name_node);
>> >>
>> >> if (dev->netdev_ops->ndo_uninit)
>> >> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> >> index 1ee6460f8275..7a2010b16e10 100644
>> >> --- a/net/core/rtnetlink.c
>> >> +++ b/net/core/rtnetlink.c
>> >> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> >> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
>> >> [IFLA_MIN_MTU] = { .type = NLA_U32 },
>> >> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> >> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> >> + .len = ALTIFNAMSIZ - 1 },
>> >> };
>> >>
>> >> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>> >> @@ -3373,6 +3375,103 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> return err;
>> >> }
>> >>
>> >> +static int rtnl_newaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> + struct netlink_ext_ack *extack)
>> >> +{
>> >> + struct net *net = sock_net(skb->sk);
>> >> + struct nlattr *tb[IFLA_MAX + 1];
>> >> + struct net_device *dev;
>> >> + struct ifinfomsg *ifm;
>> >> + char *new_alt_ifname;
>> >> + int err;
>> >> +
>> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + ifm = nlmsg_data(nlh);
>> >> + if (ifm->ifi_index > 0) {
>> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> >> + } else if (tb[IFLA_IFNAME]) {
>> >> + char ifname[IFNAMSIZ];
>> >> +
>> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> >> + dev = __dev_get_by_name(net, ifname);
>> >> + } else {
>> >> + return -EINVAL;
>> >> + }
>> >> +
>> >> + if (!dev)
>> >> + return -ENODEV;
>> >> +
>> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> >> + return -EINVAL;
>> >> +
>> >> + new_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> >> + if (!new_alt_ifname)
>> >> + return -ENOMEM;
>> >> +
>> >> + err = netdev_name_node_alt_create(dev, new_alt_ifname);
>> >> + if (err)
>> >> + goto out_free_new_alt_ifname;
>> >> +
>> >> + return 0;
>> >> +
>> >> +out_free_new_alt_ifname:
>> >> + kfree(new_alt_ifname);
>> >> + return err;
>> >> +}
>> >> +
>> >> +static int rtnl_delaltifname(struct sk_buff *skb, struct nlmsghdr *nlh,
>> >> + struct netlink_ext_ack *extack)
>> >> +{
>> >> + struct net *net = sock_net(skb->sk);
>> >> + struct nlattr *tb[IFLA_MAX + 1];
>> >> + struct net_device *dev;
>> >> + struct ifinfomsg *ifm;
>> >> + char *del_alt_ifname;
>> >> + int err;
>> >> +
>> >> + err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFLA_MAX, ifla_policy, extack);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + err = rtnl_ensure_unique_netns(tb, extack, true);
>> >> + if (err)
>> >> + return err;
>> >> +
>> >> + ifm = nlmsg_data(nlh);
>> >> + if (ifm->ifi_index > 0) {
>> >> + dev = __dev_get_by_index(net, ifm->ifi_index);
>> >> + } else if (tb[IFLA_IFNAME]) {
>> >> + char ifname[IFNAMSIZ];
>> >> +
>> >> + nla_strlcpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
>> >> + dev = __dev_get_by_name(net, ifname);
>> >> + } else {
>> >> + return -EINVAL;
>> >> + }
>> >> +
>> >> + if (!dev)
>> >> + return -ENODEV;
>> >> +
>> >> + if (!tb[IFLA_ALT_IFNAME_MOD])
>> >> + return -EINVAL;
>> >> +
>> >> + del_alt_ifname = nla_strdup(tb[IFLA_ALT_IFNAME_MOD], GFP_KERNEL);
>> >> + if (!del_alt_ifname)
>> >> + return -ENOMEM;
>> >> +
>> >> + err = netdev_name_node_alt_destroy(dev, del_alt_ifname);
>> >> + kfree(del_alt_ifname);
>> >> +
>> >> + return err;
>> >> +}
>> >> +
>> >> static u16 rtnl_calcit(struct sk_buff *skb, struct nlmsghdr *nlh)
>> >> {
>> >> struct net *net = sock_net(skb->sk);
>> >> @@ -5331,6 +5430,9 @@ void __init rtnetlink_init(void)
>> >> rtnl_register(PF_UNSPEC, RTM_GETROUTE, NULL, rtnl_dump_all, 0);
>> >> rtnl_register(PF_UNSPEC, RTM_GETNETCONF, NULL, rtnl_dump_all, 0);
>> >>
>> >> + rtnl_register(PF_UNSPEC, RTM_NEWALTIFNAME, rtnl_newaltifname, NULL, 0);
>> >> + rtnl_register(PF_UNSPEC, RTM_DELALTIFNAME, rtnl_delaltifname, NULL, 0);
>> >> +
>> >> rtnl_register(PF_BRIDGE, RTM_NEWNEIGH, rtnl_fdb_add, NULL, 0);
>> >> rtnl_register(PF_BRIDGE, RTM_DELNEIGH, rtnl_fdb_del, NULL, 0);
>> >> rtnl_register(PF_BRIDGE, RTM_GETNEIGH, rtnl_fdb_get, rtnl_fdb_dump, 0);
>> >> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
>> >> index 58345ba0528e..a712b54c666c 100644
>> >> --- a/security/selinux/nlmsgtab.c
>> >> +++ b/security/selinux/nlmsgtab.c
>> >> @@ -83,6 +83,8 @@ static const struct nlmsg_perm nlmsg_route_perms[] =
>> >> { RTM_NEWNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> { RTM_DELNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> { RTM_GETNEXTHOP, NETLINK_ROUTE_SOCKET__NLMSG_READ },
>> >> + { RTM_NEWALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> + { RTM_DELALTIFNAME, NETLINK_ROUTE_SOCKET__NLMSG_WRITE },
>> >> };
>> >>
>> >> static const struct nlmsg_perm nlmsg_tcpdiag_perms[] =
>> >> @@ -166,7 +168,7 @@ int selinux_nlmsg_lookup(u16 sclass, u16 nlmsg_type, u32 *perm)
>> >> * structures at the top of this file with the new mappings
>> >> * before updating the BUILD_BUG_ON() macro!
>> >> */
>> >> - BUILD_BUG_ON(RTM_MAX != (RTM_NEWNEXTHOP + 3));
>> >> + BUILD_BUG_ON(RTM_MAX != (RTM_NEWALTIFNAME + 3));
>> >> err = nlmsg_perm(nlmsg_type, perm, nlmsg_route_perms,
>> >> sizeof(nlmsg_route_perms));
>> >> break;
>> >> --
>> >> 2.21.0
>> >>
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Jiri Pirko @ 2019-08-10 6:30 UTC (permalink / raw)
To: David Ahern
Cc: Roopa Prabhu, netdev, David Miller, Jakub Kicinski,
Stephen Hemminger, dcbw, Michal Kubecek, Andrew Lunn, parav,
Saeed Mahameed, mlxsw
In-Reply-To: <5e7270a1-8de6-1563-4e42-df37da161b98@gmail.com>
Fri, Aug 09, 2019 at 06:14:03PM CEST, dsahern@gmail.com wrote:
>On 8/9/19 9:40 AM, Roopa Prabhu wrote:
>>>>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>>>>> index ce2a623abb75..b36cfd83eb76 100644
>>>>> --- a/include/uapi/linux/rtnetlink.h
>>>>> +++ b/include/uapi/linux/rtnetlink.h
>>>>> @@ -164,6 +164,13 @@ enum {
>>>>> RTM_GETNEXTHOP,
>>>>> #define RTM_GETNEXTHOP RTM_GETNEXTHOP
>>>>>
>>>>> + RTM_NEWALTIFNAME = 108,
>>>>> +#define RTM_NEWALTIFNAME RTM_NEWALTIFNAME
>>>>> + RTM_DELALTIFNAME,
>>>>> +#define RTM_DELALTIFNAME RTM_DELALTIFNAME
>>>>> + RTM_GETALTIFNAME,
>>>>> +#define RTM_GETALTIFNAME RTM_GETALTIFNAME
>>>>> +
>>>>
>>>> I might have missed the prior discussion, why do we need new commands
>>>> ?. can't this simply be part of RTM_*LINK and we use RTM_SETLINK to
>>>> set alternate names ?
>>>
>>> How? This is to add/remove. How do you suggest to to add/remove by
>>> setlink?
>>
>> to that point, I am also not sure why we have a new API For multiple
>> names. I mean why support more than two names (existing old name and
>> a new name to remove the length limitation) ?
>>
>> Your patch series addresses a very important problem (we run into this
>> limitation all the time and its hard to explain it to network
>> operators) and
>> its already unfortunate that we have to have more than one name
>> because we cannot resize the existing one.
>>
>> The best we can do for simpler transition/management from user-space
>> is to keep the api simple..
>> ie keep it close to the management of existing link attributes. Hence
>> the question.
>>
>> I assumed this would be like alias. A single new field that can be
>> referenced in lieu of the old one.
>>
>> Your series is very useful to many of us...but when i think about
>> changing our network manager to accommodate this, I am worried about
>> how many apps will have to change.
>> I agree they have to change regardless but now they will have to
>> listen to yet another notification and msg format for names ?
>>
>> (apologies for joining the thread late and if i missed prior discussion on this)
>
>I agree with Roopa. I do not understand why new RTM commands are needed.
>The existing IFLA + ifinfomsg struct give more than enough ways to id
>the device for adding / deleting an alternate name.
>
Could you please write me an example message of add/remove?
^ permalink raw reply
* Re: [patch net-next] netdevsim: register couple of devlink params
From: Jiri Pirko @ 2019-08-10 6:06 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, mlxsw
In-Reply-To: <20190809142635.52a6275d@cakuba.netronome.com>
Fri, Aug 09, 2019 at 11:26:35PM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 9 Aug 2019 13:05:12 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Register couple of devlink params, one generic, one driver-specific.
>> Make the values available over debugfs.
>>
>> Example:
>> $ echo "111" > /sys/bus/netdevsim/new_device
>> $ devlink dev param
>> netdevsim/netdevsim111:
>> name max_macs type generic
>> values:
>> cmode driverinit value 32
>> name test1 type driver-specific
>> values:
>> cmode driverinit value true
>> $ cat /sys/kernel/debug/netdevsim/netdevsim111/max_macs
>> 32
>> $ cat /sys/kernel/debug/netdevsim/netdevsim111/test1
>> Y
>> $ devlink dev param set netdevsim/netdevsim111 name max_macs cmode driverinit value 16
>> $ devlink dev param set netdevsim/netdevsim111 name test1 cmode driverinit value false
>> $ devlink dev reload netdevsim/netdevsim111
>> $ cat /sys/kernel/debug/netdevsim/netdevsim111/max_macs
>> 16
>> $ cat /sys/kernel/debug/netdevsim/netdevsim111/test1
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>The netdevsim patch looks good, what's the plan for tests?
I have this and a follow-up regions implementation for testing purposes
(netns notificatiosn). I will also need this for syzkaller. Selftest I
have scheduled right after.
>
>We don't need much perhaps what you have in the commit message
>as a script which can be run by automated bots would be sufficient?
^ permalink raw reply
* Re: [pull request][net-next 00/15] Mellanox, mlx5 tc flow handling for concurrent execution (Part 2)
From: David Miller @ 2019-08-10 3:11 UTC (permalink / raw)
To: saeedm; +Cc: netdev
In-Reply-To: <20190809220359.11516-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Fri, 9 Aug 2019 22:04:17 +0000
> This series, mostly from Vlad, is the 2nd part of 3 part series to
> improve mlx5 tc flow handling by removing dependency on rtnl_lock and
> providing a more fine-grained locking and rcu safe data structures to
> allow tc flow handling for concurrent execution.
>
> In this part Vlad handles hairpin, header rewrite and encapsulation
> offloads.
>
> For more information please see tag log below.
>
> Please pull and let me know if there is any problem.
Looks good, pulled, thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/1] tc-testing: added tdc tests for matchall filter
From: David Miller @ 2019-08-10 2:59 UTC (permalink / raw)
To: mrv; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1565390800-26061-1-git-send-email-mrv@mojatatu.com>
From: Roman Mashak <mrv@mojatatu.com>
Date: Fri, 9 Aug 2019 18:46:40 -0400
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net/tls: swap sk_write_space on close
From: David Miller @ 2019-08-10 2:56 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, willemb, davejwatson, borisp, aviadye, john.fastabend,
daniel, oss-drivers, syzbot+dcdc9deefaec44785f32
In-Reply-To: <20190810013623.14707-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 9 Aug 2019 18:36:23 -0700
> Now that we swap the original proto and clear the ULP pointer
> on close we have to make sure no callback will try to access
> the freed state. sk_write_space is not part of sk_prot, remember
> to swap it.
>
> Reported-by: syzbot+dcdc9deefaec44785f32@syzkaller.appspotmail.com
> Fixes: 95fa145479fb ("bpf: sockmap/tls, close can race with map free")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Applied, thanks Jakub.
^ permalink raw reply
* Re: [PATCH net-next] selftests: Fix detection of nettest command in fcnal-test
From: David Miller @ 2019-08-10 2:54 UTC (permalink / raw)
To: dsahern; +Cc: netdev, dsahern
In-Reply-To: <20190809231338.29105-1-dsahern@kernel.org>
From: David Ahern <dsahern@kernel.org>
Date: Fri, 9 Aug 2019 16:13:38 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Most of the tests run by fcnal-test.sh relies on the nettest command.
> Rather than trying to cover all of the individual tests, check for the
> binary only at the beginning.
>
> Also removes the need for log_error which is undefined.
>
> Fixes: 6f9d5cacfe07 ("selftests: Setup for functional tests for fib and socket lookups")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* [PATCH net] net/tls: swap sk_write_space on close
From: Jakub Kicinski @ 2019-08-10 1:36 UTC (permalink / raw)
To: davem
Cc: netdev, willemb, davejwatson, borisp, aviadye, john.fastabend,
daniel, oss-drivers, Jakub Kicinski, syzbot+dcdc9deefaec44785f32
Now that we swap the original proto and clear the ULP pointer
on close we have to make sure no callback will try to access
the freed state. sk_write_space is not part of sk_prot, remember
to swap it.
Reported-by: syzbot+dcdc9deefaec44785f32@syzkaller.appspotmail.com
Fixes: 95fa145479fb ("bpf: sockmap/tls, close can race with map free")
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
net/tls/tls_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 9cbbae606ced..ce6ef56a65ef 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -308,6 +308,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
if (free_ctx)
icsk->icsk_ulp_data = NULL;
sk->sk_prot = ctx->sk_proto;
+ sk->sk_write_space = ctx->sk_write_space;
write_unlock_bh(&sk->sk_callback_lock);
release_sock(sk);
if (ctx->tx_conf == TLS_SW)
--
2.21.0
^ permalink raw reply related
* [PATCH 2/2] ip nexthop: Allow flush|list operations to specify a specific protocol
From: Donald Sharp @ 2019-08-10 0:18 UTC (permalink / raw)
To: netdev, dsahern
In the case where we have a large number of nexthops from a specific
protocol, allow the flush and list operations to take a protocol
to limit the commands scopes.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
ip/ipnexthop.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index f35aab52..bc8ab431 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -19,6 +19,7 @@ static struct {
unsigned int groups;
unsigned int ifindex;
unsigned int master;
+ unsigned int proto;
} filter;
enum {
@@ -34,7 +35,7 @@ static void usage(void) __attribute__((noreturn));
static void usage(void)
{
fprintf(stderr,
- "Usage: ip nexthop { list | flush } SELECTOR\n"
+ "Usage: ip nexthop { list | flush } [ protocol ID ] SELECTOR\n"
" ip nexthop { add | replace } id ID NH [ protocol ID ]\n"
" ip nexthop { get| del } id ID\n"
"SELECTOR := [ id ID ] [ dev DEV ] [ vrf NAME ] [ master DEV ]\n"
@@ -109,6 +110,9 @@ static int flush_nexthop(struct nlmsghdr *nlh, void *arg)
return -1;
}
+ if (filter.proto && nhm->nh_protocol != filter.proto)
+ return 0;
+
parse_rtattr(tb, NHA_MAX, RTM_NHA(nhm), len);
if (tb[NHA_ID])
id = rta_getattr_u32(tb[NHA_ID]);
@@ -213,6 +217,9 @@ int print_nexthop(struct nlmsghdr *n, void *arg)
return -1;
}
+ if (filter.proto && filter.proto != nhm->nh_protocol)
+ return 0;
+
parse_rtattr(tb, NHA_MAX, RTM_NHA(nhm), len);
open_json_object(NULL);
@@ -473,6 +480,13 @@ static int ipnh_list_flush(int argc, char **argv, int action)
if (get_unsigned(&id, *argv, 0))
invarg("invalid id value", *argv);
return ipnh_get_id(id);
+ } else if (!matches(*argv, "protocol")) {
+ __u32 proto;
+
+ NEXT_ARG();
+ if (get_unsigned(&proto, *argv, 0))
+ invarg("invalid protocol value", *argv);
+ filter.proto = proto;
} else if (matches(*argv, "help") == 0) {
usage();
} else {
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] ip nexthop: Add space to display properly when showing a group
From: Donald Sharp @ 2019-08-10 0:18 UTC (permalink / raw)
To: netdev, dsahern
When displaying a nexthop group made up of other nexthops, the display
line shows this when you have additional data at the end:
id 42 group 43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74proto zebra
Modify code so that it shows:
id 42 group 43/44/45/46/47/48/49/50/51/52/53/54/55/56/57/58/59/60/61/62/63/64/65/66/67/68/69/70/71/72/73/74 proto zebra
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
ip/ipnexthop.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
index 97f09e74..f35aab52 100644
--- a/ip/ipnexthop.c
+++ b/ip/ipnexthop.c
@@ -186,6 +186,7 @@ static void print_nh_group(FILE *fp, const struct rtattr *grps_attr)
close_json_object();
}
+ print_string(PRINT_FP, NULL, "%s", " ");
close_json_array(PRINT_JSON, NULL);
}
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox