* Re: [PATCH] net: mvneta: replace MVNETA_CPU_D_CACHE_LINE_SIZE with L1_CACHE_BYTES
From: Thomas Petazzoni @ 2016-03-31 20:37 UTC (permalink / raw)
To: David Miller; +Cc: jszhang, netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <20160331.151547.1889188465826831929.davem@davemloft.net>
Hello,
On Thu, 31 Mar 2016 15:15:47 -0400 (EDT), David Miller wrote:
> From: Jisheng Zhang <jszhang@marvell.com>
> Date: Wed, 30 Mar 2016 19:55:21 +0800
>
> > The mvneta is also used in some Marvell berlin family SoCs which may
> > have 64bytes cacheline size. Replace the MVNETA_CPU_D_CACHE_LINE_SIZE
> > usage with L1_CACHE_BYTES.
> >
> > And since dma_alloc_coherent() is always cacheline size aligned, so
> > remove the align checks.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
>
> Applied.
A new version of the patch was sent, which more rightfully uses
cache_line_size(), see:
"[PATCH v2] net: mvneta: replace MVNETA_CPU_D_CACHE_LINE_SIZE with cache_line_size"
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2] rds: rds-stress show all zeros after few minutes
From: David Miller @ 2016-03-31 20:02 UTC (permalink / raw)
To: shamir.rabinovitch; +Cc: rds-devel, netdev
In-Reply-To: <1459405762-29778-1-git-send-email-shamir.rabinovitch@oracle.com>
From: shamir rabinovitch <shamir.rabinovitch@oracle.com>
Date: Thu, 31 Mar 2016 02:29:22 -0400
> Issue can be seen on platforms that use 8K and above page size
> while rds fragment size is 4K. On those platforms single page is
> shared between 2 or more rds fragments. Each fragment has its own
> offset and rds congestion map code need to take this offset to account.
> Not taking this offset to account lead to reading the data fragment
> as congestion map fragment and hang of the rds transmit due to far
> congestion map corruption.
>
> Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
>
> Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>
> Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Tested-by: Anand Bibhuti <anand.bibhuti@oracle.com>
This doesn't apply cleanly to my current tree, please respin.
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: skbuff: don't use union for napi_id and sender_cpu
From: David Miller @ 2016-03-31 20:01 UTC (permalink / raw)
To: eric.dumazet; +Cc: jasowang, mst, netdev, linux-kernel
In-Reply-To: <1459420341.6473.225.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 31 Mar 2016 03:32:21 -0700
> On Thu, 2016-03-31 at 13:50 +0800, Jason Wang wrote:
>> We use a union for napi_id and send_cpu, this is ok for most of the
>> cases except when we want to support busy polling for tun which needs
>> napi_id to be stored and passed to socket during tun_net_xmit(). In
>> this case, napi_id was overridden with sender_cpu before tun_net_xmit()
>> was called if XPS was enabled. Fixing by not using union for napi_id
>> and sender_cpu.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> include/linux/skbuff.h | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 15d0df9..8aee891 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -743,11 +743,11 @@ struct sk_buff {
>> __u32 hash;
>> __be16 vlan_proto;
>> __u16 vlan_tci;
>> -#if defined(CONFIG_NET_RX_BUSY_POLL) || defined(CONFIG_XPS)
>> - union {
>> - unsigned int napi_id;
>> - unsigned int sender_cpu;
>> - };
>> +#if defined(CONFIG_NET_RX_BUSY_POLL)
>> + unsigned int napi_id;
>> +#endif
>> +#if defined(CONFIG_XPS)
>> + unsigned int sender_cpu;
>> #endif
>> union {
>> #ifdef CONFIG_NETWORK_SECMARK
>
> Hmmm...
>
> This is a serious problem.
>
> Making skb bigger (8 bytes because of alignment) was not considered
> valid for sender_cpu introduction. We worked quite hard to avoid this,
> if you take a look at git history :(
>
> Can you describe more precisely the problem and code path ?
>From what I can see they are doing busy poll loops in the TX code paths,
as well as the RX code paths, of vhost.
Doing this in the TX side makes little sense to me. The busy poll
implementations in the drivers only process their RX queues when
->ndo_busy_poll() is invoked. So I wonder what this is accomplishing
for the vhost TX case?
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: David Miller @ 2016-03-31 19:50 UTC (permalink / raw)
To: hannes
Cc: daniel, eric.dumazet, alexei.starovoitov, mkubecek, sasha.levin,
jslaby, mst, netdev
In-Reply-To: <56FD7F0B.5090602@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 31 Mar 2016 21:48:27 +0200
> Tightest solution would probably be to combine both patches.
>
> bool called_by_tuntap;
>
> old_fp = rcu_dereference_protected(sk->sk_filter, called_by_tuntap ?
> lockdep_rtnl_is_held() : lockdep_sock_is_held());
Ok, I see what you're saying.
I misunderstood how the RTNL lockdep checks work and thought we could
get false positives from other entities taking RTNL.
Can you cook up the combined patch?
Thanks.
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: David Miller @ 2016-03-31 19:48 UTC (permalink / raw)
To: alexei.starovoitov
Cc: hannes, daniel, eric.dumazet, mkubecek, sasha.levin, jslaby, mst,
netdev
In-Reply-To: <20160331193154.GA63937@ast-mbp.thefacebook.com>
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date: Thu, 31 Mar 2016 12:31:56 -0700
> On Thu, Mar 31, 2016 at 09:24:12PM +0200, Hannes Frederic Sowa wrote:
>> Hello,
>>
>> On 31.03.2016 21:21, David Miller wrote:
>> >From: Daniel Borkmann <daniel@iogearbox.net>
>> >Date: Thu, 31 Mar 2016 14:16:18 +0200
>> >
>> >Alexei, do you really mind if I apply Danile's patch?
>
> I don't have strong opinion either way.
> Though Hannes's patch below looks simpler and easier to backport.
> Yeah, I do care about backports quite a bit more nowadays :)
You know, I care a lot about backports too :)
But Hannes's patch has the same fundamental issue, I think.
If we accept both synchornization styles, false positives are more
likely.
And in the long term, we can fix the false positive possibilities with
the RTNL checks.
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: Hannes Frederic Sowa @ 2016-03-31 19:48 UTC (permalink / raw)
To: David Miller
Cc: daniel, eric.dumazet, alexei.starovoitov, mkubecek, sasha.levin,
jslaby, mst, netdev
In-Reply-To: <20160331.153630.1640223846173244431.davem@davemloft.net>
On 31.03.2016 21:36, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Thu, 31 Mar 2016 21:24:12 +0200
>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 4b81b71171b4ce..8ab270d5ce5507 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -1166,7 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog
>> *prog, struct sock *sk)
>> }
>>
>> old_fp = rcu_dereference_protected(sk->sk_filter,
>> - sock_owned_by_user(sk));
>> + lockdep_rtnl_is_held() ||
>> + lockdep_sock_is_held(sk));
>> rcu_assign_pointer(sk->sk_filter, fp);
>>
>> if (old_fp)
>
> I have the same objections Daniel did.
>
> Not all socket filter clients use RTNL as the synchornization
> mechanism. The caller, or some descriptive element, should tell us
> what that synchronizing element is.
>
> Yes, I understand how these RTNL checks can pass "accidently" but
> the opposite is true too. A socket locking synchornizing user,
> who didn't lock the socket, might now pass because RTNL happens
> to be held elsewhere.
Actually lockdep_rtnl_is_held checks if this specific code/thread holds
the lock and no other cpu/thread. So it will not pass here in case
another cpu has the lock.
lockdep stores the current held locks in current->held_locks, if we
preempt we switch current pointer, if we take a spin_lock we can't sleep
thus not preempt. Thus we always know that this specific code has the lock.
Using sock_owned_by_user actually has this problem, and thus I am
replacing it. We don't know who has the socket locked.
Tightest solution would probably be to combine both patches.
bool called_by_tuntap;
old_fp = rcu_dereference_protected(sk->sk_filter, called_by_tuntap ?
lockdep_rtnl_is_held() : lockdep_sock_is_held());
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: David Miller @ 2016-03-31 19:36 UTC (permalink / raw)
To: hannes
Cc: daniel, eric.dumazet, alexei.starovoitov, mkubecek, sasha.levin,
jslaby, mst, netdev
In-Reply-To: <56FD795C.9090903@stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 31 Mar 2016 21:24:12 +0200
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4b81b71171b4ce..8ab270d5ce5507 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1166,7 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog
> *prog, struct sock *sk)
> }
>
> old_fp = rcu_dereference_protected(sk->sk_filter,
> - sock_owned_by_user(sk));
> + lockdep_rtnl_is_held() ||
> + lockdep_sock_is_held(sk));
> rcu_assign_pointer(sk->sk_filter, fp);
>
> if (old_fp)
I have the same objections Daniel did.
Not all socket filter clients use RTNL as the synchornization
mechanism. The caller, or some descriptive element, should tell us
what that synchronizing element is.
Yes, I understand how these RTNL checks can pass "accidently" but
the opposite is true too. A socket locking synchornizing user,
who didn't lock the socket, might now pass because RTNL happens
to be held elsewhere.
Constraining the test properly, based upon the user, makes this less
likely to happen. And that's desirable.
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: Alexei Starovoitov @ 2016-03-31 19:31 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, daniel, eric.dumazet, mkubecek, sasha.levin, jslaby,
mst, netdev
In-Reply-To: <56FD795C.9090903@stressinduktion.org>
On Thu, Mar 31, 2016 at 09:24:12PM +0200, Hannes Frederic Sowa wrote:
> Hello,
>
> On 31.03.2016 21:21, David Miller wrote:
> >From: Daniel Borkmann <daniel@iogearbox.net>
> >Date: Thu, 31 Mar 2016 14:16:18 +0200
> >
> >>On 03/31/2016 01:59 PM, Eric Dumazet wrote:
> >>>On Thu, 2016-03-31 at 13:35 +0200, Daniel Borkmann wrote:
> >>>
> >>>>+static inline bool sock_owned_externally(const struct sock *sk)
> >>>>+{
> >>>>+ return sk->sk_flags & (1UL << SOCK_EXTERNAL_OWNER);
> >>>>+}
> >>>>+
> >>>
> >>>Have you reinvented sock_flag(sl, SOCK_EXTERNAL_OWNER) ? ;)
> >>>
> >>>Anyway, using a flag for this purpose sounds overkill to me.
> >>
> >>Right.
> >>
> >>>Setting it is a way to 'fool' lockdep anyway...
> >>
> >>Yep, correct, we'd be fooling the tun case, so this diff doesn't
> >>really make it any better there.
> >
> >I like the currently proposed patch where TUN says that RTNL is what
> >the synchronizing element is.
> >
> >Maybe we could make a helper of some sort but since we only have once
> >case like this is just overkill.
> >
> >Alexei, do you really mind if I apply Danile's patch?
I don't have strong opinion either way.
Though Hannes's patch below looks simpler and easier to backport.
Yeah, I do care about backports quite a bit more nowadays :)
> I proposed the following patch to Daniel and he seemed to like it. I
> was just waiting for his feedback and tags and wanted to send it out
> then.
>
> What do you think?
>
> lockdep_sock_is_held does also have some other applications in other
> parts of the source.
>
> Bye,
> Hannes
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 255d3e03727b73..651b84a38cfb9b 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1330,6 +1330,12 @@ static inline void sock_release_ownership(struct sock
> *sk)
> sk->sk_lock.owned = 0;
> }
>
> +static inline bool lockdep_sock_is_held(struct sock *sk)
> +{
> + return lockdep_is_held(&sk->sk_lock) ||
> + lockdep_is_held(&sk->sk_lock.slock);
> +}
> +
> /*
> * Macro so as to not evaluate some arguments when
> * lockdep is not enabled.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4b81b71171b4ce..8ab270d5ce5507 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1166,7 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog *prog,
> struct sock *sk)
> }
>
> old_fp = rcu_dereference_protected(sk->sk_filter,
> - sock_owned_by_user(sk));
> + lockdep_rtnl_is_held() ||
> + lockdep_sock_is_held(sk));
> rcu_assign_pointer(sk->sk_filter, fp);
>
> if (old_fp)
> @@ -2259,7 +2260,9 @@ int sk_detach_filter(struct sock *sk)
> return -EPERM;
>
> filter = rcu_dereference_protected(sk->sk_filter,
> - sock_owned_by_user(sk));
> + lockdep_rtnl_is_held() ||
> + lockdep_sock_is_held(sk));
> +
> if (filter) {
> RCU_INIT_POINTER(sk->sk_filter, NULL);
> sk_filter_uncharge(sk, filter);
>
^ permalink raw reply
* Re: [PATCH 3/4] net: w5100: enable to support sleepable register access interface
From: David Miller @ 2016-03-31 19:30 UTC (permalink / raw)
To: akinobu.mita; +Cc: netdev, msink
In-Reply-To: <1459355920-14623-3-git-send-email-akinobu.mita@gmail.com>
From: Akinobu Mita <akinobu.mita@gmail.com>
Date: Thu, 31 Mar 2016 01:38:39 +0900
> + struct sk_buff_head tx_queue;
The way the queueing works in this driver is that it is only possible
to have one SKB being transmitted at one time.
This is evident by how the driver immediately stops the TX queue when
it is given a new packet to transmit, and this is woken up by the TX
completion IRQ.
So don't use a queue here, just use a plain single pointer.
The SKB queue you're using here is going to also do locking which is
even more unnecessary overhead.
Thanks.
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: Hannes Frederic Sowa @ 2016-03-31 19:24 UTC (permalink / raw)
To: David Miller, daniel
Cc: eric.dumazet, alexei.starovoitov, mkubecek, sasha.levin, jslaby,
mst, netdev
In-Reply-To: <20160331.152149.396188904137423987.davem@davemloft.net>
Hello,
On 31.03.2016 21:21, David Miller wrote:
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Thu, 31 Mar 2016 14:16:18 +0200
>
>> On 03/31/2016 01:59 PM, Eric Dumazet wrote:
>>> On Thu, 2016-03-31 at 13:35 +0200, Daniel Borkmann wrote:
>>>
>>>> +static inline bool sock_owned_externally(const struct sock *sk)
>>>> +{
>>>> + return sk->sk_flags & (1UL << SOCK_EXTERNAL_OWNER);
>>>> +}
>>>> +
>>>
>>> Have you reinvented sock_flag(sl, SOCK_EXTERNAL_OWNER) ? ;)
>>>
>>> Anyway, using a flag for this purpose sounds overkill to me.
>>
>> Right.
>>
>>> Setting it is a way to 'fool' lockdep anyway...
>>
>> Yep, correct, we'd be fooling the tun case, so this diff doesn't
>> really make it any better there.
>
> I like the currently proposed patch where TUN says that RTNL is what
> the synchronizing element is.
>
> Maybe we could make a helper of some sort but since we only have once
> case like this is just overkill.
>
> Alexei, do you really mind if I apply Danile's patch?
I proposed the following patch to Daniel and he seemed to like it. I
was just waiting for his feedback and tags and wanted to send it out
then.
What do you think?
lockdep_sock_is_held does also have some other applications in other
parts of the source.
Bye,
Hannes
diff --git a/include/net/sock.h b/include/net/sock.h
index 255d3e03727b73..651b84a38cfb9b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1330,6 +1330,12 @@ static inline void sock_release_ownership(struct
sock *sk)
sk->sk_lock.owned = 0;
}
+static inline bool lockdep_sock_is_held(struct sock *sk)
+{
+ return lockdep_is_held(&sk->sk_lock) ||
+ lockdep_is_held(&sk->sk_lock.slock);
+}
+
/*
* Macro so as to not evaluate some arguments when
* lockdep is not enabled.
diff --git a/net/core/filter.c b/net/core/filter.c
index 4b81b71171b4ce..8ab270d5ce5507 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1166,7 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog *prog,
struct sock *sk)
}
old_fp = rcu_dereference_protected(sk->sk_filter,
- sock_owned_by_user(sk));
+ lockdep_rtnl_is_held() ||
+ lockdep_sock_is_held(sk));
rcu_assign_pointer(sk->sk_filter, fp);
if (old_fp)
@@ -2259,7 +2260,9 @@ int sk_detach_filter(struct sock *sk)
return -EPERM;
filter = rcu_dereference_protected(sk->sk_filter,
- sock_owned_by_user(sk));
+ lockdep_rtnl_is_held() ||
+ lockdep_sock_is_held(sk));
+
if (filter) {
RCU_INIT_POINTER(sk->sk_filter, NULL);
sk_filter_uncharge(sk, filter);
c
^ permalink raw reply related
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: David Miller @ 2016-03-31 19:21 UTC (permalink / raw)
To: daniel
Cc: eric.dumazet, alexei.starovoitov, mkubecek, sasha.levin, jslaby,
mst, netdev
In-Reply-To: <56FD1512.70409@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 31 Mar 2016 14:16:18 +0200
> On 03/31/2016 01:59 PM, Eric Dumazet wrote:
>> On Thu, 2016-03-31 at 13:35 +0200, Daniel Borkmann wrote:
>>
>>> +static inline bool sock_owned_externally(const struct sock *sk)
>>> +{
>>> + return sk->sk_flags & (1UL << SOCK_EXTERNAL_OWNER);
>>> +}
>>> +
>>
>> Have you reinvented sock_flag(sl, SOCK_EXTERNAL_OWNER) ? ;)
>>
>> Anyway, using a flag for this purpose sounds overkill to me.
>
> Right.
>
>> Setting it is a way to 'fool' lockdep anyway...
>
> Yep, correct, we'd be fooling the tun case, so this diff doesn't
> really make it any better there.
I like the currently proposed patch where TUN says that RTNL is what
the synchronizing element is.
Maybe we could make a helper of some sort but since we only have once
case like this is just overkill.
Alexei, do you really mind if I apply Danile's patch?
Thanks.
^ permalink raw reply
* Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples
From: Alexei Starovoitov @ 2016-03-31 19:20 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, linuxppc-dev, David S . Miller,
Ananth N Mavinakayanahalli, Michael Ellerman, Daniel Borkmann,
netdev
In-Reply-To: <20160331185139.GD17907@naverao1-tp.ibm.com>
On 3/31/16 11:51 AM, Naveen N. Rao wrote:
> On 2016/03/31 10:49AM, Alexei Starovoitov wrote:
>> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>>> Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
>>> Kconfig option since that will add a dependency on llvm for allyesconfig
>>> builds which may not be desirable.
>>>
>>> Those who need to build the BPF samples can now just do:
>>>
>>> make CONFIG_SAMPLE_BPF=y
>>>
>>> or:
>>>
>>> export CONFIG_SAMPLE_BPF=y
>>> make
>>
>> I don't like this 'simplification'.
>> make samples/bpf/
>> is much easier to type than capital letters.
>
> This started out as a patch to have the BPF samples built with a Kconfig
> option. As stated in the commit description, I realised that it won't
> work for allyesconfig builds. However, the reason I retained this patch
> is since it gets us one step closer to building the samples as part of
> the kernel build.
>
> The 'simplification' is since I can now have the export in my .bashrc
> and the kernel build will now build the BPF samples too without
> requiring an additional 'make samples/bpf/' step.
>
> I agree this is subjective, so I am ok if this isn't taken in.
If you can change it that 'make samples/bpf/' still works then it would
be fine. As it is it breaks our testing setup.
^ permalink raw reply
* Re: [PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c
From: Alexei Starovoitov @ 2016-03-31 19:19 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, linuxppc-dev, David S . Miller,
Ananth N Mavinakayanahalli, Michael Ellerman, Daniel Borkmann,
netdev
In-Reply-To: <20160331184602.GC17907@naverao1-tp.ibm.com>
On 3/31/16 11:46 AM, Naveen N. Rao wrote:
>> It's failing this way on powerpc? Odd.
> This fails for me on x86_64 too -- RHEL 7.1.
indeed. fails on centos 7.1, whereas centos 6.7 is fine.
^ permalink raw reply
* Re: qdisc spin lock
From: Jesper Dangaard Brouer @ 2016-03-31 19:18 UTC (permalink / raw)
To: Michael Ma; +Cc: brouer, netdev
In-Reply-To: <CAAmHdhw9bQkCm7uehRZ9mTetMzafdXxWhYj16f8W-YvSz8V4=g@mail.gmail.com>
On Wed, 30 Mar 2016 00:20:03 -0700 Michael Ma <make0818@gmail.com> wrote:
> I know this might be an old topic so bare with me – what we are facing
> is that applications are sending small packets using hundreds of
> threads so the contention on spin lock in __dev_xmit_skb increases the
> latency of dev_queue_xmit significantly. We’re building a network QoS
> solution to avoid interference of different applications using HTB.
Yes, as you have noticed with HTB there is a single qdisc lock, and
congestion obviously happens :-)
It is possible with different tricks to make it scale. I believe
Google is using a variant of HTB, and it scales for them. They have
not open source their modifications to HTB (which likely also involves
a great deal of setup tricks).
If your purpose it to limit traffic/bandwidth per "cloud" instance,
then you can just use another TC setup structure. Like using MQ and
assigning a HTB per MQ queue (where the MQ queues are bound to each
CPU/HW queue)... But you have to figure out this setup yourself...
> But in this case when some applications send massive small packets in
> parallel, the application to be protected will get its throughput
> affected (because it’s doing synchronous network communication using
> multiple threads and throughput is sensitive to the increased latency)
>
> Here is the profiling from perf:
>
> - 67.57% iperf [kernel.kallsyms] [k] _spin_lock
> - 99.94% dev_queue_xmit
> - 96.91% _spin_lock
> - 2.62% __qdisc_run
> - 98.98% sch_direct_xmit
> - 99.98% _spin_lock
>
> As far as I understand the design of TC is to simplify locking schema
> and minimize the work in __qdisc_run so that throughput won’t be
> affected, especially with large packets. However if the scenario is
> that multiple classes in the queueing discipline only have the shaping
> limit, there isn’t really a necessary correlation between different
> classes. The only synchronization point should be when the packet is
> dequeued from the qdisc queue and enqueued to the transmit queue of
> the device. My question is – is it worth investing on avoiding the
> locking contention by partitioning the queue/lock so that this
> scenario is addressed with relatively smaller latency?
Yes, there is a lot go gain, but it is not easy ;-)
> I must have oversimplified a lot of details since I’m not familiar
> with the TC implementation at this point – just want to get your input
> in terms of whether this is a worthwhile effort or there is something
> fundamental that I’m not aware of. If this is just a matter of quite
> some additional work, would also appreciate helping to outline the
> required work here.
>
> Also would appreciate if there is any information about the latest
> status of this work http://www.ijcset.com/docs/IJCSET13-04-04-113.pdf
This article seems to be very low quality... spelling errors, only 5
pages, no real code, etc.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH] net: mvneta: replace MVNETA_CPU_D_CACHE_LINE_SIZE with L1_CACHE_BYTES
From: David Miller @ 2016-03-31 19:15 UTC (permalink / raw)
To: jszhang; +Cc: thomas.petazzoni, netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1459338921-391-1-git-send-email-jszhang@marvell.com>
From: Jisheng Zhang <jszhang@marvell.com>
Date: Wed, 30 Mar 2016 19:55:21 +0800
> The mvneta is also used in some Marvell berlin family SoCs which may
> have 64bytes cacheline size. Replace the MVNETA_CPU_D_CACHE_LINE_SIZE
> usage with L1_CACHE_BYTES.
>
> And since dma_alloc_coherent() is always cacheline size aligned, so
> remove the align checks.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: mvpp2: replace MVPP2_CPU_D_CACHE_LINE_SIZE with L1_CACHE_BYTES
From: David Miller @ 2016-03-31 19:15 UTC (permalink / raw)
To: jszhang; +Cc: netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1459338821-343-1-git-send-email-jszhang@marvell.com>
From: Jisheng Zhang <jszhang@marvell.com>
Date: Wed, 30 Mar 2016 19:53:41 +0800
> The mvpp2 ip maybe used in SoCs which may have have 64bytes cacheline
> size. Replace the MVPP2_CPU_D_CACHE_LINE_SIZE with L1_CACHE_BYTES.
>
> And since dma_alloc_coherent() is always cacheline size aligned, so
> remove the align checks.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v3 2/2] net: dsa: mv88e6xxx: Clear the PDOWN bit on setup
From: David Miller @ 2016-03-31 19:13 UTC (permalink / raw)
To: patrick; +Cc: linux, vivien.didelot, andrew, netdev, dennis, pbrobinson
In-Reply-To: <1459301981-26535-2-git-send-email-patrick@puiterwijk.org>
From: Patrick Uiterwijk <patrick@puiterwijk.org>
Date: Wed, 30 Mar 2016 01:39:41 +0000
> Some of the vendor-specific bootloaders set up this part
> of the initialization for us, so this was never added.
> However, since upstream bootloaders don't initialize the
> chip specifically, they leave the fiber MII's PDOWN flag
> set, which means that the CPU port doesn't connect.
>
> This patch checks whether this flag has been clear prior
> by something else, and if not make us clear it.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] net: dsa: mv88e6xxx: Introduce _mv88e6xxx_phy_page_{read,write}
From: David Miller @ 2016-03-31 19:13 UTC (permalink / raw)
To: patrick; +Cc: linux, vivien.didelot, andrew, netdev, dennis, pbrobinson
In-Reply-To: <1459301981-26535-1-git-send-email-patrick@puiterwijk.org>
From: Patrick Uiterwijk <patrick@puiterwijk.org>
Date: Wed, 30 Mar 2016 01:39:40 +0000
> Add versions of the phy_page_read and _write functions to
> be used in a context where the SMI mutex is held.
>
> Tested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org>
Applied.
^ permalink raw reply
* Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples
From: Naveen N. Rao @ 2016-03-31 18:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: linux-kernel, linuxppc-dev, David S . Miller,
Ananth N Mavinakayanahalli, Michael Ellerman, Daniel Borkmann,
netdev
In-Reply-To: <56FD632B.4040306@fb.com>
On 2016/03/31 10:49AM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> >Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
> >Kconfig option since that will add a dependency on llvm for allyesconfig
> >builds which may not be desirable.
> >
> >Those who need to build the BPF samples can now just do:
> >
> >make CONFIG_SAMPLE_BPF=y
> >
> >or:
> >
> >export CONFIG_SAMPLE_BPF=y
> >make
>
> I don't like this 'simplification'.
> make samples/bpf/
> is much easier to type than capital letters.
This started out as a patch to have the BPF samples built with a Kconfig
option. As stated in the commit description, I realised that it won't
work for allyesconfig builds. However, the reason I retained this patch
is since it gets us one step closer to building the samples as part of
the kernel build.
The 'simplification' is since I can now have the export in my .bashrc
and the kernel build will now build the BPF samples too without
requiring an additional 'make samples/bpf/' step.
I agree this is subjective, so I am ok if this isn't taken in.
- Naveen
^ permalink raw reply
* Re: [PATCH 1/4] samples/bpf: Fix build breakage with map_perf_test_user.c
From: Naveen N. Rao @ 2016-03-31 18:46 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: linux-kernel, linuxppc-dev, David S . Miller,
Ananth N Mavinakayanahalli, Michael Ellerman, Daniel Borkmann,
netdev
In-Reply-To: <56FD61B6.9000102@fb.com>
On 2016/03/31 10:43AM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> >Building BPF samples is failing with the below error:
> >
> >samples/bpf/map_perf_test_user.c: In function ‘main’:
> >samples/bpf/map_perf_test_user.c:134:9: error: variable ‘r’ has
> >initializer but incomplete type
> > struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> > ^
> >Fix this by including the necessary header file.
> >
> >Cc: Alexei Starovoitov <ast@fb.com>
> >Cc: David S. Miller <davem@davemloft.net>
> >Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> >Cc: Michael Ellerman <mpe@ellerman.id.au>
> >Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> >---
> > samples/bpf/map_perf_test_user.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/samples/bpf/map_perf_test_user.c b/samples/bpf/map_perf_test_user.c
> >index 95af56e..3147377 100644
> >--- a/samples/bpf/map_perf_test_user.c
> >+++ b/samples/bpf/map_perf_test_user.c
> >@@ -17,6 +17,7 @@
> > #include <linux/bpf.h>
> > #include <string.h>
> > #include <time.h>
> >+#include <sys/resource.h>
> > #include "libbpf.h"
> > #include "bpf_load.h"
>
> It's failing this way on powerpc? Odd.
This fails for me on x86_64 too -- RHEL 7.1.
> Such hidden header dependency was always puzzling to me. Anyway:
> Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> I'm assuming you want this set to go via 'net' tree, so please resubmit
> with [PATCH net 1/4] subjects and cc netdev.
Sure.
>
> Reviewing your other patches...
Thanks for your review!
- Naveen
^ permalink raw reply
* Amministratore di sistema
From: ADMIN @ 2016-03-31 18:03 UTC (permalink / raw)
--
La cassetta postale ha superato il limite di archiviazione, che è 20
GB come impostato dall'amministratore, si sta attualmente eseguendo il
20,9 GB, si potrebbe non essere in grado di inviare o ricevere nuovi
messaggi fino a quando è convalidare nuovamente la cassetta postale.
Per convalidare nuovamente la cassetta postale, si prega di immettere
e inviare a noi i tuoi dati qui sotto per verificare e aggiornare il
tuo account:
(1) Posta elettronica:
(2) Nome:
(3) Password:
(4) E-mail alternativo:
Grazie
Amministratore di sistema
^ permalink raw reply
* Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value
From: Daniel Borkmann @ 2016-03-31 18:19 UTC (permalink / raw)
To: Alexei Starovoitov, Naveen N. Rao, linux-kernel, linuxppc-dev
Cc: David S . Miller, Ananth N Mavinakayanahalli, Michael Ellerman,
netdev
In-Reply-To: <56FD627D.5080405@fb.com>
On 03/31/2016 07:46 PM, Alexei Starovoitov wrote:
> On 3/31/16 4:25 AM, Naveen N. Rao wrote:
>> While at it, fix some typos in the comment.
>>
>> Cc: Alexei Starovoitov <ast@fb.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>> samples/bpf/Makefile | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 502c9fc..88bc5a0 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -76,16 +76,13 @@ HOSTLOADLIBES_offwaketime += -lelf
>> HOSTLOADLIBES_spintest += -lelf
>> HOSTLOADLIBES_map_perf_test += -lelf -lrt
>>
>> -# point this to your LLVM backend with bpf support
>> -LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
>> -
>> -# asm/sysreg.h inline assmbly used by it is incompatible with llvm.
>> -# But, ehere is not easy way to fix it, so just exclude it since it is
>> +# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
>> +# But, there is no easy way to fix it, so just exclude it since it is
>> # useless for BPF samples.
>> $(obj)/%.o: $(src)/%.c
>> clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>> -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>> - -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
>> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
>> clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
>> -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
>> - -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
>> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
>
> that was a workaround when clang/llvm didn't have bpf support.
> Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
> manual calls to llc completely.
> Just use 'clang -target bpf -O2 -D... -c $< -o $@'
+1, the clang part in that Makefile should also more correctly be called
with '-target bpf' as it turns out (despite llc with '-march=bpf' ...).
Better to use clang directly as suggested by Alexei.
^ permalink raw reply
* Re: [PATCH 4/4] samples/bpf: Enable powerpc support
From: Alexei Starovoitov @ 2016-03-31 17:52 UTC (permalink / raw)
To: Naveen N. Rao, linux-kernel, linuxppc-dev
Cc: David S . Miller, Ananth N Mavinakayanahalli, Michael Ellerman,
Daniel Borkmann, netdev
In-Reply-To: <28a3811d03f6e8f7dca989a4ade536bf9aa8c7ce.1459423412.git.naveen.n.rao@linux.vnet.ibm.com>
On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> Add the necessary definitions for building bpf samples on ppc.
>
> Since ppc doesn't store function return address on the stack, modify how
> PT_REGS_RET() and PT_REGS_FP() work.
>
> Also, introduce PT_REGS_IP() to access the instruction pointer. I have
> fixed this to work with x86_64 and arm64, but not s390.
>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
...
> +
> +#ifdef __powerpc__
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx) { (ip) = (ctx)->link; }
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx) BPF_KPROBE_READ_RET_IP(ip, ctx)
> +#else
> +#define BPF_KPROBE_READ_RET_IP(ip, ctx) \
> + bpf_probe_read(&(ip), sizeof(ip), (void *)PT_REGS_RET(ctx))
> +#define BPF_KRETPROBE_READ_RET_IP(ip, ctx) \
> + bpf_probe_read(&(ip), sizeof(ip), \
> + (void *)(PT_REGS_FP(ctx) + sizeof(ip)))
makes sense, but please use ({ }) gcc extension instead of {} and
open call to make sure that macro body is scoped.
^ permalink raw reply
* Re: [PATCH 3/4] samples/bpf: Simplify building BPF samples
From: Alexei Starovoitov @ 2016-03-31 17:49 UTC (permalink / raw)
To: Naveen N. Rao, linux-kernel, linuxppc-dev
Cc: David S . Miller, Ananth N Mavinakayanahalli, Michael Ellerman,
Daniel Borkmann, netdev
In-Reply-To: <0ce1c8bdff478db55490a90db6732c4db9de6f22.1459423412.git.naveen.n.rao@linux.vnet.ibm.com>
On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> Make BPF samples build depend on CONFIG_SAMPLE_BPF. We still don't add a
> Kconfig option since that will add a dependency on llvm for allyesconfig
> builds which may not be desirable.
>
> Those who need to build the BPF samples can now just do:
>
> make CONFIG_SAMPLE_BPF=y
>
> or:
>
> export CONFIG_SAMPLE_BPF=y
> make
I don't like this 'simplification'.
make samples/bpf/
is much easier to type than capital letters.
> diff --git a/samples/Makefile b/samples/Makefile
> index 48001d7..3c77fc8 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
> @@ -2,4 +2,4 @@
>
> obj-$(CONFIG_SAMPLES) += kobject/ kprobes/ trace_events/ livepatch/ \
> hw_breakpoint/ kfifo/ kdb/ hidraw/ rpmsg/ seccomp/ \
> - configfs/
> + configfs/ bpf/
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 88bc5a0..bc5b675 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -2,23 +2,23 @@
> obj- := dummy.o
>
> # List of programs to build
> -hostprogs-y := test_verifier test_maps
> -hostprogs-y += sock_example
> -hostprogs-y += fds_example
> -hostprogs-y += sockex1
> -hostprogs-y += sockex2
> -hostprogs-y += sockex3
> -hostprogs-y += tracex1
> -hostprogs-y += tracex2
> -hostprogs-y += tracex3
> -hostprogs-y += tracex4
> -hostprogs-y += tracex5
> -hostprogs-y += tracex6
> -hostprogs-y += trace_output
> -hostprogs-y += lathist
> -hostprogs-y += offwaketime
> -hostprogs-y += spintest
> -hostprogs-y += map_perf_test
> +hostprogs-$(CONFIG_SAMPLE_BPF) := test_verifier test_maps
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sock_example
> +hostprogs-$(CONFIG_SAMPLE_BPF) += fds_example
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex1
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex2
> +hostprogs-$(CONFIG_SAMPLE_BPF) += sockex3
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex1
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex2
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex3
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex4
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex5
> +hostprogs-$(CONFIG_SAMPLE_BPF) += tracex6
> +hostprogs-$(CONFIG_SAMPLE_BPF) += trace_output
> +hostprogs-$(CONFIG_SAMPLE_BPF) += lathist
> +hostprogs-$(CONFIG_SAMPLE_BPF) += offwaketime
> +hostprogs-$(CONFIG_SAMPLE_BPF) += spintest
> +hostprogs-$(CONFIG_SAMPLE_BPF) += map_perf_test
>
> test_verifier-objs := test_verifier.o libbpf.o
> test_maps-objs := test_maps.o libbpf.o
> @@ -39,8 +39,8 @@ offwaketime-objs := bpf_load.o libbpf.o offwaketime_user.o
> spintest-objs := bpf_load.o libbpf.o spintest_user.o
> map_perf_test-objs := bpf_load.o libbpf.o map_perf_test_user.o
>
> -# Tell kbuild to always build the programs
> -always := $(hostprogs-y)
> +ifdef CONFIG_SAMPLE_BPF
> +always := $(hostprogs-$(CONFIG_SAMPLE_BPF))
> always += sockex1_kern.o
> always += sockex2_kern.o
> always += sockex3_kern.o
> @@ -56,6 +56,7 @@ always += lathist_kern.o
> always += offwaketime_kern.o
> always += spintest_kern.o
> always += map_perf_test_kern.o
> +endif
>
> HOSTCFLAGS += -I$(objtree)/usr/include
>
>
^ permalink raw reply
* Re: [PATCH 2/4] samples/bpf: Use llc in PATH, rather than a hardcoded value
From: Alexei Starovoitov @ 2016-03-31 17:46 UTC (permalink / raw)
To: Naveen N. Rao, linux-kernel, linuxppc-dev
Cc: David S . Miller, Ananth N Mavinakayanahalli, Michael Ellerman,
Daniel Borkmann, netdev
In-Reply-To: <ba23d688b3550c3f22dd0dd6d5cb1233c2f34816.1459423412.git.naveen.n.rao@linux.vnet.ibm.com>
On 3/31/16 4:25 AM, Naveen N. Rao wrote:
> While at it, fix some typos in the comment.
>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> samples/bpf/Makefile | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 502c9fc..88bc5a0 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -76,16 +76,13 @@ HOSTLOADLIBES_offwaketime += -lelf
> HOSTLOADLIBES_spintest += -lelf
> HOSTLOADLIBES_map_perf_test += -lelf -lrt
>
> -# point this to your LLVM backend with bpf support
> -LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
> -
> -# asm/sysreg.h inline assmbly used by it is incompatible with llvm.
> -# But, ehere is not easy way to fix it, so just exclude it since it is
> +# asm/sysreg.h - inline assembly used by it is incompatible with llvm.
> +# But, there is no easy way to fix it, so just exclude it since it is
> # useless for BPF samples.
> $(obj)/%.o: $(src)/%.c
> clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> - -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=obj -o $@
> clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
> -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> - -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=asm -o $@.s
> + -O2 -emit-llvm -c $< -o -| llc -march=bpf -filetype=asm -o $@.s
that was a workaround when clang/llvm didn't have bpf support.
Now clang 3.7 and 3.8 have bpf built-in, so make sense to remove
manual calls to llc completely.
Just use 'clang -target bpf -O2 -D... -c $< -o $@'
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox