* [PATCH 15/17] i40e: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Jeff Kirsher, David S. Miller,
intel-wired-lan
In-Reply-To: <20190806161128.31232-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: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../net/ethernet/intel/i40e/i40e_debugfs.c | 21 ++++---------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index 55d20acfcf70..0df9454b3315 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -1732,29 +1732,16 @@ static const struct file_operations i40e_dbg_netdev_ops_fops = {
**/
void i40e_dbg_pf_init(struct i40e_pf *pf)
{
- struct dentry *pfile;
const char *name = pci_name(pf->pdev);
const struct device *dev = &pf->pdev->dev;
pf->i40e_dbg_pf = debugfs_create_dir(name, i40e_dbg_root);
- if (!pf->i40e_dbg_pf)
- return;
-
- pfile = debugfs_create_file("command", 0600, pf->i40e_dbg_pf, pf,
- &i40e_dbg_command_fops);
- if (!pfile)
- goto create_failed;
- pfile = debugfs_create_file("netdev_ops", 0600, pf->i40e_dbg_pf, pf,
- &i40e_dbg_netdev_ops_fops);
- if (!pfile)
- goto create_failed;
+ debugfs_create_file("command", 0600, pf->i40e_dbg_pf, pf,
+ &i40e_dbg_command_fops);
- return;
-
-create_failed:
- dev_info(dev, "debugfs dir/file for %s failed\n", name);
- debugfs_remove_recursive(pf->i40e_dbg_pf);
+ debugfs_create_file("netdev_ops", 0600, pf->i40e_dbg_pf, pf,
+ &i40e_dbg_netdev_ops_fops);
}
/**
--
2.22.0
^ permalink raw reply related
* [PATCH 14/17] fm10k: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Jeff Kirsher, David S. Miller,
intel-wired-lan
In-Reply-To: <20190806161128.31232-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: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
index dca104121c05..1d27b2fb23af 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_debugfs.c
@@ -160,8 +160,6 @@ void fm10k_dbg_q_vector_init(struct fm10k_q_vector *q_vector)
snprintf(name, sizeof(name), "q_vector.%03d", q_vector->v_idx);
q_vector->dbg_q_vector = debugfs_create_dir(name, interface->dbg_intfc);
- if (!q_vector->dbg_q_vector)
- return;
/* Generate a file for each rx ring in the q_vector */
for (i = 0; i < q_vector->tx.count; i++) {
--
2.22.0
^ permalink raw reply related
* [PATCH 13/17] mvpp2: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, David S. Miller, Maxime Chevallier,
Nick Desaulniers, Nathan Huckleberry
In-Reply-To: <20190806161128.31232-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: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Huckleberry <nhuck@google.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
.../ethernet/marvell/mvpp2/mvpp2_debugfs.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 274fb07362cb..4a3baa7e0142 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -452,8 +452,6 @@ static int mvpp2_dbgfs_flow_port_init(struct dentry *parent,
struct dentry *port_dir;
port_dir = debugfs_create_dir(port->dev->name, parent);
- if (IS_ERR(port_dir))
- return PTR_ERR(port_dir);
port_entry = &port->priv->dbgfs_entries->port_flow_entries[port->id];
@@ -480,8 +478,6 @@ static int mvpp2_dbgfs_flow_entry_init(struct dentry *parent,
sprintf(flow_entry_name, "%02d", flow);
flow_entry_dir = debugfs_create_dir(flow_entry_name, parent);
- if (!flow_entry_dir)
- return -ENOMEM;
entry = &priv->dbgfs_entries->flow_entries[flow];
@@ -514,8 +510,6 @@ static int mvpp2_dbgfs_flow_init(struct dentry *parent, struct mvpp2 *priv)
int i, ret;
flow_dir = debugfs_create_dir("flows", parent);
- if (!flow_dir)
- return -ENOMEM;
for (i = 0; i < MVPP2_N_PRS_FLOWS; i++) {
ret = mvpp2_dbgfs_flow_entry_init(flow_dir, priv, i);
@@ -539,8 +533,6 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
sprintf(prs_entry_name, "%03d", tid);
prs_entry_dir = debugfs_create_dir(prs_entry_name, parent);
- if (!prs_entry_dir)
- return -ENOMEM;
entry = &priv->dbgfs_entries->prs_entries[tid];
@@ -578,8 +570,6 @@ static int mvpp2_dbgfs_prs_init(struct dentry *parent, struct mvpp2 *priv)
int i, ret;
prs_dir = debugfs_create_dir("parser", parent);
- if (!prs_dir)
- return -ENOMEM;
for (i = 0; i < MVPP2_PRS_TCAM_SRAM_SIZE; i++) {
ret = mvpp2_dbgfs_prs_entry_init(prs_dir, priv, i);
@@ -688,8 +678,6 @@ static int mvpp2_dbgfs_port_init(struct dentry *parent,
struct dentry *port_dir;
port_dir = debugfs_create_dir(port->dev->name, parent);
- if (IS_ERR(port_dir))
- return PTR_ERR(port_dir);
debugfs_create_file("parser_entries", 0444, port_dir, port,
&mvpp2_dbgfs_port_parser_fops);
@@ -716,15 +704,10 @@ void mvpp2_dbgfs_init(struct mvpp2 *priv, const char *name)
int ret, i;
mvpp2_root = debugfs_lookup(MVPP2_DRIVER_NAME, NULL);
- if (!mvpp2_root) {
+ if (!mvpp2_root)
mvpp2_root = debugfs_create_dir(MVPP2_DRIVER_NAME, NULL);
- if (IS_ERR(mvpp2_root))
- return;
- }
mvpp2_dir = debugfs_create_dir(name, mvpp2_root);
- if (IS_ERR(mvpp2_dir))
- return;
priv->dbgfs_dir = mvpp2_dir;
priv->dbgfs_entries = kzalloc(sizeof(*priv->dbgfs_entries), GFP_KERNEL);
--
2.22.0
^ permalink raw reply related
* [PATCH 12/17] skge: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, Mirko Lindner, Stephen Hemminger,
David S. Miller
In-Reply-To: <20190806161128.31232-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: Mirko Lindner <mlindner@marvell.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/ethernet/marvell/skge.c | 39 +++++++----------------------
1 file changed, 9 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 06dffee81e02..0a2ec387a482 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -3731,7 +3731,6 @@ static int skge_device_event(struct notifier_block *unused,
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
struct skge_port *skge;
- struct dentry *d;
if (dev->netdev_ops->ndo_open != &skge_up || !skge_debug)
goto done;
@@ -3739,33 +3738,20 @@ static int skge_device_event(struct notifier_block *unused,
skge = netdev_priv(dev);
switch (event) {
case NETDEV_CHANGENAME:
- if (skge->debugfs) {
- d = debugfs_rename(skge_debug, skge->debugfs,
- skge_debug, dev->name);
- if (d)
- skge->debugfs = d;
- else {
- netdev_info(dev, "rename failed\n");
- debugfs_remove(skge->debugfs);
- }
- }
+ if (skge->debugfs)
+ skge->debugfs = debugfs_rename(skge_debug,
+ skge->debugfs,
+ skge_debug, dev->name);
break;
case NETDEV_GOING_DOWN:
- if (skge->debugfs) {
- debugfs_remove(skge->debugfs);
- skge->debugfs = NULL;
- }
+ debugfs_remove(skge->debugfs);
+ skge->debugfs = NULL;
break;
case NETDEV_UP:
- d = debugfs_create_file(dev->name, 0444,
- skge_debug, dev,
- &skge_debug_fops);
- if (!d || IS_ERR(d))
- netdev_info(dev, "debugfs create failed\n");
- else
- skge->debugfs = d;
+ skge->debugfs = debugfs_create_file(dev->name, 0444, skge_debug,
+ dev, &skge_debug_fops);
break;
}
@@ -3780,15 +3766,8 @@ static struct notifier_block skge_notifier = {
static __init void skge_debug_init(void)
{
- struct dentry *ent;
-
- ent = debugfs_create_dir("skge", NULL);
- if (!ent || IS_ERR(ent)) {
- pr_info("debugfs create directory failed\n");
- return;
- }
+ skge_debug = debugfs_create_dir("skge", NULL);
- skge_debug = ent;
register_netdevice_notifier(&skge_notifier);
}
--
2.22.0
^ permalink raw reply related
* [PATCH 11/17] qca: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev
Cc: Greg Kroah-Hartman, David S. Miller, Stefan Wahren,
Michael Heimpold, Yangtao Li
In-Reply-To: <20190806161128.31232-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 10/17] dpaa2: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
To: netdev; +Cc: Greg Kroah-Hartman, Ioana Radulescu, David S. Miller
In-Reply-To: <20190806161128.31232-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 00/17] Networking driver debugfs cleanups
From: Greg Kroah-Hartman @ 2019-08-06 16:11 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
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 | 21 +--
.../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 | 12 +-
drivers/net/ieee802154/at86rf230.c | 20 +--
drivers/net/ieee802154/ca8210.c | 9 +-
drivers/net/wimax/i2400m/debugfs.c | 149 +++---------------
drivers/net/wimax/i2400m/driver.c | 7 +-
drivers/net/wimax/i2400m/i2400m.h | 7 +-
drivers/net/wimax/i2400m/usb.c | 61 ++-----
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(+), 799 deletions(-)
--
2.22.0
^ permalink raw reply
* Re: [PATCH net-next v6 5/6] flow_offload: support get multi-subsystem block
From: Pablo Neira Ayuso @ 2019-08-06 16:10 UTC (permalink / raw)
To: wenxu; +Cc: jakub.kicinski, jiri, netfilter-devel, netdev
In-Reply-To: <1564925041-23530-6-git-send-email-wenxu@ucloud.cn>
On Sun, Aug 04, 2019 at 09:24:00PM +0800, wenxu@ucloud.cn wrote:
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index 8f1a7b8..6022dd0 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
[...]
> @@ -282,6 +282,8 @@ int flow_block_cb_setup_simple(struct flow_block_offload *f,
> }
> EXPORT_SYMBOL(flow_block_cb_setup_simple);
>
> +static LIST_HEAD(block_ing_cb_list);
> +
> static struct rhashtable indr_setup_block_ht;
>
> struct flow_indr_block_cb {
> @@ -295,7 +297,6 @@ struct flow_indr_block_dev {
> struct rhash_head ht_node;
> struct net_device *dev;
> unsigned int refcnt;
> - flow_indr_block_ing_cmd_t *block_ing_cmd_cb;
> struct list_head cb_list;
> };
>
> @@ -389,6 +390,22 @@ static void flow_indr_block_cb_del(struct flow_indr_block_cb *indr_block_cb)
> kfree(indr_block_cb);
> }
>
> +static void flow_block_ing_cmd(struct net_device *dev,
> + flow_indr_block_bind_cb_t *cb,
> + void *cb_priv,
> + enum flow_block_command command)
> +{
> + struct flow_indr_block_ing_entry *entry;
> +
> + rcu_read_lock();
> +
unnecessary empty line.
> + list_for_each_entry_rcu(entry, &block_ing_cb_list, list) {
> + entry->cb(dev, cb, cb_priv, command);
> + }
> +
> + rcu_read_unlock();
OK, there's rcu_read_lock here...
> +}
> +
> int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
> flow_indr_block_bind_cb_t *cb,
> void *cb_ident)
> @@ -406,10 +423,8 @@ int __flow_indr_block_cb_register(struct net_device *dev, void *cb_priv,
> if (err)
> goto err_dev_put;
>
> - if (indr_dev->block_ing_cmd_cb)
> - indr_dev->block_ing_cmd_cb(dev, indr_block_cb->cb,
> - indr_block_cb->cb_priv,
> - FLOW_BLOCK_BIND);
> + flow_block_ing_cmd(dev, indr_block_cb->cb, indr_block_cb->cb_priv,
> + FLOW_BLOCK_BIND);
>
> return 0;
>
> @@ -448,10 +463,8 @@ void __flow_indr_block_cb_unregister(struct net_device *dev,
> if (!indr_block_cb)
> return;
>
> - if (indr_dev->block_ing_cmd_cb)
> - indr_dev->block_ing_cmd_cb(dev, indr_block_cb->cb,
> - indr_block_cb->cb_priv,
> - FLOW_BLOCK_UNBIND);
> + flow_block_ing_cmd(dev, indr_block_cb->cb, indr_block_cb->cb_priv,
> + FLOW_BLOCK_UNBIND);
>
> flow_indr_block_cb_del(indr_block_cb);
> flow_indr_block_dev_put(indr_dev);
> @@ -469,7 +482,6 @@ void flow_indr_block_cb_unregister(struct net_device *dev,
> EXPORT_SYMBOL_GPL(flow_indr_block_cb_unregister);
>
> void flow_indr_block_call(struct net_device *dev,
> - flow_indr_block_ing_cmd_t cb,
> struct flow_block_offload *bo,
> enum flow_block_command command)
> {
> @@ -480,15 +492,24 @@ void flow_indr_block_call(struct net_device *dev,
> if (!indr_dev)
> return;
>
> - indr_dev->block_ing_cmd_cb = command == FLOW_BLOCK_BIND
> - ? cb : NULL;
> -
> list_for_each_entry(indr_block_cb, &indr_dev->cb_list, list)
> indr_block_cb->cb(dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
> bo);
> }
> EXPORT_SYMBOL_GPL(flow_indr_block_call);
>
> +void flow_indr_add_block_ing_cb(struct flow_indr_block_ing_entry *entry)
> +{
... but registration does not protect the list with a mutex.
> + list_add_tail_rcu(&entry->list, &block_ing_cb_list);
> +}
> +EXPORT_SYMBOL_GPL(flow_indr_add_block_ing_cb);
^ permalink raw reply
* Re: [PATCH net-next v6 5/6] flow_offload: support get multi-subsystem block
From: Pablo Neira Ayuso @ 2019-08-06 16:07 UTC (permalink / raw)
To: wenxu; +Cc: jakub.kicinski, jiri, netfilter-devel, netdev
In-Reply-To: <20190806160509.qrldfktw37qqorae@salvia>
On Tue, Aug 06, 2019 at 06:05:09PM +0200, Pablo Neira Ayuso wrote:
> On Sun, Aug 04, 2019 at 09:24:00PM +0800, wenxu@ucloud.cn wrote:
> > From: wenxu <wenxu@ucloud.cn>
> >
> > It provide a callback list to find the blocks of tc
> > and nft subsystems
> >
> > Signed-off-by: wenxu <wenxu@ucloud.cn>
> > ---
> > v6: new patch
> >
> > include/net/flow_offload.h | 10 +++++++++-
> > net/core/flow_offload.c | 47 +++++++++++++++++++++++++++++++++-------------
> > net/sched/cls_api.c | 9 ++++++++-
> > 3 files changed, 51 insertions(+), 15 deletions(-)
> >
> > diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> > index 8f1a7b8..6022dd0 100644
> > --- a/include/net/flow_offload.h
> > +++ b/include/net/flow_offload.h
> > @@ -375,6 +375,15 @@ typedef void flow_indr_block_ing_cmd_t(struct net_device *dev,
> > void *cb_priv,
> > enum flow_block_command command);
> >
> > +struct flow_indr_block_ing_entry {
> > + flow_indr_block_ing_cmd_t *cb;
> > + struct list_head list;
> > +};
> > +
> > +void flow_indr_add_block_ing_cb(struct flow_indr_block_ing_entry *entry);
> > +
> > +void flow_indr_del_block_ing_cb(struct flow_indr_block_ing_entry *entry);
>
> Twice the same?
I overlook the _add_ _del_, sorry.
^ permalink raw reply
* Re: [PATCH net-next v6 5/6] flow_offload: support get multi-subsystem block
From: Pablo Neira Ayuso @ 2019-08-06 16:05 UTC (permalink / raw)
To: wenxu; +Cc: jakub.kicinski, jiri, netfilter-devel, netdev
In-Reply-To: <1564925041-23530-6-git-send-email-wenxu@ucloud.cn>
On Sun, Aug 04, 2019 at 09:24:00PM +0800, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> It provide a callback list to find the blocks of tc
> and nft subsystems
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> v6: new patch
>
> include/net/flow_offload.h | 10 +++++++++-
> net/core/flow_offload.c | 47 +++++++++++++++++++++++++++++++++-------------
> net/sched/cls_api.c | 9 ++++++++-
> 3 files changed, 51 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index 8f1a7b8..6022dd0 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
> @@ -375,6 +375,15 @@ typedef void flow_indr_block_ing_cmd_t(struct net_device *dev,
> void *cb_priv,
> enum flow_block_command command);
>
> +struct flow_indr_block_ing_entry {
> + flow_indr_block_ing_cmd_t *cb;
> + struct list_head list;
> +};
> +
> +void flow_indr_add_block_ing_cb(struct flow_indr_block_ing_entry *entry);
> +
> +void flow_indr_del_block_ing_cb(struct flow_indr_block_ing_entry *entry);
Twice the same?
^ permalink raw reply
* [PATCH 2/2 net,v4] netfilter: nf_tables: map basechain priority to hardware priority
From: Pablo Neira Ayuso @ 2019-08-06 16:03 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, marcelo.leitner, jiri, wenxu, saeedm, paulb,
gerlitz.or, jakub.kicinski
In-Reply-To: <20190806160310.6663-1-pablo@netfilter.org>
This patch adds initial support for offloading basechains using
the priority range from -32767 to 32767. This is restricting the
netfilter priority range to 16-bit integer since this is what most
drivers assume so far from tc.
The software to hardware priority mapping is not exposed to userspace.
Hence, it should be possible to extend this range of supported priorities
later on once drivers are updated to support for 32-bit integer
priorities.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v4: no changes wrt. [PATCH net 2/2,v3] netfilter: nf_tables: map basechain
priority to hardware priority.
include/net/netfilter/nf_tables_offload.h | 2 ++
net/netfilter/nf_tables_api.c | 4 ++++
net/netfilter/nf_tables_offload.c | 18 +++++++++++++++---
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/include/net/netfilter/nf_tables_offload.h b/include/net/netfilter/nf_tables_offload.h
index 3196663a10e3..3c31e9d55028 100644
--- a/include/net/netfilter/nf_tables_offload.h
+++ b/include/net/netfilter/nf_tables_offload.h
@@ -73,4 +73,6 @@ int nft_flow_rule_offload_commit(struct net *net);
(__reg)->key = __key; \
memset(&(__reg)->mask, 0xff, (__reg)->len);
+u32 nft_chain_offload_priority(struct nft_base_chain *basechain);
+
#endif
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 605a7cfe7ca7..9cf0fecf5cb9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1662,6 +1662,10 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
chain->flags |= NFT_BASE_CHAIN | flags;
basechain->policy = NF_ACCEPT;
+ if (chain->flags & NFT_CHAIN_HW_OFFLOAD &&
+ !nft_chain_offload_priority(basechain))
+ return -EOPNOTSUPP;
+
flow_block_init(&basechain->flow_block);
} else {
chain = kzalloc(sizeof(*chain), GFP_KERNEL);
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 64f5fd5f240e..81d636fac571 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -103,10 +103,11 @@ void nft_offload_update_dependency(struct nft_offload_ctx *ctx,
}
static void nft_flow_offload_common_init(struct flow_cls_common_offload *common,
- __be16 proto,
- struct netlink_ext_ack *extack)
+ __be16 proto, int priority,
+ struct netlink_ext_ack *extack)
{
common->protocol = proto;
+ common->prio = priority;
common->extack = extack;
}
@@ -124,6 +125,15 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
return 0;
}
+u32 nft_chain_offload_priority(struct nft_base_chain *basechain)
+{
+ if (basechain->ops.priority < SHRT_MIN ||
+ basechain->ops.priority > SHRT_MAX)
+ return 0;
+
+ return basechain->ops.priority + abs(SHRT_MIN);
+}
+
static int nft_flow_offload_rule(struct nft_trans *trans,
enum flow_cls_command command)
{
@@ -142,7 +152,9 @@ static int nft_flow_offload_rule(struct nft_trans *trans,
if (flow)
proto = flow->proto;
- nft_flow_offload_common_init(&cls_flow.common, proto, &extack);
+ nft_flow_offload_common_init(&cls_flow.common, proto,
+ nft_chain_offload_priority(basechain),
+ &extack);
cls_flow.command = command;
cls_flow.cookie = (unsigned long) rule;
if (flow)
--
2.11.0
^ permalink raw reply related
* [PATCH 1/2 net,v4] net: sched: use major priority number as hardware priority
From: Pablo Neira Ayuso @ 2019-08-06 16:03 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, marcelo.leitner, jiri, wenxu, saeedm, paulb,
gerlitz.or, jakub.kicinski
In-Reply-To: <20190806160310.6663-1-pablo@netfilter.org>
tc transparently maps the software priority number to hardware. Update
it to pass the major priority which is what most drivers expect. Update
drivers too so they do not need to lshift the priority field of the
flow_cls_common_offload object. The stmmac driver is an exception, since
this code assumes the tc software priority is fine, therefore, lshift it
just to be conservative.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
v4: no changes.
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
drivers/net/ethernet/mscc/ocelot_flower.c | 12 +++---------
drivers/net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 +-
include/net/pkt_cls.h | 2 +-
6 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 7ecfc53cf5f6..0c0365f37019 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -3158,7 +3158,7 @@ mlx5e_flow_esw_attr_init(struct mlx5_esw_flow_attr *esw_attr,
esw_attr->parse_attr = parse_attr;
esw_attr->chain = f->common.chain_index;
- esw_attr->prio = TC_H_MAJ(f->common.prio) >> 16;
+ esw_attr->prio = f->common.prio;
esw_attr->in_rep = in_rep;
esw_attr->in_mdev = in_mdev;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
index e8ac90564dbe..84a87d059333 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c
@@ -471,7 +471,7 @@ int mlxsw_sp_acl_rulei_commit(struct mlxsw_sp_acl_rule_info *rulei)
void mlxsw_sp_acl_rulei_priority(struct mlxsw_sp_acl_rule_info *rulei,
unsigned int priority)
{
- rulei->priority = priority >> 16;
+ rulei->priority = priority;
}
void mlxsw_sp_acl_rulei_keymask_u32(struct mlxsw_sp_acl_rule_info *rulei,
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 59487d446a09..b894bc0c9c16 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c
@@ -13,12 +13,6 @@ struct ocelot_port_block {
struct ocelot_port *port;
};
-static u16 get_prio(u32 prio)
-{
- /* prio starts from 0x1000 while the ids starts from 0 */
- return prio >> 16;
-}
-
static int ocelot_flower_parse_action(struct flow_cls_offload *f,
struct ocelot_ace_rule *rule)
{
@@ -168,7 +162,7 @@ static int ocelot_flower_parse(struct flow_cls_offload *f,
}
finished_key_parsing:
- ocelot_rule->prio = get_prio(f->common.prio);
+ ocelot_rule->prio = f->common.prio;
ocelot_rule->id = f->cookie;
return ocelot_flower_parse_action(f, ocelot_rule);
}
@@ -218,7 +212,7 @@ static int ocelot_flower_destroy(struct flow_cls_offload *f,
struct ocelot_ace_rule rule;
int ret;
- rule.prio = get_prio(f->common.prio);
+ rule.prio = f->common.prio;
rule.port = port_block->port;
rule.id = f->cookie;
@@ -236,7 +230,7 @@ static int ocelot_flower_stats_update(struct flow_cls_offload *f,
struct ocelot_ace_rule rule;
int ret;
- rule.prio = get_prio(f->common.prio);
+ rule.prio = f->common.prio;
rule.port = port_block->port;
rule.id = f->cookie;
ret = ocelot_ace_rule_stats_update(&rule);
diff --git a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
index 86e968cd5ffd..124a43dc136a 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/qos_conf.c
@@ -93,7 +93,7 @@ nfp_flower_install_rate_limiter(struct nfp_app *app, struct net_device *netdev,
return -EOPNOTSUPP;
}
- if (flow->common.prio != (1 << 16)) {
+ if (flow->common.prio != 1) {
NL_SET_ERR_MSG_MOD(extack, "unsupported offload: qos rate limit offload requires highest priority");
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
index 58ea18af9813..5cd040215469 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c
@@ -94,7 +94,7 @@ static int tc_fill_entry(struct stmmac_priv *priv,
struct stmmac_tc_entry *entry, *frag = NULL;
struct tc_u32_sel *sel = cls->knode.sel;
u32 off, data, mask, real_off, rem;
- u32 prio = cls->common.prio;
+ u32 prio = cls->common.prio << 16;
int ret;
/* Only 1 match per entry */
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e429809ca90d..98be18ef1ed3 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -646,7 +646,7 @@ tc_cls_common_offload_init(struct flow_cls_common_offload *cls_common,
{
cls_common->chain_index = tp->chain->index;
cls_common->protocol = tp->protocol;
- cls_common->prio = tp->prio;
+ cls_common->prio = tp->prio >> 16;
if (tc_skip_sw(flags) || flags & TCA_CLS_FLAGS_VERBOSE)
cls_common->extack = extack;
}
--
2.11.0
^ permalink raw reply related
* [PATCH 0/2 net,v4] flow_offload hardware priority fixes
From: Pablo Neira Ayuso @ 2019-08-06 16:03 UTC (permalink / raw)
To: netfilter-devel
Cc: davem, netdev, marcelo.leitner, jiri, wenxu, saeedm, paulb,
gerlitz.or, jakub.kicinski
Hi,
This patchset contains two updates for the flow_offload users:
1) Pass the major tc priority to drivers so they do not have to
lshift it. This is a preparation patch for the fix coming in
patch #2.
2) Set the hardware priority from the netfilter basechain priority,
some drivers break when using the existing hardware priority
number that is set to zero.
Please, apply.
Pablo Neira Ayuso (2):
net: sched: use major priority number as hardware priority
netfilter: nf_tables: map basechain priority to hardware priority
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 2 +-
drivers/net/ethernet/mscc/ocelot_flower.c | 12 +++---------
drivers/net/ethernet/netronome/nfp/flower/qos_conf.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c | 2 +-
include/net/netfilter/nf_tables_offload.h | 2 ++
include/net/pkt_cls.h | 2 +-
net/netfilter/nf_tables_api.c | 4 ++++
net/netfilter/nf_tables_offload.c | 18 +++++++++++++++---
9 files changed, 29 insertions(+), 17 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH iproute2-next] ss: sctp: fix typo for nodelay
From: Stephen Hemminger @ 2019-08-06 15:51 UTC (permalink / raw)
To: Patrick Talbert; +Cc: netdev
In-Reply-To: <20190803083741.24122-1-ptalbert@redhat.com>
On Sat, 3 Aug 2019 10:37:41 +0200
Patrick Talbert <ptalbert@redhat.com> wrote:
> nodealy should be nodelay.
>
> Signed-off-by: Patrick Talbert <ptalbert@redhat.com>
Both patches applied to current iproute2
^ permalink raw reply
* Re: [PATCH 15/16] net: phy: adin: add ethtool get_stats support
From: Andrew Lunn @ 2019-08-06 15:46 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <ce952e3f8d927cdbccb268d708b4e47179d69b06.camel@analog.com>
On Tue, Aug 06, 2019 at 07:11:57AM +0000, Ardelean, Alexandru wrote:
> On Mon, 2019-08-05 at 17:28 +0200, Andrew Lunn wrote:
> > [External]
> >
> > > +struct adin_hw_stat {
> > > + const char *string;
> > > +static void adin_get_strings(struct phy_device *phydev, u8 *data)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++) {
> > > + memcpy(data + i * ETH_GSTRING_LEN,
> > > + adin_hw_stats[i].string, ETH_GSTRING_LEN);
> >
> > You define string as a char *. So it will be only as long as it should
> > be. However memcpy always copies ETH_GSTRING_LEN bytes, doing off the
> > end of the string and into whatever follows.
> >
>
> hmm, will use strlcpy()
> i blindedly copied memcpy() from some other driver
Hopefully that driver used const char string[ETH_GSTRING_LEN]. Then a
memcpy is safe. If not, please let me know what driver you copied.
> i'm afraid i don't understand about the snapshot feature you are mentioning;
> i.e. i don't remember seeing it in other chips;
It is frequency done at the MAC layer for statistics. You tell the
hardware to snapshot all the statistics. It atomically makes a copy of
all the statistics into a set of registers. These values are then
static, and consistent between counters. You can read them out knowing
they are not going to change.
> regarding the danger that stat->reg1 rolls over, i guess that is
> possible, but it's a bit hard to guard against;
The normal solution is the read the MSB, the LSB and then the MSB
again. If the MSB value has changed between the two reads, you know a
roll over has happened, and you need to do it all again.
Andrew
^ permalink raw reply
* Re: [PATCH 06/16] net: phy: adin: support PHY mode converters
From: Andrew Lunn @ 2019-08-06 15:39 UTC (permalink / raw)
To: Ardelean, Alexandru
Cc: davem@davemloft.net, hkallweit1@gmail.com,
devicetree@vger.kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, f.fainelli@gmail.com,
netdev@vger.kernel.org, robh+dt@kernel.org
In-Reply-To: <15cf5732415c313a7bfe610e7039e7c97b987073.camel@analog.com>
On Tue, Aug 06, 2019 at 06:47:08AM +0000, Ardelean, Alexandru wrote:
> On Mon, 2019-08-05 at 16:51 +0200, Andrew Lunn wrote:
> > [External]
> >
> > On Mon, Aug 05, 2019 at 07:54:43PM +0300, Alexandru Ardelean wrote:
> > > Sometimes, the connection between a MAC and PHY is done via a
> > > mode/interface converter. An example is a GMII-to-RGMII converter, which
> > > would mean that the MAC operates in GMII mode while the PHY operates in
> > > RGMII. In this case there is a discrepancy between what the MAC expects &
> > > what the PHY expects and both need to be configured in their respective
> > > modes.
> > >
> > > Sometimes, this converter is specified via a board/system configuration (in
> > > the device-tree for example). But, other times it can be left unspecified.
> > > The use of these converters is common in boards that have FPGA on them.
> > >
> > > This patch also adds support for a `adi,phy-mode-internal` property that
> > > can be used in these (implicit convert) cases. The internal PHY mode will
> > > be used to specify the correct register settings for the PHY.
> > >
> > > `fwnode_handle` is used, since this property may be specified via ACPI as
> > > well in other setups, but testing has been done in DT context.
> >
> > Looking at the patch, you seems to assume phy-mode is what the MAC is
> > using? That seems rather odd, given the name. It seems like a better
> > solution would be to add a mac-mode, which the MAC uses to configure
> > its side of the link. The MAC driver would then implement this
> > property.
> >
>
> actually, that's a pretty good idea;
> i guess i was narrow-minded when writing the driver, and got stuck on phy specifics, and forgot about the MAC-side;
> [ i also catch these design elements when reviewing, but i also seem to miss them when writing stuff sometimes ]
>
Hi Ardelean
We should also consider the media converter itself. It is passive, or
does it need a driver. You seems to be considering GMII-to-RGMII. But
what about RGMII to SGMII? or RGMII to 1000Base-KX etc? Ideally we
want a generic solution and we need to think about all the parts in
the system.
Andrew
^ permalink raw reply
* Re: [PATCH] tools: bpftool: fix reading from /proc/config.gz
From: Alexei Starovoitov @ 2019-08-06 15:27 UTC (permalink / raw)
To: Quentin Monnet
Cc: Peter Wu, Jakub Kicinski, Stanislav Fomichev, Alexei Starovoitov,
Daniel Borkmann, Network Development, Stanislav Fomichev
In-Reply-To: <f0e5683b-ea6c-4966-6785-f154697f76f1@netronome.com>
On Tue, Aug 6, 2019 at 2:36 AM Quentin Monnet
<quentin.monnet@netronome.com> wrote:
>
> >>> {
> >>> - size_t line_n = 0, optlen = strlen(option);
> >>> - char *res, *strval, *line = NULL;
> >>> - ssize_t n;
> >>> + char *sep;
> >>> + ssize_t linelen;
> >>
> >> Please order the declarations in reverse-Christmas tree style.
> >
> > Does this refer to the type, name, or full line length? I did not find
> > this in CodingStyle, the closest I could get is:
> > https://lore.kernel.org/patchwork/patch/732076/
> >
> > I will assume the line length for now.
>
> I am unsure this is in the CodingStyle, but fairly certain that this is
> a convention for at least network-related code. And yes, as I understand
> it refers to the length of the line.
Let's not over focus on this.
It's a preference, but not a strong requirement.
There are plenty of cases where it shouldn't be followed.
Like logical grouping of variables take precedence of xmas tree.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: net: snps,dwmac: update reg minItems maxItems
From: Rob Herring @ 2019-08-06 15:22 UTC (permalink / raw)
To: Neil Armstrong, Maxime Ripard
Cc: Martin Blumenstingl, devicetree, netdev, linux-amlogic,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel@vger.kernel.org
In-Reply-To: <20190806125041.16105-2-narmstrong@baylibre.com>
+Maxime
On Tue, Aug 6, 2019 at 6:50 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> The Amlogic Meson DWMAC glue bindings needs a second reg cells for the
> glue registers, thus update the reg minItems/maxItems to allow more
> than a single reg cell.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> Documentation/devicetree/bindings/net/snps,dwmac.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
I haven't checked, but the derivative schema could be assuming this
schema enforced reg is 1 item. I don't think that's a major issue
though.
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/2] selftests/bpf: more loop tests
From: Alexei Starovoitov @ 2019-08-06 15:21 UTC (permalink / raw)
To: Yonghong Song
Cc: Alexei Starovoitov, davem@davemloft.net, daniel@iogearbox.net,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <ff86b565-097e-8f1a-c411-7138f429ed79@fb.com>
On Mon, Aug 5, 2019 at 9:19 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/5/19 7:17 PM, Alexei Starovoitov wrote:
> > Add two bounded loop tests.
> >
> > v1-v2: addressed feedback from Yonghong.
> >
> > Alexei Starovoitov (2):
> > selftests/bpf: add loop test 4
> > selftests/bpf: add loop test 5
>
> Looks good to me. Ack for the whole series.
> Acked-by: Yonghong Song <yhs@fb.com>
Applied. Thanks
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: net: meson-dwmac: convert to yaml
From: Rob Herring @ 2019-08-06 15:15 UTC (permalink / raw)
To: Neil Armstrong
Cc: Martin Blumenstingl, devicetree, netdev, linux-amlogic,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel@vger.kernel.org
In-Reply-To: <20190806125041.16105-3-narmstrong@baylibre.com>
On Tue, Aug 6, 2019 at 6:50 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Now that we have the DT validation in place, let's convert the device tree
> bindings for the Synopsys DWMAC Glue for Amlogic SoCs over to a YAML schemas.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> .../bindings/net/amlogic,meson-dwmac.yaml | 113 ++++++++++++++++++
> .../devicetree/bindings/net/meson-dwmac.txt | 71 -----------
> .../devicetree/bindings/net/snps,dwmac.yaml | 5 +
> 3 files changed, 118 insertions(+), 71 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> delete mode 100644 Documentation/devicetree/bindings/net/meson-dwmac.txt
I don't love the compatible schema with 'additionalItems: true' and
contains, but I guess it is what it is. I'm hopeful schemas help limit
how many variations we end up with.
Reviewed-by: Rob Herring <robh@kernel.org>
Rob
^ permalink raw reply
* Re: [PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver
From: Rob Herring @ 2019-08-06 15:04 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: netdev, devicetree, linux-kernel@vger.kernel.org, David Miller,
Mark Rutland, Florian Fainelli, Heiner Kallweit, Andrew Lunn
In-Reply-To: <20190805165453.3989-17-alexandru.ardelean@analog.com>
On Mon, Aug 5, 2019 at 7:55 AM Alexandru Ardelean
<alexandru.ardelean@analog.com> wrote:
>
> This change adds bindings for the Analog Devices ADIN PHY driver, detailing
> all the properties implemented by the driver.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> .../devicetree/bindings/net/adi,adin.yaml | 93 +++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/net/adin.h | 26 ++++++
> 3 files changed, 121 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml
> create mode 100644 include/dt-bindings/net/adin.h
>
> diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml b/Documentation/devicetree/bindings/net/adi,adin.yaml
> new file mode 100644
> index 000000000000..fcf884bb86f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
> @@ -0,0 +1,93 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/adi,adin.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADIN1200/ADIN1300 PHY
> +
> +maintainers:
> + - Alexandru Ardelean <alexandru.ardelean@analog.com>
> +
> +description: |
> + Bindings for Analog Devices Industrial Ethernet PHYs
> +
Needs an:
allOf:
- $ref: ethernet-phy.yaml#
> +properties:
> + compatible:
> + description: |
> + Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case
> + Clause 45 will be used to access device management registers. If
> + unspecified, Clause 22 will be used. Use this only when MDIO supports
> + Clause 45 access, but there is no other way to determine this.
> + enum:
> + - ethernet-phy-ieee802.3-c45
Then you can drop 'compatible' from here as it is covered by the above schema.
> +
> + adi,phy-mode-internal:
> + $ref: /schemas/types.yaml#/definitions/string
This has to be under an 'allOf' or it doesn't actually work. Same below.
> + description: |
No special formatting here, you can drop '|'.
> + The internal mode of the PHY. This assumes that there is a PHY converter
> + in-between the MAC & PHY.
> + enum: [ "rgmii", "rgmii-id", "rgmii-txid", "rgmii-rxid", "rmii", "mii" ]
Don't need quotes here.
> +
> + adi,rx-internal-delay:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> + default value is 0 (which represents 2 ns)
Use 'default: 0' to specify defaults.
> + enum: [ 0, 1, 2, 6, 7 ]
> +
> + adi,tx-internal-delay:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
> + is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
> + default value is 0 (which represents 2 ns)
> + enum: [ 0, 1, 2, 6, 7 ]
> +
> + adi,fifo-depth:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + When operating in RMII mode, this option configures the FIFO depth.
> + See `dt-bindings/net/adin.h`.
> + enum: [ 0, 1, 2, 3, 4, 5 ]
> +
> + adi,eee-enabled:
Isn't there a standard property for EEE control?
> + description: |
> + Advertise EEE capabilities on power-up/init (default disabled)
> + type: boolean
> +
> + adi,disable-energy-detect:
> + description: |
> + Disables Energy Detect Powerdown Mode (default disabled, i.e energy detect
> + is enabled if this property is unspecified)
> + type: boolean
> +
> + reset-gpios:
> + description: |
> + GPIO to reset the PHY
> + see Documentation/devicetree/bindings/gpio/gpio.txt.
Active high or low?
> +
> +examples:
> + - |
> + ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c45";
> + reg = <0>;
> + };
Not really anything specific to this binding. Drop it.
> + - |
> + #include <dt-bindings/net/adin.h>
> + ethernet-phy@1 {
> + reg = <1>;
> + adi,phy-mode-internal = "rgmii-id";
> +
> + adi,rx-internal-delay = <ADIN1300_RGMII_1_80_NS>;
> + adi,tx-internal-delay = <ADIN1300_RGMII_2_20_NS>;
> + };
> + - |
> + #include <dt-bindings/net/adin.h>
> + ethernet-phy@2 {
> + reg = <2>;
> + phy-mode = "rmii";
> +
> + adi,fifo-depth = <ADIN1300_RMII_16_BITS>;
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index faf5723610c8..6ffbb266dee4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -944,6 +944,8 @@ L: netdev@vger.kernel.org
> W: http://ez.analog.com/community/linux-device-drivers
> S: Supported
> F: drivers/net/phy/adin.c
> +F: include/dt-bindings/net/adin.h
> +F: Documentation/devicetree/bindings/net/adi,adin.yaml
>
> ANALOG DEVICES INC ADIS DRIVER LIBRARY
> M: Alexandru Ardelean <alexandru.ardelean@analog.com>
> diff --git a/include/dt-bindings/net/adin.h b/include/dt-bindings/net/adin.h
> new file mode 100644
> index 000000000000..4c3afa550c59
> --- /dev/null
> +++ b/include/dt-bindings/net/adin.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/**
> + * Device Tree constants for Analog Devices Industrial Ethernet PHYs
> + *
> + * Copyright 2019 Analog Devices Inc.
> + */
> +
> +#ifndef _DT_BINDINGS_ADIN_H
> +#define _DT_BINDINGS_ADIN_H
> +
> +/* RGMII internal delay settings for rx and tx for ADIN1300 */
> +#define ADIN1300_RGMII_1_60_NS 0x1
> +#define ADIN1300_RGMII_1_80_NS 0x2
> +#define ADIN1300_RGMII_2_00_NS 0x0
> +#define ADIN1300_RGMII_2_20_NS 0x6
> +#define ADIN1300_RGMII_2_40_NS 0x7
> +
> +/* RMII fifo depth values */
> +#define ADIN1300_RMII_4_BITS 0x0
> +#define ADIN1300_RMII_8_BITS 0x1
> +#define ADIN1300_RMII_12_BITS 0x2
> +#define ADIN1300_RMII_16_BITS 0x3
> +#define ADIN1300_RMII_20_BITS 0x4
> +#define ADIN1300_RMII_24_BITS 0x5
> +
> +#endif
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH net-next] ibmveth: Allow users to update reported speed and duplex
From: Thomas Falcon @ 2019-08-06 14:41 UTC (permalink / raw)
To: Michael Ellerman, netdev; +Cc: linuxppc-dev
In-Reply-To: <87zhkmvbec.fsf@concordia.ellerman.id.au>
On 8/6/19 5:25 AM, Michael Ellerman wrote:
> Thomas Falcon <tlfalcon@linux.ibm.com> writes:
>> Reported ethtool link settings for the ibmveth driver are currently
>> hardcoded and no longer reflect the actual capabilities of supported
>> hardware. There is no interface designed for retrieving this information
>> from device firmware nor is there any way to update current settings
>> to reflect observed or expected link speeds.
>>
>> To avoid confusion, initially define speed and duplex as unknown and
> Doesn't that risk break existing setups?
You're right, sorry for missing that.
>
>> allow the user to alter these settings to match the expected
>> capabilities of underlying hardware if needed. This update would allow
>> the use of configurations that rely on certain link speed settings,
>> such as LACP. This patch is based on the implementation in virtio_net.
> Wouldn't it be safer to keep the current values as the default, and then
> also allow them to be overridden by a motivated user.
That is a good compromise. I will resend an updated version soon with
that change.
Thanks!
>
> cheers
>
^ permalink raw reply
* [PATCH net-next v5] net: can: Fix sparse warnings for two functions
From: Mao Wenan @ 2019-08-06 14:40 UTC (permalink / raw)
To: socketcan, davem, netdev; +Cc: linux-kernel, kernel-janitors, Mao Wenan
In-Reply-To: <d928a635-accd-2a8f-1829-5d7da551a8e8@web.de>
There are two warnings in net/can, fix them by setting
bcm_sock_no_ioctlcmd and raw_sock_no_ioctlcmd as static.
net/can/bcm.c:1683:5: warning: symbol 'bcm_sock_no_ioctlcmd' was not declared. Should it be static?
net/can/raw.c:840:5: warning: symbol 'raw_sock_no_ioctlcmd' was not declared. Should it be static?
Fixes: 473d924d7d46 ("can: fix ioctl function removal")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
v2: change patch description typo error, 'warings' to 'warnings'.
v3: change subject of patch.
v4: change the alignment of two functions.
v5: change subject of patch.
net/can/bcm.c | 4 ++--
net/can/raw.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index bf1d0bbecec8..eb1d28b8c46a 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1680,8 +1680,8 @@ static int bcm_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
return size;
}
-int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
- unsigned long arg)
+static int bcm_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+ unsigned long arg)
{
/* no ioctls for socket layer -> hand it down to NIC layer */
return -ENOIOCTLCMD;
diff --git a/net/can/raw.c b/net/can/raw.c
index da386f1fa815..a30aaecd9327 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -837,8 +837,8 @@ static int raw_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
return size;
}
-int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
- unsigned long arg)
+static int raw_sock_no_ioctlcmd(struct socket *sock, unsigned int cmd,
+ unsigned long arg)
{
/* no ioctls for socket layer -> hand it down to NIC layer */
return -ENOIOCTLCMD;
--
2.20.1
^ permalink raw reply related
* Realtek r8822be wireless card fails to work with new rtw88 kernel module
From: 고준 @ 2019-08-06 14:31 UTC (permalink / raw)
To: linux-wireless, netdev, linux-kernel
Hello,
I recently reported a bug to Ubuntu regarding a regression in wireless
driver support for the Realtek r8822be wireless chipset. The issue
link on launchpad is:
https://bugs.launchpad.net/bugs/1838133
After Canonical developers triaged the bug they determined that the
problem lies upstream, and instructed me to send mails to the relevant
kernel module maintainers at Realtek and to the general kernel.org
mailing list.
I built kernel 5.3.0-rc1+ with the latest realtek drivers from
wireless-drivers-next but my Realtek r8822be doesn't work with
rtw88/rtwpci kernel modules.
Please let me know if there is any additional information I can
provide that would help in debugging this issue.
Best regards,
Jun
Link to GPG Public Key:
https://keybase.io/gojun077#show-public
Backup link:
https://keys.openpgp.org/vks/v1/by-fingerprint/79F173A93EB3623D32F86309A56930CF7235138D
^ permalink raw reply
* [PATCH] nfc: st-nci: Fix an incorrect skb_buff size in 'st_nci_i2c_read()'
From: Christophe JAILLET @ 2019-08-06 14:16 UTC (permalink / raw)
To: tglx, gregkh, colin.king, davem, allison
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
In 'st_nci_i2c_read()', we allocate a sk_buff with a size of
ST_NCI_I2C_MIN_SIZE + len.
However, later on, we first 'skb_reserve()' ST_NCI_I2C_MIN_SIZE bytes, then
we 'skb_put()' ST_NCI_I2C_MIN_SIZE bytes.
Finally, if 'len' is not 0, we 'skb_put()' 'len' bytes.
So we use ST_NCI_I2C_MIN_SIZE*2 + len bytes.
This is incorrect and should already panic. I guess that it does not occur
because of extra memory allocated because of some rounding.
Fix it and allocate enough room for the 'skb_reserve()' and the 'skb_put()'
calls.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
This patch is LIKELY INCORRECT. So think twice to what is the correct
solution before applying it.
Maybe the skb_reserve should be axed or some other sizes are incorrect.
There seems to be an issue, that's all I can say.
---
drivers/nfc/st-nci/i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nfc/st-nci/i2c.c b/drivers/nfc/st-nci/i2c.c
index 55d600cd3861..12e0425131c8 100644
--- a/drivers/nfc/st-nci/i2c.c
+++ b/drivers/nfc/st-nci/i2c.c
@@ -126,7 +126,7 @@ static int st_nci_i2c_read(struct st_nci_i2c_phy *phy,
return -EBADMSG;
}
- *skb = alloc_skb(ST_NCI_I2C_MIN_SIZE + len, GFP_KERNEL);
+ *skb = alloc_skb(ST_NCI_I2C_MIN_SIZE * 2 + len, GFP_KERNEL);
if (*skb == NULL)
return -ENOMEM;
--
2.20.1
^ 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