* [PATCH net-next 0/2] mv88e6xxx: Avoid false positive Lockdep splats
From: Andrew Lunn @ 2019-02-23 16:43 UTC (permalink / raw)
To: David Miller; +Cc: Vivien Didelot, Russell King, netdev, Andrew Lunn
When acquiring the GPIO interrupt line for the switch, it is possible
to trigger lockdep splats. These are false positives, the mutex is in
a different IRQ descriptor. But fix it anyway, since it could mask
real locking issues.
Andrew Lunn (2):
net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
net: dsa: mv88e6xxx: Release lock while requesting IRQ
drivers/net/dsa/mv88e6xxx/chip.c | 10 ++++++++++
1 file changed, 10 insertions(+)
--
2.20.1
^ permalink raw reply
* Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Stephen Hemminger @ 2019-02-23 16:46 UTC (permalink / raw)
To: Haiyang Zhang
Cc: haiyangz, sashal, linux-hyperv, kys, sthemmin, olaf, vkuznets,
davem, netdev, linux-kernel
In-Reply-To: <20190222182503.12160-1-haiyangz@linuxonhyperv.com>
On Fri, 22 Feb 2019 18:25:03 +0000
Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Incoming packets may have IP header checksum verified by the host.
> They may not have IP header checksum computed after coalescing.
> This patch re-compute the checksum when necessary, otherwise the
> packets may be dropped, because Linux network stack always checks it.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 256adbd044f5..cf4897043e83 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
> schedule_delayed_work(&ndev_ctx->dwork, 0);
> }
>
> +static void netvsc_comp_ipcsum(struct sk_buff *skb)
> +{
> + struct iphdr *iph = (struct iphdr *)skb->data;
Can you use iphdr(skb) here?
> +
> + iph->check = 0;
> + iph->check = ip_fast_csum(iph, iph->ihl);
> +}
> +
> static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> struct netvsc_channel *nvchan)
> {
> @@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> /* skb is already created with CHECKSUM_NONE */
> skb_checksum_none_assert(skb);
>
> - /*
> - * In Linux, the IP checksum is always checked.
> - * Do L4 checksum offload if enabled and present.
> + /* Incoming packets may have IP header checksum verified by the host.
> + * They may not have IP header checksum computed after coalescing.
> + * We compute it here if the flags are set, because on Linux, the IP
> + * checksum is always checked.
> + */
> + if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
> + csum_info->receive.ip_checksum_succeeded &&
> + skb->protocol == htons(ETH_P_IP))
> + netvsc_comp_ipcsum(skb);
Does this still handle for coalesced and non-coalesced packets
which are received with bad IP checksum? My concern is that you are
potentially correcting the checksum for a packet whose received checksum was bad.
> + /* Do L4 checksum offload if enabled and present.
> */
> if (csum_info && (net->features & NETIF_F_RXCSUM)) {
> if (csum_info->receive.tcp_checksum_succeeded ||
^ permalink raw reply
* Fw: [Bug 202651] New: Regression in atlantic.ko between 4.20 and 5.00-rc4 for Aquantia NIC 1d6a:87b1
From: Stephen Hemminger @ 2019-02-23 16:50 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Sat, 23 Feb 2019 09:01:29 +0000
From: bugzilla-daemon@bugzilla.kernel.org
To: stephen@networkplumber.org
Subject: [Bug 202651] New: Regression in atlantic.ko between 4.20 and 5.00-rc4 for Aquantia NIC 1d6a:87b1
https://bugzilla.kernel.org/show_bug.cgi?id=202651
Bug ID: 202651
Summary: Regression in atlantic.ko between 4.20 and 5.00-rc4
for Aquantia NIC 1d6a:87b1
Product: Networking
Version: 2.5
Kernel Version: 5.00-rc4
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: high
Priority: P1
Component: IPV4
Assignee: stephen@networkplumber.org
Reporter: nicholas.johnson@outlook.com.au
Regression: No
Hardware Details:
==============================================================================
VEN:DEV = 1d6a:87b1 "Aquantia Corp. AQC107 NBase-T/IEEE 802.3bz Ethernet
Controller [AQtion]" Specifically, this is the newer AQC107S with "S"
meaning "Secure" and the company could never elaborate on how it is more
secure than the plain AQC107 (different VEN:DEV). This chip is inside
the Promise SANLink3 N1 Thunderbolt 3 10GbE adapter.
Severity:
==============================================================================
Marked high severity because it directly impacts user experience (cannot
get the network to come up on the adapter).
Back Story and Problem:
==============================================================================
In 5.00-rc4, on my Ubuntu desktop I have had extreme problems getting
the connection up (borderline impossible). If I did manage (after
messing with systemd *a lot*) then I got a dodgy connection with a lot
of DNS resolution problems. At first I blamed it on my desktop which had
a habit of corrupting the SSD partition tables on a regular basis, and
hence I assumed that systemd was corrupted or something.
Recently, I tried on my laptop with Arch Linux and 5.00-rc4 and it had
the exact same problem. It does not matter if you manually assign the
IPv4 address, or use DHCPv4.
Just now, on my laptop, I booted into 4.20.3-arch1-1-ARCH kernel from
Arch Linux and it worked straight away.
Expected behaviour:
==============================================================================
NetworkManager (or any tool) should bring the link up successfully.
Reproduction:
==============================================================================
Should be reproducible with a 5.00-rc4 kernel and the 1d6a:87b1 NIC. I
have no way of testing if other Aquantia devices are affected - I do not
own any others. I am using Ubuntu and Arch (both the very latest) and I
just cannot get NetworkManager to bring up the connection with 5.00-rc4
(happens effortlessly with 4.20 kernel). If you want NetworkManager to
manage the adapter, you may need to use the
10-globally-managed-devices.conf fix, depending on the version.
Technical Notes:
==============================================================================
It is hard to tell, but my gut tells me that it is IPv4-related. That
would explain why some sites could work. I have a native IPv6 service
from my ISP. The "ifconfig" command shows IPv6 addresses assigned, but
it is very difficult to get a DHCPv4-assigned address on the adapter.
A couple of times I did manage to get the adapter up in 5.00-rc4 with a
lot of messing around, and the resulting internet connection was
unreliable and unstable. DNS resolution seemed to be a big part of the
problem.
All BARs are assigned correctly. Other than that, dmesg does not give
anything interesting that I can tell, so if you want some debug logs
from somewhere then please tell me what you want and how I should do it.
There are a fair few lines of diff between 4.20 and 5.00-rc4 so there is
likely an oopsie in there.
Always Remembering My Manners:
==============================================================================
Thank you for reading my bug report!
Why Are Web Developers So Annoying?
==============================================================================
The preview removes all my newlines and nice formatting which I did so have fun
trying to read this big blob of unformatted text.
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* RE: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets
From: Haiyang Zhang @ 2019-02-23 17:29 UTC (permalink / raw)
To: Stephen Hemminger, Haiyang Zhang
Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org, KY Srinivasan,
Stephen Hemminger, olaf@aepfle.de, vkuznets, davem@davemloft.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190223084624.0d0c60a0@shemminger-XPS-13-9360>
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Saturday, February 23, 2019 11:46 AM
> To: Haiyang Zhang <haiyangz@linuxonhyperv.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>; sashal@kernel.org; linux-
> hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; davem@davemloft.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for
> coalesced packets
>
> On Fri, 22 Feb 2019 18:25:03 +0000
> Haiyang Zhang <haiyangz@linuxonhyperv.com> wrote:
>
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > Incoming packets may have IP header checksum verified by the host.
> > They may not have IP header checksum computed after coalescing.
> > This patch re-compute the checksum when necessary, otherwise the
> > packets may be dropped, because Linux network stack always checks it.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c index 256adbd044f5..cf4897043e83
> > 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device
> *net,
> > schedule_delayed_work(&ndev_ctx->dwork, 0); }
> >
> > +static void netvsc_comp_ipcsum(struct sk_buff *skb) {
> > + struct iphdr *iph = (struct iphdr *)skb->data;
>
> Can you use iphdr(skb) here?
This skb is just allocated by netvsc, the skb->network_header is not set yet.
>
> > +
> > + iph->check = 0;
> > + iph->check = ip_fast_csum(iph, iph->ihl); }
> > +
> > static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> > struct netvsc_channel *nvchan)
> { @@ -770,9 +778,17 @@
> > static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
> > /* skb is already created with CHECKSUM_NONE */
> > skb_checksum_none_assert(skb);
> >
> > - /*
> > - * In Linux, the IP checksum is always checked.
> > - * Do L4 checksum offload if enabled and present.
> > + /* Incoming packets may have IP header checksum verified by the
> host.
> > + * They may not have IP header checksum computed after coalescing.
> > + * We compute it here if the flags are set, because on Linux, the IP
> > + * checksum is always checked.
> > + */
> > + if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
> > + csum_info->receive.ip_checksum_succeeded &&
> > + skb->protocol == htons(ETH_P_IP))
> > + netvsc_comp_ipcsum(skb);
>
> Does this still handle for coalesced and non-coalesced packets which are
> received with bad IP checksum? My concern is that you are potentially
> correcting the checksum for a packet whose received checksum was bad.
Windows networking team told me that the flags above indicate host side
already verified the checksum. Online doc is here:
https://docs.microsoft.com/en-us/windows-hardware/drivers/network/indicating-coalesced-segments
If the NIC or miniport driver validates the TCP and IPv4 checksums but does not recompute them for the coalesced segment, it must set the TcpChecksumValueInvalid and IpChecksumValueInvalid flags in the NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO structure. Additionally, in this case the NIC or miniport driver may optionally zero out the TCP and IPv4 header checksum values in the segment.
The NIC and miniport driver must always set the IpChecksumSucceeded and TcpChecksumSucceeded flags in the NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO structure before indicating the coalesced segment.
Thanks,
- Haiyang
^ permalink raw reply
* kTLS getsockopt TLS_RX support
From: Hayakawa Yutaro @ 2019-02-23 17:29 UTC (permalink / raw)
To: netdev
Hello,
While trying the kTLS, I found out that currently, there is no support for kTLS getsockopt TLS_RX which extracts receive side crypto information from kTLS socket. Since setting crypto information for RX side is supported, I felt wonder why it is not supported.
Is there any particular reason for it?
Regards,
Yutaro
^ permalink raw reply
* [PATCH v2 bpf-next 0/4] bpf: per program stats
From: Alexei Starovoitov @ 2019-02-23 17:44 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
Introduce per program stats to monitor the usage BPF
v1->v2:
- fixed u64 stats on 32-bit archs. Thanks Eric
- use more verbose run_time_ns in json output as suggested by Andrii
- refactored prog_alloc and clarified behavior of stats in subprogs
Alexei Starovoitov (4):
bpf: enable program stats
bpf: expose program stats via bpf_prog_info
tools/bpf: sync bpf.h into tools
tools/bpftool: recognize bpf_prog_info runtime and runcnt
include/linux/bpf.h | 9 +++++
include/linux/filter.h | 20 +++++++++-
include/uapi/linux/bpf.h | 2 +
kernel/bpf/core.c | 31 ++++++++++++++-
kernel/bpf/syscall.c | 39 ++++++++++++++++++-
kernel/bpf/verifier.c | 7 +++-
kernel/sysctl.c | 34 ++++++++++++++++
.../bpftool/Documentation/bpftool-prog.rst | 4 +-
tools/bpf/bpftool/prog.c | 7 ++++
tools/include/uapi/linux/bpf.h | 2 +
10 files changed, 148 insertions(+), 7 deletions(-)
--
2.20.0
^ permalink raw reply
* [PATCH v2 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-23 17:44 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190223174422.663270-1-ast@kernel.org>
JITed BPF programs are indistinguishable from kernel functions, but unlike
kernel code BPF code can be changed often.
Typical approach of "perf record" + "perf report" profiling and tuning of
kernel code works just as well for BPF programs, but kernel code doesn't
need to be monitored whereas BPF programs do.
Users load and run large amount of BPF programs.
These BPF stats allow tools monitor the usage of BPF on the server.
The monitoring tools will turn sysctl kernel.bpf_stats_enabled
on and off for few seconds to sample average cost of the programs.
Aggregated data over hours and days will provide an insight into cost of BPF
and alarms can trigger in case given program suddenly gets more expensive.
The cost of two sched_clock() per program invocation adds ~20 nsec.
Fast BPF progs (like selftests/bpf/progs/test_pkt_access.c) will slow down
from ~10 nsec to ~30 nsec.
static_key minimizes the cost of the stats collection.
There is no measurable difference before/after this patch
with kernel.bpf_stats_enabled=0
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf.h | 9 +++++++++
include/linux/filter.h | 20 +++++++++++++++++++-
kernel/bpf/core.c | 31 +++++++++++++++++++++++++++++--
kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++--
kernel/bpf/verifier.c | 7 ++++++-
kernel/sysctl.c | 34 ++++++++++++++++++++++++++++++++++
6 files changed, 129 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..a2132e09dc1c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -16,6 +16,7 @@
#include <linux/rbtree_latch.h>
#include <linux/numa.h>
#include <linux/wait.h>
+#include <linux/u64_stats_sync.h>
struct bpf_verifier_env;
struct perf_event;
@@ -340,6 +341,12 @@ enum bpf_cgroup_storage_type {
#define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
+struct bpf_prog_stats {
+ u64 cnt;
+ u64 nsecs;
+ struct u64_stats_sync syncp;
+};
+
struct bpf_prog_aux {
atomic_t refcnt;
u32 used_map_cnt;
@@ -389,6 +396,7 @@ struct bpf_prog_aux {
* main prog always has linfo_idx == 0
*/
u32 linfo_idx;
+ struct bpf_prog_stats __percpu *stats;
union {
struct work_struct work;
struct rcu_head rcu;
@@ -559,6 +567,7 @@ void bpf_map_area_free(void *base);
void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
extern int sysctl_unprivileged_bpf_disabled;
+extern int sysctl_bpf_stats_enabled;
int bpf_map_new_fd(struct bpf_map *map, int flags);
int bpf_prog_new_fd(struct bpf_prog *prog);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f32b3eca5a04..7e5e3db11106 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -533,7 +533,24 @@ struct sk_filter {
struct bpf_prog *prog;
};
-#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
+DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
+
+#define BPF_PROG_RUN(prog, ctx) ({ \
+ u32 ret; \
+ cant_sleep(); \
+ if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
+ struct bpf_prog_stats *stats; \
+ u64 start = sched_clock(); \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ stats = this_cpu_ptr(prog->aux->stats); \
+ u64_stats_update_begin(&stats->syncp); \
+ stats->cnt++; \
+ stats->nsecs += sched_clock() - start; \
+ u64_stats_update_end(&stats->syncp); \
+ } else { \
+ ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
+ } \
+ ret; })
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
@@ -764,6 +781,7 @@ void bpf_prog_free_jited_linfo(struct bpf_prog *prog);
void bpf_prog_free_unused_jited_linfo(struct bpf_prog *prog);
struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags);
+struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags);
struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
gfp_t gfp_extra_flags);
void __bpf_prog_free(struct bpf_prog *fp);
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ef88b167959d..1c14c347f3cf 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -78,7 +78,7 @@ void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, uns
return NULL;
}
-struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
+struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags)
{
gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
struct bpf_prog_aux *aux;
@@ -104,6 +104,26 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
return fp;
}
+
+struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
+{
+ gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
+ struct bpf_prog *prog;
+
+ prog = bpf_prog_alloc_no_stats(size, gfp_extra_flags);
+ if (!prog)
+ return NULL;
+
+ prog->aux->stats = alloc_percpu_gfp(struct bpf_prog_stats, gfp_flags);
+ if (!prog->aux->stats) {
+ kfree(prog->aux);
+ vfree(prog);
+ return NULL;
+ }
+
+ u64_stats_init(&prog->aux->stats->syncp);
+ return prog;
+}
EXPORT_SYMBOL_GPL(bpf_prog_alloc);
int bpf_prog_alloc_jited_linfo(struct bpf_prog *prog)
@@ -231,7 +251,10 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
void __bpf_prog_free(struct bpf_prog *fp)
{
- kfree(fp->aux);
+ if (fp->aux) {
+ free_percpu(fp->aux->stats);
+ kfree(fp->aux);
+ }
vfree(fp);
}
@@ -2069,6 +2092,10 @@ int __weak skb_copy_bits(const struct sk_buff *skb, int offset, void *to,
return -EFAULT;
}
+DEFINE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
+EXPORT_SYMBOL(bpf_stats_enabled_key);
+int sysctl_bpf_stats_enabled __read_mostly;
+
/* All definitions of tracepoints related to BPF. */
#define CREATE_TRACE_POINTS
#include <linux/bpf_trace.h>
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ec7c552af76b..9a8c990c5df4 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1283,24 +1283,54 @@ static int bpf_prog_release(struct inode *inode, struct file *filp)
return 0;
}
+static void bpf_prog_get_stats(const struct bpf_prog *prog,
+ struct bpf_prog_stats *stats)
+{
+ u64 nsecs = 0, cnt = 0;
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ const struct bpf_prog_stats *st;
+ unsigned int start;
+ u64 tnsecs, tcnt;
+
+ st = per_cpu_ptr(prog->aux->stats, cpu);
+ do {
+ start = u64_stats_fetch_begin_irq(&st->syncp);
+ tnsecs = st->nsecs;
+ tcnt = st->cnt;
+ } while (u64_stats_fetch_retry_irq(&st->syncp, start));
+ nsecs += tnsecs;
+ cnt += tcnt;
+ }
+ stats->nsecs = nsecs;
+ stats->cnt = cnt;
+}
+
#ifdef CONFIG_PROC_FS
static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
{
const struct bpf_prog *prog = filp->private_data;
char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
+ struct bpf_prog_stats stats;
+ bpf_prog_get_stats(prog, &stats);
bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
seq_printf(m,
"prog_type:\t%u\n"
"prog_jited:\t%u\n"
"prog_tag:\t%s\n"
"memlock:\t%llu\n"
- "prog_id:\t%u\n",
+ "prog_id:\t%u\n"
+ "runtime:\t%llu\n"
+ "runcnt:\t%llu\n",
prog->type,
prog->jited,
prog_tag,
prog->pages * 1ULL << PAGE_SHIFT,
- prog->aux->id);
+ prog->aux->id,
+ stats.nsecs,
+ stats.cnt);
}
#endif
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1b9496c41383..0e4edd7e3c5f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7320,7 +7320,12 @@ static int jit_subprogs(struct bpf_verifier_env *env)
subprog_end = env->subprog_info[i + 1].start;
len = subprog_end - subprog_start;
- func[i] = bpf_prog_alloc(bpf_prog_size(len), GFP_USER);
+ /* BPF_PROG_RUN doesn't call subprogs directly,
+ * hence main prog stats include the runtime of subprogs.
+ * subprogs don't have IDs and not reachable via prog_get_next_id
+ * func[i]->aux->stats will never be accessed and stays NULL
+ */
+ func[i] = bpf_prog_alloc_no_stats(bpf_prog_size(len), GFP_USER);
if (!func[i])
goto out_free;
memcpy(func[i]->insnsi, &prog->insnsi[subprog_start],
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ba4d9e85feb8..86e0771352f2 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -224,6 +224,9 @@ static int proc_dostring_coredump(struct ctl_table *table, int write,
#endif
static int proc_dopipe_max_size(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos);
+static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos);
#ifdef CONFIG_MAGIC_SYSRQ
/* Note: sysrq code uses its own private copy */
@@ -1230,6 +1233,15 @@ static struct ctl_table kern_table[] = {
.extra2 = &one,
},
#endif
+ {
+ .procname = "bpf_stats_enabled",
+ .data = &sysctl_bpf_stats_enabled,
+ .maxlen = sizeof(sysctl_bpf_stats_enabled),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax_bpf_stats,
+ .extra1 = &zero,
+ .extra2 = &one,
+ },
#if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU)
{
.procname = "panic_on_rcu_stall",
@@ -3260,6 +3272,28 @@ int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int write,
#endif /* CONFIG_PROC_SYSCTL */
+static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp,
+ loff_t *ppos)
+{
+ int ret, bpf_stats = *(int *)table->data;
+ struct ctl_table tmp = *table;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ tmp.data = &bpf_stats;
+ ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
+ if (write && !ret) {
+ *(int *)table->data = bpf_stats;
+ if (bpf_stats)
+ static_branch_enable(&bpf_stats_enabled_key);
+ else
+ static_branch_disable(&bpf_stats_enabled_key);
+ }
+ return ret;
+}
+
/*
* No sense putting this after each symbol definition, twice,
* exception granted :-)
--
2.20.0
^ permalink raw reply related
* [PATCH v2 bpf-next 2/4] bpf: expose program stats via bpf_prog_info
From: Alexei Starovoitov @ 2019-02-23 17:44 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190223174422.663270-1-ast@kernel.org>
Return bpf program runtime and runcnt via bpf_prog_info
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/uapi/linux/bpf.h | 2 ++
kernel/bpf/syscall.c | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..d2cb85d85b39 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2813,6 +2813,8 @@ struct bpf_prog_info {
__u32 jited_line_info_rec_size;
__u32 nr_prog_tags;
__aligned_u64 prog_tags;
+ __u64 runtime;
+ __u64 runcnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 9a8c990c5df4..3405e0a4cf77 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2152,6 +2152,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info);
struct bpf_prog_info info = {};
u32 info_len = attr->info.info_len;
+ struct bpf_prog_stats stats;
char __user *uinsns;
u32 ulen;
int err;
@@ -2191,6 +2192,10 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
if (err)
return err;
+ bpf_prog_get_stats(prog, &stats);
+ info.runtime = stats.nsecs;
+ info.runcnt = stats.cnt;
+
if (!capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
--
2.20.0
^ permalink raw reply related
* [PATCH v2 bpf-next 4/4] tools/bpftool: recognize bpf_prog_info runtime and runcnt
From: Alexei Starovoitov @ 2019-02-23 17:44 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190223174422.663270-1-ast@kernel.org>
$ bpftool p s
1: kprobe tag a56587d488d216c9 gpl runtime 79786 runcnt 8
loaded_at 2019-02-22T12:22:51-0800 uid 0
xlated 352B not jited memlock 4096B
$ bpftool --json --pretty p s
[{
"id": 1,
"type": "kprobe",
"tag": "a56587d488d216c9",
"gpl_compatible": true,
"run_time_ns": 79786,
"run_cnt": 8,
"loaded_at": 1550866971,
"uid": 0,
"bytes_xlated": 352,
"jited": false,
"bytes_memlock": 4096
}
]
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/prog.c | 7 +++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 12bc1e2d4b46..d8b46a8f17e0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -171,7 +171,7 @@ EXAMPLES
::
- 10: xdp name some_prog tag 005a3d2123620c8b gpl
+ 10: xdp name some_prog tag 005a3d2123620c8b gpl runtime 81632 runcnt 10
loaded_at 2017-09-29T20:11:00+0000 uid 0
xlated 528B jited 370B memlock 4096B map_ids 10
@@ -184,6 +184,8 @@ EXAMPLES
"type": "xdp",
"tag": "005a3d2123620c8b",
"gpl_compatible": true,
+ "run_time_ns": 81632,
+ "run_cnt": 10,
"loaded_at": 1506715860,
"uid": 0,
"bytes_xlated": 528,
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index db978c8d76a8..cdb6bd424340 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
+ if (info->runtime) {
+ jsonw_uint_field(json_wtr, "run_time_ns", info->runtime);
+ jsonw_uint_field(json_wtr, "run_cnt", info->runcnt);
+ }
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
@@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
printf("%s", info->gpl_compatible ? " gpl" : "");
+ if (info->runtime)
+ printf(" runtime %lld runcnt %lld",
+ info->runtime, info->runcnt);
printf("\n");
if (info->load_time) {
--
2.20.0
^ permalink raw reply related
* [PATCH v2 bpf-next 3/4] tools/bpf: sync bpf.h into tools
From: Alexei Starovoitov @ 2019-02-23 17:44 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190223174422.663270-1-ast@kernel.org>
sync bpf.h into tools directory
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/include/uapi/linux/bpf.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..d2cb85d85b39 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2813,6 +2813,8 @@ struct bpf_prog_info {
__u32 jited_line_info_rec_size;
__u32 nr_prog_tags;
__aligned_u64 prog_tags;
+ __u64 runtime;
+ __u64 runcnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
--
2.20.0
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 1/4] bpf: enable program stats
From: Eric Dumazet @ 2019-02-23 18:36 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190223174422.663270-2-ast@kernel.org>
On 02/23/2019 09:44 AM, Alexei Starovoitov wrote:
...
>
> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> +
> +#define BPF_PROG_RUN(prog, ctx) ({ \
> + u32 ret; \
> + cant_sleep(); \
> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> + struct bpf_prog_stats *stats; \
> + u64 start = sched_clock(); \
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + stats = this_cpu_ptr(prog->aux->stats); \
> + u64_stats_update_begin(&stats->syncp); \
> + stats->cnt++; \
> + stats->nsecs += sched_clock() - start; \
> + u64_stats_update_end(&stats->syncp); \
> + } else { \
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + } \
> + ret; })
>
It seems a cpu running there could still be interrupted (by an interrupt)
and re-enter this section ?
If yes, u64_stats_update_begin() and u64_stats_update_end() are unsafe (on 32bit arches)
u64_stats_update_{begin|end}() assume proper locking, since they use a simple increment.
But then, even on 64bit arches, the stats->{cnt|nsecs} updates are unsafe ?
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: Eric Dumazet @ 2019-02-23 18:39 UTC (permalink / raw)
To: David Ahern, brakmo, netdev
Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Kernel Team
In-Reply-To: <184fd1a4-451e-2585-6aad-d9b0b10fd9aa@gmail.com>
On 02/22/2019 07:03 PM, David Ahern wrote:
> On 2/22/19 8:06 PM, brakmo wrote:
>> Network Resource Manager is a framework for limiting the bandwidth used
>> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
>> limit egress bandwdith as well as a sample user program and script to
>> simplify NRM testing.
>
> 'resource manager' is a really generic name. Since you are referring to
> bandwidth, how about renaming to Network Bandwidth Manager?
>
Or just use the normal word for a policer ...
Really this is beyond me that TCP experts can still push policers out there,
they are really a huge pain.
^ permalink raw reply
* [PATCH bpf] bpf: properly check TCP_CONGESTION optlen
From: Eric Dumazet @ 2019-02-23 19:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, netdev, Eric Dumazet, Eric Dumazet,
Martin KaFai Lau, Song Liu, Yonghong Song, bpf, Lawrence Brakmo
If caller of bpf_setsockopt() is silly passing a negative optlen
bad things happen.
Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Lawrence Brakmo <brakmo@fb.com>
---
net/core/filter.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index f7d0004fc16096eb42ece3a6acf645540ee2326b..6a5d89464168f2f35f43986c1dbc0446c9390a3c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4194,8 +4194,9 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
char name[TCP_CA_NAME_MAX];
bool reinit = bpf_sock->op > BPF_SOCK_OPS_NEEDS_ECN;
- strncpy(name, optval, min_t(long, optlen,
- TCP_CA_NAME_MAX-1));
+ if (optlen < 0)
+ return -EINVAL;
+ strncpy(name, optval, min(optlen, TCP_CA_NAME_MAX - 1));
name[TCP_CA_NAME_MAX-1] = 0;
ret = tcp_set_congestion_control(sk, name, false,
reinit);
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* Re: [PATCH 3/8] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth
From: David Summers @ 2019-02-23 20:16 UTC (permalink / raw)
To: Vasily Khoruzhick
Cc: Stefan Wahren, Rob Herring, Mark Rutland, devicetree,
Johan Hedberg, Maxime Ripard, netdev, Marcel Holtmann,
open list:BLUETOOTH DRIVERS, Chen-Yu Tsai, David S. Miller,
arm-linux
In-Reply-To: <CA+E=qVdsU7sBkOQUmMprhD-Sdw7TdFnwFdGCv=4iRe++y1AwOg@mail.gmail.com>
On 22/02/2019 19:21, Vasily Khoruzhick wrote:
> On Fri, Feb 22, 2019 at 11:14 AM David Summers
> <beagleboard@davidjohnsummers.uk> wrote:
>> Hi Vasily,
>>
>> Just catching up with this series - good that you are doing it. We need
>> it for users on ArchLinux ARM!
>>
>> On question though, what is "firmware-postfix" used for? I see in the
>> documentation, that it points to the board name. But can't see where
>> else it is used.
> See h5_serdev_probe() in drivers/bluetooth/hci_h5.c. Basically it
> specifies what firmware config to use.
> I agree with Rob that we should probably use firmware-name here instead.
Ah OK followed it. its sets h5->id than in h5_btrtl_setup (in hci_h5)
calls up btrtl_initialise (in btrtl.c) and that gives the file to load.
Guess now need to trace what firmware files are available.
Oh yes - see why you called it postfix, as that is the name given in
btrl_initialise. I can't comment on what name is best, but I can see the
sense of keeping the device tree hook the same as used as in
btrtl_initialise.
>> Is there a need to have the board name?
> As far as I understand firmware config depends on board, so I think
> it's a good idea to use board name here.
>
>> Anyway good work.
> Thanks!
>
>> David.
>>
>> On 18/02/2019 22:28, Vasily Khoruzhick wrote:
>>> On Mon, Feb 18, 2019 at 2:08 PM Stefan Wahren <stefan.wahren@i2se.com> wrote:
>>>> Hi Vasily,
>>> Hi Stefan,
>>>
>>>>> Vasily Khoruzhick <anarsoul@gmail.com> hat am 18. Februar 2019 um 22:24 geschrieben:
>>>>>
>>>>>
>>>>> On Mon, Feb 18, 2019 at 1:10 PM Rob Herring <robh@kernel.org> wrote:
>>>>>> On Fri, Jan 18, 2019 at 09:02:27AM -0800, Vasily Khoruzhick wrote:
>>>>>>> Add binding document for bluetooth part of RTL8723BS/RTL8723CS
>>>>>>>
>>>>>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>>>>>> ---
>>>>>>> .../bindings/net/rtl8723bs-bluetooth.txt | 35 +++++++++++++++++++
>>>>>>> 1 file changed, 35 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..8357f242ae4c
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/net/rtl8723bs-bluetooth.txt
>>>>>>> @@ -0,0 +1,35 @@
>>>>>>> +RTL8723BS/RTL8723CS Bluetooth
>>>>>>> +---------------------
>>>>>>> +
>>>>>>> +RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
>>>>>>> +BT is connected over serial. It speaks H5 protocol with few extra commands
>>>>>>> +to upload firmware and change module speed.
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +
>>>>>>> + - compatible: should be one of the following:
>>>>>>> + * "realtek,rtl8723bs-bt"
>>>>>>> + * "realtek,rtl8723cs-bt"
>>>>>>> +Optional properties:
>>>>>>> +
>>>>>>> + - device-wake-gpios: GPIO specifier, used to wakeup the BT module (active high)
>>>>>>> + - enable-gpios: GPIO specifier, used to enable the BT module (active high)
>>>>>>> + - host-wake-gpios: GPIO specifier, used to wakeup the host processor (active high)
>>>>>>> + - firmware-postfix: firmware postfix to be used for firmware config
>>>> sorry, i didn't noticed your great series before. David and i working at the same stuff but for the Asus Tinker Board.
>>>>
>>>> I created a similiar yet untested patch version for hci_h5 [1]. Maybe it's useful.
>>> Looks good to me, but you may need to add firmware-postfix.
>>>
>>>> Just a comment about the binding. It's really necessary to add the reset-gpio? Can't we use the enable-gpio with inverse polarity for this?
>>> Yes, we can use enable-gpio instead of reset-gpio on pine64 and pinebook.
>>>
>>>> Stefan
>>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-23 20:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexei Starovoitov, davem, daniel, edumazet, netdev, bpf,
kernel-team
In-Reply-To: <218b9aab-5e78-46b1-1f65-dd0a305548c7@gmail.com>
On Sat, Feb 23, 2019 at 10:36:48AM -0800, Eric Dumazet wrote:
>
>
> On 02/23/2019 09:44 AM, Alexei Starovoitov wrote:
>
> ...
>
> >
> > -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> > +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> > +
> > +#define BPF_PROG_RUN(prog, ctx) ({ \
> > + u32 ret; \
> > + cant_sleep(); \
> > + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> > + struct bpf_prog_stats *stats; \
> > + u64 start = sched_clock(); \
> > + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> > + stats = this_cpu_ptr(prog->aux->stats); \
> > + u64_stats_update_begin(&stats->syncp); \
> > + stats->cnt++; \
> > + stats->nsecs += sched_clock() - start; \
> > + u64_stats_update_end(&stats->syncp); \
> > + } else { \
> > + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> > + } \
> > + ret; })
> >
>
> It seems a cpu running there could still be interrupted (by an interrupt)
> and re-enter this section ?
>
> If yes, u64_stats_update_begin() and u64_stats_update_end() are unsafe (on 32bit arches)
>
> u64_stats_update_{begin|end}() assume proper locking, since they use a simple increment.
>
> But then, even on 64bit arches, the stats->{cnt|nsecs} updates are unsafe ?
No. It cannot reenter for the same program.
socket filter prog can be interrupted by kprobe prog, but they are different
progs and different stats.
^ permalink raw reply
* Re: [PATCH bpf] bpf: properly check TCP_CONGESTION optlen
From: Alexei Starovoitov @ 2019-02-23 20:38 UTC (permalink / raw)
To: Eric Dumazet
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eric Dumazet,
Martin KaFai Lau, Song Liu, Yonghong Song, bpf, Lawrence Brakmo
In-Reply-To: <20190223190709.61558-1-edumazet@google.com>
On Sat, Feb 23, 2019 at 11:07:09AM -0800, Eric Dumazet wrote:
> If caller of bpf_setsockopt() is silly passing a negative optlen
> bad things happen.
>
> Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Lawrence Brakmo <brakmo@fb.com>
> ---
> net/core/filter.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f7d0004fc16096eb42ece3a6acf645540ee2326b..6a5d89464168f2f35f43986c1dbc0446c9390a3c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4194,8 +4194,9 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
> char name[TCP_CA_NAME_MAX];
> bool reinit = bpf_sock->op > BPF_SOCK_OPS_NEEDS_ECN;
>
> - strncpy(name, optval, min_t(long, optlen,
> - TCP_CA_NAME_MAX-1));
> + if (optlen < 0)
> + return -EINVAL;
> + strncpy(name, optval, min(optlen, TCP_CA_NAME_MAX - 1));
Unnecessary.
The verifier guarantees that optlen > 0 because
static const struct bpf_func_proto bpf_setsockopt_proto = {
.func = bpf_setsockopt,
...
.arg5_type = ARG_CONST_SIZE,
};
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: Alexei Starovoitov @ 2019-02-23 20:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Ahern, brakmo, netdev, Martin Lau, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <2507e7b5-2ffa-e080-698a-18a7b0ee36ac@gmail.com>
On Sat, Feb 23, 2019 at 10:39:53AM -0800, Eric Dumazet wrote:
>
>
> On 02/22/2019 07:03 PM, David Ahern wrote:
> > On 2/22/19 8:06 PM, brakmo wrote:
> >> Network Resource Manager is a framework for limiting the bandwidth used
> >> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
> >> limit egress bandwdith as well as a sample user program and script to
> >> simplify NRM testing.
> >
> > 'resource manager' is a really generic name. Since you are referring to
> > bandwidth, how about renaming to Network Bandwidth Manager?
> >
>
> Or just use the normal word for a policer ...
>
> Really this is beyond me that TCP experts can still push policers out there,
> they are really a huge pain.
hmm. please see our NRM presentation at LPC.
It is a networking _resource_ management for cgroups.
Bandwidth enforcement is a particular example.
It's not a policer either.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: Eric Dumazet @ 2019-02-23 20:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Ahern, brakmo, netdev, Martin Lau, Alexei Starovoitov,
Daniel Borkmann, Kernel Team
In-Reply-To: <20190223204046.zjvcxh7cgzshqllm@ast-mbp.dhcp.thefacebook.com>
On 02/23/2019 12:40 PM, Alexei Starovoitov wrote:
> On Sat, Feb 23, 2019 at 10:39:53AM -0800, Eric Dumazet wrote:
>>
>>
>> On 02/22/2019 07:03 PM, David Ahern wrote:
>>> On 2/22/19 8:06 PM, brakmo wrote:
>>>> Network Resource Manager is a framework for limiting the bandwidth used
>>>> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
>>>> limit egress bandwdith as well as a sample user program and script to
>>>> simplify NRM testing.
>>>
>>> 'resource manager' is a really generic name. Since you are referring to
>>> bandwidth, how about renaming to Network Bandwidth Manager?
>>>
>>
>> Or just use the normal word for a policer ...
>>
>> Really this is beyond me that TCP experts can still push policers out there,
>> they are really a huge pain.
>
> hmm. please see our NRM presentation at LPC.
> It is a networking _resource_ management for cgroups.
> Bandwidth enforcement is a particular example.
> It's not a policer either.
>
Well, this definitely looks a policer to me, sorry if we disagree, this is fine.
^ permalink raw reply
* Re: [PATCH bpf] bpf: properly check TCP_CONGESTION optlen
From: Eric Dumazet @ 2019-02-23 20:48 UTC (permalink / raw)
To: Alexei Starovoitov, Eric Dumazet
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Martin KaFai Lau,
Song Liu, Yonghong Song, bpf, Lawrence Brakmo
In-Reply-To: <20190223203808.f2r54xz2z7j2gecg@ast-mbp.dhcp.thefacebook.com>
On 02/23/2019 12:38 PM, Alexei Starovoitov wrote:
> On Sat, Feb 23, 2019 at 11:07:09AM -0800, Eric Dumazet wrote:
>> If caller of bpf_setsockopt() is silly passing a negative optlen
>> bad things happen.
>>
>> Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Lawrence Brakmo <brakmo@fb.com>
>> ---
>> net/core/filter.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index f7d0004fc16096eb42ece3a6acf645540ee2326b..6a5d89464168f2f35f43986c1dbc0446c9390a3c 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -4194,8 +4194,9 @@ BPF_CALL_5(bpf_setsockopt, struct bpf_sock_ops_kern *, bpf_sock,
>> char name[TCP_CA_NAME_MAX];
>> bool reinit = bpf_sock->op > BPF_SOCK_OPS_NEEDS_ECN;
>>
>> - strncpy(name, optval, min_t(long, optlen,
>> - TCP_CA_NAME_MAX-1));
>> + if (optlen < 0)
>> + return -EINVAL;
>> + strncpy(name, optval, min(optlen, TCP_CA_NAME_MAX - 1));
>
> Unnecessary.
> The verifier guarantees that optlen > 0 because
> static const struct bpf_func_proto bpf_setsockopt_proto = {
> .func = bpf_setsockopt,
> ...
> .arg5_type = ARG_CONST_SIZE,
> };
>
Even on 32bit kernel ?
The suspect thing to me is the min_t(long, ....)
optlen is an integer, why do we need to promote to a long ?
^ permalink raw reply
* [PATCH AUTOSEL 4.20 03/72] xfrm: Make set-mark default behavior backward compatible
From: Sasha Levin @ 2019-02-23 21:03 UTC (permalink / raw)
To: linux-kernel, stable; +Cc: Benedict Wong, Steffen Klassert, Sasha Levin, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Benedict Wong <benedictwong@google.com>
[ Upstream commit e2612cd496e7b465711d219ea6118893d7253f52 ]
Fixes 9b42c1f179a6, which changed the default route lookup behavior for
tunnel mode SAs in the outbound direction to use the skb mark, whereas
previously mark=0 was used if the output mark was unspecified. In
mark-based routing schemes such as Android’s, this change in default
behavior causes routing loops or lookup failures.
This patch restores the default behavior of using a 0 mark while still
incorporating the skb mark if the SET_MARK (and SET_MARK_MASK) is
specified.
Tested with additions to Android's kernel unit test suite:
https://android-review.googlesource.com/c/kernel/tests/+/860150
Fixes: 9b42c1f179a6 ("xfrm: Extend the output_mark to support input direction and masking")
Signed-off-by: Benedict Wong <benedictwong@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/xfrm/xfrm_policy.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 119a427d9b2b2..6ea8036fcdbeb 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1628,7 +1628,10 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
dst_copy_metrics(dst1, dst);
if (xfrm[i]->props.mode != XFRM_MODE_TRANSPORT) {
- __u32 mark = xfrm_smark_get(fl->flowi_mark, xfrm[i]);
+ __u32 mark = 0;
+
+ if (xfrm[i]->props.smark.v || xfrm[i]->props.smark.m)
+ mark = xfrm_smark_get(fl->flowi_mark, xfrm[i]);
family = xfrm[i]->props.family;
dst = xfrm_dst_lookup(xfrm[i], tos, fl->flowi_oif,
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 05/72] netfilter: nft_compat: use refcnt_t type for nft_xt reference count
From: Sasha Levin @ 2019-02-23 21:03 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Pablo Neira Ayuso, Sasha Levin, netfilter-devel,
coreteam, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Florian Westphal <fw@strlen.de>
[ Upstream commit 12c44aba6618b7f6c437076e5722237190f6cd5f ]
Using standard integer type was fine while all operations on it were
guarded by the nftnl subsys mutex.
This isn't true anymore:
1. transactions are guarded only by a pernet mutex, so concurrent
rule manipulation in different netns is racy
2. the ->destroy hook runs from a work queue after the transaction
mutex has been released already.
cpu0 cpu1 (net 1) cpu2 (net 2)
kworker
nft_compat->destroy nft_compat->init nft_compat->init
if (--nft_xt->ref == 0) nft_xt->ref++ nft_xt->ref++
Switch to refcount_t. Doing this however only fixes a minor aspect,
nft_compat also performs linked-list operations in an unsafe way.
This is addressed in the next two patches.
Fixes: f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")
Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
Reported-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nft_compat.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index 7334e0b80a5ef..acc85acad31bd 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -26,7 +26,7 @@
struct nft_xt {
struct list_head head;
struct nft_expr_ops ops;
- unsigned int refcnt;
+ refcount_t refcnt;
/* Unlike other expressions, ops doesn't have static storage duration.
* nft core assumes they do. We use kfree_rcu so that nft core can
@@ -45,7 +45,7 @@ struct nft_xt_match_priv {
static bool nft_xt_put(struct nft_xt *xt)
{
- if (--xt->refcnt == 0) {
+ if (refcount_dec_and_test(&xt->refcnt)) {
list_del(&xt->head);
kfree_rcu(xt, rcu_head);
return true;
@@ -273,7 +273,7 @@ nft_target_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
return -EINVAL;
nft_xt = container_of(expr->ops, struct nft_xt, ops);
- nft_xt->refcnt++;
+ refcount_inc(&nft_xt->refcnt);
return 0;
}
@@ -486,7 +486,7 @@ __nft_match_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
return ret;
nft_xt = container_of(expr->ops, struct nft_xt, ops);
- nft_xt->refcnt++;
+ refcount_inc(&nft_xt->refcnt);
return 0;
}
@@ -789,7 +789,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
goto err;
}
- nft_match->refcnt = 0;
+ refcount_set(&nft_match->refcnt, 0);
nft_match->ops.type = &nft_match_type;
nft_match->ops.eval = nft_match_eval;
nft_match->ops.init = nft_match_init;
@@ -893,7 +893,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
goto err;
}
- nft_target->refcnt = 0;
+ refcount_set(&nft_target->refcnt, 0);
nft_target->ops.type = &nft_target_type;
nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
nft_target->ops.init = nft_target_init;
@@ -964,7 +964,7 @@ static void __exit nft_compat_module_exit(void)
list_for_each_entry_safe(xt, next, &nft_target_list, head) {
struct xt_target *target = xt->ops.data;
- if (WARN_ON_ONCE(xt->refcnt))
+ if (WARN_ON_ONCE(refcount_read(&xt->refcnt)))
continue;
module_put(target->me);
kfree(xt);
@@ -973,7 +973,7 @@ static void __exit nft_compat_module_exit(void)
list_for_each_entry_safe(xt, next, &nft_match_list, head) {
struct xt_match *match = xt->ops.data;
- if (WARN_ON_ONCE(xt->refcnt))
+ if (WARN_ON_ONCE(refcount_read(&xt->refcnt)))
continue;
module_put(match->me);
kfree(xt);
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 06/72] netfilter: nft_compat: make lists per netns
From: Sasha Levin @ 2019-02-23 21:03 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Pablo Neira Ayuso, Sasha Levin, netfilter-devel,
coreteam, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Florian Westphal <fw@strlen.de>
[ Upstream commit cf52572ebbd7189a1966c2b5fc34b97078cd1dce ]
There are two problems with nft_compat since the netlink config
plane uses a per-netns mutex:
1. Concurrent add/del accesses to the same list
2. accesses to a list element after it has been free'd already.
This patch fixes the first problem.
Freeing occurs from a work queue, after transaction mutexes have been
released, i.e., it still possible for a new transaction (even from
same net ns) to find the to-be-deleted expression in the list.
The ->destroy functions are not allowed to have any such side effects,
i.e. the list_del() in the destroy function is not allowed.
This part of the problem is solved in the next patch.
I tried to make this work by serializing list access via mutex
and by moving list_del() to a deactivate callback, but
Taehee spotted following race on this approach:
NET #0 NET #1
>select_ops()
->init()
->select_ops()
->deactivate()
->destroy()
nft_xt_put()
kfree_rcu(xt, rcu_head);
->init() <-- use-after-free occurred.
Unfortunately, we can't increment reference count in
select_ops(), because we can't undo the refcount increase in
case a different expression fails in the same batch.
(The destroy hook will only be called in case the expression
was initialized successfully).
Fixes: f102d66b335a ("netfilter: nf_tables: use dedicated mutex to guard transactions")
Reported-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nft_compat.c | 129 +++++++++++++++++++++++++------------
1 file changed, 89 insertions(+), 40 deletions(-)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index acc85acad31bd..abed3490a8f8a 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -22,6 +22,7 @@
#include <linux/netfilter_bridge/ebtables.h>
#include <linux/netfilter_arp/arp_tables.h>
#include <net/netfilter/nf_tables.h>
+#include <net/netns/generic.h>
struct nft_xt {
struct list_head head;
@@ -43,6 +44,20 @@ struct nft_xt_match_priv {
void *info;
};
+struct nft_compat_net {
+ struct list_head nft_target_list;
+ struct list_head nft_match_list;
+};
+
+static unsigned int nft_compat_net_id __read_mostly;
+static struct nft_expr_type nft_match_type;
+static struct nft_expr_type nft_target_type;
+
+static struct nft_compat_net *nft_compat_pernet(struct net *net)
+{
+ return net_generic(net, nft_compat_net_id);
+}
+
static bool nft_xt_put(struct nft_xt *xt)
{
if (refcount_dec_and_test(&xt->refcnt)) {
@@ -734,10 +749,6 @@ static const struct nfnetlink_subsystem nfnl_compat_subsys = {
.cb = nfnl_nft_compat_cb,
};
-static LIST_HEAD(nft_match_list);
-
-static struct nft_expr_type nft_match_type;
-
static bool nft_match_cmp(const struct xt_match *match,
const char *name, u32 rev, u32 family)
{
@@ -749,6 +760,7 @@ static const struct nft_expr_ops *
nft_match_select_ops(const struct nft_ctx *ctx,
const struct nlattr * const tb[])
{
+ struct nft_compat_net *cn;
struct nft_xt *nft_match;
struct xt_match *match;
unsigned int matchsize;
@@ -765,8 +777,10 @@ nft_match_select_ops(const struct nft_ctx *ctx,
rev = ntohl(nla_get_be32(tb[NFTA_MATCH_REV]));
family = ctx->family;
+ cn = nft_compat_pernet(ctx->net);
+
/* Re-use the existing match if it's already loaded. */
- list_for_each_entry(nft_match, &nft_match_list, head) {
+ list_for_each_entry(nft_match, &cn->nft_match_list, head) {
struct xt_match *match = nft_match->ops.data;
if (nft_match_cmp(match, mt_name, rev, family))
@@ -810,7 +824,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.size = matchsize;
- list_add(&nft_match->head, &nft_match_list);
+ list_add(&nft_match->head, &cn->nft_match_list);
return &nft_match->ops;
err:
@@ -826,10 +840,6 @@ static struct nft_expr_type nft_match_type __read_mostly = {
.owner = THIS_MODULE,
};
-static LIST_HEAD(nft_target_list);
-
-static struct nft_expr_type nft_target_type;
-
static bool nft_target_cmp(const struct xt_target *tg,
const char *name, u32 rev, u32 family)
{
@@ -841,6 +851,7 @@ static const struct nft_expr_ops *
nft_target_select_ops(const struct nft_ctx *ctx,
const struct nlattr * const tb[])
{
+ struct nft_compat_net *cn;
struct nft_xt *nft_target;
struct xt_target *target;
char *tg_name;
@@ -861,8 +872,9 @@ nft_target_select_ops(const struct nft_ctx *ctx,
strcmp(tg_name, "standard") == 0)
return ERR_PTR(-EINVAL);
+ cn = nft_compat_pernet(ctx->net);
/* Re-use the existing target if it's already loaded. */
- list_for_each_entry(nft_target, &nft_target_list, head) {
+ list_for_each_entry(nft_target, &cn->nft_target_list, head) {
struct xt_target *target = nft_target->ops.data;
if (!target->target)
@@ -907,7 +919,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
else
nft_target->ops.eval = nft_target_eval_xt;
- list_add(&nft_target->head, &nft_target_list);
+ list_add(&nft_target->head, &cn->nft_target_list);
return &nft_target->ops;
err:
@@ -923,13 +935,74 @@ static struct nft_expr_type nft_target_type __read_mostly = {
.owner = THIS_MODULE,
};
+static int __net_init nft_compat_init_net(struct net *net)
+{
+ struct nft_compat_net *cn = nft_compat_pernet(net);
+
+ INIT_LIST_HEAD(&cn->nft_target_list);
+ INIT_LIST_HEAD(&cn->nft_match_list);
+
+ return 0;
+}
+
+static void __net_exit nft_compat_exit_net(struct net *net)
+{
+ struct nft_compat_net *cn = nft_compat_pernet(net);
+ struct nft_xt *xt, *next;
+
+ if (list_empty(&cn->nft_match_list) &&
+ list_empty(&cn->nft_target_list))
+ return;
+
+ /* If there was an error that caused nft_xt expr to not be initialized
+ * fully and noone else requested the same expression later, the lists
+ * contain 0-refcount entries that still hold module reference.
+ *
+ * Clean them here.
+ */
+ mutex_lock(&net->nft.commit_mutex);
+ list_for_each_entry_safe(xt, next, &cn->nft_target_list, head) {
+ struct xt_target *target = xt->ops.data;
+
+ list_del_init(&xt->head);
+
+ if (refcount_read(&xt->refcnt))
+ continue;
+ module_put(target->me);
+ kfree(xt);
+ }
+
+ list_for_each_entry_safe(xt, next, &cn->nft_match_list, head) {
+ struct xt_match *match = xt->ops.data;
+
+ list_del_init(&xt->head);
+
+ if (refcount_read(&xt->refcnt))
+ continue;
+ module_put(match->me);
+ kfree(xt);
+ }
+ mutex_unlock(&net->nft.commit_mutex);
+}
+
+static struct pernet_operations nft_compat_net_ops = {
+ .init = nft_compat_init_net,
+ .exit = nft_compat_exit_net,
+ .id = &nft_compat_net_id,
+ .size = sizeof(struct nft_compat_net),
+};
+
static int __init nft_compat_module_init(void)
{
int ret;
+ ret = register_pernet_subsys(&nft_compat_net_ops);
+ if (ret < 0)
+ goto err_target;
+
ret = nft_register_expr(&nft_match_type);
if (ret < 0)
- return ret;
+ goto err_pernet;
ret = nft_register_expr(&nft_target_type);
if (ret < 0)
@@ -942,45 +1015,21 @@ static int __init nft_compat_module_init(void)
}
return ret;
-
err_target:
nft_unregister_expr(&nft_target_type);
err_match:
nft_unregister_expr(&nft_match_type);
+err_pernet:
+ unregister_pernet_subsys(&nft_compat_net_ops);
return ret;
}
static void __exit nft_compat_module_exit(void)
{
- struct nft_xt *xt, *next;
-
- /* list should be empty here, it can be non-empty only in case there
- * was an error that caused nft_xt expr to not be initialized fully
- * and noone else requested the same expression later.
- *
- * In this case, the lists contain 0-refcount entries that still
- * hold module reference.
- */
- list_for_each_entry_safe(xt, next, &nft_target_list, head) {
- struct xt_target *target = xt->ops.data;
-
- if (WARN_ON_ONCE(refcount_read(&xt->refcnt)))
- continue;
- module_put(target->me);
- kfree(xt);
- }
-
- list_for_each_entry_safe(xt, next, &nft_match_list, head) {
- struct xt_match *match = xt->ops.data;
-
- if (WARN_ON_ONCE(refcount_read(&xt->refcnt)))
- continue;
- module_put(match->me);
- kfree(xt);
- }
nfnetlink_subsys_unregister(&nfnl_compat_subsys);
nft_unregister_expr(&nft_target_type);
nft_unregister_expr(&nft_match_type);
+ unregister_pernet_subsys(&nft_compat_net_ops);
}
MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_NFT_COMPAT);
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 07/72] netfilter: nft_compat: destroy function must not have side effects
From: Sasha Levin @ 2019-02-23 21:03 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Florian Westphal, Pablo Neira Ayuso, Sasha Levin, netfilter-devel,
coreteam, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Florian Westphal <fw@strlen.de>
[ Upstream commit b2e3d68d1251a051a620f9086e18f7ffa6833b5b ]
The nft_compat destroy function deletes the nft_xt object from a list.
This isn't allowed anymore. Destroy functions are called asynchronously,
i.e. next batch can find the object that has a pending ->destroy()
invocation:
cpu0 cpu1
worker
->destroy for_each_entry()
if (x == ...
return x->ops;
list_del(x)
kfree_rcu(x)
expr->ops->... // ops was free'd
To resolve this, the list_del needs to occur before the transaction
mutex gets released. nf_tables has a 'deactivate' hook for this
purpose, so use that to unlink the object from the list.
Fixes: 0935d5588400 ("netfilter: nf_tables: asynchronous release")
Reported-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/netfilter/nft_compat.c | 48 +++++++++++++++++++++++++++++++++++++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c
index abed3490a8f8a..5eb269428832c 100644
--- a/net/netfilter/nft_compat.c
+++ b/net/netfilter/nft_compat.c
@@ -29,6 +29,9 @@ struct nft_xt {
struct nft_expr_ops ops;
refcount_t refcnt;
+ /* used only when transaction mutex is locked */
+ unsigned int listcnt;
+
/* Unlike other expressions, ops doesn't have static storage duration.
* nft core assumes they do. We use kfree_rcu so that nft core can
* can check expr->ops->size even after nft_compat->destroy() frees
@@ -61,7 +64,7 @@ static struct nft_compat_net *nft_compat_pernet(struct net *net)
static bool nft_xt_put(struct nft_xt *xt)
{
if (refcount_dec_and_test(&xt->refcnt)) {
- list_del(&xt->head);
+ WARN_ON_ONCE(!list_empty(&xt->head));
kfree_rcu(xt, rcu_head);
return true;
}
@@ -555,6 +558,43 @@ nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr)
__nft_match_destroy(ctx, expr, nft_expr_priv(expr));
}
+static void nft_compat_activate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ struct list_head *h)
+{
+ struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
+
+ if (xt->listcnt == 0)
+ list_add(&xt->head, h);
+
+ xt->listcnt++;
+}
+
+static void nft_compat_activate_mt(const struct nft_ctx *ctx,
+ const struct nft_expr *expr)
+{
+ struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
+
+ nft_compat_activate(ctx, expr, &cn->nft_match_list);
+}
+
+static void nft_compat_activate_tg(const struct nft_ctx *ctx,
+ const struct nft_expr *expr)
+{
+ struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
+
+ nft_compat_activate(ctx, expr, &cn->nft_target_list);
+}
+
+static void nft_compat_deactivate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr)
+{
+ struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
+
+ if (--xt->listcnt == 0)
+ list_del_init(&xt->head);
+}
+
static void
nft_match_large_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr)
{
@@ -808,6 +848,8 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.eval = nft_match_eval;
nft_match->ops.init = nft_match_init;
nft_match->ops.destroy = nft_match_destroy;
+ nft_match->ops.activate = nft_compat_activate_mt;
+ nft_match->ops.deactivate = nft_compat_deactivate;
nft_match->ops.dump = nft_match_dump;
nft_match->ops.validate = nft_match_validate;
nft_match->ops.data = match;
@@ -824,6 +866,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.size = matchsize;
+ nft_match->listcnt = 1;
list_add(&nft_match->head, &cn->nft_match_list);
return &nft_match->ops;
@@ -910,6 +953,8 @@ nft_target_select_ops(const struct nft_ctx *ctx,
nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
nft_target->ops.init = nft_target_init;
nft_target->ops.destroy = nft_target_destroy;
+ nft_target->ops.activate = nft_compat_activate_tg;
+ nft_target->ops.deactivate = nft_compat_deactivate;
nft_target->ops.dump = nft_target_dump;
nft_target->ops.validate = nft_target_validate;
nft_target->ops.data = target;
@@ -919,6 +964,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
else
nft_target->ops.eval = nft_target_eval_xt;
+ nft_target->listcnt = 1;
list_add(&nft_target->head, &cn->nft_target_list);
return &nft_target->ops;
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 43/72] qed: Fix bug in tx promiscuous mode settings
From: Sasha Levin @ 2019-02-23 21:03 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Manish Chopra, Ariel Elior, David S . Miller, Sasha Levin, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Manish Chopra <manishc@marvell.com>
[ Upstream commit 9e71a15d8b5bbce25c637f7f8833cd3f45b65646 ]
When running tx switched traffic between VNICs
created via a bridge(to which VFs are added),
adapter drops the unicast packets in tx flow due to
VNIC's ucast mac being unknown to it. But VF interfaces
being in promiscuous mode should have caused adapter
to accept all the unknown ucast packets. Later, it
was found that driver doesn't really configure tx
promiscuous mode settings to accept all unknown unicast macs.
This patch fixes tx promiscuous mode settings to accept all
unknown/unmatched unicast macs and works out the scenario.
Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/qlogic/qed/qed_l2.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_l2.c b/drivers/net/ethernet/qlogic/qed/qed_l2.c
index 67c02ea939062..b8baa6fcef8e3 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_l2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_l2.c
@@ -609,6 +609,10 @@ qed_sp_update_accept_mode(struct qed_hwfn *p_hwfn,
(!!(accept_filter & QED_ACCEPT_MCAST_MATCHED) &&
!!(accept_filter & QED_ACCEPT_MCAST_UNMATCHED)));
+ SET_FIELD(state, ETH_VPORT_TX_MODE_UCAST_ACCEPT_ALL,
+ (!!(accept_filter & QED_ACCEPT_UCAST_MATCHED) &&
+ !!(accept_filter & QED_ACCEPT_UCAST_UNMATCHED)));
+
SET_FIELD(state, ETH_VPORT_TX_MODE_BCAST_ACCEPT_ALL,
!!(accept_filter & QED_ACCEPT_BCAST));
@@ -2688,7 +2692,8 @@ static int qed_configure_filter_rx_mode(struct qed_dev *cdev,
if (type == QED_FILTER_RX_MODE_TYPE_PROMISC) {
accept_flags.rx_accept_filter |= QED_ACCEPT_UCAST_UNMATCHED |
QED_ACCEPT_MCAST_UNMATCHED;
- accept_flags.tx_accept_filter |= QED_ACCEPT_MCAST_UNMATCHED;
+ accept_flags.tx_accept_filter |= QED_ACCEPT_UCAST_UNMATCHED |
+ QED_ACCEPT_MCAST_UNMATCHED;
} else if (type == QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC) {
accept_flags.rx_accept_filter |= QED_ACCEPT_MCAST_UNMATCHED;
accept_flags.tx_accept_filter |= QED_ACCEPT_MCAST_UNMATCHED;
--
2.19.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.20 55/72] net: macb: Apply RXUBR workaround only to versions with errata
From: Sasha Levin @ 2019-02-23 21:04 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Harini Katakam, David S . Miller, Sasha Levin, netdev
In-Reply-To: <20190223210422.199966-1-sashal@kernel.org>
From: Harini Katakam <harini.katakam@xilinx.com>
[ Upstream commit e501070e4db0b67a4c17a5557d1e9d098f3db310 ]
The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91RM9200 only. Subsequent versions do not need this
workaround. This workaround unnecessarily resets RX whenever RX used
bit read is observed, which can be often under heavy traffic. There
is no other action performed on RX UBR interrupt. Hence introduce a
CAPS mask; enable this interrupt and workaround only on affected
versions.
Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/net/ethernet/cadence/macb.h | 3 +++
drivers/net/ethernet/cadence/macb_main.c | 28 ++++++++++++++----------
2 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c92cf6e..9bbaad9f3d632 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -643,6 +643,7 @@
#define MACB_CAPS_JUMBO 0x00000020
#define MACB_CAPS_GEM_HAS_PTP 0x00000040
#define MACB_CAPS_BD_RD_PREFETCH 0x00000080
+#define MACB_CAPS_NEEDS_RSTONUBR 0x00000100
#define MACB_CAPS_FIFO_MODE 0x10000000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
@@ -1214,6 +1215,8 @@ struct macb {
int rx_bd_rd_prefetch;
int tx_bd_rd_prefetch;
+
+ u32 rx_intr_mask;
};
#ifdef CONFIG_MACB_USE_HWSTAMP
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 4c816e5a841fa..73aa7969db962 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -56,8 +56,7 @@
/* level of occupied TX descriptors under which we wake up TX process */
#define MACB_TX_WAKEUP_THRESH(bp) (3 * (bp)->tx_ring_size / 4)
-#define MACB_RX_INT_FLAGS (MACB_BIT(RCOMP) | MACB_BIT(RXUBR) \
- | MACB_BIT(ISR_ROVR))
+#define MACB_RX_INT_FLAGS (MACB_BIT(RCOMP) | MACB_BIT(ISR_ROVR))
#define MACB_TX_ERR_FLAGS (MACB_BIT(ISR_TUND) \
| MACB_BIT(ISR_RLE) \
| MACB_BIT(TXERR))
@@ -1270,7 +1269,7 @@ static int macb_poll(struct napi_struct *napi, int budget)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
napi_reschedule(napi);
} else {
- queue_writel(queue, IER, MACB_RX_INT_FLAGS);
+ queue_writel(queue, IER, bp->rx_intr_mask);
}
}
@@ -1288,7 +1287,7 @@ static void macb_hresp_error_task(unsigned long data)
u32 ctrl;
for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
- queue_writel(queue, IDR, MACB_RX_INT_FLAGS |
+ queue_writel(queue, IDR, bp->rx_intr_mask |
MACB_TX_INT_FLAGS |
MACB_BIT(HRESP));
}
@@ -1318,7 +1317,7 @@ static void macb_hresp_error_task(unsigned long data)
/* Enable interrupts */
queue_writel(queue, IER,
- MACB_RX_INT_FLAGS |
+ bp->rx_intr_mask |
MACB_TX_INT_FLAGS |
MACB_BIT(HRESP));
}
@@ -1372,14 +1371,14 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
(unsigned int)(queue - bp->queues),
(unsigned long)status);
- if (status & MACB_RX_INT_FLAGS) {
+ if (status & bp->rx_intr_mask) {
/* There's no point taking any more interrupts
* until we have processed the buffers. The
* scheduling call may fail if the poll routine
* is already scheduled, so disable interrupts
* now.
*/
- queue_writel(queue, IDR, MACB_RX_INT_FLAGS);
+ queue_writel(queue, IDR, bp->rx_intr_mask);
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RCOMP));
@@ -1412,8 +1411,9 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
/* There is a hardware issue under heavy load where DMA can
* stop, this causes endless "used buffer descriptor read"
* interrupts but it can be cleared by re-enabling RX. See
- * the at91 manual, section 41.3.1 or the Zynq manual
- * section 16.7.4 for details.
+ * the at91rm9200 manual, section 41.3.1 or the Zynq manual
+ * section 16.7.4 for details. RXUBR is only enabled for
+ * these two versions.
*/
if (status & MACB_BIT(RXUBR)) {
ctrl = macb_readl(bp, NCR);
@@ -2263,7 +2263,7 @@ static void macb_init_hw(struct macb *bp)
/* Enable interrupts */
queue_writel(queue, IER,
- MACB_RX_INT_FLAGS |
+ bp->rx_intr_mask |
MACB_TX_INT_FLAGS |
MACB_BIT(HRESP));
}
@@ -3911,6 +3911,7 @@ static const struct macb_config sama5d4_config = {
};
static const struct macb_config emac_config = {
+ .caps = MACB_CAPS_NEEDS_RSTONUBR,
.clk_init = at91ether_clk_init,
.init = at91ether_init,
};
@@ -3932,7 +3933,8 @@ static const struct macb_config zynqmp_config = {
};
static const struct macb_config zynq_config = {
- .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF,
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
+ MACB_CAPS_NEEDS_RSTONUBR,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
@@ -4087,6 +4089,10 @@ static int macb_probe(struct platform_device *pdev)
macb_dma_desc_get_size(bp);
}
+ bp->rx_intr_mask = MACB_RX_INT_FLAGS;
+ if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR)
+ bp->rx_intr_mask |= MACB_BIT(RXUBR);
+
mac = of_get_mac_address(np);
if (mac) {
ether_addr_copy(bp->dev->dev_addr, mac);
--
2.19.1
^ 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