* [PATCH net] ipv4: Don't override return code from ip_route_input_noref()
From: Stefano Brivio @ 2017-08-31 16:11 UTC (permalink / raw)
To: David S . Miller, netdev; +Cc: Sabrina Dubroca, Wei Wang
After ip_route_input() calls ip_route_input_noref(), another
check on skb_dst() is done, but if this fails, we shouldn't
override the return code from ip_route_input_noref(), as it
could have been more specific (i.e. -EHOSTUNREACH).
This also saves one call to skb_dst_force_safe() and one to
skb_dst() in case the ip_route_input_noref() check fails.
Reported-by: Sabrina Dubroca <sdubroca@redhat.com>
Fixes: ad65a2f05695 ("ipv4: call dst_hold_safe() properly")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
include/net/route.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index cb0a76d9dde1..1b09a9368c68 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -189,10 +189,11 @@ static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
rcu_read_lock();
err = ip_route_input_noref(skb, dst, src, tos, devin);
- if (!err)
+ if (!err) {
skb_dst_force_safe(skb);
- if (!skb_dst(skb))
- err = -EINVAL;
+ if (!skb_dst(skb))
+ err = -EINVAL;
+ }
rcu_read_unlock();
return err;
--
2.9.4
^ permalink raw reply related
* [patch net-next repost 8/8] mlxsw: spectrum_dpipe: Add support for controlling IPv6 neighbor counters
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Add support for controlling IPv6 neighbor counters via dpipe.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 75da2ef..51e6846 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -679,8 +679,15 @@ mlxsw_sp_dpipe_table_host_counters_update(struct mlxsw_sp *mlxsw_sp,
if (!rif)
continue;
mlxsw_sp_rif_neigh_for_each(neigh_entry, rif) {
- if (mlxsw_sp_neigh_entry_type(neigh_entry) != type)
+ int neigh_type = mlxsw_sp_neigh_entry_type(neigh_entry);
+
+ if (neigh_type != type)
+ continue;
+
+ if (neigh_type == AF_INET6 &&
+ mlxsw_sp_neigh_ipv6_ignore(neigh_entry))
continue;
+
mlxsw_sp_neigh_entry_counter_update(mlxsw_sp,
neigh_entry,
enable);
@@ -778,6 +785,14 @@ mlxsw_sp_dpipe_table_host6_entries_dump(void *priv, bool counters_enabled,
dump_ctx, AF_INET6);
}
+static int mlxsw_sp_dpipe_table_host6_counters_update(void *priv, bool enable)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ mlxsw_sp_dpipe_table_host_counters_update(mlxsw_sp, enable, AF_INET6);
+ return 0;
+}
+
static u64 mlxsw_sp_dpipe_table_host6_size_get(void *priv)
{
struct mlxsw_sp *mlxsw_sp = priv;
@@ -789,6 +804,7 @@ static struct devlink_dpipe_table_ops mlxsw_sp_host6_ops = {
.matches_dump = mlxsw_sp_dpipe_table_host6_matches_dump,
.actions_dump = mlxsw_sp_dpipe_table_host_actions_dump,
.entries_dump = mlxsw_sp_dpipe_table_host6_entries_dump,
+ .counters_set_update = mlxsw_sp_dpipe_table_host6_counters_update,
.size_get = mlxsw_sp_dpipe_table_host6_size_get,
};
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 7/8] mlxsw: spectrum_router: Add support for setting counters on IPv6 neighbors
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Add support for setting counters on IPv6 neighbors based on dpipe's host6
table counter status.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index db57c0c..0cf6810 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1008,21 +1008,33 @@ mlxsw_sp_neigh_entry_remove(struct mlxsw_sp *mlxsw_sp,
}
static bool
-mlxsw_sp_neigh4_counter_should_alloc(struct mlxsw_sp *mlxsw_sp)
+mlxsw_sp_neigh_counter_should_alloc(struct mlxsw_sp *mlxsw_sp,
+ struct mlxsw_sp_neigh_entry *neigh_entry)
{
struct devlink *devlink;
+ const char *table_name;
+
+ switch (mlxsw_sp_neigh_entry_type(neigh_entry)) {
+ case AF_INET:
+ table_name = MLXSW_SP_DPIPE_TABLE_NAME_HOST4;
+ break;
+ case AF_INET6:
+ table_name = MLXSW_SP_DPIPE_TABLE_NAME_HOST6;
+ break;
+ default:
+ WARN_ON(1);
+ return false;
+ }
devlink = priv_to_devlink(mlxsw_sp->core);
- return devlink_dpipe_table_counter_enabled(devlink,
- MLXSW_SP_DPIPE_TABLE_NAME_HOST4);
+ return devlink_dpipe_table_counter_enabled(devlink, table_name);
}
static void
mlxsw_sp_neigh_counter_alloc(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_neigh_entry *neigh_entry)
{
- if (mlxsw_sp_neigh_entry_type(neigh_entry) != AF_INET ||
- !mlxsw_sp_neigh4_counter_should_alloc(mlxsw_sp))
+ if (!mlxsw_sp_neigh_counter_should_alloc(mlxsw_sp, neigh_entry))
return;
if (mlxsw_sp_flow_counter_alloc(mlxsw_sp, &neigh_entry->counter_index))
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 6/8] mlxsw: spectrum_dpipe: Add support for IPv6 host table dump
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Add support for IPv6 host table dump.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 75 ++++++++++++++++++++--
1 file changed, 70 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 5924e97..75da2ef 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -386,8 +386,19 @@ mlxsw_sp_dpipe_table_host_match_action_prepare(struct devlink_dpipe_match *match
match = &matches[MLXSW_SP_DPIPE_TABLE_HOST_MATCH_DIP];
match->type = DEVLINK_DPIPE_MATCH_TYPE_FIELD_EXACT;
- match->header = &devlink_dpipe_header_ipv4;
- match->field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP;
+ switch (type) {
+ case AF_INET:
+ match->header = &devlink_dpipe_header_ipv4;
+ match->field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP;
+ break;
+ case AF_INET6:
+ match->header = &devlink_dpipe_header_ipv6;
+ match->field_id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP;
+ break;
+ default:
+ WARN_ON(1);
+ return;
+ }
action->type = DEVLINK_DPIPE_ACTION_TYPE_FIELD_MODIFY;
action->header = &devlink_dpipe_header_ethernet;
@@ -424,7 +435,18 @@ mlxsw_sp_dpipe_table_host_entry_prepare(struct devlink_dpipe_entry *entry,
match_value = &match_values[MLXSW_SP_DPIPE_TABLE_HOST_MATCH_DIP];
match_value->match = match;
- match_value->value_size = sizeof(u32);
+ switch (type) {
+ case AF_INET:
+ match_value->value_size = sizeof(u32);
+ break;
+ case AF_INET6:
+ match_value->value_size = sizeof(struct in6_addr);
+ break;
+ default:
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
match_value->value = kmalloc(match_value->value_size, GFP_KERNEL);
if (!match_value->value)
return -ENOMEM;
@@ -479,6 +501,20 @@ mlxsw_sp_dpipe_table_host4_entry_fill(struct devlink_dpipe_entry *entry,
}
static void
+mlxsw_sp_dpipe_table_host6_entry_fill(struct devlink_dpipe_entry *entry,
+ struct mlxsw_sp_neigh_entry *neigh_entry,
+ struct mlxsw_sp_rif *rif)
+{
+ struct in6_addr *dip;
+ unsigned char *ha;
+
+ ha = mlxsw_sp_neigh_entry_ha(neigh_entry);
+ dip = mlxsw_sp_neigh6_entry_dip(neigh_entry);
+
+ __mlxsw_sp_dpipe_table_host_entry_fill(entry, rif, ha, dip);
+}
+
+static void
mlxsw_sp_dpipe_table_host_entry_fill(struct mlxsw_sp *mlxsw_sp,
struct devlink_dpipe_entry *entry,
struct mlxsw_sp_neigh_entry *neigh_entry,
@@ -487,7 +523,18 @@ mlxsw_sp_dpipe_table_host_entry_fill(struct mlxsw_sp *mlxsw_sp,
{
int err;
- mlxsw_sp_dpipe_table_host4_entry_fill(entry, neigh_entry, rif);
+ switch (type) {
+ case AF_INET:
+ mlxsw_sp_dpipe_table_host4_entry_fill(entry, neigh_entry, rif);
+ break;
+ case AF_INET6:
+ mlxsw_sp_dpipe_table_host6_entry_fill(entry, neigh_entry, rif);
+ break;
+ default:
+ WARN_ON(1);
+ return;
+ }
+
err = mlxsw_sp_neigh_counter_get(mlxsw_sp, neigh_entry,
&entry->counter);
if (!err)
@@ -526,7 +573,13 @@ mlxsw_sp_dpipe_table_host_entries_get(struct mlxsw_sp *mlxsw_sp,
rif_neigh_count = 0;
mlxsw_sp_rif_neigh_for_each(neigh_entry, rif) {
- if (mlxsw_sp_neigh_entry_type(neigh_entry) != type)
+ int neigh_type = mlxsw_sp_neigh_entry_type(neigh_entry);
+
+ if (neigh_type != type)
+ continue;
+
+ if (neigh_type == AF_INET6 &&
+ mlxsw_sp_neigh_ipv6_ignore(neigh_entry))
continue;
if (rif_neigh_count < rif_neigh_skip)
@@ -714,6 +767,17 @@ mlxsw_sp_dpipe_table_host6_matches_dump(void *priv, struct sk_buff *skb)
return mlxsw_sp_dpipe_table_host_matches_dump(skb, AF_INET6);
}
+static int
+mlxsw_sp_dpipe_table_host6_entries_dump(void *priv, bool counters_enabled,
+ struct devlink_dpipe_dump_ctx *dump_ctx)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ return mlxsw_sp_dpipe_table_host_entries_dump(mlxsw_sp,
+ counters_enabled,
+ dump_ctx, AF_INET6);
+}
+
static u64 mlxsw_sp_dpipe_table_host6_size_get(void *priv)
{
struct mlxsw_sp *mlxsw_sp = priv;
@@ -724,6 +788,7 @@ static u64 mlxsw_sp_dpipe_table_host6_size_get(void *priv)
static struct devlink_dpipe_table_ops mlxsw_sp_host6_ops = {
.matches_dump = mlxsw_sp_dpipe_table_host6_matches_dump,
.actions_dump = mlxsw_sp_dpipe_table_host_actions_dump,
+ .entries_dump = mlxsw_sp_dpipe_table_host6_entries_dump,
.size_get = mlxsw_sp_dpipe_table_host6_size_get,
};
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 5/8] mlxsw: spectrum_dpipe: Make host entry fill handler more generic
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Change the host entry filler helper to be applicable for both IPv4/6
addresses.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 93ba6a6..5924e97 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -439,13 +439,12 @@ mlxsw_sp_dpipe_table_host_entry_prepare(struct devlink_dpipe_entry *entry,
}
static void
-__mlxsw_sp_dpipe_table_host4_entry_fill(struct devlink_dpipe_entry *entry,
- struct mlxsw_sp_rif *rif,
- unsigned char *ha, u32 dip)
+__mlxsw_sp_dpipe_table_host_entry_fill(struct devlink_dpipe_entry *entry,
+ struct mlxsw_sp_rif *rif,
+ unsigned char *ha, void *dip)
{
struct devlink_dpipe_value *value;
u32 *rif_value;
- u32 *dip_value;
u8 *ha_value;
/* Set Match RIF index */
@@ -458,9 +457,7 @@ __mlxsw_sp_dpipe_table_host4_entry_fill(struct devlink_dpipe_entry *entry,
/* Set Match DIP */
value = &entry->match_values[MLXSW_SP_DPIPE_TABLE_HOST_MATCH_DIP];
-
- dip_value = value->value;
- *dip_value = dip;
+ memcpy(value->value, dip, value->value_size);
/* Set Action DMAC */
value = entry->action_values;
@@ -478,7 +475,7 @@ mlxsw_sp_dpipe_table_host4_entry_fill(struct devlink_dpipe_entry *entry,
ha = mlxsw_sp_neigh_entry_ha(neigh_entry);
dip = mlxsw_sp_neigh4_entry_dip(neigh_entry);
- __mlxsw_sp_dpipe_table_host4_entry_fill(entry, rif, ha, dip);
+ __mlxsw_sp_dpipe_table_host_entry_fill(entry, rif, ha, &dip);
}
static void
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 4/8] mlxsw: spectrum_router: Add IPv6 neighbor access helper
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Add helper for accessing destination IP in case of IPv6 neighbor.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 9 +++++++++
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 1f41bcd..db57c0c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -947,6 +947,15 @@ u32 mlxsw_sp_neigh4_entry_dip(struct mlxsw_sp_neigh_entry *neigh_entry)
return ntohl(*((__be32 *) n->primary_key));
}
+struct in6_addr *
+mlxsw_sp_neigh6_entry_dip(struct mlxsw_sp_neigh_entry *neigh_entry)
+{
+ struct neighbour *n;
+
+ n = neigh_entry->key.n;
+ return (struct in6_addr *) &n->primary_key;
+}
+
int mlxsw_sp_neigh_counter_get(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_neigh_entry *neigh_entry,
u64 *p_counter)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index 5b68616..87a04af 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -65,6 +65,8 @@ int mlxsw_sp_neigh_entry_type(struct mlxsw_sp_neigh_entry *neigh_entry);
unsigned char *
mlxsw_sp_neigh_entry_ha(struct mlxsw_sp_neigh_entry *neigh_entry);
u32 mlxsw_sp_neigh4_entry_dip(struct mlxsw_sp_neigh_entry *neigh_entry);
+struct in6_addr *
+mlxsw_sp_neigh6_entry_dip(struct mlxsw_sp_neigh_entry *neigh_entry);
#define mlxsw_sp_rif_neigh_for_each(neigh_entry, rif) \
for (neigh_entry = mlxsw_sp_rif_neigh_next(rif, NULL); neigh_entry; \
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 3/8] mlxsw: spectrum_dpipe: Add IPv6 host table initial support
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Add IPv6 host table initial support. The action behavior for both IPv4/6
tables is the same, thus the same action dump op is used. Neighbors with
link local address are ignored.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 76 ++++++++++++++++++++--
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.h | 1 +
2 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
index 3ea1314..93ba6a6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c
@@ -76,6 +76,7 @@ static struct devlink_dpipe_header *mlxsw_dpipe_headers[] = {
&mlxsw_sp_dpipe_header_metadata,
&devlink_dpipe_header_ethernet,
&devlink_dpipe_header_ipv4,
+ &devlink_dpipe_header_ipv6,
};
static struct devlink_dpipe_headers mlxsw_sp_dpipe_headers = {
@@ -328,9 +329,21 @@ static int mlxsw_sp_dpipe_table_host_matches_dump(struct sk_buff *skb, int type)
if (err)
return err;
- match.type = DEVLINK_DPIPE_MATCH_TYPE_FIELD_EXACT;
- match.header = &devlink_dpipe_header_ipv4;
- match.field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP;
+ switch (type) {
+ case AF_INET:
+ match.type = DEVLINK_DPIPE_MATCH_TYPE_FIELD_EXACT;
+ match.header = &devlink_dpipe_header_ipv4;
+ match.field_id = DEVLINK_DPIPE_FIELD_IPV4_DST_IP;
+ break;
+ case AF_INET6:
+ match.type = DEVLINK_DPIPE_MATCH_TYPE_FIELD_EXACT;
+ match.header = &devlink_dpipe_header_ipv6;
+ match.field_id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP;
+ break;
+ default:
+ WARN_ON(1);
+ return -EINVAL;
+ }
return devlink_dpipe_match_put(skb, &match);
}
@@ -342,7 +355,7 @@ mlxsw_sp_dpipe_table_host4_matches_dump(void *priv, struct sk_buff *skb)
}
static int
-mlxsw_sp_dpipe_table_host4_actions_dump(void *priv, struct sk_buff *skb)
+mlxsw_sp_dpipe_table_host_actions_dump(void *priv, struct sk_buff *skb)
{
struct devlink_dpipe_action action = {0};
@@ -648,8 +661,15 @@ mlxsw_sp_dpipe_table_host_size_get(struct mlxsw_sp *mlxsw_sp, int type)
if (!rif)
continue;
mlxsw_sp_rif_neigh_for_each(neigh_entry, rif) {
- if (mlxsw_sp_neigh_entry_type(neigh_entry) != type)
+ int neigh_type = mlxsw_sp_neigh_entry_type(neigh_entry);
+
+ if (neigh_type != type)
continue;
+
+ if (neigh_type == AF_INET6 &&
+ mlxsw_sp_neigh_ipv6_ignore(neigh_entry))
+ continue;
+
size++;
}
}
@@ -667,7 +687,7 @@ static u64 mlxsw_sp_dpipe_table_host4_size_get(void *priv)
static struct devlink_dpipe_table_ops mlxsw_sp_host4_ops = {
.matches_dump = mlxsw_sp_dpipe_table_host4_matches_dump,
- .actions_dump = mlxsw_sp_dpipe_table_host4_actions_dump,
+ .actions_dump = mlxsw_sp_dpipe_table_host_actions_dump,
.entries_dump = mlxsw_sp_dpipe_table_host4_entries_dump,
.counters_set_update = mlxsw_sp_dpipe_table_host4_counters_update,
.size_get = mlxsw_sp_dpipe_table_host4_size_get,
@@ -691,6 +711,43 @@ static void mlxsw_sp_dpipe_host4_table_fini(struct mlxsw_sp *mlxsw_sp)
MLXSW_SP_DPIPE_TABLE_NAME_HOST4);
}
+static int
+mlxsw_sp_dpipe_table_host6_matches_dump(void *priv, struct sk_buff *skb)
+{
+ return mlxsw_sp_dpipe_table_host_matches_dump(skb, AF_INET6);
+}
+
+static u64 mlxsw_sp_dpipe_table_host6_size_get(void *priv)
+{
+ struct mlxsw_sp *mlxsw_sp = priv;
+
+ return mlxsw_sp_dpipe_table_host_size_get(mlxsw_sp, AF_INET6);
+}
+
+static struct devlink_dpipe_table_ops mlxsw_sp_host6_ops = {
+ .matches_dump = mlxsw_sp_dpipe_table_host6_matches_dump,
+ .actions_dump = mlxsw_sp_dpipe_table_host_actions_dump,
+ .size_get = mlxsw_sp_dpipe_table_host6_size_get,
+};
+
+static int mlxsw_sp_dpipe_host6_table_init(struct mlxsw_sp *mlxsw_sp)
+{
+ struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+
+ return devlink_dpipe_table_register(devlink,
+ MLXSW_SP_DPIPE_TABLE_NAME_HOST6,
+ &mlxsw_sp_host6_ops,
+ mlxsw_sp, false);
+}
+
+static void mlxsw_sp_dpipe_host6_table_fini(struct mlxsw_sp *mlxsw_sp)
+{
+ struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+
+ devlink_dpipe_table_unregister(devlink,
+ MLXSW_SP_DPIPE_TABLE_NAME_HOST6);
+}
+
int mlxsw_sp_dpipe_init(struct mlxsw_sp *mlxsw_sp)
{
struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
@@ -707,8 +764,14 @@ int mlxsw_sp_dpipe_init(struct mlxsw_sp *mlxsw_sp)
err = mlxsw_sp_dpipe_host4_table_init(mlxsw_sp);
if (err)
goto err_host4_table_init;
+
+ err = mlxsw_sp_dpipe_host6_table_init(mlxsw_sp);
+ if (err)
+ goto err_host6_table_init;
return 0;
+err_host6_table_init:
+ mlxsw_sp_dpipe_host4_table_fini(mlxsw_sp);
err_host4_table_init:
mlxsw_sp_dpipe_erif_table_fini(mlxsw_sp);
err_erif_table_init:
@@ -720,6 +783,7 @@ void mlxsw_sp_dpipe_fini(struct mlxsw_sp *mlxsw_sp)
{
struct devlink *devlink = priv_to_devlink(mlxsw_sp->core);
+ mlxsw_sp_dpipe_host6_table_fini(mlxsw_sp);
mlxsw_sp_dpipe_host4_table_fini(mlxsw_sp);
mlxsw_sp_dpipe_erif_table_fini(mlxsw_sp);
devlink_dpipe_headers_unregister(devlink);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.h
index c56a3d9..283fde4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.h
@@ -55,5 +55,6 @@ static inline void mlxsw_sp_dpipe_fini(struct mlxsw_sp *mlxsw_sp)
#define MLXSW_SP_DPIPE_TABLE_NAME_ERIF "mlxsw_erif"
#define MLXSW_SP_DPIPE_TABLE_NAME_HOST4 "mlxsw_host4"
+#define MLXSW_SP_DPIPE_TABLE_NAME_HOST6 "mlxsw_host6"
#endif /* _MLXSW_PIPELINE_H_*/
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 2/8] mlxsw: spectrum_router: Export IPv6 link local address check helper
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
Neighbors with link local addresses are not offloaded to the host table,
yet, the are maintained in the driver for adjacency table usage. When
dumping the IPv6 host neighbors this link local neighbors should be
ignored. This patch exports this helper for dpipe usage.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c | 6 ++++--
drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 377d85c..1f41bcd 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -1396,8 +1396,10 @@ mlxsw_sp_router_neigh_entry_op6(struct mlxsw_sp *mlxsw_sp,
mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(rauht), rauht_pl);
}
-static bool mlxsw_sp_neigh_ipv6_ignore(struct neighbour *n)
+bool mlxsw_sp_neigh_ipv6_ignore(struct mlxsw_sp_neigh_entry *neigh_entry)
{
+ struct neighbour *n = neigh_entry->key.n;
+
/* Packets with a link-local destination address are trapped
* after LPM lookup and never reach the neighbour table, so
* there is no need to program such neighbours to the device.
@@ -1420,7 +1422,7 @@ mlxsw_sp_neigh_entry_update(struct mlxsw_sp *mlxsw_sp,
mlxsw_sp_router_neigh_entry_op4(mlxsw_sp, neigh_entry,
mlxsw_sp_rauht_op(adding));
} else if (neigh_entry->key.n->tbl->family == AF_INET6) {
- if (mlxsw_sp_neigh_ipv6_ignore(neigh_entry->key.n))
+ if (mlxsw_sp_neigh_ipv6_ignore(neigh_entry))
return;
mlxsw_sp_router_neigh_entry_op6(mlxsw_sp, neigh_entry,
mlxsw_sp_rauht_op(adding));
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
index fb0f971..5b68616 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.h
@@ -76,5 +76,6 @@ void
mlxsw_sp_neigh_entry_counter_update(struct mlxsw_sp *mlxsw_sp,
struct mlxsw_sp_neigh_entry *neigh_entry,
bool adding);
+bool mlxsw_sp_neigh_ipv6_ignore(struct mlxsw_sp_neigh_entry *neigh_entry);
#endif /* _MLXSW_ROUTER_H_*/
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 1/8] devlink: Add IPv6 header for dpipe
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
In-Reply-To: <20170831155919.1366-1-jiri@resnulli.us>
From: Arkadi Sharshevsky <arkadis@mellanox.com>
This will be used by the IPv6 host table which will be introduced in the
following patches. The fields in the header are added per-use. This header
is global and can be reused by many drivers.
Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 5 +++++
net/core/devlink.c | 17 +++++++++++++++++
3 files changed, 23 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index aaf7178..b9654e1 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -330,6 +330,7 @@ int devlink_dpipe_match_put(struct sk_buff *skb,
struct devlink_dpipe_match *match);
extern struct devlink_dpipe_header devlink_dpipe_header_ethernet;
extern struct devlink_dpipe_header devlink_dpipe_header_ipv4;
+extern struct devlink_dpipe_header devlink_dpipe_header_ipv6;
#else
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 6c17254..0cbca96 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -234,9 +234,14 @@ enum devlink_dpipe_field_ipv4_id {
DEVLINK_DPIPE_FIELD_IPV4_DST_IP,
};
+enum devlink_dpipe_field_ipv6_id {
+ DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+};
+
enum devlink_dpipe_header_id {
DEVLINK_DPIPE_HEADER_ETHERNET,
DEVLINK_DPIPE_HEADER_IPV4,
+ DEVLINK_DPIPE_HEADER_IPV6,
};
#endif /* _UAPI_LINUX_DEVLINK_H_ */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index cbc4b04..7d430c1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -63,6 +63,23 @@ struct devlink_dpipe_header devlink_dpipe_header_ipv4 = {
};
EXPORT_SYMBOL(devlink_dpipe_header_ipv4);
+static struct devlink_dpipe_field devlink_dpipe_fields_ipv6[] = {
+ {
+ .name = "destination ip",
+ .id = DEVLINK_DPIPE_FIELD_IPV6_DST_IP,
+ .bitwidth = 128,
+ },
+};
+
+struct devlink_dpipe_header devlink_dpipe_header_ipv6 = {
+ .name = "ipv6",
+ .id = DEVLINK_DPIPE_HEADER_IPV6,
+ .fields = devlink_dpipe_fields_ipv6,
+ .fields_count = ARRAY_SIZE(devlink_dpipe_fields_ipv6),
+ .global = true,
+};
+EXPORT_SYMBOL(devlink_dpipe_header_ipv6);
+
EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_hwmsg);
static LIST_HEAD(devlink_list);
--
2.9.3
^ permalink raw reply related
* [patch net-next repost 0/8] mlxsw: Add IPv6 host dpipe table
From: Jiri Pirko @ 2017-08-31 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, arkadis, idosch, mlxsw
From: Jiri Pirko <jiri@mellanox.com>
This patchset adds IPv6 host dpipe table support. This will provide the
ability to observe the hardware offloaded IPv6 neighbors.
Arkadi Sharshevsky (8):
devlink: Add IPv6 header for dpipe
mlxsw: spectrum_router: Export IPv6 link local address check helper
mlxsw: spectrum_dpipe: Add IPv6 host table initial support
mlxsw: spectrum_router: Add IPv6 neighbor access helper
mlxsw: spectrum_dpipe: Make host entry fill handler more generic
mlxsw: spectrum_dpipe: Add support for IPv6 host table dump
mlxsw: spectrum_router: Add support for setting counters on IPv6
neighbors
mlxsw: spectrum_dpipe: Add support for controlling IPv6 neighbor
counters
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c | 182 ++++++++++++++++++---
.../net/ethernet/mellanox/mlxsw/spectrum_dpipe.h | 1 +
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 37 ++++-
.../net/ethernet/mellanox/mlxsw/spectrum_router.h | 3 +
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 5 +
net/core/devlink.c | 17 ++
7 files changed, 219 insertions(+), 27 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [RFC PATCH] net: frag limit checks need to use percpu_counter_compare
From: Stephen Hemminger @ 2017-08-31 15:58 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: liujian56, netdev, Florian Westphal
In-Reply-To: <150417481955.28907.15567119824187929000.stgit@firesoul>
On Thu, 31 Aug 2017 12:20:19 +0200
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> +static inline bool frag_mem_over_limit(struct netns_frags *nf, int thresh)
> {
> - return percpu_counter_read(&nf->mem);
> + /* When reading counter here, __percpu_counter_compare() call
> + * will invoke __percpu_counter_sum() when needed. Which
> + * depend on num_online_cpus()*batch size, as each CPU can
> + * potentential can hold a batch count.
> + *
> + * With many CPUs this heavier sum operation will
> + * unfortunately always occur.
> + */
> + if (__percpu_counter_compare(&nf->mem, thresh,
> + frag_percpu_counter_batch) > 0)
> + return true;
> + else
> + return false;
You don't need an if() here.
return __percpu_counter_compare(&nf->mem, thresh,
frag_percpu_counter_batch) > 0;
^ permalink raw reply
* Re: [PATCH net-next] samples/bpf: Fix compilation issue in redirect dummy program
From: Y Song @ 2017-08-31 15:54 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jesper Dangaard Brouer, Tariq Toukan, David S. Miller, netdev,
Eran Ben Elisha, Alexei Starovoitov
In-Reply-To: <59A7F658.4020606@iogearbox.net>
On Thu, Aug 31, 2017 at 4:43 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/31/2017 01:27 PM, Jesper Dangaard Brouer wrote:
>>
>> On Thu, 31 Aug 2017 14:16:39 +0300
>> Tariq Toukan <tariqt@mellanox.com> wrote:
>>
>>> Fix compilation error below:
>>>
>>> $ make samples/bpf/
>>>
>>> LLVM ERROR: 'xdp_redirect_dummy' label emitted multiple times to
>>> assembly file
>>> make[1]: *** [samples/bpf/xdp_redirect_kern.o] Error 1
>>> make: *** [samples/bpf/] Error 2
>>>
>>> Fixes: 306da4e685b4 ("samples/bpf: xdp_redirect load XDP dummy prog on TX
>>> device")
>>> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>>> ---
>>
>>
>> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
>
>> What LLVM/clang version do you use?
>>
>> I don't see this compile error, and I have:
>> $ clang --version
>> clang version 3.9.1 (tags/RELEA
>
>
> I'm seeing the error as well with a fairly recent LLVM from git
> tree (6.0.0git-2d810c2).
>
> Looks like the llvm error is triggered when section name and
> the function name for XDP prog is the same. Changing either the
> function or the section name right above resolves the issue. If
> such error didn't trigger on older versions, people could be
> using such naming scheme as done here, so seems to me like a
> regression on LLVM side we might need to look at ...
Martin fixed a similar bug earlier:
=====
commit a2e8bbd2ef5457485f00b6b947bbbfa2778e5b1e
Author: Martin KaFai Lau <kafai@fb.com>
Date: Thu Jun 8 22:30:17 2017 -0700
bpf: Fix test_obj_id.c for llvm 5.0
llvm 5.0 does not like the section name and the function name
to be the same:
...
=====
gcc also has this behavior. Section name is treated as global
and hence cannot collide with a function name...
^ permalink raw reply
* Re: [PATCH v2 net-next 4/6] udp: flow dissector offload
From: Willem de Bruijn @ 2017-08-31 15:53 UTC (permalink / raw)
To: Paolo Abeni; +Cc: Tom Herbert, David Miller, Network Development
In-Reply-To: <1504089372.2480.55.camel@redhat.com>
On Wed, Aug 30, 2017 at 6:36 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> On Tue, 2017-08-29 at 16:27 -0700, Tom Herbert wrote:
>> Add support to perform UDP specific flow dissection. This is
>> primarily intended for dissecting encapsulated packets in UDP
>> encapsulation.
>>
>> This patch adds a flow_dissect offload for UDP4 and UDP6. The backend
>> function performs a socket lookup and calls the flow_dissect function
>> if a socket is found.
>>
>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>> ---
>> include/linux/udp.h | 8 ++++++++
>> include/net/udp.h | 8 ++++++++
>> include/net/udp_tunnel.h | 8 ++++++++
>> net/ipv4/udp_offload.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>> net/ipv4/udp_tunnel.c | 1 +
>> net/ipv6/udp_offload.c | 13 +++++++++++++
>> 6 files changed, 83 insertions(+)
>>
>> diff --git a/include/linux/udp.h b/include/linux/udp.h
>> index eaea63bc79bb..2e90b189ef6a 100644
>> --- a/include/linux/udp.h
>> +++ b/include/linux/udp.h
>> @@ -79,6 +79,14 @@ struct udp_sock {
>> int (*gro_complete)(struct sock *sk,
>> struct sk_buff *skb,
>> int nhoff);
>> + /* Flow dissector function for a UDP socket */
>> + enum flow_dissect_ret (*flow_dissect)(struct sock *sk,
>> + const struct sk_buff *skb,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>>
>> /* udp_recvmsg try to use this before splicing sk_receive_queue */
>> struct sk_buff_head reader_queue ____cacheline_aligned_in_smp;
>> diff --git a/include/net/udp.h b/include/net/udp.h
>> index f3d1de6f0983..499e4faf8b14 100644
>> --- a/include/net/udp.h
>> +++ b/include/net/udp.h
>> @@ -174,6 +174,14 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
>> struct udphdr *uh, udp_lookup_t lookup);
>> int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>>
>> +enum flow_dissect_ret udp_flow_dissect(const struct sk_buff *skb,
>> + udp_lookup_t lookup,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>> +
>> static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
>> {
>> struct udphdr *uh;
>> diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
>> index 10cce0dd4450..b7102e0f41a9 100644
>> --- a/include/net/udp_tunnel.h
>> +++ b/include/net/udp_tunnel.h
>> @@ -69,6 +69,13 @@ typedef struct sk_buff **(*udp_tunnel_gro_receive_t)(struct sock *sk,
>> struct sk_buff *skb);
>> typedef int (*udp_tunnel_gro_complete_t)(struct sock *sk, struct sk_buff *skb,
>> int nhoff);
>> +typedef enum flow_dissect_ret (*udp_tunnel_flow_dissect_t)(struct sock *sk,
>> + const struct sk_buff *skb,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags);
>>
>> struct udp_tunnel_sock_cfg {
>> void *sk_user_data; /* user data used by encap_rcv call back */
>> @@ -78,6 +85,7 @@ struct udp_tunnel_sock_cfg {
>> udp_tunnel_encap_destroy_t encap_destroy;
>> udp_tunnel_gro_receive_t gro_receive;
>> udp_tunnel_gro_complete_t gro_complete;
>> + udp_tunnel_flow_dissect_t flow_dissect;
>> };
>>
>> /* Setup the given (UDP) sock to receive UDP encapsulated packets */
>> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
>> index 97658bfc1b58..7f0a7ed4a6f7 100644
>> --- a/net/ipv4/udp_offload.c
>> +++ b/net/ipv4/udp_offload.c
>> @@ -328,11 +328,56 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
>> return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb);
>> }
>>
>> +enum flow_dissect_ret udp_flow_dissect(const struct sk_buff *skb,
>> + udp_lookup_t lookup,
>> + struct flow_dissector_key_control *key_control,
>> + struct flow_dissector *flow_dissector,
>> + void *target_container, void *data,
>> + __be16 *p_proto, u8 *p_ip_proto, int *p_nhoff,
>> + int *p_hlen, unsigned int flags)
>> +{
>> + enum flow_dissect_ret ret = FLOW_DISSECT_RET_CONTINUE;
>> + struct udphdr *uh, _uh;
>> + struct sock *sk;
>> +
>> + uh = __skb_header_pointer(skb, *p_nhoff, sizeof(_uh), data,
>> + *p_hlen, &_uh);
>> + if (!uh)
>> + return FLOW_DISSECT_RET_OUT_BAD;
>> +
>> + rcu_read_lock();
>> +
>> + sk = (*lookup)(skb, uh->source, uh->dest);
>> +
>> + if (sk && udp_sk(sk)->flow_dissect)
>> + ret = udp_sk(sk)->flow_dissect(sk, skb, key_control,
>> + flow_dissector, target_container,
>> + data, p_proto, p_ip_proto,
>> + p_nhoff, p_hlen, flags);
>> + rcu_read_unlock();
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(udp_flow_dissect);
>
> If I read the above correctly, this is going to add another full UDP
> lookup per UDP packet, can we avoid it with some static key enabled by
> vxlan/fou/etc. ?
That would also limit the exposure if a bug is discovered. The
vulnerability of this critical path also came up with the recent
flow dissector expansion for ipv6 neighbor discovery.
http://patchwork.ozlabs.org/patch/722957/
Ideally, flow dissector support for less common protocols can be
disabled administratively.
That said, the parser code looks great to me.
^ permalink raw reply
* Re: DSA mv88e6xxx RX frame errors and TCP/IP RX failure
From: Tim Harvey @ 2017-08-31 15:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vivien Didelot, linux-kernel@vger.kernel.org, Fugang Duan,
Ilia Mirkin
In-Reply-To: <20170831014624.GA18554@lunn.ch>
On Wed, Aug 30, 2017 at 6:46 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > /* Report late collisions as a frame error. */
>> > if (status & (BD_ENET_RX_NO | BD_ENET_RX_CL))
>> > ndev->stats.rx_frame_errors++;
>> >
>> > I don't see anywhere else frame errors are counted, but it would be
>> > good to prove we are looking in the right place.
>> >
>>
>> Andrew,
>>
>> (adding IMX FEC driver maintainer to CC)
>>
>> Yes, that's one of them being hit. It looks like ifconfig reports
>> 'frame' as the accumulation of a few stats so here are some more
>> specifics from /sys/class/net/eth0/statistics:
>>
>> root@xenial:/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/net/eth0/statistics#
>> for i in `ls rx_*`; do echo $i:$(cat $i); done
>> rx_bytes:103229
>> rx_compressed:0
>> rx_crc_errors:22
>> rx_dropped:0
>> rx_errors:22
>> rx_fifo_errors:0
>> rx_frame_errors:22
>> rx_length_errors:22
>> rx_missed_errors:0
>> rx_nohandler:0
>> rx_over_errors:0
>> rx_packets:1174
>> root@xenial:/sys/devices/soc0/soc/2100000.aips-bus/2188000.ethernet/net/eth0/statistics#
>> ifconfig eth0
>> eth0 Link encap:Ethernet HWaddr 00:D0:12:41:F3:E7
>> inet6 addr: fe80::2d0:12ff:fe41:f3e7/64 Scope:Link
>> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
>> RX packets:1207 errors:22 dropped:0 overruns:0 frame:66
>> TX packets:42 errors:0 dropped:0 overruns:0 carrier:0
>> collisions:0 txqueuelen:1000
>> RX bytes:106009 (103.5 KiB) TX bytes:4604 (4.4 KiB)
>>
>> Instrumenting fec driver I see the following getting hit:
>>
>> status & BD_ENET_RX_LG /* rx_length_errors: Frame too long */
>> status & BD_ENET_RX_CR /* rx_crc_errors: CRC Error */
>> status & BD_ENET_RX_CL /* rx_frame_errors: Collision? */
>>
>> Is this a frame size issue where the MV88E6176 is sending frames down
>> that exceed the MTU because of headers added?
>
> I did fix an issue recently with that. See
>
> commit fbbeefdd21049fcf9437c809da3828b210577f36
> Author: Andrew Lunn <andrew@lunn.ch>
> Date: Sun Jul 30 19:36:05 2017 +0200
>
> net: fec: Allow reception of frames bigger than 1522 bytes
>
> The FEC Receive Control Register has a 14 bit field indicating the
> longest frame that may be received. It is being set to 1522. Frames
> longer than this are discarded, but counted as being in error.
>
> When using DSA, frames from the switch has an additional header,
> either 4 or 8 bytes if a Marvell switch is used. Thus a full MTU frame
> of 1522 bytes received by the switch on a port becomes 1530 bytes when
> passed to the host via the FEC interface.
>
> Change the maximum receive size to 2048 - 64, where 64 is the maximum
> rx_alignment applied on the receive buffer for AVB capable FEC
> cores. Use this value also for the maximum receive buffer size. The
> driver is already allocating a receive SKB of 2048 bytes, so this
> change should not have any significant effects.
>
> Tested on imx51, imx6, vf610.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
>
> However, this is was of an all/nothing problem. All frames with the
> full MTU were getting dropped, where as i think you are only seeing a
> few dropped?
>
Andrew,
Indeed this does resolve the issue. I see a burst of FIFO overruns
initially when receiving an iperf bandwidth test but that would be
caused by the IMX6 errata and should be mitigated via pause frames.
After that short burst I see no other errors and iperf works fine.
Should we get this patch in the linux-stable tree for the 4.9 kernel?
Thanks!
Tim
^ permalink raw reply
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: David Ahern @ 2017-08-31 15:30 UTC (permalink / raw)
To: Roopa Prabhu, Jesper Dangaard Brouer
Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
Florian Fainelli, Andrew Lunn, bridge
In-Reply-To: <CAJieiUjZ5dTNpBFb8c2KMRgjXRCKVGhJsTfQcpiw+p6fyNVwDw@mail.gmail.com>
On 8/31/17 9:21 AM, Roopa Prabhu wrote:
> On Thu, Aug 31, 2017 at 5:38 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
>> On Wed, 30 Aug 2017 22:18:13 -0700
>> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This extends bridge fdb table tracepoints to also cover
>>> learned fdb entries in the br_fdb_update path. Note that
>>> unlike other tracepoints I have moved this to when the fdb
>>> is modified because this is in the datapath and can generate
>>> a lot of noise in the trace output. br_fdb_update is also called
>>> from added_by_user context in the NTF_USE case which is already
>>> traced ..hence the !added_by_user check.
>>>
>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> ---
>>> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
>>> net/bridge/br_fdb.c | 5 ++++-
>>> net/core/net-traces.c | 1 +
>>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
>>> index 0f1cde0..1bee3e7 100644
>>> --- a/include/trace/events/bridge.h
>>> +++ b/include/trace/events/bridge.h
>>> @@ -92,6 +92,37 @@ TRACE_EVENT(fdb_delete,
>>> __entry->addr[4], __entry->addr[5], __entry->vid)
>>> );
>>>
>>> +TRACE_EVENT(br_fdb_update,
>>> +
>>> + TP_PROTO(struct net_bridge *br, struct net_bridge_port *source,
>>> + const unsigned char *addr, u16 vid, bool added_by_user),
>>> +
>>> + TP_ARGS(br, source, addr, vid, added_by_user),
>>> +
>>> + TP_STRUCT__entry(
>>> + __string(br_dev, br->dev->name)
>>> + __string(dev, source->dev->name)
>>
>> I have found that using the device string name is
>>
>> (1) slow as it involves strcpy+strlen
>>
>> See [1]+[2] where a single dev-name costed me 16 ns, and the base
>> overhead of a bpf attached tracepoint is 25 ns (see [3]).
>>
>> [1] https://git.kernel.org/davem/net-next/c/e7d12ce121a
>> [2] https://git.kernel.org/davem/net-next/c/315ec3990ef
>> [3] https://git.kernel.org/davem/net-next/c/25d4dae1a64
>>
>> (2) strings are also harder to work-with/extract when attaching a bpf_prog
>>
>> See the trouble I'm in accessing a dev string here napi:napi_poll here:
>> https://github.com/netoptimizer/prototype-kernel/blob/103b955a080/kernel/samples/bpf/napi_monitor_kern.c#L52-L58
>>
>> Using ifindex'es in userspace is fairly easy see man if_indextoname(3).
>>
>
> Jesper thanks for the data!. GTK. Looking at include/trace/events,
> currently almost all tracepoints use dev->name.
> These bridge tracepoints in context are primarily for debugging fdb
> updates only, not for every packet and hence not in the performance
> path.
> In large scale deployments with thousands of bridge ports and fdb
> entries, dev->name will definately make it easier to trouble-shoot.
> So, I did like to leave these with dev->name unless there are strong objections.
+1 for user friendliness for debugging tracepoints. The device name is
also more user friendly when adding filters to the data collection.
Being able to add bpf everywhere certainly changes the game a bit, but
we should not relinquish ease of use and understanding for the potential
that someone might want to put a bpf program on the tracepoint and want
to maintain high performance.
^ permalink raw reply
* Re: [PATCH] rtlwifi: refactor code in halbtcoutsrc module
From: Larry Finger @ 2017-08-31 15:25 UTC (permalink / raw)
To: Gustavo A. R. Silva, Chaoming Li, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20170830164602.GA16413@embeddedgus>
On 08/30/2017 11:46 AM, Gustavo A. R. Silva wrote:
> Function halbtc_get_wifi_rssi always returns rtlpriv->dm.undec_sm_pwdb.
> So this function can be removed and the value of
> rtlpriv->dm.undec_sm_pwdb assigned to *s32_tmp directly.
>
> This issue was first reported by Coverity as "identical code for different
> branches" in function halbtc_get_wifi_rssi.
>
> Addresses-Coverity-ID: 1226793
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> This code was reported by Coverity and it was tested by compilation only.
> Chances are this may be a copy/paste error in function
> halbtc_get_wifi_rssi. Please, verify.
> Also, notice this code has been there since 2014.
The value of *s32_tmp is not dependent on the link state, thus this patch is
correct, but I request that it be changed. Future developments will modify
halbtc_get_wifi_rssi() making it more complicated and not as easily inlined as
this patch. In short, if you remove it here, we will have to add it later.
Thanks,
Larrt
>
> .../net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> index c1eacd8..2a47b97 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
> @@ -373,17 +373,6 @@ u32 halbtc_get_wifi_link_status(struct btc_coexist *btcoexist)
> return ret_val;
> }
>
> -static s32 halbtc_get_wifi_rssi(struct rtl_priv *rtlpriv)
> -{
> - int undec_sm_pwdb = 0;
> -
> - if (rtlpriv->mac80211.link_state >= MAC80211_LINKED)
> - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
> - else /* associated entry pwdb */
> - undec_sm_pwdb = rtlpriv->dm.undec_sm_pwdb;
> - return undec_sm_pwdb;
> -}
> -
> static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf)
> {
> struct btc_coexist *btcoexist = (struct btc_coexist *)void_btcoexist;
> @@ -479,7 +468,7 @@ static bool halbtc_get(void *void_btcoexist, u8 get_type, void *out_buf)
> *bool_tmp = false;
> break;
> case BTC_GET_S4_WIFI_RSSI:
> - *s32_tmp = halbtc_get_wifi_rssi(rtlpriv);
> + *s32_tmp = rtlpriv->dm.undec_sm_pwdb;
> break;
> case BTC_GET_S4_HS_RSSI:
> *s32_tmp = 0;
>
^ permalink raw reply
* Re: [PATCH net-next] net/ncsi: Define {add, kill}_vid callbacks for !CONFIG_NET_NCSI
From: Vernon Mauery @ 2017-08-31 15:24 UTC (permalink / raw)
To: Samuel Mendoza-Jonas
Cc: David S . Miller, netdev, linux-kernel, OpenBMC Maillist,
Gavin Shan
In-Reply-To: <20170831033846.23538-1-sam@mendozajonas.com>
On 31-Aug-2017 01:38 PM, Samuel Mendoza-Jonas wrote:
> Patch "net/ncsi: Configure VLAN tag filter" defined two new callback
> functions in include/net/ncsi.h, but neglected the !CONFIG_NET_NCSI
> case. This can cause a build error if these are referenced elsewhere
> without NCSI enabled, for example in ftgmac100:
>
> >>> ERROR: "ncsi_vlan_rx_kill_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
> >>> ERROR: "ncsi_vlan_rx_add_vid" [drivers/net/ethernet/faraday/ftgmac100.ko] undefined!
>
> Add definitions for !CONFIG_NET_NCSI to bring it into line with the rest
> of ncsi.h
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> ---
> include/net/ncsi.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/net/ncsi.h b/include/net/ncsi.h
> index 1f96af46df49..2b13b6b91a4d 100644
> --- a/include/net/ncsi.h
> +++ b/include/net/ncsi.h
> @@ -36,6 +36,14 @@ int ncsi_start_dev(struct ncsi_dev *nd);
> void ncsi_stop_dev(struct ncsi_dev *nd);
> void ncsi_unregister_dev(struct ncsi_dev *nd);
> #else /* !CONFIG_NET_NCSI */
> +int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> + return -ENOTTY;
> +}
> +int ncsi_vlan_rx_kill_vid(struct net_device *dev, __be16 proto, u16 vid)
> +{
> + return -ENOTTY;
> +}
These should be static functions because they are defined in the header
file or you will get multiple symbol definitions.
--Vernon
> static inline struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
> void (*notifier)(struct ncsi_dev *nd))
> {
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: Roopa Prabhu @ 2017-08-31 15:21 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem@davemloft.net, netdev@vger.kernel.org, Nikolay Aleksandrov,
Florian Fainelli, Andrew Lunn, bridge
In-Reply-To: <20170831143814.68709334@redhat.com>
On Thu, Aug 31, 2017 at 5:38 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Wed, 30 Aug 2017 22:18:13 -0700
> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This extends bridge fdb table tracepoints to also cover
>> learned fdb entries in the br_fdb_update path. Note that
>> unlike other tracepoints I have moved this to when the fdb
>> is modified because this is in the datapath and can generate
>> a lot of noise in the trace output. br_fdb_update is also called
>> from added_by_user context in the NTF_USE case which is already
>> traced ..hence the !added_by_user check.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
>> net/bridge/br_fdb.c | 5 ++++-
>> net/core/net-traces.c | 1 +
>> 3 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
>> index 0f1cde0..1bee3e7 100644
>> --- a/include/trace/events/bridge.h
>> +++ b/include/trace/events/bridge.h
>> @@ -92,6 +92,37 @@ TRACE_EVENT(fdb_delete,
>> __entry->addr[4], __entry->addr[5], __entry->vid)
>> );
>>
>> +TRACE_EVENT(br_fdb_update,
>> +
>> + TP_PROTO(struct net_bridge *br, struct net_bridge_port *source,
>> + const unsigned char *addr, u16 vid, bool added_by_user),
>> +
>> + TP_ARGS(br, source, addr, vid, added_by_user),
>> +
>> + TP_STRUCT__entry(
>> + __string(br_dev, br->dev->name)
>> + __string(dev, source->dev->name)
>
> I have found that using the device string name is
>
> (1) slow as it involves strcpy+strlen
>
> See [1]+[2] where a single dev-name costed me 16 ns, and the base
> overhead of a bpf attached tracepoint is 25 ns (see [3]).
>
> [1] https://git.kernel.org/davem/net-next/c/e7d12ce121a
> [2] https://git.kernel.org/davem/net-next/c/315ec3990ef
> [3] https://git.kernel.org/davem/net-next/c/25d4dae1a64
>
> (2) strings are also harder to work-with/extract when attaching a bpf_prog
>
> See the trouble I'm in accessing a dev string here napi:napi_poll here:
> https://github.com/netoptimizer/prototype-kernel/blob/103b955a080/kernel/samples/bpf/napi_monitor_kern.c#L52-L58
>
> Using ifindex'es in userspace is fairly easy see man if_indextoname(3).
>
Jesper thanks for the data!. GTK. Looking at include/trace/events,
currently almost all tracepoints use dev->name.
These bridge tracepoints in context are primarily for debugging fdb
updates only, not for every packet and hence not in the performance
path.
In large scale deployments with thousands of bridge ports and fdb
entries, dev->name will definately make it easier to trouble-shoot.
So, I did like to leave these with dev->name unless there are strong objections.
thanks.
^ permalink raw reply
* [PATCH] wl1251: add a missing spin_lock_init()
From: Pavel Machek @ 2017-08-31 14:47 UTC (permalink / raw)
To: Kalle Valo, Linus Torvalds
Cc: David Miller, xiyou.wangcong, torvalds, akpm, netdev,
linux-kernel, linux-wireless
In-Reply-To: <87y3q0q7ln.fsf@kamboji.qca.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 3388 bytes --]
From: Cong Wang <xiyou.wangcong@gmail.com>
wl1251: add a missing spin_lock_init()
This fixes the following kernel warning:
[ 5668.771453] BUG: spinlock bad magic on CPU#0, kworker/u2:3/9745
[ 5668.771850] lock: 0xce63ef20, .magic: 00000000, .owner: <none>/-1,
.owner_cpu: 0
[ 5668.772277] CPU: 0 PID: 9745 Comm: kworker/u2:3 Tainted: G W
4.12.0-03002-gec979a4-dirty #40
[ 5668.772796] Hardware name: Nokia RX-51 board
[ 5668.773071] Workqueue: phy1 wl1251_irq_work
[ 5668.773345] [<c010c9e4>] (unwind_backtrace) from [<c010a274>]
(show_stack+0x10/0x14)
[ 5668.773803] [<c010a274>] (show_stack) from [<c01545a4>]
(do_raw_spin_lock+0x6c/0xa0)
[ 5668.774230] [<c01545a4>] (do_raw_spin_lock) from [<c06ca578>]
(_raw_spin_lock_irqsave+0x10/0x18)
[ 5668.774658] [<c06ca578>] (_raw_spin_lock_irqsave) from [<c048c010>]
(wl1251_op_tx+0x38/0x5c)
[ 5668.775115] [<c048c010>] (wl1251_op_tx) from [<c06a12e8>]
(ieee80211_tx_frags+0x188/0x1c0)
[ 5668.775543] [<c06a12e8>] (ieee80211_tx_frags) from [<c06a138c>]
(__ieee80211_tx+0x6c/0x130)
[ 5668.775970] [<c06a138c>] (__ieee80211_tx) from [<c06a3dbc>]
(ieee80211_tx+0xdc/0x104)
[ 5668.776367] [<c06a3dbc>] (ieee80211_tx) from [<c06a4af0>]
(__ieee80211_subif_start_xmit+0x454/0x8c8)
[ 5668.776824] [<c06a4af0>] (__ieee80211_subif_start_xmit) from
[<c06a4f94>] (ieee80211_subif_start_xmit+0x30/0x2fc)
[ 5668.777343] [<c06a4f94>] (ieee80211_subif_start_xmit) from
[<c0578848>] (dev_hard_start_xmit+0x80/0x118)
...
by adding the missing spin_lock_init().
Reported-by: Pavel Machek <pavel@ucw.cz>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Cc: stable@kernel.org
---
> >> Yeah, you are right there. I did actually ponder which I tree should
> >> commit it back in July but due to various reasons decided differently.
> >
> > Can we still get the fix to v4.13-final? :-).
>
> I'm not planning to submit pull requests to 4.13 anymore. If you think
> this is so important that it needs to be applied in the last minute (I
> don't) you could always try to convince Dave to take it directly.
>
> Or better yet, push it to the stable tree. If the merge window opens on
> Sunday I suspect that the commit will be in Linus' tree sometime next
> week. Then you can submit the request to the stable team to take it.
I don't think we should use stable tree as an excuse for not fixing
the bugs in mainline. Original patch is from Jul 6, thats 7 weeks ago.
Dave, Linus -- can you still take the patch?
Thanks,
Pavel
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 08f0477..9915d83 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1571,6 +1571,7 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
wl->state = WL1251_STATE_OFF;
mutex_init(&wl->mutex);
+ spin_lock_init(&wl->wl_lock);
wl->tx_mgmt_frm_rate = DEFAULT_HW_GEN_TX_RATE;
wl->tx_mgmt_frm_mod = DEFAULT_HW_GEN_MODULATION_TYPE;
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related
* Re: [PATCH net-next] x86: bpf_jit: small optimization in emit_bpf_tail_call()
From: Alexei Starovoitov @ 2017-08-31 14:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <1504180422.15310.12.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Aug 31, 2017 at 04:53:42AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Saves 4 bytes replacing following instructions :
>
> lea rax, [rsi + rdx * 8 + offsetof(...)]
> mov rax, qword ptr [rax]
> cmp rax, 0
>
> by :
>
> mov rax, [rsi + rdx * 8 + offsetof(...)]
> test rax, rax
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Nicely spotted. Much appreciate it!
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next] virtio-net: invoke zerocopy callback on xmit path if no tx napi
From: Willem de Bruijn @ 2017-08-31 14:30 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Koichiro Den, Jason Wang, virtualization, Network Development
In-Reply-To: <CAF=yD-+Wk9sc9dXMUq1+x_hh=3ThTXa6BnZkygP3tgVpjbp93g@mail.gmail.com>
On Tue, Aug 29, 2017 at 3:35 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> On Fri, Aug 25, 2017 at 9:03 PM, Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>> On Fri, Aug 25, 2017 at 7:32 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Fri, Aug 25, 2017 at 06:44:36PM -0400, Willem de Bruijn wrote:
>>>> >> >> > We don't enable network watchdog on virtio but we could and maybe
>>>> >> >> > should.
>>>> >> >>
>>>> >> >> Can you elaborate?
>>>> >> >
>>>> >> > The issue is that holding onto buffers for very long times makes guests
>>>> >> > think they are stuck. This is funamentally because from guest point of
>>>> >> > view this is a NIC, so it is supposed to transmit things out in
>>>> >> > a timely manner. If host backs the virtual NIC by something that is not
>>>> >> > a NIC, with traffic shaping etc introducing unbounded latencies,
>>>> >> > guest will be confused.
>>>> >>
>>>> >> That assumes that guests are fragile in this regard. A linux guest
>>>> >> does not make such assumptions.
>>>> >
>>>> > Yes it does. Examples above:
>>>> > > > - a single slow flow can occupy the whole ring, you will not
>>>> > > > be able to make any new buffers available for the fast flow
>>>>
>>>> Oh, right. Though those are due to vring_desc pool exhaustion
>>>> rather than an upper bound on latency of any single packet.
>>>>
>>>> Limiting the number of zerocopy packets in flight to some fraction
>>>> of the ring ensures that fast flows can always grab a slot.
>>>> Running
>>>> out of ubuf_info slots reverts to copy, so indirectly does this. But
>>>> I read it correclty the zerocopy pool may be equal to or larger than
>>>> the descriptor pool. Should we refine the zcopy_used test
>>>>
>>>> (nvq->upend_idx + 1) % UIO_MAXIOV != nvq->done_idx
>>>>
>>>> to also return false if the number of outstanding ubuf_info is greater
>>>> than, say, vq->num >> 1?
>>>
>>>
>>> We'll need to think about where to put the threshold, but I think it's
>>> a good idea.
>>>
>>> Maybe even a fixed number, e.g. max(vq->num >> 1, X) to limit host
>>> resources.
>>>
>>> In a sense it still means once you run out of slots zcopt gets disabled possibly permanently.
>>>
>>> Need to experiment with some numbers.
>>
>> I can take a stab with two flows, one delayed in a deep host qdisc
>> queue. See how this change affects the other flow and also how
>> sensitive that is to the chosen threshold value.
>
> Incomplete results at this stage, but I do see this correlation between
> flows. It occurs even while not running out of zerocopy descriptors,
> which I cannot yet explain.
>
> Running two threads in a guest, each with a udp socket, each
> sending up to 100 datagrams, or until EAGAIN, every msec.
>
> Sender A sends 1B datagrams.
> Sender B sends VHOST_GOODCOPY_LEN, which is enough
> to trigger zcopy_used in vhost net.
>
> A local receive process on the host receives both flows. To avoid
> a deep copy when looping the packet onto the receive path,
> changed skb_orphan_frags_rx to always return false (gross hack).
>
> The flow with the larger packets is redirected through netem on ifb0:
>
> modprobe ifb
> ip link set dev ifb0 up
> tc qdisc add dev ifb0 root netem limit $LIMIT rate 1MBit
>
> tc qdisc add dev tap0 ingress
> tc filter add dev tap0 parent ffff: protocol ip \
> u32 match ip dport 8000 0xffff \
> action mirred egress redirect dev ifb0
>
> For 10 second run, packet count with various ifb0 queue lengths $LIMIT:
>
> no filter
> rx.A: ~840,000
> rx.B: ~840,000
>
> limit 1
> rx.A: ~500,000
> rx.B: ~3100
> ifb0: 3273 sent, 371141 dropped
>
> limit 100
> rx.A: ~9000
> rx.B: ~4200
> ifb0: 4630 sent, 1491 dropped
>
> limit 1000
> rx.A: ~6800
> rx.B: ~4200
> ifb0: 4651 sent, 0 dropped
>
> Sender B is always correctly rate limited to 1 MBps or less. With a
> short queue, it ends up dropping a lot and sending even less.
>
> When a queue builds up for sender B, sender A throughput is strongly
> correlated with queue length. With queue length 1, it can send almost
> at unthrottled speed. But even at limit 100 its throughput is on the
> same order as sender B.
>
> What is surprising to me is that this happens even though the number
> of ubuf_info in use at limit 100 is around 100 at all times. In other words,
> it does not exhaust the pool.
>
> When forcing zcopy_used to be false for all packets, this effect of
> sender A throughput being correlated with sender B does not happen.
>
> no filter
> rx.A: ~850,000
> rx.B: ~850,000
>
> limit 100
> rx.A: ~850,000
> rx.B: ~4200
> ifb0: 4518 sent, 876182 dropped
>
> Also relevant is that with zerocopy, the sender processes back off
> and report the same count as the receiver. Without zerocopy,
> both senders send at full speed, even if only 4200 packets from flow
> B arrive at the receiver.
>
> This is with the default virtio_net driver, so without napi-tx.
>
> It appears that the zerocopy notifications are pausing the guest.
> Will look at that now.
It was indeed as simple as that. With 256 descriptors, queuing even
a hundred or so packets causes the guest to stall the device as soon
as the qdisc is installed.
Adding this check
+ in_use = nvq->upend_idx - nvq->done_idx;
+ if (nvq->upend_idx < nvq->done_idx)
+ in_use += UIO_MAXIOV;
+
+ if (in_use > (vq->num >> 2))
+ zcopy_used = false;
Has the desired behavior of reverting zerocopy requests to copying.
Without this change, the result is, as previously reported, throughput
dropping to hundreds of packets per second on both flows.
With the change, pps as observed for a few seconds at handle_tx is
zerocopy=165 copy=168435
zerocopy=0 copy=168500
zerocopy=65 copy=168535
Both flows continue to send at more or less normal rate, with only
sender B observing massive drops at the netem.
With the queue removed the rate reverts to
zerocopy=58878 copy=110239
zerocopy=58833 copy=110207
This is not a 50/50 split, which implies that some packets from the large
packet flow are still converted to copying. Without the change the rate
without queue was 80k zerocopy vs 80k copy, so this choice of
(vq->num >> 2) appears too conservative.
However, testing with (vq->num >> 1) was not as effective at mitigating
stalls. I did not save that data, unfortunately. Can run more tests on fine
tuning this variable, if the idea sounds good.
I also compiled qemu with 1024 descriptors in the tx ring instead of 256.
In that case the test
if (unlikely(vhost_exceeds_maxpend(net)))
break;
which is
return (nvq->upend_idx + vq->num - VHOST_MAX_PEND) % UIO_MAXIOV
== nvq->done_idx;
Is hit before my proposed change, again stalling the device instead of
reverting to copying.
^ permalink raw reply
* Re: [PATCH v2 net-next 1/8] bpf: Add support for recursively running cgroup sock filters
From: Tejun Heo @ 2017-08-31 14:22 UTC (permalink / raw)
To: David Ahern; +Cc: Alexei Starovoitov, netdev, daniel, ast, davem
In-Reply-To: <bb4398e5-55a3-d63e-ac4d-3f9d7e2efc02@gmail.com>
Hello, David, Alexei.
Sorry about late reply.
On Sun, Aug 27, 2017 at 08:49:23AM -0600, David Ahern wrote:
> On 8/25/17 8:49 PM, Alexei Starovoitov wrote:
> >
> >> + if (prog && curr_recursive && !new_recursive)
> >> + /* if a parent has recursive prog attached, only
> >> + * allow recursive programs in descendent cgroup
> >> + */
> >> + return -EINVAL;
> >> +
> >> old_prog = cgrp->bpf.prog[type];
> >
> > ... I'm struggling to completely understand how it interacts
> > with BPF_F_ALLOW_OVERRIDE.
>
> The 2 flags are completely independent. The existing override logic is
> unchanged. If a program can not be overridden, then the new recursive
> flag is irrelevant.
I'm not sure all four combo of the two flags makes sense. Can't we
have something simpler like the following?
1. None: No further bpf programs allowed in the subtree.
2. Overridable: If a sub-cgroup installs the same bpf program, this
one yields to that one.
3. Recursive: If a sub-cgroup installs the same bpf program, that
cgroup program gets run in addition to this one.
Note that we can have combinations of overridables and recursives -
both allow further programs in the sub-hierarchy and the only
distinction is whether that specific program behaves when that
happens.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
From: Marc Gonzalez @ 2017-08-31 14:21 UTC (permalink / raw)
To: Florian Fainelli, Mans Rullgard
Cc: David Daney, netdev, Geert Uytterhoeven, David Miller,
Andrew Lunn, Mason
In-Reply-To: <f4bb5ac8-dae8-c0af-7aa6-e546fc0783fa@sigmadesigns.com>
On 31/08/2017 14:29, Marc Gonzalez wrote:
> On 31/08/2017 02:49, Florian Fainelli wrote:
>
>> The original motivation for this change originated from Marc Gonzalez
>> indicating that his network driver did not have its adjust_link callback
>> executing with phydev->link = 0 while he was expecting it.
>
> I expect the core to call phy_adjust_link() for link changes.
> This used to work back in 3.4 and was broken somewhere along
> the way.
>
>> PHYLIB has never made any such guarantees ever because phy_stop() merely
>> just tells the workqueue to move into PHY_HALTED state which will happen
>> asynchronously.
>
> My original proposal was to fix the issue in the driver.
> I'll try locating it in my archives.
The original proposal was:
(I.e. basically a copy of phy_state_machine()'s PHY_HALTED case)
Is this what I need to submit, now that the synchronous call to
phy_state_machine() has been removed?
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index 607064a6d7a1..8b9a981c55c1 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1017,6 +1017,10 @@ static int nb8800_stop(struct net_device *dev)
phy_disconnect(phydev);
+ phydev->link = 0;
+ netif_carrier_off(dev);
+ nb8800_link_reconfigure(dev);
+
free_irq(dev->irq, dev);
nb8800_dma_free(dev);
^ permalink raw reply related
* Re: [PATCH net-next] bridge: add tracepoint in br_fdb_update
From: Nikolay Aleksandrov @ 2017-08-31 14:19 UTC (permalink / raw)
To: Roopa Prabhu, davem; +Cc: netdev, f.fainelli, andrew, bridge
In-Reply-To: <1504156693-24692-1-git-send-email-roopa@cumulusnetworks.com>
On 31/08/17 08:18, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This extends bridge fdb table tracepoints to also cover
> learned fdb entries in the br_fdb_update path. Note that
> unlike other tracepoints I have moved this to when the fdb
> is modified because this is in the datapath and can generate
> a lot of noise in the trace output. br_fdb_update is also called
> from added_by_user context in the NTF_USE case which is already
> traced ..hence the !added_by_user check.
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> include/trace/events/bridge.h | 31 +++++++++++++++++++++++++++++++
> net/bridge/br_fdb.c | 5 ++++-
> net/core/net-traces.c | 1 +
> 3 files changed, 36 insertions(+), 1 deletion(-)
>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* [PATCH] ipv6: sr: Use ARRAY_SIZE macro
From: Thomas Meyer @ 2017-08-31 14:18 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: Thomas Meyer
Grepping for "sizeof\(.+\) / sizeof\(" found this as one of the first
candidates.
Maybe a coccinelle can catch all of those.
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
net/ipv6/seg6_hmac.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f950cb53d5e3..33fb35cbfac1 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -12,6 +12,7 @@
*/
#include <linux/errno.h>
+#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/socket.h>
#include <linux/sockios.h>
@@ -110,7 +111,7 @@ static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
struct seg6_hmac_algo *algo;
int i, alg_count;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
algo = &hmac_algos[i];
if (algo->alg_id == alg_id)
@@ -360,7 +361,7 @@ static int seg6_hmac_init_algo(void)
struct shash_desc *shash;
int i, alg_count, cpu;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
struct crypto_shash **p_tfm;
@@ -421,7 +422,7 @@ void seg6_hmac_exit(void)
struct seg6_hmac_algo *algo = NULL;
int i, alg_count, cpu;
- alg_count = sizeof(hmac_algos) / sizeof(struct seg6_hmac_algo);
+ alg_count = ARRAY_SIZE(hmac_algos);
for (i = 0; i < alg_count; i++) {
algo = &hmac_algos[i];
for_each_possible_cpu(cpu) {
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox