Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
From: Joe Smith @ 2017-08-10  0:20 UTC (permalink / raw)
  To: Jerry Chu
  Cc: Yuchung Cheng, Rao Shoaib, David Miller, Alexey Kuznetsov, netdev
In-Reply-To: <CAPshTCgaYVPEbZxDr9sUz7OFibweQn6fwRtP66cgw5eNrHUOWA@mail.gmail.com>

On Wed, Aug 9, 2017 at 4:52 PM, Jerry Chu <hkchu@google.com> wrote:
> [try to recover from long lost memory]
>
> On Tue, Aug 8, 2017 at 10:25 AM, Yuchung Cheng <ycheng@google.com> wrote:
>> On Mon, Aug 7, 2017 at 11:16 AM, Rao Shoaib <rao.shoaib@oracle.com> wrote:
>>> Change from version 0: Rationale behind the change:
>>>
>>> The man page for tcp(7) states
>>>
>>> when used with the TCP keepalive (SO_KEEPALIVE) option, TCP_USER_TIMEOUT will
>>> override keepalive to  determine  when to close a connection due to keepalive
>>> failure.
>>>
>>> This is ambigious at best. user expectation is most likely that the connection
>>> will be reset after TCP_USER_TIMEOUT milliseconds of inactivity.
>> ccing the original author Jerry Chu who can tell more.
>>
>
> There was a reason for the above otherwise I wouldn't have explicitly
> spelled it out in
> my commit msg. But unfortunately it was seven years ago and I can't
> remember why.
> It could range from micro-optimization (saving a syscall() because
> this facility was
> used by servers handling millions of Android clients) to something more critical
> but I can't remember.

The issue is that the man page is ambiguous and does not conform to
any standard.
Whether  RFC 5482 is in little use or not that was cited as the basis
of this change and
I want to change the behavior to conform to it as users are confused.

I doubt that saving a syscall is of any benefit when the connection
has been idle for 2hrs. If anything the user expects the keep alive
probes to start after TCP_USER_TIMEOUT of inactivity. In which case
keep alive should be adjusted.

>
>>>
>>> The code however waits for the keepalive to kick-in (default 2hrs) and than
>>> after one failure resets the conenction.
>>>
>>> What is the rationale for that ? The same effect can be obtained by simply
>>> changing the value of tcp_keep_alive_probes.
>>>
>>> Since the TCP_USER_TIMEOUT option was added based on RFC 5482 we need to follow
>>> the RFC. Which states
>
> Well the patch has little to do with RFC5482 other than borrowing the name, and
> also conveniently providing a mechanism for RFC5482 apps to program the local
> timeout value. As far as I knew back when I worked on the patch, RFC5482 was
> under little use (told directly by Lars).
>
> Your proposed change may not be unreasonable but my fear is it may
> cause breakage
> on apps that depend on "TCP_USER_TIMEOUT will overtake keepalive to determine
> when to close a connection due to keepalive failure". What is your
> case for "RFC5482
> compliance" after all? I know the TCP_USER_TIMEOUT option has been very popular
> among apps since its inception.

The only use of TCP_USER_TIMEOUT has been for flushing unacknowledged
data (evident from all the fixes). That behavior is not being touched.

Making Linux conform to standards and behavior that is logical seems
like a good enough reason. Mixing keep alive and TCP_USER_TIMEOUT does
not make any sense. I doubt very much if this change will break
anything but if it does than we need to see why that is needed and
implement a proper fix and document it.

Shoaib

>
>>>
>>> 4.2 TCP keep-Alives:
>>>    Some TCP implementations, such as those in BSD systems, use a
>>>    different abort policy for TCP keep-alives than for user data.  Thus,
>>>    the TCP keep-alive mechanism might abort a connection that would
>>>    otherwise have survived the transient period without connectivity.
>>>    Therefore, if a connection that enables keep-alives is also using the
>>>    TCP User Timeout Option, then the keep-alive timer MUST be set to a
>>>    value larger than that of the adopted USER TIMEOUT.
>>>
>>> This patch enforces the MUST and also dis-associates user timeout from keep
>>> alive.  A man page patch will be submitted separately.
>>>
>>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>>> ---
>>>  net/ipv4/tcp.c       | 10 ++++++++--
>>>  net/ipv4/tcp_timer.c |  9 +--------
>>>  2 files changed, 9 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index 71ce33d..f2af44d 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -2628,7 +2628,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>>                 break;
>>>
>>>         case TCP_KEEPIDLE:
>>> -               if (val < 1 || val > MAX_TCP_KEEPIDLE)
>>> +               /* Per RFC5482 keepalive_time must be > user_timeout */
>>> +               if (val < 1 || val > MAX_TCP_KEEPIDLE ||
>>> +                   ((val * HZ) <= icsk->icsk_user_timeout))
>>>                         err = -EINVAL;
>>>                 else {
>>>                         tp->keepalive_time = val * HZ;
>>> @@ -2724,8 +2726,12 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>>         case TCP_USER_TIMEOUT:
>>>                 /* Cap the max time in ms TCP will retry or probe the window
>>>                  * before giving up and aborting (ETIMEDOUT) a connection.
>>> +                * Per RFC5482 TCP user timeout must be < keepalive_time.
>>> +                * If the default value changes later -- all bets are off.
>>>                  */
>>> -               if (val < 0)
>>> +               if (val < 0 || (tp->keepalive_time &&
>>> +                               tp->keepalive_time <= msecs_to_jiffies(val)) ||
>>> +                  net->ipv4.sysctl_tcp_keepalive_time <= msecs_to_jiffies(val))
>>>                         err = -EINVAL;
>>>                 else
>>>                         icsk->icsk_user_timeout = msecs_to_jiffies(val);
>>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>>> index c0feeee..d39fe60 100644
>>> --- a/net/ipv4/tcp_timer.c
>>> +++ b/net/ipv4/tcp_timer.c
>>> @@ -664,14 +664,7 @@ static void tcp_keepalive_timer (unsigned long data)
>>>         elapsed = keepalive_time_elapsed(tp);
>>>
>>>         if (elapsed >= keepalive_time_when(tp)) {
>>> -               /* If the TCP_USER_TIMEOUT option is enabled, use that
>>> -                * to determine when to timeout instead.
>>> -                */
>>> -               if ((icsk->icsk_user_timeout != 0 &&
>>> -                   elapsed >= icsk->icsk_user_timeout &&
>>> -                   icsk->icsk_probes_out > 0) ||
>>> -                   (icsk->icsk_user_timeout == 0 &&
>>> -                   icsk->icsk_probes_out >= keepalive_probes(tp))) {
>>> +               if (icsk->icsk_probes_out >= keepalive_probes(tp)) {
>>>                         tcp_send_active_reset(sk, GFP_ATOMIC);
>>>                         tcp_write_err(sk);
>>>                         goto out;
>>> --
>>> 2.7.4
>>>



-- 
JS

^ permalink raw reply

* Re: [PATCH net-next v2 0/9] Add BPF_J{LT,LE,SLT,SLE} instructions
From: David Miller @ 2017-08-09 23:55 UTC (permalink / raw)
  To: daniel; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 10 Aug 2017 01:39:54 +0200

> This set adds BPF_J{LT,LE,SLT,SLE} instructions to the BPF
> insn set, interpreter, JIT hardening code and all JITs are
> also updated to support the new instructions. Basic idea is
> to reduce register pressure by avoiding BPF_J{GT,GE,SGT,SGE}
> rewrites. Removing the workaround for the rewrites in LLVM,
> this can result in shorter BPF programs, less stack usage
> and less verification complexity. First patch provides some
> more details on rationale and integration.
> 
> Thanks a lot!
> 
> v1 -> v2:
>   - Reworded commit msg in patch 1

Ok this looks a lot better.

Thanks for taking care of all of the JITs as well.

Series applied.

^ permalink raw reply

* Re: [PATCH v1 net] TCP_USER_TIMEOUT and tcp_keepalive should conform to RFC5482
From: Jerry Chu @ 2017-08-09 23:52 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: Rao Shoaib, David Miller, Alexey Kuznetsov, netdev
In-Reply-To: <CAK6E8=ehu5uK6bsprzAQc-Nu6WS2YQEcktD0H2RrjgPpS47-Yg@mail.gmail.com>

[try to recover from long lost memory]

On Tue, Aug 8, 2017 at 10:25 AM, Yuchung Cheng <ycheng@google.com> wrote:
> On Mon, Aug 7, 2017 at 11:16 AM, Rao Shoaib <rao.shoaib@oracle.com> wrote:
>> Change from version 0: Rationale behind the change:
>>
>> The man page for tcp(7) states
>>
>> when used with the TCP keepalive (SO_KEEPALIVE) option, TCP_USER_TIMEOUT will
>> override keepalive to  determine  when to close a connection due to keepalive
>> failure.
>>
>> This is ambigious at best. user expectation is most likely that the connection
>> will be reset after TCP_USER_TIMEOUT milliseconds of inactivity.
> ccing the original author Jerry Chu who can tell more.
>

There was a reason for the above otherwise I wouldn't have explicitly
spelled it out in
my commit msg. But unfortunately it was seven years ago and I can't
remember why.
It could range from micro-optimization (saving a syscall() because
this facility was
used by servers handling millions of Android clients) to something more critical
but I can't remember.

>>
>> The code however waits for the keepalive to kick-in (default 2hrs) and than
>> after one failure resets the conenction.
>>
>> What is the rationale for that ? The same effect can be obtained by simply
>> changing the value of tcp_keep_alive_probes.
>>
>> Since the TCP_USER_TIMEOUT option was added based on RFC 5482 we need to follow
>> the RFC. Which states

Well the patch has little to do with RFC5482 other than borrowing the name, and
also conveniently providing a mechanism for RFC5482 apps to program the local
timeout value. As far as I knew back when I worked on the patch, RFC5482 was
under little use (told directly by Lars).

Your proposed change may not be unreasonable but my fear is it may
cause breakage
on apps that depend on "TCP_USER_TIMEOUT will overtake keepalive to determine
when to close a connection due to keepalive failure". What is your
case for "RFC5482
compliance" after all? I know the TCP_USER_TIMEOUT option has been very popular
among apps since its inception.

>>
>> 4.2 TCP keep-Alives:
>>    Some TCP implementations, such as those in BSD systems, use a
>>    different abort policy for TCP keep-alives than for user data.  Thus,
>>    the TCP keep-alive mechanism might abort a connection that would
>>    otherwise have survived the transient period without connectivity.
>>    Therefore, if a connection that enables keep-alives is also using the
>>    TCP User Timeout Option, then the keep-alive timer MUST be set to a
>>    value larger than that of the adopted USER TIMEOUT.
>>
>> This patch enforces the MUST and also dis-associates user timeout from keep
>> alive.  A man page patch will be submitted separately.
>>
>> Signed-off-by: Rao Shoaib <rao.shoaib@oracle.com>
>> ---
>>  net/ipv4/tcp.c       | 10 ++++++++--
>>  net/ipv4/tcp_timer.c |  9 +--------
>>  2 files changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 71ce33d..f2af44d 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -2628,7 +2628,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>                 break;
>>
>>         case TCP_KEEPIDLE:
>> -               if (val < 1 || val > MAX_TCP_KEEPIDLE)
>> +               /* Per RFC5482 keepalive_time must be > user_timeout */
>> +               if (val < 1 || val > MAX_TCP_KEEPIDLE ||
>> +                   ((val * HZ) <= icsk->icsk_user_timeout))
>>                         err = -EINVAL;
>>                 else {
>>                         tp->keepalive_time = val * HZ;
>> @@ -2724,8 +2726,12 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>         case TCP_USER_TIMEOUT:
>>                 /* Cap the max time in ms TCP will retry or probe the window
>>                  * before giving up and aborting (ETIMEDOUT) a connection.
>> +                * Per RFC5482 TCP user timeout must be < keepalive_time.
>> +                * If the default value changes later -- all bets are off.
>>                  */
>> -               if (val < 0)
>> +               if (val < 0 || (tp->keepalive_time &&
>> +                               tp->keepalive_time <= msecs_to_jiffies(val)) ||
>> +                  net->ipv4.sysctl_tcp_keepalive_time <= msecs_to_jiffies(val))
>>                         err = -EINVAL;
>>                 else
>>                         icsk->icsk_user_timeout = msecs_to_jiffies(val);
>> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
>> index c0feeee..d39fe60 100644
>> --- a/net/ipv4/tcp_timer.c
>> +++ b/net/ipv4/tcp_timer.c
>> @@ -664,14 +664,7 @@ static void tcp_keepalive_timer (unsigned long data)
>>         elapsed = keepalive_time_elapsed(tp);
>>
>>         if (elapsed >= keepalive_time_when(tp)) {
>> -               /* If the TCP_USER_TIMEOUT option is enabled, use that
>> -                * to determine when to timeout instead.
>> -                */
>> -               if ((icsk->icsk_user_timeout != 0 &&
>> -                   elapsed >= icsk->icsk_user_timeout &&
>> -                   icsk->icsk_probes_out > 0) ||
>> -                   (icsk->icsk_user_timeout == 0 &&
>> -                   icsk->icsk_probes_out >= keepalive_probes(tp))) {
>> +               if (icsk->icsk_probes_out >= keepalive_probes(tp)) {
>>                         tcp_send_active_reset(sk, GFP_ATOMIC);
>>                         tcp_write_err(sk);
>>                         goto out;
>> --
>> 2.7.4
>>

^ permalink raw reply

* Re: [PATCH net-next 0/2] zerocopy fixes
From: David Miller @ 2017-08-09 23:49 UTC (permalink / raw)
  To: willemdebruijn.kernel; +Cc: netdev, dsahern, willemb
In-Reply-To: <20170809230944.59289-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed,  9 Aug 2017 19:09:42 -0400

> Fix two issues introduced in the msg_zerocopy patchset.

Series applied, thanks for following up on this so quickly.

^ permalink raw reply

* Re: [net-next 00/12][pull request] 1GbE Intel Wired LAN Driver Updates 2017-08-08
From: David Miller @ 2017-08-09 23:49 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20170809214746.28139-1-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed,  9 Aug 2017 14:47:34 -0700

> This series contains updates to e1000e and igb/igbvf.
 ...
> The following are changes since commit 53b948356554376ec6f89016376825d48bf396c3:
>   net: vrf: Add extack messages for newlink failures
> and are available in the git repository at:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE

Pulled, thanks Jeff.

^ permalink raw reply

* Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
From: John Stultz @ 2017-08-09 23:44 UTC (permalink / raw)
  To: Cong Wang
  Cc: lkml, Network Development, Linux USB List, David S. Miller,
	Felipe Balbi, Wei Wang
In-Reply-To: <CAM_iQpW2=aKKUmWanbuhqXdZGxr_TqB5pQ+U+A0+=optuGX5-Q@mail.gmail.com>

On Wed, Aug 9, 2017 at 4:34 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> (Cc'ing Wei whose commit was blamed)
>
> On Mon, Aug 7, 2017 at 2:15 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Mon, Aug 7, 2017 at 2:05 PM, John Stultz <john.stultz@linaro.org> wrote:
>>> So, with recent testing with my HiKey board, I've been noticing some
>>> quirky behavior with my USB eth adapter.
>>>
>>> Basically, pluging the usb eth adapter in and then removing it, when
>>> plugging it back in I often find that its not detected, and the system
>>> slowly spits out the following message over and over:
>>>   unregister_netdevice: waiting for eth0 to become free. Usage count = 1
>>
>> The other bit is that after this starts printing, the board will no
>> longer reboot (it hangs continuing to occasionally print the above
>> message), and I have to manually reset the device.
>>
>
> So this warning is not temporarily shown but lasts until a reboot,
> right? If so it is a dst refcnt leak.

Correct, once I get into the state it lasts until a reboot.

> How reproducible is it for you? From my reading, it seems always
> reproduced when you unplug and plug your usb eth interface?
> Is there anything else involved? For example, network namespace.

So with 4.13-rc3/4 I seem to trigger it easily, often with the first
unplug of the USB eth adapter.

But as I get back closer to 4.12, it seemingly becomes harder to
trigger, but sometimes still happens.

So far, I've not been able to trigger it with 4.12.

I don't think network namespaces are involved?  Though its out of my
area, so AOSP may be using them these days.  Is there a simple way to
check?

I'll also do another bisection to see if the bad point moves back any further.

thanks
-john

^ permalink raw reply

* [PATCH net-next v2 9/9] bpf: add test cases for new BPF_J{LT, LE, SLT, SLE} instructions
From: Daniel Borkmann @ 2017-08-09 23:40 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

Add test cases to the verifier selftest suite in order to verify that
i) direct packet access, and ii) dynamic map value access is working
with the changes related to the new instructions.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/testing/selftests/bpf/test_verifier.c | 313 ++++++++++++++++++++++++++++
 1 file changed, 313 insertions(+)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 65aa562..70973c1 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -2830,6 +2830,79 @@ struct test_val {
 		.result = ACCEPT,
 	},
 	{
+		"direct packet access: test25 (marking on <, good access)",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, data)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_end)),
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+			BPF_JMP_REG(BPF_JLT, BPF_REG_0, BPF_REG_3, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+			BPF_JMP_IMM(BPF_JA, 0, 0, -4),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	},
+	{
+		"direct packet access: test26 (marking on <, bad access)",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, data)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_end)),
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+			BPF_JMP_REG(BPF_JLT, BPF_REG_0, BPF_REG_3, 3),
+			BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JA, 0, 0, -3),
+		},
+		.result = REJECT,
+		.errstr = "invalid access to packet",
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	},
+	{
+		"direct packet access: test27 (marking on <=, good access)",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, data)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_end)),
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+			BPF_JMP_REG(BPF_JLE, BPF_REG_3, BPF_REG_0, 1),
+			BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 1),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	},
+	{
+		"direct packet access: test28 (marking on <=, bad access)",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+				    offsetof(struct __sk_buff, data)),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+				    offsetof(struct __sk_buff, data_end)),
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+			BPF_JMP_REG(BPF_JLE, BPF_REG_3, BPF_REG_0, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 1),
+			BPF_EXIT_INSN(),
+			BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+			BPF_JMP_IMM(BPF_JA, 0, 0, -4),
+		},
+		.result = REJECT,
+		.errstr = "invalid access to packet",
+		.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+	},
+	{
 		"helper access to packet: test1, valid packet_ptr range",
 		.insns = {
 			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
@@ -4488,6 +4561,246 @@ struct test_val {
 		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
 	},
 	{
+		"helper access to map: bounds check using <, good access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JLT, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using <, bad access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JLT, BPF_REG_3, 32, 4),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = REJECT,
+		.errstr = "R1 unbounded memory access",
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using <=, good access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JLE, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using <=, bad access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JLE, BPF_REG_3, 32, 4),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = REJECT,
+		.errstr = "R1 unbounded memory access",
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<, good access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, 0, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<, good access 2",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, -3, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<, bad access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLT, BPF_REG_3, -3, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = REJECT,
+		.errstr = "R1 min value is negative",
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<=, good access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, 0, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<=, good access 2",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, -3, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
+		"helper access to map: bounds check using s<=, bad access",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+			BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+			BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 0),
+			BPF_LD_MAP_FD(BPF_REG_1, 0),
+			BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+			BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+			BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+			BPF_LDX_MEM(BPF_DW, BPF_REG_3, BPF_REG_0, 0),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, 32, 2),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JSLE, BPF_REG_3, -3, -3),
+			BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_3),
+			BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 0),
+			BPF_MOV64_IMM(BPF_REG_0, 0),
+			BPF_EXIT_INSN(),
+		},
+		.fixup_map2 = { 3 },
+		.result = REJECT,
+		.errstr = "R1 min value is negative",
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
+	},
+	{
 		"map element value is preserved across register spilling",
 		.insns = {
 			BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 8/9] bpf: enable BPF_J{LT,LE,SLT,SLE} opcodes in verifier
From: Daniel Borkmann @ 2017-08-09 23:40 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

Enable the newly added jump opcodes, main parts are in two
different areas, namely direct packet access and dynamic map
value access. For the direct packet access, we now allow for
the following two new patterns to match in order to trigger
markings with find_good_pkt_pointers():

Variant 1 (access ok when taking the branch):

  0: (61) r2 = *(u32 *)(r1 +76)
  1: (61) r3 = *(u32 *)(r1 +80)
  2: (bf) r0 = r2
  3: (07) r0 += 8
  4: (ad) if r0 < r3 goto pc+2
  R0=pkt(id=0,off=8,r=0) R1=ctx R2=pkt(id=0,off=0,r=0)
  R3=pkt_end R10=fp
  5: (b7) r0 = 0
  6: (95) exit

  from 4 to 7: R0=pkt(id=0,off=8,r=8) R1=ctx
               R2=pkt(id=0,off=0,r=8) R3=pkt_end R10=fp
  7: (71) r0 = *(u8 *)(r2 +0)
  8: (05) goto pc-4
  5: (b7) r0 = 0
  6: (95) exit
  processed 11 insns, stack depth 0

Variant 2 (access ok on fall-through):

  0: (61) r2 = *(u32 *)(r1 +76)
  1: (61) r3 = *(u32 *)(r1 +80)
  2: (bf) r0 = r2
  3: (07) r0 += 8
  4: (bd) if r3 <= r0 goto pc+1
  R0=pkt(id=0,off=8,r=8) R1=ctx R2=pkt(id=0,off=0,r=8)
  R3=pkt_end R10=fp
  5: (71) r0 = *(u8 *)(r2 +0)
  6: (b7) r0 = 1
  7: (95) exit

  from 4 to 6: R0=pkt(id=0,off=8,r=0) R1=ctx
               R2=pkt(id=0,off=0,r=0) R3=pkt_end R10=fp
  6: (b7) r0 = 1
  7: (95) exit
  processed 10 insns, stack depth 0

The above two basically just swap the branches where we need
to handle an exception and allow packet access compared to the
two already existing variants for find_good_pkt_pointers().

For the dynamic map value access, we add the new instructions
to reg_set_min_max() and reg_set_min_max_inv() in order to
learn bounds. Verifier test cases for both are added in a
follow-up patch.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
 kernel/bpf/verifier.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8160a81..ecc590e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -312,11 +312,15 @@ static void print_verifier_state(struct bpf_verifier_state *state)
 	[BPF_JA >> 4]   = "jmp",
 	[BPF_JEQ >> 4]  = "==",
 	[BPF_JGT >> 4]  = ">",
+	[BPF_JLT >> 4]  = "<",
 	[BPF_JGE >> 4]  = ">=",
+	[BPF_JLE >> 4]  = "<=",
 	[BPF_JSET >> 4] = "&",
 	[BPF_JNE >> 4]  = "!=",
 	[BPF_JSGT >> 4] = "s>",
+	[BPF_JSLT >> 4] = "s<",
 	[BPF_JSGE >> 4] = "s>=",
+	[BPF_JSLE >> 4] = "s<=",
 	[BPF_CALL >> 4] = "call",
 	[BPF_EXIT >> 4] = "exit",
 };
@@ -2383,27 +2387,37 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *state,
 		 */
 		return;
 
-	/* LLVM can generate two kind of checks:
+	/* LLVM can generate four kind of checks:
 	 *
-	 * Type 1:
+	 * Type 1/2:
 	 *
 	 *   r2 = r3;
 	 *   r2 += 8;
 	 *   if (r2 > pkt_end) goto <handle exception>
 	 *   <access okay>
 	 *
+	 *   r2 = r3;
+	 *   r2 += 8;
+	 *   if (r2 < pkt_end) goto <access okay>
+	 *   <handle exception>
+	 *
 	 *   Where:
 	 *     r2 == dst_reg, pkt_end == src_reg
 	 *     r2=pkt(id=n,off=8,r=0)
 	 *     r3=pkt(id=n,off=0,r=0)
 	 *
-	 * Type 2:
+	 * Type 3/4:
 	 *
 	 *   r2 = r3;
 	 *   r2 += 8;
 	 *   if (pkt_end >= r2) goto <access okay>
 	 *   <handle exception>
 	 *
+	 *   r2 = r3;
+	 *   r2 += 8;
+	 *   if (pkt_end <= r2) goto <handle exception>
+	 *   <access okay>
+	 *
 	 *   Where:
 	 *     pkt_end == dst_reg, r2 == src_reg
 	 *     r2=pkt(id=n,off=8,r=0)
@@ -2471,6 +2485,14 @@ static void reg_set_min_max(struct bpf_reg_state *true_reg,
 		false_reg->smax_value = min_t(s64, false_reg->smax_value, val);
 		true_reg->smin_value = max_t(s64, true_reg->smin_value, val + 1);
 		break;
+	case BPF_JLT:
+		false_reg->umin_value = max(false_reg->umin_value, val);
+		true_reg->umax_value = min(true_reg->umax_value, val - 1);
+		break;
+	case BPF_JSLT:
+		false_reg->smin_value = max_t(s64, false_reg->smin_value, val);
+		true_reg->smax_value = min_t(s64, true_reg->smax_value, val - 1);
+		break;
 	case BPF_JGE:
 		false_reg->umax_value = min(false_reg->umax_value, val - 1);
 		true_reg->umin_value = max(true_reg->umin_value, val);
@@ -2479,6 +2501,14 @@ static void reg_set_min_max(struct bpf_reg_state *true_reg,
 		false_reg->smax_value = min_t(s64, false_reg->smax_value, val - 1);
 		true_reg->smin_value = max_t(s64, true_reg->smin_value, val);
 		break;
+	case BPF_JLE:
+		false_reg->umin_value = max(false_reg->umin_value, val + 1);
+		true_reg->umax_value = min(true_reg->umax_value, val);
+		break;
+	case BPF_JSLE:
+		false_reg->smin_value = max_t(s64, false_reg->smin_value, val + 1);
+		true_reg->smax_value = min_t(s64, true_reg->smax_value, val);
+		break;
 	default:
 		break;
 	}
@@ -2527,6 +2557,14 @@ static void reg_set_min_max_inv(struct bpf_reg_state *true_reg,
 		true_reg->smax_value = min_t(s64, true_reg->smax_value, val - 1);
 		false_reg->smin_value = max_t(s64, false_reg->smin_value, val);
 		break;
+	case BPF_JLT:
+		true_reg->umin_value = max(true_reg->umin_value, val + 1);
+		false_reg->umax_value = min(false_reg->umax_value, val);
+		break;
+	case BPF_JSLT:
+		true_reg->smin_value = max_t(s64, true_reg->smin_value, val + 1);
+		false_reg->smax_value = min_t(s64, false_reg->smax_value, val);
+		break;
 	case BPF_JGE:
 		true_reg->umax_value = min(true_reg->umax_value, val);
 		false_reg->umin_value = max(false_reg->umin_value, val + 1);
@@ -2535,6 +2573,14 @@ static void reg_set_min_max_inv(struct bpf_reg_state *true_reg,
 		true_reg->smax_value = min_t(s64, true_reg->smax_value, val);
 		false_reg->smin_value = max_t(s64, false_reg->smin_value, val + 1);
 		break;
+	case BPF_JLE:
+		true_reg->umin_value = max(true_reg->umin_value, val);
+		false_reg->umax_value = min(false_reg->umax_value, val - 1);
+		break;
+	case BPF_JSLE:
+		true_reg->smin_value = max_t(s64, true_reg->smin_value, val);
+		false_reg->smax_value = min_t(s64, false_reg->smax_value, val - 1);
+		break;
 	default:
 		break;
 	}
@@ -2659,7 +2705,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 	u8 opcode = BPF_OP(insn->code);
 	int err;
 
-	if (opcode > BPF_EXIT) {
+	if (opcode > BPF_JSLE) {
 		verbose("invalid BPF_JMP opcode %x\n", opcode);
 		return -EINVAL;
 	}
@@ -2761,10 +2807,18 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 		   dst_reg->type == PTR_TO_PACKET &&
 		   regs[insn->src_reg].type == PTR_TO_PACKET_END) {
 		find_good_pkt_pointers(this_branch, dst_reg);
+	} else if (BPF_SRC(insn->code) == BPF_X && opcode == BPF_JLT &&
+		   dst_reg->type == PTR_TO_PACKET &&
+		   regs[insn->src_reg].type == PTR_TO_PACKET_END) {
+		find_good_pkt_pointers(other_branch, dst_reg);
 	} else if (BPF_SRC(insn->code) == BPF_X && opcode == BPF_JGE &&
 		   dst_reg->type == PTR_TO_PACKET_END &&
 		   regs[insn->src_reg].type == PTR_TO_PACKET) {
 		find_good_pkt_pointers(other_branch, &regs[insn->src_reg]);
+	} else if (BPF_SRC(insn->code) == BPF_X && opcode == BPF_JLE &&
+		   dst_reg->type == PTR_TO_PACKET_END &&
+		   regs[insn->src_reg].type == PTR_TO_PACKET) {
+		find_good_pkt_pointers(this_branch, &regs[insn->src_reg]);
 	} else if (is_pointer_value(env, insn->dst_reg)) {
 		verbose("R%d pointer comparison prohibited\n", insn->dst_reg);
 		return -EACCES;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 7/9] bpf, nfp: implement jiting of BPF_J{LT,LE}
From: Daniel Borkmann @ 2017-08-09 23:40 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE} instructions with
BPF_X/BPF_K variants for the nfp eBPF JIT. The two BPF_J{SLT,SLE}
instructions have not been added yet given BPF_J{SGT,SGE} are
not supported yet either.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/jit.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 8e57fda..239dfbe 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1238,6 +1238,16 @@ static int jge_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 	return wrp_cmp_imm(nfp_prog, meta, BR_BHS, true);
 }
 
+static int jlt_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+	return wrp_cmp_imm(nfp_prog, meta, BR_BHS, false);
+}
+
+static int jle_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+	return wrp_cmp_imm(nfp_prog, meta, BR_BLO, true);
+}
+
 static int jset_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 {
 	const struct bpf_insn *insn = &meta->insn;
@@ -1325,6 +1335,16 @@ static int jge_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 	return wrp_cmp_reg(nfp_prog, meta, BR_BHS, true);
 }
 
+static int jlt_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+	return wrp_cmp_reg(nfp_prog, meta, BR_BHS, false);
+}
+
+static int jle_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
+{
+	return wrp_cmp_reg(nfp_prog, meta, BR_BLO, true);
+}
+
 static int jset_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 {
 	return wrp_test_reg(nfp_prog, meta, ALU_OP_AND, BR_BNE);
@@ -1383,11 +1403,15 @@ static int goto_out(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
 	[BPF_JMP | BPF_JEQ | BPF_K] =	jeq_imm,
 	[BPF_JMP | BPF_JGT | BPF_K] =	jgt_imm,
 	[BPF_JMP | BPF_JGE | BPF_K] =	jge_imm,
+	[BPF_JMP | BPF_JLT | BPF_K] =	jlt_imm,
+	[BPF_JMP | BPF_JLE | BPF_K] =	jle_imm,
 	[BPF_JMP | BPF_JSET | BPF_K] =	jset_imm,
 	[BPF_JMP | BPF_JNE | BPF_K] =	jne_imm,
 	[BPF_JMP | BPF_JEQ | BPF_X] =	jeq_reg,
 	[BPF_JMP | BPF_JGT | BPF_X] =	jgt_reg,
 	[BPF_JMP | BPF_JGE | BPF_X] =	jge_reg,
+	[BPF_JMP | BPF_JLT | BPF_X] =	jlt_reg,
+	[BPF_JMP | BPF_JLE | BPF_X] =	jle_reg,
 	[BPF_JMP | BPF_JSET | BPF_X] =	jset_reg,
 	[BPF_JMP | BPF_JNE | BPF_X] =	jne_reg,
 	[BPF_JMP | BPF_EXIT] =		goto_out,
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 6/9] bpf, ppc64: implement jiting of BPF_J{LT,LE,SLT,SLE}
From: Daniel Borkmann @ 2017-08-09 23:40 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE,SLT,SLE} instructions
with BPF_X/BPF_K variants for the ppc64 eBPF JIT.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/net/bpf_jit.h        |  1 +
 arch/powerpc/net/bpf_jit_comp64.c | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 30cf03f..47fc666 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -263,6 +263,7 @@ static inline bool is_nearbranch(int offset)
 #define COND_EQ		(CR0_EQ | COND_CMP_TRUE)
 #define COND_NE		(CR0_EQ | COND_CMP_FALSE)
 #define COND_LT		(CR0_LT | COND_CMP_TRUE)
+#define COND_LE		(CR0_GT | COND_CMP_FALSE)
 
 #endif
 
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 861c5af..faf2016 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -795,12 +795,24 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
 		case BPF_JMP | BPF_JSGT | BPF_X:
 			true_cond = COND_GT;
 			goto cond_branch;
+		case BPF_JMP | BPF_JLT | BPF_K:
+		case BPF_JMP | BPF_JLT | BPF_X:
+		case BPF_JMP | BPF_JSLT | BPF_K:
+		case BPF_JMP | BPF_JSLT | BPF_X:
+			true_cond = COND_LT;
+			goto cond_branch;
 		case BPF_JMP | BPF_JGE | BPF_K:
 		case BPF_JMP | BPF_JGE | BPF_X:
 		case BPF_JMP | BPF_JSGE | BPF_K:
 		case BPF_JMP | BPF_JSGE | BPF_X:
 			true_cond = COND_GE;
 			goto cond_branch;
+		case BPF_JMP | BPF_JLE | BPF_K:
+		case BPF_JMP | BPF_JLE | BPF_X:
+		case BPF_JMP | BPF_JSLE | BPF_K:
+		case BPF_JMP | BPF_JSLE | BPF_X:
+			true_cond = COND_LE;
+			goto cond_branch;
 		case BPF_JMP | BPF_JEQ | BPF_K:
 		case BPF_JMP | BPF_JEQ | BPF_X:
 			true_cond = COND_EQ;
@@ -817,14 +829,18 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
 cond_branch:
 			switch (code) {
 			case BPF_JMP | BPF_JGT | BPF_X:
+			case BPF_JMP | BPF_JLT | BPF_X:
 			case BPF_JMP | BPF_JGE | BPF_X:
+			case BPF_JMP | BPF_JLE | BPF_X:
 			case BPF_JMP | BPF_JEQ | BPF_X:
 			case BPF_JMP | BPF_JNE | BPF_X:
 				/* unsigned comparison */
 				PPC_CMPLD(dst_reg, src_reg);
 				break;
 			case BPF_JMP | BPF_JSGT | BPF_X:
+			case BPF_JMP | BPF_JSLT | BPF_X:
 			case BPF_JMP | BPF_JSGE | BPF_X:
+			case BPF_JMP | BPF_JSLE | BPF_X:
 				/* signed comparison */
 				PPC_CMPD(dst_reg, src_reg);
 				break;
@@ -834,7 +850,9 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
 			case BPF_JMP | BPF_JNE | BPF_K:
 			case BPF_JMP | BPF_JEQ | BPF_K:
 			case BPF_JMP | BPF_JGT | BPF_K:
+			case BPF_JMP | BPF_JLT | BPF_K:
 			case BPF_JMP | BPF_JGE | BPF_K:
+			case BPF_JMP | BPF_JLE | BPF_K:
 				/*
 				 * Need sign-extended load, so only positive
 				 * values can be used as imm in cmpldi
@@ -849,7 +867,9 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
 				}
 				break;
 			case BPF_JMP | BPF_JSGT | BPF_K:
+			case BPF_JMP | BPF_JSLT | BPF_K:
 			case BPF_JMP | BPF_JSGE | BPF_K:
+			case BPF_JMP | BPF_JSLE | BPF_K:
 				/*
 				 * signed comparison, so any 16-bit value
 				 * can be used in cmpdi
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 4/9] bpf, sparc64: implement jiting of BPF_J{LT, LE, SLT, SLE}
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE,SLT,SLE} instructions
with BPF_X/BPF_K variants for the sparc64 eBPF JIT.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 arch/sparc/net/bpf_jit_comp_64.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/sparc/net/bpf_jit_comp_64.c b/arch/sparc/net/bpf_jit_comp_64.c
index 8799ae9..c340af7 100644
--- a/arch/sparc/net/bpf_jit_comp_64.c
+++ b/arch/sparc/net/bpf_jit_comp_64.c
@@ -128,6 +128,8 @@ static u32 WDISP10(u32 off)
 
 #define BA		(BRANCH | CONDA)
 #define BG		(BRANCH | CONDG)
+#define BL		(BRANCH | CONDL)
+#define BLE		(BRANCH | CONDLE)
 #define BGU		(BRANCH | CONDGU)
 #define BLEU		(BRANCH | CONDLEU)
 #define BGE		(BRANCH | CONDGE)
@@ -715,9 +717,15 @@ static int emit_compare_and_branch(const u8 code, const u8 dst, u8 src,
 		case BPF_JGT:
 			br_opcode = BGU;
 			break;
+		case BPF_JLT:
+			br_opcode = BLU;
+			break;
 		case BPF_JGE:
 			br_opcode = BGEU;
 			break;
+		case BPF_JLE:
+			br_opcode = BLEU;
+			break;
 		case BPF_JSET:
 		case BPF_JNE:
 			br_opcode = BNE;
@@ -725,9 +733,15 @@ static int emit_compare_and_branch(const u8 code, const u8 dst, u8 src,
 		case BPF_JSGT:
 			br_opcode = BG;
 			break;
+		case BPF_JSLT:
+			br_opcode = BL;
+			break;
 		case BPF_JSGE:
 			br_opcode = BGE;
 			break;
+		case BPF_JSLE:
+			br_opcode = BLE;
+			break;
 		default:
 			/* Make sure we dont leak kernel information to the
 			 * user.
@@ -746,18 +760,30 @@ static int emit_compare_and_branch(const u8 code, const u8 dst, u8 src,
 		case BPF_JGT:
 			cbcond_opcode = CBCONDGU;
 			break;
+		case BPF_JLT:
+			cbcond_opcode = CBCONDLU;
+			break;
 		case BPF_JGE:
 			cbcond_opcode = CBCONDGEU;
 			break;
+		case BPF_JLE:
+			cbcond_opcode = CBCONDLEU;
+			break;
 		case BPF_JNE:
 			cbcond_opcode = CBCONDNE;
 			break;
 		case BPF_JSGT:
 			cbcond_opcode = CBCONDG;
 			break;
+		case BPF_JSLT:
+			cbcond_opcode = CBCONDL;
+			break;
 		case BPF_JSGE:
 			cbcond_opcode = CBCONDGE;
 			break;
+		case BPF_JSLE:
+			cbcond_opcode = CBCONDLE;
+			break;
 		default:
 			/* Make sure we dont leak kernel information to the
 			 * user.
@@ -1176,10 +1202,14 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	/* IF (dst COND src) JUMP off */
 	case BPF_JMP | BPF_JEQ | BPF_X:
 	case BPF_JMP | BPF_JGT | BPF_X:
+	case BPF_JMP | BPF_JLT | BPF_X:
 	case BPF_JMP | BPF_JGE | BPF_X:
+	case BPF_JMP | BPF_JLE | BPF_X:
 	case BPF_JMP | BPF_JNE | BPF_X:
 	case BPF_JMP | BPF_JSGT | BPF_X:
+	case BPF_JMP | BPF_JSLT | BPF_X:
 	case BPF_JMP | BPF_JSGE | BPF_X:
+	case BPF_JMP | BPF_JSLE | BPF_X:
 	case BPF_JMP | BPF_JSET | BPF_X: {
 		int err;
 
@@ -1191,10 +1221,14 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	/* IF (dst COND imm) JUMP off */
 	case BPF_JMP | BPF_JEQ | BPF_K:
 	case BPF_JMP | BPF_JGT | BPF_K:
+	case BPF_JMP | BPF_JLT | BPF_K:
 	case BPF_JMP | BPF_JGE | BPF_K:
+	case BPF_JMP | BPF_JLE | BPF_K:
 	case BPF_JMP | BPF_JNE | BPF_K:
 	case BPF_JMP | BPF_JSGT | BPF_K:
+	case BPF_JMP | BPF_JSLT | BPF_K:
 	case BPF_JMP | BPF_JSGE | BPF_K:
+	case BPF_JMP | BPF_JSLE | BPF_K:
 	case BPF_JMP | BPF_JSET | BPF_K: {
 		int err;
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 5/9] bpf, s390x: implement jiting of BPF_J{LT,LE,SLT,SLE}
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE,SLT,SLE} instructions
with BPF_X/BPF_K variants for the s390x eBPF JIT.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 arch/s390/net/bpf_jit_comp.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01c6fbc..b5228ca 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1093,15 +1093,27 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 	case BPF_JMP | BPF_JSGT | BPF_K: /* ((s64) dst > (s64) imm) */
 		mask = 0x2000; /* jh */
 		goto branch_ks;
+	case BPF_JMP | BPF_JSLT | BPF_K: /* ((s64) dst < (s64) imm) */
+		mask = 0x4000; /* jl */
+		goto branch_ks;
 	case BPF_JMP | BPF_JSGE | BPF_K: /* ((s64) dst >= (s64) imm) */
 		mask = 0xa000; /* jhe */
 		goto branch_ks;
+	case BPF_JMP | BPF_JSLE | BPF_K: /* ((s64) dst <= (s64) imm) */
+		mask = 0xc000; /* jle */
+		goto branch_ks;
 	case BPF_JMP | BPF_JGT | BPF_K: /* (dst_reg > imm) */
 		mask = 0x2000; /* jh */
 		goto branch_ku;
+	case BPF_JMP | BPF_JLT | BPF_K: /* (dst_reg < imm) */
+		mask = 0x4000; /* jl */
+		goto branch_ku;
 	case BPF_JMP | BPF_JGE | BPF_K: /* (dst_reg >= imm) */
 		mask = 0xa000; /* jhe */
 		goto branch_ku;
+	case BPF_JMP | BPF_JLE | BPF_K: /* (dst_reg <= imm) */
+		mask = 0xc000; /* jle */
+		goto branch_ku;
 	case BPF_JMP | BPF_JNE | BPF_K: /* (dst_reg != imm) */
 		mask = 0x7000; /* jne */
 		goto branch_ku;
@@ -1119,15 +1131,27 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp, int i
 	case BPF_JMP | BPF_JSGT | BPF_X: /* ((s64) dst > (s64) src) */
 		mask = 0x2000; /* jh */
 		goto branch_xs;
+	case BPF_JMP | BPF_JSLT | BPF_X: /* ((s64) dst < (s64) src) */
+		mask = 0x4000; /* jl */
+		goto branch_xs;
 	case BPF_JMP | BPF_JSGE | BPF_X: /* ((s64) dst >= (s64) src) */
 		mask = 0xa000; /* jhe */
 		goto branch_xs;
+	case BPF_JMP | BPF_JSLE | BPF_X: /* ((s64) dst <= (s64) src) */
+		mask = 0xc000; /* jle */
+		goto branch_xs;
 	case BPF_JMP | BPF_JGT | BPF_X: /* (dst > src) */
 		mask = 0x2000; /* jh */
 		goto branch_xu;
+	case BPF_JMP | BPF_JLT | BPF_X: /* (dst < src) */
+		mask = 0x4000; /* jl */
+		goto branch_xu;
 	case BPF_JMP | BPF_JGE | BPF_X: /* (dst >= src) */
 		mask = 0xa000; /* jhe */
 		goto branch_xu;
+	case BPF_JMP | BPF_JLE | BPF_X: /* (dst <= src) */
+		mask = 0xc000; /* jle */
+		goto branch_xu;
 	case BPF_JMP | BPF_JNE | BPF_X: /* (dst != src) */
 		mask = 0x7000; /* jne */
 		goto branch_xu;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 3/9] bpf, arm64: implement jiting of BPF_J{LT,LE,SLT,SLE}
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE,SLT,SLE} instructions
with BPF_X/BPF_K variants for the arm64 eBPF JIT.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 arch/arm64/net/bpf_jit.h      |  4 ++++
 arch/arm64/net/bpf_jit_comp.c | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/arch/arm64/net/bpf_jit.h b/arch/arm64/net/bpf_jit.h
index b02a926..783de51 100644
--- a/arch/arm64/net/bpf_jit.h
+++ b/arch/arm64/net/bpf_jit.h
@@ -44,8 +44,12 @@
 #define A64_COND_NE	AARCH64_INSN_COND_NE /* != */
 #define A64_COND_CS	AARCH64_INSN_COND_CS /* unsigned >= */
 #define A64_COND_HI	AARCH64_INSN_COND_HI /* unsigned > */
+#define A64_COND_LS	AARCH64_INSN_COND_LS /* unsigned <= */
+#define A64_COND_CC	AARCH64_INSN_COND_CC /* unsigned < */
 #define A64_COND_GE	AARCH64_INSN_COND_GE /* signed >= */
 #define A64_COND_GT	AARCH64_INSN_COND_GT /* signed > */
+#define A64_COND_LE	AARCH64_INSN_COND_LE /* signed <= */
+#define A64_COND_LT	AARCH64_INSN_COND_LT /* signed < */
 #define A64_B_(cond, imm19) A64_COND_BRANCH(cond, (imm19) << 2)
 
 /* Unconditional branch (immediate) */
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index f32144b..ba38d40 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -527,10 +527,14 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	/* IF (dst COND src) JUMP off */
 	case BPF_JMP | BPF_JEQ | BPF_X:
 	case BPF_JMP | BPF_JGT | BPF_X:
+	case BPF_JMP | BPF_JLT | BPF_X:
 	case BPF_JMP | BPF_JGE | BPF_X:
+	case BPF_JMP | BPF_JLE | BPF_X:
 	case BPF_JMP | BPF_JNE | BPF_X:
 	case BPF_JMP | BPF_JSGT | BPF_X:
+	case BPF_JMP | BPF_JSLT | BPF_X:
 	case BPF_JMP | BPF_JSGE | BPF_X:
+	case BPF_JMP | BPF_JSLE | BPF_X:
 		emit(A64_CMP(1, dst, src), ctx);
 emit_cond_jmp:
 		jmp_offset = bpf2a64_offset(i + off, i, ctx);
@@ -542,9 +546,15 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 		case BPF_JGT:
 			jmp_cond = A64_COND_HI;
 			break;
+		case BPF_JLT:
+			jmp_cond = A64_COND_CC;
+			break;
 		case BPF_JGE:
 			jmp_cond = A64_COND_CS;
 			break;
+		case BPF_JLE:
+			jmp_cond = A64_COND_LS;
+			break;
 		case BPF_JSET:
 		case BPF_JNE:
 			jmp_cond = A64_COND_NE;
@@ -552,9 +562,15 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 		case BPF_JSGT:
 			jmp_cond = A64_COND_GT;
 			break;
+		case BPF_JSLT:
+			jmp_cond = A64_COND_LT;
+			break;
 		case BPF_JSGE:
 			jmp_cond = A64_COND_GE;
 			break;
+		case BPF_JSLE:
+			jmp_cond = A64_COND_LE;
+			break;
 		default:
 			return -EFAULT;
 		}
@@ -566,10 +582,14 @@ static int build_insn(const struct bpf_insn *insn, struct jit_ctx *ctx)
 	/* IF (dst COND imm) JUMP off */
 	case BPF_JMP | BPF_JEQ | BPF_K:
 	case BPF_JMP | BPF_JGT | BPF_K:
+	case BPF_JMP | BPF_JLT | BPF_K:
 	case BPF_JMP | BPF_JGE | BPF_K:
+	case BPF_JMP | BPF_JLE | BPF_K:
 	case BPF_JMP | BPF_JNE | BPF_K:
 	case BPF_JMP | BPF_JSGT | BPF_K:
+	case BPF_JMP | BPF_JSLT | BPF_K:
 	case BPF_JMP | BPF_JSGE | BPF_K:
+	case BPF_JMP | BPF_JSLE | BPF_K:
 		emit_a64_mov_i(1, tmp, imm, ctx);
 		emit(A64_CMP(1, dst, tmp), ctx);
 		goto emit_cond_jmp;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 1/9] bpf: add BPF_J{LT,LE,SLT,SLE} instructions
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

Currently, eBPF only understands BPF_JGT (>), BPF_JGE (>=),
BPF_JSGT (s>), BPF_JSGE (s>=) instructions, this means that
particularly *JLT/*JLE counterparts involving immediates need
to be rewritten from e.g. X < [IMM] by swapping arguments into
[IMM] > X, meaning the immediate first is required to be loaded
into a register Y := [IMM], such that then we can compare with
Y > X. Note that the destination operand is always required to
be a register.

This has the downside of having unnecessarily increased register
pressure, meaning complex program would need to spill other
registers temporarily to stack in order to obtain an unused
register for the [IMM]. Loading to registers will thus also
affect state pruning since we need to account for that register
use and potentially those registers that had to be spilled/filled
again. As a consequence slightly more stack space might have
been used due to spilling, and BPF programs are a bit longer
due to extra code involving the register load and potentially
required spill/fills.

Thus, add BPF_JLT (<), BPF_JLE (<=), BPF_JSLT (s<), BPF_JSLE (s<=)
counterparts to the eBPF instruction set. Modifying LLVM to
remove the NegateCC() workaround in a PoC patch at [1] and
allowing it to also emit the new instructions resulted in
cilium's BPF programs that are injected into the fast-path to
have a reduced program length in the range of 2-3% (e.g.
accumulated main and tail call sections from one of the object
file reduced from 4864 to 4729 insns), reduced complexity in
the range of 10-30% (e.g. accumulated sections reduced in one
of the cases from 116432 to 88428 insns), and reduced stack
usage in the range of 1-5% (e.g. accumulated sections from one
of the object files reduced from 824 to 784b).

The modification for LLVM will be incorporated in a backwards
compatible way. Plan is for LLVM to have i) a target specific
option to offer a possibility to explicitly enable the extension
by the user (as we have with -m target specific extensions today
for various CPU insns), and ii) have the kernel checked for
presence of the extensions and enable them transparently when
the user is selecting more aggressive options such as -march=native
in a bpf target context. (Other frontends generating BPF byte
code, e.g. ply can probe the kernel directly for its code
generation.)

  [1] https://github.com/borkmann/llvm/tree/bpf-insns

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 Documentation/networking/filter.txt |   4 +
 include/uapi/linux/bpf.h            |   5 +
 kernel/bpf/core.c                   |  60 ++++++
 lib/test_bpf.c                      | 364 ++++++++++++++++++++++++++++++++++++
 net/core/filter.c                   |  21 ++-
 tools/include/uapi/linux/bpf.h      |   5 +
 6 files changed, 455 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index d0fdba7..6a0df8d 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -906,6 +906,10 @@ If BPF_CLASS(code) == BPF_JMP, BPF_OP(code) is one of:
   BPF_JSGE  0x70  /* eBPF only: signed '>=' */
   BPF_CALL  0x80  /* eBPF only: function call */
   BPF_EXIT  0x90  /* eBPF only: function return */
+  BPF_JLT   0xa0  /* eBPF only: unsigned '<' */
+  BPF_JLE   0xb0  /* eBPF only: unsigned '<=' */
+  BPF_JSLT  0xc0  /* eBPF only: signed '<' */
+  BPF_JSLE  0xd0  /* eBPF only: signed '<=' */
 
 So BPF_ADD | BPF_X | BPF_ALU means 32-bit addition in both classic BPF
 and eBPF. There are only two registers in classic BPF, so it means A += X.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1d06be1..91da837 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -30,9 +30,14 @@
 #define BPF_FROM_LE	BPF_TO_LE
 #define BPF_FROM_BE	BPF_TO_BE
 
+/* jmp encodings */
 #define BPF_JNE		0x50	/* jump != */
+#define BPF_JLT		0xa0	/* LT is unsigned, '<' */
+#define BPF_JLE		0xb0	/* LE is unsigned, '<=' */
 #define BPF_JSGT	0x60	/* SGT is signed '>', GT in x86 */
 #define BPF_JSGE	0x70	/* SGE is signed '>=', GE in x86 */
+#define BPF_JSLT	0xc0	/* SLT is signed, '<' */
+#define BPF_JSLE	0xd0	/* SLE is signed, '<=' */
 #define BPF_CALL	0x80	/* function call */
 #define BPF_EXIT	0x90	/* function return */
 
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ad5f559..c69e7f5 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -595,9 +595,13 @@ static int bpf_jit_blind_insn(const struct bpf_insn *from,
 	case BPF_JMP | BPF_JEQ  | BPF_K:
 	case BPF_JMP | BPF_JNE  | BPF_K:
 	case BPF_JMP | BPF_JGT  | BPF_K:
+	case BPF_JMP | BPF_JLT  | BPF_K:
 	case BPF_JMP | BPF_JGE  | BPF_K:
+	case BPF_JMP | BPF_JLE  | BPF_K:
 	case BPF_JMP | BPF_JSGT | BPF_K:
+	case BPF_JMP | BPF_JSLT | BPF_K:
 	case BPF_JMP | BPF_JSGE | BPF_K:
+	case BPF_JMP | BPF_JSLE | BPF_K:
 	case BPF_JMP | BPF_JSET | BPF_K:
 		/* Accommodate for extra offset in case of a backjump. */
 		off = from->off;
@@ -833,12 +837,20 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn,
 		[BPF_JMP | BPF_JNE | BPF_K] = &&JMP_JNE_K,
 		[BPF_JMP | BPF_JGT | BPF_X] = &&JMP_JGT_X,
 		[BPF_JMP | BPF_JGT | BPF_K] = &&JMP_JGT_K,
+		[BPF_JMP | BPF_JLT | BPF_X] = &&JMP_JLT_X,
+		[BPF_JMP | BPF_JLT | BPF_K] = &&JMP_JLT_K,
 		[BPF_JMP | BPF_JGE | BPF_X] = &&JMP_JGE_X,
 		[BPF_JMP | BPF_JGE | BPF_K] = &&JMP_JGE_K,
+		[BPF_JMP | BPF_JLE | BPF_X] = &&JMP_JLE_X,
+		[BPF_JMP | BPF_JLE | BPF_K] = &&JMP_JLE_K,
 		[BPF_JMP | BPF_JSGT | BPF_X] = &&JMP_JSGT_X,
 		[BPF_JMP | BPF_JSGT | BPF_K] = &&JMP_JSGT_K,
+		[BPF_JMP | BPF_JSLT | BPF_X] = &&JMP_JSLT_X,
+		[BPF_JMP | BPF_JSLT | BPF_K] = &&JMP_JSLT_K,
 		[BPF_JMP | BPF_JSGE | BPF_X] = &&JMP_JSGE_X,
 		[BPF_JMP | BPF_JSGE | BPF_K] = &&JMP_JSGE_K,
+		[BPF_JMP | BPF_JSLE | BPF_X] = &&JMP_JSLE_X,
+		[BPF_JMP | BPF_JSLE | BPF_K] = &&JMP_JSLE_K,
 		[BPF_JMP | BPF_JSET | BPF_X] = &&JMP_JSET_X,
 		[BPF_JMP | BPF_JSET | BPF_K] = &&JMP_JSET_K,
 		/* Program return */
@@ -1073,6 +1085,18 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn,
 			CONT_JMP;
 		}
 		CONT;
+	JMP_JLT_X:
+		if (DST < SRC) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
+	JMP_JLT_K:
+		if (DST < IMM) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
 	JMP_JGE_X:
 		if (DST >= SRC) {
 			insn += insn->off;
@@ -1085,6 +1109,18 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn,
 			CONT_JMP;
 		}
 		CONT;
+	JMP_JLE_X:
+		if (DST <= SRC) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
+	JMP_JLE_K:
+		if (DST <= IMM) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
 	JMP_JSGT_X:
 		if (((s64) DST) > ((s64) SRC)) {
 			insn += insn->off;
@@ -1097,6 +1133,18 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn,
 			CONT_JMP;
 		}
 		CONT;
+	JMP_JSLT_X:
+		if (((s64) DST) < ((s64) SRC)) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
+	JMP_JSLT_K:
+		if (((s64) DST) < ((s64) IMM)) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
 	JMP_JSGE_X:
 		if (((s64) DST) >= ((s64) SRC)) {
 			insn += insn->off;
@@ -1109,6 +1157,18 @@ static unsigned int ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn,
 			CONT_JMP;
 		}
 		CONT;
+	JMP_JSLE_X:
+		if (((s64) DST) <= ((s64) SRC)) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
+	JMP_JSLE_K:
+		if (((s64) DST) <= ((s64) IMM)) {
+			insn += insn->off;
+			CONT_JMP;
+		}
+		CONT;
 	JMP_JSET_X:
 		if (DST & SRC) {
 			insn += insn->off;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index d9d5a41..aa8812a 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -952,6 +952,32 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ { 2, 0 }, { 3, 1 }, { 4, MAX_K } },
 	},
 	{
+		"JGE (jt 0), test 1",
+		.u.insns = {
+			BPF_STMT(BPF_LDX | BPF_LEN, 0),
+			BPF_STMT(BPF_LD | BPF_B | BPF_ABS, 2),
+			BPF_JUMP(BPF_JMP | BPF_JGE | BPF_X, 0, 0, 1),
+			BPF_STMT(BPF_RET | BPF_K, 1),
+			BPF_STMT(BPF_RET | BPF_K, MAX_K)
+		},
+		CLASSIC,
+		{ 4, 4, 4, 3, 3 },
+		{ { 2, 0 }, { 3, 1 }, { 4, 1 } },
+	},
+	{
+		"JGE (jt 0), test 2",
+		.u.insns = {
+			BPF_STMT(BPF_LDX | BPF_LEN, 0),
+			BPF_STMT(BPF_LD | BPF_B | BPF_ABS, 2),
+			BPF_JUMP(BPF_JMP | BPF_JGE | BPF_X, 0, 0, 1),
+			BPF_STMT(BPF_RET | BPF_K, 1),
+			BPF_STMT(BPF_RET | BPF_K, MAX_K)
+		},
+		CLASSIC,
+		{ 4, 4, 5, 3, 3 },
+		{ { 4, 1 }, { 5, 1 }, { 6, MAX_K } },
+	},
+	{
 		"JGE",
 		.u.insns = {
 			BPF_STMT(BPF_LDX | BPF_LEN, 0),
@@ -4492,6 +4518,35 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JSLT | BPF_K */
+	{
+		"JMP_JSLT_K: Signed jump: if (-2 < -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 0xfffffffffffffffeLL),
+			BPF_JMP_IMM(BPF_JSLT, R1, -1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLT_K: Signed jump: if (-1 < -1) return 0",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_LD_IMM64(R1, 0xffffffffffffffffLL),
+			BPF_JMP_IMM(BPF_JSLT, R1, -1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JSGT | BPF_K */
 	{
 		"JMP_JSGT_K: Signed jump: if (-1 > -2) return 1",
@@ -4521,6 +4576,73 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JSLE | BPF_K */
+	{
+		"JMP_JSLE_K: Signed jump: if (-2 <= -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 0xfffffffffffffffeLL),
+			BPF_JMP_IMM(BPF_JSLE, R1, -1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLE_K: Signed jump: if (-1 <= -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 0xffffffffffffffffLL),
+			BPF_JMP_IMM(BPF_JSLE, R1, -1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLE_K: Signed jump: value walk 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 6),
+			BPF_ALU64_IMM(BPF_SUB, R1, 1),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 4),
+			BPF_ALU64_IMM(BPF_SUB, R1, 1),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 2),
+			BPF_ALU64_IMM(BPF_SUB, R1, 1),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 1),
+			BPF_EXIT_INSN(),		/* bad exit */
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),	/* good exit */
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLE_K: Signed jump: value walk 2",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 4),
+			BPF_ALU64_IMM(BPF_SUB, R1, 2),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 2),
+			BPF_ALU64_IMM(BPF_SUB, R1, 2),
+			BPF_JMP_IMM(BPF_JSLE, R1, 0, 1),
+			BPF_EXIT_INSN(),		/* bad exit */
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),	/* good exit */
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JSGE | BPF_K */
 	{
 		"JMP_JSGE_K: Signed jump: if (-1 >= -2) return 1",
@@ -4617,6 +4739,35 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JLT | BPF_K */
+	{
+		"JMP_JLT_K: if (2 < 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 2),
+			BPF_JMP_IMM(BPF_JLT, R1, 3, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JGT_K: Unsigned jump: if (1 < -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 1),
+			BPF_JMP_IMM(BPF_JLT, R1, -1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JGE | BPF_K */
 	{
 		"JMP_JGE_K: if (3 >= 2) return 1",
@@ -4632,6 +4783,21 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JLE | BPF_K */
+	{
+		"JMP_JLE_K: if (2 <= 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 2),
+			BPF_JMP_IMM(BPF_JLE, R1, 3, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JGT | BPF_K jump backwards */
 	{
 		"JMP_JGT_K: if (3 > 2) return 1 (jump backwards)",
@@ -4662,6 +4828,36 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JLT | BPF_K jump backwards */
+	{
+		"JMP_JGT_K: if (2 < 3) return 1 (jump backwards)",
+		.u.insns_int = {
+			BPF_JMP_IMM(BPF_JA, 0, 0, 2), /* goto start */
+			BPF_ALU32_IMM(BPF_MOV, R0, 1), /* out: */
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 0), /* start: */
+			BPF_LD_IMM64(R1, 2), /* note: this takes 2 insns */
+			BPF_JMP_IMM(BPF_JLT, R1, 3, -6), /* goto out */
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JLE_K: if (3 <= 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_JMP_IMM(BPF_JLE, R1, 3, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JNE | BPF_K */
 	{
 		"JMP_JNE_K: if (3 != 2) return 1",
@@ -4752,6 +4948,37 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JSLT | BPF_X */
+	{
+		"JMP_JSLT_X: Signed jump: if (-2 < -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, -1),
+			BPF_LD_IMM64(R2, -2),
+			BPF_JMP_REG(BPF_JSLT, R2, R1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLT_X: Signed jump: if (-1 < -1) return 0",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_LD_IMM64(R1, -1),
+			BPF_LD_IMM64(R2, -1),
+			BPF_JMP_REG(BPF_JSLT, R1, R2, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JSGE | BPF_X */
 	{
 		"JMP_JSGE_X: Signed jump: if (-1 >= -2) return 1",
@@ -4783,6 +5010,37 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JSLE | BPF_X */
+	{
+		"JMP_JSLE_X: Signed jump: if (-2 <= -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, -1),
+			BPF_LD_IMM64(R2, -2),
+			BPF_JMP_REG(BPF_JSLE, R2, R1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JSLE_X: Signed jump: if (-1 <= -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, -1),
+			BPF_LD_IMM64(R2, -1),
+			BPF_JMP_REG(BPF_JSLE, R1, R2, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JGT | BPF_X */
 	{
 		"JMP_JGT_X: if (3 > 2) return 1",
@@ -4814,6 +5072,37 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JLT | BPF_X */
+	{
+		"JMP_JLT_X: if (2 < 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 2),
+			BPF_JMP_REG(BPF_JLT, R2, R1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JLT_X: Unsigned jump: if (1 < -1) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, -1),
+			BPF_LD_IMM64(R2, 1),
+			BPF_JMP_REG(BPF_JLT, R2, R1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JGE | BPF_X */
 	{
 		"JMP_JGE_X: if (3 >= 2) return 1",
@@ -4845,6 +5134,37 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	/* BPF_JMP | BPF_JLE | BPF_X */
+	{
+		"JMP_JLE_X: if (2 <= 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 2),
+			BPF_JMP_REG(BPF_JLE, R2, R1, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
+	{
+		"JMP_JLE_X: if (3 <= 3) return 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 3),
+			BPF_JMP_REG(BPF_JLE, R1, R2, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	{
 		/* Mainly testing JIT + imm64 here. */
 		"JMP_JGE_X: ldimm64 test 1",
@@ -4890,6 +5210,50 @@ static int bpf_fill_stxdw(struct bpf_test *self)
 		{ },
 		{ { 0, 1 } },
 	},
+	{
+		"JMP_JLE_X: ldimm64 test 1",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 2),
+			BPF_JMP_REG(BPF_JLE, R2, R1, 2),
+			BPF_LD_IMM64(R0, 0xffffffffffffffffULL),
+			BPF_LD_IMM64(R0, 0xeeeeeeeeeeeeeeeeULL),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0xeeeeeeeeU } },
+	},
+	{
+		"JMP_JLE_X: ldimm64 test 2",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 0),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 2),
+			BPF_JMP_REG(BPF_JLE, R2, R1, 0),
+			BPF_LD_IMM64(R0, 0xffffffffffffffffULL),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 0xffffffffU } },
+	},
+	{
+		"JMP_JLE_X: ldimm64 test 3",
+		.u.insns_int = {
+			BPF_ALU32_IMM(BPF_MOV, R0, 1),
+			BPF_LD_IMM64(R1, 3),
+			BPF_LD_IMM64(R2, 2),
+			BPF_JMP_REG(BPF_JLE, R2, R1, 4),
+			BPF_LD_IMM64(R0, 0xffffffffffffffffULL),
+			BPF_LD_IMM64(R0, 0xeeeeeeeeeeeeeeeeULL),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } },
+	},
 	/* BPF_JMP | BPF_JNE | BPF_X */
 	{
 		"JMP_JNE_X: if (3 != 2) return 1",
diff --git a/net/core/filter.c b/net/core/filter.c
index 78d0093..5afe3ac 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -514,14 +514,27 @@ static int bpf_convert_filter(struct sock_filter *prog, int len,
 				break;
 			}
 
-			/* Convert JEQ into JNE when 'jump_true' is next insn. */
-			if (fp->jt == 0 && BPF_OP(fp->code) == BPF_JEQ) {
-				insn->code = BPF_JMP | BPF_JNE | bpf_src;
+			/* Convert some jumps when 'jump_true' is next insn. */
+			if (fp->jt == 0) {
+				switch (BPF_OP(fp->code)) {
+				case BPF_JEQ:
+					insn->code = BPF_JMP | BPF_JNE | bpf_src;
+					break;
+				case BPF_JGT:
+					insn->code = BPF_JMP | BPF_JLE | bpf_src;
+					break;
+				case BPF_JGE:
+					insn->code = BPF_JMP | BPF_JLT | bpf_src;
+					break;
+				default:
+					goto jmp_rest;
+				}
+
 				target = i + fp->jf + 1;
 				BPF_EMIT_JMP;
 				break;
 			}
-
+jmp_rest:
 			/* Other jumps are mapped into two insns: Jxx and JA. */
 			target = i + fp->jt + 1;
 			insn->code = BPF_JMP | BPF_OP(fp->code) | bpf_src;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 8d9bfcc..bf3b2e2 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -30,9 +30,14 @@
 #define BPF_FROM_LE	BPF_TO_LE
 #define BPF_FROM_BE	BPF_TO_BE
 
+/* jmp encodings */
 #define BPF_JNE		0x50	/* jump != */
+#define BPF_JLT		0xa0	/* LT is unsigned, '<' */
+#define BPF_JLE		0xb0	/* LE is unsigned, '<=' */
 #define BPF_JSGT	0x60	/* SGT is signed '>', GT in x86 */
 #define BPF_JSGE	0x70	/* SGE is signed '>=', GE in x86 */
+#define BPF_JSLT	0xc0	/* SLT is signed, '<' */
+#define BPF_JSLE	0xd0	/* SLE is signed, '<=' */
 #define BPF_CALL	0x80	/* function call */
 #define BPF_EXIT	0x90	/* function return */
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 2/9] bpf, x86: implement jiting of BPF_J{LT,LE,SLT,SLE}
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann
In-Reply-To: <cover.1502320623.git.daniel@iogearbox.net>

This work implements jiting of BPF_J{LT,LE,SLT,SLE} instructions
with BPF_X/BPF_K variants for the x86_64 eBPF JIT.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index e1324f2..8194696 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -94,7 +94,9 @@ static int bpf_size_to_x86_bytes(int bpf_size)
 #define X86_JNE 0x75
 #define X86_JBE 0x76
 #define X86_JA  0x77
+#define X86_JL  0x7C
 #define X86_JGE 0x7D
+#define X86_JLE 0x7E
 #define X86_JG  0x7F
 
 static void bpf_flush_icache(void *start, void *end)
@@ -888,9 +890,13 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 		case BPF_JMP | BPF_JEQ | BPF_X:
 		case BPF_JMP | BPF_JNE | BPF_X:
 		case BPF_JMP | BPF_JGT | BPF_X:
+		case BPF_JMP | BPF_JLT | BPF_X:
 		case BPF_JMP | BPF_JGE | BPF_X:
+		case BPF_JMP | BPF_JLE | BPF_X:
 		case BPF_JMP | BPF_JSGT | BPF_X:
+		case BPF_JMP | BPF_JSLT | BPF_X:
 		case BPF_JMP | BPF_JSGE | BPF_X:
+		case BPF_JMP | BPF_JSLE | BPF_X:
 			/* cmp dst_reg, src_reg */
 			EMIT3(add_2mod(0x48, dst_reg, src_reg), 0x39,
 			      add_2reg(0xC0, dst_reg, src_reg));
@@ -911,9 +917,13 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 		case BPF_JMP | BPF_JEQ | BPF_K:
 		case BPF_JMP | BPF_JNE | BPF_K:
 		case BPF_JMP | BPF_JGT | BPF_K:
+		case BPF_JMP | BPF_JLT | BPF_K:
 		case BPF_JMP | BPF_JGE | BPF_K:
+		case BPF_JMP | BPF_JLE | BPF_K:
 		case BPF_JMP | BPF_JSGT | BPF_K:
+		case BPF_JMP | BPF_JSLT | BPF_K:
 		case BPF_JMP | BPF_JSGE | BPF_K:
+		case BPF_JMP | BPF_JSLE | BPF_K:
 			/* cmp dst_reg, imm8/32 */
 			EMIT1(add_1mod(0x48, dst_reg));
 
@@ -935,18 +945,34 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 				/* GT is unsigned '>', JA in x86 */
 				jmp_cond = X86_JA;
 				break;
+			case BPF_JLT:
+				/* LT is unsigned '<', JB in x86 */
+				jmp_cond = X86_JB;
+				break;
 			case BPF_JGE:
 				/* GE is unsigned '>=', JAE in x86 */
 				jmp_cond = X86_JAE;
 				break;
+			case BPF_JLE:
+				/* LE is unsigned '<=', JBE in x86 */
+				jmp_cond = X86_JBE;
+				break;
 			case BPF_JSGT:
 				/* signed '>', GT in x86 */
 				jmp_cond = X86_JG;
 				break;
+			case BPF_JSLT:
+				/* signed '<', LT in x86 */
+				jmp_cond = X86_JL;
+				break;
 			case BPF_JSGE:
 				/* signed '>=', GE in x86 */
 				jmp_cond = X86_JGE;
 				break;
+			case BPF_JSLE:
+				/* signed '<=', LE in x86 */
+				jmp_cond = X86_JLE;
+				break;
 			default: /* to silence gcc warning */
 				return -EFAULT;
 			}
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2 0/9] Add BPF_J{LT,LE,SLT,SLE} instructions
From: Daniel Borkmann @ 2017-08-09 23:39 UTC (permalink / raw)
  To: davem; +Cc: ast, holzheu, naveen.n.rao, jakub.kicinski, netdev,
	Daniel Borkmann

This set adds BPF_J{LT,LE,SLT,SLE} instructions to the BPF
insn set, interpreter, JIT hardening code and all JITs are
also updated to support the new instructions. Basic idea is
to reduce register pressure by avoiding BPF_J{GT,GE,SGT,SGE}
rewrites. Removing the workaround for the rewrites in LLVM,
this can result in shorter BPF programs, less stack usage
and less verification complexity. First patch provides some
more details on rationale and integration.

Thanks a lot!

v1 -> v2:
  - Reworded commit msg in patch 1

Daniel Borkmann (9):
  bpf: add BPF_J{LT,LE,SLT,SLE} instructions
  bpf, x86: implement jiting of BPF_J{LT,LE,SLT,SLE}
  bpf, arm64: implement jiting of BPF_J{LT,LE,SLT,SLE}
  bpf, sparc64: implement jiting of BPF_J{LT, LE, SLT, SLE}
  bpf, s390x: implement jiting of BPF_J{LT,LE,SLT,SLE}
  bpf, ppc64: implement jiting of BPF_J{LT,LE,SLT,SLE}
  bpf, nfp: implement jiting of BPF_J{LT,LE}
  bpf: enable BPF_J{LT,LE,SLT,SLE} opcodes in verifier
  bpf: add test cases for new BPF_J{LT, LE, SLT, SLE} instructions

 Documentation/networking/filter.txt          |   4 +
 arch/arm64/net/bpf_jit.h                     |   4 +
 arch/arm64/net/bpf_jit_comp.c                |  20 ++
 arch/powerpc/net/bpf_jit.h                   |   1 +
 arch/powerpc/net/bpf_jit_comp64.c            |  20 ++
 arch/s390/net/bpf_jit_comp.c                 |  24 ++
 arch/sparc/net/bpf_jit_comp_64.c             |  34 +++
 arch/x86/net/bpf_jit_comp.c                  |  26 ++
 drivers/net/ethernet/netronome/nfp/bpf/jit.c |  24 ++
 include/uapi/linux/bpf.h                     |   5 +
 kernel/bpf/core.c                            |  60 +++++
 kernel/bpf/verifier.c                        |  62 ++++-
 lib/test_bpf.c                               | 364 +++++++++++++++++++++++++++
 net/core/filter.c                            |  21 +-
 tools/include/uapi/linux/bpf.h               |   5 +
 tools/testing/selftests/bpf/test_verifier.c  | 313 +++++++++++++++++++++++
 16 files changed, 979 insertions(+), 8 deletions(-)

-- 
1.9.3

^ permalink raw reply

* Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
From: Cong Wang @ 2017-08-09 23:34 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Network Development, Linux USB List, David S. Miller,
	Felipe Balbi, Wei Wang
In-Reply-To: <CALAqxLXG8X2nkLEHM63oiY=M17vcpU2+-ec7x_aBWDCqrj4U-Q@mail.gmail.com>

(Cc'ing Wei whose commit was blamed)

On Mon, Aug 7, 2017 at 2:15 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Mon, Aug 7, 2017 at 2:05 PM, John Stultz <john.stultz@linaro.org> wrote:
>> So, with recent testing with my HiKey board, I've been noticing some
>> quirky behavior with my USB eth adapter.
>>
>> Basically, pluging the usb eth adapter in and then removing it, when
>> plugging it back in I often find that its not detected, and the system
>> slowly spits out the following message over and over:
>>   unregister_netdevice: waiting for eth0 to become free. Usage count = 1
>
> The other bit is that after this starts printing, the board will no
> longer reboot (it hangs continuing to occasionally print the above
> message), and I have to manually reset the device.
>

So this warning is not temporarily shown but lasts until a reboot,
right? If so it is a dst refcnt leak.

How reproducible is it for you? From my reading, it seems always
reproduced when you unplug and plug your usb eth interface?
Is there anything else involved? For example, network namespace.

Thanks.

^ permalink raw reply

* Re: skb allocation from interrupt handler?
From: Stephen Hemminger @ 2017-08-09 23:31 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Murali Karicheri, David Miller, netdev
In-Reply-To: <20170809222919.GA4101@electric-eye.fr.zoreil.com>

On Thu, 10 Aug 2017 00:29:19 +0200
Francois Romieu <romieu@fr.zoreil.com> wrote:

> Murali Karicheri <m-karicheri2@ti.com> :
> [...]
> > The internal memory or FIFO can store only up to 3 MTU sized packets. So that has to
> > be processed before PRU gets another packets to send to CPU. So per above, 
> > it is not ideal to run NAPI for this scenario, right? Also for NetCP we use
> > about 128 descriptors with MTU size buffers to handle 1Gbps Ethernet link.
> > Based on that roughly we would need at least 10-12 buffers in the FIFO.
> > 
> > Currently we have a NAPI implementation in use that gives throughput of 95Mbps for
> > MTU sized packets, but our UDP iperf tests shows less than 1% packet loss for an
> > offered traffic of 95Mbps with MTU sized packets.  This is not good for industrial
> > network using HSR/PRP protocol for network redundancy. We need to have zero packet
> > loss for MTU sized packets at 95Mbps throughput. That is the problem description.  
> 
> Imvho you should instrument the kernel to figure where the excess latency that
> prevents NAPI processing to take place within 125 us of physical packet reception
> comes from.
> 
> > As an experiment, I have moved the packet processing to irq handler to see if we 
> > can take advantage of CPU cycle to processing the packet instead of NAPI
> > and to check if the firmware encounters buffer overflow. The result is positive 
> > with no buffer overflow seen at the firmware and no packet loss in the iperf test.
> > But we want to do more testing as an experiment and ran into a uart console locks
> > up after running traffic for about 2 minutes. So I tried enabling the DEBUG HACK 
> > options to get some clue on what is happening and ran into the trace I shared 
> > earlier. So what function can I use to allocate SKB from interrupt handler ?  
> 
> Is your design also so tight on memory that you can't even refill your own
> software skb pool from some non-irq context then only swap buffers in the
> irq handler ?
> 

The current best practice in network drivers is to receive into
an allocated page, then create skb meta data with build_skb() in the NAPI poll
routine.

^ permalink raw reply

* Re: Some traffic stress testing with 4.13.0-rc4-next-20170808 and BUG: [61183.212237] BUG: Bad page state in process ksoftirqd/52 pfn:855f2f
From: Cong Wang @ 2017-08-09 23:27 UTC (permalink / raw)
  To: Paweł Staszewski; +Cc: Linux Kernel Network Developers
In-Reply-To: <9c49b487-2307-27ab-78cd-ed9ad8edbb65@itcare.pl>

On Wed, Aug 9, 2017 at 8:50 AM, Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>
>
>
> [60961.112120] BUG: Bad page state in process ksoftirqd/44 pfn:855f2f
> [60961.112123] page:ffffea002157cbc0 count:51910 mapcount:0 mapping:
> (null) index:0x0
> [60961.112259] flags: 0x600000000000000()
> [60961.112375] raw: 0600000000000000 0000000000000000 0000000000000000
> 0000cac6ffffffff
> [60961.112376] raw: dead000000000100 dead000000000200 0000000000000000
> 0000000000000000
> [60961.112377] page dumped because: nonzero _count
> [60961.112377] Modules linked in: sch_fq x86_pkg_temp_thermal ipmi_si
> [60961.112381] CPU: 44 PID: 232 Comm: ksoftirqd/44 Tainted: G B   W
> 4.13.0-rc4-next-20170808 #1
> [60961.112382] Call Trace:
> [60961.112390]  dump_stack+0x4d/0x63
> [60961.112394]  bad_page+0xf3/0x10f
> [60961.112396]  check_new_page_bad+0x73/0x75
> [60961.112397]  get_page_from_freelist+0x2a9/0x63f
> [60961.112398]  __alloc_pages_nodemask+0xf2/0x187
> [60961.112403]  alloc_pages_current+0x85/0x8c
> [60961.112405]  new_slab+0xc2/0x2f7
> [60961.112407]  ___slab_alloc+0x350/0x3cd
> [60961.112409]  ? fq_enqueue+0x24e/0x3e3 [sch_fq]
> [60961.112411]  __slab_alloc+0x12/0x17
> [60961.112412]  ? __slab_alloc+0x12/0x17
> [60961.112413]  kmem_cache_alloc+0x50/0x112
> [60961.112414]  fq_enqueue+0x24e/0x3e3 [sch_fq]
> [60961.112419]  ? ixgbe_select_queue+0x15/0x17
> [60961.112422]  __dev_queue_xmit+0x2a5/0x4b9
> [60961.112423]  dev_queue_xmit+0xb/0xd


Is this the first stack trace in your dmesg?

It doesn't look like a bug in sch_fq, but like we have
a bad mm page somewhere else, perhaps ixgbe driver.



> [60961.112423]  ? dev_queue_xmit+0xb/0xd
> [60961.112425]  vlan_dev_hard_start_xmit+0x81/0xaa
> [60961.112426]  dev_hard_start_xmit+0xc3/0x197
> [60961.112427]  __dev_queue_xmit+0x38d/0x4b9
> [60961.112429]  ? eth_header+0x27/0xab
> [60961.112429]  dev_queue_xmit+0xb/0xd
> [60961.112430]  ? dev_queue_xmit+0xb/0xd
> [60961.112432]  neigh_connected_output+0x9b/0xb2
> [60961.112436]  ip_finish_output2+0x24e/0x292
> [60961.112437]  ip_finish_output+0x11f/0x12b
> [60961.112438]  ip_output+0x56/0xa7
> [60961.112441]  ? ip_route_input_rcu+0x489/0x7d3
> [60961.112442]  ip_forward_finish+0x53/0x58
> [60961.112443]  ip_forward+0x2ff/0x350
> [60961.112443]  ? ip_frag_mem+0x1e/0x1e
> [60961.112444]  ip_rcv_finish+0x27f/0x28a
> [60961.112445]  ip_rcv+0x2c0/0x30d
> [60961.112448]  __netif_receive_skb_core+0x325/0x4c0
> [60961.112449]  __netif_receive_skb+0x18/0x5a
> [60961.112450]  ? dev_gro_receive+0x2a9/0x3b4
> [60961.112451]  ? __netif_receive_skb+0x18/0x5a
> [60961.112452]  netif_receive_skb_internal+0x4b/0x96
> [60961.112453]  napi_gro_receive+0x75/0xcc
> [60961.112455]  ixgbe_poll+0xed8/0xeef
> [60961.112459]  ? __printk_ratelimit+0x8/0x15
> [60961.112460]  net_rx_action+0xd3/0x22d
> [60961.112462]  __do_softirq+0xe4/0x23a
> [60961.112465]  ? sort_range+0x1d/0x1d
> [60961.112466]  run_ksoftirqd+0x15/0x2a
> [60961.112467]  smpboot_thread_fn+0x128/0x13f
> [60961.112470]  kthread+0xf7/0xfc
> [60961.112471]  ? init_completion+0x24/0x24
> [60961.112475]  ret_from_fork+0x22/0x30
> [60983.541418] capability: warning: `turbostat' uses 32-bit capabilities
> (legacy support in use)
> [61179.159683] ixgbe 0000:d8:00.1 enp216s0f1: detected SFP+: 4
> [61179.503705] ixgbe 0000:d8:00.1 enp216s0f1: NIC Link is Up 10 Gbps, Flow
> Control: None
> [61183.212237] BUG: Bad page state in process ksoftirqd/52 pfn:855f2f
> [61183.212240] page:ffffea002157cbc0 count:-1 mapcount:0 mapping:
> (null) index:0x0
> [61183.212375] flags: 0x600000000000000()
> [61183.212492] raw: 0600000000000000 0000000000000000 0000000000000000
> ffffffffffffffff
> [61183.212493] raw: dead000000000100 dead000000000200 0000000000000000
> 0000000000000000
> [61183.212493] page dumped because: nonzero _count
> [61183.212494] Modules linked in: sch_fq x86_pkg_temp_thermal ipmi_si
> [61183.212498] CPU: 52 PID: 272 Comm: ksoftirqd/52 Tainted: G B   W
> 4.13.0-rc4-next-20170808 #1
> [61183.212500] Call Trace:
> [61183.212507]  dump_stack+0x4d/0x63
> [61183.212512]  bad_page+0xf3/0x10f
> [61183.212514]  check_new_page_bad+0x73/0x75
> [61183.212515]  get_page_from_freelist+0x2a9/0x63f
> [61183.212517]  __alloc_pages_nodemask+0xf2/0x187
> [61183.212523]  ixgbe_alloc_rx_buffers+0x77/0x1bb
> [61183.212524]  ixgbe_poll+0x421/0xeef
> [61183.212529]  ? cfs_rq_util_change+0x1e/0x20
> [61183.212532]  net_rx_action+0xd3/0x22d
> [61183.212534]  __do_softirq+0xe4/0x23a
> [61183.212536]  ? sort_range+0x1d/0x1d
> [61183.212537]  run_ksoftirqd+0x15/0x2a
> [61183.212538]  smpboot_thread_fn+0x128/0x13f
> [61183.212540]  kthread+0xf7/0xfc
> [61183.212541]  ? init_completion+0x24/0x24
> [61183.212546]  ret_from_fork+0x22/0x30
> [61183.283732] ixgbe 0000:d8:00.0 enp216s0f0: detected SFP+: 3
> [61183.527747] ixgbe 0000:d8:00.0 enp216s0f0: NIC Link is Up 10 Gbps, Flow
> Control: None
> [61223.651684] ixgbe 0000:d8:00.1 enp216s0f1: detected SFP+: 4
> [61223.891712] ixgbe 0000:d8:00.1 enp216s0f1: NIC Link is Up 10 Gbps, Flow
> Control: None
> [61228.651748] ixgbe 0000:d8:00.0 enp216s0f0: detected SFP+: 3
> [61228.891759] ixgbe 0000:d8:00.0 enp216s0f0: NIC Link is Up 10 Gbps, Flow
> Control: None
> [61408.583698] ixgbe 0000:d8:00.1 enp216s0f1: detected SFP+: 4
> [61408.831720] ixgbe 0000:d8:00.1 enp216s0f1: NIC Link is Up 10 Gbps, Flow
> Control: None
> [61414.003718] ixgbe 0000:d8:00.0 enp216s0f0: detected SFP+: 3
> [61414.247764] ixgbe 0000:d8:00.0 enp216s0f0: NIC Link is Up 10 Gbps, Flow
> Control: None
> [63318.496105] igb 0000:18:00.3 enp24s0f3: Reset adapter
> [63321.828156] igb 0000:18:00.3 enp24s0f3: igb: enp24s0f3 NIC Link is Up
> 1000 Mbps Full Duplex, Flow Control: RX
>
>
>
>
>
> If matters configuration below:
>
>  tc -s -d qdisc show
> qdisc noqueue 0: dev lo root refcnt 2
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> qdisc mq 0: dev enp24s0f3 root
>  Sent 10595979 bytes 44275 pkt (dropped 0, overlimits 0 requeues 7)
>  backlog 0b 0p requeues 7
> qdisc pfifo_fast 0: dev enp24s0f3 parent :8 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 2562064 bytes 7304 pkt (dropped 0, overlimits 0 requeues 1)
>  backlog 0b 0p requeues 1
> qdisc pfifo_fast 0: dev enp24s0f3 parent :7 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 152292 bytes 3589 pkt (dropped 0, overlimits 0 requeues 2)
>  backlog 0b 0p requeues 2
> qdisc pfifo_fast 0: dev enp24s0f3 parent :6 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 7298 bytes 15 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev enp24s0f3 parent :5 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 5042 bytes 5 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev enp24s0f3 parent :4 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 3194 bytes 4 pkt (dropped 0, overlimits 0 requeues 1)
>  backlog 0b 0p requeues 1
> qdisc pfifo_fast 0: dev enp24s0f3 parent :3 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 5386576 bytes 13206 pkt (dropped 0, overlimits 0 requeues 1)
>  backlog 0b 0p requeues 1
> qdisc pfifo_fast 0: dev enp24s0f3 parent :2 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 6286 bytes 24 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> qdisc pfifo_fast 0: dev enp24s0f3 parent :1 bands 3 priomap  1 2 2 2 1 2 0 0
> 1 1 1 1 1 1 1 1
>  Sent 2473227 bytes 20128 pkt (dropped 0, overlimits 0 requeues 2)
>  backlog 0b 0p requeues 2
> qdisc mq 1: dev enp216s0f0 root
>  Sent 140 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
> qdisc fq 831c: dev enp216s0f0 parent 1:a limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8320: dev enp216s0f0 parent 1:e limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 831e: dev enp216s0f0 parent 1:c limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8322: dev enp216s0f0 parent 1:10 limit 8192p flow_limit 100p
> buckets 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate
> 10Gbit refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8313: dev enp216s0f0 parent 1:1 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8315: dev enp216s0f0 parent 1:3 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8317: dev enp216s0f0 parent 1:5 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 831b: dev enp216s0f0 parent 1:9 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8319: dev enp216s0f0 parent 1:7 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 140 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   1 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 831d: dev enp216s0f0 parent 1:b limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 831f: dev enp216s0f0 parent 1:d limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8321: dev enp216s0f0 parent 1:f limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8316: dev enp216s0f0 parent 1:4 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8314: dev enp216s0f0 parent 1:2 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 8318: dev enp216s0f0 parent 1:6 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc fq 831a: dev enp216s0f0 parent 1:8 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>   0 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 0 throttled
> qdisc mq 1: dev enp216s0f1 root
>  Sent 5724464060294 bytes 3017574303 pkt (dropped 261703917, overlimits 0
> requeues 1588)
>  backlog 19340684b 68101p requeues 1588
> qdisc fq 8331: dev enp216s0f1 parent 1:f limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357754034478 bytes 1261904590 pkt (dropped 16345926, overlimits 0
> requeues 99)
>  backlog 1190528b 4198p requeues 99
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 15 throttled, 2238 ns latency, 16345926 flows_plimit
> qdisc fq 8324: dev enp216s0f1 parent 1:2 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357941986556 bytes 1265190082 pkt (dropped 16334441, overlimits 0
> requeues 114)
>  backlog 1203308b 4237p requeues 114
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 56 throttled, 1988 ns latency, 16334441 flows_plimit
> qdisc fq 8326: dev enp216s0f1 parent 1:4 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357756741808 bytes 1261951272 pkt (dropped 16352852, overlimits 0
> requeues 108)
>  backlog 1195356b 4210p requeues 108
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 10 throttled, 1652 ns latency, 16352852 flows_plimit
> qdisc fq 832a: dev enp216s0f1 parent 1:8 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357754168172 bytes 1261906634 pkt (dropped 16375523, overlimits 0
> requeues 93)
>  backlog 1191664b 4196p requeues 93
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 4 throttled, 1722 ns latency, 16375523 flows_plimit
> qdisc fq 8328: dev enp216s0f1 parent 1:6 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357755800928 bytes 1261934832 pkt (dropped 16297692, overlimits 0
> requeues 89)
>  backlog 1206432b 4248p requeues 89
>   65 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 4 throttled, 1520 ns latency, 16297692 flows_plimit
> qdisc fq 832c: dev enp216s0f1 parent 1:a limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357757943172 bytes 1261969807 pkt (dropped 16393936, overlimits 0
> requeues 97)
>  backlog 1185984b 4176p requeues 97
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 4 throttled, 1257 ns latency, 16393936 flows_plimit
> qdisc fq 832e: dev enp216s0f1 parent 1:c limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357754956814 bytes 1261920403 pkt (dropped 16373257, overlimits 0
> requeues 111)
>  backlog 1184848b 4172p requeues 111
>   65 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 11 throttled, 2070 ns latency, 16373257 flows_plimit
> qdisc fq 8332: dev enp216s0f1 parent 1:10 limit 8192p flow_limit 100p
> buckets 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate
> 10Gbit refill_delay 40.0ms
>  Sent 357754779582 bytes 1261916755 pkt (dropped 16306559, overlimits 0
> requeues 80)
>  backlog 1151336b 4054p requeues 80
>   65 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 3 throttled, 1530 ns latency, 16306559 flows_plimit
> qdisc fq 8330: dev enp216s0f1 parent 1:e limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357754801422 bytes 1261919138 pkt (dropped 16360298, overlimits 0
> requeues 68)
>  backlog 1266072b 4457p requeues 68
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 7 throttled, 1114 ns latency, 16360298 flows_plimit
> qdisc fq 8325: dev enp216s0f1 parent 1:3 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357756968400 bytes 1261953663 pkt (dropped 16377069, overlimits 0
> requeues 84)
>  backlog 1267492b 4463p requeues 84
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 6 throttled, 2470 ns latency, 16377069 flows_plimit
> qdisc fq 8323: dev enp216s0f1 parent 1:1 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357944308424 bytes 1265224502 pkt (dropped 16351766, overlimits 0
> requeues 125)
>  backlog 1227164b 4321p requeues 125
>   65 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 58 throttled, 2143 ns latency, 16351766 flows_plimit
> qdisc fq 8327: dev enp216s0f1 parent 1:5 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357756634544 bytes 1261949446 pkt (dropped 16354220, overlimits 0
> requeues 114)
>  backlog 1176896b 4143p requeues 114
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 5 throttled, 2274 ns latency, 16354220 flows_plimit
> qdisc fq 8329: dev enp216s0f1 parent 1:7 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357755020876 bytes 1261921756 pkt (dropped 16366535, overlimits 0
> requeues 108)
>  backlog 1171216b 4124p requeues 108
>   65 flows (1 inactive, 0 throttled)
>   0 gc, 0 highprio, 3 throttled, 932 ns latency, 16366535 flows_plimit
> qdisc fq 832d: dev enp216s0f1 parent 1:b limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357755470474 bytes 1261930198 pkt (dropped 16347462, overlimits 0
> requeues 99)
>  backlog 1255848b 4422p requeues 99
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 5 throttled, 1699 ns latency, 16347462 flows_plimit
> qdisc fq 832b: dev enp216s0f1 parent 1:9 limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357754977102 bytes 1261920885 pkt (dropped 16407954, overlimits 0
> requeues 92)
>  backlog 1260108b 4437p requeues 92
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 5 throttled, 1638 ns latency, 16407954 flows_plimit
> qdisc fq 832f: dev enp216s0f1 parent 1:d limit 8192p flow_limit 100p buckets
> 1024 orphan_mask 1023 quantum 3028 initial_quantum 15140 maxrate 10Gbit
> refill_delay 40.0ms
>  Sent 357755499918 bytes 1261929638 pkt (dropped 16358427, overlimits 0
> requeues 107)
>  backlog 1194220b 4205p requeues 107
>   64 flows (0 inactive, 0 throttled)
>   0 gc, 0 highprio, 8 throttled, 2257 ns latency, 16358427 flows_plimit
> qdisc noqueue 0: dev vlan1000 root refcnt 2
>  Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
>  backlog 0b 0p requeues 0
>
>  ethtool -k enp216s0f1
> Features for enp216s0f1:
> Cannot get device udp-fragmentation-offload settings: Operation not
> supported
> rx-checksumming: on
> tx-checksumming: on
>         tx-checksum-ipv4: off [fixed]
>         tx-checksum-ip-generic: on
>         tx-checksum-ipv6: off [fixed]
>         tx-checksum-fcoe-crc: off [fixed]
>         tx-checksum-sctp: on
> scatter-gather: on
>         tx-scatter-gather: on
>         tx-scatter-gather-fraglist: off [fixed]
> tcp-segmentation-offload: on
>         tx-tcp-segmentation: on
>         tx-tcp-ecn-segmentation: off [fixed]
>         tx-tcp-mangleid-segmentation: on
>         tx-tcp6-segmentation: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: on
> large-receive-offload: off
> rx-vlan-offload: on
> tx-vlan-offload: on
> ntuple-filters: on
> receive-hashing: on
> highdma: on [fixed]
> rx-vlan-filter: on
> vlan-challenged: off [fixed]
> tx-lockless: off [fixed]
> netns-local: off [fixed]
> tx-gso-robust: off [fixed]
> tx-fcoe-segmentation: off [fixed]
> tx-gre-segmentation: on
> tx-gre-csum-segmentation: on
> tx-ipxip4-segmentation: on
> tx-ipxip6-segmentation: on
> tx-udp_tnl-segmentation: on
> tx-udp_tnl-csum-segmentation: on
> tx-gso-partial: on
> tx-sctp-segmentation: off [fixed]
> tx-esp-segmentation: off [fixed]
> fcoe-mtu: off [fixed]
> tx-nocache-copy: on
> loopback: off [fixed]
> rx-fcs: off [fixed]
> rx-all: off
> tx-vlan-stag-hw-insert: off [fixed]
> rx-vlan-stag-hw-parse: off [fixed]
> rx-vlan-stag-filter: off [fixed]
> l2-fwd-offload: off
> hw-tc-offload: on
> esp-hw-offload: off [fixed]
> esp-tx-csum-hw-offload: off [fixed]
> rx-udp_tunnel-port-offload: on
>
>  ethtool -k enp216s0f0
> Features for enp216s0f0:
> Cannot get device udp-fragmentation-offload settings: Operation not
> supported
> rx-checksumming: on
> tx-checksumming: on
>         tx-checksum-ipv4: off [fixed]
>         tx-checksum-ip-generic: on
>         tx-checksum-ipv6: off [fixed]
>         tx-checksum-fcoe-crc: off [fixed]
>         tx-checksum-sctp: on
> scatter-gather: on
>         tx-scatter-gather: on
>         tx-scatter-gather-fraglist: off [fixed]
> tcp-segmentation-offload: on
>         tx-tcp-segmentation: on
>         tx-tcp-ecn-segmentation: off [fixed]
>         tx-tcp-mangleid-segmentation: on
>         tx-tcp6-segmentation: on
> udp-fragmentation-offload: off
> generic-segmentation-offload: off
> generic-receive-offload: on
> large-receive-offload: off
> rx-vlan-offload: on
> tx-vlan-offload: on
> ntuple-filters: on
> receive-hashing: on
> highdma: on [fixed]
> rx-vlan-filter: on
> vlan-challenged: off [fixed]
> tx-lockless: off [fixed]
> netns-local: off [fixed]
> tx-gso-robust: off [fixed]
> tx-fcoe-segmentation: off [fixed]
> tx-gre-segmentation: on
> tx-gre-csum-segmentation: on
> tx-ipxip4-segmentation: on
> tx-ipxip6-segmentation: on
> tx-udp_tnl-segmentation: on
> tx-udp_tnl-csum-segmentation: on
> tx-gso-partial: on
> tx-sctp-segmentation: off [fixed]
> tx-esp-segmentation: off [fixed]
> fcoe-mtu: off [fixed]
> tx-nocache-copy: on
> loopback: off [fixed]
> rx-fcs: off [fixed]
> rx-all: off
> tx-vlan-stag-hw-insert: off [fixed]
> rx-vlan-stag-hw-parse: off [fixed]
> rx-vlan-stag-filter: off [fixed]
> l2-fwd-offload: off
> hw-tc-offload: off
> esp-hw-offload: off [fixed]
> esp-tx-csum-hw-offload: off [fixed]
> rx-udp_tunnel-port-offload: on
>
>

^ permalink raw reply

* [PATCH net-next 2/2] sock: fix zerocopy_success regression with msg_zerocopy
From: Willem de Bruijn @ 2017-08-09 23:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, dsahern, Willem de Bruijn
In-Reply-To: <20170809230944.59289-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemb@google.com>

Do not use uarg->zerocopy outside msg_zerocopy. In other paths the
field is not explicitly initialized and aliases another field.

Those paths have only one reference so do not need this intermediate
variable. Call uarg->callback directly.

Fixes: 1f8b977ab32d ("sock: enable MSG_ZEROCOPY")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8c0708d2e5e6..7594e19bce62 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1273,8 +1273,13 @@ static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
 	struct ubuf_info *uarg = skb_zcopy(skb);
 
 	if (uarg) {
-		uarg->zerocopy = uarg->zerocopy && zerocopy;
-		sock_zerocopy_put(uarg);
+		if (uarg->callback == sock_zerocopy_callback) {
+			uarg->zerocopy = uarg->zerocopy && zerocopy;
+			sock_zerocopy_put(uarg);
+		} else {
+			uarg->callback(uarg, zerocopy);
+		}
+
 		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
 	}
 }
-- 
2.14.0.434.g98096fd7a8-goog

^ permalink raw reply related

* [PATCH net-next 1/2] sock: fix zerocopy panic in mem accounting
From: Willem de Bruijn @ 2017-08-09 23:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, dsahern, Willem de Bruijn
In-Reply-To: <20170809230944.59289-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemb@google.com>

Only call mm_unaccount_pinned_pages when releasing a struct ubuf_info
that has initialized its field uarg->mmp.

Before this patch, a vhost-net with experimental_zcopytx can crash in

  mm_unaccount_pinned_pages
  sock_zerocopy_put
  skb_zcopy_clear
  skb_release_data

Only sock_zerocopy_alloc initializes this field. Move the unaccount
call from generic sock_zerocopy_put to its specific callback
sock_zerocopy_callback.

Fixes: a91dbff551a6 ("sock: ulimit on MSG_ZEROCOPY pages")
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/core/skbuff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 42b62c716a33..cb123590c674 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1044,6 +1044,8 @@ void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
 	u32 lo, hi;
 	u16 len;
 
+	mm_unaccount_pinned_pages(&uarg->mmp);
+
 	/* if !len, there was only 1 call, and it was aborted
 	 * so do not queue a completion notification
 	 */
@@ -1084,8 +1086,6 @@ EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
 void sock_zerocopy_put(struct ubuf_info *uarg)
 {
 	if (uarg && atomic_dec_and_test(&uarg->refcnt)) {
-		mm_unaccount_pinned_pages(&uarg->mmp);
-
 		if (uarg->callback)
 			uarg->callback(uarg, uarg->zerocopy);
 		else
-- 
2.14.0.434.g98096fd7a8-goog

^ permalink raw reply related

* [PATCH net-next 0/2] zerocopy fixes
From: Willem de Bruijn @ 2017-08-09 23:09 UTC (permalink / raw)
  To: netdev; +Cc: davem, dsahern, Willem de Bruijn

From: Willem de Bruijn <willemb@google.com>

Fix two issues introduced in the msg_zerocopy patchset.

Willem de Bruijn (2):
  sock: fix zerocopy panic in mem accounting
  sock: fix zerocopy_success regression with msg_zerocopy

 include/linux/skbuff.h | 9 +++++++--
 net/core/skbuff.c      | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.14.0.434.g98096fd7a8-goog

^ permalink raw reply

* RE: [ovs-dev] [PATCH net-next] openvswitch: add NSH support
From: Yang, Yi Y @ 2017-08-09 22:53 UTC (permalink / raw)
  To: Ben Pfaff
  Cc: Jan Scheurich, dev@openvswitch.org, netdev@vger.kernel.org,
	Jiri Benc, davem@davemloft.net, Zoltán Balogh
In-Reply-To: <20170809205348.GJ6175@ovn.org>

struct ovs_action_encap_nsh is the only one way we transfer all the data for encap_nsh, netlink allows variable attribute, so I don't think we break netlink convention or abuse this variable feature.

Even if we bring nested attributes to handle this, OVS_ACTION_ATTR_ENCAP_NSH is still length-variable, OVS_NSH_ATTR_MD2 is also length-variable (it can be from 0 to 248), so I don't think such way can avoid the issue you're addressing.

The result will be worse, it will make many difficulties when we transfer all the data for encap_nsh between OVS' components.

-----Original Message-----
From: Ben Pfaff [mailto:blp@ovn.org] 
Sent: Thursday, August 10, 2017 4:54 AM
To: Yang, Yi Y <yi.y.yang@intel.com>
Cc: Jan Scheurich <jan.scheurich@ericsson.com>; dev@openvswitch.org; netdev@vger.kernel.org; Jiri Benc <jbenc@redhat.com>; davem@davemloft.net; Zoltán Balogh <zoltan.balogh@ericsson.com>
Subject: Re: [ovs-dev] [PATCH net-next] openvswitch: add NSH support

On Wed, Aug 09, 2017 at 08:12:36PM +0000, Yang, Yi Y wrote:
> Ben, do you mean we bring two new attributes (OVS_NSH_ATTR_MD1 and
> OVS_NSH_ATTR_MD2) and embed one of them in OVS_ACTION_ATTR_ENCAP_NSH?

Yes.

> Anyway we need to use a struct or something else to transfer those 
> metadata between functions, how do you think we can handle this 
> without metadata in struct ovs_action_encap_nsh? I mean how we handle 
> the arguments for function encap_nsh.

I guess that a pointer to the embedded nlattr with type OVS_NSH_ATTR_MD1 or OVS_NSH_ATTR2 should work OK.

Keep in mind that I'm not a kernel-side maintainer of any kind.  I am only passing along what I've perceived to be common Netlink protocol design patterns.

> -----Original Message-----
> From: netdev-owner@vger.kernel.org 
> [mailto:netdev-owner@vger.kernel.org] On Behalf Of Ben Pfaff
> Sent: Thursday, August 10, 2017 2:09 AM
> To: Yang, Yi Y <yi.y.yang@intel.com>
> Cc: Jan Scheurich <jan.scheurich@ericsson.com>; dev@openvswitch.org; 
> netdev@vger.kernel.org; Jiri Benc <jbenc@redhat.com>; 
> davem@davemloft.net; Zoltán Balogh <zoltan.balogh@ericsson.com>
> Subject: Re: [ovs-dev] [PATCH net-next] openvswitch: add NSH support
> 
> On Wed, Aug 09, 2017 at 09:41:51AM +0000, Yang, Yi Y wrote:
> > Hi,  Jan
> > 
> > I have worked out a patch, will send it quickly for Ben. In 
> > addition, I also will send out a patch to change encap_nsh 
> > &decap_nsh to push_nsh and pop_nsh. Per comments from all the 
> > people, we all agreed to do so :-)
> > 
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index bc6c94b..4936c12 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -793,7 +793,7 @@ struct ovs_action_push_eth {
> >         struct ovs_key_ethernet addresses;  };
> > 
> > -#define OVS_ENCAP_NSH_MAX_MD_LEN 16
> > +#define OVS_ENCAP_NSH_MAX_MD_LEN 248
> >  /*
> >   * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
> >   * @flags: NSH header flags.
> > @@ -809,7 +809,7 @@ struct ovs_action_encap_nsh {
> >      uint8_t mdlen;
> >      uint8_t np;
> >      __be32 path_hdr;
> > -    uint8_t metadata[OVS_ENCAP_NSH_MAX_MD_LEN];
> > +    uint8_t metadata[];
> >  };
> 
> This brings the overall format of ovs_action_encap_nsh to:
> 
> struct ovs_action_encap_nsh {
>     uint8_t flags;
>     uint8_t mdtype;
>     uint8_t mdlen;
>     uint8_t np;
>     __be32 path_hdr;
>     uint8_t metadata[];
> };
> 
> This is an unusual format for a Netlink attribute.  More commonly, one would put variable-length data into an attribute of its own, which allows that data to be handled using the regular Netlink means.  Then the mdlen and metadata members could be removed, since they would be part of the additional attribute, and one might expect the mdtype member to be removed as well since each type of metadata would be in a different attribute type.
> 
> So, a format closer to what I expect to see in Netlink is something 
> like
> this:
> 
> /**
>  * enum ovs_nsh_attr - Metadata attributes for %OVS_ACTION_ENCAP_NSH action.
>  *
>  * @OVS_NSH_ATTR_MD1: Contains 16-byte NSH type-1 metadata.
>  * @OVS_NSH_ATTR_MD2: Contains 0- to 255-byte variable-length NSH 
> type-2
>  * metadata. */
> enum ovs_nsh_attr {
>     OVS_NSH_ATTR_MD1,
>     OVS_NSH_ATTR_MD2
> };
> 
> /*
>  * struct ovs_action_encap_nsh - %OVS_ACTION_ATTR_ENCAP_NSH
>  *
>  * @path_hdr: NSH service path id and service index.
>  * @flags: NSH header flags.
>  * @np: NSH next_protocol: Inner packet type.
>  *
>  * Followed by either %OVS_NSH_ATTR_MD1 or %OVS_NSH_ATTR_MD2 attribute.
>  */
> struct ovs_action_encap_nsh {
>     __be32 path_hdr;
>     uint8_t flags;
>     uint8_t np;
> };

^ permalink raw reply

* Re: [PATCH v4 10/12] ARM: dts: rk3228-evb: Enable the internal phy for gmac
From: Florian Fainelli @ 2017-08-09 22:50 UTC (permalink / raw)
  To: David Wu, davem, heiko, andrew, robh+dt, mark.rutland,
	catalin.marinas, will.deacon, olof, linux, arnd
  Cc: peppe.cavallaro, alexandre.torgue, huangtao, hwg, netdev,
	linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
In-Reply-To: <1502280799-2514-1-git-send-email-david.wu@rock-chips.com>

On August 9, 2017 5:13:19 AM PDT, David Wu <david.wu@rock-chips.com> wrote:
>This patch enables the internal phy for rk3228 evb board
>by default.
>To use the external 1000M phy on evb board, need to make
>some switch of evb board to be on.
>
>Signed-off-by: David Wu <david.wu@rock-chips.com>


LGTM

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

-- 
Florian

^ permalink raw reply

* Re: [PATCH v4 05/12] Documentation: net: phy: Add phy-is-internal binding
From: Florian Fainelli @ 2017-08-09 22:47 UTC (permalink / raw)
  To: David Wu, davem, heiko, andrew, robh+dt, mark.rutland,
	catalin.marinas, will.deacon, olof, linux, arnd
  Cc: peppe.cavallaro, alexandre.torgue, huangtao, hwg, netdev,
	linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
In-Reply-To: <1502280630-2254-1-git-send-email-david.wu@rock-chips.com>

On August 9, 2017 5:10:30 AM PDT, David Wu <david.wu@rock-chips.com> wrote:
>Add the documentation for internal phy. A boolean property
>indicates that a internal phy will be used.
>
>Signed-off-by: David Wu <david.wu@rock-chips.com>
>---
> Documentation/devicetree/bindings/net/phy.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/net/phy.txt
>b/Documentation/devicetree/bindings/net/phy.txt
>index b558576..942c892 100644
>--- a/Documentation/devicetree/bindings/net/phy.txt
>+++ b/Documentation/devicetree/bindings/net/phy.txt
>@@ -52,6 +52,9 @@ Optional Properties:
>   Mark the corresponding energy efficient ethernet mode as broken and
>   request the ethernet to stop advertising it.
> 
>+- phy-is-internal: If set, indicates that phy will connect to the MAC
>as a
>+  internal phy.

Something along the lines of:

If set, indicates that the PHY is integrated into the same physical package as the Ethernet MAC.

Please always capitalize PHY.


-- 
Florian

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox