* [PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post
From: Jia-Ju Bai @ 2018-01-09 1:40 UTC (permalink / raw)
To: Larry.Finger, kvalo, kstewart, johannes.berg, tiwai, gregkh,
colin.king, andrew.zaborowski
Cc: linux-kernel, linux-wireless, netdev, b43-dev, Jia-Ju Bai
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Replace mdelay with usleep_range, instead of msleep in v1.
Thank Larry for good advice.
---
drivers/net/wireless/broadcom/b43/phy_n.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev *dev)
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
- mdelay(2);
+ usleep_range(2000, 3000);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] ath9k: add a quirk to set use_msi automatically
From: AceLan Kao @ 2018-01-09 1:39 UTC (permalink / raw)
To: QCA ath9k Development, Kalle Valo, linux-wireless, netdev,
linux-kernel
Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding the following platforms to the quirk.
Dell Inspiron 24-3460
Dell Inspiron 3472
Dell Inspiron 14-3473
Dell Vostro 3262
Dell Vostro 15-3572
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/net/wireless/ath/ath9k/init.c | 53 +++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 43adead..e479fae 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_net.h>
#include <linux/relay.h>
+#include <linux/dmi.h>
#include <net/ieee80211_radiotap.h>
#include "ath9k.h"
@@ -96,6 +97,56 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
};
#endif
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+ ath9k_use_msi = 1;
+ return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 24-3460",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Vostro 3262",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 3472",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Vostro 15-3572",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+ },
+ },
+ {
+ .callback = set_use_msi,
+ .ident = "Dell Inspiron 14-3473",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+ },
+ },
+ {}
+};
+
static void ath9k_deinit_softc(struct ath_softc *sc);
static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1155,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
+ dmi_check_system(ath9k_quirks);
+
return 0;
err_pci_exit:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 12/20] net: hns3: Add packet statistics of netdev
From: David Miller @ 2018-01-09 1:39 UTC (permalink / raw)
To: kubakici; +Cc: lipeng321, netdev, linux-kernel, linuxarm, salil.mehta
In-Reply-To: <20180108120431.1e4b45e1@cakuba.netronome.com>
From: Jakub Kicinski <kubakici@wp.pl>
Date: Mon, 8 Jan 2018 12:04:31 -0800
> Ugh, I so didn't review this in time :( I think there is a consensus
> that we should avoid duplicating standard stats in ethtool. Especially
> those old ones. Like "collisions", I assume this is a modern NIC, are
> collisions still a thing?
There is no standard way to get per-queue values, and ethtool stats are
how pretty much every driver provides it.
^ permalink raw reply
* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Jia-Ju Bai @ 2018-01-09 1:36 UTC (permalink / raw)
To: Larry Finger, Kalle Valo
Cc: kstewart, johannes.berg, tiwai, gregkh, linux-wireless,
linux-kernel, andrew.zaborowski, b43-dev, netdev, colin.king
In-Reply-To: <fc3e2558-c31f-289d-8413-33fef4f5ca64@lwfinger.net>
On 2018/1/9 0:31, Larry Finger wrote:
> On 01/08/2018 10:21 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>>> b43_radio_2057_init_post is not called in an interrupt handler
>>> nor holding a spinlock.
>>> The function mdelay in it can be replaced with msleep, to reduce
>>> busy wait.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>
>> You submitted an identical patch a week earlier:
>>
>> https://patchwork.kernel.org/patch/10137671/
>>
>> How is this different? Also always add version number to the patch so
>> that the
>> maintainers can follow the changes easily:
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
>>
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>>
>
> I had negative comments on one of those due to the possibility of
> msleep(2) extending as long as 20 msec. Until the author, or someone
> else, can test that this is OK, then the mdelay(2) can only be
> replaced with usleep_range(2000, 3000).
>
> NACK for both.
>
> Larry
>
Sorry for my mistake.
I have sent a patch v2 using usleep_range(2000, 3000), and you can have
a look :)
Thanks,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH net 0/3] Some sockopt optlen fixes
From: Neil Horman @ 2018-01-09 1:23 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev, linux-sctp, Vlad Yasevich, haliu
In-Reply-To: <cover.1515436112.git.marcelo.leitner@gmail.com>
On Mon, Jan 08, 2018 at 07:02:26PM -0200, Marcelo Ricardo Leitner wrote:
> Hangbin Liu reported that some SCTP sockopt are allowing the user to get
> the kernel to allocate really large buffers by not having a ceiling on
> optlen.
>
> This patchset address this issue (in patch 2), replace an GFP_ATOMIC
> that isn't needed and avoid calculating the option size multiple times
> in some setsockopt.
>
> Marcelo Ricardo Leitner (3):
> sctp: GFP_ATOMIC is not needed in sctp_setsockopt_events
> sctp: add a ceiling to optlen in some sockopts
> sctp: make use of pre-calculated len
>
> net/sctp/socket.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> --
> 2.14.3
>
>
Series
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* RE: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
From: Chris Mi @ 2018-01-09 1:21 UTC (permalink / raw)
To: Phil Sutter
Cc: dsahern@gmail.com, marcelo.leitner@gmail.com,
netdev@vger.kernel.org, gerlitz.or@gmail.com,
stephen@networkplumber.org
In-Reply-To: <20180108133150.GE14358@orbyte.nwl.cc>
> -----Original Message-----
> From: n0-1@orbyte.nwl.cc [mailto:n0-1@orbyte.nwl.cc] On Behalf Of Phil
> Sutter
> Sent: Monday, January 8, 2018 9:32 PM
> To: Chris Mi <chrism@mellanox.com>
> Cc: dsahern@gmail.com; marcelo.leitner@gmail.com;
> netdev@vger.kernel.org; gerlitz.or@gmail.com;
> stephen@networkplumber.org
> Subject: Re: [patch iproute2 v6 0/3] tc: Add -bs option to batch mode
>
> Hi Chris,
>
> On Mon, Jan 08, 2018 at 02:03:53AM +0000, Chris Mi wrote:
> > > On Thu, Jan 04, 2018 at 04:34:51PM +0900, Chris Mi wrote:
> > > > The insertion rate is improved more than 10%.
> > >
> > > Did you measure the effect of increasing batch sizes?
> > Yes. Even if we enlarge the batch size bigger than 10, there is no big
> improvement.
> > I think that's because current kernel doesn't process the requests in
> parallel.
> > If kernel processes the requests in parallel, I believe specifying a
> > bigger batch size will get a better result.
>
> But throughput doesn't regress at some point, right? I think that's the critical
> aspect when considering an "unlimited" batch size.
Yes.
>
> On Mon, Jan 08, 2018 at 08:00:00AM +0000, Chris Mi wrote:
> > After testing, I find that the message passed to kernel should not be too
> big.
> > If it is bigger than about 64K, sendmsg returns -1, errno is 90 (EMSGSIZE).
> > That is about 400 commands. So how about set batch size to 128 which is
> big enough?
>
> If that's the easiest way, why not. At first, I thought one could maybe send
> the collected messages in chunks of suitable size, but that's probably not
> worth the effort.
OK.
-Chris
^ permalink raw reply
* Re: [RFC PATCH bpf-next v2 0/4] Separate error injection table from kprobes
From: Masami Hiramatsu @ 2018-01-09 1:17 UTC (permalink / raw)
To: Josef Bacik
Cc: mhiramat, Alexei Starovoitov, Josef Bacik, rostedt, mingo, davem,
netdev, linux-kernel, ast, kernel-team, daniel, linux-btrfs,
darrick.wong, Akinobu Mita
In-Reply-To: <20180104160715.7wz7pifbizkilisw@destiny>
On Thu, 4 Jan 2018 11:07:16 -0500
Josef Bacik <josef@toxicpanda.com> wrote:
> On Tue, Dec 26, 2017 at 04:46:28PM +0900, Masami Hiramatsu wrote:
> > Hi Josef and Alexei,
> >
> > Here are the 2nd version of patches to moving error injection
> > table from kprobes. In this series I did a small fixes and
> > add function-based fault injection.
> >
> > Here is the previous version:
> >
> > https://lkml.org/lkml/2017/12/22/554
> >
> > There are 2 main reasons why I separate it from kprobes.
> >
> > - kprobes users can modify execution path not only at
> > error-injection whitelist functions but also other
> > functions. I don't like to suggest user that such
> > limitation is from kprobes itself.
> >
> > - This error injection information is also useful for
> > ftrace (function-hook) and livepatch. It should not
> > be limited by CONFIG_KPROBES.
> >
> > So I introduced CONFIG_FUNCTION_ERROR_INJECTION for this feature.
> > Also CONFIG_FAIL_FUNCTION is added, which provides function-based
> > error injection interface via debugfs following fault-injection
> > framework. See [4/4].
> >
> > Any thoughts?
>
> Sorry Masami, I've been on vacation for the last two weeks. This approach is
> fine by me, if we want to allow other mechanisms other than bpf to use this
> functionality then hooray. I'll do a proper review when you post v3, just
> wanted to let you know I wasn't ignoring you. Thanks,
Yeah, thank you for the kindful notice ;)
BTW, could you tell me how I can run your test case?
When I tried to build the tests (samples/bpf) I got below error and stopped.
[mhiramat@devbox bpf]$ LANG=C make
make -C ../../ /home/mhiramat/ksrc/linux/samples/bpf/
make[1]: Entering directory '/home/mhiramat/ksrc/linux'
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/bounds.h
CHK include/generated/timeconst.h
CHK include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
DESCEND objtool
CHK scripts/mod/devicetable-offsets.h
HOSTCC /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o
/home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:39:8: error: redefinition of 'struct list_head'
struct list_head {
^~~~~~~~~
In file included from /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.c:9:0:
./tools/include/linux/types.h:69:8: note: originally defined here
struct list_head {
^~~~~~~~~
make[2]: *** [scripts/Makefile.host:107: /home/mhiramat/ksrc/linux/samples/bpf/test_lru_dist.o] Error 1
make[1]: *** [Makefile:1675: /home/mhiramat/ksrc/linux/samples/bpf/] Error 2
make[1]: Leaving directory '/home/mhiramat/ksrc/linux'
make: *** [Makefile:204: all] Error 2
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the bpf tree
From: Alexei Starovoitov @ 2018-01-09 0:29 UTC (permalink / raw)
To: Stephen Rothwell
Cc: David Miller, Networking, Daniel Borkmann, Alexei Starovoitov,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <20180109112125.78277253@canb.auug.org.au>
On Tue, Jan 09, 2018 at 11:21:25AM +1100, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
> tools/testing/selftests/bpf/test_align.c
>
> between commit:
>
> 2b36047e7889 ("selftests/bpf: fix test_align")
>
> from the bpf tree and commit:
>
> 6a28b446b7d2 ("selftests/bpf: adjust test_align expected output")
>
> from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging. You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell
>
> diff --cc tools/testing/selftests/bpf/test_align.c
> index 471bbbdb94db,fe916d29e166..000000000000
> --- a/tools/testing/selftests/bpf/test_align.c
> +++ b/tools/testing/selftests/bpf/test_align.c
> @@@ -473,8 -473,28 +473,8 @@@ static struct bpf_align_test tests[] =
> .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> .result = REJECT,
> .matches = {
> - {4, "R5=pkt(id=0,off=0,r=0,imm=0)"},
> + {4, "R5_w=pkt(id=0,off=0,r=0,imm=0)"},
thanks. That's correct resolution.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the bpf tree
From: Stephen Rothwell @ 2018-01-09 0:21 UTC (permalink / raw)
To: David Miller, Networking, Daniel Borkmann, Alexei Starovoitov
Cc: Linux-Next Mailing List, Linux Kernel Mailing List
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
tools/testing/selftests/bpf/test_align.c
between commit:
2b36047e7889 ("selftests/bpf: fix test_align")
from the bpf tree and commit:
6a28b446b7d2 ("selftests/bpf: adjust test_align expected output")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc tools/testing/selftests/bpf/test_align.c
index 471bbbdb94db,fe916d29e166..000000000000
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@@ -473,8 -473,28 +473,8 @@@ static struct bpf_align_test tests[] =
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
.result = REJECT,
.matches = {
- {4, "R5=pkt(id=0,off=0,r=0,imm=0)"},
+ {4, "R5_w=pkt(id=0,off=0,r=0,imm=0)"},
- /* ptr & 0x40 == either 0 or 0x40 */
- {5, "R5_w=inv(id=0,umax_value=64,var_off=(0x0; 0x40))"},
- /* ptr << 2 == unknown, (4n) */
- {7, "R5_w=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
- /* (4n) + 14 == (4n+2). We blow our bounds, because
- * the add could overflow.
- */
- {8, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
- /* Checked s>=0 */
- {10, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
- /* packet pointer + nonnegative (4n+2) */
- {12, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
- {14, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
- /* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
- * We checked the bounds, but it might have been able
- * to overflow if the packet pointer started in the
- * upper half of the address space.
- * So we did not get a 'range' on R6, and the access
- * attempt will fail.
- */
- {16, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+ /* R5 bitwise operator &= on pointer prohibited */
}
},
{
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Linus Torvalds @ 2018-01-09 0:12 UTC (permalink / raw)
To: Dan Williams
Cc: Linux Kernel Mailing List, linux-arch, Andi Kleen, Arnd Bergmann,
Greg Kroah-Hartman, Peter Zijlstra, Network Development,
the arch/x86 maintainers, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Alan Cox
On Mon, Jan 8, 2018 at 3:53 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>
> I've been thinking the "and" is only suitable for the array bounds
> check, for get_user() we're trying to block speculation past
> access_ok() at which point we can only do the lfence?
Well, we *could* do the "and", at least for the simple cases (ie the
true "get_user()" that integrates the access_ok with the access).
IOW, mainly the code in arch/x86/lib/getuser.S.
But it probably is a lot simpler to just add the "lfence" to ASM_STAC,
because by definition those cases don't tend to be the truly critical
ones - people who use those functions tend to do one or two accesses,
and the real cost is likely the I$ misses and the D$ miss to get
current->addr_limit. Not to mention the "stac" itself, which is much
more expensive than the access on current microarchitectures.
But something like this *might* work:
index c97d935a29e8..7fa3d293beaf 100644
--- a/arch/x86/lib/getuser.S
+++ b/arch/x86/lib/getuser.S
@@ -38,8 +38,11 @@
.text
ENTRY(__get_user_1)
mov PER_CPU_VAR(current_task), %_ASM_DX
- cmp TASK_addr_limit(%_ASM_DX),%_ASM_AX
+ mov TASK_addr_limit(%_ASM_DX),%_ASM_DX
+ cmp %_ASM_DX,%_ASM_AX
jae bad_get_user
+ or $0xfff,%_ASM_DX
+ and %_ASM_DX,%_ASM_AX
ASM_STAC
1: movzbl (%_ASM_AX),%edx
xor %eax,%eax
(this only does the one-byte case - the 2/4/8 byte cases are exactly the same).
The above is completely untested and might have some stupid
thinko/typo, so take it purely as a "example patch" to show the
concept, rather than actually do it.
But just adding "lfence" to the existing ASM_STAC is a hell of a lot
easier, and the performance difference between that trivial patch and
the above "let's be clever with 'and'" might not be measurable.
I really have no idea how expensive lfence might actually end up being
in practice. It's possible that lfence is actually fairly cheap in
kernel code, since we tend to not have very high IPC anyway.
Linus
^ permalink raw reply
* Re: [PATCH v2] openvswitch: Trim off padding before L3+ netfilter processing
From: Pravin Shelar @ 2018-01-09 0:05 UTC (permalink / raw)
To: Ed Swierk; +Cc: ovs-dev, netdev, Benjamin Warren, Keith Holleman
In-Reply-To: <CAOrHB_A1sQWforWuUve5phrxPjSOpbTEQ4F5h3beh1C9qWmUTw@mail.gmail.com>
On Sat, Jan 6, 2018 at 10:57 AM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Fri, Jan 5, 2018 at 10:59 PM, Ed Swierk <eswierk@skyportsystems.com> wrote:
>>
>>
>> On Jan 5, 2018 22:17, "Pravin Shelar" <pshelar@ovn.org> wrote:
>>
>> On Fri, Jan 5, 2018 at 3:20 PM, Ed Swierk <eswierk@skyportsystems.com>
>> wrote:
>>> On Fri, Jan 5, 2018 at 10:14 AM, Ed Swierk <eswierk@skyportsystems.com>
>>> wrote:
>>>> On Thu, Jan 4, 2018 at 7:36 PM, Pravin Shelar <pshelar@ovn.org> wrote:
>>>>> OVS already pull all required headers in skb linear data, so no need
>>>>> to redo all of it. only check required is the ip-checksum validation.
>>>>> I think we could avoid it in most of cases by checking skb length to
>>>>> ipheader length before verifying the ip header-checksum.
>>>>
>>>> Shouldn't the IP header checksum be verified even earlier, like in
>>>> key_extract(), before actually using any of the fields in the IP
>>>> header?
>>>
>>> Something like this for verifying the IP header checksum (not tested):
>>>
>> AFAIU openflow does not need this verification, so it is not required
>> in flow extract.
>>
>>
>> Okay. How about my proposed trimming implementation, caching the pad length
>> in the ovs cb?
>>
> Caching the length is not that simple, OVS actions can change the
> length. Keeping it consistent with packet would be more work, so lets
> calculate it in ovs-ct function.
You could make it specific for skb-len-trimming, something like
boolean flag. so that it is easy to reason with.
^ permalink raw reply
* Re: [PATCH net-next 2/2] openvswitch: add erspan version II support
From: Pravin Shelar @ 2018-01-09 0:03 UTC (permalink / raw)
To: William Tu; +Cc: Linux Kernel Network Developers
In-Reply-To: <1515191361-107730-3-git-send-email-u9012063@gmail.com>
On Fri, Jan 5, 2018 at 2:29 PM, William Tu <u9012063@gmail.com> wrote:
> The patch adds support for configuring the erspan version II
> fields for openvswitch.
>
The patch looks good, But it could change userspace API for
OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, how are we going to handle
compatibility?
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
> include/uapi/linux/openvswitch.h | 12 +++-
> net/openvswitch/flow_netlink.c | 125 +++++++++++++++++++++++++++++++++++----
> 2 files changed, 126 insertions(+), 11 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 4265d7f9e1f2..3b1950c59a0c 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -273,6 +273,16 @@ enum {
>
> #define OVS_VXLAN_EXT_MAX (__OVS_VXLAN_EXT_MAX - 1)
>
> +enum {
> + OVS_ERSPAN_OPT_UNSPEC,
> + OVS_ERSPAN_OPT_IDX, /* be32 index */
> + OVS_ERSPAN_OPT_VER, /* u8 version number */
> + OVS_ERSPAN_OPT_DIR, /* u8 direction */
> + OVS_ERSPAN_OPT_HWID, /* u8 hardware ID */
> + __OVS_ERSPAN_OPT_MAX,
> +};
> +
> +#define OVS_ERSPAN_OPT_MAX (__OVS_ERSPAN_OPT_MAX - 1)
>
> /* OVS_VPORT_ATTR_OPTIONS attributes for tunnels.
> */
> @@ -363,7 +373,7 @@ enum ovs_tunnel_key_attr {
> OVS_TUNNEL_KEY_ATTR_IPV6_SRC, /* struct in6_addr src IPv6 address. */
> OVS_TUNNEL_KEY_ATTR_IPV6_DST, /* struct in6_addr dst IPv6 address. */
> OVS_TUNNEL_KEY_ATTR_PAD,
> - OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* be32 ERSPAN index. */
> + OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* Nested OVS_ERSPAN_OPT_* */
> __OVS_TUNNEL_KEY_ATTR_MAX
> };
>
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index bce1f78b0de5..696198cf3765 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -334,8 +334,10 @@ size_t ovs_tun_key_attr_size(void)
> * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
> */
> + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_SRC */
> - + nla_total_size(2) /* OVS_TUNNEL_KEY_ATTR_TP_DST */
> - + nla_total_size(4); /* OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS */
> + + nla_total_size(2); /* OVS_TUNNEL_KEY_ATTR_TP_DST */
> + /* OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS is mutually exclusive with
> + * OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS and covered by it.
> + */
> }
>
> static size_t ovs_nsh_key_attr_size(void)
> @@ -386,6 +388,13 @@ static const struct ovs_len_tbl ovs_vxlan_ext_key_lens[OVS_VXLAN_EXT_MAX + 1] =
> [OVS_VXLAN_EXT_GBP] = { .len = sizeof(u32) },
> };
>
> +static const struct ovs_len_tbl ovs_erspan_opt_lens[OVS_ERSPAN_OPT_MAX + 1] = {
> + [OVS_ERSPAN_OPT_IDX] = { .len = sizeof(u32) },
> + [OVS_ERSPAN_OPT_VER] = { .len = sizeof(u8) },
> + [OVS_ERSPAN_OPT_DIR] = { .len = sizeof(u8) },
> + [OVS_ERSPAN_OPT_HWID] = { .len = sizeof(u8) },
> +};
> +
> static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1] = {
> [OVS_TUNNEL_KEY_ATTR_ID] = { .len = sizeof(u64) },
> [OVS_TUNNEL_KEY_ATTR_IPV4_SRC] = { .len = sizeof(u32) },
> @@ -402,7 +411,8 @@ static const struct ovs_len_tbl ovs_tunnel_key_lens[OVS_TUNNEL_KEY_ATTR_MAX + 1]
> .next = ovs_vxlan_ext_key_lens },
> [OVS_TUNNEL_KEY_ATTR_IPV6_SRC] = { .len = sizeof(struct in6_addr) },
> [OVS_TUNNEL_KEY_ATTR_IPV6_DST] = { .len = sizeof(struct in6_addr) },
> - [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = sizeof(u32) },
> + [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = OVS_ATTR_NESTED,
> + .next = ovs_erspan_opt_lens },
> };
>
> static const struct ovs_len_tbl
> @@ -640,16 +650,78 @@ static int erspan_tun_opt_from_nlattr(const struct nlattr *attr,
> {
> unsigned long opt_key_offset;
> struct erspan_metadata opts;
> + struct nlattr *a;
> + u16 hwid, dir;
> + int rem;
>
> BUILD_BUG_ON(sizeof(opts) > sizeof(match->key->tun_opts));
>
> memset(&opts, 0, sizeof(opts));
> - opts.u.index = nla_get_be32(attr);
> + nla_for_each_nested(a, attr, rem) {
> + int type = nla_type(a);
>
> - /* Index has only 20-bit */
> - if (ntohl(opts.u.index) & ~INDEX_MASK) {
> - OVS_NLERR(log, "ERSPAN index number %x too large.",
> - ntohl(opts.u.index));
> + if (type > OVS_ERSPAN_OPT_MAX) {
> + OVS_NLERR(log, "ERSPAN option %d out of range max %d",
> + type, OVS_ERSPAN_OPT_MAX);
> + return -EINVAL;
> + }
> +
> + if (!check_attr_len(nla_len(a),
> + ovs_erspan_opt_lens[type].len)) {
> + OVS_NLERR(log, "ERSPAN option %d has unexpected len %d expected %d",
> + type, nla_len(a),
> + ovs_erspan_opt_lens[type].len);
> + return -EINVAL;
> + }
> +
> + switch (type) {
> + case OVS_ERSPAN_OPT_IDX:
> + opts.u.index = nla_get_be32(a);
> + if (ntohl(opts.u.index) & ~INDEX_MASK) {
> + OVS_NLERR(log,
> + "ERSPAN index number %x too large.",
> + ntohl(opts.u.index));
> + return -EINVAL;
> + }
> + break;
> + case OVS_ERSPAN_OPT_VER:
> + opts.version = nla_get_u8(a);
> + if (opts.version != 1 && opts.version != 2) {
> + OVS_NLERR(log,
> + "ERSPAN version %d not supported.",
> + opts.version);
> + return -EINVAL;
> + }
> + break;
> + case OVS_ERSPAN_OPT_DIR:
> + dir = nla_get_u8(a);
> + if (dir != 0 && dir != 1) {
> + OVS_NLERR(log,
> + "ERSPAN direction %d invalid.",
> + dir);
> + return -EINVAL;
> + }
> + opts.u.md2.dir = dir;
> + break;
> + case OVS_ERSPAN_OPT_HWID:
> + hwid = nla_get_u8(a);
> + if (hwid & ~(HWID_MASK >> HWID_OFFSET)) {
> + OVS_NLERR(log,
> + "ERSPAN hardware ID %x invalid.",
> + hwid);
> + return -EINVAL;
> + }
> + set_hwid(&opts.u.md2, hwid);
> + break;
> + default:
> + OVS_NLERR(log, "Unknown ERSPAN opt attribute %d",
> + type);
> + return -EINVAL;
> + }
> + }
> + if (rem) {
> + OVS_NLERR(log, "ERSPAN opt message has %d unknown bytes.",
> + rem);
> return -EINVAL;
> }
>
> @@ -846,6 +918,39 @@ static int vxlan_opt_to_nlattr(struct sk_buff *skb,
> return 0;
> }
>
> +static int erspan_opt_to_nlattr(struct sk_buff *skb,
> + const void *tun_opts, int swkey_tun_opts_len)
> +{
> + const struct erspan_metadata *opts = tun_opts;
> + struct nlattr *nla;
> +
> + nla = nla_nest_start(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS);
> + if (!nla)
> + return -EMSGSIZE;
> +
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_VER, opts->version) < 0)
> + return -EMSGSIZE;
> +
> + if (opts->version == 1) {
> + if (nla_put_be32(skb, OVS_ERSPAN_OPT_IDX, opts->u.index) < 0)
> + return -EMSGSIZE;
> +
> + } else if (opts->version == 2) {
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_DIR,
> + opts->u.md2.dir) < 0)
> + return -EMSGSIZE;
> +
> + if (nla_put_u8(skb, OVS_ERSPAN_OPT_HWID,
> + get_hwid(&opts->u.md2)) < 0)
> + return -EMSGSIZE;
> + } else {
> + return -EINVAL;
> + }
> +
> + nla_nest_end(skb, nla);
> + return 0;
> +}
> +
> static int __ip_tun_to_nlattr(struct sk_buff *skb,
> const struct ip_tunnel_key *output,
> const void *tun_opts, int swkey_tun_opts_len,
> @@ -906,8 +1011,8 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb,
> vxlan_opt_to_nlattr(skb, tun_opts, swkey_tun_opts_len))
> return -EMSGSIZE;
> else if (output->tun_flags & TUNNEL_ERSPAN_OPT &&
> - nla_put_be32(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,
> - ((struct erspan_metadata *)tun_opts)->u.index))
> + erspan_opt_to_nlattr(skb, tun_opts,
> + swkey_tun_opts_len))
> return -EMSGSIZE;
> }
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: introduce BPF_JIT_ALWAYS_ON config
From: Jakub Kicinski @ 2018-01-09 0:02 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, davem, torvalds, jannh, alan, netdev,
kernel-team
In-Reply-To: <1f0c1df2-3e27-a0ef-90b2-41ce499af14b@iogearbox.net>
On Mon, 8 Jan 2018 22:59:04 +0100, Daniel Borkmann wrote:
> > @@ -1453,6 +1457,11 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
> > */
> > *err = bpf_check_tail_call(fp);
> >
> > +#ifdef CONFIG_BPF_JIT_ALWAYS_ON
> > + if (!fp->jited)
> > + *err = -ENOTSUPP;
> > +#endif
I think programs JITed for offload won't have fp->jited set, but
those are pretty safe from CPU bugs. Should we set fp->jited = 1; in
bpf_prog_offload_compile()? Just throwing "&& !bpf_prog_is_dev_bound()"
in here seems cleaner to me.
FWIW if you have netdevsim compiled and recent iproute2, this will
work to check:
# ip link add type netdevsim
# ip link set netdevsim0 xdpoffload obj ~/xdp/pass.o
^ permalink raw reply
* Re: [PATCH v3 bpf] bpf: prevent out-of-bounds speculation
From: Daniel Borkmann @ 2018-01-08 23:59 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: torvalds, alan, netdev, kernel-team
In-Reply-To: <20180108013302.2675682-1-ast@kernel.org>
On 01/08/2018 02:33 AM, Alexei Starovoitov wrote:
> Under speculation, CPUs may mis-predict branches in bounds checks. Thus,
> memory accesses under a bounds check may be speculated even if the
> bounds check fails, providing a primitive for building a side channel.
>
> To avoid leaking kernel data round up array-based maps and mask the index
> after bounds check, so speculated load with out of bounds index will load
> either valid value from the array or zero from the padded area.
>
> Unconditionally mask index for all array types even when max_entries
> are not rounded to power of 2 for root user.
> When map is created by unpriv user generate a sequence of bpf insns
> that includes AND operation to make sure that JITed code includes
> the same 'index & index_mask' operation.
>
> If prog_array map is created by unpriv user replace
> bpf_tail_call(ctx, map, index);
> with
> if (index >= max_entries) {
> index &= map->index_mask;
> bpf_tail_call(ctx, map, index);
> }
> (along with roundup to power 2) to prevent out-of-bounds speculation.
> There is secondary redundant 'if (index >= max_entries)' in the interpreter
> and in all JITs, but they can be optimized later if necessary.
>
> Other array-like maps (cpumap, devmap, sockmap, perf_event_array, cgroup_array)
> cannot be used by unpriv, so no changes there.
>
> That fixes bpf side of "Variant 1: bounds check bypass (CVE-2017-5753)" on
> all architectures with and without JIT.
>
> v2->v3:
> Daniel noticed that attack potentially can be crafted via syscall commands
> without loading the program, so add masking to those paths as well.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
Applied to bpf tree, thanks Alexei!
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Dan Williams @ 2018-01-08 23:53 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, linux-arch, Andi Kleen, Arnd Bergmann,
Greg Kroah-Hartman, Peter Zijlstra, Network Development,
the arch/x86 maintainers, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Alan Cox
In-Reply-To: <CA+55aFwU4nxo7yMN9bUmCK_f-yp2f3KQ=ot_epW7yuKwwZ8uCQ@mail.gmail.com>
On Mon, Jan 8, 2018 at 3:44 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Jan 8, 2018 at 1:09 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Sat, Jan 6, 2018 at 5:20 PM, Linus Torvalds
>> <torvalds@linux-foundation.org> wrote:
>>> On Sat, Jan 6, 2018 at 3:31 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>>
>>>> I assume if we put this in uaccess_begin() we also need audit for
>>>> paths that use access_ok but don't do on to call uaccess_begin()? A
>>>> quick glance shows a few places where we are open coding the stac().
>>>> Perhaps land the lfence in stac() directly?
>>>
>>> Yeah, we should put it in uaccess_begin(), and in the actual user
>>> accessor helpers that do stac. Some of them probably should be changed
>>> to use uaccess_begin() instead while at it.
>>>
>>> One question for the CPU people: do we actually care and need to do
>>> this for things that might *write* to something? The speculative write
>>> obviously is killed, but does it perhaps bring in a cacheline even
>>> when killed?
>>
>> As far as I understand a write could trigger a request-for-ownership
>> read for the target cacheline.
>
> Oh, absolutely.
>
> I just wonder at what point that happens.
>
> Honestly, trying to get exclusive access to a cacheline can be _very_
> expensive (not just for the local thread), so I would actually expect
> that doing so for speculative writes is actually bad for performance.
>
> That's doubly true because - unlike reads - there is no critical
> latency issue, so trying to get the cache access started as early as
> possible simply isn't all that important.
>
> So I suspect that a write won't actually try to allocate the cacheline
> until the write has actually retired.
>
> End result: writes - unlike reads - *probably* will not speculatively
> perturb the cache with speculative write addresses.
>
>> Even though writes can trigger reads, as far as I can see the write
>> needs to be dependent on the first out-of-bounds read
>
> Yeah. A write on its own wouldn't matter, even if it were to perturb
> the cache state, because the address already comes from user space, so
> there's no new information in the cache perturbation for the attacker.
>
> But that all implies that we shouldn't need the lfence for the
> "put_user()" case, only for the get_user() (where the value we read
> would then perhaps be used to do another access).
>
> So we want to add the lfence (or "and") to get_user(), but not
> necessarily put_user().
Yes, perhaps __uaccess_begin_get() and __uaccess_begin_put() to keep
things separate?
> Agreed?
I've been thinking the "and" is only suitable for the array bounds
check, for get_user() we're trying to block speculation past
access_ok() at which point we can only do the lfence?
^ permalink raw reply
* Re: [PATCH 06/18] x86, barrier: stop speculation for failed access_ok
From: Linus Torvalds @ 2018-01-08 23:44 UTC (permalink / raw)
To: Dan Williams
Cc: Linux Kernel Mailing List, linux-arch, Andi Kleen, Arnd Bergmann,
Greg Kroah-Hartman, Peter Zijlstra, Network Development,
the arch/x86 maintainers, Ingo Molnar, H. Peter Anvin,
Thomas Gleixner, Alan Cox
In-Reply-To: <CAPcyv4hVX4h9j-Uf=Ja5_0p3+qiWOiPd4cUaxoJD9en5TiVGPw@mail.gmail.com>
On Mon, Jan 8, 2018 at 1:09 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Sat, Jan 6, 2018 at 5:20 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Sat, Jan 6, 2018 at 3:31 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>
>>> I assume if we put this in uaccess_begin() we also need audit for
>>> paths that use access_ok but don't do on to call uaccess_begin()? A
>>> quick glance shows a few places where we are open coding the stac().
>>> Perhaps land the lfence in stac() directly?
>>
>> Yeah, we should put it in uaccess_begin(), and in the actual user
>> accessor helpers that do stac. Some of them probably should be changed
>> to use uaccess_begin() instead while at it.
>>
>> One question for the CPU people: do we actually care and need to do
>> this for things that might *write* to something? The speculative write
>> obviously is killed, but does it perhaps bring in a cacheline even
>> when killed?
>
> As far as I understand a write could trigger a request-for-ownership
> read for the target cacheline.
Oh, absolutely.
I just wonder at what point that happens.
Honestly, trying to get exclusive access to a cacheline can be _very_
expensive (not just for the local thread), so I would actually expect
that doing so for speculative writes is actually bad for performance.
That's doubly true because - unlike reads - there is no critical
latency issue, so trying to get the cache access started as early as
possible simply isn't all that important.
So I suspect that a write won't actually try to allocate the cacheline
until the write has actually retired.
End result: writes - unlike reads - *probably* will not speculatively
perturb the cache with speculative write addresses.
> Even though writes can trigger reads, as far as I can see the write
> needs to be dependent on the first out-of-bounds read
Yeah. A write on its own wouldn't matter, even if it were to perturb
the cache state, because the address already comes from user space, so
there's no new information in the cache perturbation for the attacker.
But that all implies that we shouldn't need the lfence for the
"put_user()" case, only for the get_user() (where the value we read
would then perhaps be used to do another access).
So we want to add the lfence (or "and") to get_user(), but not
necessarily put_user().
Agreed?
Linus
^ permalink raw reply
* Re: [PATCH bpf] selftests/bpf: fix test_align
From: Alexei Starovoitov @ 2018-01-08 23:35 UTC (permalink / raw)
To: Edward Cree, David S . Miller; +Cc: Daniel Borkmann, netdev
In-Reply-To: <14ffd15b-6f12-5d92-992a-66616af1a38f@solarflare.com>
On 1/8/18 8:38 AM, Edward Cree wrote:
> On 05/01/18 23:02, Alexei Starovoitov wrote:
>> since commit 82abbf8d2fc4 the verifier rejects the bit-wise
>> arithmetic on pointers earlier.
>> The test 'dubious pointer arithmetic' now has less output to match on.
>> Adjust it.
>>
>> Fixes: 82abbf8d2fc4 ("bpf: do not allow root to mangle valid pointers")
>> Reported-by: kernel test robot <xiaolong.ye@intel.com>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>> tools/testing/selftests/bpf/test_align.c | 22 +---------------------
>> 1 file changed, 1 insertion(+), 21 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
>> index 8591c89c0828..471bbbdb94db 100644
>> --- a/tools/testing/selftests/bpf/test_align.c
>> +++ b/tools/testing/selftests/bpf/test_align.c
>> @@ -474,27 +474,7 @@ static struct bpf_align_test tests[] = {
>> .result = REJECT,
>> .matches = {
>> {4, "R5=pkt(id=0,off=0,r=0,imm=0)"},
>> - /* ptr & 0x40 == either 0 or 0x40 */
>> - {5, "R5=inv(id=0,umax_value=64,var_off=(0x0; 0x40))"},
>> - /* ptr << 2 == unknown, (4n) */
>> - {7, "R5=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
>> - /* (4n) + 14 == (4n+2). We blow our bounds, because
>> - * the add could overflow.
>> - */
>> - {8, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
>> - /* Checked s>=0 */
>> - {10, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
>> - /* packet pointer + nonnegative (4n+2) */
>> - {12, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
>> - {14, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
>> - /* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
>> - * We checked the bounds, but it might have been able
>> - * to overflow if the packet pointer started in the
>> - * upper half of the address space.
>> - * So we did not get a 'range' on R6, and the access
>> - * attempt will fail.
>> - */
>> - {16, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
>> + /* R5 bitwise operator &= on pointer prohibited */
>> }
>> },
>> {
> Rather than neutering this test, we should change it to keep the part where
> it tests that a large pkt_ptr offset prevents us getting a reg->range.
> Specifically, in this test we have
> r2 = pkt
> r5 = large unknown scalar
> r6 = r2 + r5
> r4 = r6 + 4
> Then we check r4 < pkt_end, which normally would give r6->range = 4, but in
> this case must not do so since r6 could be (u64)(-2) in which case r4 = 2
> < pkt_end despite r6 not pointing into the packet.
> AFAICT there is not other coverage of this case in test_align, and I don't
> recall such a test being in test_verifier either. So please instead replace
> the insns that do prohibited ops on pointers with some other way of creating
> a large unknown scalar, and keep the rest of the test case intact.
makes sense. will send a follow up patch when security dust settles.
^ permalink raw reply
* Re: [PATCH bpf] bpf: prevent out-of-bounds speculation
From: Alexei Starovoitov @ 2018-01-08 23:20 UTC (permalink / raw)
To: Mark Rutland
Cc: David S . Miller, Daniel Borkmann, Jann Horn, Linus Torvalds,
Dan Williams, Peter Zijlstra, Elena Reshetova, Alan Cox, netdev,
kernel-team, will.deacon
In-Reply-To: <20180108170553.yrs46fawfpr62wtr@lakrids.cambridge.arm.com>
On 1/8/18 9:05 AM, Mark Rutland wrote:
> Hi Alexei,
>
> On Thu, Jan 04, 2018 at 08:28:11PM -0800, Alexei Starovoitov wrote:
>> From: Alexei Starovoitov <ast@kernel.org>
>>
>> Under speculation, CPUs may mis-predict branches in bounds checks. Thus,
>> memory accesses under a bounds check may be speculated even if the
>> bounds check fails, providing a primitive for building a side channel.
>>
>> To avoid leaking kernel data round up array-based maps and mask the index
>> after bounds check, so speculated load with out of bounds index will load
>> either valid value from the array or zero from the padded area.
>
> Thanks for putting this together, this certainly looks neat.
>
> I'm a little worried that in the presence of some CPU/compiler
> optimisations, the masking may effectively be skipped under speculation.
> So I'm not sure how robust this is going to be.
>
> More on that below.
>
>> To avoid duplicating map_lookup functions for root/unpriv always generate
>> a sequence of bpf instructions equivalent to map_lookup function for
>> array and array_of_maps map types when map was created by unpriv user.
>> And unconditionally mask index for percpu_array, since it's fast enough,
>> even when max_entries are not rounded to power of 2 for root user,
>> since percpu_array doesn't have map_gen_lookup callback yet.
>
> Is there a noticeable slowdown from the masking? Can't we always have
> that in place?
right. Please see v3 version:
https://patchwork.ozlabs.org/patch/856645/
Daniel noticed that speculation can happen without program being
loaded and we need to tighten the path via syscall as well.
so v3 is doing masking for all array types unconditionally.
The perf cost is within noise for interpreter and
not seen with JITed root code, since gen_lookup does not
emit AND for root.
>> @@ -157,7 +175,7 @@ static void *percpu_array_map_lookup_elem(struct bpf_map *map, void *key)
>> if (unlikely(index >= array->map.max_entries))
>> return NULL;
>>
>> - return this_cpu_ptr(array->pptrs[index]);
>> + return this_cpu_ptr(array->pptrs[index & array->index_mask]);
>
> As above, I think this isn't necessarily robust, as CPU/compiler
> optimisations can break the dependency on the index_mask, allowing
> speculation without a mask.
>
> e.g. a compiler could re-write this as:
>
> if (array->index_mask != 0xffffffff)
> index &= array->index_mask;
> return this_cpu_ptr(array->pptrs[index]);
>
> ... which would allow an unmasked index to be used in speculated paths.
prior to kernel I've been working on sun, gcc, llvm compilers
and I've never seen such optimization ever proposed for AND.
It makes no sense.
For heavy ALU like div/mod and calls compiler does indeed try
to predict the value. de-virtualization is an example optimization
for indirect calls. Intel compiler pioneered this approach back in 2000.
Compilers can also optimize "div by X" into
if (x == const)
unroll div by const into something faster;
else
div by X
Such optimizations are rarely done without profile feedback,
since branch is costly the compiler will add a branch only if there
is a clear win from introducing it instead of doing the operation.
For and, or, shift, add, sub there is never a case to do so.
Instead compiler is always trying to remove branches instead of
introducing them.
> Similar cases could occur with some CPU implementations. For example, HW
> value-prediction could result in the use of an all-ones mask under
> speculation.
please see the paper that Alan mentioned.
HW value speculation predicts likely valid values. It makes no sense
for HW to continue speculative execution with random value.
Consider array[index & index_mask]
if load index_mask stalls and cpu decides to continue speculation with
random value (both zero and ffff are considered random) it will proceed
through AND and second load will populate the precious cache
with completely irrelevant data.
Such cpu will be slower with speculative execution than without,
since it populates the caches with random data.
> I think that we may need to be able to provide an arch-specific
> pointer sanitization sequence (though we could certainly have masking as
> the default).
I still don't understand where this paranoia is coming from.
Kernel doesn't need to kill speculation. It needs to manage it.
> I have a rough idea as to how that could be plumbed into the JIT. First
> I need to verify the sequence I have in mind for arm/arm64 is
> sufficient.
hmm? the patch provided (both v2 and v3) doesn't need any JIT changes
on either x64, arm, etc.
gen_lookup() emits BPF_AND on index that JIT converts into actual AND
in native instruction set.
^ permalink raw reply
* [PATCH next-queue 2/2] ixgbe: add unlikely notes to tx fastpath expressions
From: Shannon Nelson @ 2018-01-08 22:47 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev, steffen.klassert
In-Reply-To: <1515451669-927-1-git-send-email-shannon.nelson@oracle.com>
Add unlikely() to a few error checking expressions in the Tx
offload handling.
Suggested-by: Yanjun Zhu <yanjun.zhu@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 57c10e6..3d069a2 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -749,28 +749,28 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
struct xfrm_state *xs;
struct tx_sa *tsa;
- if (!first->skb->sp->len) {
+ if (unlikely(!first->skb->sp->len)) {
netdev_err(tx_ring->netdev, "%s: no xfrm state len = %d\n",
__func__, first->skb->sp->len);
return 0;
}
xs = xfrm_input_state(first->skb);
- if (!xs) {
+ if (unlikely(!xs)) {
netdev_err(tx_ring->netdev, "%s: no xfrm_input_state() xs = %p\n",
__func__, xs);
return 0;
}
itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
- if (itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT) {
+ if (unlikely(itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
__func__, itd->sa_idx, xs->xso.offload_handle);
return 0;
}
tsa = &ipsec->tx_tbl[itd->sa_idx];
- if (!tsa->used) {
+ if (unlikely(!tsa->used)) {
netdev_err(tx_ring->netdev, "%s: unused sa_idx=%d\n",
__func__, itd->sa_idx);
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH next-queue 1/2] ixgbe: fix clean hw loop count
From: Shannon Nelson @ 2018-01-08 22:47 UTC (permalink / raw)
To: intel-wired-lan, jeffrey.t.kirsher; +Cc: netdev, steffen.klassert
Fix a cut-paste error so that we can clean all the table entries.
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index 12c7132..57c10e6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -148,7 +148,7 @@ static void ixgbe_ipsec_clear_hw_tables(struct ixgbe_adapter *adapter)
ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
ixgbe_ipsec_set_rx_ip(hw, idx, (__be32 *)buf);
}
- for (; idx < IXGBE_IPSEC_MAX_RX_IP_COUNT; idx++) {
+ for (; idx < IXGBE_IPSEC_MAX_SA_COUNT; idx++) {
ixgbe_ipsec_set_tx_sa(hw, idx, buf, 0);
ixgbe_ipsec_set_rx_sa(hw, idx, 0, buf, 0, 0, 0);
}
--
2.7.4
^ permalink raw reply related
* RE: [Intel-wired-lan] [PATCH 01/27] timecounter: Make cyclecounter struct part of timecounter struct
From: Brown, Aaron F @ 2018-01-08 22:20 UTC (permalink / raw)
To: Kamble, Sagar A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Richard Cochran,
Stephen Boyd, Chris Wilson, John Stultz,
intel-wired-lan-qjLDD68F18P21nG7glBr7A@public.gmane.org,
Thomas Gleixner, Kamble, Sagar A,
kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1513323522-15021-2-git-send-email-sagar.a.kamble-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces-CJG/fkoVOTIdnm+yROfE0A@public.gmane.org] On
> Behalf Of Sagar Arun Kamble
> Sent: Thursday, December 14, 2017 11:38 PM
> To: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>;
> Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>; Chris Wilson <chris@chris-
> wilson.co.uk>; John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>; intel-wired-
> lan-qjLDD68F18P21nG7glBr7A@public.gmane.org; Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>; Kamble, Sagar A
> <sagar.a.kamble-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; linux-arm-
> kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Subject: [Intel-wired-lan] [PATCH 01/27] timecounter: Make cyclecounter
> struct part of timecounter struct
>
> There is no real need for the users of timecounters to define cyclecounter
> and timecounter variables separately. Since timecounter will always be
> based on cyclecounter, have cyclecounter struct as member of timecounter
> struct.
>
> v2: Rebase.
>
> Suggested-by: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Chris Wilson <chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>
> Cc: Richard Cochran <richardcochran-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: John Stultz <john.stultz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: intel-wired-lan-qjLDD68F18P21nG7glBr7A@public.gmane.org
> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org
> Cc: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org
> Acked-by: Jeff Kirsher <jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> (Intel drivers)
> ---
> arch/microblaze/kernel/timer.c | 20 ++++++------
> drivers/clocksource/arm_arch_timer.c | 19 ++++++------
> drivers/net/ethernet/amd/xgbe/xgbe-dev.c | 3 +-
> drivers/net/ethernet/amd/xgbe/xgbe-ptp.c | 9 +++---
> drivers/net/ethernet/amd/xgbe/xgbe.h | 1 -
> drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 1 -
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 20 ++++++------
> drivers/net/ethernet/freescale/fec.h | 1 -
> drivers/net/ethernet/freescale/fec_ptp.c | 30 +++++++++---------
> drivers/net/ethernet/intel/e1000e/e1000.h | 1 -
> drivers/net/ethernet/intel/e1000e/netdev.c | 27 ++++++++--------
> drivers/net/ethernet/intel/e1000e/ptp.c | 2 +-
> drivers/net/ethernet/intel/igb/igb.h | 1 -
> drivers/net/ethernet/intel/igb/igb_ptp.c | 25 ++++++++-------
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 -
> drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 17 +++++-----
> drivers/net/ethernet/mellanox/mlx4/en_clock.c | 28 ++++++++---------
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 1 -
> .../net/ethernet/mellanox/mlx5/core/lib/clock.c | 34 ++++++++++----------
> drivers/net/ethernet/qlogic/qede/qede_ptp.c | 20 ++++++------
> drivers/net/ethernet/ti/cpts.c | 36 ++++++++++++----------
> drivers/net/ethernet/ti/cpts.h | 1 -
> include/linux/mlx5/driver.h | 1 -
> include/linux/timecounter.h | 4 +--
> include/sound/hdaudio.h | 1 -
> kernel/time/timecounter.c | 28 ++++++++---------
> sound/hda/hdac_stream.c | 7 +++--
> virt/kvm/arm/arch_timer.c | 6 ++--
> 28 files changed, 163 insertions(+), 182 deletions(-)
>
For Intel e1000e and igb drivers:
Tested-by: Aaron Brown <aaron.f.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: dvb usb issues since kernel 4.9
From: Jesper Dangaard Brouer @ 2018-01-08 22:16 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Josef Griebichler, Mauro Carvalho Chehab, Alan Stern,
Greg Kroah-Hartman, linux-usb, Eric Dumazet, Rik van Riel,
Paolo Abeni, Hannes Frederic Sowa, linux-kernel, netdev,
Jonathan Corbet, LMML, David Miller, torvalds
In-Reply-To: <20180108214427.GT29822@worktop.programming.kicks-ass.net>
On Mon, 8 Jan 2018 22:44:27 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Jan 08, 2018 at 10:31:09PM +0100, Jesper Dangaard Brouer wrote:
> > I did expected the issue to get worse, when you load the Pi with
> > network traffic, as now the softirq time-budget have to be shared
> > between networking and USB/DVB. Thus, I guess you are running TCP and
> > USB/mpeg2ts on the same CPU (why when you have 4 CPUs?...)
>
> Isn't networking also over USB on the Pi ?
Darn, that is true. Looking at the dmesg output in http://ix.io/DOg:
[ 0.405942] usbcore: registered new interface driver smsc95xx
[ 5.821104] smsc95xx 1-1.1:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0x45E1
I don't know enough about USB... is it possible to control which CPU
handles the individual USB ports, or on some other level (than ports)?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: introduce BPF_JIT_ALWAYS_ON config
From: Daniel Borkmann @ 2018-01-08 21:59 UTC (permalink / raw)
To: Alexei Starovoitov, davem; +Cc: torvalds, jannh, alan, netdev, kernel-team
In-Reply-To: <20180108033519.3232547-1-ast@kernel.org>
On 01/08/2018 04:35 AM, Alexei Starovoitov wrote:
> The BPF interpreter has been used as part of the spectre 2 attack CVE-2017-5715.
>
> A quote from goolge project zero blog:
> "At this point, it would normally be necessary to locate gadgets in
> the host kernel code that can be used to actually leak data by reading
> from an attacker-controlled location, shifting and masking the result
> appropriately and then using the result of that as offset to an
> attacker-controlled address for a load. But piecing gadgets together
> and figuring out which ones work in a speculation context seems annoying.
> So instead, we decided to use the eBPF interpreter, which is built into
> the host kernel - while there is no legitimate way to invoke it from inside
> a VM, the presence of the code in the host kernel's text section is sufficient
> to make it usable for the attack, just like with ordinary ROP gadgets."
>
> To make attacker job harder introduce BPF_JIT_ALWAYS_ON config
> option that removes interpreter from the kernel in favor of JIT-only mode.
> So far eBPF JIT is supported by:
> x64, arm64, arm32, sparc64, s390, powerpc64, mips64
>
> The start of JITed program is randomized and code page is marked as read-only.
> In addition "constant blinding" can be turned on with net.core.bpf_jit_harden
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> init/Kconfig | 7 +++++++
> kernel/bpf/core.c | 9 +++++++++
> kernel/bpf/verifier.c | 4 ++++
> net/core/sysctl_net_core.c | 9 +++++++++
> 4 files changed, 29 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 2934249fba46..5e2a4a391ba9 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1392,6 +1392,13 @@ config BPF_SYSCALL
> Enable the bpf() system call that allows to manipulate eBPF
> programs and maps via file descriptors.
>
> +config BPF_JIT_ALWAYS_ON
> + bool "Permanently enable BPF JIT and remove BPF interpreter"
> + depends on BPF_SYSCALL && HAVE_EBPF_JIT && BPF_JIT
> + help
> + Enables BPF JIT and removes BPF interpreter to avoid
> + speculative execution of BPF instructions by the interpreter
> +
> config USERFAULTFD
> bool "Enable userfaultfd() system call"
> select ANON_INODES
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 70a534549cd3..42756c434e0b 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -781,6 +781,7 @@ noinline u64 __bpf_call_base(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
> }
> EXPORT_SYMBOL_GPL(__bpf_call_base);
>
> +#ifndef CONFIG_BPF_JIT_ALWAYS_ON
> /**
> * __bpf_prog_run - run eBPF program on a given context
> * @ctx: is the data we are operating on
> @@ -1376,6 +1377,7 @@ void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth)
> __bpf_call_base_args;
> insn->code = BPF_JMP | BPF_CALL_ARGS;
> }
> +#endif
>
> bool bpf_prog_array_compatible(struct bpf_array *array,
> const struct bpf_prog *fp)
> @@ -1427,9 +1429,11 @@ static int bpf_check_tail_call(const struct bpf_prog *fp)
> */
> struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
> {
> +#ifndef CONFIG_BPF_JIT_ALWAYS_ON
> u32 stack_depth = max_t(u32, fp->aux->stack_depth, 1);
>
> fp->bpf_func = interpreters[(round_up(stack_depth, 32) / 32) - 1];
> +#endif
>
> /* eBPF JITs can rewrite the program in case constant
> * blinding is active. However, in case of error during
> @@ -1453,6 +1457,11 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
> */
> *err = bpf_check_tail_call(fp);
>
> +#ifdef CONFIG_BPF_JIT_ALWAYS_ON
> + if (!fp->jited)
> + *err = -ENOTSUPP;
> +#endif
This part here and ...
> return fp;
> }
> EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
[...]
> @@ -524,6 +530,9 @@ static __net_initdata struct pernet_operations sysctl_core_ops = {
>
> static __init int sysctl_core_init(void)
> {
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_JIT_ALWAYS_ON)
> + bpf_jit_enable = 1;
> +#endif
... this one will race and break stuff in the current shape, one example
is the PTP classifier in the tree: sysctl_core_init() is done in fs_initcall(),
whereas ptp_classifier_init() is done in sock_init() which is done out of
core_initcall().
So what will happen is that at this point in time bpf_jit_enable is not yet
set to 1, so when ptp_classifier_init() calls the cBPF bpf_prog_create(), it
will migrate the insns over to eBPF and in bpf_prog_select_runtime() called
from bpf_migrate_filter() have the assumption that we always succeed here
since when JIT fails, we will fall back to the interpreter anyway. The only
error up until now in bpf_prog_select_runtime() that could happen is out of
native eBPF prog load, so bpf_migrate_filter() will thus return just fine
and on first call to PTP classifier from a network packet, we'll get NULL
pointer deref since the fp->bpf_func is still NULL. So this would rather
need to be set much earlier on init or e.g. in the JITs themselves.
Other than that I was wondering whether the arm32 eBPF JIT could cause
trouble for cBPF as well, but it looks not the case since only alu64 div/mod
and xadd is not implemented there yet, so that should be ok since not used
in the migration.
> register_net_sysctl(&init_net, "net/core", net_core_table);
> return register_pernet_subsys(&sysctl_core_ops);
> }
>
^ permalink raw reply
* Re: dvb usb issues since kernel 4.9
From: Peter Zijlstra @ 2018-01-08 21:44 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Josef Griebichler, Mauro Carvalho Chehab, Alan Stern,
Greg Kroah-Hartman, linux-usb, Eric Dumazet, Rik van Riel,
Paolo Abeni, Hannes Frederic Sowa, linux-kernel, netdev,
Jonathan Corbet, LMML, David Miller, torvalds
In-Reply-To: <20180108223109.66c91554@redhat.com>
On Mon, Jan 08, 2018 at 10:31:09PM +0100, Jesper Dangaard Brouer wrote:
> I did expected the issue to get worse, when you load the Pi with
> network traffic, as now the softirq time-budget have to be shared
> between networking and USB/DVB. Thus, I guess you are running TCP and
> USB/mpeg2ts on the same CPU (why when you have 4 CPUs?...)
Isn't networking also over USB on the Pi ?
^ permalink raw reply
* Re: dvb usb issues since kernel 4.9
From: Jesper Dangaard Brouer @ 2018-01-08 21:31 UTC (permalink / raw)
To: Josef Griebichler
Cc: Mauro Carvalho Chehab, Alan Stern, Greg Kroah-Hartman, linux-usb,
Eric Dumazet, Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
linux-kernel, netdev, Jonathan Corbet, LMML, Peter Zijlstra,
David Miller, torvalds
In-Reply-To: <trinity-c7ec7cbd-a186-4a2a-bcb6-cce8993d6a90-1515428770628@3c-app-gmx-bs32>
On Mon, 8 Jan 2018 17:26:10 +0100
"Josef Griebichler" <griebichler.josef@gmx.at> wrote:
> I tried your mentioned patch but unfortunately no real improvement for me.
> dmesg http://ix.io/DOg
> tvheadend service log http://ix.io/DOi
>
> Errors during recording are still there.
Are you _also_ recording the stream on the Raspberry Pi?
It seems to me, that you are expecting too much from this small device.
> Errors increase if there is additional tcp load on raspberry.
I did expected the issue to get worse, when you load the Pi with
network traffic, as now the softirq time-budget have to be shared
between networking and USB/DVB. Thus, I guess you are running TCP and
USB/mpeg2ts on the same CPU (why when you have 4 CPUs?...)
If you expect/want to get stable performance out of such a small box,
then you (or LibreELEC) need to tune the box for this usage. And it
does not have to be that complicated. First step is to move IRQ
handling for the NIC to another CPU and than the USB port handling the
DVB signal (/proc/irq/*/smp_affinity_list). And then pin the
userspace process (taskset) to another CPU than the one handling
USB-softirq.
> Unfortunately there's no usbmon or tshark on libreelec so I can't
> provide further logs.
Do you have perf or trace-cmd on the box? Maybe we could come up with
some kernel functions to trace, to measure/show the latency spikes?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ 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