* Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: sync the first fragment unconditionally
From: Alexander Duyck @ 2019-08-06 19:52 UTC (permalink / raw)
To: David Miller; +Cc: firo.yang, Netdev, intel-wired-lan, LKML
In-Reply-To: <20190806.113640.171591509807004446.davem@davemloft.net>
On Tue, Aug 6, 2019 at 11:36 AM David Miller <davem@davemloft.net> wrote:
>
> From: Firo Yang <firo.yang@suse.com>
> Date: Tue, 6 Aug 2019 09:29:51 +0000
>
> > In Xen environment, if Xen-swiotlb is enabled, ixgbe driver
> > could possibly allocate a page, DMA memory buffer, for the first
> > fragment which is not suitable for Xen-swiotlb to do DMA operations.
> > Xen-swiotlb will internally allocate another page for doing DMA
> > operations. It requires syncing between those two pages. Otherwise,
> > we may get an incomplete skb. To fix this problem, sync the first
> > fragment no matter the first fargment is makred as "page_released"
> > or not.
> >
> > Signed-off-by: Firo Yang <firo.yang@suse.com>
>
> I don't understand, an unmap operation implies a sync operation.
Actually it doesn't because ixgbe is mapping and unmapping with
DMA_ATTR_SKIP_CPU_SYNC.
The patch description isn't very good. The issue is that the sync in
this case is being skipped in ixgbe_get_rx_buffer for a frame where
the buffer spans more then a single page. As such we need to do both
the sync and the unmap call on the last frame when we encounter the
End Of Packet.
^ permalink raw reply
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 19:48 UTC (permalink / raw)
To: Stefan Schmidt
Cc: netdev, Michael Hennerich, Alexander Aring, David S. Miller,
Harry Morris, linux-wpan
In-Reply-To: <a16654ca-078a-8ce4-91a5-055a3ad6e838@datenfreihafen.org>
On Tue, Aug 06, 2019 at 09:22:43PM +0200, Stefan Schmidt wrote:
> Hello.
>
> On 06.08.19 18:11, Greg Kroah-Hartman wrote:
> > 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: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Alexander Aring <alex.aring@gmail.com>
> > Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Harry Morris <h.morris@cascoda.com>
> > Cc: linux-wpan@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/net/ieee802154/adf7242.c | 12 +++---------
> > drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> > drivers/net/ieee802154/ca8210.c | 9 +--------
> > 3 files changed, 9 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> > index c9392d70e639..7b95add2235a 100644
> > --- a/drivers/net/ieee802154/adf7242.c
> > +++ b/drivers/net/ieee802154/adf7242.c
> > @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> > return 0;
> > }
> >
> > -static int adf7242_debugfs_init(struct adf7242_local *lp)
> > +static void adf7242_debugfs_init(struct adf7242_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> > struct dentry *stats;
>
> A quick look over the code indicates that the stats variable can go as
> well now as it is only used in the now removed code.
Odd 0-day never gave me a build warning for it, sorry about that.
>
> > @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (IS_ERR_OR_NULL(lp->debugfs_root))
> > - return PTR_ERR_OR_ZERO(lp->debugfs_root);
> >
> > - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> > - lp->debugfs_root,
> > - adf7242_stats_show);
> > - return PTR_ERR_OR_ZERO(stats);
> > -
> > - return 0;
> > + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> > + adf7242_stats_show);
> > }
> >
> > static const s32 adf7242_powers[] = {
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index 595cf7e2a651..7d67f41387f5 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> > }
> > DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
> >
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> > - struct dentry *stats;
> >
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (!at86rf230_debugfs_root)
> > - return -ENOMEM;
> > -
> > - stats = debugfs_create_file("trac_stats", 0444,
> > - at86rf230_debugfs_root, lp,
> > - &at86rf230_stats_fops);
> > - if (!stats)
> > - return -ENOMEM;
> >
> > - return 0;
> > + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> > + &at86rf230_stats_fops);
> > }
> >
> > static void at86rf230_debugfs_remove(void)
> > @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> > debugfs_remove_recursive(at86rf230_debugfs_root);
> > }
> > #else
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> > static void at86rf230_debugfs_remove(void) { }
> > #endif
> >
> > @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> > /* going into sleep by default */
> > at86rf230_sleep(lp);
> >
> > - rc = at86rf230_debugfs_init(lp);
> > - if (rc)
> > - goto free_dev;
> > + at86rf230_debugfs_init(lp);
> >
> > rc = ieee802154_register_hw(lp->hw);
> > if (rc)
> > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> > index b188fce3f641..11402dc347db 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> > priv,
> > &test_int_fops
> > );
> > - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> > - dev_err(
> > - &priv->spi->dev,
> > - "Error %ld when creating debugfs node\n",
> > - PTR_ERR(test->ca8210_dfs_spi_int)
> > - );
> > - return PTR_ERR(test->ca8210_dfs_spi_int);
> > - }
> > +
> > debugfs_create_symlink("ca8210", NULL, node_name);
> > init_waitqueue_head(&test->readq);
> > return kfifo_alloc(
> >
>
> With a fix for the above included you can have my
>
>
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
>
> for version 2.
I'll fix it up and resend tomorrow, thanks!
greg k-h
^ permalink raw reply
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
From: gregkh @ 2019-08-06 19:47 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: netdev@vger.kernel.org, leon@kernel.org
In-Reply-To: <d681be03ea2c1997004c8144c3a6062f895817a4.camel@mellanox.com>
On Tue, Aug 06, 2019 at 07:41:57PM +0000, Saeed Mahameed wrote:
> On Tue, 2019-08-06 at 18:11 +0200, Greg Kroah-Hartman wrote:
> > 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
> > files, making all of this much simpler and easier to understand as we
> > don't need to keep the dentries saved anymore.
> >
>
> Hi Greg,
>
> Basically i am ok with this patch and i like it very much.., but i am
> concerned about some of the driver internal flows that are dependent on
> these debug fs entries being valid.
That's never good, that's a bug in the driver :)
> for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
> know it is wrong even before your patch.. but maybe we should deal with
> it now ? or let me know if you want me to follow up with my own patch.
Your own patch would be good as I do not know this part of the codebase
at all, thanks.
> All we need to improve in this patch is to void out add_res_tree()
> implemented in
> drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
> as i will comment below.
>
>
> > Cc: Saeed Mahameed <saeedm@mellanox.com>
> > Cc: Leon Romanovsky <leon@kernel.org>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > 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 +-
> > include/linux/mlx5/driver.h | 12 +--
> > 7 files changed, 24 insertions(+), 163 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > index 8cdd7e66f8df..973f90888b1f 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> > mlx5_core_dev *dev)
> > debugfs_remove_recursive(dbg->dbg_root);
> > }
> >
>
> [...]
>
> > void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> > {
> > - if (!mlx5_debugfs_root)
> > - return;
> > -
> > debugfs_remove_recursive(dev->priv.cq_debugfs);
> > }
> >
> > @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
>
> Basically this function is a debugfs wrapper that should behave the
> same as debug_fs_*, we should fix it to return void as well, and
> improve all the its callers to ignore the return value.
But mlx5_cq_debugfs_cleanup() is a void function.
> callers are:
> mlx5_debug_qp_add()
> mlx5_debug_eq_add()
> mlx5_debug_cq_add()
Ah, you mean add_res_tree(). Yes, make that void as well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
From: Saeed Mahameed @ 2019-08-06 19:41 UTC (permalink / raw)
To: gregkh@linuxfoundation.org, netdev@vger.kernel.org; +Cc: leon@kernel.org
In-Reply-To: <20190806161128.31232-4-gregkh@linuxfoundation.org>
On Tue, 2019-08-06 at 18:11 +0200, Greg Kroah-Hartman wrote:
> 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
> files, making all of this much simpler and easier to understand as we
> don't need to keep the dentries saved anymore.
>
Hi Greg,
Basically i am ok with this patch and i like it very much.., but i am
concerned about some of the driver internal flows that are dependent on
these debug fs entries being valid.
for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
know it is wrong even before your patch.. but maybe we should deal with
it now ? or let me know if you want me to follow up with my own patch.
All we need to improve in this patch is to void out add_res_tree()
implemented in
drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
as i will comment below.
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 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 +-
> include/linux/mlx5/driver.h | 12 +--
> 7 files changed, 24 insertions(+), 163 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> index 8cdd7e66f8df..973f90888b1f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> mlx5_core_dev *dev)
> debugfs_remove_recursive(dbg->dbg_root);
> }
>
[...]
> void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> {
> - if (!mlx5_debugfs_root)
> - return;
> -
> debugfs_remove_recursive(dev->priv.cq_debugfs);
> }
>
> @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
Basically this function is a debugfs wrapper that should behave the
same as debug_fs_*, we should fix it to return void as well, and
improve all the its callers to ignore the return value.
callers are:
mlx5_debug_qp_add()
mlx5_debug_eq_add()
mlx5_debug_cq_add()
> *dev, enum dbg_rsc_type type,
> {
> struct mlx5_rsc_debug *d;
> char resn[32];
> - int err;
> int i;
>
> d = kzalloc(struct_size(d, fields, nfile), GFP_KERNEL);
> @@ -496,30 +429,15 @@ static int add_res_tree(struct mlx5_core_dev
> *dev, enum dbg_rsc_type type,
> d->type = type;
> sprintf(resn, "0x%x", rsn);
> d->root = debugfs_create_dir(resn, root);
> - if (!d->root) {
> - err = -ENOMEM;
> - goto out_free;
> - }
>
> for (i = 0; i < nfile; i++) {
> d->fields[i].i = i;
> - d->fields[i].dent = debugfs_create_file(field[i], 0400,
> - d->root, &d-
> >fields[i],
> - &fops);
> - if (!d->fields[i].dent) {
> - err = -ENOMEM;
> - goto out_rem;
> - }
> + debugfs_create_file(field[i], 0400, d->root, &d-
> >fields[i],
> + &fops);
> }
> *dbg = d;
>
> return 0;
> -out_rem:
> - debugfs_remove_recursive(d->root);
> -
> -out_free:
> - kfree(d);
> - return err;
> }
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: stmmac: Improvements for -next
From: Jakub Kicinski @ 2019-08-06 19:40 UTC (permalink / raw)
To: Jose Abreu
Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <cover.1565098881.git.joabreu@synopsys.com>
On Tue, 6 Aug 2019 15:42:41 +0200, Jose Abreu wrote:
> Couple of improvements for -next tree. More info in commit logs.
Code looks good to me now, thanks!
^ permalink raw reply
* Re: [PATCH net-next 0/6] drop_monitor: Various improvements and cleanups
From: David Miller @ 2019-08-06 19:38 UTC (permalink / raw)
To: idosch; +Cc: netdev, nhorman, toke, jiri, dsahern, mlxsw, idosch
In-Reply-To: <20190806131956.26168-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Tue, 6 Aug 2019 16:19:50 +0300
> From: Ido Schimmel <idosch@mellanox.com>
>
> This patchset performs various improvements and cleanups in drop monitor
> with no functional changes intended. There are no changes in these
> patches relative to the RFC I sent two weeks ago [1].
>
> A followup patchset will extend drop monitor with a packet alert mode in
> which the dropped packet is notified to user space instead of just a
> summary of recent drops. Subsequent patchsets will add the ability to
> monitor hardware originated drops via drop monitor.
>
> [1] https://patchwork.ozlabs.org/cover/1135226/
These all look fine to me, series applied.
Thanks.
^ permalink raw reply
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
From: Stefan Schmidt @ 2019-08-06 19:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, netdev
Cc: Michael Hennerich, Alexander Aring, David S. Miller, Harry Morris,
linux-wpan
In-Reply-To: <20190806161128.31232-18-gregkh@linuxfoundation.org>
Hello.
On 06.08.19 18:11, Greg Kroah-Hartman wrote:
> 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: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Harry Morris <h.morris@cascoda.com>
> Cc: linux-wpan@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ieee802154/adf7242.c | 12 +++---------
> drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> drivers/net/ieee802154/ca8210.c | 9 +--------
> 3 files changed, 9 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index c9392d70e639..7b95add2235a 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> return 0;
> }
>
> -static int adf7242_debugfs_init(struct adf7242_local *lp)
> +static void adf7242_debugfs_init(struct adf7242_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> struct dentry *stats;
A quick look over the code indicates that the stats variable can go as
well now as it is only used in the now removed code.
> @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (IS_ERR_OR_NULL(lp->debugfs_root))
> - return PTR_ERR_OR_ZERO(lp->debugfs_root);
>
> - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> - lp->debugfs_root,
> - adf7242_stats_show);
> - return PTR_ERR_OR_ZERO(stats);
> -
> - return 0;
> + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> + adf7242_stats_show);
> }
>
> static const s32 adf7242_powers[] = {
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index 595cf7e2a651..7d67f41387f5 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> }
> DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
>
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> - struct dentry *stats;
>
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (!at86rf230_debugfs_root)
> - return -ENOMEM;
> -
> - stats = debugfs_create_file("trac_stats", 0444,
> - at86rf230_debugfs_root, lp,
> - &at86rf230_stats_fops);
> - if (!stats)
> - return -ENOMEM;
>
> - return 0;
> + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> + &at86rf230_stats_fops);
> }
>
> static void at86rf230_debugfs_remove(void)
> @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> debugfs_remove_recursive(at86rf230_debugfs_root);
> }
> #else
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> static void at86rf230_debugfs_remove(void) { }
> #endif
>
> @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> /* going into sleep by default */
> at86rf230_sleep(lp);
>
> - rc = at86rf230_debugfs_init(lp);
> - if (rc)
> - goto free_dev;
> + at86rf230_debugfs_init(lp);
>
> rc = ieee802154_register_hw(lp->hw);
> if (rc)
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index b188fce3f641..11402dc347db 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> priv,
> &test_int_fops
> );
> - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> - dev_err(
> - &priv->spi->dev,
> - "Error %ld when creating debugfs node\n",
> - PTR_ERR(test->ca8210_dfs_spi_int)
> - );
> - return PTR_ERR(test->ca8210_dfs_spi_int);
> - }
> +
> debugfs_create_symlink("ca8210", NULL, node_name);
> init_waitqueue_head(&test->readq);
> return kfifo_alloc(
>
With a fix for the above included you can have my
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
for version 2.
regards
Stefan Schmidt
^ permalink raw reply
* Re: [PATCH net 0/3] net: stmmac: Fixes for -net
From: David Miller @ 2019-08-06 19:26 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1565097294.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Tue, 6 Aug 2019 15:16:15 +0200
> Couple of fixes for -net. More info in commit log.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 1/1] qed: Add new ethtool supported port types based on media.
From: Jakub Kicinski @ 2019-08-06 19:14 UTC (permalink / raw)
To: Rahul Verma; +Cc: davem, netdev, aelior, mkalderon
In-Reply-To: <20190806065950.19073-1-rahulv@marvell.com>
On Mon, 5 Aug 2019 23:59:50 -0700, Rahul Verma wrote:
> Supported ports in ethtool <eth1> are displayed based on media type.
> For media type fibre and twinaxial, port type is "FIBRE". Media type
> Base-T is "TP" and media KR is "Backplane".
>
> V1->V2:
> Corrected the subject.
>
> Signed-off-by: Rahul Verma <rahulv@marvell.com>
> Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>
LGTM
^ permalink raw reply
* [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries
From: David Ahern @ 2019-08-06 19:15 UTC (permalink / raw)
To: davem; +Cc: netdev, jiri, David Ahern
From: David Ahern <dsahern@gmail.com>
Prior to the commit in the fixes tag, the resource controller in netdevsim
tracked fib entries and rules per network namespace. Restore that behavior.
Fixes: 5fc494225c1e ("netdevsim: create devlink instance per netdevsim instance")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/netdevsim/dev.c | 63 ++++++++++-------------
drivers/net/netdevsim/fib.c | 102 +++++++++++++++++++++++---------------
drivers/net/netdevsim/netdev.c | 9 +++-
drivers/net/netdevsim/netdevsim.h | 10 ++--
4 files changed, 98 insertions(+), 86 deletions(-)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index c5c417a3c0ce..bcc40a236624 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -73,46 +73,47 @@ static void nsim_dev_port_debugfs_exit(struct nsim_dev_port *nsim_dev_port)
debugfs_remove_recursive(nsim_dev_port->ddir);
}
+static struct net *nsim_devlink_net(struct devlink *devlink)
+{
+ return &init_net;
+}
+
static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, false);
}
static u64 nsim_dev_ipv4_fib_rules_res_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB_RULES, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, false);
}
static u64 nsim_dev_ipv6_fib_resource_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, false);
}
static u64 nsim_dev_ipv6_fib_rules_res_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB_RULES, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, false);
}
static int nsim_dev_resources_register(struct devlink *devlink)
{
- struct nsim_dev *nsim_dev = devlink_priv(devlink);
struct devlink_resource_size_params params = {
.size_max = (u64)-1,
.size_granularity = 1,
.unit = DEVLINK_RESOURCE_UNIT_ENTRY
};
+ struct net *net = nsim_devlink_net(devlink);
int err;
u64 n;
@@ -126,8 +127,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
goto out;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV4_FIB,
NSIM_RESOURCE_IPV4, ¶ms);
@@ -136,8 +136,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
return err;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB_RULES, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV4_FIB_RULES,
NSIM_RESOURCE_IPV4, ¶ms);
@@ -156,8 +155,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
goto out;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV6_FIB,
NSIM_RESOURCE_IPV6, ¶ms);
@@ -166,8 +164,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
return err;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB_RULES, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV6_FIB_RULES,
NSIM_RESOURCE_IPV6, ¶ms);
@@ -179,19 +176,19 @@ static int nsim_dev_resources_register(struct devlink *devlink)
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV4_FIB,
nsim_dev_ipv4_fib_resource_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV4_FIB_RULES,
nsim_dev_ipv4_fib_rules_res_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV6_FIB,
nsim_dev_ipv6_fib_resource_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV6_FIB_RULES,
nsim_dev_ipv6_fib_rules_res_occ_get,
- nsim_dev);
+ net);
out:
return err;
}
@@ -199,11 +196,11 @@ static int nsim_dev_resources_register(struct devlink *devlink)
static int nsim_dev_reload(struct devlink *devlink,
struct netlink_ext_ack *extack)
{
- struct nsim_dev *nsim_dev = devlink_priv(devlink);
enum nsim_resource_id res_ids[] = {
NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
};
+ struct net *net = nsim_devlink_net(devlink);
int i;
for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
@@ -212,8 +209,7 @@ static int nsim_dev_reload(struct devlink *devlink,
err = devlink_resource_size_get(devlink, res_ids[i], &val);
if (!err) {
- err = nsim_fib_set_max(nsim_dev->fib_data,
- res_ids[i], val, extack);
+ err = nsim_fib_set_max(net, res_ids[i], val, extack);
if (err)
return err;
}
@@ -285,15 +281,9 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
mutex_init(&nsim_dev->port_list_lock);
nsim_dev->fw_update_status = true;
- nsim_dev->fib_data = nsim_fib_create();
- if (IS_ERR(nsim_dev->fib_data)) {
- err = PTR_ERR(nsim_dev->fib_data);
- goto err_devlink_free;
- }
-
err = nsim_dev_resources_register(devlink);
if (err)
- goto err_fib_destroy;
+ goto err_devlink_free;
err = devlink_register(devlink, &nsim_bus_dev->dev);
if (err)
@@ -315,8 +305,6 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
devlink_unregister(devlink);
err_resources_unregister:
devlink_resources_unregister(devlink, NULL);
-err_fib_destroy:
- nsim_fib_destroy(nsim_dev->fib_data);
err_devlink_free:
devlink_free(devlink);
return ERR_PTR(err);
@@ -330,7 +318,6 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
nsim_dev_debugfs_exit(nsim_dev);
devlink_unregister(devlink);
devlink_resources_unregister(devlink, NULL);
- nsim_fib_destroy(nsim_dev->fib_data);
mutex_destroy(&nsim_dev->port_list_lock);
devlink_free(devlink);
}
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index 8c57ba747772..f61d094746c0 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -18,6 +18,7 @@
#include <net/ip_fib.h>
#include <net/ip6_fib.h>
#include <net/fib_rules.h>
+#include <net/netns/generic.h>
#include "netdevsim.h"
@@ -32,14 +33,15 @@ struct nsim_per_fib_data {
};
struct nsim_fib_data {
- struct notifier_block fib_nb;
struct nsim_per_fib_data ipv4;
struct nsim_per_fib_data ipv6;
};
-u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, bool max)
+static unsigned int nsim_fib_net_id;
+
+u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
{
+ struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
struct nsim_fib_entry *entry;
switch (res_id) {
@@ -62,10 +64,10 @@ u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
return max ? entry->max : entry->num;
}
-int nsim_fib_set_max(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, u64 val,
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
struct netlink_ext_ack *extack)
{
+ struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
struct nsim_fib_entry *entry;
int err = 0;
@@ -118,9 +120,9 @@ static int nsim_fib_rule_account(struct nsim_fib_entry *entry, bool add,
return err;
}
-static int nsim_fib_rule_event(struct nsim_fib_data *data,
- struct fib_notifier_info *info, bool add)
+static int nsim_fib_rule_event(struct fib_notifier_info *info, bool add)
{
+ struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
struct netlink_ext_ack *extack = info->extack;
int err = 0;
@@ -155,9 +157,9 @@ static int nsim_fib_account(struct nsim_fib_entry *entry, bool add,
return err;
}
-static int nsim_fib_event(struct nsim_fib_data *data,
- struct fib_notifier_info *info, bool add)
+static int nsim_fib_event(struct fib_notifier_info *info, bool add)
{
+ struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
struct netlink_ext_ack *extack = info->extack;
int err = 0;
@@ -176,22 +178,18 @@ static int nsim_fib_event(struct nsim_fib_data *data,
static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
void *ptr)
{
- struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
- fib_nb);
struct fib_notifier_info *info = ptr;
int err = 0;
switch (event) {
case FIB_EVENT_RULE_ADD: /* fall through */
case FIB_EVENT_RULE_DEL:
- err = nsim_fib_rule_event(data, info,
- event == FIB_EVENT_RULE_ADD);
+ err = nsim_fib_rule_event(info, event == FIB_EVENT_RULE_ADD);
break;
case FIB_EVENT_ENTRY_ADD: /* fall through */
case FIB_EVENT_ENTRY_DEL:
- err = nsim_fib_event(data, info,
- event == FIB_EVENT_ENTRY_ADD);
+ err = nsim_fib_event(info, event == FIB_EVENT_ENTRY_ADD);
break;
}
@@ -201,23 +199,30 @@ static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
/* inconsistent dump, trying again */
static void nsim_fib_dump_inconsistent(struct notifier_block *nb)
{
- struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
- fib_nb);
+ struct nsim_fib_data *data;
+ struct net *net;
+
+ rcu_read_lock();
+ for_each_net_rcu(net) {
+ data = net_generic(net, nsim_fib_net_id);
+
+ data->ipv4.fib.num = 0ULL;
+ data->ipv4.rules.num = 0ULL;
- data->ipv4.fib.num = 0ULL;
- data->ipv4.rules.num = 0ULL;
- data->ipv6.fib.num = 0ULL;
- data->ipv6.rules.num = 0ULL;
+ data->ipv6.fib.num = 0ULL;
+ data->ipv6.rules.num = 0ULL;
+ }
+ rcu_read_unlock();
}
-struct nsim_fib_data *nsim_fib_create(void)
-{
- struct nsim_fib_data *data;
- int err;
+static struct notifier_block nsim_fib_nb = {
+ .notifier_call = nsim_fib_event_nb,
+};
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return ERR_PTR(-ENOMEM);
+/* Initialize per network namespace state */
+static int __net_init nsim_fib_netns_init(struct net *net)
+{
+ struct nsim_fib_data *data = net_generic(net, nsim_fib_net_id);
data->ipv4.fib.max = (u64)-1;
data->ipv4.rules.max = (u64)-1;
@@ -225,22 +230,37 @@ struct nsim_fib_data *nsim_fib_create(void)
data->ipv6.fib.max = (u64)-1;
data->ipv6.rules.max = (u64)-1;
- data->fib_nb.notifier_call = nsim_fib_event_nb;
- err = register_fib_notifier(&data->fib_nb, nsim_fib_dump_inconsistent);
- if (err) {
- pr_err("Failed to register fib notifier\n");
- goto err_out;
- }
+ return 0;
+}
- return data;
+static struct pernet_operations nsim_fib_net_ops = {
+ .init = nsim_fib_netns_init,
+ .id = &nsim_fib_net_id,
+ .size = sizeof(struct nsim_fib_data),
+};
-err_out:
- kfree(data);
- return ERR_PTR(err);
+void nsim_fib_exit(void)
+{
+ unregister_pernet_subsys(&nsim_fib_net_ops);
+ unregister_fib_notifier(&nsim_fib_nb);
}
-void nsim_fib_destroy(struct nsim_fib_data *data)
+int nsim_fib_init(void)
{
- unregister_fib_notifier(&data->fib_nb);
- kfree(data);
+ int err;
+
+ err = register_pernet_subsys(&nsim_fib_net_ops);
+ if (err < 0) {
+ pr_err("Failed to register pernet subsystem\n");
+ goto err_out;
+ }
+
+ err = register_fib_notifier(&nsim_fib_nb, nsim_fib_dump_inconsistent);
+ if (err < 0) {
+ pr_err("Failed to register fib notifier\n");
+ goto err_out;
+ }
+
+err_out:
+ return err;
}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 0740940f41b1..55f57f76d01b 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -357,12 +357,18 @@ static int __init nsim_module_init(void)
if (err)
goto err_dev_exit;
- err = rtnl_link_register(&nsim_link_ops);
+ err = nsim_fib_init();
if (err)
goto err_bus_exit;
+ err = rtnl_link_register(&nsim_link_ops);
+ if (err)
+ goto err_fib_exit;
+
return 0;
+err_fib_exit:
+ nsim_fib_exit();
err_bus_exit:
nsim_bus_exit();
err_dev_exit:
@@ -373,6 +379,7 @@ static int __init nsim_module_init(void)
static void __exit nsim_module_exit(void)
{
rtnl_link_unregister(&nsim_link_ops);
+ nsim_fib_exit();
nsim_bus_exit();
nsim_dev_exit();
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 79c05af2a7c0..9404637d34b7 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -169,12 +169,10 @@ int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev,
int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
unsigned int port_index);
-struct nsim_fib_data *nsim_fib_create(void);
-void nsim_fib_destroy(struct nsim_fib_data *fib_data);
-u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, bool max);
-int nsim_fib_set_max(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, u64 val,
+int nsim_fib_init(void);
+void nsim_fib_exit(void);
+u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max);
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
struct netlink_ext_ack *extack);
#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] hv_netvsc: Fix a warning of suspicious RCU usage
From: Jakub Kicinski @ 2019-08-06 19:12 UTC (permalink / raw)
To: Dexuan Cui
Cc: netdev@vger.kernel.org, David S. Miller, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, KY Srinivasan,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com
In-Reply-To: <PU1P153MB0169AECABF6094A3E7BEE381BFD50@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
On Tue, 6 Aug 2019 05:17:44 +0000, Dexuan Cui wrote:
> This fixes a warning of "suspicious rcu_dereference_check() usage"
> when nload runs.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Minor change in behaviour would perhaps be worth acknowledging in the
commit message (since you check ndev for NULL later now), and a Fixes
tag would be good.
But the looks pretty straightforward and correct!
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: net: meson-dwmac: convert to yaml
From: Martin Blumenstingl @ 2019-08-06 19:11 UTC (permalink / raw)
To: Neil Armstrong
Cc: robh+dt, devicetree, netdev, linux-amlogic, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190806125041.16105-3-narmstrong@baylibre.com>
On Tue, Aug 6, 2019 at 2:50 PM 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>
thank you for taking care of this conversion!
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
[...]
> + amlogic,tx-delay-ns:
> + $ref: /schemas/types.yaml#definitions/uint32
> + description:
> + The internal RGMII TX clock delay (provided by this driver) in
> + nanoseconds. Allowed values are 0ns, 2ns, 4ns, 6ns.
once I have more time I will try to see whether we can define an enum
with these values, then invalid values will yield a warning/error when
building the .dtb (which seems to be a good idea)
this comment shouldn't prevent this patch from being applied as the
initial conversion will already make life a lot easier
Martin
^ permalink raw reply
* Re: [RFC] implicit per-namespace devlink instance to set kernel resource limitations
From: Andrew Lunn @ 2019-08-06 19:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jiri Pirko, dsahern, netdev, davem, mlxsw, f.fainelli,
vivien.didelot, mkubecek, stephen, daniel, brouer, eric.dumazet
In-Reply-To: <20190806115449.5b3a9d97@cakuba.netronome.com>
On Tue, Aug 06, 2019 at 11:54:49AM -0700, Jakub Kicinski wrote:
> On Tue, 6 Aug 2019 20:38:41 +0200, Jiri Pirko wrote:
> > >> So the proposal is to have some new device, say "kernelnet", that
> > >> would implicitly create per-namespace devlink instance. This devlink
> > >> instance would be used to setup resource limits. Like:
> > >>
> > >> devlink resource set kernelnet path /IPv4/fib size 96
> > >> devlink -N ns1name resource set kernelnet path /IPv6/fib size 100
> > >> devlink -N ns2name resource set kernelnet path /IPv4/fib-rules size 8
> > >>
> > >> To me it sounds a bit odd for kernel namespace to act as a device, but
> > >> thinking about it more, it makes sense. Probably better than to define
> > >> a new api. User would use the same tool to work with kernel and hw.
> > >>
> > >> Also we can implement other devlink functionality, like dpipe.
> > >> User would then have visibility of network pipeline, tables,
> > >> utilization, etc. It is related to the resources too.
> > >>
> > >> What do you think?
> > >
> > >I'm no expert here but seems counter intuitive that device tables would
> > >be aware of namespaces in the first place. Are we not reinventing
> > >cgroup controllers based on a device API? IMHO from a perspective of
> > >someone unfamiliar with routing offload this seems backwards :)
> >
> > Can we use cgroup for fib and other limitations instead?
>
> Not sure the question is to me, I don't feel particularly qualified,
> I've never worked with VDCs or wrote a switch driver.. But I'd see
> cgroups as a natural fit, and if I read Andrew's reply right so does
> he..
Hi Jakub
I think there needs to be a clearly reasoned argument why cgroups is
the wrong answer to this problem. I myself don't know enough to give
that answer, but i can pose the question.
Andrew
^ permalink raw reply
* Re: [PATCH net-next] mlx5: use correct counter
From: Saeed Mahameed @ 2019-08-06 19:03 UTC (permalink / raw)
To: jonathan.lemon@gmail.com; +Cc: kernel-team@fb.com, netdev@vger.kernel.org
In-Reply-To: <20190806182819.788750-1-jonathan.lemon@gmail.com>
On Tue, 2019-08-06 at 11:28 -0700, Jonathan Lemon wrote:
> mlx5e_grp_q_update_stats seems to be using the wrong counter
> for if_down_packets.
>
> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> index 6eee3c7d4b06..1d16e03a987d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> @@ -363,7 +363,7 @@ static void mlx5e_grp_q_update_stats(struct
> mlx5e_priv *priv)
> !mlx5_core_query_q_counter(priv->mdev, priv-
> >drop_rq_q_counter, 0,
> out, sizeof(out)))
> qcnt->rx_if_down_packets =
> MLX5_GET(query_q_counter_out, out,
> - out_of_buffer);
> + rx_if_down_packets)
> ;
Hi Jonathan,
This patch in not applicable (won't compile and there is no issue with
current code).
Although it is confusing but the code is correct as is.
1) your patch won't compile since there is no rx_if_down_packets field
in query_q_counter_out hw definition struct: please check
include/linux/mlx5/mlx5_ifc.h
mlx5_ifc_query_q_counter_out_bits
2) the code works as is since when interface is down and port is up,
technically from hw perspective there is "no buffer available" so the
out_of_buffer counter of the drop_rq_q_counter will count packets
dropped due to interface down..
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH 3/3] net: dsa: ksz: Drop NET_DSA_TAG_KSZ9477
From: David Miller @ 2019-08-06 18:59 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, vivien.didelot,
woojung.huh
In-Reply-To: <20190806130609.29686-3-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Tue, 6 Aug 2019 15:06:09 +0200
> This Kconfig option is unused, drop it.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3] net: dsa: ksz: Merge ksz_priv.h into ksz_common.h
From: David Miller @ 2019-08-06 18:59 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, vivien.didelot,
woojung.huh
In-Reply-To: <20190806130609.29686-2-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Tue, 6 Aug 2019 15:06:08 +0200
> Merge the two headers into one, no functional change.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Applied.
^ permalink raw reply
* Re: [PATCH 1/3] net: dsa: ksz: Remove dead code and fix warnings
From: David Miller @ 2019-08-06 18:59 UTC (permalink / raw)
To: marex; +Cc: netdev, andrew, f.fainelli, Tristram.Ha, vivien.didelot,
woojung.huh
In-Reply-To: <20190806130609.29686-1-marex@denx.de>
From: Marek Vasut <marex@denx.de>
Date: Tue, 6 Aug 2019 15:06:07 +0200
> Remove ksz_port_cleanup(), which is unused. Add missing include
> "ksz_common.h", which fixes the following warning when built with
> make ... W=1
>
> drivers/net/dsa/microchip/ksz_common.c:23:6: warning: no previous prototype for ‘...’ [-Wmissing-prototypes]
>
> Note that the order of the headers cannot be swapped, as that would
> trigger missing forward declaration errors, which would indicate the
> way forward is to merge the two headers into one.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Applied.
^ permalink raw reply
* Re: [RFC] implicit per-namespace devlink instance to set kernel resource limitations
From: Jakub Kicinski @ 2019-08-06 18:54 UTC (permalink / raw)
To: Jiri Pirko
Cc: dsahern, netdev, davem, mlxsw, andrew, f.fainelli, vivien.didelot,
mkubecek, stephen, daniel, brouer, eric.dumazet
In-Reply-To: <20190806183841.GD2332@nanopsycho.orion>
On Tue, 6 Aug 2019 20:38:41 +0200, Jiri Pirko wrote:
> >> So the proposal is to have some new device, say "kernelnet", that
> >> would implicitly create per-namespace devlink instance. This devlink
> >> instance would be used to setup resource limits. Like:
> >>
> >> devlink resource set kernelnet path /IPv4/fib size 96
> >> devlink -N ns1name resource set kernelnet path /IPv6/fib size 100
> >> devlink -N ns2name resource set kernelnet path /IPv4/fib-rules size 8
> >>
> >> To me it sounds a bit odd for kernel namespace to act as a device, but
> >> thinking about it more, it makes sense. Probably better than to define
> >> a new api. User would use the same tool to work with kernel and hw.
> >>
> >> Also we can implement other devlink functionality, like dpipe.
> >> User would then have visibility of network pipeline, tables,
> >> utilization, etc. It is related to the resources too.
> >>
> >> What do you think?
> >
> >I'm no expert here but seems counter intuitive that device tables would
> >be aware of namespaces in the first place. Are we not reinventing
> >cgroup controllers based on a device API? IMHO from a perspective of
> >someone unfamiliar with routing offload this seems backwards :)
>
> Can we use cgroup for fib and other limitations instead?
Not sure the question is to me, I don't feel particularly qualified,
I've never worked with VDCs or wrote a switch driver.. But I'd see
cgroups as a natural fit, and if I read Andrew's reply right so does
he.. There's certainly a feeling of reinventing the wheel here.
We usually model things in software and then compile that abstraction
into device terms. Devlink allows for low level access to the device,
it allows us to, in a sense, see the result of that compilation. But
that's more of a debugging/low level knob than first class citizen :(
^ permalink raw reply
* Re: [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Saeed Mahameed @ 2019-08-06 18:54 UTC (permalink / raw)
To: jgg@ziepe.ca, leon@kernel.org
Cc: linux-kernel@vger.kernel.org, davem@davemloft.net,
hslester96@gmail.com, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <20190806183821.GR11627@ziepe.ca>
On Tue, 2019-08-06 at 15:38 -0300, Jason Gunthorpe wrote:
> On Tue, Aug 06, 2019 at 09:59:58AM +0300, Leon Romanovsky wrote:
> > On Mon, Aug 05, 2019 at 08:06:36PM +0000, Saeed Mahameed wrote:
> > > On Mon, 2019-08-05 at 14:55 +0800, Chuhong Yuan wrote:
> > > > On Mon, Aug 5, 2019 at 2:13 PM Leon Romanovsky <leon@kernel.org
> > > > >
> > > > wrote:
> > > > > On Sun, Aug 04, 2019 at 10:44:47PM +0800, Chuhong Yuan wrote:
> > > > > > On Sun, Aug 4, 2019 at 8:59 PM Leon Romanovsky <
> > > > > > leon@kernel.org>
> > > > > > wrote:
> > > > > > > On Sat, Aug 03, 2019 at 12:48:28AM +0800, Chuhong Yuan
> > > > > > > wrote:
> > > > > > > > refcount_t is better for reference counters since its
> > > > > > > > implementation can prevent overflows.
> > > > > > > > So convert atomic_t ref counters to refcount_t.
> > > > > > >
> > > > > > > I'm not thrilled to see those automatic conversion
> > > > > > > patches,
> > > > > > > especially
> > > > > > > for flows which can't overflow. There is nothing wrong in
> > > > > > > using
> > > > > > > atomic_t
> > > > > > > type of variable, do you have in mind flow which will
> > > > > > > cause to
> > > > > > > overflow?
> > > > > > >
> > > > > > > Thanks
> > > > > >
> > > > > > I have to say that these patches are not done
> > > > > > automatically...
> > > > > > Only the detection of problems is done by a script.
> > > > > > All conversions are done manually.
> > > > >
> > > > > Even worse, you need to audit usage of atomic_t and replace
> > > > > there
> > > > > it can overflow.
> > > > >
> > > > > > I am not sure whether the flow can cause an overflow.
> > > > >
> > > > > It can't.
> > > > >
> > > > > > But I think it is hard to ensure that a data path is
> > > > > > impossible
> > > > > > to have problems in any cases including being attacked.
> > > > >
> > > > > It is not data path, and I doubt that such conversion will be
> > > > > allowed
> > > > > in data paths without proving that no performance regression
> > > > > is
> > > > > introduced.
> > > > > > So I think it is better to do this minor revision to
> > > > > > prevent
> > > > > > potential risk, just like we have done in mlx5/core/cq.c.
> > > > >
> > > > > mlx5/core/cq.c is a different beast, refcount there means
> > > > > actual
> > > > > users
> > > > > of CQ which are limited in SW, so in theory, they have
> > > > > potential
> > > > > to be overflown.
> > > > >
> > > > > It is not the case here, there your are adding new port.
> > > > > There is nothing wrong with atomic_t.
> > > > >
> > > >
> > > > Thanks for your explanation!
> > > > I will pay attention to this point in similar cases.
> > > > But it seems that the semantic of refcount is not always as
> > > > clear as
> > > > here...
> > > >
> > >
> > > Semantically speaking, there is nothing wrong with moving to
> > > refcount_t
> > > in the case of vxlan ports.. it also seems more accurate and will
> > > provide the type protection, even if it is not necessary. Please
> > > let me
> > > know what is the verdict here, i can apply this patch to net-
> > > next-mlx5.
> >
> > There is no verdict here, it is up to you., if you like code churn,
> > go
> > for it.
>
> IMHO CONFIG_REFCOUNT_FULL is a valuable enough reason to not open
> code
> with an atomic even if overflow is not possible.
>
> Races resulting in incrs on 0 refcounts is a common enough mistake.
>
> Jason
Indeed, thanks Jason, I will take this patch to net-next-mlx5.
^ permalink raw reply
* Re: pull-request: wireless-drivers 2019-08-06
From: David Miller @ 2019-08-06 18:49 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87h86ufs89.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Tue, 06 Aug 2019 14:28:22 +0300
> here's a pull request to net tree for v5.3, more information below.
> Please let me know if there are any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH v3 net-next] be2net: disable bh with spin_lock in be_process_mcc
From: David Miller @ 2019-08-06 18:47 UTC (permalink / raw)
To: kda; +Cc: sathya.perla, ajit.khaparde, sriharsha.basavapatna, netdev
In-Reply-To: <20190806105111.27058-1-dkirjanov@suse.com>
From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Tue, 6 Aug 2019 12:51:11 +0200
> be_process_mcc() is invoked in 3 different places and
> always with BHs disabled except the be_poll function
> but since it's invoked from softirq with BHs
> disabled it won't hurt.
>
> v1->v2: added explanation to the patch
> v2->v3: add a missing call from be_cmds.c
>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
Applied.
^ permalink raw reply
* Re: [PATCH] net/ipv4: reset mac head before call ip_tunnel_rcv()
From: David Miller @ 2019-08-06 18:43 UTC (permalink / raw)
To: ptpt52; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20190806104731.30603-1-ptpt52@gmail.com>
From: Chen Minqiang <ptpt52@gmail.com>
Date: Tue, 6 Aug 2019 18:47:31 +0800
> Signed-off-by: Chen Minqiang <ptpt52@gmail.com>
No commit message means I'm not even going to look at this patch and
try to understand it.
You must always completely explain, in detail, what change you are
making, how you are making it, and above all why you are making this
change.
Is there some bug you are fixing? What is that bug and what does that
bug cause to happen? Are there potential negative side effects to
your fix? What are they and what was the cost/benefit analysis for
that?
Where was the bug introduced? You must provide a proper Fixes: tag
which shows this.
Thank you.
^ permalink raw reply
* Re: [PATCH 0/2 net,v4] flow_offload hardware priority fixes
From: Jakub Kicinski @ 2019-08-06 18:41 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netfilter-devel, davem, netdev, marcelo.leitner, jiri, wenxu,
saeedm, paulb, gerlitz.or
In-Reply-To: <20190806160310.6663-1-pablo@netfilter.org>
On Tue, 6 Aug 2019 18:03:08 +0200, Pablo Neira Ayuso wrote:
> 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.
Seems reasonable, thanks.
^ permalink raw reply
* Re: [PATCH] net: sched: sch_taprio: fix memleak in error path for sched list parse
From: David Miller @ 2019-08-06 18:41 UTC (permalink / raw)
To: ivan.khoronzhuk
Cc: vinicius.gomes, jhs, xiyou.wangcong, jiri, netdev, linux-kernel
In-Reply-To: <20190806100425.4356-1-ivan.khoronzhuk@linaro.org>
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Tue, 6 Aug 2019 13:04:25 +0300
> Based on net/master
I wonder about that because:
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -1451,7 +1451,8 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
> spin_unlock_bh(qdisc_lock(sch));
>
> free_sched:
> - kfree(new_admin);
> + if (new_admin)
> + call_rcu(&new_admin->rcu, taprio_free_sched_cb);
>
> return err;
In my tree the context around line 1451 is:
nla_nest_end(skb, sched_nest);
done:
rcu_read_unlock();
return nla_nest_end(skb, nest);
which is part of function taprio_dump().
Please respin this properly against current 'net' sources.
^ permalink raw reply
* Re: [RFC] implicit per-namespace devlink instance to set kernel resource limitations
From: Jiri Pirko @ 2019-08-06 18:38 UTC (permalink / raw)
To: Jakub Kicinski
Cc: dsahern, netdev, davem, mlxsw, andrew, f.fainelli, vivien.didelot,
mkubecek, stephen, daniel, brouer, eric.dumazet
In-Reply-To: <20190806112717.3b070d07@cakuba.netronome.com>
Tue, Aug 06, 2019 at 08:27:17PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 6 Aug 2019 18:40:36 +0200, Jiri Pirko wrote:
>> Hi all.
>>
>> I just discussed this with DavidA and I would like to bring this to
>> broader audience. David wants to limit kernel resources in network
>> namespaces, for example fibs, fib rules, etc.
>>
>> He claims that devlink api is rich enough to program this limitations
>> as it already does for mlxsw hw resources for example.
>
>TBH I don't see how you changed anything to do with FIB notifications,
>so the fact that the accounting is off now is a bit confusing. I don't
>understand how devlink, FIB and namespaces mix :(
>
>> If we have this api for hardware, why don't to reuse it for the
>> kernel and it's resources too?
>
>IMHO the netdevsim use of this API is a slight abuse, to prove the
>device can fail the FIB changes, nothing more..
It's slightly bigger abuse :) But in this thread, we are not discussing
netdevsim, but separate "dev".
>
>> So the proposal is to have some new device, say "kernelnet", that
>> would implicitly create per-namespace devlink instance. This devlink
>> instance would be used to setup resource limits. Like:
>>
>> devlink resource set kernelnet path /IPv4/fib size 96
>> devlink -N ns1name resource set kernelnet path /IPv6/fib size 100
>> devlink -N ns2name resource set kernelnet path /IPv4/fib-rules size 8
>>
>> To me it sounds a bit odd for kernel namespace to act as a device, but
>> thinking about it more, it makes sense. Probably better than to define
>> a new api. User would use the same tool to work with kernel and hw.
>>
>> Also we can implement other devlink functionality, like dpipe.
>> User would then have visibility of network pipeline, tables,
>> utilization, etc. It is related to the resources too.
>>
>> What do you think?
>
>I'm no expert here but seems counter intuitive that device tables would
>be aware of namespaces in the first place. Are we not reinventing
>cgroup controllers based on a device API? IMHO from a perspective of
>someone unfamiliar with routing offload this seems backwards :)
Can we use cgroup for fib and other limitations instead?
^ permalink raw reply
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