* Re: [PATCH 1/5] net: dsa: fix a leaked reference by adding a missing of_node_put
From: Linus Walleij @ 2019-02-22 10:38 UTC (permalink / raw)
To: Wen Yang
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel@vger.kernel.org, Alexandre Belloni,
UNGLinuxDriver, Felix Fietkau, Lorenzo Bianconi, Kalle Valo,
Matthias Brugger, linux-wireless, Linux ARM,
moderated list:ARM/Mediatek SoC support, anirudh, John Linn,
Michal Simek, wang.yi59
In-Reply-To: <1550819742-32155-1-git-send-email-wen.yang99@zte.com.cn>
On Fri, Feb 22, 2019 at 8:15 AM Wen Yang <wen.yang99@zte.com.cn> wrote:
> The call to of_get_child_by_name returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./drivers/net/dsa/rtl8366rb.c:510:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:518:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:540:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:548:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:556:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:561:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Vivien Didelot <vivien.didelot@gmail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH] selftests: fib_tests: sleep after changing carrier. again.
From: Thadeu Lima de Souza Cascardo @ 2019-02-22 10:27 UTC (permalink / raw)
To: netdev; +Cc: linux-kselftest, Thadeu Lima de Souza Cascardo, David Ahern
Just like commit e2ba732a1681 ("selftests: fib_tests: sleep after
changing carrier"), wait one second to allow linkwatch to propagate the
carrier change to the stack.
There are two sets of carrier tests. The first slept after the carrier
was set to off, and when the second set ran, it was likely that the
linkwatch would be able to run again without much delay, reducing the
likelihood of a race. However, if you run 'fib_tests.sh -t carrier' on a
loop, you will quickly notice the failures.
Sleeping on the second set of tests make the failures go away.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
---
tools/testing/selftests/net/fib_tests.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 802b4af18729..1080ff55a788 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -388,6 +388,7 @@ fib_carrier_unicast_test()
set -e
$IP link set dev dummy0 carrier off
+ sleep 1
set +e
echo " Carrier down"
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next v2] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: Nikolay Aleksandrov @ 2019-02-22 10:26 UTC (permalink / raw)
To: wenxu, davem; +Cc: netdev
In-Reply-To: <1550827209-23440-1-git-send-email-wenxu@ucloud.cn>
On 22/02/2019 11:20, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> Current fib_multipath_hash_policy can make hash based on the L3 or
> L4. But it only work on the outer IP. So a specific tunnel always
> has the same hash value. But a specific tunnel may contain so many
> inner connections. However there is no good way for tunnel packet.
> A specific tunnel route based on the percpu dst_cache. It will not
> lookup route table for each packet.
>
> This patch provide a based cpu id hash policy. The different
> connection run on different cpu and there will be different hash
> value for percpu dst_cache.
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> net/ipv4/route.c | 6 ++++++
> net/ipv4/sysctl_net_ipv4.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
Hi,
When we had the same issue in the bonding, we added a new mode which used
the flow dissector to get the inner headers and hash on them.
I believe that is even easier nowadays, but I think people wanted to
use different hash algorithms as well and last we discussed this we
were talking about yet another bpf use to generate the hash.
Now we got bpf_set_hash() which can be used to achieve this from various
places, if you use that with hash_policy=1 (L4) then skb->hash will
be used and you can achieve any hashing that you desire.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-22 10:13 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190221163627.7b8aa2ce@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index b63bc77af2d1..629661db36ee 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -7527,6 +7527,12 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
>> prog->dst_needed = 1;
>> if (insn->imm == BPF_FUNC_get_prandom_u32)
>> bpf_user_rnd_init_once();
>> + if (insn->imm == BPF_FUNC_redirect) {
>> + int err = dev_map_alloc_default_map();
>> +
>> + if (err)
>> + return err;
>> + }
>> if (insn->imm == BPF_FUNC_override_return)
>> prog->kprobe_override = 1;
>> if (insn->imm == BPF_FUNC_tail_call) {
>
>> +int dev_map_alloc_default_map(void)
>> +{
>> + struct net *net = current->nsproxy->net_ns;
>> + struct bpf_dtab *dtab, *old_dtab;
>> + struct net_device *netdev;
>> + union bpf_attr attr = {};
>> + u32 idx;
>> + int err;
>
> BPF programs don't obey by netns boundaries. The fact the program is
> verified in one ns doesn't mean this is the only ns it will be used in :(
> Meaning if any program is using the redirect map you may need a secret
> map in every ns.. no?
Ah, yes, good point. Totally didn't think about the fact that load and
attach are decoupled. Hmm, guess I'll just have to move the call to
alloc_default_map() to the point where the program is attached to an
interface, then...
I trust it's safe to skip the allocation in case the program is
offloaded to hardware, right? I.e., an offloaded program will never need
to fall back to the kernel helper?
-Toke
^ permalink raw reply
* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Michal Kubecek @ 2019-02-22 10:04 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190221174620.12144-5-jakub.kicinski@netronome.com>
On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> Support getting devlink instance from a new NDO.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> drivers/net/ethernet/netronome/nfp/nfp_app.h | 2 ++
> drivers/net/ethernet/netronome/nfp/nfp_devlink.c | 11 +++++++++++
> drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 1 +
> drivers/net/ethernet/netronome/nfp/nfp_net_repr.c | 1 +
> 4 files changed, 15 insertions(+)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> index d578d856a009..f8d422713705 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> struct nfp_net *nn, unsigned int id);
>
> +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> +
> #endif
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> index db2da99f6aa7..e9eca99cf493 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> {
> devlink_port_unregister(&port->dl_port);
> }
> +
> +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> +{
> + struct nfp_app *app;
> +
> + app = nfp_app_from_netdev(netdev);
> + if (!app)
> + return NULL;
> +
> + return priv_to_devlink(app->pf);
> +}
AFAICS this would return a pointer to zero initialized struct devlink
when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
would execute
if (!dev->netdev_ops->ndo_get_devlink)
return;
devlink = dev->netdev_ops->ndo_get_devlink(dev);
if (!devlink || !devlink->ops->info_get)
return;
with non-null devlink but null devlink->ops so that it dereferences null
pointer (and so does devlink_compat_flash_update()).
Maybe it would be safer not to call ndo_get_devlink directly and have
an inline wrapper like
#if IS_ENABLED(CONFIG_NET_DEVLINK)
static inline struct devlink *dev_get_devlink(struct net_device *dev)
{
if (dev->netdev_ops->ndo_get_devlink)
return dev->netdev_ops->ndo_get_devlink();
else
retrurn NULL;
}
#else
static inline struct devlink *dev_get_devlink(struct net_device *dev)
{
return NULL;
}
#endif
so that one can simply call the wrapper and check return value for NULL.
Michal
^ permalink raw reply
* Re: [PATCH] sctp: don't compare hb_timer expire date before starting it
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2019-02-22 10:03 UTC (permalink / raw)
To: kwiecienmaciek@gmail.com, linux-sctp@vger.kernel.org
Cc: netdev@vger.kernel.org, Kwiecien, Maciej (Nokia - PL/Wroclaw)
In-Reply-To: <20190222084526.8214-1-maciej.kwiecien@nokia.com>
On 22/02/2019 09:45, kwiecienmaciek@gmail.com wrote:
> From: Maciej Kwiecien <maciej.kwiecien@nokia.com>
>
> hb_timer might not start at all for a particular transport because its
> start is conditional. In a result a node is not sending heartbeats.
>
> Function sctp_transport_reset_hb_timer has two roles:
> - initial start of hb_timer for a given transport,
> - update expire date of hb_timer for a given transport.
> The function is optimized to update timer's expire only if it is before
> a new calculated one but this comparison is invalid for a timer which
> has not yet started. Such a timer has expire == 0 and if a new expire
> value is bigger than (MAX_JIFFIES / 2 + 2) then "time_before" macro will
> fail and timer will not start resulting in no heartbeat packets send by
> the node.
>
> This was found when association was initialized within first 5 mins
> after system boot due to jiffies init value which is near to MAX_JIFFIES.
>
> Test kernel version: 4.9.154 (ARCH=arm)
> hb_timer.expire = 0; //initialized, not started timer
> new_expire = MAX_JIFFIES / 2 + 2; //or more
> time_before(hb_timer.expire, new_expire) == false
>
> Fixes: ba6f5e33bdbb ("sctp: avoid refreshing heartbeat timer too often")
> Reported-by: Marcin Stojek <marcin.stojek@nokia.com>
> Tested-by: Marcin Stojek <marcin.stojek@nokia.com>
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> Signed-off-by: Maciej Kwiecien <maciej.kwiecien@nokia.com>
> ---
> net/sctp/transport.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index 033696e6f74f..ad158d311ffa 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -207,7 +207,8 @@ void sctp_transport_reset_hb_timer(struct sctp_transport *transport)
>
> /* When a data chunk is sent, reset the heartbeat interval. */
> expires = jiffies + sctp_transport_timeout(transport);
> - if (time_before(transport->hb_timer.expires, expires) &&
> + if ((time_before(transport->hb_timer.expires, expires) ||
> + !timer_pending(&transport->hb_timer)) &&
> !mod_timer(&transport->hb_timer,
> expires + prandom_u32_max(transport->rto)))
> sctp_transport_hold(transport);
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply
* Re: [PATCH net-next 3/5] devlink: create a special NDO for getting the devlink instance
From: Michal Kubecek @ 2019-02-22 9:51 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190221174620.12144-4-jakub.kicinski@netronome.com>
On Thu, Feb 21, 2019 at 09:46:18AM -0800, Jakub Kicinski wrote:
> Instead of iterating over all devlink ports add a NDO which
> will return the devlink instance from the driver.
>
> Suggested-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> include/linux/netdevice.h | 6 ++++
> net/core/devlink.c | 62 +++++++++++++--------------------------
> 2 files changed, 26 insertions(+), 42 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index aab4d9f6613d..eebcef6b8191 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -940,6 +940,8 @@ struct dev_ifalias {
> char ifalias[];
> };
>
> +struct devlink;
> +
> /*
> * This structure defines the management hooks for network devices.
> * The following hooks can be defined; unless noted otherwise, they are
> @@ -1248,6 +1250,9 @@ struct dev_ifalias {
> * that got dropped are freed/returned via xdp_return_frame().
> * Returns negative number, means general error invoking ndo, meaning
> * no frames were xmit'ed and core-caller will free all frames.
> + * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
> + * Get devlink instance associated with a given netdev.
> + * Called with a reference on the device only, rtnl_lock is not held.
The formulation is a bit ambiguous. Do I understand correctly that what
it says is that device reference is sufficient and rtnl_lock is not
necessary (but there is no harm if caller holds rtnl_lock)?
Michal
^ permalink raw reply
* [PATCH 3/3] selftests/bpf: add tests for bpf_sk_check_syncookie
From: Lorenz Bauer @ 2019-02-22 9:50 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: linux-api, Lorenz Bauer
In-Reply-To: <20190222095057.9442-1-lmb@cloudflare.com>
Add tests which verify that the new helper works for both IPv4 and
IPv6, by forcing SYN cookies to always on. Use a new network namespace
to avoid clobbering the global SYN cookie settings.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 5 +-
tools/testing/selftests/bpf/bpf_helpers.h | 3 +
.../bpf/progs/test_sk_syncookie_kern.c | 119 ++++++++++
.../selftests/bpf/test_sk_syncookie.sh | 81 +++++++
.../selftests/bpf/test_sk_syncookie_user.c | 212 ++++++++++++++++++
6 files changed, 419 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_sk_syncookie_kern.c
create mode 100755 tools/testing/selftests/bpf/test_sk_syncookie.sh
create mode 100644 tools/testing/selftests/bpf/test_sk_syncookie_user.c
diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index e47168d1257d..44b2853b8534 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -29,4 +29,5 @@ test_netcnt
test_section_names
test_tcpnotify_user
test_libbpf
+test_sk_syncookie_user
alu32
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ccffaa0a0787..4ad1f7a915b3 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -51,7 +51,8 @@ TEST_PROGS := test_kmod.sh \
test_skb_cgroup_id.sh \
test_flow_dissector.sh \
test_xdp_vlan.sh \
- test_lwt_ip_encap.sh
+ test_lwt_ip_encap.sh \
+ test_sk_syncookie.sh
TEST_PROGS_EXTENDED := with_addr.sh \
with_tunnels.sh \
@@ -60,7 +61,7 @@ TEST_PROGS_EXTENDED := with_addr.sh \
# Compile but not part of 'make run_tests'
TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user \
- flow_dissector_load test_flow_dissector
+ flow_dissector_load test_flow_dissector test_sk_syncookie_user
include ../lib.mk
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index d9999f1ed1d2..b80cec7af445 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -166,6 +166,9 @@ static struct bpf_sock *(*bpf_sk_lookup_udp)(void *ctx,
(void *) BPF_FUNC_sk_lookup_udp;
static int (*bpf_sk_release)(struct bpf_sock *sk) =
(void *) BPF_FUNC_sk_release;
+static int (*bpf_sk_check_syncookie)(struct bpf_sock *sk,
+ void *ip, int ip_len, void *tcp, int tcp_len) =
+ (void *) BPF_FUNC_sk_check_syncookie;
static int (*bpf_skb_vlan_push)(void *ctx, __be16 vlan_proto, __u16 vlan_tci) =
(void *) BPF_FUNC_skb_vlan_push;
static int (*bpf_skb_vlan_pop)(void *ctx) =
diff --git a/tools/testing/selftests/bpf/progs/test_sk_syncookie_kern.c b/tools/testing/selftests/bpf/progs/test_sk_syncookie_kern.c
new file mode 100644
index 000000000000..4918457efc1e
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_sk_syncookie_kern.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+// Copyright (c) 2019 Cloudflare
+
+#include <string.h>
+
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <linux/if_ether.h>
+#include <linux/in.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <sys/socket.h>
+#include <linux/tcp.h>
+
+#include "bpf_helpers.h"
+#include "bpf_endian.h"
+
+struct bpf_map_def SEC("maps") results = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(__u64),
+ .max_entries = 1,
+};
+
+static __always_inline void check_syncookie(void *ctx, void *data,
+ void *data_end)
+{
+ struct bpf_sock_tuple tup;
+ struct bpf_sock *sk;
+ struct ethhdr *ethh;
+ struct iphdr *ipv4h;
+ struct ipv6hdr *ipv6h;
+ struct tcphdr *tcph;
+ int ret;
+ __u32 key = 0;
+ __u64 value = 1;
+
+ ethh = data;
+ if (ethh + 1 > data_end)
+ return;
+
+ switch (bpf_ntohs(ethh->h_proto)) {
+ case ETH_P_IP:
+ ipv4h = data + sizeof(struct ethhdr);
+ if (ipv4h + 1 > data_end)
+ return;
+
+ tcph = data + sizeof(struct ethhdr) + sizeof(struct iphdr);
+ if (tcph + 1 > data_end)
+ return;
+
+ tup.ipv4.saddr = ipv4h->saddr;
+ tup.ipv4.daddr = ipv4h->daddr;
+ tup.ipv4.sport = tcph->source;
+ tup.ipv4.dport = tcph->dest;
+
+ sk = bpf_sk_lookup_tcp(ctx, &tup, sizeof(tup.ipv4),
+ BPF_F_CURRENT_NETNS, 0);
+ if (!sk)
+ return;
+
+ ret = bpf_sk_check_syncookie(sk, ipv4h, sizeof(*ipv4h),
+ tcph, sizeof(*tcph));
+ break;
+
+ case ETH_P_IPV6:
+ ipv6h = data + sizeof(struct ethhdr);
+ if (ipv6h + 1 > data_end)
+ return;
+
+ if (ipv6h->nexthdr != IPPROTO_TCP)
+ return;
+
+ tcph = data + sizeof(struct ethhdr) + sizeof(struct ipv6hdr);
+ if (tcph + 1 > data_end)
+ return;
+
+ memcpy(tup.ipv6.saddr, &ipv6h->saddr, sizeof(tup.ipv6.saddr));
+ memcpy(tup.ipv6.daddr, &ipv6h->daddr, sizeof(tup.ipv6.daddr));
+ tup.ipv6.sport = tcph->source;
+ tup.ipv6.dport = tcph->dest;
+
+ sk = bpf_sk_lookup_tcp(ctx, &tup, sizeof(tup.ipv6),
+ BPF_F_CURRENT_NETNS, 0);
+ if (!sk)
+ return;
+
+ ret = bpf_sk_check_syncookie(sk, ipv6h, sizeof(*ipv6h),
+ tcph, sizeof(*tcph));
+ break;
+
+ default:
+ return;
+ }
+
+ if (ret == 0)
+ bpf_map_update_elem(&results, &key, &value, 0);
+
+ bpf_sk_release(sk);
+}
+
+SEC("clsact/check_syncookie")
+int check_syncookie_clsact(struct __sk_buff *skb)
+{
+ check_syncookie(skb, (void *)(long)skb->data,
+ (void *)(long)skb->data_end);
+ return TC_ACT_OK;
+}
+
+SEC("xdp/check_syncookie")
+int check_syncookie_xdp(struct xdp_md *ctx)
+{
+ check_syncookie(ctx, (void *)(long)ctx->data,
+ (void *)(long)ctx->data_end);
+ return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_sk_syncookie.sh b/tools/testing/selftests/bpf/test_sk_syncookie.sh
new file mode 100755
index 000000000000..429ca8c04c5e
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sk_syncookie.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Facebook
+# Copyright (c) 2019 Cloudflare
+
+set -eu
+
+wait_for_ip()
+{
+ local _i
+ printf "Wait for IP %s to become available " "$1"
+ for _i in $(seq ${MAX_PING_TRIES}); do
+ printf "."
+ if ns1_exec ping -c 1 -W 1 "$1" >/dev/null 2>&1; then
+ echo " OK"
+ return
+ fi
+ sleep 1
+ done
+ echo 1>&2 "ERROR: Timeout waiting for test IP to become available."
+ exit 1
+}
+
+get_prog_id()
+{
+ awk '/ id / {sub(/.* id /, "", $0); print($1)}'
+}
+
+ns1_exec()
+{
+ ip netns exec ns1 "$@"
+}
+
+setup()
+{
+ ip netns add ns1
+ ns1_exec ip link set lo up
+
+ ns1_exec sysctl -w net.ipv4.tcp_syncookies=2
+
+ wait_for_ip 127.0.0.1
+ wait_for_ip ::1
+}
+
+cleanup()
+{
+ ip netns del ns1 2>/dev/null || :
+}
+
+main()
+{
+ trap cleanup EXIT 2 3 6 15
+ setup
+
+ printf "Testing clsact..."
+ ns1_exec tc qdisc add dev "${TEST_IF}" clsact
+ ns1_exec tc filter add dev "${TEST_IF}" ingress \
+ bpf obj "${BPF_PROG_OBJ}" sec "${CLSACT_SECTION}" da
+
+ BPF_PROG_ID=$(ns1_exec tc filter show dev "${TEST_IF}" ingress | \
+ get_prog_id)
+ ns1_exec "${PROG}" "${BPF_PROG_ID}"
+ ns1_exec tc qdisc del dev "${TEST_IF}" clsact
+
+ printf "Testing XDP..."
+ ns1_exec ip link set "${TEST_IF}" xdp \
+ object "${BPF_PROG_OBJ}" section "${XDP_SECTION}"
+ BPF_PROG_ID=$(ns1_exec ip link show "${TEST_IF}" | get_prog_id)
+ ns1_exec "${PROG}" "${BPF_PROG_ID}"
+}
+
+DIR=$(dirname $0)
+TEST_IF=lo
+MAX_PING_TRIES=5
+BPF_PROG_OBJ="${DIR}/test_sk_syncookie_kern.o"
+CLSACT_SECTION="clsact/check_syncookie"
+XDP_SECTION="xdp/check_syncookie"
+BPF_PROG_ID=0
+PROG="${DIR}/test_sk_syncookie_user"
+
+main
diff --git a/tools/testing/selftests/bpf/test_sk_syncookie_user.c b/tools/testing/selftests/bpf/test_sk_syncookie_user.c
new file mode 100644
index 000000000000..87829c86c746
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_sk_syncookie_user.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+// Copyright (c) 2019 Cloudflare
+
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "cgroup_helpers.h"
+
+static int start_server(const struct sockaddr *addr, socklen_t len)
+{
+ int fd;
+
+ fd = socket(addr->sa_family, SOCK_STREAM, 0);
+ if (fd == -1) {
+ log_err("Failed to create server socket");
+ goto out;
+ }
+
+ if (bind(fd, addr, len) == -1) {
+ log_err("Failed to bind server socket");
+ goto close_out;
+ }
+
+ if (listen(fd, 128) == -1) {
+ log_err("Failed to listen on server socket");
+ goto close_out;
+ }
+
+ goto out;
+
+close_out:
+ close(fd);
+ fd = -1;
+out:
+ return fd;
+}
+
+static int connect_to_server(int server_fd)
+{
+ struct sockaddr_storage addr;
+ socklen_t len = sizeof(addr);
+ int fd = -1;
+
+ if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
+ log_err("Failed to get server addr");
+ goto out;
+ }
+
+ fd = socket(addr.ss_family, SOCK_STREAM, 0);
+ if (fd == -1) {
+ log_err("Failed to create client socket");
+ goto out;
+ }
+
+ if (connect(fd, (const struct sockaddr *)&addr, len) == -1) {
+ log_err("Fail to connect to server");
+ goto close_out;
+ }
+
+ goto out;
+
+close_out:
+ close(fd);
+ fd = -1;
+out:
+ return fd;
+}
+
+static int get_map_fd_by_prog_id(int prog_id)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ __u32 map_ids[1];
+ int prog_fd = -1;
+ int map_fd = -1;
+
+ prog_fd = bpf_prog_get_fd_by_id(prog_id);
+ if (prog_fd < 0) {
+ log_err("Failed to get fd by prog id %d", prog_id);
+ goto err;
+ }
+
+ info.nr_map_ids = 1;
+ info.map_ids = (__u64)(unsigned long)map_ids;
+
+ if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
+ log_err("Failed to get info by prog fd %d", prog_fd);
+ goto err;
+ }
+
+ if (!info.nr_map_ids) {
+ log_err("No maps found for prog fd %d", prog_fd);
+ goto err;
+ }
+
+ map_fd = bpf_map_get_fd_by_id(map_ids[0]);
+ if (map_fd < 0)
+ log_err("Failed to get fd by map id %d", map_ids[0]);
+err:
+ if (prog_fd >= 0)
+ close(prog_fd);
+ return map_fd;
+}
+
+static int run_test(int server_fd, int results_fd)
+{
+ int client = -1, srv_client = -1;
+ int ret = 0;
+ __u32 key = 0;
+ __u64 value = 0;
+
+ if (bpf_map_update_elem(results_fd, &key, &value, 0) < 0) {
+ log_err("Can't clear results");
+ goto err;
+ }
+
+ client = connect_to_server(server_fd);
+ if (client == -1)
+ goto err;
+
+ srv_client = accept(server_fd, NULL, 0);
+ if (srv_client == -1) {
+ log_err("Can't accept connection");
+ goto err;
+ }
+
+ if (bpf_map_lookup_elem(results_fd, &key, &value) < 0) {
+ log_err("Can't lookup result");
+ goto err;
+ }
+
+ if (value != 1) {
+ log_err("Didn't match syncookie: %llu", value);
+ goto err;
+ }
+
+ goto out;
+
+err:
+ ret = 1;
+out:
+ close(client);
+ close(srv_client);
+ return ret;
+}
+
+int main(int argc, char **argv)
+{
+ struct sockaddr_in addr4;
+ struct sockaddr_in6 addr6;
+ int server = -1;
+ int server_v6 = -1;
+ int results = -1;
+ int err = 0;
+
+ if (argc < 2) {
+ fprintf(stderr, "Usage: %s prog_id\n", argv[0]);
+ exit(1);
+ }
+
+ results = get_map_fd_by_prog_id(atoi(argv[1]));
+ if (results < 0) {
+ log_err("Can't get map");
+ goto err;
+ }
+
+ memset(&addr4, 0, sizeof(addr4));
+ addr4.sin_family = AF_INET;
+ addr4.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+ addr4.sin_port = 0;
+
+ memset(&addr6, 0, sizeof(addr6));
+ addr6.sin6_family = AF_INET6;
+ addr6.sin6_addr = in6addr_loopback;
+ addr6.sin6_port = 0;
+
+ server = start_server((const struct sockaddr *)&addr4, sizeof(addr4));
+ if (server == -1)
+ goto err;
+
+ server_v6 = start_server((const struct sockaddr *)&addr6,
+ sizeof(addr6));
+ if (server_v6 == -1)
+ goto err;
+
+ if (run_test(server, results))
+ goto err;
+
+ if (run_test(server_v6, results))
+ goto err;
+
+ printf("ok\n");
+ goto out;
+err:
+ err = 1;
+out:
+ close(server);
+ close(server_v6);
+ close(results);
+ return err;
+}
--
2.19.1
^ permalink raw reply related
* [PATCH 2/3] tools: sync changes to uapi/linux/bpf.h
From: Lorenz Bauer @ 2019-02-22 9:50 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: linux-api, Lorenz Bauer
In-Reply-To: <20190222095057.9442-1-lmb@cloudflare.com>
Pull in definitions for bpf_sk_check_syncookie.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
tools/include/uapi/linux/bpf.h | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..bc2af87e9621 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2359,6 +2359,21 @@ union bpf_attr {
* Return
* A **struct bpf_tcp_sock** pointer on success, or NULL in
* case of failure.
+ *
+ * int bpf_sk_check_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
+ * Description
+ * Check whether iph and th contain a valid SYN cookie ACK for
+ * the listening socket in sk.
+ *
+ * iph points to the start of the IPv4 or IPv6 header, while
+ * iph_len contains sizeof(struct iphdr) or sizeof(struct ip6hdr).
+ *
+ * th points to the start of the TCP header, while th_len contains
+ * sizeof(struct tcphdr).
+ *
+ * Return
+ * 0 if iph and th are a valid SYN cookie ACK, or a negative error
+ * otherwise.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2457,7 +2472,8 @@ union bpf_attr {
FN(spin_lock), \
FN(spin_unlock), \
FN(sk_fullsock), \
- FN(tcp_sock),
+ FN(tcp_sock), \
+ FN(sk_check_syncookie),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
--
2.19.1
^ permalink raw reply related
* [PATCH 1/3] bpf: add helper to check for a valid SYN cookie
From: Lorenz Bauer @ 2019-02-22 9:50 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: linux-api, Lorenz Bauer
In-Reply-To: <20190222095057.9442-1-lmb@cloudflare.com>
Using bpf_sk_lookup_tcp it's possible to ascertain whether a packet belongs
to a known connection. However, there is one corner case: no sockets are
created if SYN cookies are active. This means that the final ACK in the
3WHS is misclassified.
Using the helper, we can look up the listening socket via bpf_sk_lookup_tcp
and then check whether a packet is a valid SYN cookie ACK.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
include/uapi/linux/bpf.h | 18 ++++++++++-
net/core/filter.c | 68 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..bc2af87e9621 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2359,6 +2359,21 @@ union bpf_attr {
* Return
* A **struct bpf_tcp_sock** pointer on success, or NULL in
* case of failure.
+ *
+ * int bpf_sk_check_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
+ * Description
+ * Check whether iph and th contain a valid SYN cookie ACK for
+ * the listening socket in sk.
+ *
+ * iph points to the start of the IPv4 or IPv6 header, while
+ * iph_len contains sizeof(struct iphdr) or sizeof(struct ip6hdr).
+ *
+ * th points to the start of the TCP header, while th_len contains
+ * sizeof(struct tcphdr).
+ *
+ * Return
+ * 0 if iph and th are a valid SYN cookie ACK, or a negative error
+ * otherwise.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -2457,7 +2472,8 @@ union bpf_attr {
FN(spin_lock), \
FN(spin_unlock), \
FN(sk_fullsock), \
- FN(tcp_sock),
+ FN(tcp_sock), \
+ FN(sk_check_syncookie),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 85749f6ec789..9e68897cc7ed 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5426,6 +5426,70 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
.arg1_type = ARG_PTR_TO_SOCK_COMMON,
};
+BPF_CALL_5(bpf_sk_check_syncookie, struct sock *, sk, void *, iph, u32, iph_len,
+ struct tcphdr *, th, u32, th_len)
+{
+#if IS_ENABLED(CONFIG_SYN_COOKIES)
+ u32 cookie;
+ int ret;
+
+ if (unlikely(th_len < sizeof(*th)))
+ return -EINVAL;
+
+ /* sk_listener() allows TCP_NEW_SYN_RECV, which makes no sense here. */
+ if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
+ return -EINVAL;
+
+ if (!sock_net(sk)->ipv4.sysctl_tcp_syncookies)
+ return -EINVAL;
+
+ if (!th->ack || th->rst)
+ return -ENOENT;
+
+ cookie = ntohl(th->ack_seq) - 1;
+
+ switch (sk->sk_family) {
+ case AF_INET:
+ if (unlikely(iph_len < sizeof(struct iphdr)))
+ return -EINVAL;
+
+ ret = __cookie_v4_check((struct iphdr *)iph, th, cookie);
+ break;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ case AF_INET6:
+ if (unlikely(iph_len < sizeof(struct ipv6hdr)))
+ return -EINVAL;
+
+ ret = __cookie_v6_check((struct ipv6hdr *)iph, th, cookie);
+ break;
+#endif /* CONFIG_IPV6 */
+
+ default:
+ return -EPROTONOSUPPORT;
+ }
+
+ if (ret > 0)
+ return 0;
+
+ return -ENOENT;
+#else
+ return -ENOTSUP;
+#endif
+}
+
+static const struct bpf_func_proto bpf_sk_check_syncookie_proto = {
+ .func = bpf_sk_check_syncookie,
+ .gpl_only = true,
+ .pkt_access = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_SOCKET,
+ .arg2_type = ARG_PTR_TO_MEM,
+ .arg3_type = ARG_CONST_SIZE,
+ .arg4_type = ARG_PTR_TO_MEM,
+ .arg5_type = ARG_CONST_SIZE,
+};
+
#endif /* CONFIG_INET */
bool bpf_helper_changes_pkt_data(void *func)
@@ -5678,6 +5742,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_release_proto;
case BPF_FUNC_tcp_sock:
return &bpf_tcp_sock_proto;
+ case BPF_FUNC_sk_check_syncookie:
+ return &bpf_sk_check_syncookie_proto;
#endif
default:
return bpf_base_func_proto(func_id);
@@ -5713,6 +5779,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_xdp_sk_lookup_tcp_proto;
case BPF_FUNC_sk_release:
return &bpf_sk_release_proto;
+ case BPF_FUNC_sk_check_syncookie:
+ return &bpf_sk_check_syncookie_proto;
#endif
default:
return bpf_base_func_proto(func_id);
--
2.19.1
^ permalink raw reply related
* [PATCH 0/3] Allow checking SYN cookies from XDP and tc cls act
From: Lorenz Bauer @ 2019-02-22 9:50 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: linux-api, Lorenz Bauer
This series adds a helper bpf_sk_check_syncookie, which allows checking
whether a packet is a valid SYN cookie for a given socket. This is
useful for implementing load-balancing approaches like glb-director [1]
or Beamer [2] in pure eBPF.
Specifically, we'd like to replace the functionality of the glb-redirect
kernel module by an XDP program or tc classifier.
1: https://github.com/github/glb-director
2: https://www.usenix.org/conference/nsdi18/presentation/olteanu
Lorenz Bauer (3):
bpf: add helper to check for a valid SYN cookie
tools: sync changes to uapi/linux/bpf.h
selftests/bpf: add tests for bpf_sk_check_syncookie
include/uapi/linux/bpf.h | 18 +-
net/core/filter.c | 68 ++++++
tools/include/uapi/linux/bpf.h | 18 +-
tools/testing/selftests/bpf/.gitignore | 1 +
tools/testing/selftests/bpf/Makefile | 5 +-
tools/testing/selftests/bpf/bpf_helpers.h | 3 +
.../bpf/progs/test_sk_syncookie_kern.c | 119 ++++++++++
.../selftests/bpf/test_sk_syncookie.sh | 81 +++++++
.../selftests/bpf/test_sk_syncookie_user.c | 212 ++++++++++++++++++
9 files changed, 521 insertions(+), 4 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/test_sk_syncookie_kern.c
create mode 100755 tools/testing/selftests/bpf/test_sk_syncookie.sh
create mode 100644 tools/testing/selftests/bpf/test_sk_syncookie_user.c
--
2.19.1
^ permalink raw reply
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-22 9:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190221163218.72905325@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Fri, 22 Feb 2019 00:02:23 +0100, Toke Høiland-Jørgensen wrote:
>> Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>>
>> > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> >> A common pattern when using xdp_redirect_map() is to create a device map
>> >> where the lookup key is simply ifindex. Because device maps are arrays,
>> >> this leaves holes in the map, and the map has to be sized to fit the
>> >> largest ifindex, regardless of how many devices actually are actually
>> >> needed in the map.
>> >>
>> >> This patch adds a second type of device map where the key is interpreted as
>> >> an ifindex and looked up using a hashmap, instead of being used as an array
>> >> index. This leads to maps being densely packed, so they can be smaller.
>> >>
>> >> The default maps used by xdp_redirect() are changed to use the new map
>> >> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
>> >> but instead to 64 total simultaneous interfaces per network namespace. This
>> >> also provides an easy way to compare the performance of devmap and
>> >> devmap_idx:
>> >>
>> >> xdp_redirect_map (devmap): 8394560 pkt/s
>> >> xdp_redirect (devmap_idx): 8179480 pkt/s
>> >>
>> >> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
>> >
>> > Could you share what the ifindex mix was here, to arrive at these
>> > numbers? How does it compare to using an array but not keying with
>> > ifindex?
>>
>> Just the standard set on my test machine; ifindex 1 through 9, except 8
>> in this case. So certainly no more than 1 ifindex in each hash bucket
>> for those numbers.
>
> Oh, I clearly misread your numbers, it's still slower than array, you
> just don't need the size limit.
Yeah, this is not about speeding up devmap, it's about lifting the size
restriction.
>> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> >
>> >> +static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
>> >> + u64 map_flags)
>> >> +{
>> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
>> >> + struct bpf_dtab_netdev *dev, *old_dev;
>> >> + u32 idx = *(u32 *)key;
>> >> + u32 val = *(u32 *)value;
>> >> + u32 bit;
>> >> +
>> >> + if (unlikely(map_flags > BPF_EXIST))
>> >> + return -EINVAL;
>> >> + if (unlikely(map_flags == BPF_NOEXIST))
>> >> + return -EEXIST;
>> >> +
>> >> + old_dev = __dev_map_idx_lookup_elem(map, idx);
>> >> + if (!val) {
>> >> + if (!old_dev)
>> >> + return 0;
>> >
>> > IMHO this is a fairly strange mix of array and hashmap semantics. I
>> > think you should stick to hashmap behaviour AFA flags and
>> > update/delete goes.
>>
>> Yeah, the double book-keeping is a bit strange, but it allows the actual
>> forwarding and flush code to be reused between both types of maps. I
>> think this is worth the slight semantic confusion :)
>
> I'm not sure I was clear, let me try again :) Your get_next_key only
> reports existing indexes if I read the code right, so that's not an
> array - in an array indexes always exist. What follows inserting 0
> should not be equivalent to delete and BPF_NOEXIST should be handled
> appropriately.
Ah, I see what you mean. Yeah, sure, I guess I can restrict deletion to
only working through explicit delete.
I could also add a fail on NOEXIST, but since each index is tied to a
particular value, you can't actually change the contents of each index,
only insert and remove. So why would you ever set that flag?
> Different maps behave differently, I think it's worth trying to limit
> the divergence in how things behave to the basic array and a hashmap
> models when possible.
So I don't actually think of this as a hashmap in the general sense;
after all, you can only store ifindexes in it, and key and value are
tied to one another. So it's an ifindex'ed devmap (which is also why I
named it devmap_idx and not devmap_hash); the fact that it's implemented
as a hashmap is just incidental.
So I guess it's a choice between being consistent with the other devmap
type, or with a general hashmap. I'm not actually sure that the latter
is less surprising? :)
-Toke
^ permalink raw reply
* [PATCH][net-next] ipv6: sanitize RCU usage on fib6_next
From: Li RongQing @ 2019-02-22 9:44 UTC (permalink / raw)
To: netdev
using rcu_assign_pointer when setting, which has a memory
barrier to ensure the initialization is seen first.
using rcu_dereference when dereference this pointer
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/ipv6/ip6_fib.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 6613d8dbb0e5..b73d40d68178 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1143,7 +1143,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct fib6_info *rt,
atomic_inc(&rt->fib6_ref);
rcu_assign_pointer(rt->fib6_node, fn);
- rt->fib6_next = iter->fib6_next;
+ rcu_assign_pointer(rt->fib6_next, iter->fib6_next);
rcu_assign_pointer(*ins, rt);
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
@@ -1761,7 +1761,9 @@ static void fib6_del_route(struct fib6_table *table, struct fib6_node *fn,
RT6_TRACE("fib6_del_route\n");
/* Unlink it */
- *rtp = rt->fib6_next;
+ *rtp = rcu_dereference_protected(rt->fib6_next,
+ lockdep_is_held(&rt->fib6_table->tb6_lock));
+
rt->fib6_node = NULL;
net->ipv6.rt6_stats->fib_rt_entries--;
net->ipv6.rt6_stats->fib_discarded_routes++;
--
2.16.2
^ permalink raw reply related
* [PATCH] cfg80211: reg: Fix use-after-free in call_crda
From: Yue Haibing @ 2019-02-22 9:42 UTC (permalink / raw)
To: johannes, davem; +Cc: linux-kernel, netdev, linux-wireless, YueHaibing
From: YueHaibing <yuehaibing@huawei.com>
KASAN report this:
BUG: KASAN: use-after-free in kobject_uevent_env+0xedb/0xf20 lib/kobject_uevent.c:474
Read of size 8 at addr ffff8881e52d5dc0 by task kworker/0:2/1066
CPU: 0 PID: 1066 Comm: kworker/0:2 Not tainted 5.0.0-rc7+ #45
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
Workqueue: events reg_todo [cfg80211]
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xfa/0x1ce lib/dump_stack.c:113
print_address_description+0x65/0x270 mm/kasan/report.c:187
kasan_report+0x149/0x18d mm/kasan/report.c:317
kobject_uevent_env+0xedb/0xf20 lib/kobject_uevent.c:474
reg_query_database+0x23e/0x390 [cfg80211]
reg_process_hint+0x34a/0xc00 [cfg80211]
reg_todo+0x3f0/0xa80 [cfg80211]
process_one_work+0xc4d/0x1b40 kernel/workqueue.c:2173
worker_thread+0x171/0x1130 kernel/workqueue.c:2319
kthread+0x302/0x3c0 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Allocated by task 3500:
set_track mm/kasan/common.c:85 [inline]
__kasan_kmalloc.constprop.3+0xa0/0xd0 mm/kasan/common.c:496
kmalloc include/linux/slab.h:550 [inline]
kzalloc include/linux/slab.h:740 [inline]
platform_device_alloc+0x2a/0xf0 drivers/base/platform.c:268
platform_device_register_full+0x6d/0x4d0 drivers/base/platform.c:509
regulatory_init+0x13c/0x5e0 [cfg80211]
cfg80211_init+0x7a/0x10f [cfg80211]
do_one_initcall+0xfa/0x5ca init/main.c:887
do_init_module+0x204/0x5f6 kernel/module.c:3460
load_module+0x66b2/0x8570 kernel/module.c:3808
__do_sys_finit_module+0x238/0x2a0 kernel/module.c:3902
do_syscall_64+0x147/0x600 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 2957:
set_track mm/kasan/common.c:85 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/common.c:458
slab_free_hook mm/slub.c:1409 [inline]
slab_free_freelist_hook mm/slub.c:1436 [inline]
slab_free mm/slub.c:2986 [inline]
kfree+0xe1/0x270 mm/slub.c:3938
device_release+0x78/0x200 drivers/base/core.c:919
kobject_cleanup lib/kobject.c:662 [inline]
kobject_release lib/kobject.c:691 [inline]
kref_put include/linux/kref.h:67 [inline]
kobject_put+0x146/0x240 lib/kobject.c:708
put_device+0x1c/0x30 drivers/base/core.c:2060
request_firmware_work_func+0x16d/0x290 drivers/base/firmware_loader/main.c:786
process_one_work+0xc4d/0x1b40 kernel/workqueue.c:2173
worker_thread+0x171/0x1130 kernel/workqueue.c:2319
kthread+0x302/0x3c0 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8881e52d5d80
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 64 bytes inside of
2048-byte region [ffff8881e52d5d80, ffff8881e52d6580)
The buggy address belongs to the page:
page:ffffea000794b400 count:1 mapcount:0 mapping:ffff8881f6c02800 index:0xffff8881e52d3300 compound_mapcount: 0
flags: 0x2fffc0000010200(slab|head)
raw: 02fffc0000010200 ffffea00079d5200 0000000400000004 ffff8881f6c02800
raw: ffff8881e52d3300 00000000800f000e 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881e52d5c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8881e52d5d00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff8881e52d5d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881e52d5e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881e52d5e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
In function reg_query_database, query_regdb_file call
request_firmware_nowait to do request_firmware asynchronously,
which need the caller hold the reference of dev, otherwise it will
do put_device freeing '®_pdev->dev'. After that, call_crda access
the dev will trigger use-after-free bug.
This patch fix this by holding a reference of dev in regulatory_init
after platform_device_register_simple registered successly, which
releasing in platform_device_unregister.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 007f6c5e6eb4 ("cfg80211: support loading regulatory database as firmware file")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/wireless/reg.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index adfa58f..3de568f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -3884,6 +3884,8 @@ int __init regulatory_init(void)
if (IS_ERR(reg_pdev))
return PTR_ERR(reg_pdev);
+ get_device(®_pdev->dev);
+
spin_lock_init(®_requests_lock);
spin_lock_init(®_pending_beacons_lock);
spin_lock_init(®_indoor_lock);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v5 0/5] M_CAN Framework re-write
From: Wolfgang Grandegger @ 2019-02-22 9:38 UTC (permalink / raw)
To: Dan Murphy, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <43e01edf-9b9a-c68d-ec1a-218a8117f59e@grandegger.com>
Hello Dan,
what kernel version is that patch series for. I have problems to apply it!
Wolfgang.
Am 21.02.19 um 17:41 schrieb Wolfgang Grandegger:
> Hello Dan,
>
> I will have a closer look end of this week!
>
> Wolfgang.
>
> Am 21.02.19 um 17:24 schrieb Dan Murphy:
>> Bump
>>
>> On 2/14/19 12:27 PM, Dan Murphy wrote:
>>> Hello
>>>
>>> OK I did not give up on this patch series just got a little preoccupied with
>>> some other kernel work. But here is the update per the comments.
>>>
>>> It should be understood I broke these out for reviewability.
>>> For instance the first patch does not compile on its own as including this
>>> patch should not change the current functionality and it pulls all the io-mapped
>>> code from the m_can base file to a platfrom file.
>>>
>>> The next patch "Migrate the m_can code to use the framework"
>>> is the change to the kernel for the io-mapped conversion from a flat file to use
>>> the framework. Finally the rename patch just renames the m_can_priv to
>>> m_can_classdev. I broke this change out specifically for readability of the
>>> migration patch per comments on the code.
>>>
>>> AFAIC the first 3 patches can all be squashed into a single patch. Or the
>>> first 2 patches in the series can be re-arranged but then m_can functionality is
>>> affected in the migration patch.
>>>
>>> Again the first 3 patches here are all just for readability and review purposes.
>>>
>>> Dan
>>>
>>> Dan Murphy (5):
>>> can: m_can: Create a m_can platform framework
>>> can: m_can: Migrate the m_can code to use the framework
>>> can: m_can: Rename m_can_priv to m_can_classdev
>>> dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
>>> can: tcan4x5x: Add tcan4x5x driver to the kernel
>>>
>>> .../devicetree/bindings/net/can/tcan4x5x.txt | 37 +
>>> drivers/net/can/m_can/Kconfig | 14 +-
>>> drivers/net/can/m_can/Makefile | 2 +
>>> drivers/net/can/m_can/m_can.c | 788 +++++++++---------
>>> drivers/net/can/m_can/m_can.h | 159 ++++
>>> drivers/net/can/m_can/m_can_platform.c | 198 +++++
>>> drivers/net/can/m_can/tcan4x5x.c | 531 ++++++++++++
>>> 7 files changed, 1320 insertions(+), 409 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
>>> create mode 100644 drivers/net/can/m_can/m_can.h
>>> create mode 100644 drivers/net/can/m_can/m_can_platform.c
>>> create mode 100644 drivers/net/can/m_can/tcan4x5x.c
>>>
>>
>>
^ permalink raw reply
* [PATCH net-next v2] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: wenxu @ 2019-02-22 9:20 UTC (permalink / raw)
To: davem; +Cc: netdev
From: wenxu <wenxu@ucloud.cn>
Current fib_multipath_hash_policy can make hash based on the L3 or
L4. But it only work on the outer IP. So a specific tunnel always
has the same hash value. But a specific tunnel may contain so many
inner connections. However there is no good way for tunnel packet.
A specific tunnel route based on the percpu dst_cache. It will not
lookup route table for each packet.
This patch provide a based cpu id hash policy. The different
connection run on different cpu and there will be different hash
value for percpu dst_cache.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
net/ipv4/route.c | 6 ++++++
net/ipv4/sysctl_net_ipv4.c | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ecc12a7..6cf2fd4 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1821,6 +1821,7 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
const struct sk_buff *skb, struct flow_keys *flkeys)
{
struct flow_keys hash_keys;
+ u32 cpu = 0;
u32 mhash;
switch (net->ipv4.sysctl_fib_multipath_hash_policy) {
@@ -1834,6 +1835,8 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
hash_keys.addrs.v4addrs.dst = fl4->daddr;
}
break;
+ case 2:
+ cpu = smp_processor_id() + 1;
case 1:
/* skb is currently provided only when forwarding */
if (skb) {
@@ -1870,6 +1873,9 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
}
mhash = flow_hash_from_keys(&hash_keys);
+ if (cpu)
+ mhash = jhash_2words(mhash, cpu, 0);
+
return mhash >> 1;
}
#endif /* CONFIG_IP_ROUTE_MULTIPATH */
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index ba0fc4b..708bbbb 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -951,7 +951,7 @@ static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write,
.mode = 0644,
.proc_handler = proc_fib_multipath_hash_policy,
.extra1 = &zero,
- .extra2 = &one,
+ .extra2 = &two,
},
#endif
{
--
1.8.3.1
^ permalink raw reply related
* Re: mlx5 stable backport help
From: Or Gerlitz @ 2019-02-22 9:13 UTC (permalink / raw)
To: Saeed Mahameed
Cc: davem@davemloft.net, netdev@vger.kernel.org, Or Gerlitz,
Roi Dayan
In-Reply-To: <7b4945782c33f4dbb3b4538fc2c40af86e7c5ae1.camel@mellanox.com>
On Fri, Feb 22, 2019 at 12:35 AM Saeed Mahameed <saeedm@mellanox.com> wrote:
> On Thu, 2019-02-21 at 12:21 +0200, Or Gerlitz wrote:
> So we all agree that the offending patch is "net/mlx5e: Support tunnel
> encap over tagged Ethernet" even if the issue existed before,
as you said the issue existed before we added support for offloading
tunnels in the presence of vlan on the underlay, it's like this since 4.12
when we introduced support for neigh update in 232c001398ae "net/mlx5e:
Add support to neighbour update flow" which is basically the one to blame/fix
since some code was moved and some code was added (e->route_dev)
backporting the patch without pulling more patches can be done as I sketched
below.
Anyway, we can maybe let it go without backporting, production environments
are typically not changing their source mac in prime time. So this can be seen
more as future proofing.
> in order to fix the issue you will have to port not only this patch but
> the whole series which claimed to fix the issue, so the fixes tag was
> wrong.. this patch on its own is no good.
>> [1] using e->out_dev instead of e->route_dev
>> [2] use the hunks from mlx5e_tc_tun_create_header_ipv4/6() in
>> mlx5e_create_encap_header_ipv4/6()
^ permalink raw reply
* [PATCH][net-next] net: Use RCU_INIT_POINTER() to set sk_wq
From: Li RongQing @ 2019-02-22 9:08 UTC (permalink / raw)
To: netdev
This pointer is RCU protected, so proper primitives should be used.
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/core/sock.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index f4b8b78535f8..f5d82f3fa474 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1865,7 +1865,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
*/
sk_refcnt_debug_inc(newsk);
sk_set_socket(newsk, NULL);
- newsk->sk_wq = NULL;
+ RCU_INIT_POINTER(newsk->sk_wq, NULL);
if (newsk->sk_prot->sockets_allocated)
sk_sockets_allocated_inc(newsk);
@@ -2828,11 +2828,11 @@ void sock_init_data(struct socket *sock, struct sock *sk)
if (sock) {
sk->sk_type = sock->type;
- sk->sk_wq = sock->wq;
+ RCU_INIT_POINTER(sk->sk_wq, sock->wq);
sock->sk = sk;
sk->sk_uid = SOCK_INODE(sock)->i_uid;
} else {
- sk->sk_wq = NULL;
+ RCU_INIT_POINTER(sk->sk_wq, NULL);
sk->sk_uid = make_kuid(sock_net(sk)->user_ns, 0);
}
--
2.16.2
^ permalink raw reply related
* Re: [PATCH v2 iproute2-next 04/11] devlink: Add helper functions for name and value separately
From: Jiri Pirko @ 2019-02-22 8:58 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Aya Levin, David Ahern, netdev, Jiri Pirko, Moshe Shemesh,
Eran Ben Elisha
In-Reply-To: <20190221100211.28cef41a@shemminger-XPS-13-9360>
Thu, Feb 21, 2019 at 07:02:11PM CET, stephen@networkplumber.org wrote:
>On Thu, 21 Feb 2019 15:42:40 +0200
>Aya Levin <ayal@mellanox.com> wrote:
>
>> Add a new helper functions which outputs only values (without name
>> label) for different types: boolean, uint, uint64, string and binary.
>> In addition add a helper function which prints only the name label.
>>
>> Signed-off-by: Aya Levin <ayal@mellanox.com>
>> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> devlink/devlink.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/devlink/devlink.c b/devlink/devlink.c
>> index b073ae020d52..5d69c4f24f29 100644
>> --- a/devlink/devlink.c
>> +++ b/devlink/devlink.c
>> @@ -1588,7 +1588,6 @@ static void pr_out_port_handle_end(struct dl *dl)
>> pr_out("\n");
>> }
>>
>> -
>> static void pr_out_str(struct dl *dl, const char *name, const char *val)
>> {
>> if (dl->json_output) {
>> @@ -1636,6 +1635,71 @@ static void pr_out_u64(struct dl *dl, const char *name, uint64_t val)
>> }
>> }
>>
>> +static void pr_out_bool_value(struct dl *dl, bool value)
>> +{
>> + if (dl->json_output)
>> + jsonw_bool(dl->jw, value);
>> + else
>> + pr_out(" %s", value ? "true" : "false");
>> +}
>> +
>> +static void pr_out_uint_value(struct dl *dl, unsigned int value)
>> +{
>> + if (dl->json_output)
>> + jsonw_uint(dl->jw, value);
>> + else
>> + pr_out(" %u", value);
>> +}
>> +
>> +static void pr_out_uint64_value(struct dl *dl, uint64_t value)
>> +{
>> + if (dl->json_output)
>> + jsonw_u64(dl->jw, value);
>> + else
>> + pr_out(" %lu", value);
>> +}
>> +
>> +static void pr_out_binary_value(struct dl *dl, uint8_t *data, uint32_t len)
>> +{
>> + int i = 1;
>> +
>> + if (dl->json_output)
>> + jsonw_start_array(dl->jw);
>> + else
>> + pr_out("\n");
>> +
>> + while (i < len) {
>> + if (dl->json_output) {
>> + jsonw_printf(dl->jw, "%d", data[i]);
>> + } else {
>> + pr_out(" %02x", data[i]);
>> + if (!(i % 16))
>> + pr_out("\n");
>> + }
>> + i++;
>> + }
>> + if (dl->json_output)
>> + jsonw_end_array(dl->jw);
>> + else if ((i - 1) % 16)
>> + pr_out("\n");
>> +}
>> +
>> +static void pr_out_str_value(struct dl *dl, const char *value)
>> +{
>> + if (dl->json_output)
>> + jsonw_string(dl->jw, value);
>> + else
>> + pr_out(" %s", value);
>> +}
>> +
>> +static void pr_out_name(struct dl *dl, const char *name)
>> +{
>> + if (dl->json_output)
>> + jsonw_name(dl->jw, name);
>> + else
>> + pr_out(" %s:", name);
>> +}
>> +
>> static void pr_out_region_chunk_start(struct dl *dl, uint64_t addr)
>> {
>> if (dl->json_output) {
>
>NAK
>
>Please convert devlink to use existing iproute2 json_print infrasructure
>rather than reinventing it.
That requires whole devlink/devlink.c code change. Could this be done as
a follow-up?
^ permalink raw reply
* Re: [PATCH net-next] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: Sergei Shtylyov @ 2019-02-22 9:06 UTC (permalink / raw)
To: wenxu, davem; +Cc: netdev
In-Reply-To: <1550807549-22720-1-git-send-email-wenxu@ucloud.cn>
Hello!
On 22.02.2019 6:52, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> Current fib_multipath_hash_policy can make hash based on the L3 or
> L4. But it only work on the outer IP. So a specific tunnel always
> has the same hash value. But a specific tunnel may contain so many
> inner connection. However there is no good ways for tunnel packet.
Connections? Way?
> A specific tunnel route based on the percpu dst_cache, It will not
> lookup route table each packet.
For each packet?
> This patch provide a based cpu id hash policy. The different
> connection run on differnt cpu and There will differnet hash
Different. Will be?
> value for percpu dst_cache.
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> net/ipv4/route.c | 6 ++++++
> net/ipv4/sysctl_net_ipv4.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index ecc12a7..6cf2fd4 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
[...]
> @@ -1834,6 +1835,8 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
> hash_keys.addrs.v4addrs.dst = fl4->daddr;
> }
> break;
> + case 2:
> + cpu = smp_processor_id() + 1;
Should be a comment /* fall through */ if this fall thru isn't in error.
> case 1:
> /* skb is currently provided only when forwarding */
> if (skb) {
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 2/5] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: Or Gerlitz @ 2019-02-22 9:06 UTC (permalink / raw)
To: Tonghao Zhang; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <CAMDZJNWrg_q9p8Av-xWwiCRte4XBn3Nd66EXh8EXpQG2gB0-=A@mail.gmail.com>
On Fri, Feb 22, 2019 at 9:49 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>
> On Fri, Feb 22, 2019 at 12:32 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> >
> > On Thu, Feb 21, 2019 at 3:42 PM <xiangxia.m.yue@gmail.com> wrote:
> > >
> > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > >
> > > If we try to offload decapsulation actions to VFs hw, we get the log [1].
> >
> > but the switching was on the tunnel type (if (tunnel_type == [...]) -
> Yes, but we try to offload tc flow to VF device. For example
> the p2p1_0 is VF, but not rep
so this should go to the nic and not esw tc offload code path in en_tc.c
and the nic path (look for parse_tc_nic_actions or a like) doesn't have
a case for the tunnel key action - you should not have got to this code
at all, please look deeper to realize what is going on there, maybe p2p1_0
is a rep? what does ip -d link show gives on it?
^ permalink raw reply
* Re: [PATCH v2] iwlwifi: mvm: Use div_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Arnd Bergmann @ 2019-02-22 8:52 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, linux-wireless, Networking,
LKML
In-Reply-To: <CAKwvOdmCeHxiGPkXXzjwnTtgzEDDjYRE1QBdu7BHeuzCE4YtRA@mail.gmail.com>
On Fri, Feb 22, 2019 at 1:14 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Thu, Feb 21, 2019 at 12:08 AM Nathan Chancellor <natechancellor@gmail.com> wrote:
> One thing I'm curious about, is "why does do_div exist?" When should I
> use do_div vs div_u64 (not div_s64 as is used in this patch)?
I think do_div() is mostly historic, we've had it since the early days
when C compilers were not as good with inline functions. The various
other versions are regular functions, and I tend to prefer them for new
code, but do_div() is widely known and documented, so I have little
hope of it going away any time soon.
Arnd
^ permalink raw reply
* [PATCH v2] bpf: test_bpf: turn of preemption in function __run_once
From: Anders Roxell @ 2019-02-22 8:46 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev, linux-kernel, Anders Roxell, Arnd Bergmann
When running BPF test suite the following splat occurs:
[ 415.930950] test_bpf: #0 TAX jited:0
[ 415.931067] BUG: assuming atomic context at lib/test_bpf.c:6674
[ 415.946169] in_atomic(): 0, irqs_disabled(): 0, pid: 11556, name: modprobe
[ 415.953176] INFO: lockdep is turned off.
[ 415.957207] CPU: 1 PID: 11556 Comm: modprobe Tainted: G W 5.0.0-rc7-next-20190220 #1
[ 415.966328] Hardware name: HiKey Development Board (DT)
[ 415.971592] Call trace:
[ 415.974069] dump_backtrace+0x0/0x160
[ 415.977761] show_stack+0x24/0x30
[ 415.981104] dump_stack+0xc8/0x114
[ 415.984534] __cant_sleep+0xf0/0x108
[ 415.988145] test_bpf_init+0x5e0/0x1000 [test_bpf]
[ 415.992971] do_one_initcall+0x90/0x428
[ 415.996837] do_init_module+0x60/0x1e4
[ 416.000614] load_module+0x1de0/0x1f50
[ 416.004391] __se_sys_finit_module+0xc8/0xe0
[ 416.008691] __arm64_sys_finit_module+0x24/0x30
[ 416.013255] el0_svc_common+0x78/0x130
[ 416.017031] el0_svc_handler+0x38/0x78
[ 416.020806] el0_svc+0x8/0xc
Rework so that preemption is disabled when we loop over function
'BPF_PROG_RUN(...)'.
Commit 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
highlighted the issue.
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
lib/test_bpf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index f3e570722a7e..0845f635f404 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6668,12 +6668,14 @@ static int __run_one(const struct bpf_prog *fp, const void *data,
u64 start, finish;
int ret = 0, i;
+ preempt_disable();
start = ktime_get_ns();
for (i = 0; i < runs; i++)
ret = BPF_PROG_RUN(fp, data);
finish = ktime_get_ns();
+ preempt_enable();
*duration = finish - start;
do_div(*duration, runs);
--
2.20.1
^ permalink raw reply related
* [PATCH] sctp: don't compare hb_timer expire date before starting it
From: kwiecienmaciek @ 2019-02-22 8:45 UTC (permalink / raw)
To: linux-sctp; +Cc: netdev, alexander.sverdlin, Maciej Kwiecien
From: Maciej Kwiecien <maciej.kwiecien@nokia.com>
hb_timer might not start at all for a particular transport because its
start is conditional. In a result a node is not sending heartbeats.
Function sctp_transport_reset_hb_timer has two roles:
- initial start of hb_timer for a given transport,
- update expire date of hb_timer for a given transport.
The function is optimized to update timer's expire only if it is before
a new calculated one but this comparison is invalid for a timer which
has not yet started. Such a timer has expire == 0 and if a new expire
value is bigger than (MAX_JIFFIES / 2 + 2) then "time_before" macro will
fail and timer will not start resulting in no heartbeat packets send by
the node.
This was found when association was initialized within first 5 mins
after system boot due to jiffies init value which is near to MAX_JIFFIES.
Test kernel version: 4.9.154 (ARCH=arm)
hb_timer.expire = 0; //initialized, not started timer
new_expire = MAX_JIFFIES / 2 + 2; //or more
time_before(hb_timer.expire, new_expire) == false
Fixes: ba6f5e33bdbb ("sctp: avoid refreshing heartbeat timer too often")
Reported-by: Marcin Stojek <marcin.stojek@nokia.com>
Tested-by: Marcin Stojek <marcin.stojek@nokia.com>
Signed-off-by: Maciej Kwiecien <maciej.kwiecien@nokia.com>
---
net/sctp/transport.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 033696e6f74f..ad158d311ffa 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -207,7 +207,8 @@ void sctp_transport_reset_hb_timer(struct sctp_transport *transport)
/* When a data chunk is sent, reset the heartbeat interval. */
expires = jiffies + sctp_transport_timeout(transport);
- if (time_before(transport->hb_timer.expires, expires) &&
+ if ((time_before(transport->hb_timer.expires, expires) ||
+ !timer_pending(&transport->hb_timer)) &&
!mod_timer(&transport->hb_timer,
expires + prandom_u32_max(transport->rto)))
sctp_transport_hold(transport);
--
2.14.1
^ permalink raw reply related
* [PATCH V2 4/8] net: ethernet: stmmac: add management of clk_csr property
From: Christophe Roullier @ 2019-02-22 8:28 UTC (permalink / raw)
To: robh, davem, joabreu, mark.rutland, mcoquelin.stm32,
alexandre.torgue, peppe.cavallaro
Cc: linux-stm32, linux-kernel, devicetree, linux-arm-kernel, netdev,
christophe.roullier, andrew
In-Reply-To: <1550824089-19961-1-git-send-email-christophe.roullier@st.com>
In Documentation stmmac.txt there is possibility to
fixed CSR Clock range selection with property clk_csr.
This patch add the management of this property
For example to use it, add in your ethernet node DT:
clk_csr = <3>;
Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 2b800ce..3031f2b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -408,6 +408,9 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
/* Default to phy auto-detection */
plat->phy_addr = -1;
+ /* Get clk_csr from device tree */
+ of_property_read_u32(np, "clk_csr", &plat->clk_csr);
+
/* "snps,phy-addr" is not a standard property. Mark it as deprecated
* and warn of its use. Remove this when phy node support is added.
*/
--
2.7.4
^ 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