* Re: [RFC PATCH net-next 01/20] net: dsa: mv88e6xxx: factorize PHY access with PPU
From: Andrew Lunn @ 2016-05-05 22:59 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-2-git-send-email-vivien.didelot@savoirfairelinux.com>
> -static int mv88e6xxx_ppu_access_get(struct mv88e6xxx_priv_state *ps)
> +static int _mv88e6xxx_ppu_access_get(struct mv88e6xxx_priv_state *ps)
Hi Vivien
We agreed to stop adding _ to functions that assume the lock has been
taken. Now that we check the lock is held in lowest level function, it
quickly becomes clear if the locks are wrong. It either deadlocks, or
it prints a warning when there is an error.
Please don't rename these functions.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 01/20] net: dsa: mv88e6xxx: factorize PHY access with PPU
From: Andrew Lunn @ 2016-05-05 23:06 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-2-git-send-email-vivien.didelot@savoirfairelinux.com>
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> int addr = mv88e6xxx_port_to_phy_addr(ps, port);
> - int ret;
> + int err;
>
> if (addr < 0)
> return 0xffff;
>
> +
We don't want an extra blank line. I'm surprised checkpatch did not
warn about this.
> mutex_lock(&ps->smi_mutex);
> - ret = _mv88e6xxx_phy_write(ps, addr, regnum, val);
> +
> + if (mv88e6xxx_has(ps, MV88E6XXX_FLAG_PPU))
> + err = _mv88e6xxx_phy_write_ppu(ps, addr, regnum, val);
> + else
> + err = _mv88e6xxx_phy_write(ps, addr, regnum, val);
> +
> mutex_unlock(&ps->smi_mutex);
> - return ret;
> +
> + return err;
Please don't rename ret to err. This patch is about phy reading and
writing, and i don't expect to see anything else here. It makes it
harder to review.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 04/20] net: dsa: mv88e6xxx: factorize temperature access
From: Andrew Lunn @ 2016-05-05 23:40 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-5-git-send-email-vivien.didelot@savoirfairelinux.com>
> int mv88e6xxx_get_temp(struct dsa_switch *ds, int *temp)
> {
> struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> + int err;
> +
> + if (!mv88e6xxx_has(ps, MV88E6XXX_FLAG_TEMP))
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&ps->smi_mutex);
>
> if (mv88e6xxx_6320_family(ps) || mv88e6xxx_6352_family(ps))
> - return mv88e63xx_get_temp(ds, temp);
> + err = _mv88e63xx_get_temp(ps, temp);
> + else
> + err = _mv88e61xx_get_temp(ps, temp);
It seems like this should be done with a flag, rather than the family.
However, don't spend too much time on this. I plan to rip it all out
and put it in the correct place, in the Marvell PHY driver. The
temperature sensor is in the PHY, and other Marvell PHYs also have
this temperature sensor.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 13/20] net: dsa: mv88e6xxx: factorize GLOBAL_CONTROL setup
From: Andrew Lunn @ 2016-05-06 0:55 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-14-git-send-email-vivien.didelot@savoirfairelinux.com>
On Thu, May 05, 2016 at 06:40:57PM -0400, Vivien Didelot wrote:
> All switch models configure the GLOBAL_CONTROL register with slightly
> differences.
And as far as i know, we have no idea why.
> Assume a common setup to enable the PHY Polling Unit if present, discard
> packets with excessive collisions, set the maximum frame size to 1632,
> and mask all interrupt sources.
This is a behaviour change, not just a refactor. It would be good to
say on which families it has been tested, and which have not.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 14/20] net: dsa: mv88e6xxx: factorize VLAN Ethertype
From: Andrew Lunn @ 2016-05-06 0:57 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-15-git-send-email-vivien.didelot@savoirfairelinux.com>
On Thu, May 05, 2016 at 06:40:58PM -0400, Vivien Didelot wrote:
> Only the 6131 driver was setting the VLAN Ethertype to 0x8100. Set it to
> all models.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> drivers/net/dsa/mv88e6131.c | 9 ++++-----
> drivers/net/dsa/mv88e6xxx.c | 8 ++++++++
> drivers/net/dsa/mv88e6xxx.h | 7 +++++++
> 3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
> index 013668c..4c0b1e9 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -25,6 +25,7 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
> .num_databases = 256,
> .num_ports = 11,
> .flags = MV88E6XXX_FLAG_ATU |
> + MV88E6XXX_FLAG_CORE_TAG_TYPE |
> MV88E6XXX_FLAG_PPU |
> MV88E6XXX_FLAG_VLANTABLE |
> MV88E6XXX_FLAG_VTU,
> @@ -35,6 +36,7 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
> .num_databases = 4096,
> .num_ports = 10,
> .flags = MV88E6XXX_FLAG_ATU |
> + MV88E6XXX_FLAG_CORE_TAG_TYPE |
> MV88E6XXX_FLAG_PPU |
> MV88E6XXX_FLAG_VLANTABLE |
> MV88E6XXX_FLAG_VTU,
> @@ -45,6 +47,7 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
> .num_databases = 256,
> .num_ports = 8,
> .flags = MV88E6XXX_FLAG_ATU |
> + MV88E6XXX_FLAG_CORE_TAG_TYPE |
> MV88E6XXX_FLAG_PPU |
> MV88E6XXX_FLAG_VLANTABLE |
> MV88E6XXX_FLAG_VTU,
> @@ -55,6 +58,7 @@ static const struct mv88e6xxx_info mv88e6131_table[] = {
> .num_databases = 256,
> .num_ports = 10,
> .flags = MV88E6XXX_FLAG_ATU |
> + MV88E6XXX_FLAG_CORE_TAG_TYPE |
> MV88E6XXX_FLAG_PPU |
> MV88E6XXX_FLAG_VLANTABLE |
> MV88E6XXX_FLAG_VTU,
Hi Vivien
Rather than repeating these flags again and again, could you add one
#define containing the flags, and then use that to initialise .flags.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 19/20] net: dsa: mv88e6xxx: factorize tag protocol
From: Andrew Lunn @ 2016-05-06 1:09 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-20-git-send-email-vivien.didelot@savoirfairelinux.com>
On Thu, May 05, 2016 at 06:41:03PM -0400, Vivien Didelot wrote:
> 6131 is the only driver to set the tag protocol to DSA_TAG_PROTO_DSA.
> Since it works fine with DSA_TAG_PROTO_EDSA, change its value, like all
> other mv88e6xxx drivers.
Hi Vivien
You might as well remove net/dsa/tag_dsa.c as well, since it is now
unused. Please do that as a separate patch.
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 20/20] net: dsa: mv88e6xxx: factorize the switch driver
From: Andrew Lunn @ 2016-05-06 1:14 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <1462488064-1841-21-git-send-email-vivien.didelot@savoirfairelinux.com>
> +config NET_DSA_MV88E6XXX
> tristate "Marvell 88E6085/6095/6095F/6131 ethernet switch chip support"
> depends on NET_DSA
> - select NET_DSA_MV88E6XXX
> select NET_DSA_TAG_EDSA
> ---help---
> This enables support for the Marvell 88E6085/6095/6095F/6131
> ethernet switch chips.
Hi Vivien
You should update the list of devices. We need some text in the
tristate and help which suggests this driver supports everything
except mv88e6060.
Andrew
^ permalink raw reply
* [PATCH net-next 7/7] samples/bpf: add verifier tests
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
add few tests for "pointer to packet" logic of the verifier
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
samples/bpf/test_verifier.c | 80 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c
index 9eba8d1d9dcc..fe2fcec98c1f 100644
--- a/samples/bpf/test_verifier.c
+++ b/samples/bpf/test_verifier.c
@@ -1448,6 +1448,86 @@ static struct bpf_test tests[] = {
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
},
+ {
+ "pkt: test1",
+ .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_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ },
+ {
+ "pkt: test2",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_MOV64_REG(BPF_REG_5, BPF_REG_3),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, 14),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_5, BPF_REG_4, 15),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_3, 7),
+ BPF_LDX_MEM(BPF_B, BPF_REG_4, BPF_REG_3, 12),
+ BPF_ALU64_IMM(BPF_MUL, BPF_REG_4, 14),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_3, BPF_REG_4),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_1),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_2, 48),
+ BPF_ALU64_IMM(BPF_RSH, BPF_REG_2, 48),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_3, BPF_REG_2),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_3),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, 8),
+ BPF_LDX_MEM(BPF_W, BPF_REG_1, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_2, BPF_REG_1, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_1, BPF_REG_3, 4),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ },
+ {
+ "pkt: test3",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "invalid bpf_context access off=76",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+ },
+ {
+ "pkt: test4",
+ .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_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_STX_MEM(BPF_B, BPF_REG_2, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "cannot write",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ },
};
static int probe_filter_length(struct bpf_insn *fp)
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 0/7] bpf: introduce direct packet access
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
This set of patches introduce 'direct packet access' from
cls_bpf and act_bpf programs (which are root only).
Current bpf programs use LD_ABS, LD_INS instructions which have
to do 'if (off < skb_headlen)' for every packet access.
It's ok for socket filters, but too slow for XDP, since single
LD_ABS insn consumes 3% of cpu. Therefore we have to amortize the cost
of length check over multiple packet accesses via direct access
to skb->data, data_end pointers.
The existing packet parser typically look like:
if (load_half(skb, offsetof(struct ethhdr, h_proto)) != ETH_P_IP)
return 0;
if (load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol)) != IPPROTO_UDP ||
load_byte(skb, ETH_HLEN) != 0x45)
return 0;
...
with 'direct packet access' the bpf program becomes:
void *data = (void *)(long)skb->data;
void *data_end = (void *)(long)skb->data_end;
struct eth_hdr *eth = data;
struct iphdr *iph = data + sizeof(*eth);
if (data + sizeof(*eth) + sizeof(*iph) + sizeof(*udp) > data_end)
return 0;
if (eth->h_proto != htons(ETH_P_IP))
return 0;
if (iph->protocol != IPPROTO_UDP || iph->ihl != 5)
return 0;
...
which is more natural to write and significantly faster.
See patch 6 for performance tests:
21Mpps(old) vs 24Mpps(new) with just 5 loads.
For more complex parsers the performance gain is higher.
The other approach implemented in [1] was adding two new instructions
to interpreter and JITs and was too hard to use from llvm side.
The approach presented here doesn't need any instruction changes,
but the verifier has to work harder to check safety of the packet access.
Patch 1 prepares the code and Patch 2 adds new checks for direct
packet access and all of them are gated with 'env->allow_ptr_leaks'
which is true for root only.
Patch 3 improves search pruning for large programs.
Patch 4 wires in verifier's changes with net/core/filter side.
Patch 5 updates docs
Patches 6 and 7 add tests.
[1] https://git.kernel.org/cgit/linux/kernel/git/ast/bpf.git/?h=ld_abs_dw
Alexei Starovoitov (7):
bpf: cleanup verifier code
bpf: direct packet access
bpf: improve verifier state equivalence
bpf: wire in data and data_end for cls_act_bpf
bpf: add documentation for 'direct packet access'
samples/bpf: add 'pointer to packet' tests
samples/bpf: add verifier tests
Documentation/networking/filter.txt | 85 +++++-
include/linux/filter.h | 16 +
include/uapi/linux/bpf.h | 2 +
kernel/bpf/core.c | 5 +
kernel/bpf/verifier.c | 562 +++++++++++++++++++++++++++++++-----
net/core/filter.c | 51 +++-
net/sched/act_bpf.c | 2 +
net/sched/cls_bpf.c | 2 +
samples/bpf/Makefile | 2 +
samples/bpf/parse_ldabs.c | 41 +++
samples/bpf/parse_simple.c | 48 +++
samples/bpf/parse_varlen.c | 153 ++++++++++
samples/bpf/test_cls_bpf.sh | 37 +++
samples/bpf/test_verifier.c | 80 +++++
14 files changed, 1004 insertions(+), 82 deletions(-)
create mode 100644 samples/bpf/parse_ldabs.c
create mode 100644 samples/bpf/parse_simple.c
create mode 100644 samples/bpf/parse_varlen.c
create mode 100755 samples/bpf/test_cls_bpf.sh
--
2.8.0
^ permalink raw reply
* [PATCH net-next 2/7] bpf: direct packet access
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
Extended BPF carried over two instructions from classic to access
packet data: LD_ABS and LD_IND. They're highly optimized in JITs,
but due to their design they have to do length check for every access.
When BPF is processing 20M packets per second single LD_ABS after JIT
is consuming 3% cpu. Hence the need to optimize it further by amortizing
the cost of 'off < skb_headlen' over multiple packet accesses.
One option is to introduce two new eBPF instructions LD_ABS_DW and LD_IND_DW
with similar usage as skb_header_pointer().
The kernel part for interpreter and x64 JIT was implemented in [1], but such
new insns behave like old ld_abs and abort the program with 'return 0' if
access is beyond linear data. Such hidden control flow is hard to workaround
plus changing JITs and rolling out new llvm is incovenient.
Therefore allow cls_bpf/act_bpf program access skb->data directly:
int bpf_prog(struct __sk_buff *skb)
{
struct iphdr *ip;
if (skb->data + sizeof(struct iphdr) + ETH_HLEN > skb->data_end)
/* packet too small */
return 0;
ip = skb->data + ETH_HLEN;
/* access IP header fields with direct loads */
if (ip->version != 4 || ip->saddr == 0x7f000001)
return 1;
[...]
}
This solution avoids introduction of new instructions. llvm stays
the same and all JITs stay the same, but verifier has to work extra hard
to prove safety of the above program.
For XDP the direct store instructions can be allowed as well.
The skb->data is NET_IP_ALIGNED, so for common cases the verifier can check
the alignment. The complex packet parsers where packet pointer is adjusted
incrementally cannot be tracked for alignment, so allow byte access in such cases
and misaligned access on architectures that define efficient_unaligned_access
[1] https://git.kernel.org/cgit/linux/kernel/git/ast/bpf.git/?h=ld_abs_dw
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/uapi/linux/bpf.h | 2 +
kernel/bpf/core.c | 5 +
kernel/bpf/verifier.c | 441 ++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 440 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b7b0fb1292e7..406459b935a2 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -370,6 +370,8 @@ struct __sk_buff {
__u32 cb[5];
__u32 hash;
__u32 tc_classid;
+ __u32 data;
+ __u32 data_end;
};
struct bpf_tunnel_key {
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index e4248fe79513..d781b077431f 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -794,6 +794,11 @@ void __weak bpf_int_jit_compile(struct bpf_prog *prog)
{
}
+bool __weak bpf_helper_changes_skb_data(void *func)
+{
+ return false;
+}
+
/* To execute LD_ABS/LD_IND instructions __bpf_prog_run() may call
* skb_copy_bits(), so provide a weak definition of it for NET-less config.
*/
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index afeb62808902..6338c61fc2a1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1,4 +1,5 @@
/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ * Copyright (c) 2016 Facebook
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -136,13 +137,32 @@ enum bpf_reg_type {
FRAME_PTR, /* reg == frame_pointer */
PTR_TO_STACK, /* reg == frame_pointer + imm */
CONST_IMM, /* constant integer value */
+
+ /* PTR_TO_PACKET represents:
+ * skb->data
+ * skb->data + imm
+ * skb->data + (u16) var
+ * skb->data + (u16) var + imm
+ * if (range > 0) then [ptr, ptr + range - off) is safe to access
+ * if (id > 0) means that some 'var' was added
+ * if (off > 0) menas that 'imm' was added
+ */
+ PTR_TO_PACKET,
+ PTR_TO_PACKET_END, /* skb->data + headlen */
};
struct reg_state {
enum bpf_reg_type type;
union {
- /* valid when type == CONST_IMM | PTR_TO_STACK */
- long imm;
+ /* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
+ s64 imm;
+
+ /* valid when type == PTR_TO_PACKET* */
+ struct {
+ u32 id;
+ u16 off;
+ u16 range;
+ };
/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
* PTR_TO_MAP_VALUE_OR_NULL
@@ -247,6 +267,8 @@ static const char * const reg_type_str[] = {
[FRAME_PTR] = "fp",
[PTR_TO_STACK] = "fp",
[CONST_IMM] = "imm",
+ [PTR_TO_PACKET] = "pkt",
+ [PTR_TO_PACKET_END] = "pkt_end",
};
static void print_verifier_state(struct verifier_state *state)
@@ -262,7 +284,12 @@ static void print_verifier_state(struct verifier_state *state)
continue;
verbose(" R%d=%s", i, reg_type_str[t]);
if (t == CONST_IMM || t == PTR_TO_STACK)
- verbose("%ld", reg->imm);
+ verbose("%lld", reg->imm);
+ else if (t == PTR_TO_PACKET)
+ verbose("(id=%d,off=%d,r=%d)",
+ reg->id, reg->off, reg->range);
+ else if (t == UNKNOWN_VALUE && reg->imm)
+ verbose("%lld", reg->imm);
else if (t == CONST_PTR_TO_MAP || t == PTR_TO_MAP_VALUE ||
t == PTR_TO_MAP_VALUE_OR_NULL)
verbose("(ks=%d,vs=%d)",
@@ -548,6 +575,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
case PTR_TO_MAP_VALUE_OR_NULL:
case PTR_TO_STACK:
case PTR_TO_CTX:
+ case PTR_TO_PACKET:
+ case PTR_TO_PACKET_END:
case FRAME_PTR:
case CONST_PTR_TO_MAP:
return true;
@@ -647,6 +676,27 @@ static int check_map_access(struct verifier_env *env, u32 regno, int off,
return 0;
}
+#define MAX_PACKET_OFF 0xffff
+
+static int check_packet_access(struct verifier_env *env, u32 regno, int off,
+ int size)
+{
+ struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *reg = ®s[regno];
+ int linear_size = (int) reg->range - (int) reg->off;
+
+ if (linear_size < 0 || linear_size >= MAX_PACKET_OFF) {
+ verbose("verifier bug\n");
+ return -EFAULT;
+ }
+ if (off < 0 || off + size > linear_size) {
+ verbose("invalid access to packet, off=%d size=%d, allowed=%d\n",
+ off, size, linear_size);
+ return -EACCES;
+ }
+ return 0;
+}
+
/* check access to 'struct bpf_context' fields */
static int check_ctx_access(struct verifier_env *env, int off, int size,
enum bpf_access_type t)
@@ -677,6 +727,45 @@ static bool is_pointer_value(struct verifier_env *env, int regno)
}
}
+static int check_ptr_alignment(struct verifier_env *env, struct reg_state *reg,
+ int off, int size)
+{
+ if (reg->type != PTR_TO_PACKET) {
+ if (off % size != 0) {
+ verbose("misaligned access off %d size %d\n", off, size);
+ return -EACCES;
+ } else {
+ return 0;
+ }
+ }
+
+ switch (env->prog->type) {
+ case BPF_PROG_TYPE_SCHED_CLS:
+ case BPF_PROG_TYPE_SCHED_ACT:
+ break;
+ default:
+ verbose("verifier is misconfigured\n");
+ return -EACCES;
+ }
+
+ if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
+ /* misaligned access to packet is ok on x86,arm,arm64 */
+ return 0;
+
+ if (reg->id && size != 1) {
+ verbose("Unknown packet alignment. Only byte-sized access allowed\n");
+ return -EACCES;
+ }
+
+ /* skb->data is NET_IP_ALIGN-ed */
+ if ((NET_IP_ALIGN + reg->off + off) % size != 0) {
+ verbose("misaligned packet access off %d+%d+%d size %d\n",
+ NET_IP_ALIGN, reg->off, off, size);
+ return -EACCES;
+ }
+ return 0;
+}
+
/* check whether memory at (regno + off) is accessible for t = (read | write)
* if t==write, value_regno is a register which value is stored into memory
* if t==read, value_regno is a register which will receive the value from memory
@@ -698,10 +787,9 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
if (size < 0)
return size;
- if (off % size != 0) {
- verbose("misaligned access off %d size %d\n", off, size);
- return -EACCES;
- }
+ err = check_ptr_alignment(env, reg, off, size);
+ if (err)
+ return err;
if (reg->type == PTR_TO_MAP_VALUE) {
if (t == BPF_WRITE && value_regno >= 0 &&
@@ -720,8 +808,16 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
return -EACCES;
}
err = check_ctx_access(env, off, size, t);
- if (!err && t == BPF_READ && value_regno >= 0)
+ if (!err && t == BPF_READ && value_regno >= 0) {
mark_reg_unknown_value(state->regs, value_regno);
+ if (off == offsetof(struct __sk_buff, data) &&
+ env->allow_ptr_leaks)
+ /* note that reg.[id|off|range] == 0 */
+ state->regs[value_regno].type = PTR_TO_PACKET;
+ else if (off == offsetof(struct __sk_buff, data_end) &&
+ env->allow_ptr_leaks)
+ state->regs[value_regno].type = PTR_TO_PACKET_END;
+ }
} else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
if (off >= 0 || off < -MAX_BPF_STACK) {
@@ -739,11 +835,28 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
} else {
err = check_stack_read(state, off, size, value_regno);
}
+ } else if (state->regs[regno].type == PTR_TO_PACKET) {
+ if (t == BPF_WRITE) {
+ verbose("cannot write into packet\n");
+ return -EACCES;
+ }
+ err = check_packet_access(env, regno, off, size);
+ if (!err && t == BPF_READ && value_regno >= 0)
+ mark_reg_unknown_value(state->regs, value_regno);
} else {
verbose("R%d invalid mem access '%s'\n",
regno, reg_type_str[reg->type]);
return -EACCES;
}
+
+ if (!err && size <= 2 && value_regno >= 0 && env->allow_ptr_leaks &&
+ state->regs[value_regno].type == UNKNOWN_VALUE) {
+ /* 1 or 2 byte load zero-extends, determine the number of
+ * zero upper bits. Not doing it fo 4 byte load, since
+ * such values cannot be added to ptr_to_packet anyway.
+ */
+ state->regs[value_regno].imm = 64 - size * 8;
+ }
return err;
}
@@ -1001,6 +1114,29 @@ static int check_raw_mode(const struct bpf_func_proto *fn)
return count > 1 ? -EINVAL : 0;
}
+static void clear_all_pkt_pointers(struct verifier_env *env)
+{
+ struct verifier_state *state = &env->cur_state;
+ struct reg_state *regs = state->regs, *reg;
+ int i;
+
+ for (i = 0; i < MAX_BPF_REG; i++)
+ if (regs[i].type == PTR_TO_PACKET ||
+ regs[i].type == PTR_TO_PACKET_END)
+ mark_reg_unknown_value(regs, i);
+
+ for (i = 0; i < MAX_BPF_STACK; i += BPF_REG_SIZE) {
+ if (state->stack_slot_type[i] != STACK_SPILL)
+ continue;
+ reg = &state->spilled_regs[i / BPF_REG_SIZE];
+ if (reg->type != PTR_TO_PACKET &&
+ reg->type != PTR_TO_PACKET_END)
+ continue;
+ reg->type = UNKNOWN_VALUE;
+ reg->imm = 0;
+ }
+}
+
static int check_call(struct verifier_env *env, int func_id)
{
struct verifier_state *state = &env->cur_state;
@@ -1008,6 +1144,7 @@ static int check_call(struct verifier_env *env, int func_id)
struct reg_state *regs = state->regs;
struct reg_state *reg;
struct bpf_call_arg_meta meta;
+ bool changes_data;
int i, err;
/* find function prototype */
@@ -1030,6 +1167,8 @@ static int check_call(struct verifier_env *env, int func_id)
return -EINVAL;
}
+ changes_data = bpf_helper_changes_skb_data(fn->func);
+
memset(&meta, 0, sizeof(meta));
/* We only support one arg being in raw mode at the moment, which
@@ -1100,6 +1239,189 @@ static int check_call(struct verifier_env *env, int func_id)
if (err)
return err;
+ if (changes_data)
+ clear_all_pkt_pointers(env);
+ return 0;
+}
+
+static int check_packet_ptr_add(struct verifier_env *env, struct bpf_insn *insn)
+{
+ struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *dst_reg = ®s[insn->dst_reg];
+ struct reg_state *src_reg = ®s[insn->src_reg];
+ s32 imm;
+
+ if (BPF_SRC(insn->code) == BPF_K) {
+ /* pkt_ptr += imm */
+ imm = insn->imm;
+
+add_imm:
+ if (imm <= 0) {
+ verbose("addition of negative constant to packet pointer is not allowed\n");
+ return -EACCES;
+ }
+ if (imm >= MAX_PACKET_OFF ||
+ imm + dst_reg->off >= MAX_PACKET_OFF) {
+ verbose("constant %d is too large to add to packet pointer\n",
+ imm);
+ return -EACCES;
+ }
+ /* a constant was added to pkt_ptr.
+ * Remember it while keeping the same 'id'
+ */
+ dst_reg->off += imm;
+ } else {
+ if (src_reg->type == CONST_IMM) {
+ /* pkt_ptr += reg where reg is known constant */
+ imm = src_reg->imm;
+ goto add_imm;
+ }
+ /* disallow pkt_ptr += reg
+ * if reg is not uknown_value with guaranteed zero upper bits
+ * otherwise pkt_ptr may overflow and addition will become
+ * subtraction which is not allowed
+ */
+ if (src_reg->type != UNKNOWN_VALUE) {
+ verbose("cannot add '%s' to ptr_to_packet\n",
+ reg_type_str[src_reg->type]);
+ return -EACCES;
+ }
+ if (src_reg->imm < 48) {
+ verbose("cannot add integer value with %lld upper zero bits to ptr_to_packet\n",
+ src_reg->imm);
+ return -EACCES;
+ }
+ /* dst_reg stays as pkt_ptr type and since some positive
+ * integer value was added to the pointer, increment its 'id'
+ */
+ dst_reg->id++;
+
+ /* something was added to pkt_ptr, set range and off to zero */
+ dst_reg->off = 0;
+ dst_reg->range = 0;
+ }
+ return 0;
+}
+
+static int evaluate_reg_alu(struct verifier_env *env, struct bpf_insn *insn)
+{
+ struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *dst_reg = ®s[insn->dst_reg];
+ u8 opcode = BPF_OP(insn->code);
+ s64 imm_log2;
+
+ /* for type == UNKNOWN_VALUE:
+ * imm > 0 -> number of zero upper bits
+ * imm == 0 -> don't track which is the same as all bits can be non-zero
+ */
+
+ if (BPF_SRC(insn->code) == BPF_X) {
+ struct reg_state *src_reg = ®s[insn->src_reg];
+
+ if (src_reg->type == UNKNOWN_VALUE && src_reg->imm > 0 &&
+ dst_reg->imm && opcode == BPF_ADD) {
+ /* dreg += sreg
+ * where both have zero upper bits. Adding them
+ * can only result making one more bit non-zero
+ * in the larger value.
+ * Ex. 0xffff (imm=48) + 1 (imm=63) = 0x10000 (imm=47)
+ * 0xffff (imm=48) + 0xffff = 0x1fffe (imm=47)
+ */
+ dst_reg->imm = min(dst_reg->imm, src_reg->imm);
+ dst_reg->imm--;
+ return 0;
+ }
+ if (src_reg->type == CONST_IMM && src_reg->imm > 0 &&
+ dst_reg->imm && opcode == BPF_ADD) {
+ /* dreg += sreg
+ * where dreg has zero upper bits and sreg is const.
+ * Adding them can only result making one more bit
+ * non-zero in the larger value.
+ */
+ imm_log2 = __ilog2_u64((long long)src_reg->imm);
+ dst_reg->imm = min(dst_reg->imm, 63 - imm_log2);
+ dst_reg->imm--;
+ return 0;
+ }
+ /* all other cases non supported yet, just mark dst_reg */
+ dst_reg->imm = 0;
+ return 0;
+ }
+
+ /* sign extend 32-bit imm into 64-bit to make sure that
+ * negative values occupy bit 63. Note ilog2() would have
+ * been incorrect, since sizeof(insn->imm) == 4
+ */
+ imm_log2 = __ilog2_u64((long long)insn->imm);
+
+ if (dst_reg->imm && opcode == BPF_LSH) {
+ /* reg <<= imm
+ * if reg was a result of 2 byte load, then its imm == 48
+ * which means that upper 48 bits are zero and shifting this reg
+ * left by 4 would mean that upper 44 bits are still zero
+ */
+ dst_reg->imm -= insn->imm;
+ } else if (dst_reg->imm && opcode == BPF_MUL) {
+ /* reg *= imm
+ * if multiplying by 14 subtract 4
+ * This is conservative calculation of upper zero bits.
+ * It's not trying to special case insn->imm == 1 or 0 cases
+ */
+ dst_reg->imm -= imm_log2 + 1;
+ } else if (opcode == BPF_AND) {
+ /* reg &= imm */
+ dst_reg->imm = 63 - imm_log2;
+ } else if (dst_reg->imm && opcode == BPF_ADD) {
+ /* reg += imm */
+ dst_reg->imm = min(dst_reg->imm, 63 - imm_log2);
+ dst_reg->imm--;
+ } else if (opcode == BPF_RSH) {
+ /* reg >>= imm
+ * which means that after right shift, upper bits will be zero
+ * note that verifier already checked that
+ * 0 <= imm < 64 for shift insn
+ */
+ dst_reg->imm += insn->imm;
+ if (unlikely(dst_reg->imm > 64))
+ /* some dumb code did:
+ * r2 = *(u32 *)mem;
+ * r2 >>= 32;
+ * and all bits are zero now */
+ dst_reg->imm = 64;
+ } else {
+ /* all other alu ops, means that we don't know what will
+ * happen to the value, mark it with unknown number of zero bits
+ */
+ dst_reg->imm = 0;
+ }
+
+ if (dst_reg->imm < 0) {
+ /* all 64 bits of the register can contain non-zero bits
+ * and such value cannot be added to ptr_to_packet, since it
+ * may overflow, mark it as unknown to avoid further eval
+ */
+ dst_reg->imm = 0;
+ }
+ return 0;
+}
+
+static int evaluate_reg_imm_alu(struct verifier_env *env, struct bpf_insn *insn)
+{
+ struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *dst_reg = ®s[insn->dst_reg];
+ struct reg_state *src_reg = ®s[insn->src_reg];
+ u8 opcode = BPF_OP(insn->code);
+
+ /* dst_reg->type == CONST_IMM here, simulate execution of 'add' insn.
+ * Don't care about overflow or negative values, just add them
+ */
+ if (opcode == BPF_ADD && BPF_SRC(insn->code) == BPF_K)
+ dst_reg->imm += insn->imm;
+ else if (opcode == BPF_ADD && BPF_SRC(insn->code) == BPF_X &&
+ src_reg->type == CONST_IMM)
+ dst_reg->imm += src_reg->imm;
+ else
+ mark_reg_unknown_value(regs, insn->dst_reg);
return 0;
}
@@ -1245,6 +1567,21 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
dst_reg->type = PTR_TO_STACK;
dst_reg->imm = insn->imm;
return 0;
+ } else if (opcode == BPF_ADD &&
+ BPF_CLASS(insn->code) == BPF_ALU64 &&
+ dst_reg->type == PTR_TO_PACKET) {
+ /* ptr_to_packet += K|X */
+ return check_packet_ptr_add(env, insn);
+ } else if (BPF_CLASS(insn->code) == BPF_ALU64 &&
+ dst_reg->type == UNKNOWN_VALUE &&
+ env->allow_ptr_leaks) {
+ /* unknown += K|X */
+ return evaluate_reg_alu(env, insn);
+ } else if (BPF_CLASS(insn->code) == BPF_ALU64 &&
+ dst_reg->type == CONST_IMM &&
+ env->allow_ptr_leaks) {
+ /* reg_imm += K|X */
+ return evaluate_reg_imm_alu(env, insn);
} else if (is_pointer_value(env, insn->dst_reg)) {
verbose("R%d pointer arithmetic prohibited\n",
insn->dst_reg);
@@ -1263,6 +1600,34 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
return 0;
}
+static void find_good_pkt_pointers(struct verifier_env *env,
+ struct reg_state *dst_reg)
+{
+ struct verifier_state *state = &env->cur_state;
+ struct reg_state *regs = state->regs, *reg;
+ int i;
+ /* r2 = r3;
+ * r2 += 8
+ * if (r2 > pkt_end) goto somewhere
+ * r2 == dst_reg, pkt_end == src_reg,
+ * r2=pkt(id=n,off=8,r=0)
+ * r3=pkt(id=n,off=0,r=0)
+ * find register r3 and mark its range as r3=pkt(id=n,off=0,r=8)
+ * so that range of bytes [r3, r3 + 8) is safe to access
+ */
+ for (i = 0; i < MAX_BPF_REG; i++)
+ if (regs[i].type == PTR_TO_PACKET && regs[i].id == dst_reg->id)
+ regs[i].range = dst_reg->off;
+
+ for (i = 0; i < MAX_BPF_STACK; i += BPF_REG_SIZE) {
+ if (state->stack_slot_type[i] != STACK_SPILL)
+ continue;
+ reg = &state->spilled_regs[i / BPF_REG_SIZE];
+ if (reg->type == PTR_TO_PACKET && reg->id == dst_reg->id)
+ reg->range = dst_reg->off;
+ }
+}
+
static int check_cond_jmp_op(struct verifier_env *env,
struct bpf_insn *insn, int *insn_idx)
{
@@ -1346,6 +1711,10 @@ static int check_cond_jmp_op(struct verifier_env *env,
regs[insn->dst_reg].type = CONST_IMM;
regs[insn->dst_reg].imm = 0;
}
+ } else if (BPF_SRC(insn->code) == BPF_X && opcode == BPF_JGT &&
+ dst_reg->type == PTR_TO_PACKET &&
+ regs[insn->src_reg].type == PTR_TO_PACKET_END) {
+ find_good_pkt_pointers(env, dst_reg);
} else if (is_pointer_value(env, insn->dst_reg)) {
verbose("R%d pointer comparison prohibited\n", insn->dst_reg);
return -EACCES;
@@ -1685,6 +2054,58 @@ err_free:
return ret;
}
+/* the following conditions reduce the number of explored insns
+ * from ~140k to ~80k for ultra large programs that use a lot of ptr_to_packet
+ */
+static bool compare_ptrs_to_packet(struct reg_state *old, struct reg_state *cur)
+{
+ if (old->id != cur->id)
+ return false;
+
+ /* old ptr_to_packet is more conservative, since it allows smaller
+ * range. Ex:
+ * old(off=0,r=10) is equal to cur(off=0,r=20), because
+ * old(off=0,r=10) means that with range=10 the verifier proceeded
+ * further and found no issues with the program. Now we're in the same
+ * spot with cur(off=0,r=20), so we're safe too, since anything further
+ * will only be looking at most 10 bytes after this pointer.
+ */
+ if (old->off == cur->off && old->range < cur->range)
+ return true;
+
+ /* old(off=20,r=10) is equal to cur(off=22,re=22 or 5 or 0)
+ * since both cannot be used for packet access and safe(old)
+ * pointer has smaller off that could be used for further
+ * 'if (ptr > data_end)' check
+ * Ex:
+ * old(off=20,r=10) and cur(off=22,r=22) and cur(off=22,r=0) mean
+ * that we cannot access the packet.
+ * The safe range is:
+ * [ptr, ptr + range - off)
+ * so whenever off >=range, it means no safe bytes from this pointer.
+ * When comparing old->off <= cur->off, it means that older code
+ * went with smaller offset and that offset was later
+ * used to figure out the safe range after 'if (ptr > data_end)' check
+ * Say, 'old' state was explored like:
+ * ... R3(off=0, r=0)
+ * R4 = R3 + 20
+ * ... now R4(off=20,r=0) <-- here
+ * if (R4 > data_end)
+ * ... R4(off=20,r=20), R3(off=0,r=20) and R3 can be used to access.
+ * ... the code further went all the way to bpf_exit.
+ * Now the 'cur' state at the mark 'here' has R4(off=30,r=0).
+ * old_R4(off=20,r=0) equal to cur_R4(off=30,r=0), since if the verifier
+ * goes further, such cur_R4 will give larger safe packet range after
+ * 'if (R4 > data_end)' and all further insn were already good with r=20,
+ * so they will be good with r=30 and we can prune the search.
+ */
+ if (old->off <= cur->off &&
+ old->off >= old->range && cur->off >= cur->range)
+ return true;
+
+ return false;
+}
+
/* compare two verifier states
*
* all states stored in state_list are known to be valid, since
@@ -1727,6 +2148,10 @@ static bool states_equal(struct verifier_state *old, struct verifier_state *cur)
(rold->type == UNKNOWN_VALUE && rcur->type != NOT_INIT))
continue;
+ if (rold->type == PTR_TO_PACKET && rcur->type == PTR_TO_PACKET &&
+ compare_ptrs_to_packet(rold, rcur))
+ continue;
+
return false;
}
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 6/7] samples/bpf: add 'pointer to packet' tests
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
parse_simple.c - packet parser exapmle with single length check that
filters out udp packets for port 9
parse_varlen.c - variable length parser that understand multiple vlan headers,
ipip, ipip6 and ip options to filter out udp or tcp packets on port 9.
The packet is parsed layer by layer with multitple length checks.
parse_ldabs.c - classic style of packet parsing using LD_ABS instruction.
Same functionality as parse_simple.
simple = 24.1Mpps per core
varlen = 22.7Mpps
ldabs = 21.4Mpps
Parser with LD_ABS instructions is slower than full direct access parser
which does more packet accesses and checks.
These examples demonstrate the choice bpf program authors can make between
flexibility of the parser vs speed.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
samples/bpf/Makefile | 2 +
samples/bpf/parse_ldabs.c | 41 ++++++++++++
samples/bpf/parse_simple.c | 48 ++++++++++++++
samples/bpf/parse_varlen.c | 153 ++++++++++++++++++++++++++++++++++++++++++++
samples/bpf/test_cls_bpf.sh | 37 +++++++++++
5 files changed, 281 insertions(+)
create mode 100644 samples/bpf/parse_ldabs.c
create mode 100644 samples/bpf/parse_simple.c
create mode 100644 samples/bpf/parse_varlen.c
create mode 100755 samples/bpf/test_cls_bpf.sh
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 66897e61232c..0bf2478cb7df 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -60,6 +60,7 @@ always += spintest_kern.o
always += map_perf_test_kern.o
always += test_overhead_tp_kern.o
always += test_overhead_kprobe_kern.o
+always += parse_varlen.o parse_simple.o parse_ldabs.o
HOSTCFLAGS += -I$(objtree)/usr/include
@@ -120,4 +121,5 @@ $(src)/*.c: verify_target_bpf
$(obj)/%.o: $(src)/%.c
$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
-D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
+ -Wno-compare-distinct-pointer-types \
-O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
diff --git a/samples/bpf/parse_ldabs.c b/samples/bpf/parse_ldabs.c
new file mode 100644
index 000000000000..d17550198d06
--- /dev/null
+++ b/samples/bpf/parse_ldabs.c
@@ -0,0 +1,41 @@
+/* Copyright (c) 2016 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/in.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+#define DEFAULT_PKTGEN_UDP_PORT 9
+#define IP_MF 0x2000
+#define IP_OFFSET 0x1FFF
+
+static inline int ip_is_fragment(struct __sk_buff *ctx, __u64 nhoff)
+{
+ return load_half(ctx, nhoff + offsetof(struct iphdr, frag_off))
+ & (IP_MF | IP_OFFSET);
+}
+
+SEC("ldabs")
+int handle_ingress(struct __sk_buff *skb)
+{
+ __u64 troff = ETH_HLEN + sizeof(struct iphdr);
+
+ if (load_half(skb, offsetof(struct ethhdr, h_proto)) != ETH_P_IP)
+ return 0;
+ if (load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol)) != IPPROTO_UDP ||
+ load_byte(skb, ETH_HLEN) != 0x45)
+ return 0;
+ if (ip_is_fragment(skb, ETH_HLEN))
+ return 0;
+ if (load_half(skb, troff + offsetof(struct udphdr, dest)) == DEFAULT_PKTGEN_UDP_PORT)
+ return TC_ACT_SHOT;
+ return 0;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/parse_simple.c b/samples/bpf/parse_simple.c
new file mode 100644
index 000000000000..cf2511c33905
--- /dev/null
+++ b/samples/bpf/parse_simple.c
@@ -0,0 +1,48 @@
+/* Copyright (c) 2016 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/in.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <uapi/linux/bpf.h>
+#include <net/ip.h>
+#include "bpf_helpers.h"
+
+#define DEFAULT_PKTGEN_UDP_PORT 9
+
+/* copy of 'struct ethhdr' without __packed */
+struct eth_hdr {
+ unsigned char h_dest[ETH_ALEN];
+ unsigned char h_source[ETH_ALEN];
+ unsigned short h_proto;
+};
+
+SEC("simple")
+int handle_ingress(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ struct eth_hdr *eth = data;
+ struct iphdr *iph = data + sizeof(*eth);
+ struct udphdr *udp = data + sizeof(*eth) + sizeof(*iph);
+ void *data_end = (void *)(long)skb->data_end;
+
+ /* single length check */
+ if (data + sizeof(*eth) + sizeof(*iph) + sizeof(*udp) > data_end)
+ return 0;
+
+ if (eth->h_proto != htons(ETH_P_IP))
+ return 0;
+ if (iph->protocol != IPPROTO_UDP || iph->ihl != 5)
+ return 0;
+ if (ip_is_fragment(iph))
+ return 0;
+ if (udp->dest == htons(DEFAULT_PKTGEN_UDP_PORT))
+ return TC_ACT_SHOT;
+ return 0;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/parse_varlen.c b/samples/bpf/parse_varlen.c
new file mode 100644
index 000000000000..edab34dce79b
--- /dev/null
+++ b/samples/bpf/parse_varlen.c
@@ -0,0 +1,153 @@
+/* Copyright (c) 2016 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <linux/if_ether.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/in.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
+#include <uapi/linux/bpf.h>
+#include <net/ip.h>
+#include "bpf_helpers.h"
+
+#define DEFAULT_PKTGEN_UDP_PORT 9
+#define DEBUG 0
+
+static int tcp(void *data, uint64_t tp_off, void *data_end)
+{
+ struct tcphdr *tcp = data + tp_off;
+
+ if (tcp + 1 > data_end)
+ return 0;
+ if (tcp->dest == htons(80) || tcp->source == htons(80))
+ return TC_ACT_SHOT;
+ return 0;
+}
+
+static int udp(void *data, uint64_t tp_off, void *data_end)
+{
+ struct udphdr *udp = data + tp_off;
+
+ if (udp + 1 > data_end)
+ return 0;
+ if (udp->dest == htons(DEFAULT_PKTGEN_UDP_PORT) ||
+ udp->source == htons(DEFAULT_PKTGEN_UDP_PORT)) {
+ if (DEBUG) {
+ char fmt[] = "udp port 9 indeed\n";
+
+ bpf_trace_printk(fmt, sizeof(fmt));
+ }
+ return TC_ACT_SHOT;
+ }
+ return 0;
+}
+
+static int parse_ipv4(void *data, uint64_t nh_off, void *data_end)
+{
+ struct iphdr *iph;
+ uint64_t ihl_len;
+
+ iph = data + nh_off;
+ if (iph + 1 > data_end)
+ return 0;
+
+ if (ip_is_fragment(iph))
+ return 0;
+ ihl_len = iph->ihl * 4;
+
+ if (iph->protocol == IPPROTO_IPIP) {
+ iph = data + nh_off + ihl_len;
+ if (iph + 1 > data_end)
+ return 0;
+ ihl_len += iph->ihl * 4;
+ }
+
+ if (iph->protocol == IPPROTO_TCP)
+ return tcp(data, nh_off + ihl_len, data_end);
+ else if (iph->protocol == IPPROTO_UDP)
+ return udp(data, nh_off + ihl_len, data_end);
+ return 0;
+}
+
+static int parse_ipv6(void *data, uint64_t nh_off, void *data_end)
+{
+ struct ipv6hdr *ip6h;
+ struct iphdr *iph;
+ uint64_t ihl_len = sizeof(struct ipv6hdr);
+ uint64_t nexthdr;
+
+ ip6h = data + nh_off;
+ if (ip6h + 1 > data_end)
+ return 0;
+
+ nexthdr = ip6h->nexthdr;
+
+ if (nexthdr == IPPROTO_IPIP) {
+ iph = data + nh_off + ihl_len;
+ if (iph + 1 > data_end)
+ return 0;
+ ihl_len += iph->ihl * 4;
+ nexthdr = iph->protocol;
+ } else if (nexthdr == IPPROTO_IPV6) {
+ ip6h = data + nh_off + ihl_len;
+ if (ip6h + 1 > data_end)
+ return 0;
+ ihl_len += sizeof(struct ipv6hdr);
+ nexthdr = ip6h->nexthdr;
+ }
+
+ if (nexthdr == IPPROTO_TCP)
+ return tcp(data, nh_off + ihl_len, data_end);
+ else if (nexthdr == IPPROTO_UDP)
+ return udp(data, nh_off + ihl_len, data_end);
+ return 0;
+}
+
+struct vlan_hdr {
+ uint16_t h_vlan_TCI;
+ uint16_t h_vlan_encapsulated_proto;
+};
+
+SEC("varlen")
+int handle_ingress(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ struct ethhdr *eth = data;
+ void *data_end = (void *)(long)skb->data_end;
+ uint64_t h_proto, nh_off;
+
+ nh_off = sizeof(*eth);
+ if (data + nh_off > data_end)
+ return 0;
+
+ h_proto = eth->h_proto;
+
+ if (h_proto == ETH_P_8021Q || h_proto == ETH_P_8021AD) {
+ struct vlan_hdr *vhdr;
+
+ vhdr = data + nh_off;
+ nh_off += sizeof(struct vlan_hdr);
+ if (data + nh_off > data_end)
+ return 0;
+ h_proto = vhdr->h_vlan_encapsulated_proto;
+ }
+ if (h_proto == ETH_P_8021Q || h_proto == ETH_P_8021AD) {
+ struct vlan_hdr *vhdr;
+
+ vhdr = data + nh_off;
+ nh_off += sizeof(struct vlan_hdr);
+ if (data + nh_off > data_end)
+ return 0;
+ h_proto = vhdr->h_vlan_encapsulated_proto;
+ }
+ if (h_proto == htons(ETH_P_IP))
+ return parse_ipv4(data, nh_off, data_end);
+ else if (h_proto == htons(ETH_P_IPV6))
+ return parse_ipv6(data, nh_off, data_end);
+ return 0;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/test_cls_bpf.sh b/samples/bpf/test_cls_bpf.sh
new file mode 100755
index 000000000000..0365d5ee512c
--- /dev/null
+++ b/samples/bpf/test_cls_bpf.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+
+function pktgen {
+ ../pktgen/pktgen_bench_xmit_mode_netif_receive.sh -i $IFC -s 64 \
+ -m 90:e2:ba:ff:ff:ff -d 192.168.0.1 -t 4
+ local dropped=`tc -s qdisc show dev $IFC | tail -3 | awk '/drop/{print $7}'`
+ if [ "$dropped" == "0," ]; then
+ echo "FAIL"
+ else
+ echo "Successfully filtered " $dropped " packets"
+ fi
+}
+
+function test {
+ echo -n "Loading bpf program '$2'... "
+ tc qdisc add dev $IFC clsact
+ tc filter add dev $IFC ingress bpf da obj $1 sec $2
+ local status=$?
+ if [ $status -ne 0 ]; then
+ echo "FAIL"
+ else
+ echo "ok"
+ pktgen
+ fi
+ tc qdisc del dev $IFC clsact
+}
+
+IFC=test_veth
+
+ip link add name $IFC type veth peer name pair_$IFC
+ip link set $IFC up
+ip link set pair_$IFC up
+
+test ./parse_simple.o simple
+test ./parse_varlen.o varlen
+test ./parse_ldabs.o ldabs
+ip link del dev $IFC
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 3/7] bpf: improve verifier state equivalence
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
since UNKNOWN_VALUE type is weaker than CONST_IMM we can un-teach
verifier its recognition of constants in conditional branches
without affecting safety.
Ex:
if (reg == 123) {
.. here verifier was marking reg->type as CONST_IMM
instead keep reg as UNKNOWN_VALUE
}
Two verifier states with UNKNOWN_VALUE are equivalent, whereas
CONST_IMM_X != CONST_IMM_Y, since CONST_IMM is used for stack range
verification and other cases.
So help search pruning by marking registers as UNKNOWN_VALUE
where possible instead of CONST_IMM.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/verifier.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6338c61fc2a1..84bff68cf80e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1704,12 +1704,11 @@ static int check_cond_jmp_op(struct verifier_env *env,
*/
regs[insn->dst_reg].type = PTR_TO_MAP_VALUE;
/* branch targer cannot access it, since reg == 0 */
- other_branch->regs[insn->dst_reg].type = CONST_IMM;
- other_branch->regs[insn->dst_reg].imm = 0;
+ mark_reg_unknown_value(other_branch->regs,
+ insn->dst_reg);
} else {
other_branch->regs[insn->dst_reg].type = PTR_TO_MAP_VALUE;
- regs[insn->dst_reg].type = CONST_IMM;
- regs[insn->dst_reg].imm = 0;
+ mark_reg_unknown_value(regs, insn->dst_reg);
}
} else if (BPF_SRC(insn->code) == BPF_X && opcode == BPF_JGT &&
dst_reg->type == PTR_TO_PACKET &&
@@ -1718,22 +1717,6 @@ static int check_cond_jmp_op(struct verifier_env *env,
} else if (is_pointer_value(env, insn->dst_reg)) {
verbose("R%d pointer comparison prohibited\n", insn->dst_reg);
return -EACCES;
- } else if (BPF_SRC(insn->code) == BPF_K &&
- (opcode == BPF_JEQ || opcode == BPF_JNE)) {
-
- if (opcode == BPF_JEQ) {
- /* detect if (R == imm) goto
- * and in the target state recognize that R = imm
- */
- other_branch->regs[insn->dst_reg].type = CONST_IMM;
- other_branch->regs[insn->dst_reg].imm = insn->imm;
- } else {
- /* detect if (R != imm) goto
- * and in the fall-through state recognize that R = imm
- */
- regs[insn->dst_reg].type = CONST_IMM;
- regs[insn->dst_reg].imm = insn->imm;
- }
}
if (log_level)
print_verifier_state(&env->cur_state);
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 1/7] bpf: cleanup verifier code
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
cleanup verifier code and prepare it for addition of "pointer to packet" logic
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/verifier.c | 100 ++++++++++++++++++++++++++------------------------
1 file changed, 53 insertions(+), 47 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 63554b6d4e25..afeb62808902 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -249,28 +249,30 @@ static const char * const reg_type_str[] = {
[CONST_IMM] = "imm",
};
-static void print_verifier_state(struct verifier_env *env)
+static void print_verifier_state(struct verifier_state *state)
{
+ struct reg_state *reg;
enum bpf_reg_type t;
int i;
for (i = 0; i < MAX_BPF_REG; i++) {
- t = env->cur_state.regs[i].type;
+ reg = &state->regs[i];
+ t = reg->type;
if (t == NOT_INIT)
continue;
verbose(" R%d=%s", i, reg_type_str[t]);
if (t == CONST_IMM || t == PTR_TO_STACK)
- verbose("%ld", env->cur_state.regs[i].imm);
+ verbose("%ld", reg->imm);
else if (t == CONST_PTR_TO_MAP || t == PTR_TO_MAP_VALUE ||
t == PTR_TO_MAP_VALUE_OR_NULL)
verbose("(ks=%d,vs=%d)",
- env->cur_state.regs[i].map_ptr->key_size,
- env->cur_state.regs[i].map_ptr->value_size);
+ reg->map_ptr->key_size,
+ reg->map_ptr->value_size);
}
for (i = 0; i < MAX_BPF_STACK; i += BPF_REG_SIZE) {
- if (env->cur_state.stack_slot_type[i] == STACK_SPILL)
+ if (state->stack_slot_type[i] == STACK_SPILL)
verbose(" fp%d=%s", -MAX_BPF_STACK + i,
- reg_type_str[env->cur_state.spilled_regs[i / BPF_REG_SIZE].type]);
+ reg_type_str[state->spilled_regs[i / BPF_REG_SIZE].type]);
}
verbose("\n");
}
@@ -686,10 +688,11 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
int value_regno)
{
struct verifier_state *state = &env->cur_state;
+ struct reg_state *reg = &state->regs[regno];
int size, err = 0;
- if (state->regs[regno].type == PTR_TO_STACK)
- off += state->regs[regno].imm;
+ if (reg->type == PTR_TO_STACK)
+ off += reg->imm;
size = bpf_size_to_bytes(bpf_size);
if (size < 0)
@@ -700,7 +703,7 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
return -EACCES;
}
- if (state->regs[regno].type == PTR_TO_MAP_VALUE) {
+ if (reg->type == PTR_TO_MAP_VALUE) {
if (t == BPF_WRITE && value_regno >= 0 &&
is_pointer_value(env, value_regno)) {
verbose("R%d leaks addr into map\n", value_regno);
@@ -710,7 +713,7 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown_value(state->regs, value_regno);
- } else if (state->regs[regno].type == PTR_TO_CTX) {
+ } else if (reg->type == PTR_TO_CTX) {
if (t == BPF_WRITE && value_regno >= 0 &&
is_pointer_value(env, value_regno)) {
verbose("R%d leaks addr into ctx\n", value_regno);
@@ -720,8 +723,7 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown_value(state->regs, value_regno);
- } else if (state->regs[regno].type == FRAME_PTR ||
- state->regs[regno].type == PTR_TO_STACK) {
+ } else if (reg->type == FRAME_PTR || reg->type == PTR_TO_STACK) {
if (off >= 0 || off < -MAX_BPF_STACK) {
verbose("invalid stack off=%d size=%d\n", off, size);
return -EACCES;
@@ -739,7 +741,7 @@ static int check_mem_access(struct verifier_env *env, u32 regno, int off,
}
} else {
verbose("R%d invalid mem access '%s'\n",
- regno, reg_type_str[state->regs[regno].type]);
+ regno, reg_type_str[reg->type]);
return -EACCES;
}
return err;
@@ -1104,7 +1106,7 @@ static int check_call(struct verifier_env *env, int func_id)
/* check validity of 32-bit and 64-bit arithmetic operations */
static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
{
- struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *regs = env->cur_state.regs, *dst_reg;
u8 opcode = BPF_OP(insn->code);
int err;
@@ -1193,8 +1195,6 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
} else { /* all other ALU ops: and, sub, xor, add, ... */
- bool stack_relative = false;
-
if (BPF_SRC(insn->code) == BPF_X) {
if (insn->imm != 0 || insn->off != 0) {
verbose("BPF_ALU uses reserved fields\n");
@@ -1232,11 +1232,19 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
}
}
+ /* check dest operand */
+ err = check_reg_arg(regs, insn->dst_reg, DST_OP_NO_MARK);
+ if (err)
+ return err;
+
+ dst_reg = ®s[insn->dst_reg];
+
/* pattern match 'bpf_add Rx, imm' instruction */
if (opcode == BPF_ADD && BPF_CLASS(insn->code) == BPF_ALU64 &&
- regs[insn->dst_reg].type == FRAME_PTR &&
- BPF_SRC(insn->code) == BPF_K) {
- stack_relative = true;
+ dst_reg->type == FRAME_PTR && BPF_SRC(insn->code) == BPF_K) {
+ dst_reg->type = PTR_TO_STACK;
+ dst_reg->imm = insn->imm;
+ return 0;
} else if (is_pointer_value(env, insn->dst_reg)) {
verbose("R%d pointer arithmetic prohibited\n",
insn->dst_reg);
@@ -1248,15 +1256,8 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
return -EACCES;
}
- /* check dest operand */
- err = check_reg_arg(regs, insn->dst_reg, DST_OP);
- if (err)
- return err;
-
- if (stack_relative) {
- regs[insn->dst_reg].type = PTR_TO_STACK;
- regs[insn->dst_reg].imm = insn->imm;
- }
+ /* mark dest operand */
+ mark_reg_unknown_value(regs, insn->dst_reg);
}
return 0;
@@ -1265,7 +1266,7 @@ static int check_alu_op(struct verifier_env *env, struct bpf_insn *insn)
static int check_cond_jmp_op(struct verifier_env *env,
struct bpf_insn *insn, int *insn_idx)
{
- struct reg_state *regs = env->cur_state.regs;
+ struct reg_state *regs = env->cur_state.regs, *dst_reg;
struct verifier_state *other_branch;
u8 opcode = BPF_OP(insn->code);
int err;
@@ -1303,11 +1304,12 @@ static int check_cond_jmp_op(struct verifier_env *env,
if (err)
return err;
+ dst_reg = ®s[insn->dst_reg];
+
/* detect if R == 0 where R was initialized to zero earlier */
if (BPF_SRC(insn->code) == BPF_K &&
(opcode == BPF_JEQ || opcode == BPF_JNE) &&
- regs[insn->dst_reg].type == CONST_IMM &&
- regs[insn->dst_reg].imm == insn->imm) {
+ dst_reg->type == CONST_IMM && dst_reg->imm == insn->imm) {
if (opcode == BPF_JEQ) {
/* if (imm == imm) goto pc+off;
* only follow the goto, ignore fall-through
@@ -1329,9 +1331,8 @@ static int check_cond_jmp_op(struct verifier_env *env,
/* detect if R == 0 where R is returned value from bpf_map_lookup_elem() */
if (BPF_SRC(insn->code) == BPF_K &&
- insn->imm == 0 && (opcode == BPF_JEQ ||
- opcode == BPF_JNE) &&
- regs[insn->dst_reg].type == PTR_TO_MAP_VALUE_OR_NULL) {
+ insn->imm == 0 && (opcode == BPF_JEQ || opcode == BPF_JNE) &&
+ dst_reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
if (opcode == BPF_JEQ) {
/* next fallthrough insn can access memory via
* this register
@@ -1366,7 +1367,7 @@ static int check_cond_jmp_op(struct verifier_env *env,
}
}
if (log_level)
- print_verifier_state(env);
+ print_verifier_state(&env->cur_state);
return 0;
}
@@ -1444,14 +1445,14 @@ static int check_ld_abs(struct verifier_env *env, struct bpf_insn *insn)
int i, err;
if (!may_access_skb(env->prog->type)) {
- verbose("BPF_LD_ABS|IND instructions not allowed for this program type\n");
+ verbose("BPF_LD_[ABS|IND] instructions not allowed for this program type\n");
return -EINVAL;
}
if (insn->dst_reg != BPF_REG_0 || insn->off != 0 ||
BPF_SIZE(insn->code) == BPF_DW ||
(mode == BPF_ABS && insn->src_reg != BPF_REG_0)) {
- verbose("BPF_LD_ABS uses reserved fields\n");
+ verbose("BPF_LD_[ABS|IND] uses reserved fields\n");
return -EINVAL;
}
@@ -1712,17 +1713,21 @@ err_free:
*/
static bool states_equal(struct verifier_state *old, struct verifier_state *cur)
{
+ struct reg_state *rold, *rcur;
int i;
for (i = 0; i < MAX_BPF_REG; i++) {
- if (memcmp(&old->regs[i], &cur->regs[i],
- sizeof(old->regs[0])) != 0) {
- if (old->regs[i].type == NOT_INIT ||
- (old->regs[i].type == UNKNOWN_VALUE &&
- cur->regs[i].type != NOT_INIT))
- continue;
- return false;
- }
+ rold = &old->regs[i];
+ rcur = &cur->regs[i];
+
+ if (memcmp(rold, rcur, sizeof(*rold)) == 0)
+ continue;
+
+ if (rold->type == NOT_INIT ||
+ (rold->type == UNKNOWN_VALUE && rcur->type != NOT_INIT))
+ continue;
+
+ return false;
}
for (i = 0; i < MAX_BPF_STACK; i++) {
@@ -1844,7 +1849,7 @@ static int do_check(struct verifier_env *env)
if (log_level && do_print_state) {
verbose("\nfrom %d to %d:", prev_insn_idx, insn_idx);
- print_verifier_state(env);
+ print_verifier_state(&env->cur_state);
do_print_state = false;
}
@@ -2056,6 +2061,7 @@ process_bpf_exit:
insn_idx++;
}
+ verbose("processed %d insns\n", insn_processed);
return 0;
}
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 4/7] bpf: wire in data and data_end for cls_act_bpf
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
allow cls_bpf and act_bpf programs access skb->data and skb->data_end pointers.
The bpf helpers that change skb->data need to update data_end pointer as well.
The verifier checks that programs always reload data, data_end pointers
after calls to such bpf helpers.
We cannot add 'data_end' pointer to struct qdisc_skb_cb directly,
since it's embedded as-is by infiniband ipoib, so wrapper struct is needed.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/filter.h | 16 ++++++++++++++++
net/core/filter.c | 51 ++++++++++++++++++++++++++++++++++++++++++++------
net/sched/act_bpf.c | 2 ++
net/sched/cls_bpf.c | 2 ++
4 files changed, 65 insertions(+), 6 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 43aa1f8855c7..ec1411c89105 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -352,6 +352,22 @@ struct sk_filter {
#define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
+struct bpf_skb_data_end {
+ struct qdisc_skb_cb qdisc_cb;
+ void *data_end;
+};
+
+/* compute the linear packet data range [data, data_end) which
+ * will be accessed by cls_bpf and act_bpf programs
+ */
+static inline void bpf_compute_data_end(struct sk_buff *skb)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ BUILD_BUG_ON(sizeof(*cb) > FIELD_SIZEOF(struct sk_buff, cb));
+ cb->data_end = skb->data + skb_headlen(skb);
+}
+
static inline u8 *bpf_skb_cb(struct sk_buff *skb)
{
/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/net/core/filter.c b/net/core/filter.c
index 218e5de8c402..71c2a1f473ad 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1344,6 +1344,21 @@ struct bpf_scratchpad {
static DEFINE_PER_CPU(struct bpf_scratchpad, bpf_sp);
+static inline int bpf_try_make_writable(struct sk_buff *skb,
+ unsigned int write_len)
+{
+ int err;
+
+ if (!skb_cloned(skb))
+ return 0;
+ if (skb_clone_writable(skb, write_len))
+ return 0;
+ err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (!err)
+ bpf_compute_data_end(skb);
+ return err;
+}
+
static u64 bpf_skb_store_bytes(u64 r1, u64 r2, u64 r3, u64 r4, u64 flags)
{
struct bpf_scratchpad *sp = this_cpu_ptr(&bpf_sp);
@@ -1366,7 +1381,7 @@ static u64 bpf_skb_store_bytes(u64 r1, u64 r2, u64 r3, u64 r4, u64 flags)
*/
if (unlikely((u32) offset > 0xffff || len > sizeof(sp->buff)))
return -EFAULT;
- if (unlikely(skb_try_make_writable(skb, offset + len)))
+ if (unlikely(bpf_try_make_writable(skb, offset + len)))
return -EFAULT;
ptr = skb_header_pointer(skb, offset, len, sp->buff);
@@ -1444,7 +1459,7 @@ static u64 bpf_l3_csum_replace(u64 r1, u64 r2, u64 from, u64 to, u64 flags)
return -EINVAL;
if (unlikely((u32) offset > 0xffff))
return -EFAULT;
- if (unlikely(skb_try_make_writable(skb, offset + sizeof(sum))))
+ if (unlikely(bpf_try_make_writable(skb, offset + sizeof(sum))))
return -EFAULT;
ptr = skb_header_pointer(skb, offset, sizeof(sum), &sum);
@@ -1499,7 +1514,7 @@ static u64 bpf_l4_csum_replace(u64 r1, u64 r2, u64 from, u64 to, u64 flags)
return -EINVAL;
if (unlikely((u32) offset > 0xffff))
return -EFAULT;
- if (unlikely(skb_try_make_writable(skb, offset + sizeof(sum))))
+ if (unlikely(bpf_try_make_writable(skb, offset + sizeof(sum))))
return -EFAULT;
ptr = skb_header_pointer(skb, offset, sizeof(sum), &sum);
@@ -1699,12 +1714,15 @@ static u64 bpf_skb_vlan_push(u64 r1, u64 r2, u64 vlan_tci, u64 r4, u64 r5)
{
struct sk_buff *skb = (struct sk_buff *) (long) r1;
__be16 vlan_proto = (__force __be16) r2;
+ int ret;
if (unlikely(vlan_proto != htons(ETH_P_8021Q) &&
vlan_proto != htons(ETH_P_8021AD)))
vlan_proto = htons(ETH_P_8021Q);
- return skb_vlan_push(skb, vlan_proto, vlan_tci);
+ ret = skb_vlan_push(skb, vlan_proto, vlan_tci);
+ bpf_compute_data_end(skb);
+ return ret;
}
const struct bpf_func_proto bpf_skb_vlan_push_proto = {
@@ -1720,8 +1738,11 @@ EXPORT_SYMBOL_GPL(bpf_skb_vlan_push_proto);
static u64 bpf_skb_vlan_pop(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
{
struct sk_buff *skb = (struct sk_buff *) (long) r1;
+ int ret;
- return skb_vlan_pop(skb);
+ ret = skb_vlan_pop(skb);
+ bpf_compute_data_end(skb);
+ return ret;
}
const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
@@ -2066,8 +2087,12 @@ static bool __is_valid_access(int off, int size, enum bpf_access_type type)
static bool sk_filter_is_valid_access(int off, int size,
enum bpf_access_type type)
{
- if (off == offsetof(struct __sk_buff, tc_classid))
+ switch (off) {
+ case offsetof(struct __sk_buff, tc_classid):
+ case offsetof(struct __sk_buff, data):
+ case offsetof(struct __sk_buff, data_end):
return false;
+ }
if (type == BPF_WRITE) {
switch (off) {
@@ -2215,6 +2240,20 @@ static u32 bpf_net_convert_ctx_access(enum bpf_access_type type, int dst_reg,
*insn++ = BPF_LDX_MEM(BPF_H, dst_reg, src_reg, ctx_off);
break;
+ case offsetof(struct __sk_buff, data):
+ *insn++ = BPF_LDX_MEM(bytes_to_bpf_size(FIELD_SIZEOF(struct sk_buff, data)),
+ dst_reg, src_reg,
+ offsetof(struct sk_buff, data));
+ break;
+
+ case offsetof(struct __sk_buff, data_end):
+ ctx_off -= offsetof(struct __sk_buff, data_end);
+ ctx_off += offsetof(struct sk_buff, cb);
+ ctx_off += offsetof(struct bpf_skb_data_end, data_end);
+ *insn++ = BPF_LDX_MEM(bytes_to_bpf_size(sizeof(void *)),
+ dst_reg, src_reg, ctx_off);
+ break;
+
case offsetof(struct __sk_buff, tc_index):
#ifdef CONFIG_NET_SCHED
BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, tc_index) != 2);
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 4fd703362563..c7123e01c2ca 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -53,9 +53,11 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
filter = rcu_dereference(prog->filter);
if (at_ingress) {
__skb_push(skb, skb->mac_len);
+ bpf_compute_data_end(skb);
filter_res = BPF_PROG_RUN(filter, skb);
__skb_pull(skb, skb->mac_len);
} else {
+ bpf_compute_data_end(skb);
filter_res = BPF_PROG_RUN(filter, skb);
}
rcu_read_unlock();
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 425fe6a0eda3..7b342c779da7 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -96,9 +96,11 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
if (at_ingress) {
/* It is safe to push/pull even if skb_shared() */
__skb_push(skb, skb->mac_len);
+ bpf_compute_data_end(skb);
filter_res = BPF_PROG_RUN(prog->filter, skb);
__skb_pull(skb, skb->mac_len);
} else {
+ bpf_compute_data_end(skb);
filter_res = BPF_PROG_RUN(prog->filter, skb);
}
--
2.8.0
^ permalink raw reply related
* [PATCH net-next 5/7] bpf: add documentation for 'direct packet access'
From: Alexei Starovoitov @ 2016-05-06 2:49 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, netdev, kernel-team
In-Reply-To: <1462502955-1731797-1-git-send-email-ast@fb.com>
explain how verifier checks safety of packet access
and update email addresses.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
Documentation/networking/filter.txt | 85 ++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index 96da119a47e7..6aef0b5f3bc7 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -1095,6 +1095,87 @@ all use cases.
See details of eBPF verifier in kernel/bpf/verifier.c
+Direct packet access
+--------------------
+In cls_bpf and act_bpf programs the verifier allows direct access to the packet
+data via skb->data and skb->data_end pointers.
+Ex:
+1: r4 = *(u32 *)(r1 +80) /* load skb->data_end */
+2: r3 = *(u32 *)(r1 +76) /* load skb->data */
+3: r5 = r3
+4: r5 += 14
+5: if r5 > r4 goto pc+16
+R1=ctx R3=pkt(id=0,off=0,r=14) R4=pkt_end R5=pkt(id=0,off=14,r=14) R10=fp
+6: r0 = *(u16 *)(r3 +12) /* access 12 and 13 bytes of the packet */
+
+this 2byte load from the packet is safe to do, since the program author
+did check 'if (skb->data + 14 > skb->data_end) goto err' at insn #5 which
+means that in the fall-through case the register R3 (which points to skb->data)
+has at least 14 directly accessible bytes. The verifier marks it
+as R3=pkt(id=0,off=0,r=14).
+id=0 means that no additional variables were added to the register.
+off=0 means that no additional constants were added.
+r=14 is the range of safe access which means that bytes [R3, R3 + 14) are ok.
+Note that R5 is marked as R5=pkt(id=0,off=14,r=14). It also points
+to the packet data, but constant 14 was added to the register, so
+it now points to 'skb->data + 14' and accessible range is [R5, R5 + 14 - 14)
+which is zero bytes.
+
+More complex packet access may look like:
+ R0=imm1 R1=ctx R3=pkt(id=0,off=0,r=14) R4=pkt_end R5=pkt(id=0,off=14,r=14) R10=fp
+ 6: r0 = *(u8 *)(r3 +7) /* load 7th byte from the packet */
+ 7: r4 = *(u8 *)(r3 +12)
+ 8: r4 *= 14
+ 9: r3 = *(u32 *)(r1 +76) /* load skb->data */
+10: r3 += r4
+11: r2 = r1
+12: r2 <<= 48
+13: r2 >>= 48
+14: r3 += r2
+15: r2 = r3
+16: r2 += 8
+17: r1 = *(u32 *)(r1 +80) /* load skb->data_end */
+18: if r2 > r1 goto pc+2
+ R0=inv56 R1=pkt_end R2=pkt(id=2,off=8,r=8) R3=pkt(id=2,off=0,r=8) R4=inv52 R5=pkt(id=0,off=14,r=14) R10=fp
+19: r1 = *(u8 *)(r3 +4)
+The state of the register R3 is R3=pkt(id=2,off=0,r=8)
+id=2 means that two 'r3 += rX' instructions were seen, so r3 points to some
+offset within a packet and since the program author did
+'if (r3 + 8 > r1) goto err' at insn #18, the safe range is [R3, R3 + 8).
+The verifier only allows 'add' operation on packet registers. Any other
+operation will set the register state to 'unknown_value' and it won't be
+available for direct packet access.
+Operation 'r3 += rX' may overflow and become less than original skb->data,
+therefore the verifier has to prevent that. So it tracks the number of
+upper zero bits in all 'uknown_value' registers, so when it sees
+'r3 += rX' instruction and rX is more than 16-bit value, it will error as:
+"cannot add integer value with N upper zero bits to ptr_to_packet"
+Ex. after insn 'r4 = *(u8 *)(r3 +12)' (insn #7 above) the state of r4 is
+R4=inv56 which means that upper 56 bits on the register are guaranteed
+to be zero. After insn 'r4 *= 14' the state becomes R4=inv52, since
+multiplying 8-bit value by constant 14 will keep upper 52 bits as zero.
+Similarly 'r2 >>= 48' will make R2=inv48, since the shift is not sign
+extending. This logic is implemented in evaluate_reg_alu() function.
+
+The end result is that bpf program author can access packet directly
+using normal C code as:
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct eth_hdr *eth = data;
+ struct iphdr *iph = data + sizeof(*eth);
+ struct udphdr *udp = data + sizeof(*eth) + sizeof(*iph);
+
+ if (data + sizeof(*eth) + sizeof(*iph) + sizeof(*udp) > data_end)
+ return 0;
+ if (eth->h_proto != htons(ETH_P_IP))
+ return 0;
+ if (iph->protocol != IPPROTO_UDP || iph->ihl != 5)
+ return 0;
+ if (udp->dest == 53 || udp->source == 9)
+ ...;
+which makes such programs easier to write comparing to LD_ABS insn
+and significantly faster.
+
eBPF maps
---------
'maps' is a generic storage of different types for sharing data between kernel
@@ -1293,5 +1374,5 @@ to give potential BPF hackers or security auditors a better overview of
the underlying architecture.
Jay Schulist <jschlst@samba.org>
-Daniel Borkmann <dborkman@redhat.com>
-Alexei Starovoitov <ast@plumgrid.com>
+Daniel Borkmann <daniel@iogearbox.net>
+Alexei Starovoitov <ast@kernel.org>
--
2.8.0
^ permalink raw reply related
* Re: [PATCH v2 net-next] net/mlx4: Avoid wrong virtual mappings
From: David Miller @ 2016-05-06 3:22 UTC (permalink / raw)
To: hagaya
Cc: dledford, linux-rdma, netdev, okaya, timur, eli, ogerlitz, eranbe,
yishaih, talal, saeedm
In-Reply-To: <1462362615-20238-1-git-send-email-hagaya@mellanox.com>
From: Haggai Abramovsky <hagaya@mellanox.com>
Date: Wed, 4 May 2016 14:50:15 +0300
> The dma_alloc_coherent() function returns a virtual address which can
> be used for coherent access to the underlying memory. On some
> architectures, like arm64, undefined behavior results if this memory is
> also accessed via virtual mappings that are not coherent. Because of
> their undefined nature, operations like virt_to_page() return garbage
> when passed virtual addresses obtained from dma_alloc_coherent(). Any
> subsequent mappings via vmap() of the garbage page values are unusable
> and result in bad things like bus errors (synchronous aborts in ARM64
> speak).
>
> The mlx4 driver contains code that does the equivalent of:
> vmap(virt_to_page(dma_alloc_coherent)), this results in an OOPs when the
> device is opened.
>
> Prevent Ethernet driver to run this problematic code by forcing it to
> allocate contiguous memory. As for the Infiniband driver, at first we
> are trying to allocate contiguous memory, but in case of failure roll
> back to work with fragmented memory.
>
> Signed-off-by: Haggai Abramovsky <hagaya@mellanox.com>
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Reported-by: David Daney <david.daney@cavium.com>
> Tested-by: Sinan Kaya <okaya@codeaurora.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net/mlx4_en: Fix endianness bug in IPV6 csum calculation
From: David Miller @ 2016-05-06 3:27 UTC (permalink / raw)
To: tariqt; +Cc: netdev, danielj
In-Reply-To: <1462363233-8598-1-git-send-email-tariqt@mellanox.com>
From: Tariq Toukan <tariqt@mellanox.com>
Date: Wed, 4 May 2016 15:00:33 +0300
> From: Daniel Jurgens <danielj@mellanox.com>
>
> Use htons instead of unconditionally byte swapping nexthdr. On a little
> endian systems shifting the byte is correct behavior, but it results in
> incorrect csums on big endian architectures.
>
> Fixes: f8c6455bb04b ('net/mlx4_en: Extend checksum offloading by CHECKSUM COMPLETE')
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Reviewed-by: Carol Soto <clsoto@us.ibm.com>
> Tested-by: Carol Soto <clsoto@us.ibm.com>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH] VSOCK: do not disconnect socket when peer has shutdown SEND only
From: David Miller @ 2016-05-06 3:32 UTC (permalink / raw)
To: ian.campbell; +Cc: netdev, stefanha, imbrenda, acking, dtor, jhansen, aditr
In-Reply-To: <1462368113-5493-1-git-send-email-ian.campbell@docker.com>
From: Ian Campbell <ian.campbell@docker.com>
Date: Wed, 4 May 2016 14:21:53 +0100
> The peer may be expecting a reply having sent a request and then done a
> shutdown(SHUT_WR), so tearing down the whole socket at this point seems
> wrong and breaks for me with a client which does a SHUT_WR.
>
> Looking at other socket family's stream_recvmsg callbacks doing a shutdown
> here does not seem to be the norm and removing it does not seem to have
> had any adverse effects that I can see.
>
> I'm using Stefan's RFC virtio transport patches, I'm unsure of the impact
> on the vmci transport.
>
> Signed-off-by: Ian Campbell <ian.campbell@docker.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] net: bridge: fix old ioctl unlocked net device walk
From: David Miller @ 2016-05-06 3:33 UTC (permalink / raw)
To: nikolay; +Cc: netdev, roopa, stephen
In-Reply-To: <1462371525-26786-1-git-send-email-nikolay@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Wed, 4 May 2016 16:18:45 +0200
> get_bridge_ifindices() is used from the old "deviceless" bridge ioctl
> calls which aren't called with rtnl held. The comment above says that it is
> called with rtnl but that is not really the case.
> Here's a sample output from a test ASSERT_RTNL() which I put in
> get_bridge_ifindices and executed "brctl show":
...
> Since it only reads bridge ifindices, we can use rcu to safely walk the net
> device list. Also remove the wrong rtnl comment above.
>
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> ---
> AFAICT this has been present since the beginning of git history thus no
> fixes tag.
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH v9 net-next 3/7] openvswitch: add support to push and pop mpls for layer3 packets
From: Simon Horman @ 2016-05-06 4:33 UTC (permalink / raw)
To: pravin shelar; +Cc: Linux Kernel Network Developers, ovs dev
In-Reply-To: <CAOrHB_A__kJiHOTRNd3Vj4sF2dcEw5w33_PpJLR_hosvbKG52A@mail.gmail.com>
On Thu, May 05, 2016 at 10:35:52AM -0700, pravin shelar wrote:
> On Wed, May 4, 2016 at 12:36 AM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > Allow push and pop mpls actions to act on layer 3 packets by teaching
> > them not to access non-existent L2 headers of such packets.
> >
> > Signed-off-by: Simon Horman <simon.horman@netronome.com>
> > ---
> > v9
> > * New Patch
> > ---
> > net/openvswitch/actions.c | 22 ++++++++++++++--------
> > 1 file changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> > index 879185fe183f..fe885a89f501 100644
> > --- a/net/openvswitch/actions.c
> > +++ b/net/openvswitch/actions.c
> > @@ -160,8 +160,10 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> >
> > skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
> >
> > - hdr = eth_hdr(skb);
> > - hdr->h_proto = mpls->mpls_ethertype;
> > + if (skb->mac_len) {
> can you move hdr definition to this block?
Sure, sorry for overlooking that.
> > + hdr = eth_hdr(skb);
> > + hdr->h_proto = mpls->mpls_ethertype;
> > + }
> >
> We need to update skb checksum here. This bug is not related to your
> patch. since you are changing this code can you also fix this?
Yes, of course. Would something like this broken out into a patch earlier
in the series resolve the problem you see? If so it looks like a similar
fix is also needed for pop_mpls().
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 63d29263d51a..89ad0027420a 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -161,6 +160,14 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
struct ethhdr *hdr;
hdr = eth_hdr(skb);
+
+ if (skb->ip_summed == CHECKSUM_COMPLETE) {
+ __be16 diff[] = { ~(hdr->h_proto), mpls->mpls_ethertype };
+
+ skb->csum = ~csum_partial((char *)diff, sizeof(diff),
+ ~skb->csum);
+ }
+
hdr->h_proto = mpls->mpls_ethertype;
}
>
> > if (!skb->inner_protocol)
> > skb_set_inner_protocol(skb, skb->protocol);
> > @@ -174,7 +176,6 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> > static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
> > const __be16 ethertype)
> > {
> > - struct ethhdr *hdr;
> > int err;
> >
^ permalink raw reply related
* Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support
From: Simon Horman @ 2016-05-06 5:57 UTC (permalink / raw)
To: pravin shelar
Cc: Linux Kernel Network Developers, ovs dev, Lorand Jakab,
Thomas Morin, Jiri Benc
In-Reply-To: <CAOrHB_AMC4x=NnLTpGbXdW31Vp0f1R3VoaD9ksST5EesrdGsug@mail.gmail.com>
[CC Jiri Benc]
On Thu, May 05, 2016 at 10:37:08AM -0700, pravin shelar wrote:
> On Wed, May 4, 2016 at 12:36 AM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > From: Lorand Jakab <lojakab@cisco.com>
> >
> > Implementation of the pop_eth and push_eth actions in the kernel, and
> > layer 3 flow support.
> >
> > This doesn't actually do anything yet as no layer 2 tunnel ports are
> > supported yet. The original patch by Lorand was against the Open vSwtich
> > tree which has L2 LISP tunnels but that is not supported in mainline Linux.
> > I (Simon) plan to follow up with support for non-TEB GRE ports based on
> > work by Thomas Morin.
> >
> > Cc: Thomas Morin <thomas.morin@orange.com>
> > Signed-off-by: Lorand Jakab <lojakab@cisco.com>
> > Signed-off-by: Simon Horman <simon.horman@netronome.com>
> >
> > ---
>
> ...
>
> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> > index 0ea128eeeab2..6e174ea5f2bb 100644
> > --- a/net/openvswitch/flow.c
> > +++ b/net/openvswitch/flow.c
> > @@ -468,28 +468,31 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
> >
> > skb_reset_mac_header(skb);
> >
> > - /* Link layer. We are guaranteed to have at least the 14 byte Ethernet
> > - * header in the linear data area.
> > - */
> > - eth = eth_hdr(skb);
> > - ether_addr_copy(key->eth.src, eth->h_source);
> > - ether_addr_copy(key->eth.dst, eth->h_dest);
> > + /* Link layer. */
> > + if (key->phy.is_layer3) {
> > + key->eth.tci = 0;
> > + key->eth.type = skb->protocol;
> > + } else {
> > + eth = eth_hdr(skb);
> > + ether_addr_copy(key->eth.src, eth->h_source);
> > + ether_addr_copy(key->eth.dst, eth->h_dest);
> >
> > - __skb_pull(skb, 2 * ETH_ALEN);
> > - /* We are going to push all headers that we pull, so no need to
> > - * update skb->csum here.
> > - */
> > + __skb_pull(skb, 2 * ETH_ALEN);
> > + /* We are going to push all headers that we pull, so no need to
> > + * update skb->csum here.
> > + */
> >
> > - key->eth.tci = 0;
> > - if (skb_vlan_tag_present(skb))
> > - key->eth.tci = htons(skb->vlan_tci);
> > - else if (eth->h_proto == htons(ETH_P_8021Q))
> > - if (unlikely(parse_vlan(skb, key)))
> > - return -ENOMEM;
> > + key->eth.tci = 0;
> > + if (skb_vlan_tag_present(skb))
> > + key->eth.tci = htons(skb->vlan_tci);
> > + else if (eth->h_proto == htons(ETH_P_8021Q))
> > + if (unlikely(parse_vlan(skb, key)))
> > + return -ENOMEM;
> >
> > - key->eth.type = parse_ethertype(skb);
> > - if (unlikely(key->eth.type == htons(0)))
> > - return -ENOMEM;
> > + key->eth.type = parse_ethertype(skb);
> > + if (unlikely(key->eth.type == htons(0)))
> > + return -ENOMEM;
> > + }
> >
> > skb_reset_network_header(skb);
> > skb_reset_mac_len(skb);
> > @@ -696,11 +699,23 @@ int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
> > int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
> > struct sk_buff *skb, struct sw_flow_key *key)
> > {
> > + bool is_layer3 = false;
> > + bool is_teb = false;
> is_layer3 and is_teb are mutually exclusive, so can't we use single
> boolean here?
Sure, I can do something like the following if you prefer.
To my mind it makes things a bit less readable. But I don't feel
strongly about this.
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index d320c2657627..fc92cf542101 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -701,7 +701,6 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
struct sk_buff *skb, struct sw_flow_key *key)
{
bool is_layer3 = false;
- bool is_teb = false;
int err;
/* Extract metadata from packet. */
@@ -709,13 +708,9 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
key->tun_proto = ip_tunnel_info_af(tun_info);
memcpy(&key->tun_key, &tun_info->key, sizeof(key->tun_key));
- if (OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER) {
- if (skb->protocol == htons(ETH_P_TEB))
- is_teb = true;
- else
- is_layer3 = true;
- }
-
+ if (OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER &&
+ skb->protocol != htons(ETH_P_TEB))
+ is_layer3 = true;
if (tun_info->options_len) {
BUILD_BUG_ON((1 << (sizeof(tun_info->options_len) *
@@ -746,10 +741,12 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
if (err < 0)
return err;
- if (is_teb)
- skb->protocol = key->eth.type;
- else if (is_layer3)
- key->eth.type = skb->protocol;
+ if (tun_info && OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER) {
+ if (is_layer3)
+ key->eth.type = skb->protocol;
+ else
+ skb->protocol = key->eth.type;
+ }
return err;
}
> > + int err;
> > +
> > /* Extract metadata from packet. */
> > if (tun_info) {
> > key->tun_proto = ip_tunnel_info_af(tun_info);
> > memcpy(&key->tun_key, &tun_info->key, sizeof(key->tun_key));
> >
> > + if (OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER) {
> > + if (skb->protocol == htons(ETH_P_TEB))
> > + is_teb = true;
> > + else
> > + is_layer3 = true;
> > + }
> > +
> On transmit side you are using mac_len to detect l3 packet, why not do
> same while extracting the key?
Unfortunately mac_len can't be relied on here, emprically it has the same
value (28 in my tests) for both the TEB and layer3 case above.
Perhaps that could be changed by futher enhancements in the tunneling code
but I think things are symetric as they stand:
* On recieve skb->protocol can be read to distinguish TEB and layer3 packets
* On transmit skb->protocol should be set to distinguish TEB and layer3 packets
^ permalink raw reply related
* Re: [PATCH v9 net-next 7/7] openvswitch: use ipgre tunnel rather than gretap tunnel
From: Simon Horman @ 2016-05-06 6:54 UTC (permalink / raw)
To: pravin shelar; +Cc: Linux Kernel Network Developers, ovs dev, Jiri Benc
In-Reply-To: <CAOrHB_BAFuZjoUgza9n8g1pborjCsYAC0DU4cXsff_9q9Cp+Fg@mail.gmail.com>
[CC Jiri Benc]
On Thu, May 05, 2016 at 02:45:15PM -0700, pravin shelar wrote:
> On Wed, May 4, 2016 at 12:36 AM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > This allows GRE tunnels to send and receive both
> > layer 2 packets (packets with an ethernet header) and
> > layer 3 packets (packets without an ethernet header).
> >
> > Signed-off-by: Simon Horman <simon.horman@netronome.com>
> > ---
> > v9
> > New Patch
> > ---
> > include/net/gre.h | 4 ++--
> > net/ipv4/ip_gre.c | 8 ++++----
> > net/openvswitch/vport-gre.c | 4 ++--
> > net/openvswitch/vport-netdev.c | 12 +++++++++++-
> > net/openvswitch/vport-netdev.h | 1 +
> > 5 files changed, 20 insertions(+), 9 deletions(-)
> >
> ...
> ...
> > diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
> > index f003225de994..b1aa02904ae4 100644
> > --- a/net/openvswitch/vport-gre.c
> > +++ b/net/openvswitch/vport-gre.c
> > @@ -60,7 +60,7 @@ static struct vport *gre_tnl_create(const struct vport_parms *parms)
> > return vport;
> >
> > rtnl_lock();
> > - dev = gretap_fb_dev_create(net, parms->name, NET_NAME_USER);
> > + dev = gre_fb_dev_create(net, parms->name, NET_NAME_USER);
> > if (IS_ERR(dev)) {
> > rtnl_unlock();
> > ovs_vport_free(vport);
> > @@ -87,7 +87,7 @@ static struct vport *gre_create(const struct vport_parms *parms)
> > static struct vport_ops ovs_gre_vport_ops = {
> > .type = OVS_VPORT_TYPE_GRE,
> > .create = gre_create,
> > - .send = ovs_netdev_send_tap,
> > + .send = ovs_netdev_send_raw_tun,
> > .destroy = ovs_netdev_tunnel_destroy,
> > };
> >
>
> This trick of using vport-send only works in case of compat tunnel
> device mode. But in normal case the LWT interface allows us to use net
> devices for tunnel traffic. So you need some sort of mechanism to
> handle l3 only packets on vport-netdev.
Thanks. Clearly I did not consider that and clearly I should have.
On the kernel side I wonder if something like the following (untested
with tunnel vport-netdev) would resolve the problem.
I think that some work probably also needs to be done in user-space to make it
aware of how to handle l3 only packets on such vports.
diff --git a/net/openvswitch/vport-geneve.c b/net/openvswitch/vport-geneve.c
index 906ae9c58c2e..7a06e19f5279 100644
--- a/net/openvswitch/vport-geneve.c
+++ b/net/openvswitch/vport-geneve.c
@@ -116,7 +116,7 @@ static struct vport_ops ovs_geneve_vport_ops = {
.create = geneve_create,
.destroy = ovs_netdev_tunnel_destroy,
.get_options = geneve_get_options,
- .send = ovs_netdev_send_tap,
+ .send = ovs_netdev_send,
};
static int __init ovs_geneve_tnl_init(void)
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index b1aa02904ae4..c1cab9dd392f 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -87,7 +87,7 @@ static struct vport *gre_create(const struct vport_parms *parms)
static struct vport_ops ovs_gre_vport_ops = {
.type = OVS_VPORT_TYPE_GRE,
.create = gre_create,
- .send = ovs_netdev_send_raw_tun,
+ .send = ovs_netdev_send,
.destroy = ovs_netdev_tunnel_destroy,
};
diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index e6a2718204a8..0ffe07721336 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -197,25 +197,20 @@ void ovs_netdev_tunnel_destroy(struct vport *vport)
}
EXPORT_SYMBOL_GPL(ovs_netdev_tunnel_destroy);
-int ovs_netdev_send_tap(struct sk_buff *skb)
+int ovs_netdev_send(struct sk_buff *skb)
{
- /* Only send L2 packets */
- if (skb->mac_len)
- return dev_queue_xmit(skb);
+ struct net_device *dev = skb->dev;
- kfree_skb(skb);
- return -EINVAL;
-}
-EXPORT_SYMBOL_GPL(ovs_netdev_send_tap);
-
-int ovs_netdev_send_raw_tun(struct sk_buff *skb)
-{
- if (skb->mac_len)
- skb->protocol = ntohs(ETH_P_TEB);
+ if (dev->type != ARPHRD_ETHER && skb->mac_len) {
+ skb->protocol = htons(ETH_P_TEB);
+ } else if (dev->type == ARPHRD_ETHER && !skb->mac_len) {
+ kfree_skb(skb);
+ return -EINVAL;
+ }
return dev_queue_xmit(skb);
}
-EXPORT_SYMBOL_GPL(ovs_netdev_send_raw_tun);
+EXPORT_SYMBOL_GPL(ovs_netdev_send);
/* Returns null if this device is not attached to a datapath. */
struct vport *ovs_netdev_get_vport(struct net_device *dev)
@@ -231,7 +226,7 @@ static struct vport_ops ovs_netdev_vport_ops = {
.type = OVS_VPORT_TYPE_NETDEV,
.create = netdev_create,
.destroy = netdev_destroy,
- .send = ovs_netdev_send_tap,
+ .send = ovs_netdev_send,
};
int __init ovs_netdev_init(void)
diff --git a/net/openvswitch/vport-netdev.h b/net/openvswitch/vport-netdev.h
index ae59c02ba6a9..637b14a9963c 100644
--- a/net/openvswitch/vport-netdev.h
+++ b/net/openvswitch/vport-netdev.h
@@ -34,6 +34,5 @@ void ovs_netdev_exit(void);
void ovs_netdev_tunnel_destroy(struct vport *vport);
-int ovs_netdev_send_tap(struct sk_buff *skb);
-int ovs_netdev_send_raw_tun(struct sk_buff *skb);
+int ovs_netdev_send(struct sk_buff *skb);
#endif /* vport_netdev.h */
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index 009a4aab505a..13f11ad7e35a 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -153,7 +153,7 @@ static struct vport_ops ovs_vxlan_netdev_vport_ops = {
.create = vxlan_create,
.destroy = ovs_netdev_tunnel_destroy,
.get_options = vxlan_get_options,
- .send = ovs_netdev_send_tap,
+ .send = ovs_netdev_send,
};
static int __init ovs_vxlan_tnl_init(void)
^ permalink raw reply related
* [net-next 01/11] i40e/i40evf: Refactor tunnel interpretation
From: Jeff Kirsher @ 2016-05-06 7:03 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <1462518228-30527-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
Refactor the interpretation of a tunnel. This removes
some code and lets us start using the hardware's parsing.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 13 ++++++-------
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 13 ++++++-------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 2765d7e..dab733c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1392,7 +1392,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
u16 rx_ptype)
{
struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
- bool ipv4, ipv6, ipv4_tunnel, ipv6_tunnel;
+ bool ipv4, ipv6, tunnel = false;
skb->ip_summed = CHECKSUM_NONE;
@@ -1441,14 +1441,13 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
* doesn't make it a hard requirement so if we have validated the
* inner checksum report CHECKSUM_UNNECESSARY.
*/
-
- ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
- (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
- ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
- (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+ if (decoded.inner_prot & (I40E_RX_PTYPE_INNER_PROT_TCP |
+ I40E_RX_PTYPE_INNER_PROT_UDP |
+ I40E_RX_PTYPE_INNER_PROT_SCTP))
+ tunnel = true;
skb->ip_summed = CHECKSUM_UNNECESSARY;
- skb->csum_level = ipv4_tunnel || ipv6_tunnel;
+ skb->csum_level = tunnel ? 1 : 0;
return;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index ede8dfc..a37a3f3 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -864,7 +864,7 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
u16 rx_ptype)
{
struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
- bool ipv4, ipv6, ipv4_tunnel, ipv6_tunnel;
+ bool ipv4, ipv6, tunnel = false;
skb->ip_summed = CHECKSUM_NONE;
@@ -913,14 +913,13 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
* doesn't make it a hard requirement so if we have validated the
* inner checksum report CHECKSUM_UNNECESSARY.
*/
-
- ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
- (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
- ipv6_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
- (rx_ptype <= I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+ if (decoded.inner_prot & (I40E_RX_PTYPE_INNER_PROT_TCP |
+ I40E_RX_PTYPE_INNER_PROT_UDP |
+ I40E_RX_PTYPE_INNER_PROT_SCTP))
+ tunnel = true;
skb->ip_summed = CHECKSUM_UNNECESSARY;
- skb->csum_level = ipv4_tunnel || ipv6_tunnel;
+ skb->csum_level = tunnel ? 1 : 0;
return;
--
2.5.5
^ permalink raw reply related
* [net-next 00/11][pull request] 40GbE Intel Wired LAN Driver Updates 2016-05-05
From: Jeff Kirsher @ 2016-05-06 7:03 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann, jogreene, john.ronciak
This series contains updates to i40e and i40evf.
The theme behind this series is code reduction, yeah! Jesse provides
most of the changes starting with a refactor of the interpretation of
a tunnel which lets us start using the hardware's parsing. Removed
the packet split receive routine and ancillary code in preparation
for the Rx-refactor. The refactor of the receive routine,
aligns the receive routine with the one in ixgbe which was highly
optimized. The hardware supports a 16 byte descriptor for receive,
but the driver was never using it in production. There was no performance
benefit to the real driver of 16 byte descriptors, so drop a whole lot
of complexity while getting rid of the code. Fixed a bug where while
changing the number of descriptors using ethtool, the driver did not
test the limits of the system memory before permanently assuming it
would be able to get receive buffer memory.
Mitch fixes a memory leak of one page each time the driver is opened by
allocating the correct number of receive buffers and do not fiddle with
next_to_use in the VF driver.
Arnd Bergmann fixed a indentation issue by adding the appropriate
curly braces in i40e_vc_config_promiscuous_mode_msg().
Julia Lawall fixed an issue found by Coccinelle, where i40e_client_ops
structure can be const since it is never modified.
The following are changes since commit 035cd6ba53eff060760c4f4d11339fcc916a967c:
MAINTAINERS: Cleanup Intel Wired LAN maintainers list
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE
Arnd Bergmann (1):
i40e: fix misleading indentation
Jesse Brandeburg (8):
i40e/i40evf: Refactor tunnel interpretation
i40e: Drop packet split receive routine
i40e/i40evf: Remove reference to ring->dtype
i40e: Refactor receive routine
i40evf: Drop packet split receive routine
i40evf: refactor receive routine
i40e/i40evf: Remove unused hardware receive descriptor code
i40e: Test memory before ethtool alloc succeeds
Julia Lawall (1):
i40e: constify i40e_client_ops structure
Mitch Williams (1):
i40evf: Allocate Rx buffers properly
drivers/infiniband/hw/i40iw/i40iw_main.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e.h | 11 +-
drivers/net/ethernet/intel/i40e/i40e_client.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 31 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 54 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 73 +-
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 968 ++++++++++-----------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 69 +-
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 5 +-
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 930 ++++++++++----------
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 69 +-
drivers/net/ethernet/intel/i40evf/i40evf.h | 7 -
drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c | 65 --
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 34 +-
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 4 -
15 files changed, 1062 insertions(+), 1262 deletions(-)
--
2.5.5
^ permalink raw reply
* [net-next 03/11] i40e/i40evf: Remove reference to ring->dtype
From: Jeff Kirsher @ 2016-05-06 7:03 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <1462518228-30527-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
As part of the rx-refactor, the dtype variable in the i40e_ring
struct is no longer used, so remove it.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 8 ++------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 1 -
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 1 -
3 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
index f119a74..c0a01e0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
@@ -268,10 +268,9 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
rx_ring->queue_index,
rx_ring->reg_idx);
dev_info(&pf->pdev->dev,
- " rx_rings[%i]: rx_hdr_len = %d, rx_buf_len = %d, dtype = %d\n",
+ " rx_rings[%i]: rx_hdr_len = %d, rx_buf_len = %d\n",
i, rx_ring->rx_hdr_len,
- rx_ring->rx_buf_len,
- rx_ring->dtype);
+ rx_ring->rx_buf_len);
dev_info(&pf->pdev->dev,
" rx_rings[%i]: next_to_use = %d, next_to_clean = %d, ring_active = %i\n",
i,
@@ -326,9 +325,6 @@ static void i40e_dbg_dump_vsi_seid(struct i40e_pf *pf, int seid)
tx_ring->queue_index,
tx_ring->reg_idx);
dev_info(&pf->pdev->dev,
- " tx_rings[%i]: dtype = %d\n",
- i, tx_ring->dtype);
- dev_info(&pf->pdev->dev,
" tx_rings[%i]: next_to_use = %d, next_to_clean = %d, ring_active = %i\n",
i,
tx_ring->next_to_use,
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 5a2d0fe..03e21d9 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -282,7 +282,6 @@ struct i40e_ring {
u16 reg_idx; /* HW register index of the ring */
u16 rx_hdr_len;
u16 rx_buf_len;
- u8 dtype;
#define I40E_RX_DTYPE_NO_SPLIT 0
#define I40E_RX_DTYPE_HEADER_SPLIT 1
#define I40E_RX_DTYPE_SPLIT_ALWAYS 2
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index 54b52e8..3b3f976 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -280,7 +280,6 @@ struct i40e_ring {
u16 reg_idx; /* HW register index of the ring */
u16 rx_hdr_len;
u16 rx_buf_len;
- u8 dtype;
#define I40E_RX_DTYPE_NO_SPLIT 0
#define I40E_RX_DTYPE_HEADER_SPLIT 1
#define I40E_RX_DTYPE_SPLIT_ALWAYS 2
--
2.5.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox