* [PATCH net-next v6 07/11] ipv6/route: Change return code of rt6_dump_route() for partial node dumps
From: Stefano Brivio @ 2019-06-19 23:59 UTC (permalink / raw)
To: David Miller
Cc: Jianlin Shi, Wei Wang, David Ahern, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <cover.1560987611.git.sbrivio@redhat.com>
In the next patch, we are going to add optional dump of exceptions to
rt6_dump_route().
Change the return code of rt6_dump_route() to accomodate partial node
dumps: we might dump multiple routes per node, and might be able to dump
only a given number of them, so fib6_dump_node() will need to know how
many routes have been dumped on partial dump, to restart the dump from the
point where it was interrupted.
Note that fib6_dump_node() is the only caller and already handles all
non-negative return codes as success: those become -1 to signal that we're
done with the node. If we fail, return 0, as we were unable to dump the
single route in the node, but we're not done with it.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v6: New patch
net/ipv6/ip6_fib.c | 2 +-
net/ipv6/route.c | 16 ++++++++++------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 2baca6ef9e01..0e9a2ec69336 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -464,7 +464,7 @@ static int fib6_dump_node(struct fib6_walker *w)
for_each_fib6_walker_rt(w) {
res = rt6_dump_route(rt, w->args);
- if (res < 0) {
+ if (res >= 0) {
/* Frame is full, suspend walking */
w->leaf = rt;
return 1;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 86859023cd01..1282f5a55b08 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5503,6 +5503,7 @@ static bool fib6_info_uses_dev(const struct fib6_info *f6i,
return false;
}
+/* Return -1 if done with node, number of handled routes on partial dump */
int rt6_dump_route(struct fib6_info *rt, void *p_arg)
{
struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) p_arg;
@@ -5511,25 +5512,28 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg)
struct net *net = arg->net;
if (rt == net->ipv6.fib6_null_entry)
- return 0;
+ return -1;
if ((filter->flags & RTM_F_PREFIX) &&
!(rt->fib6_flags & RTF_PREFIX_RT)) {
/* success since this is not a prefix route */
- return 1;
+ return -1;
}
if (filter->filter_set) {
if ((filter->rt_type && rt->fib6_type != filter->rt_type) ||
(filter->dev && !fib6_info_uses_dev(rt, filter->dev)) ||
(filter->protocol && rt->fib6_protocol != filter->protocol)) {
- return 1;
+ return -1;
}
flags |= NLM_F_DUMP_FILTERED;
}
- return rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL, 0,
- RTM_NEWROUTE, NETLINK_CB(arg->cb->skb).portid,
- arg->cb->nlh->nlmsg_seq, flags);
+ if (rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL, 0, RTM_NEWROUTE,
+ NETLINK_CB(arg->cb->skb).portid,
+ arg->cb->nlh->nlmsg_seq, flags))
+ return 0;
+
+ return -1;
}
static int inet6_rtm_valid_getroute_req(struct sk_buff *skb,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v6 08/11] ipv6: Dump route exceptions if requested
From: Stefano Brivio @ 2019-06-19 23:59 UTC (permalink / raw)
To: David Miller
Cc: Jianlin Shi, Wei Wang, David Ahern, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <cover.1560987611.git.sbrivio@redhat.com>
Since commit 2b760fcf5cfb ("ipv6: hook up exception table to store dst
cache"), route exceptions reside in a separate hash table, and won't be
found by walking the FIB, so they won't be dumped to userspace on a
RTM_GETROUTE message.
This causes 'ip -6 route list cache' and 'ip -6 route flush cache' to
have no function anymore:
# ip -6 route get fc00:3::1
fc00:3::1 via fc00:1::2 dev veth_A-R1 src fc00:1::1 metric 1024 expires 539sec mtu 1400 pref medium
# ip -6 route get fc00:4::1
fc00:4::1 via fc00:2::2 dev veth_A-R2 src fc00:2::1 metric 1024 expires 536sec mtu 1500 pref medium
# ip -6 route list cache
# ip -6 route flush cache
# ip -6 route get fc00:3::1
fc00:3::1 via fc00:1::2 dev veth_A-R1 src fc00:1::1 metric 1024 expires 520sec mtu 1400 pref medium
# ip -6 route get fc00:4::1
fc00:4::1 via fc00:2::2 dev veth_A-R2 src fc00:2::1 metric 1024 expires 519sec mtu 1500 pref medium
because iproute2 lists cached routes using RTM_GETROUTE, and flushes them
by listing all the routes, and deleting them with RTM_DELROUTE one by one.
If cached routes are requested using the RTM_F_CLONED flag together with
strict checking, or if no strict checking is requested (and hence we can't
consistently apply filters), look up exceptions in the hash table
associated with the current fib6_info in rt6_dump_route(), and, if present
and not expired, add them to the dump.
We might be unable to dump all the entries for a given node in a single
message, so keep track of how many entries were handled for the current
node in fib6_walker, and skip that amount in case we start from the same
partially dumped node.
Note that, with the current version of iproute2, this only fixes the
'ip -6 route list cache': on a flush command, iproute2 doesn't pass
RTM_F_CLONED and, due to this inconsistency, 'ip -6 route flush cache' is
still unable to fetch the routes to be flushed. This will be addressed in
a patch for iproute2.
To flush cached routes, a procfs entry could be introduced instead: that's
how it works for IPv4. We already have a rt6_flush_exception() function
ready to be wired to it. However, this would not solve the issue for
listing.
Versions of iproute2 and kernel tested:
iproute2
kernel 4.14.0 4.15.0 4.19.0 5.0.0 5.1.0 5.1.0, patched
3.18 list + + + + + +
flush + + + + + +
4.4 list + + + + + +
flush + + + + + +
4.9 list + + + + + +
flush + + + + + +
4.14 list + + + + + +
flush + + + + + +
4.15 list
flush
4.19 list
flush
5.0 list
flush
5.1 list
flush
with list + + + + + +
fix flush + + + +
v6:
- Rebase onto net-next, use recently introduced nexthop walker
- Make rt6_nh_dump_exceptions() a separate function, suggested by David
Ahern
- Move change of rt6_dump_route() return codes to separate patch, 7/11,
also suggested by David Ahern
v5:
- use dump_routes and dump_exceptions from filter, ignore NLM_F_MATCH,
update test results (flushing works with iproute2 < 5.0.0 now)
v4:
- split NLM_F_MATCH and strict check handling in separate patches
- filter routes using RTM_F_CLONED: if it's not set, only return
non-cached routes, and if it's set, only return cached routes:
change requested by David Ahern and Martin Lau. This implies that
iproute2 needs a separate patch to be able to flush IPv6 cached
routes. This is not ideal because we can't fix the breakage caused
by 2b760fcf5cfb entirely in kernel. However, two years have passed
since then, and this makes it more tolerable
v3:
- more descriptive comment about expired exceptions in rt6_dump_route()
- swap return values of rt6_dump_route() (suggested by Martin Lau)
- don't zero skip_in_node in case we don't dump anything in a given pass
(also suggested by Martin Lau)
- remove check on RTM_F_CLONED altogether: in the current UAPI semantic,
it's just a flag to indicate the route was cloned, not to filter on
routes
v2: Add tracking of number of entries to be skipped in current node after
a partial dump. As we restart from the same node, if not all the
exceptions for a given node fit in a single message, the dump will
not terminate, as suggested by Martin Lau. This is a concrete
possibility, setting up a big number of exceptions for the same route
actually causes the issue, suggested by David Ahern.
Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: 2b760fcf5cfb ("ipv6: hook up exception table to store dst cache")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
include/net/ip6_fib.h | 1 +
include/net/ip6_route.h | 2 +-
net/ipv6/ip6_fib.c | 12 ++++-
net/ipv6/route.c | 114 ++++++++++++++++++++++++++++++++++++----
4 files changed, 116 insertions(+), 13 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 87331f2c4af0..4b5656c71abc 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -316,6 +316,7 @@ struct fib6_walker {
enum fib6_walk_state state;
unsigned int skip;
unsigned int count;
+ unsigned int skip_in_node;
int (*func)(struct fib6_walker *);
void *args;
};
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 7375a165fd98..a849a379f426 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -182,7 +182,7 @@ struct rt6_rtnl_dump_arg {
struct fib_dump_filter filter;
};
-int rt6_dump_route(struct fib6_info *f6i, void *p_arg);
+int rt6_dump_route(struct fib6_info *f6i, void *p_arg, unsigned int skip);
void rt6_mtu_change(struct net_device *dev, unsigned int mtu);
void rt6_remove_prefsrc(struct inet6_ifaddr *ifp);
void rt6_clean_tohost(struct net *net, struct in6_addr *gateway);
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 0e9a2ec69336..92a6bde57594 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -463,12 +463,19 @@ static int fib6_dump_node(struct fib6_walker *w)
struct fib6_info *rt;
for_each_fib6_walker_rt(w) {
- res = rt6_dump_route(rt, w->args);
+ res = rt6_dump_route(rt, w->args, w->skip_in_node);
if (res >= 0) {
/* Frame is full, suspend walking */
w->leaf = rt;
+
+ /* We'll restart from this node, so if some routes were
+ * already dumped, skip them next time.
+ */
+ w->skip_in_node += res;
+
return 1;
}
+ w->skip_in_node = 0;
/* Multipath routes are dumped in one route with the
* RTA_MULTIPATH attribute. Jump 'rt' to point to the
@@ -520,6 +527,7 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
if (cb->args[4] == 0) {
w->count = 0;
w->skip = 0;
+ w->skip_in_node = 0;
spin_lock_bh(&table->tb6_lock);
res = fib6_walk(net, w);
@@ -535,6 +543,7 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
w->state = FWS_INIT;
w->node = w->root;
w->skip = w->count;
+ w->skip_in_node = 0;
} else
w->skip = 0;
@@ -2093,6 +2102,7 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
c.w.func = fib6_clean_node;
c.w.count = 0;
c.w.skip = 0;
+ c.w.skip_in_node = 0;
c.func = func;
c.sernum = sernum;
c.arg = arg;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1282f5a55b08..e2900cd0410c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -5503,13 +5503,73 @@ static bool fib6_info_uses_dev(const struct fib6_info *f6i,
return false;
}
+struct fib6_nh_exception_dump_walker {
+ struct rt6_rtnl_dump_arg *dump;
+ struct fib6_info *rt;
+ unsigned int flags;
+ unsigned int skip;
+ unsigned int count;
+};
+
+static int rt6_nh_dump_exceptions(struct fib6_nh *nh, void *arg)
+{
+ struct fib6_nh_exception_dump_walker *w = arg;
+ struct rt6_rtnl_dump_arg *dump = w->dump;
+ struct rt6_exception_bucket *bucket;
+ struct rt6_exception *rt6_ex;
+ int i, err;
+
+ bucket = fib6_nh_get_excptn_bucket(nh, NULL);
+ if (!bucket)
+ return 0;
+
+ for (i = 0; i < FIB6_EXCEPTION_BUCKET_SIZE; i++) {
+ hlist_for_each_entry(rt6_ex, &bucket->chain, hlist) {
+ if (w->skip) {
+ w->skip--;
+ continue;
+ }
+
+ /* Expiration of entries doesn't bump sernum, insertion
+ * does. Removal is triggered by insertion, so we can
+ * rely on the fact that if entries change between two
+ * partial dumps, this node is scanned again completely,
+ * see rt6_insert_exception() and fib6_dump_table().
+ *
+ * Count expired entries we go through as handled
+ * entries that we'll skip next time, in case of partial
+ * node dump. Otherwise, if entries expire meanwhile,
+ * we'll skip the wrong amount.
+ */
+ if (rt6_check_expired(rt6_ex->rt6i)) {
+ w->count++;
+ continue;
+ }
+
+ err = rt6_fill_node(dump->net, dump->skb, w->rt,
+ &rt6_ex->rt6i->dst, NULL, NULL, 0,
+ RTM_NEWROUTE,
+ NETLINK_CB(dump->cb->skb).portid,
+ dump->cb->nlh->nlmsg_seq, w->flags);
+ if (err)
+ return err;
+
+ w->count++;
+ }
+ bucket++;
+ }
+
+ return 0;
+}
+
/* Return -1 if done with node, number of handled routes on partial dump */
-int rt6_dump_route(struct fib6_info *rt, void *p_arg)
+int rt6_dump_route(struct fib6_info *rt, void *p_arg, unsigned int skip)
{
struct rt6_rtnl_dump_arg *arg = (struct rt6_rtnl_dump_arg *) p_arg;
struct fib_dump_filter *filter = &arg->filter;
unsigned int flags = NLM_F_MULTI;
struct net *net = arg->net;
+ int count = 0;
if (rt == net->ipv6.fib6_null_entry)
return -1;
@@ -5519,19 +5579,51 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg)
/* success since this is not a prefix route */
return -1;
}
- if (filter->filter_set) {
- if ((filter->rt_type && rt->fib6_type != filter->rt_type) ||
- (filter->dev && !fib6_info_uses_dev(rt, filter->dev)) ||
- (filter->protocol && rt->fib6_protocol != filter->protocol)) {
- return -1;
- }
+ if (filter->filter_set &&
+ ((filter->rt_type && rt->fib6_type != filter->rt_type) ||
+ (filter->dev && !fib6_info_uses_dev(rt, filter->dev)) ||
+ (filter->protocol && rt->fib6_protocol != filter->protocol))) {
+ return -1;
+ }
+
+ if (filter->filter_set ||
+ !filter->dump_routes || !filter->dump_exceptions) {
flags |= NLM_F_DUMP_FILTERED;
}
- if (rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL, 0, RTM_NEWROUTE,
- NETLINK_CB(arg->cb->skb).portid,
- arg->cb->nlh->nlmsg_seq, flags))
- return 0;
+ if (filter->dump_routes) {
+ if (skip) {
+ skip--;
+ } else {
+ if (rt6_fill_node(net, arg->skb, rt, NULL, NULL, NULL,
+ 0, RTM_NEWROUTE,
+ NETLINK_CB(arg->cb->skb).portid,
+ arg->cb->nlh->nlmsg_seq, flags)) {
+ return 0;
+ }
+ count++;
+ }
+ }
+
+ if (filter->dump_exceptions) {
+ struct fib6_nh_exception_dump_walker w = { .dump = arg,
+ .rt = rt,
+ .flags = flags,
+ .skip = skip,
+ .count = 0 };
+ int err;
+
+ if (rt->nh) {
+ err = nexthop_for_each_fib6_nh(rt->nh,
+ rt6_nh_dump_exceptions,
+ &w);
+ } else {
+ err = rt6_nh_dump_exceptions(rt->fib6_nh, &w);
+ }
+
+ if (err)
+ return count += w.count;
+ }
return -1;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v6 09/11] ip6_fib: Don't discard nodes with valid routing information in fib6_locate_1()
From: Stefano Brivio @ 2019-06-19 23:59 UTC (permalink / raw)
To: David Miller
Cc: Jianlin Shi, Wei Wang, David Ahern, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <cover.1560987611.git.sbrivio@redhat.com>
When we perform an inexact match on FIB nodes via fib6_locate_1(), longer
prefixes will be preferred to shorter ones. However, it might happen that
a node, with higher fn_bit value than some other, has no valid routing
information.
In this case, we'll pick that node, but it will be discarded by the check
on RTN_RTINFO in fib6_locate(), and we might miss nodes with valid routing
information but with lower fn_bit value.
This is apparent when a routing exception is created for a default route:
# ip -6 route list
fc00:1::/64 dev veth_A-R1 proto kernel metric 256 pref medium
fc00:2::/64 dev veth_A-R2 proto kernel metric 256 pref medium
fc00:4::1 via fc00:2::2 dev veth_A-R2 metric 1024 pref medium
fe80::/64 dev veth_A-R1 proto kernel metric 256 pref medium
fe80::/64 dev veth_A-R2 proto kernel metric 256 pref medium
default via fc00:1::2 dev veth_A-R1 metric 1024 pref medium
# ip -6 route list cache
fc00:4::1 via fc00:2::2 dev veth_A-R2 metric 1024 expires 593sec mtu 1500 pref medium
fc00:3::1 via fc00:1::2 dev veth_A-R1 metric 1024 expires 593sec mtu 1500 pref medium
# ip -6 route flush cache # node for default route is discarded
Failed to send flush request: No such process
# ip -6 route list cache
fc00:3::1 via fc00:1::2 dev veth_A-R1 metric 1024 expires 586sec mtu 1500 pref medium
Check right away if the node has a RTN_RTINFO flag, before replacing the
'prev' pointer, that indicates the longest matching prefix found so far.
Fixes: 38fbeeeeccdb ("ipv6: prepare fib6_locate() for exception table")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
v6: Rebased onto net-next, no changes
v5: No changes
v4: No changes
v3: No changes
v2: No changes
net/ipv6/ip6_fib.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 92a6bde57594..8f36e6742c36 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1595,7 +1595,8 @@ static struct fib6_node *fib6_locate_1(struct fib6_node *root,
if (plen == fn->fn_bit)
return fn;
- prev = fn;
+ if (fn->fn_flags & RTN_RTINFO)
+ prev = fn;
next:
/*
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v6 10/11] selftests: pmtu: Introduce list_flush_ipv4_exception test case
From: Stefano Brivio @ 2019-06-19 23:59 UTC (permalink / raw)
To: David Miller
Cc: Jianlin Shi, Wei Wang, David Ahern, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <cover.1560987611.git.sbrivio@redhat.com>
This test checks that route exceptions can be successfully listed and
flushed using ip -6 route {list,flush} cache.
v6:
- Merge this patch into series including fix, as it's also targeted
for net-next
- Drop left-over print of 'ip route list cache | wc -l'
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tools/testing/selftests/net/pmtu.sh | 60 +++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 269e839b747e..f5004a9df229 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -112,6 +112,10 @@
# - cleanup_ipv6_exception
# Same as above, but use IPv6 transport from A to B
#
+# - list_flush_ipv4_exception
+# Using the same topology as in pmtu_ipv4, create exceptions, and check
+# they are shown when listing exception caches, gone after flushing them
+#
# - list_flush_ipv6_exception
# Using the same topology as in pmtu_ipv6, create exceptions, and check
# they are shown when listing exception caches, gone after flushing them
@@ -156,6 +160,7 @@ tests="
pmtu_vti6_link_change_mtu vti6: MTU changes on link changes 0
cleanup_ipv4_exception ipv4: cleanup of cached exceptions 1
cleanup_ipv6_exception ipv6: cleanup of cached exceptions 1
+ list_flush_ipv4_exception ipv4: list and flush cached exceptions 1
list_flush_ipv6_exception ipv6: list and flush cached exceptions 1"
NS_A="ns-A"
@@ -1207,6 +1212,61 @@ run_test_nh() {
USE_NH=no
}
+test_list_flush_ipv4_exception() {
+ setup namespaces routing || return 2
+ trace "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \
+ "${ns_r1}" veth_R1-B "${ns_b}" veth_B-R1 \
+ "${ns_a}" veth_A-R2 "${ns_r2}" veth_R2-A \
+ "${ns_r2}" veth_R2-B "${ns_b}" veth_B-R2
+
+ dst_prefix1="${prefix4}.${b_r1}."
+ dst2="${prefix4}.${b_r2}.1"
+
+ # Set up initial MTU values
+ mtu "${ns_a}" veth_A-R1 2000
+ mtu "${ns_r1}" veth_R1-A 2000
+ mtu "${ns_r1}" veth_R1-B 1500
+ mtu "${ns_b}" veth_B-R1 1500
+
+ mtu "${ns_a}" veth_A-R2 2000
+ mtu "${ns_r2}" veth_R2-A 2000
+ mtu "${ns_r2}" veth_R2-B 1500
+ mtu "${ns_b}" veth_B-R2 1500
+
+ fail=0
+
+ # Add 100 addresses for veth endpoint on B reached by default A route
+ for i in $(seq 100 199); do
+ run_cmd ${ns_b} ip addr add "${dst_prefix1}${i}" dev veth_B-R1
+ done
+
+ # Create 100 cached route exceptions for path via R1, one via R2. Note
+ # that with IPv4 we need to actually cause a route lookup that matches
+ # the exception caused by ICMP, in order to actually have a cached
+ # route, so we need to ping each destination twice
+ for i in $(seq 100 199); do
+ run_cmd ${ns_a} ping -q -M want -i 0.1 -c 2 -s 1800 "${dst_prefix1}${i}"
+ done
+ run_cmd ${ns_a} ping -q -M want -i 0.1 -c 2 -s 1800 "${dst2}"
+
+ # Each exception is printed as two lines
+ if [ "$(${ns_a} ip route list cache | wc -l)" -ne 202 ]; then
+ err " can't list cached exceptions"
+ fail=1
+ fi
+
+ run_cmd ${ns_a} ip route flush cache
+ pmtu1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst_prefix}1)"
+ pmtu2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst_prefix}2)"
+ if [ -n "${pmtu1}" ] || [ -n "${pmtu2}" ] || \
+ [ -n "$(${ns_a} ip route list cache)" ]; then
+ err " can't flush cached exceptions"
+ fail=1
+ fi
+
+ return ${fail}
+}
+
test_list_flush_ipv6_exception() {
setup namespaces routing || return 2
trace "${ns_a}" veth_A-R1 "${ns_r1}" veth_R1-A \
--
2.20.1
^ permalink raw reply related
* [PATCH net-next v6 11/11] selftests: pmtu: Make list_flush_ipv6_exception test more demanding
From: Stefano Brivio @ 2019-06-19 23:59 UTC (permalink / raw)
To: David Miller
Cc: Jianlin Shi, Wei Wang, David Ahern, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <cover.1560987611.git.sbrivio@redhat.com>
Instead of just listing and flushing two cached exceptions, create
a relatively big number of them, and count how many are listed. Single
netlink dump messages contain approximately 25 entries each, and this
way we can make sure the partial dump tracking mechanism is working
properly.
While at it, also ensure that no cached routes can be listed after
flush, and remove 'sleep 1' calls, they are not actually needed.
v6:
- Merge this patch into series including fix, as it's also targeted
for net-next. No actual changes
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tools/testing/selftests/net/pmtu.sh | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index f5004a9df229..ab367e75f095 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -1274,7 +1274,7 @@ test_list_flush_ipv6_exception() {
"${ns_a}" veth_A-R2 "${ns_r2}" veth_R2-A \
"${ns_r2}" veth_R2-B "${ns_b}" veth_B-R2
- dst1="${prefix6}:${b_r1}::1"
+ dst_prefix1="${prefix6}:${b_r1}::"
dst2="${prefix6}:${b_r2}::1"
# Set up initial MTU values
@@ -1290,20 +1290,26 @@ test_list_flush_ipv6_exception() {
fail=0
- # Create route exceptions
- run_cmd ${ns_a} ${ping6} -q -M want -i 0.1 -w 1 -s 1800 ${dst1}
- run_cmd ${ns_a} ${ping6} -q -M want -i 0.1 -w 1 -s 1800 ${dst2}
+ # Add 100 addresses for veth endpoint on B reached by default A route
+ for i in $(seq 100 199); do
+ run_cmd ${ns_b} ip addr add "${dst_prefix1}${i}" dev veth_B-R1
+ done
- if [ "$(${ns_a} ip -6 route list cache | wc -l)" -ne 2 ]; then
+ # Create 100 cached route exceptions for path via R1, one via R2
+ for i in $(seq 100 199); do
+ run_cmd ${ns_a} ping -q -M want -i 0.1 -w 1 -s 1800 "${dst_prefix1}${i}"
+ done
+ run_cmd ${ns_a} ping -q -M want -i 0.1 -w 1 -s 1800 "${dst2}"
+ if [ "$(${ns_a} ip -6 route list cache | wc -l)" -ne 101 ]; then
err " can't list cached exceptions"
fail=1
fi
run_cmd ${ns_a} ip -6 route flush cache
- sleep 1
- pmtu1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+ pmtu1="$(route_get_dst_pmtu_from_exception "${ns_a}" "${dst_prefix1}100")"
pmtu2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
- if [ -n "${pmtu1}" ] || [ -n "${pmtu2}" ]; then
+ if [ -n "${pmtu1}" ] || [ -n "${pmtu2}" ] || \
+ [ -n "$(${ns_a} ip -6 route list cache)" ]; then
err " can't flush cached exceptions"
fail=1
fi
--
2.20.1
^ permalink raw reply related
* RE: [PATCH v3 bpf-next 1/9] bpf: track spill/fill of constants
From: John Fastabend @ 2019-06-20 0:24 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190615191225.2409862-2-ast@kernel.org>
Alexei Starovoitov wrote:
> Compilers often spill induction variables into the stack,
> hence it is necessary for the verifier to track scalar values
> of the registers through stack slots.
>
> Also few bpf programs were incorrectly rejected in the past,
> since the verifier was not able to track such constants while
> they were used to compute offsets into packet headers.
>
> Tracking constants through the stack significantly decreases
> the chances of state pruning, since two different constants
> are considered to be different by state equivalency.
> End result that cilium tests suffer serious degradation in the number
> of states processed and corresponding verification time increase.
>
> before after
> bpf_lb-DLB_L3.o 1838 6441
> bpf_lb-DLB_L4.o 3218 5908
> bpf_lb-DUNKNOWN.o 1064 1064
> bpf_lxc-DDROP_ALL.o 26935 93790
> bpf_lxc-DUNKNOWN.o 34439 123886
> bpf_netdev.o 9721 31413
> bpf_overlay.o 6184 18561
> bpf_lxc_jit.o 39389 359445
>
> After further debugging turned out that cillium progs are
> getting hurt by clang due to the same constant tracking issue.
> Newer clang generates better code by spilling less to the stack.
> Instead it keeps more constants in the registers which
> hurts state pruning since the verifier already tracks constants
> in the registers:
> old clang new clang
> (no spill/fill tracking introduced by this patch)
> bpf_lb-DLB_L3.o 1838 1923
> bpf_lb-DLB_L4.o 3218 3077
> bpf_lb-DUNKNOWN.o 1064 1062
> bpf_lxc-DDROP_ALL.o 26935 166729
> bpf_lxc-DUNKNOWN.o 34439 174607
^^^^^^^^^^^^^^
Any idea what happened here? Going from 34439 -> 174607 on the new clang?
> bpf_netdev.o 9721 8407
> bpf_overlay.o 6184 5420
> bpf_lcx_jit.o 39389 39389
>
> The final table is depressing:
> old clang old clang new clang new clang
> const spill/fill const spill/fill
> bpf_lb-DLB_L3.o 1838 6441 1923 8128
> bpf_lb-DLB_L4.o 3218 5908 3077 6707
> bpf_lb-DUNKNOWN.o 1064 1064 1062 1062
> bpf_lxc-DDROP_ALL.o 26935 93790 166729 380712
> bpf_lxc-DUNKNOWN.o 34439 123886 174607 440652
> bpf_netdev.o 9721 31413 8407 31904
> bpf_overlay.o 6184 18561 5420 23569
> bpf_lxc_jit.o 39389 359445 39389 359445
>
> Tracking constants in the registers hurts state pruning already.
> Adding tracking of constants through stack hurts pruning even more.
> The later patch address this general constant tracking issue
> with coarse/precise logic.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> ---
> kernel/bpf/verifier.c | 90 +++++++++++++++++++++++++++++++------------
> 1 file changed, 65 insertions(+), 25 deletions(-)
I know these are already in bpf-next sorry it took me awhile to get
time to review, but looks good to me. Thanks! We had something similar
in the earlier loop test branch from last year.
^ permalink raw reply
* Re: [PATCH v3 0/7] Hexdump Enhancements
From: Joe Perches @ 2019-06-20 0:35 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <c68cb819257f251cbb66f8998a95c31cebe2d72e.camel@d-silva.org>
On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote:
> On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote:
> > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > >
> > > Apologies for the large CC list, it's a heads up for those
> > > responsible
> > > for subsystems where a prototype change in generic code causes a
> > > change
> > > in those subsystems.
> > >
> > > This series enhances hexdump.
> >
> > Still not a fan of these patches.
>
> I'm afraid there's not too much action I can take on that, I'm happy to
> address specific issues though.
>
> > > These improve the readability of the dumped data in certain
> > > situations
> > > (eg. wide terminals are available, many lines of empty bytes exist,
> > > etc).
I think it's generally overkill for the desired uses.
> > Changing hexdump's last argument from bool to int is odd.
> >
>
> Think of it as replacing a single boolean with many booleans.
I understand it. It's odd.
I would rather not have a mixture of true, false, and apparently
random collections of bitfields like 0xd or 0b1011 or their
equivalent or'd defines.
> There's only a handful of consumers, I don't think there is a value-add
> in creating more wrappers vs updating the existing callers.
Perhaps more reason not to modify the existing api.
^ permalink raw reply
* [PATCH v2] perf cs-etm: Improve completeness for kernel address space
From: Leo Yan @ 2019-06-20 0:54 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Mathieu Poirier, Suzuki K Poulose,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, linux-kernel, linux-arm-kernel, netdev, bpf
Cc: Leo Yan, Peter Zijlstra, coresight
Arm and arm64 architecture reserve some memory regions prior to the
symbol '_stext' and these memory regions later will be used by device
module and BPF jit. The current code misses to consider these memory
regions thus any address in the regions will be taken as user space
mode, but perf cannot find the corresponding dso with the wrong CPU
mode so we misses to generate samples for device module and BPF
related trace data.
This patch parse the link scripts to get the memory size prior to start
address and reduce this size from 'etmq->etm->kernel_start', then can
get a fixed up kernel start address which contain memory regions for
device module and BPF. Finally, cs_etm__cpu_mode() can return right
mode for these memory regions and perf can successfully generate
samples.
The reason for parsing the link scripts is Arm architecture changes text
offset dependent on different platforms, which define multiple text
offsets in $kernel/arch/arm/Makefile. This offset is decided when build
kernel and the final value is extended in the link script, so we can
extract the used value from the link script. We use the same way to
parse arm64 link script as well. If fail to find the link script, the
pre start memory size is assumed as zero, in this case it has no any
change caused with this patch.
Below is detailed info for testing this patch:
- Build LLVM/Clang 8.0 or later version;
- Configure perf with ~/.perfconfig:
root@debian:~# cat ~/.perfconfig
# this file is auto-generated.
[llvm]
clang-path = /mnt/build/llvm-build/build/install/bin/clang
kbuild-dir = /mnt/linux-kernel/linux-cs-dev/
clang-opt = "-g"
dump-obj = true
[trace]
show_zeros = yes
show_duration = no
no_inherit = yes
show_timestamp = no
show_arg_names = no
args_alignment = 40
show_prefix = yes
- Run 'perf trace' command with eBPF event:
root@debian:~# perf trace -e string \
-e $kernel/tools/perf/examples/bpf/augmented_raw_syscalls.c
- Read eBPF program memory mapping in kernel:
root@debian:~# echo 1 > /proc/sys/net/core/bpf_jit_kallsyms
root@debian:~# cat /proc/kallsyms | grep -E "bpf_prog_.+_sys_[enter|exit]"
ffff000000086a84 t bpf_prog_f173133dc38ccf87_sys_enter [bpf]
ffff000000088618 t bpf_prog_c1bd85c092d6e4aa_sys_exit [bpf]
- Launch any program which accesses file system frequently so can hit
the system calls trace flow with eBPF event;
- Capture CoreSight trace data with filtering eBPF program:
root@debian:~# perf record -e cs_etm/@20070000.etr/ \
--filter 'filter 0xffff000000086a84/0x800' -a sleep 5s
- Annotate for symbol 'bpf_prog_f173133dc38ccf87_sys_enter':
root@debian:~# perf report
Then select 'branches' samples and press 'a' to annotate symbol
'bpf_prog_f173133dc38ccf87_sys_enter', press 'P' to print to the
bpf_prog_f173133dc38ccf87_sys_enter.annotation file:
root@debian:~# cat bpf_prog_f173133dc38ccf87_sys_enter.annotation
bpf_prog_f173133dc38ccf87_sys_enter() bpf_prog_f173133dc38ccf87_sys_enter
Event: branches
Percent int sys_enter(struct syscall_enter_args *args)
stp x29, x30, [sp, #-16]!
int key = 0;
mov x29, sp
augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
stp x19, x20, [sp, #-16]!
augmented_args = bpf_map_lookup_elem(&augmented_filename_map, &key);
stp x21, x22, [sp, #-16]!
stp x25, x26, [sp, #-16]!
return bpf_get_current_pid_tgid();
mov x25, sp
return bpf_get_current_pid_tgid();
mov x26, #0x0 // #0
sub sp, sp, #0x10
return bpf_map_lookup_elem(pids, &pid) != NULL;
add x19, x0, #0x0
mov x0, #0x0 // #0
mov x10, #0xfffffffffffffff8 // #-8
if (pid_filter__has(&pids_filtered, getpid()))
str w0, [x25, x10]
probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
add x1, x25, #0x0
probe_read(&augmented_args->args, sizeof(augmented_args->args), args);
mov x10, #0xfffffffffffffff8 // #-8
syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
add x1, x1, x10
syscall = bpf_map_lookup_elem(&syscalls, &augmented_args->args.syscall_nr);
mov x0, #0xffff8009ffffffff // #-140694538682369
movk x0, #0x6698, lsl #16
movk x0, #0x3e00
mov x10, #0xffffffffffff1040 // #-61376
if (syscall == NULL || !syscall->enabled)
movk x10, #0x1023, lsl #16
if (syscall == NULL || !syscall->enabled)
movk x10, #0x0, lsl #32
loop_iter_first()
3.69 → blr bpf_prog_f173133dc38ccf87_sys_enter
loop_iter_first()
add x7, x0, #0x0
loop_iter_first()
add x20, x7, #0x0
int size = probe_read_str(&augmented_filename->value, filename_len, filename_arg);
mov x0, #0x1 // #1
[...]
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
tools/perf/Makefile.config | 20 ++++++++++++++++++++
tools/perf/util/cs-etm.c | 19 ++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 51dd00f65709..cf5906d667aa 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -418,6 +418,26 @@ ifdef CORESIGHT
endif
LDFLAGS += $(LIBOPENCSD_LDFLAGS)
EXTLIBS += $(OPENCSDLIBS)
+ ARM_PRE_START_SIZE := 0
+ ifeq ($(SRCARCH),arm64)
+ # Extract info from lds:
+ # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
+ # ARM_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000)
+ ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
+ $(srctree)/arch/arm64/kernel/vmlinux.lds | \
+ sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
+ awk -F' ' '{print "("$$6 "+" $$7 "+" $$8")"}' 2>/dev/null)
+ endif
+ ifeq ($(SRCARCH),arm)
+ # Extract info from lds:
+ # . = ((0xC0000000)) + 0x00208000;
+ # ARM_PRE_START_SIZE := 0x00208000
+ ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
+ $(srctree)/arch/arm/kernel/vmlinux.lds | \
+ sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
+ awk -F' ' '{print "("$$2")"}' 2>/dev/null)
+ endif
+ CFLAGS += -DARM_PRE_START_SIZE="$(ARM_PRE_START_SIZE)"
$(call detected,CONFIG_LIBOPENCSD)
ifdef CSTRACE_RAW
CFLAGS += -DCS_DEBUG_RAW
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 0c7776b51045..5fa0be3a3904 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -613,10 +613,27 @@ static void cs_etm__free(struct perf_session *session)
static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
{
struct machine *machine;
+ u64 fixup_kernel_start = 0;
machine = etmq->etm->machine;
- if (address >= etmq->etm->kernel_start) {
+ /*
+ * Since arm and arm64 specify some memory regions prior to
+ * 'kernel_start', kernel addresses can be less than 'kernel_start'.
+ *
+ * For arm architecture, the 16MB virtual memory space prior to
+ * 'kernel_start' is allocated to device modules, a PMD table if
+ * CONFIG_HIGHMEM is enabled and a PGD table.
+ *
+ * For arm64 architecture, the root PGD table, device module memory
+ * region and BPF jit region are prior to 'kernel_start'.
+ *
+ * To reflect the complete kernel address space, compensate these
+ * pre-defined regions for kernel start address.
+ */
+ fixup_kernel_start = etmq->etm->kernel_start - ARM_PRE_START_SIZE;
+
+ if (address >= fixup_kernel_start) {
if (machine__is_host(machine))
return PERF_RECORD_MISC_KERNEL;
else
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] perf cs-etm: Improve completeness for kernel address space
From: Leo Yan @ 2019-06-20 0:58 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Arnaldo Carvalho de Melo, Linux Kernel Mailing List,
linux-arm-kernel, netdev, bpf, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, Coresight ML
In-Reply-To: <CANLsYkyMW=WG+=yWTLSyMT3JXqd_2kvsrx9c-EwCoKEnRZvErA@mail.gmail.com>
Hi Mathieu,
On Wed, Jun 19, 2019 at 11:49:44AM -0600, Mathieu Poirier wrote:
[...]
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 51dd00f65709..4776c2c1fb6d 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -418,6 +418,30 @@ ifdef CORESIGHT
> > endif
> > LDFLAGS += $(LIBOPENCSD_LDFLAGS)
> > EXTLIBS += $(OPENCSDLIBS)
> > + ifneq ($(wildcard $(srctree)/arch/arm64/kernel/vmlinux.lds),)
> > + # Extract info from lds:
> > + # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> > + # ARM64_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000)
> > + ARM64_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> > + $(srctree)/arch/arm64/kernel/vmlinux.lds | \
> > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > + awk -F' ' '{print "("$$6 "+" $$7 "+" $$8")"}' 2>/dev/null)
> > + else
> > + ARM64_PRE_START_SIZE := 0
> > + endif
> > + CFLAGS += -DARM64_PRE_START_SIZE="$(ARM64_PRE_START_SIZE)"
> > + ifneq ($(wildcard $(srctree)/arch/arm/kernel/vmlinux.lds),)
> > + # Extract info from lds:
> > + # . = ((0xC0000000)) + 0x00208000;
> > + # ARM_PRE_START_SIZE := 0x00208000
> > + ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> > + $(srctree)/arch/arm/kernel/vmlinux.lds | \
> > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > + awk -F' ' '{print "("$$2")"}' 2>/dev/null)
> > + else
> > + ARM_PRE_START_SIZE := 0
> > + endif
> > + CFLAGS += -DARM_PRE_START_SIZE="$(ARM_PRE_START_SIZE)"
> > $(call detected,CONFIG_LIBOPENCSD)
> > ifdef CSTRACE_RAW
> > CFLAGS += -DCS_DEBUG_RAW
> > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > index 0c7776b51045..ae831f836c70 100644
> > --- a/tools/perf/util/cs-etm.c
> > +++ b/tools/perf/util/cs-etm.c
> > @@ -613,10 +613,34 @@ static void cs_etm__free(struct perf_session *session)
> > static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> > {
> > struct machine *machine;
> > + u64 fixup_kernel_start = 0;
> > + const char *arch;
> >
> > machine = etmq->etm->machine;
> > + arch = perf_env__arch(machine->env);
> >
> > - if (address >= etmq->etm->kernel_start) {
> > + /*
> > + * Since arm and arm64 specify some memory regions prior to
> > + * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> > + *
> > + * For arm architecture, the 16MB virtual memory space prior to
> > + * 'kernel_start' is allocated to device modules, a PMD table if
> > + * CONFIG_HIGHMEM is enabled and a PGD table.
> > + *
> > + * For arm64 architecture, the root PGD table, device module memory
> > + * region and BPF jit region are prior to 'kernel_start'.
> > + *
> > + * To reflect the complete kernel address space, compensate these
> > + * pre-defined regions for kernel start address.
> > + */
> > + if (!strcmp(arch, "arm64"))
> > + fixup_kernel_start = etmq->etm->kernel_start -
> > + ARM64_PRE_START_SIZE;
> > + else if (!strcmp(arch, "arm"))
> > + fixup_kernel_start = etmq->etm->kernel_start -
> > + ARM_PRE_START_SIZE;
>
> I will test your work but from a quick look wouldn't it be better to
> have a single define name here? From looking at the modifications you
> did to Makefile.config there doesn't seem to be a reason to have two.
Thanks for suggestion. I changed to use single define
ARM_PRE_START_SIZE and sent patch v2 [1].
If possible, please test patch v2.
Thanks,
Leo Yan
[1] https://lore.kernel.org/linux-arm-kernel/20190620005428.20883-1-leo.yan@linaro.org/T/#u
> > +
> > + if (address >= fixup_kernel_start) {
> > if (machine__is_host(machine))
> > return PERF_RECORD_MISC_KERNEL;
> > else
> > --
> > 2.17.1
> >
^ permalink raw reply
* [net-next:master 1038/1074] drivers/net//ethernet/cadence/macb_main.c:4003:21: error: variable 'fu540_c000_ops' has initializer but incomplete type
From: kbuild test robot @ 2019-06-20 0:59 UTC (permalink / raw)
To: Yash Shah; +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 5393 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: 6c9bef32c6e742e847fc28c9bf7721cbfa003fa6
commit: c218ad559020ff98f5d7de6c8bca05b635581777 [1038/1074] macb: Add support for SiFive FU540-C000
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout c218ad559020ff98f5d7de6c8bca05b635581777
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net//ethernet/cadence/macb_main.c:48:16: error: field 'hw' has incomplete type
struct clk_hw hw;
^~
>> drivers/net//ethernet/cadence/macb_main.c:4003:21: error: variable 'fu540_c000_ops' has initializer but incomplete type
static const struct clk_ops fu540_c000_ops = {
^~~~~~~
>> drivers/net//ethernet/cadence/macb_main.c:4004:3: error: 'const struct clk_ops' has no member named 'recalc_rate'
.recalc_rate = fu540_macb_tx_recalc_rate,
^~~~~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4004:17: warning: excess elements in struct initializer
.recalc_rate = fu540_macb_tx_recalc_rate,
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4004:17: note: (near initialization for 'fu540_c000_ops')
>> drivers/net//ethernet/cadence/macb_main.c:4005:3: error: 'const struct clk_ops' has no member named 'round_rate'
.round_rate = fu540_macb_tx_round_rate,
^~~~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4005:16: warning: excess elements in struct initializer
.round_rate = fu540_macb_tx_round_rate,
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4005:16: note: (near initialization for 'fu540_c000_ops')
>> drivers/net//ethernet/cadence/macb_main.c:4006:3: error: 'const struct clk_ops' has no member named 'set_rate'
.set_rate = fu540_macb_tx_set_rate,
^~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4006:14: warning: excess elements in struct initializer
.set_rate = fu540_macb_tx_set_rate,
^~~~~~~~~~~~~~~~~~~~~~
drivers/net//ethernet/cadence/macb_main.c:4006:14: note: (near initialization for 'fu540_c000_ops')
drivers/net//ethernet/cadence/macb_main.c: In function 'fu540_c000_clk_init':
>> drivers/net//ethernet/cadence/macb_main.c:4013:23: error: storage size of 'init' isn't known
struct clk_init_data init;
^~~~
>> drivers/net//ethernet/cadence/macb_main.c:4032:12: error: implicit declaration of function 'clk_register'; did you mean 'sock_register'? [-Werror=implicit-function-declaration]
*tx_clk = clk_register(NULL, &mgmt->hw);
^~~~~~~~~~~~
sock_register
drivers/net//ethernet/cadence/macb_main.c:4013:23: warning: unused variable 'init' [-Wunused-variable]
struct clk_init_data init;
^~~~
drivers/net//ethernet/cadence/macb_main.c: In function 'macb_probe':
>> drivers/net//ethernet/cadence/macb_main.c:4366:2: error: implicit declaration of function 'clk_unregister'; did you mean 'sock_unregister'? [-Werror=implicit-function-declaration]
clk_unregister(tx_clk);
^~~~~~~~~~~~~~
sock_unregister
drivers/net//ethernet/cadence/macb_main.c: At top level:
>> drivers/net//ethernet/cadence/macb_main.c:4003:29: error: storage size of 'fu540_c000_ops' isn't known
static const struct clk_ops fu540_c000_ops = {
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/fu540_c000_ops +4003 drivers/net//ethernet/cadence/macb_main.c
4002
> 4003 static const struct clk_ops fu540_c000_ops = {
> 4004 .recalc_rate = fu540_macb_tx_recalc_rate,
> 4005 .round_rate = fu540_macb_tx_round_rate,
> 4006 .set_rate = fu540_macb_tx_set_rate,
4007 };
4008
4009 static int fu540_c000_clk_init(struct platform_device *pdev, struct clk **pclk,
4010 struct clk **hclk, struct clk **tx_clk,
4011 struct clk **rx_clk, struct clk **tsu_clk)
4012 {
> 4013 struct clk_init_data init;
4014 int err = 0;
4015
4016 err = macb_clk_init(pdev, pclk, hclk, tx_clk, rx_clk, tsu_clk);
4017 if (err)
4018 return err;
4019
4020 mgmt = devm_kzalloc(&pdev->dev, sizeof(*mgmt), GFP_KERNEL);
4021 if (!mgmt)
4022 return -ENOMEM;
4023
4024 init.name = "sifive-gemgxl-mgmt";
4025 init.ops = &fu540_c000_ops;
4026 init.flags = 0;
4027 init.num_parents = 0;
4028
4029 mgmt->rate = 0;
4030 mgmt->hw.init = &init;
4031
> 4032 *tx_clk = clk_register(NULL, &mgmt->hw);
4033 if (IS_ERR(*tx_clk))
4034 return PTR_ERR(*tx_clk);
4035
4036 err = clk_prepare_enable(*tx_clk);
4037 if (err)
4038 dev_err(&pdev->dev, "failed to enable tx_clk (%u)\n", err);
4039 else
4040 dev_info(&pdev->dev, "Registered clk switch '%s'\n", init.name);
4041
4042 return 0;
4043 }
4044
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54085 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/7] Hexdump Enhancements
From: Alastair D'Silva @ 2019-06-20 1:14 UTC (permalink / raw)
To: Joe Perches
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <d8316be322f33ea67640ff83f2248fe433078407.camel@perches.com>
On Wed, 2019-06-19 at 17:35 -0700, Joe Perches wrote:
> On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote:
> > On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote:
> > > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > >
> > > > Apologies for the large CC list, it's a heads up for those
> > > > responsible
> > > > for subsystems where a prototype change in generic code causes
> > > > a
> > > > change
> > > > in those subsystems.
> > > >
> > > > This series enhances hexdump.
> > >
> > > Still not a fan of these patches.
> >
> > I'm afraid there's not too much action I can take on that, I'm
> > happy to
> > address specific issues though.
> >
> > > > These improve the readability of the dumped data in certain
> > > > situations
> > > > (eg. wide terminals are available, many lines of empty bytes
> > > > exist,
> > > > etc).
>
> I think it's generally overkill for the desired uses.
I understand where you're coming from, however, these patches make it a
lot easier to work with large chucks of binary data. I think it makes
more sense to have these patches upstream, even though committed code
may not necessarily have all the features enabled, as it means that
devs won't have to apply out-of-tree patches during development to make
larger dumps manageable.
>
> > > Changing hexdump's last argument from bool to int is odd.
> > >
> >
> > Think of it as replacing a single boolean with many booleans.
>
> I understand it. It's odd.
>
> I would rather not have a mixture of true, false, and apparently
> random collections of bitfields like 0xd or 0b1011 or their
> equivalent or'd defines.
>
Where's the mixture? What would you propose instead?
--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org
^ permalink raw reply
* [PATCH nf-next v2 2/2] netfilter: nft_meta: Add NFT_META_BRI_VLAN support
From: wenxu @ 2019-06-20 1:17 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
In-Reply-To: <1560993460-25569-1-git-send-email-wenxu@ucloud.cn>
From: wenxu <wenxu@ucloud.cn>
nft add table bridge firewall
nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
As above set the bridge port with pvid, the received packet don't contain
the vlan tag which means the packet should belong to vlan 200 through pvid.
With this pacth user can set the pvid in the prerouting hook before set zone
id and conntrack.
So add the following rule for as the first rule in the chain of zones.
nft add rule bridge firewall zones counter meta brvlan set meta brpvid
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nft_meta.c | 19 +++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 4a16124..7be0307 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -794,6 +794,7 @@ enum nft_exthdr_attributes {
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_BRI_PVID: packet input bridge port pvid
+ * @NFT_META_BRI_VLAN: set vlan tag on packet
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -825,6 +826,7 @@ enum nft_meta_keys {
NFT_META_IIFKIND,
NFT_META_OIFKIND,
NFT_META_BRI_PVID,
+ NFT_META_BRI_VLAN,
};
/**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index cb877e01..f30d11b 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -278,8 +278,13 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
{
const struct nft_meta *meta = nft_expr_priv(expr);
struct sk_buff *skb = pkt->skb;
+ const struct net_device *in = nft_in(pkt);
u32 *sreg = ®s->data[meta->sreg];
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ const struct net_bridge_port *p;
+#endif
u32 value = *sreg;
+ u16 value16;
u8 value8;
switch (meta->key) {
@@ -302,6 +307,13 @@ static void nft_meta_set_eval(const struct nft_expr *expr,
skb->nf_trace = !!value8;
break;
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ case NFT_META_BRI_VLAN:
+ value16 = nft_reg_load16(sreg);
+ if (in && (p = br_port_get_rtnl_rcu(in)))
+ __vlan_hwaccel_put_tag(skb, p->br->vlan_proto, value16);
+ break;
+#endif
#ifdef CONFIG_NETWORK_SECMARK
case NFT_META_SECMARK:
skb->secmark = value;
@@ -477,6 +489,13 @@ static int nft_meta_set_init(const struct nft_ctx *ctx,
case NFT_META_PKTTYPE:
len = sizeof(u8);
break;
+#ifdef CONFIG_NF_TABLES_BRIDGE
+ case NFT_META_BRI_VLAN:
+ if (ctx->family != NFPROTO_BRIDGE)
+ return -EOPNOTSUPP;
+ len = sizeof(u16);
+ break;
+#endif
default:
return -EOPNOTSUPP;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH nf-next v2 1/2] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: wenxu @ 2019-06-20 1:17 UTC (permalink / raw)
To: pablo, fw; +Cc: netfilter-devel, netdev
From: wenxu <wenxu@ucloud.cn>
nft add table bridge firewall
nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
As above set the bridge port with pvid, the received packet don't contain
the vlan tag which means the packet should belong to vlan 200 through pvid.
With this pacth user can get the pvid of bridge ports.
So add the following rule for as the first rule in the chain of zones.
nft add rule bridge firewall zones counter meta brvlan set meta brpvid
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
include/uapi/linux/netfilter/nf_tables.h | 2 ++
net/netfilter/nft_meta.c | 13 +++++++++++++
2 files changed, 15 insertions(+)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 31a6b8f..4a16124 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -793,6 +793,7 @@ enum nft_exthdr_attributes {
* @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp)
* @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
* @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
+ * @NFT_META_BRI_PVID: packet input bridge port pvid
*/
enum nft_meta_keys {
NFT_META_LEN,
@@ -823,6 +824,7 @@ enum nft_meta_keys {
NFT_META_SECPATH,
NFT_META_IIFKIND,
NFT_META_OIFKIND,
+ NFT_META_BRI_PVID,
};
/**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 987d2d6..cb877e01 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -243,6 +243,14 @@ void nft_meta_get_eval(const struct nft_expr *expr,
goto err;
strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
return;
+ case NFT_META_BRI_PVID:
+ if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL)
+ goto err;
+ if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) {
+ nft_reg_store16(dest, br_get_pvid(nbp_vlan_group_rcu(p)));
+ return;
+ }
+ goto err;
#endif
case NFT_META_IIFKIND:
if (in == NULL || in->rtnl_link_ops == NULL)
@@ -370,6 +378,11 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
return -EOPNOTSUPP;
len = IFNAMSIZ;
break;
+ case NFT_META_BRI_PVID:
+ if (ctx->family != NFPROTO_BRIDGE)
+ return -EOPNOTSUPP;
+ len = sizeof(u16);
+ break;
#endif
default:
return -EOPNOTSUPP;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/2 nf-next] netfilter: nft_meta: add NFT_META_BRI_PVID support
From: wenxu @ 2019-06-20 1:19 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: fw, netfilter-devel, netdev
In-Reply-To: <20190619170254.an2aklx6abqh646l@salvia>
I will post the nftable patches latter. Thanks!
在 2019/6/20 1:02, Pablo Neira Ayuso 写道:
> On Wed, Jun 19, 2019 at 03:16:24PM +0800, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> nft add table bridge firewall
>> nft add chain bridge firewall zones { type filter hook prerouting priority - 300 \; }
>> nft add rule bridge firewall zones counter ct zone set vlan id map { 100 : 1, 200 : 2 }
>> As above set the bridge port with pvid, the received packet don't contain
>> the vlan tag which means the packet should belong to vlan 200 through pvid.
>> With this pacth user can get the pvid of bridge ports: "meta brpvid"
> Would you also post the patches for nftables for review?
>
> Would you post an example on how you use "meta brpvid" in your
> ruleset? I don't see this is used in the example above,
>
> More comments below.
>
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> ---
>> include/uapi/linux/netfilter/nf_tables.h | 2 ++
>> net/netfilter/nft_meta.c | 17 +++++++++++++++++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
>> index 31a6b8f..4a16124 100644
>> --- a/include/uapi/linux/netfilter/nf_tables.h
>> +++ b/include/uapi/linux/netfilter/nf_tables.h
>> @@ -793,6 +793,7 @@ enum nft_exthdr_attributes {
>> * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp)
>> * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
>> * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
>> + * @NFT_META_BRI_PVID: packet input bridge port pvid
>> */
>> enum nft_meta_keys {
>> NFT_META_LEN,
>> @@ -823,6 +824,7 @@ enum nft_meta_keys {
>> NFT_META_SECPATH,
>> NFT_META_IIFKIND,
>> NFT_META_OIFKIND,
>> + NFT_META_BRI_PVID,
>> };
>>
>> /**
>> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
>> index 987d2d6..1fdb565 100644
>> --- a/net/netfilter/nft_meta.c
>> +++ b/net/netfilter/nft_meta.c
>> @@ -243,6 +243,18 @@ void nft_meta_get_eval(const struct nft_expr *expr,
>> goto err;
>> strncpy((char *)dest, p->br->dev->name, IFNAMSIZ);
>> return;
>> + case NFT_META_BRI_PVID:
>> + if (in == NULL || (p = br_port_get_rtnl_rcu(in)) == NULL)
>> + goto err;
>> + if (br_opt_get(p->br, BROPT_VLAN_ENABLED)) {
>> + u16 pvid = br_get_pvid(nbp_vlan_group_rcu(p));
>> +
>> + if (pvid) {
>> + nft_reg_store16(dest, pvid);
> I think you should store pvid into dest if pvid is zero too, right?
> You cannot assume destination register is set to zero.
>
>> + return;
>> + }
>> + }
>> + goto err;
>> #endif
>> case NFT_META_IIFKIND:
>> if (in == NULL || in->rtnl_link_ops == NULL)
>> @@ -370,6 +382,11 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
>> return -EOPNOTSUPP;
>> len = IFNAMSIZ;
>> break;
>> + case NFT_META_BRI_PVID:
>> + if (ctx->family != NFPROTO_BRIDGE)
>> + return -EOPNOTSUPP;
>> + len = sizeof(u16);
>> + break;
>> #endif
>> default:
>> return -EOPNOTSUPP;
>> --
>> 1.8.3.1
>>
^ permalink raw reply
* Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver
From: Dan Williams @ 2019-06-20 1:25 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan, Arnd Bergmann
Cc: Johannes Berg, Alex Elder, abhishek.esse, Ben Chan,
Bjorn Andersson, cpratapa, David Miller, DTML, Eric Caruso,
evgreen, Ilias Apalodimas, Linux ARM, linux-arm-msm,
Linux Kernel Mailing List, linux-soc, Networking, syadagir
In-Reply-To: <2926e45fd7ff62fd7c4af9b338bf0caa@codeaurora.org>
On Wed, 2019-06-19 at 12:47 -0600, Subash Abhinov Kasiviswanathan
wrote:
> > > There is a n:1 relationship between rmnet and IPA.
> > > rmnet does the de-muxing to multiple netdevs based on the mux id
> > > in the MAP header for RX packets and vice versa.
> >
> > Oh, so you mean that even though IPA supports multiple channels
> > and multiple netdev instances for a physical device, all the
> > rmnet devices end up being thrown into a single channel in IPA?
> >
> > What are the other channels for in IPA? I understand that there
> > is one channel for commands that is separate, while the others
> > are for network devices, but that seems to make no sense if
> > we only use a single channel for rmnet data.
> >
>
> AFAIK, the other channels are for use cases like tethering.
> There is only a single channel which is used for RX
> data which is then de-muxed using rmnet.
That seems odd, since tethering is no different than any other data
channel in QMI, just that it may have a different APN and QoS
guarantees.
Dan
^ permalink raw reply
* Re: [PATCH] page_pool: fix compile warning when CONFIG_PAGE_POOL is disabled
From: David Miller @ 2019-06-20 1:26 UTC (permalink / raw)
To: brouer; +Cc: netdev
In-Reply-To: <156098255254.17592.13888583206213518228.stgit@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Thu, 20 Jun 2019 00:15:52 +0200
> Kbuild test robot reported compile warning:
> warning: no return statement in function returning non-void
> in function page_pool_request_shutdown, when CONFIG_PAGE_POOL is disabled.
>
> The fix makes the code a little more verbose, with a descriptive variable.
>
> Fixes: 99c07c43c4ea ("xdp: tracking page_pool resources and safe removal")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Reported-by: kbuild test robot <lkp@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call
From: Martin Blumenstingl @ 2019-06-20 1:34 UTC (permalink / raw)
To: Colin Ian King
Cc: alexandre.torgue, davem, joabreu, kernel-janitors,
linux-arm-kernel, linux-kernel, linux-stm32, mcoquelin.stm32,
netdev, peppe.cavallaro
In-Reply-To: <92f9e5a6-d2a2-6bf2-ff8a-2430fe977f93@canonical.com>
Hi Colin,
On Wed, Jun 19, 2019 at 8:55 AM Colin Ian King <colin.king@canonical.com> wrote:
>
> On 19/06/2019 06:13, Martin Blumenstingl wrote:
> > Hi Colin,
> >
> >> Currently the call to device_property_read_u32_array is not error checked
> >> leading to potential garbage values in the delays array that are then used
> >> in msleep delays. Add a sanity check to the property fetching.
> >>
> >> Addresses-Coverity: ("Uninitialized scalar variable")
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > I have also sent a patch [0] to fix initialize the array.
> > can you please look at my patch so we can work out which one to use?
> >
> > my concern is that the "snps,reset-delays-us" property is optional,
> > the current dt-bindings documentation states that it's a required
> > property. in reality it isn't, there are boards (two examples are
> > mentioned in my patch: [0]) without it.
> >
> > so I believe that the resulting behavior has to be:
> > 1. don't delay if this property is missing (instead of delaying for
> > <garbage value> ms)
> > 2. don't error out if this property is missing
> >
> > your patch covers #1, can you please check whether #2 is also covered?
> > I tested case #2 when submitting my patch and it worked fine (even
> > though I could not reproduce the garbage values which are being read
> > on some boards)
> >
> >
> > Thank you!
> > Martin
> >
> >
> > [0] https://lkml.org/lkml/2019/4/19/638
> >
> Is that the correct link?
sorry, that is a totally unrelated link
the correct link is: https://patchwork.ozlabs.org/patch/1118313/
^ permalink raw reply
* Re: [PATCH v3 0/7] Hexdump Enhancements
From: Joe Perches @ 2019-06-20 2:00 UTC (permalink / raw)
To: Alastair D'Silva
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <9456ca2a4ae827635bb6d864e5095a9e51f2ac45.camel@d-silva.org>
On Thu, 2019-06-20 at 11:14 +1000, Alastair D'Silva wrote:
> On Wed, 2019-06-19 at 17:35 -0700, Joe Perches wrote:
> > On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote:
> > > On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote:
> > > > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> > > > > From: Alastair D'Silva <alastair@d-silva.org>
> > > > >
> > > > > Apologies for the large CC list, it's a heads up for those
> > > > > responsible
> > > > > for subsystems where a prototype change in generic code causes
> > > > > a
> > > > > change
> > > > > in those subsystems.
> > > > >
> > > > > This series enhances hexdump.
> > > >
> > > > Still not a fan of these patches.
> > >
> > > I'm afraid there's not too much action I can take on that, I'm
> > > happy to
> > > address specific issues though.
> > >
> > > > > These improve the readability of the dumped data in certain
> > > > > situations
> > > > > (eg. wide terminals are available, many lines of empty bytes
> > > > > exist,
> > > > > etc).
> >
> > I think it's generally overkill for the desired uses.
>
> I understand where you're coming from, however, these patches make it a
> lot easier to work with large chucks of binary data. I think it makes
> more sense to have these patches upstream, even though committed code
> may not necessarily have all the features enabled, as it means that
> devs won't have to apply out-of-tree patches during development to make
> larger dumps manageable.
>
> > > > Changing hexdump's last argument from bool to int is odd.
> > > >
> > >
> > > Think of it as replacing a single boolean with many booleans.
> >
> > I understand it. It's odd.
> >
> > I would rather not have a mixture of true, false, and apparently
> > random collections of bitfields like 0xd or 0b1011 or their
> > equivalent or'd defines.
> >
>
> Where's the mixture? What would you propose instead?
create a hex_dump_to_buffer_ext with a new argument
and a new static inline for the old hex_dump_to_buffer
without modifying the argument list that calls
hex_dump_to_buffer with whatever added argument content
you need.
Something like:
static inline
int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
bool ascii)
{
return hex_dump_to_buffer_ext(buf, len, rowsize, groupsize,
linebuf, linebuflen, ascii, 0);
}
and remove EXPORT_SYMBOL(hex_dump_to_buffer)
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the rdma tree
From: Stephen Rothwell @ 2019-06-20 2:15 UTC (permalink / raw)
To: David Miller, Networking, Doug Ledford, Jason Gunthorpe
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Florian Westphal
[-- Attachment #1: Type: text/plain, Size: 724 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
drivers/infiniband/hw/nes/nes.c
between commit:
2d3c72ed5041 ("rdma: Remove nes")
from the rdma tree and commit:
2638eb8b50cf ("net: ipv4: provide __rcu annotation for ifa_list")
from the net-next tree.
I fixed it up (I removed the file) and can carry the fix as necessary.
This is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in p54u_load_firmware_cb
From: syzbot @ 2019-06-20 2:20 UTC (permalink / raw)
To: andreyknvl, chunkeey, davem, kvalo, linux-kernel, linux-usb,
linux-wireless, netdev, syzkaller-bugs
In-Reply-To: <00000000000001de810588363aaf@google.com>
syzbot has found a reproducer for the following crash on:
HEAD commit: 9939f56e usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=135e29faa00000
kernel config: https://syzkaller.appspot.com/x/.config?x=df134eda130bb43a
dashboard link: https://syzkaller.appspot.com/bug?extid=6d237e74cdc13f036473
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=175d946ea00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6d237e74cdc13f036473@syzkaller.appspotmail.com
usb 3-1: Direct firmware load for isl3887usb failed with error -2
usb 3-1: Firmware not found.
==================================================================
BUG: KASAN: slab-out-of-bounds in p54u_load_firmware_cb.cold+0x97/0x13d
drivers/net/wireless/intersil/p54/p54usb.c:936
Read of size 8 at addr ffff8881c9cf7588 by task kworker/1:5/2759
CPU: 1 PID: 2759 Comm: kworker/1:5 Not tainted 5.2.0-rc5+ #11
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: events request_firmware_work_func
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xca/0x13e lib/dump_stack.c:113
print_address_description+0x67/0x231 mm/kasan/report.c:188
__kasan_report.cold+0x1a/0x32 mm/kasan/report.c:317
kasan_report+0xe/0x20 mm/kasan/common.c:614
p54u_load_firmware_cb.cold+0x97/0x13d
drivers/net/wireless/intersil/p54/p54usb.c:936
request_firmware_work_func+0x126/0x242
drivers/base/firmware_loader/main.c:785
process_one_work+0x905/0x1570 kernel/workqueue.c:2269
worker_thread+0x96/0xe20 kernel/workqueue.c:2415
kthread+0x30b/0x410 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Allocated by task 1612:
save_stack+0x1b/0x80 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:462
kmalloc include/linux/slab.h:547 [inline]
syslog_print kernel/printk/printk.c:1346 [inline]
do_syslog kernel/printk/printk.c:1519 [inline]
do_syslog+0x4f4/0x12e0 kernel/printk/printk.c:1493
kmsg_read+0x8a/0xb0 fs/proc/kmsg.c:40
proc_reg_read+0x1c1/0x280 fs/proc/inode.c:221
__vfs_read+0x76/0x100 fs/read_write.c:425
vfs_read+0x18e/0x3d0 fs/read_write.c:461
ksys_read+0x127/0x250 fs/read_write.c:587
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 1612:
save_stack+0x1b/0x80 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:451
slab_free_hook mm/slub.c:1421 [inline]
slab_free_freelist_hook mm/slub.c:1448 [inline]
slab_free mm/slub.c:2994 [inline]
kfree+0xd7/0x280 mm/slub.c:3949
syslog_print kernel/printk/printk.c:1405 [inline]
do_syslog kernel/printk/printk.c:1519 [inline]
do_syslog+0xff3/0x12e0 kernel/printk/printk.c:1493
kmsg_read+0x8a/0xb0 fs/proc/kmsg.c:40
proc_reg_read+0x1c1/0x280 fs/proc/inode.c:221
__vfs_read+0x76/0x100 fs/read_write.c:425
vfs_read+0x18e/0x3d0 fs/read_write.c:461
ksys_read+0x127/0x250 fs/read_write.c:587
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8881c9cf7180
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 8 bytes to the right of
1024-byte region [ffff8881c9cf7180, ffff8881c9cf7580)
The buggy address belongs to the page:
page:ffffea0007273d00 refcount:1 mapcount:0 mapping:ffff8881dac02a00
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000200 ffff8881dac02a00
raw: 0000000000000000 00000000000e000e 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881c9cf7480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881c9cf7500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881c9cf7580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff8881c9cf7600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881c9cf7680: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
^ permalink raw reply
* [PATCH bpf-next] selftests: Add test for veth native XDP
From: Toshiaki Makita @ 2019-06-20 2:23 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend
Cc: Toshiaki Makita, netdev, xdp-newbies, bpf
Add a test case for veth native XDP. It checks if XDP_PASS, XDP_TX and
XDP_REDIRECT work properly.
$ cd tools/testing/selftests/bpf
$ make \
TEST_CUSTOM_PROGS= \
TEST_GEN_PROGS= \
TEST_GEN_PROGS_EXTENDED= \
TEST_PROGS_EXTENDED= \
TEST_PROGS="test_xdp_veth.sh" \
run_tests
TAP version 13
1..1
# selftests: bpf: test_xdp_veth.sh
# PING 10.1.1.33 (10.1.1.33) 56(84) bytes of data.
# 64 bytes from 10.1.1.33: icmp_seq=1 ttl=64 time=0.073 ms
#
# --- 10.1.1.33 ping statistics ---
# 1 packets transmitted, 1 received, 0% packet loss, time 0ms
# rtt min/avg/max/mdev = 0.073/0.073/0.073/0.000 ms
# selftests: xdp_veth [PASS]
ok 1 selftests: bpf: test_xdp_veth.sh
Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../testing/selftests/bpf/progs/xdp_redirect_map.c | 31 ++++++
tools/testing/selftests/bpf/progs/xdp_tx.c | 12 +++
tools/testing/selftests/bpf/test_xdp_veth.sh | 118 +++++++++++++++++++++
4 files changed, 162 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/xdp_redirect_map.c
create mode 100644 tools/testing/selftests/bpf/progs/xdp_tx.c
create mode 100755 tools/testing/selftests/bpf/test_xdp_veth.sh
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 44fb61f..11128ba 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -46,6 +46,7 @@ TEST_PROGS := test_kmod.sh \
test_libbpf.sh \
test_xdp_redirect.sh \
test_xdp_meta.sh \
+ test_xdp_veth.sh \
test_offload.py \
test_sock_addr.sh \
test_tunnel.sh \
diff --git a/tools/testing/selftests/bpf/progs/xdp_redirect_map.c b/tools/testing/selftests/bpf/progs/xdp_redirect_map.c
new file mode 100644
index 0000000..e87a985
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_redirect_map.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") tx_port = {
+ .type = BPF_MAP_TYPE_DEVMAP,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .max_entries = 8,
+};
+
+SEC("redirect_map_0")
+int xdp_redirect_map_0(struct xdp_md *xdp)
+{
+ return bpf_redirect_map(&tx_port, 0, 0);
+}
+
+SEC("redirect_map_1")
+int xdp_redirect_map_1(struct xdp_md *xdp)
+{
+ return bpf_redirect_map(&tx_port, 1, 0);
+}
+
+SEC("redirect_map_2")
+int xdp_redirect_map_2(struct xdp_md *xdp)
+{
+ return bpf_redirect_map(&tx_port, 2, 0);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/xdp_tx.c b/tools/testing/selftests/bpf/progs/xdp_tx.c
new file mode 100644
index 0000000..57912e7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xdp_tx.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+SEC("tx")
+int xdp_tx(struct xdp_md *xdp)
+{
+ return XDP_TX;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_xdp_veth.sh b/tools/testing/selftests/bpf/test_xdp_veth.sh
new file mode 100755
index 0000000..ba8ffcd
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_xdp_veth.sh
@@ -0,0 +1,118 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Create 3 namespaces with 3 veth peers, and
+# forward packets in-between using native XDP
+#
+# XDP_TX
+# NS1(veth11) NS2(veth22) NS3(veth33)
+# | | |
+# | | |
+# (veth1, (veth2, (veth3,
+# id:111) id:122) id:133)
+# ^ | ^ | ^ |
+# | | XDP_REDIRECT | | XDP_REDIRECT | |
+# | ------------------ ------------------ |
+# -----------------------------------------
+# XDP_REDIRECT
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+TESTNAME=xdp_veth
+BPF_FS=$(awk '$3 == "bpf" {print $2; exit}' /proc/mounts)
+BPF_DIR=$BPF_FS/test_$TESTNAME
+
+_cleanup()
+{
+ set +e
+ ip link del veth1 2> /dev/null
+ ip link del veth2 2> /dev/null
+ ip link del veth3 2> /dev/null
+ ip netns del ns1 2> /dev/null
+ ip netns del ns2 2> /dev/null
+ ip netns del ns3 2> /dev/null
+ rm -rf $BPF_DIR 2> /dev/null
+}
+
+cleanup_skip()
+{
+ echo "selftests: $TESTNAME [SKIP]"
+ _cleanup
+
+ exit $ksft_skip
+}
+
+cleanup()
+{
+ if [ "$?" = 0 ]; then
+ echo "selftests: $TESTNAME [PASS]"
+ else
+ echo "selftests: $TESTNAME [FAILED]"
+ fi
+ _cleanup
+}
+
+if [ $(id -u) -ne 0 ]; then
+ echo "selftests: $TESTNAME [SKIP] Need root privileges"
+ exit $ksft_skip
+fi
+
+if ! ip link set dev lo xdp off > /dev/null 2>&1; then
+ echo "selftests: $TESTNAME [SKIP] Could not run test without the ip xdp support"
+ exit $ksft_skip
+fi
+
+if [ -z "$BPF_FS" ]; then
+ echo "selftests: $TESTNAME [SKIP] Could not run test without bpffs mounted"
+ exit $ksft_skip
+fi
+
+if ! bpftool version > /dev/null 2>&1; then
+ echo "selftests: $TESTNAME [SKIP] Could not run test without bpftool"
+ exit $ksft_skip
+fi
+
+set -e
+
+trap cleanup_skip EXIT
+
+ip netns add ns1
+ip netns add ns2
+ip netns add ns3
+
+ip link add veth1 index 111 type veth peer name veth11 netns ns1
+ip link add veth2 index 122 type veth peer name veth22 netns ns2
+ip link add veth3 index 133 type veth peer name veth33 netns ns3
+
+ip link set veth1 up
+ip link set veth2 up
+ip link set veth3 up
+
+ip -n ns1 addr add 10.1.1.11/24 dev veth11
+ip -n ns3 addr add 10.1.1.33/24 dev veth33
+
+ip -n ns1 link set dev veth11 up
+ip -n ns2 link set dev veth22 up
+ip -n ns3 link set dev veth33 up
+
+mkdir $BPF_DIR
+bpftool prog loadall \
+ xdp_redirect_map.o $BPF_DIR/progs type xdp \
+ pinmaps $BPF_DIR/maps
+bpftool map update pinned $BPF_DIR/maps/tx_port key 0 0 0 0 value 122 0 0 0
+bpftool map update pinned $BPF_DIR/maps/tx_port key 1 0 0 0 value 133 0 0 0
+bpftool map update pinned $BPF_DIR/maps/tx_port key 2 0 0 0 value 111 0 0 0
+ip link set dev veth1 xdp pinned $BPF_DIR/progs/redirect_map_0
+ip link set dev veth2 xdp pinned $BPF_DIR/progs/redirect_map_1
+ip link set dev veth3 xdp pinned $BPF_DIR/progs/redirect_map_2
+
+ip -n ns1 link set dev veth11 xdp obj xdp_dummy.o sec xdp_dummy
+ip -n ns2 link set dev veth22 xdp obj xdp_tx.o sec tx
+ip -n ns3 link set dev veth33 xdp obj xdp_dummy.o sec xdp_dummy
+
+trap cleanup EXIT
+
+ip netns exec ns1 ping -c 1 -W 1 10.1.1.33
+
+exit 0
--
1.8.3.1
^ permalink raw reply related
* RE: [RFC net-next 1/5] net: stmmac: introduce IEEE 802.1Qbv configuration functionalities
From: Ong, Boon Leong @ 2019-06-20 3:14 UTC (permalink / raw)
To: Andrew Lunn, Voon, Weifeng
Cc: David S. Miller, Maxime Coquelin, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Jose Abreu, Giuseppe Cavallaro,
Florian Fainelli, Alexandre Torgue, Gomes, Vinicius
In-Reply-To: <20190619124433.GC26784@lunn.ch>
>> > It looks like most o the TSN_WARN should actually be netdev_dbg().
>> >
>> > Andrew
>>
>> Hi Andrew,
>> This file is targeted for dual licensing which is GPL-2.0 OR BSD-3-Clause.
>> This is the reason why we are using wrappers around the functions so that
>> all the function call is generic.
>
>I don't see why dual licenses should require wrappers. Please explain.
>
> Thanks
> Andrew
Agree with the Andrew. We can change those wrapper functions that have
serve the internal development needs for multiple OS scaling reason.
We will update the kernel codes as suggsted.
^ permalink raw reply
* Re: [PATCH v3 0/4] Ethernet PHY reset GPIO updates for Amlogic SoCs
From: Kevin Hilman @ 2019-06-20 3:35 UTC (permalink / raw)
To: Martin Blumenstingl, linux-amlogic
Cc: linux-kernel, linux-arm-kernel, netdev, linus.walleij, andrew,
robin.murphy, narmstrong, Martin Blumenstingl
In-Reply-To: <20190615103832.5126-1-martin.blumenstingl@googlemail.com>
Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> While trying to add the Ethernet PHY interrupt on the X96 Max I found
> that the current reset line definition is incorrect. Patch #1 fixes
> this.
>
> Since the fix requires moving from the deprecated "snps,reset-gpio"
> property to the generic Ethernet PHY reset bindings I decided to move
> all Amlogic boards over to the non-deprecated bindings. That's what
> patches #2 and #3 do.
>
> Finally I found that Odroid-N2 doesn't define the Ethernet PHY's reset
> GPIO yet. I don't have that board so I can't test whether it really
> works but based on the schematics it should.
>
> This series is a partial successor to "stmmac: honor the GPIO flags
> for the PHY reset GPIO" from [0]. I decided not to take Linus W.'s
> Reviewed-by from patch #4 of that series because I had to change the
> wording and I want to be sure that he's happy with that now.
>
> One quick note regarding patches #1 and #4: I decided to violate the
> "max 80 characters per line" (by 4 characters) limit because I find
> that the result is easier to read then it would be if I split the
> line.
>
>
> Changes since v1 at [1]:
> - fixed the reset deassert delay for RTL8211F PHYs - spotted by Robin
> Murphy (thank you). according to the public RTL8211E datasheet the
> correct values seem to be: 10ms assert, 30ms deassert
> - fixed the reset assert and deassert delays for IP101GR PHYs. There
> are two values given in the public datasheet, use the higher one
> (10ms instead of 2.5)
> - update the patch descriptions to quote the datasheets (the RTL8211F
> quotes are taken from the public RTL8211E datasheet because as far
> as I can tell the reset sequence is identical on both PHYs)
>
> Changes since v2 at [2]:
> - add Neil's Reviewed/Acked/Tested-by's (thank you!)
> - rebased on top of "arm64: dts: meson-g12a-x96-max: add sound card"
>
>
> [0] https://patchwork.kernel.org/cover/10983801/
> [1] https://patchwork.kernel.org/cover/10985155/
> [2] https://patchwork.kernel.org/cover/10990863/
Queued for v5.3...
> Martin Blumenstingl (4):
> arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY reset line
> ARM: dts: meson: switch to the generic Ethernet PHY reset bindings
...in branch v5.3/dt
> arm64: dts: meson: use the generic Ethernet PHY reset GPIO bindings
> arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY reset line
The other 3 in v5.3/dt64,
Thanks,
Kevin
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/9] bpf: track spill/fill of constants
From: Alexei Starovoitov @ 2019-06-20 3:35 UTC (permalink / raw)
To: John Fastabend
Cc: Alexei Starovoitov, davem, daniel, netdev, bpf, kernel-team
In-Reply-To: <5d0ad24027106_8822adea29a05b47c@john-XPS-13-9370.notmuch>
On Wed, Jun 19, 2019 at 05:24:32PM -0700, John Fastabend wrote:
> Alexei Starovoitov wrote:
> > Compilers often spill induction variables into the stack,
> > hence it is necessary for the verifier to track scalar values
> > of the registers through stack slots.
> >
> > Also few bpf programs were incorrectly rejected in the past,
> > since the verifier was not able to track such constants while
> > they were used to compute offsets into packet headers.
> >
> > Tracking constants through the stack significantly decreases
> > the chances of state pruning, since two different constants
> > are considered to be different by state equivalency.
> > End result that cilium tests suffer serious degradation in the number
> > of states processed and corresponding verification time increase.
> >
> > before after
> > bpf_lb-DLB_L3.o 1838 6441
> > bpf_lb-DLB_L4.o 3218 5908
> > bpf_lb-DUNKNOWN.o 1064 1064
> > bpf_lxc-DDROP_ALL.o 26935 93790
> > bpf_lxc-DUNKNOWN.o 34439 123886
> > bpf_netdev.o 9721 31413
> > bpf_overlay.o 6184 18561
> > bpf_lxc_jit.o 39389 359445
> >
> > After further debugging turned out that cillium progs are
> > getting hurt by clang due to the same constant tracking issue.
> > Newer clang generates better code by spilling less to the stack.
> > Instead it keeps more constants in the registers which
> > hurts state pruning since the verifier already tracks constants
> > in the registers:
> > old clang new clang
> > (no spill/fill tracking introduced by this patch)
> > bpf_lb-DLB_L3.o 1838 1923
> > bpf_lb-DLB_L4.o 3218 3077
> > bpf_lb-DUNKNOWN.o 1064 1062
> > bpf_lxc-DDROP_ALL.o 26935 166729
> > bpf_lxc-DUNKNOWN.o 34439 174607
> ^^^^^^^^^^^^^^
> Any idea what happened here? Going from 34439 -> 174607 on the new clang?
As I was alluding in commit log newer clang is smarter and generates
less spill/fill of constants.
In particular older clang loads two constants into r8 and r9
and immediately spills them into stack. Then fills later,
does a bunch of unrelated code and calls into helper that
has ARG_ANYTHING for that position. Then doing a bit more math
on filled constants, spills them again and so on.
Before this patch (that tracks spill/fill of constants into stack)
pruning points were equivalent, but with the patch it sees the difference
in registers and declares states not equivalent, though any constant
is fine from safety standpoint.
With new clang only r9 has this pattern of spill/fill.
New clang manages to keep constant in r8 to be around without spill/fill.
Existing verifier tracks constants so even without this patch
the same pathalogical behavior is observed.
The verifier need to walk a lot more instructions only because
r8 has different constants.
> > bpf_netdev.o 9721 8407
> > bpf_overlay.o 6184 5420
> > bpf_lcx_jit.o 39389 39389
> >
> > The final table is depressing:
> > old clang old clang new clang new clang
> > const spill/fill const spill/fill
> > bpf_lb-DLB_L3.o 1838 6441 1923 8128
> > bpf_lb-DLB_L4.o 3218 5908 3077 6707
> > bpf_lb-DUNKNOWN.o 1064 1064 1062 1062
> > bpf_lxc-DDROP_ALL.o 26935 93790 166729 380712
> > bpf_lxc-DUNKNOWN.o 34439 123886 174607 440652
> > bpf_netdev.o 9721 31413 8407 31904
> > bpf_overlay.o 6184 18561 5420 23569
> > bpf_lxc_jit.o 39389 359445 39389 359445
> >
> > Tracking constants in the registers hurts state pruning already.
> > Adding tracking of constants through stack hurts pruning even more.
> > The later patch address this general constant tracking issue
> > with coarse/precise logic.
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > kernel/bpf/verifier.c | 90 +++++++++++++++++++++++++++++++------------
> > 1 file changed, 65 insertions(+), 25 deletions(-)
>
> I know these are already in bpf-next sorry it took me awhile to get
> time to review, but looks good to me. Thanks! We had something similar
> in the earlier loop test branch from last year.
It's not in bpf-next yet :)
Code reviews are appreciated at any time.
Looks like we were just lucky with older clang.
I haven't tracked which clang version became smarter.
If you haven't seen this issue and haven't changed cilium C source
to workaround that then there is chance you'll hit it as well.
By "new clang" I meant version 9.0
"old clang" is unknown. I just had cilium elf .o around that
I kept using for testing without recompiling them.
Just by chance I recompiled them to see annotated verifier line info
messages with BTF and hit this interesting issue.
See patch 9 backtracking logic that resolves this 'precision of scalar'
issue for progs compiled with both new and old clangs.
^ permalink raw reply
* RE: [RFC net-next 1/5] net: stmmac: introduce IEEE 802.1Qbv configuration functionalities
From: Ong, Boon Leong @ 2019-06-20 3:37 UTC (permalink / raw)
To: Gomes, Vinicius, Voon, Weifeng, David S. Miller, Maxime Coquelin
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Jose Abreu,
Giuseppe Cavallaro, Andrew Lunn, Florian Fainelli,
Alexandre Torgue, Voon, Weifeng
In-Reply-To: <874l4lsaue.fsf@intel.com>
>-----Original Message-----
>From: Gomes, Vinicius
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dw_tsn_lib.c
>> @@ -0,0 +1,790 @@
>> +
>> +static struct tsn_hw_cap dw_tsn_hwcap;
>> +static bool dw_tsn_feat_en[TSN_FEAT_ID_MAX];
>> +static unsigned int dw_tsn_hwtunable[TSN_HWTUNA_MAX];
>> +static struct est_gc_config dw_est_gc_config;
>
>If it's at all possible to have more than one of these devices in a
>system, this should be moved to a per-device structure. That
>mac_device_info struct perhaps?
I do see value in scaling the code to more than one device there.
Thanks.
>> +void dwmac_tsn_init(void *ioaddr)
>
>Perhaps this should return an error if TSN is not supported. It may help
>simplify the initialization below.
Thanks for the input. It may not be apparent because this code does not
include Qbu detection yet. The thinking here is to avoid caller function
not need to handle and IP configuration difference, i.e. SoC-1 may have only
Qbv and SoC-2 have both.
>
>> +{
>> + unsigned int hwid = TSN_RD32(ioaddr + GMAC4_VERSION) &
>TSN_VER_MASK;
>> + unsigned int hw_cap2 = TSN_RD32(ioaddr + GMAC_HW_FEATURE2);
>> + unsigned int hw_cap3 = TSN_RD32(ioaddr + GMAC_HW_FEATURE3);
>> + struct tsn_hw_cap *cap = &dw_tsn_hwcap;
>> + unsigned int gcl_depth;
>> + unsigned int tils_max;
>> + unsigned int ti_wid;
>> +
>> + memset(cap, 0, sizeof(*cap));
>> +
>> + if (hwid < TSN_CORE_VER) {
>> + TSN_WARN_NA("IP v5.00 does not support TSN\n");
Perhaps, we just print info here instead of warning because SoC with EQoS v5
can be built without Qbv.
>> + return;
>> + }
>> +
>> + if (!(hw_cap3 & GMAC_HW_FEAT_ESTSEL)) {
>> + TSN_WARN_NA("EST NOT supported\n");
>> + cap->est_support = 0;
Same here.
>> +
>> + return;
>> + }
>> +
>> + gcl_depth = est_get_gcl_depth(hw_cap3);
>> + ti_wid = est_get_ti_width(hw_cap3);
>> +
>> + cap->ti_wid = ti_wid;
>> + cap->gcl_depth = gcl_depth;
>> +
>> + tils_max = (hw_cap3 & GMAC_HW_FEAT_ESTSEL ? 3 : 0);
>> + tils_max = (1 << tils_max) - 1;
>> + cap->tils_max = tils_max;
>> +
>> + cap->ext_max = EST_TIWID_TO_EXTMAX(ti_wid);
>> + cap->txqcnt = ((hw_cap2 & GMAC_HW_FEAT_TXQCNT) >> 6) + 1;
>> + cap->est_support = 1;
>> +
>> + TSN_INFO("EST: depth=%u, ti_wid=%u, tils_max=%u tqcnt=%u\n",
>> + gcl_depth, ti_wid, tils_max, cap->txqcnt);
>> +}
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h
>b/drivers/net/ethernet/stmicro/stmmac/hwif.h
>> index 2acfbc70e3c8..518a72805185 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
>> @@ -7,6 +7,7 @@
>>
>> #include <linux/netdevice.h>
>> #include <linux/stmmac.h>
>> +#include "dw_tsn_lib.h"
>>
>> #define stmmac_do_void_callback(__priv, __module, __cname, __arg0,
>__args...) \
>> ({ \
>> @@ -311,6 +312,31 @@ struct stmmac_ops {
>> bool loopback);
>> void (*pcs_rane)(void __iomem *ioaddr, bool restart);
>> void (*pcs_get_adv_lp)(void __iomem *ioaddr, struct rgmii_adv *adv);
>> + /* TSN functions */
>> + void (*tsn_init)(void __iomem *ioaddr);
>> + void (*get_tsn_hwcap)(struct tsn_hw_cap **tsn_hwcap);
>> + void (*set_est_gcb)(struct est_gc_entry *gcl,
>> + u32 bank);
>> + void (*set_tsn_feat)(enum tsn_feat_id featid, bool enable);
>> + int (*set_tsn_hwtunable)(void __iomem *ioaddr,
>> + enum tsn_hwtunable_id id,
>> + const unsigned int *data);
>> + int (*get_tsn_hwtunable)(enum tsn_hwtunable_id id,
>> + unsigned int *data);
>> + int (*get_est_bank)(void __iomem *ioaddr, u32 own);
>> + int (*set_est_gce)(void __iomem *ioaddr,
>> + struct est_gc_entry *gce, u32 row,
>> + u32 dbgb, u32 dbgm);
>> + int (*get_est_gcrr_llr)(void __iomem *ioaddr, u32 *gcl_len,
>> + u32 dbgb, u32 dbgm);
>> + int (*set_est_gcrr_llr)(void __iomem *ioaddr, u32 gcl_len,
>> + u32 dbgb, u32 dbgm);
>> + int (*set_est_gcrr_times)(void __iomem *ioaddr,
>> + struct est_gcrr *gcrr,
>> + u32 dbgb, u32 dbgm);
>> + int (*set_est_enable)(void __iomem *ioaddr, bool enable);
>> + int (*get_est_gcc)(void __iomem *ioaddr,
>> + struct est_gc_config **gcc, bool frmdrv);
>
>These functions do not seem to be consistent with the rest of the
>stmmac_ops: most of the operations already there receive an
>mac_device_info as first argument, which seem much less error prone than
>a void* ioaddr.
Thanks for the input. We will look into this together with mac_device_info
and adjust accordingly.
^ 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