* [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
* Re: [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Jason Gunthorpe @ 2019-08-06 18:38 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Saeed Mahameed, hslester96@gmail.com, davem@davemloft.net,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190806065958.GQ4832@mtr-leonro.mtl.com>
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
^ permalink raw reply
* Re: [PATCH 1/1] ixgbe: sync the first fragment unconditionally
From: David Miller @ 2019-08-06 18:36 UTC (permalink / raw)
To: firo.yang; +Cc: netdev, jeffrey.t.kirsher, intel-wired-lan, linux-kernel
In-Reply-To: <20190806092919.13211-1-firo.yang@suse.com>
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.
^ permalink raw reply
* Re: [PATCH] net/mlx4_core: Use refcount_t for refcount
From: Jason Gunthorpe @ 2019-08-06 18:36 UTC (permalink / raw)
To: Chuhong Yuan
Cc: Saeed Mahameed, linux-kernel@vger.kernel.org, davem@davemloft.net,
Tariq Toukan, linux-rdma@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <CANhBUQ1wZPinWicu2c_VZjpTtP_9+AxB=7zn+ymPyYVo_rsxZQ@mail.gmail.com>
On Sat, Aug 03, 2019 at 10:42:31AM +0800, Chuhong Yuan wrote:
> Saeed Mahameed <saeedm@mellanox.com> 于2019年8月3日周六 上午2:38写道：
> >
> > On Sat, 2019-08-03 at 00:10 +0800, Chuhong Yuan wrote:
> > > Chuhong Yuan <hslester96@gmail.com> 于2019年8月2日周五 下午8:10写道：
> > > > refcount_t is better for reference counters since its
> > > > implementation can prevent overflows.
> > > > So convert atomic_t ref counters to refcount_t.
> > > >
> > > > Also convert refcount from 0-based to 1-based.
> > > >
> > >
> > > It seems that directly converting refcount from 0-based
> > > to 1-based is infeasible.
> > > I am sorry for this mistake.
> >
> > Just curious, why not keep it 0 based and use refcout_t ?
> >
> > refcount API should have the same semantics as atomic_t API .. no ?
>
> refcount API will warn when increase a 0 refcount.
> It regards this as a use-after-free.
If this causes failures then the code is not doing atomic as a
refcount properly anyhow..
There are some cases where the atomic refcount is just a imprecise
debugging aide.
Jason
^ permalink raw reply
* Re: [PATCH] net: cxgb3_main: Fix a resource leak in a error path in 'init_one()'
From: David Miller @ 2019-08-06 18:34 UTC (permalink / raw)
To: christophe.jaillet; +Cc: vishal, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190806085512.11729-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Tue, 6 Aug 2019 10:55:12 +0200
> A call to 'kfree_skb()' is missing in the error handling path of
> 'init_one()'.
> This is already present in 'remove_one()' but is missing here.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Looks good, applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net: ethernet: sun4i-emac: Support phy-handle property for finding PHYs
From: David Miller @ 2019-08-06 18:29 UTC (permalink / raw)
To: wens; +Cc: mripard, wens, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20190806073539.32519-1-wens@kernel.org>
From: Chen-Yu Tsai <wens@kernel.org>
Date: Tue, 6 Aug 2019 15:35:39 +0800
> From: Chen-Yu Tsai <wens@csie.org>
>
> The sun4i-emac uses the "phy" property to find the PHY it's supposed to
> use. This property was deprecated in favor of "phy-handle" in commit
> 8c5b09447625 ("dt-bindings: net: sun4i-emac: Convert the binding to a
> schemas").
>
> Add support for this new property name, and fall back to the old one in
> case the device tree hasn't been updated.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Applied.
^ permalink raw reply
* [PATCH net-next] mlx5: use correct counter
From: Jonathan Lemon @ 2019-08-06 18:28 UTC (permalink / raw)
To: saeedm; +Cc: kernel-team, netdev
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);
}
#define VNIC_ENV_OFF(c) MLX5_BYTE_OFF(query_vnic_env_out, c)
--
2.17.1
^ permalink raw reply related
* Re: [RFC] implicit per-namespace devlink instance to set kernel resource limitations
From: Jakub Kicinski @ 2019-08-06 18:27 UTC (permalink / raw)
To: Jiri Pirko, dsahern
Cc: netdev, davem, mlxsw, andrew, f.fainelli, vivien.didelot,
mkubecek, stephen, daniel, brouer, eric.dumazet
In-Reply-To: <20190806164036.GA2332@nanopsycho.orion>
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..
> 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 :)
^ permalink raw reply
* Re: [patch net-next v2 1/3] net: devlink: allow to change namespaces
From: Jiri Pirko @ 2019-08-06 18:07 UTC (permalink / raw)
To: David Ahern; +Cc: Jakub Kicinski, netdev, davem, sthemmin, mlxsw
In-Reply-To: <fc6a7342-246c-2fe1-a7d1-c7be5bd0a3a3@gmail.com>
Tue, Aug 06, 2019 at 07:55:30PM CEST, dsahern@gmail.com wrote:
>On 8/6/19 11:53 AM, Jiri Pirko wrote:
>> Let's figure out the devlink-controlling-kernel-resources thread first.
>> What you describe here is exactly that.
>
>as I mentioned on the phone, any outcome of that thread will be in 5.4
>at best. Meanwhile this breakage in 5.2 and 5.3 needs to be fixed.
Why? netdevsim is a dummy device, the purpose of existence is to test
kernel api. No real harm breaking it.
^ permalink raw reply
* Re: [RFC] implicit per-namespace devlink instance to set kernel resource limitations
From: Andrew Lunn @ 2019-08-06 18:03 UTC (permalink / raw)
To: David Ahern
Cc: Jiri Pirko, netdev, davem, mlxsw, jakub.kicinski, f.fainelli,
vivien.didelot, mkubecek, stephen, daniel, brouer, eric.dumazet
In-Reply-To: <c615dce5-9307-7640-2877-4e5c01e565c0@gmail.com>
On Tue, Aug 06, 2019 at 11:38:32AM -0600, David Ahern wrote:
> On 8/6/19 10:40 AM, 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. If we have this
> > api for hardware, why don't to reuse it for the kernel and it's
> > resources too?
>
> The analogy is that a kernel is 'programmed' just like hardware, it has
> resources just like hardware (e.g., memory) and those resources are
> limited as well. So the resources consumed by fib entries, rules,
> nexthops, etc should be controllable.
I expect one question that will come up is why not control
groups. That is often used by the rest of the kernel for resource
control.
But cgroups are mostly about limiting resources for a collection of
processes. I don't think that is true for networking resources. The
resources we are talking about are orthogonal to processes. Or are
there any resources which should be linked to processes? eBPF
resources?
> > So the proposal is to have some new device, say "kernelnet", that would
> > implicitly create per-namespace devlink instance.
Maybe kernelns, to make it clear we are talking about namespace
resources.
Going back to cgroups concept. They are generally hierarchical. Do we
need any sort of hierarchy here? Are there some resources we want to
set a global limit on, and then a per namespace limit on top of that?
We would then need two names, and kernelnet sounds more like the
global level?
Andrew
^ 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