* [PATCH 0/3] ibmvnic: Fix bugs and memory leaks
From: Thomas Falcon @ 2018-05-16 20:49 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
This is a small patch series fixing up some bugs and memory leaks
in the ibmvnic driver. The first fix frees up previously allocated
memory that should be freed in case of an error. The second fixes
a reset case that was failing due to TX/RX queue IRQ's being
erroneously disabled without being enabled again. The final patch
fixes incorrect reallocated of statistics buffers during a device
reset, resulting in loss of statistics information and a memory leak.
Thomas Falcon (3):
ibmvnic: Free coherent DMA memory if FW map failed
ibmvnic: Fix non-fatal firmware error reset
ibmvnic: Fix statistics buffers memory leak
drivers/net/ethernet/ibm/ibmvnic.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 2/3] ibmvnic: Fix non-fatal firmware error reset
From: Thomas Falcon @ 2018-05-16 20:49 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
In-Reply-To: <1526503745-14421-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
It is not necessary to disable interrupt lines here during a reset
to handle a non-fatal firmware error. Move that call within the code
block that handles the other cases that do require interrupts to be
disabled and re-enabled.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 9e08917..1b9c22f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1822,9 +1822,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (rc)
return rc;
}
+ ibmvnic_disable_irqs(adapter);
}
-
- ibmvnic_disable_irqs(adapter);
adapter->state = VNIC_CLOSED;
if (reset_state == VNIC_CLOSED)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/3] ibmvnic: Free coherent DMA memory if FW map failed
From: Thomas Falcon @ 2018-05-16 20:49 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
In-Reply-To: <1526503745-14421-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
If the firmware map fails for whatever reason, remember to free
up the memory after.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 6e8d6a6..9e08917 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -192,6 +192,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
if (adapter->fw_done_rc) {
dev_err(dev, "Couldn't map long term buffer,rc = %d\n",
adapter->fw_done_rc);
+ dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
return -1;
}
return 0;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3] ibmvnic: Fix statistics buffers memory leak
From: Thomas Falcon @ 2018-05-16 20:49 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont, Thomas Falcon
In-Reply-To: <1526503745-14421-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
Move initialization of statistics buffers from ibmvnic_init function
into ibmvnic_probe. In the current state, ibmvnic_init will be called
again during a device reset, resulting in the allocation of new
buffers without freeing the old ones.
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 1b9c22f..4bb4646 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4586,14 +4586,6 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
release_crq_queue(adapter);
}
- rc = init_stats_buffers(adapter);
- if (rc)
- return rc;
-
- rc = init_stats_token(adapter);
- if (rc)
- return rc;
-
return rc;
}
@@ -4662,13 +4654,21 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
goto ibmvnic_init_fail;
} while (rc == EAGAIN);
+ rc = init_stats_buffers(adapter);
+ if (rc)
+ goto ibmvnic_init_fail;
+
+ rc = init_stats_token(adapter);
+ if (rc)
+ goto ibmvnic_stats_fail;
+
netdev->mtu = adapter->req_mtu - ETH_HLEN;
netdev->min_mtu = adapter->min_mtu - ETH_HLEN;
netdev->max_mtu = adapter->max_mtu - ETH_HLEN;
rc = device_create_file(&dev->dev, &dev_attr_failover);
if (rc)
- goto ibmvnic_init_fail;
+ goto ibmvnic_dev_file_err;
netif_carrier_off(netdev);
rc = register_netdev(netdev);
@@ -4687,6 +4687,12 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
ibmvnic_register_fail:
device_remove_file(&dev->dev, &dev_attr_failover);
+ibmvnic_dev_file_err:
+ release_stats_token(adapter);
+
+ibmvnic_stats_fail:
+ release_stats_buffers(adapter);
+
ibmvnic_init_fail:
release_sub_crqs(adapter, 1);
release_crq_queue(adapter);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 net] net/ipv4: Initialize proto and ports in flow struct
From: Roopa Prabhu @ 2018-05-16 20:53 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <20180516203640.12568-1-dsahern@gmail.com>
On Wed, May 16, 2018 at 1:36 PM, David Ahern <dsahern@gmail.com> wrote:
> Updating the FIB tracepoint for the recent change to allow rules using
> the protocol and ports exposed a few places where the entries in the flow
> struct are not initialized.
>
> For __fib_validate_source add the call to fib4_rules_early_flow_dissect
> since it is invoked for the input path. For netfilter, add the memset on
> the flow struct to avoid future problems like this. In ip_route_input_slow
> need to set the fields if the skb dissection does not happen.
>
> Fixes: bfff4862653b ("net: fib_rules: support for match on ip_proto, sport and dport")
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
LGTM,
Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH net-next v12 4/7] sch_cake: Add NAT awareness to packet classifier
From: Cong Wang @ 2018-05-16 20:56 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <152650254618.25701.1794377356779114652.stgit@alrua-kau>
On Wed, May 16, 2018 at 1:29 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> When CAKE is deployed on a gateway that also performs NAT (which is a
> common deployment mode), the host fairness mechanism cannot distinguish
> internal hosts from each other, and so fails to work correctly.
>
> To fix this, we add an optional NAT awareness mode, which will query the
> kernel conntrack mechanism to obtain the pre-NAT addresses for each packet
> and use that in the flow and host hashing.
>
> When the shaper is enabled and the host is already performing NAT, the cost
> of this lookup is negligible. However, in unlimited mode with no NAT being
> performed, there is a significant CPU cost at higher bandwidths. For this
> reason, the feature is turned off by default.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
> net/sched/sch_cake.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 73 insertions(+)
>
> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index 65439b643c92..e1038a7b6686 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c
> @@ -71,6 +71,12 @@
> #include <net/tcp.h>
> #include <net/flow_dissector.h>
>
> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)
> +#include <net/netfilter/nf_conntrack_core.h>
> +#include <net/netfilter/nf_conntrack_zones.h>
> +#include <net/netfilter/nf_conntrack.h>
> +#endif
> +
> #define CAKE_SET_WAYS (8)
> #define CAKE_MAX_TINS (8)
> #define CAKE_QUEUES (1024)
> @@ -514,6 +520,60 @@ static bool cobalt_should_drop(struct cobalt_vars *vars,
> return drop;
> }
>
> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)
> +
> +static void cake_update_flowkeys(struct flow_keys *keys,
> + const struct sk_buff *skb)
> +{
> + const struct nf_conntrack_tuple *tuple;
> + enum ip_conntrack_info ctinfo;
> + struct nf_conn *ct;
> + bool rev = false;
> +
> + if (tc_skb_protocol(skb) != htons(ETH_P_IP))
> + return;
> +
> + ct = nf_ct_get(skb, &ctinfo);
> + if (ct) {
> + tuple = nf_ct_tuple(ct, CTINFO2DIR(ctinfo));
> + } else {
> + const struct nf_conntrack_tuple_hash *hash;
> + struct nf_conntrack_tuple srctuple;
> +
> + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
> + NFPROTO_IPV4, dev_net(skb->dev),
> + &srctuple))
> + return;
> +
> + hash = nf_conntrack_find_get(dev_net(skb->dev),
> + &nf_ct_zone_dflt,
> + &srctuple);
> + if (!hash)
> + return;
> +
> + rev = true;
> + ct = nf_ct_tuplehash_to_ctrack(hash);
> + tuple = nf_ct_tuple(ct, !hash->tuple.dst.dir);
> + }
> +
> + keys->addrs.v4addrs.src = rev ? tuple->dst.u3.ip : tuple->src.u3.ip;
> + keys->addrs.v4addrs.dst = rev ? tuple->src.u3.ip : tuple->dst.u3.ip;
> +
> + if (keys->ports.ports) {
> + keys->ports.src = rev ? tuple->dst.u.all : tuple->src.u.all;
> + keys->ports.dst = rev ? tuple->src.u.all : tuple->dst.u.all;
> + }
> + if (rev)
> + nf_ct_put(ct);
> +}
> +#else
> +static void cake_update_flowkeys(struct flow_keys *keys,
> + const struct sk_buff *skb)
> +{
> + /* There is nothing we can do here without CONNTRACK */
> +}
> +#endif
> +
> /* Cake has several subtle multiple bit settings. In these cases you
> * would be matching triple isolate mode as well.
> */
> @@ -541,6 +601,9 @@ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
> skb_flow_dissect_flow_keys(skb, &keys,
> FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
>
> + if (flow_mode & CAKE_FLOW_NAT_FLAG)
> + cake_update_flowkeys(&keys, skb);
> +
> /* flow_hash_from_keys() sorts the addresses by value, so we have
> * to preserve their order in a separate data structure to treat
> * src and dst host addresses as independently selectable.
> @@ -1727,6 +1790,12 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
> q->flow_mode = (nla_get_u32(tb[TCA_CAKE_FLOW_MODE]) &
> CAKE_FLOW_MASK);
>
> + if (tb[TCA_CAKE_NAT]) {
> + q->flow_mode &= ~CAKE_FLOW_NAT_FLAG;
> + q->flow_mode |= CAKE_FLOW_NAT_FLAG *
> + !!nla_get_u32(tb[TCA_CAKE_NAT]);
> + }
I think it's better to return -EOPNOTSUPP when CONFIG_NF_CONNTRACK
is not enabled.
> +
> if (tb[TCA_CAKE_RTT]) {
> q->interval = nla_get_u32(tb[TCA_CAKE_RTT]);
>
> @@ -1892,6 +1961,10 @@ static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
> if (nla_put_u32(skb, TCA_CAKE_ACK_FILTER, q->ack_filter))
> goto nla_put_failure;
>
> + if (nla_put_u32(skb, TCA_CAKE_NAT,
> + !!(q->flow_mode & CAKE_FLOW_NAT_FLAG)))
> + goto nla_put_failure;
> +
> return nla_nest_end(skb, opts);
>
> nla_put_failure:
>
^ permalink raw reply
* Re: [PATCH 0/3] ibmvnic: Fix bugs and memory leaks
From: Thomas Falcon @ 2018-05-16 20:59 UTC (permalink / raw)
To: netdev; +Cc: jallen, nfont
In-Reply-To: <1526503745-14421-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
On 05/16/2018 03:49 PM, Thomas Falcon wrote:
> This is a small patch series fixing up some bugs and memory leaks
> in the ibmvnic driver. The first fix frees up previously allocated
> memory that should be freed in case of an error. The second fixes
> a reset case that was failing due to TX/RX queue IRQ's being
> erroneously disabled without being enabled again. The final patch
> fixes incorrect reallocated of statistics buffers during a device
> reset, resulting in loss of statistics information and a memory leak.
>
> Thomas Falcon (3):
> ibmvnic: Free coherent DMA memory if FW map failed
> ibmvnic: Fix non-fatal firmware error reset
> ibmvnic: Fix statistics buffers memory leak
Sorry, these are meant for the 'net' tree.
Tom
>
> drivers/net/ethernet/ibm/ibmvnic.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
^ permalink raw reply
* [PATCH bpf-next] libbpf: add ifindex to enable offload support
From: Jakub Kicinski @ 2018-05-16 21:02 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, David Beckett
From: David Beckett <david.beckett@netronome.com>
BPF programs currently can only be offloaded using iproute2. This
patch will allow programs to be offloaded using libbpf calls.
Signed-off-by: David Beckett <david.beckett@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
tools/lib/bpf/bpf.c | 2 ++
tools/lib/bpf/bpf.h | 2 ++
tools/lib/bpf/libbpf.c | 18 +++++++++++++++---
tools/lib/bpf/libbpf.h | 1 +
4 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index a3a8fb2ac697..6a8a00097fd8 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -91,6 +91,7 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
attr.btf_fd = create_attr->btf_fd;
attr.btf_key_id = create_attr->btf_key_id;
attr.btf_value_id = create_attr->btf_value_id;
+ attr.map_ifindex = create_attr->map_ifindex;
return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
@@ -201,6 +202,7 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
attr.log_size = 0;
attr.log_level = 0;
attr.kern_version = load_attr->kern_version;
+ attr.prog_ifindex = load_attr->prog_ifindex;
memcpy(attr.prog_name, load_attr->name,
min(name_len, BPF_OBJ_NAME_LEN - 1));
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index fb3a146d92ff..15bff7728cf1 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -38,6 +38,7 @@ struct bpf_create_map_attr {
__u32 btf_fd;
__u32 btf_key_id;
__u32 btf_value_id;
+ __u32 map_ifindex;
};
int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr);
@@ -64,6 +65,7 @@ struct bpf_load_program_attr {
size_t insns_cnt;
const char *license;
__u32 kern_version;
+ __u32 prog_ifindex;
};
/* Recommend log buffer size */
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index df54c4c9e48a..3dbe217bf23e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -178,6 +178,7 @@ struct bpf_program {
/* Index in elf obj file, for relocation use. */
int idx;
char *name;
+ int prog_ifindex;
char *section_name;
struct bpf_insn *insns;
size_t insns_cnt, main_prog_cnt;
@@ -213,6 +214,7 @@ struct bpf_map {
int fd;
char *name;
size_t offset;
+ int map_ifindex;
struct bpf_map_def def;
uint32_t btf_key_id;
uint32_t btf_value_id;
@@ -1091,6 +1093,7 @@ bpf_object__create_maps(struct bpf_object *obj)
int *pfd = &map->fd;
create_attr.name = map->name;
+ create_attr.map_ifindex = map->map_ifindex;
create_attr.map_type = def->type;
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
@@ -1273,7 +1276,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
static int
load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
const char *name, struct bpf_insn *insns, int insns_cnt,
- char *license, u32 kern_version, int *pfd)
+ char *license, u32 kern_version, int *pfd, int prog_ifindex)
{
struct bpf_load_program_attr load_attr;
char *log_buf;
@@ -1287,6 +1290,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
load_attr.insns_cnt = insns_cnt;
load_attr.license = license;
load_attr.kern_version = kern_version;
+ load_attr.prog_ifindex = prog_ifindex;
if (!load_attr.insns || !load_attr.insns_cnt)
return -EINVAL;
@@ -1368,7 +1372,8 @@ bpf_program__load(struct bpf_program *prog,
}
err = load_program(prog->type, prog->expected_attach_type,
prog->name, prog->insns, prog->insns_cnt,
- license, kern_version, &fd);
+ license, kern_version, &fd,
+ prog->prog_ifindex);
if (!err)
prog->instances.fds[0] = fd;
goto out;
@@ -1399,7 +1404,8 @@ bpf_program__load(struct bpf_program *prog,
err = load_program(prog->type, prog->expected_attach_type,
prog->name, result.new_insn_ptr,
result.new_insn_cnt,
- license, kern_version, &fd);
+ license, kern_version, &fd,
+ prog->prog_ifindex);
if (err) {
pr_warning("Loading the %dth instance of program '%s' failed\n",
@@ -2188,6 +2194,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
enum bpf_attach_type expected_attach_type;
enum bpf_prog_type prog_type;
struct bpf_object *obj;
+ struct bpf_map *map;
int section_idx;
int err;
@@ -2207,6 +2214,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
* section name.
*/
prog_type = attr->prog_type;
+ prog->prog_ifindex = attr->ifindex;
expected_attach_type = attr->expected_attach_type;
if (prog_type == BPF_PROG_TYPE_UNSPEC) {
section_idx = bpf_program__identify_section(prog);
@@ -2227,6 +2235,10 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
first_prog = prog;
}
+ bpf_map__for_each(map, obj) {
+ map->map_ifindex = attr->ifindex;
+ }
+
if (!first_prog) {
pr_warning("object file doesn't contain bpf program\n");
bpf_object__close(obj);
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4574b9563278..cd3fd8d782c7 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -259,6 +259,7 @@ struct bpf_prog_load_attr {
const char *file;
enum bpf_prog_type prog_type;
enum bpf_attach_type expected_attach_type;
+ int ifindex;
};
int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
--
2.17.0
^ permalink raw reply related
* [PATCH v3 0/2] IR decoding using BPF
From: Sean Young @ 2018-05-16 21:04 UTC (permalink / raw)
To: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller, Y Song
The kernel IR decoders (drivers/media/rc/ir-*-decoder.c) support the most
widely used IR protocols, but there are many protocols which are not
supported[1]. For example, the lirc-remotes[2] repo has over 2700 remotes,
many of which are not supported by rc-core. There is a "long tail" of
unsupported IR protocols, for which lircd is need to decode the IR .
IR encoding is done in such a way that some simple circuit can decode it;
therefore, bpf is ideal.
In order to support all these protocols, here we have bpf based IR decoding.
The idea is that user-space can define a decoder in bpf, attach it to
the rc device through the lirc chardev.
Separate work is underway to extend ir-keytable to have an extensive library
of bpf-based decoders, and a much expanded library of rc keymaps.
Another future application would be to compile IRP[3] to a IR BPF program, and
so support virtually every remote without having to write a decoder for each.
It might also be possible to support non-button devices such as analog
directional pads or air conditioning remote controls and decode the target
temperature in bpf, and pass that to an input device.
Thanks,
Sean Young
[1] http://www.hifi-remote.com/wiki/index.php?title=DecodeIR
[2] https://sourceforge.net/p/lirc-remotes/code/ci/master/tree/remotes/
[3] http://www.hifi-remote.com/wiki/index.php?title=IRP_Notation
Changes since v2:
- Fixed locking issues
- Improved self-test to cover more cases
- Rebased on bpf-next again
Changes since v1:
- Code review comments from Y Song <ys114321@gmail.com> and
Randy Dunlap <rdunlap@infradead.org>
- Re-wrote sample bpf to be selftest
- Renamed RAWIR_DECODER -> RAWIR_EVENT (Kconfig, context, bpf prog type)
- Rebase on bpf-next
- Introduced bpf_rawir_event context structure with simpler access checking
Sean Young (2):
media: rc: introduce BPF_PROG_RAWIR_EVENT
bpf: add selftest for rawir_event type program
drivers/media/rc/Kconfig | 13 +
drivers/media/rc/Makefile | 1 +
drivers/media/rc/bpf-rawir-event.c | 363 ++++++++++++++++++
drivers/media/rc/lirc_dev.c | 24 ++
drivers/media/rc/rc-core-priv.h | 24 ++
drivers/media/rc/rc-ir-raw.c | 14 +-
include/linux/bpf_rcdev.h | 30 ++
include/linux/bpf_types.h | 3 +
include/uapi/linux/bpf.h | 55 ++-
kernel/bpf/syscall.c | 7 +
tools/bpf/bpftool/prog.c | 1 +
tools/include/uapi/linux/bpf.h | 57 ++-
tools/lib/bpf/libbpf.c | 1 +
tools/testing/selftests/bpf/Makefile | 8 +-
tools/testing/selftests/bpf/bpf_helpers.h | 6 +
tools/testing/selftests/bpf/test_rawir.sh | 37 ++
.../selftests/bpf/test_rawir_event_kern.c | 26 ++
.../selftests/bpf/test_rawir_event_user.c | 130 +++++++
18 files changed, 792 insertions(+), 8 deletions(-)
create mode 100644 drivers/media/rc/bpf-rawir-event.c
create mode 100644 include/linux/bpf_rcdev.h
create mode 100755 tools/testing/selftests/bpf/test_rawir.sh
create mode 100644 tools/testing/selftests/bpf/test_rawir_event_kern.c
create mode 100644 tools/testing/selftests/bpf/test_rawir_event_user.c
--
2.17.0
^ permalink raw reply
* [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT
From: Sean Young @ 2018-05-16 21:04 UTC (permalink / raw)
To: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller, Y Song
In-Reply-To: <cover.1526504511.git.sean@mess.org>
Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
that the last key should be repeated.
The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
the target_fd must be the /dev/lircN device.
Signed-off-by: Sean Young <sean@mess.org>
---
drivers/media/rc/Kconfig | 13 ++
drivers/media/rc/Makefile | 1 +
drivers/media/rc/bpf-rawir-event.c | 363 +++++++++++++++++++++++++++++
drivers/media/rc/lirc_dev.c | 24 ++
drivers/media/rc/rc-core-priv.h | 24 ++
drivers/media/rc/rc-ir-raw.c | 14 +-
include/linux/bpf_rcdev.h | 30 +++
include/linux/bpf_types.h | 3 +
include/uapi/linux/bpf.h | 55 ++++-
kernel/bpf/syscall.c | 7 +
10 files changed, 531 insertions(+), 3 deletions(-)
create mode 100644 drivers/media/rc/bpf-rawir-event.c
create mode 100644 include/linux/bpf_rcdev.h
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index eb2c3b6eca7f..2172d65b0213 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -25,6 +25,19 @@ config LIRC
passes raw IR to and from userspace, which is needed for
IR transmitting (aka "blasting") and for the lirc daemon.
+config BPF_RAWIR_EVENT
+ bool "Support for eBPF programs attached to lirc devices"
+ depends on BPF_SYSCALL
+ depends on RC_CORE=y
+ depends on LIRC
+ help
+ Allow attaching eBPF programs to a lirc device using the bpf(2)
+ syscall command BPF_PROG_ATTACH. This is supported for raw IR
+ receivers.
+
+ These eBPF programs can be used to decode IR into scancodes, for
+ IR protocols not supported by the kernel decoders.
+
menuconfig RC_DECODERS
bool "Remote controller decoders"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 2e1c87066f6c..74907823bef8 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -5,6 +5,7 @@ obj-y += keymaps/
obj-$(CONFIG_RC_CORE) += rc-core.o
rc-core-y := rc-main.o rc-ir-raw.o
rc-core-$(CONFIG_LIRC) += lirc_dev.o
+rc-core-$(CONFIG_BPF_RAWIR_EVENT) += bpf-rawir-event.o
obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o
diff --git a/drivers/media/rc/bpf-rawir-event.c b/drivers/media/rc/bpf-rawir-event.c
new file mode 100644
index 000000000000..7cb48b8d87b5
--- /dev/null
+++ b/drivers/media/rc/bpf-rawir-event.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+// bpf-rawir-event.c - handles bpf
+//
+// Copyright (C) 2018 Sean Young <sean@mess.org>
+
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <linux/bpf_rcdev.h>
+#include "rc-core-priv.h"
+
+/*
+ * BPF interface for raw IR
+ */
+const struct bpf_prog_ops rawir_event_prog_ops = {
+};
+
+BPF_CALL_1(bpf_rc_repeat, struct bpf_rawir_event*, event)
+{
+ struct ir_raw_event_ctrl *ctrl;
+
+ ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
+
+ rc_repeat(ctrl->dev);
+
+ return 0;
+}
+
+static const struct bpf_func_proto rc_repeat_proto = {
+ .func = bpf_rc_repeat,
+ .gpl_only = true, /* rc_repeat is EXPORT_SYMBOL_GPL */
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_4(bpf_rc_keydown, struct bpf_rawir_event*, event, u32, protocol,
+ u32, scancode, u32, toggle)
+{
+ struct ir_raw_event_ctrl *ctrl;
+
+ ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
+
+ rc_keydown(ctrl->dev, protocol, scancode, toggle != 0);
+
+ return 0;
+}
+
+static const struct bpf_func_proto rc_keydown_proto = {
+ .func = bpf_rc_keydown,
+ .gpl_only = true, /* rc_keydown is EXPORT_SYMBOL_GPL */
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_ANYTHING,
+ .arg4_type = ARG_ANYTHING,
+};
+
+static const struct bpf_func_proto *
+rawir_event_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_rc_repeat:
+ return &rc_repeat_proto;
+ case BPF_FUNC_rc_keydown:
+ return &rc_keydown_proto;
+ case BPF_FUNC_map_lookup_elem:
+ return &bpf_map_lookup_elem_proto;
+ case BPF_FUNC_map_update_elem:
+ return &bpf_map_update_elem_proto;
+ case BPF_FUNC_map_delete_elem:
+ return &bpf_map_delete_elem_proto;
+ case BPF_FUNC_ktime_get_ns:
+ return &bpf_ktime_get_ns_proto;
+ case BPF_FUNC_tail_call:
+ return &bpf_tail_call_proto;
+ case BPF_FUNC_get_prandom_u32:
+ return &bpf_get_prandom_u32_proto;
+ case BPF_FUNC_trace_printk:
+ if (capable(CAP_SYS_ADMIN))
+ return bpf_get_trace_printk_proto();
+ /* fall through */
+ default:
+ return NULL;
+ }
+}
+
+static bool rawir_event_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ /* struct bpf_rawir_event has two u32 fields */
+ if (type == BPF_WRITE)
+ return false;
+
+ if (size != sizeof(__u32))
+ return false;
+
+ if (!(off == offsetof(struct bpf_rawir_event, duration) ||
+ off == offsetof(struct bpf_rawir_event, type)))
+ return false;
+
+ return true;
+}
+
+const struct bpf_verifier_ops rawir_event_verifier_ops = {
+ .get_func_proto = rawir_event_func_proto,
+ .is_valid_access = rawir_event_is_valid_access
+};
+
+#define BPF_MAX_PROGS 64
+
+static int rc_dev_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
+{
+ struct ir_raw_event_ctrl *raw;
+ struct bpf_prog_list *pl;
+ int ret, size;
+
+ if (rcdev->driver_type != RC_DRIVER_IR_RAW)
+ return -EINVAL;
+
+ ret = mutex_lock_interruptible(&ir_raw_handler_lock);
+ if (ret)
+ return ret;
+
+ raw = rcdev->raw;
+ if (!raw) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ size = 0;
+ list_for_each_entry(pl, &raw->progs, node) {
+ if (pl->prog == prog) {
+ ret = -EEXIST;
+ goto out;
+ }
+
+ size++;
+ }
+
+ if (size >= BPF_MAX_PROGS) {
+ ret = -E2BIG;
+ goto out;
+ }
+
+ pl = kmalloc(sizeof(*pl), GFP_KERNEL);
+ if (!pl) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ pl->prog = prog;
+ list_add(&pl->node, &raw->progs);
+out:
+ mutex_unlock(&ir_raw_handler_lock);
+ return ret;
+}
+
+static int rc_dev_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
+{
+ struct ir_raw_event_ctrl *raw;
+ struct bpf_prog_list *pl, *tmp;
+ int ret;
+
+ if (rcdev->driver_type != RC_DRIVER_IR_RAW)
+ return -EINVAL;
+
+ ret = mutex_lock_interruptible(&ir_raw_handler_lock);
+ if (ret)
+ return ret;
+
+ raw = rcdev->raw;
+ if (!raw) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ ret = -ENOENT;
+
+ list_for_each_entry_safe(pl, tmp, &raw->progs, node) {
+ if (pl->prog == prog) {
+ list_del(&pl->node);
+ kfree(pl);
+ bpf_prog_put(prog);
+ ret = 0;
+ goto out;
+ }
+ }
+out:
+ mutex_unlock(&ir_raw_handler_lock);
+ return ret;
+}
+
+void rc_dev_bpf_init(struct rc_dev *rcdev)
+{
+ INIT_LIST_HEAD(&rcdev->raw->progs);
+}
+
+void rc_dev_bpf_run(struct rc_dev *rcdev, struct ir_raw_event ev)
+{
+ struct ir_raw_event_ctrl *raw = rcdev->raw;
+ struct bpf_prog_list *pl;
+
+ if (list_empty(&raw->progs))
+ return;
+
+ if (unlikely(ev.carrier_report)) {
+ raw->bpf_rawir_event.carrier = ev.carrier;
+ raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_CARRIER;
+ } else {
+ raw->bpf_rawir_event.duration = ev.duration;
+
+ if (ev.pulse)
+ raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_PULSE;
+ else if (ev.timeout)
+ raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_TIMEOUT;
+ else if (ev.reset)
+ raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_RESET;
+ else
+ raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_SPACE;
+ }
+
+ list_for_each_entry(pl, &raw->progs, node)
+ BPF_PROG_RUN(pl->prog, &raw->bpf_rawir_event);
+}
+
+void rc_dev_bpf_free(struct rc_dev *rcdev)
+{
+ struct bpf_prog_list *pl, *tmp;
+
+ list_for_each_entry_safe(pl, tmp, &rcdev->raw->progs, node) {
+ list_del(&pl->node);
+ bpf_prog_put(pl->prog);
+ kfree(pl);
+ }
+}
+
+int rc_dev_prog_attach(const union bpf_attr *attr)
+{
+ struct bpf_prog *prog;
+ struct rc_dev *rcdev;
+ int ret;
+
+ if (attr->attach_flags)
+ return -EINVAL;
+
+ prog = bpf_prog_get_type(attr->attach_bpf_fd,
+ BPF_PROG_TYPE_RAWIR_EVENT);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ rcdev = rc_dev_get_from_fd(attr->target_fd);
+ if (IS_ERR(rcdev)) {
+ bpf_prog_put(prog);
+ return PTR_ERR(rcdev);
+ }
+
+ ret = rc_dev_bpf_attach(rcdev, prog);
+ if (ret)
+ bpf_prog_put(prog);
+
+ put_device(&rcdev->dev);
+
+ return ret;
+}
+
+int rc_dev_prog_detach(const union bpf_attr *attr)
+{
+ struct bpf_prog *prog;
+ struct rc_dev *rcdev;
+ int ret;
+
+ if (attr->attach_flags)
+ return -EINVAL;
+
+ prog = bpf_prog_get_type(attr->attach_bpf_fd,
+ BPF_PROG_TYPE_RAWIR_EVENT);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ rcdev = rc_dev_get_from_fd(attr->target_fd);
+ if (IS_ERR(rcdev)) {
+ bpf_prog_put(prog);
+ return PTR_ERR(rcdev);
+ }
+
+ ret = rc_dev_bpf_detach(rcdev, prog);
+
+ bpf_prog_put(prog);
+ put_device(&rcdev->dev);
+
+ return ret;
+}
+
+int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr)
+{
+ __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
+ struct ir_raw_event_ctrl *raw;
+ struct bpf_prog_list *pl;
+ struct rc_dev *rcdev;
+ u32 cnt, flags = 0, *ids, i;
+ int ret;
+
+ if (attr->query.query_flags)
+ return -EINVAL;
+
+ rcdev = rc_dev_get_from_fd(attr->query.target_fd);
+ if (IS_ERR(rcdev))
+ return PTR_ERR(rcdev);
+
+ if (rcdev->driver_type != RC_DRIVER_IR_RAW) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = mutex_lock_interruptible(&ir_raw_handler_lock);
+ if (ret)
+ goto out;
+
+ raw = rcdev->raw;
+ if (!raw) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ cnt = 0;
+ list_for_each_entry(pl, &raw->progs, node)
+ cnt++;
+
+ if (copy_to_user(&uattr->query.prog_cnt, &cnt, sizeof(cnt))) {
+ ret = -EFAULT;
+ goto out;
+ }
+ if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (attr->query.prog_cnt != 0 && prog_ids && cnt) {
+ if (attr->query.prog_cnt < cnt)
+ cnt = attr->query.prog_cnt;
+
+ ids = kmalloc_array(cnt, sizeof(u32), GFP_KERNEL);
+ if (!ids) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ i = 0;
+ list_for_each_entry(pl, &raw->progs, node) {
+ ids[i++] = pl->prog->aux->id;
+ if (i == cnt)
+ break;
+ }
+
+ ret = copy_to_user(prog_ids, ids, cnt * sizeof(u32));
+ }
+out:
+ mutex_unlock(&ir_raw_handler_lock);
+ put_device(&rcdev->dev);
+
+ return ret;
+}
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 24e9fbb80e81..193540ded626 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -20,6 +20,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/device.h>
+#include <linux/file.h>
#include <linux/idr.h>
#include <linux/poll.h>
#include <linux/sched.h>
@@ -816,4 +817,27 @@ void __exit lirc_dev_exit(void)
unregister_chrdev_region(lirc_base_dev, RC_DEV_MAX);
}
+struct rc_dev *rc_dev_get_from_fd(int fd)
+{
+ struct fd f = fdget(fd);
+ struct lirc_fh *fh;
+ struct rc_dev *dev;
+
+ if (!f.file)
+ return ERR_PTR(-EBADF);
+
+ if (f.file->f_op != &lirc_fops) {
+ fdput(f);
+ return ERR_PTR(-EINVAL);
+ }
+
+ fh = f.file->private_data;
+ dev = fh->rc;
+
+ get_device(&dev->dev);
+ fdput(f);
+
+ return dev;
+}
+
MODULE_ALIAS("lirc_dev");
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index e0e6a17460f6..148db73cfa0c 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -13,6 +13,7 @@
#define MAX_IR_EVENT_SIZE 512
#include <linux/slab.h>
+#include <uapi/linux/bpf.h>
#include <media/rc-core.h>
/**
@@ -57,6 +58,11 @@ struct ir_raw_event_ctrl {
/* raw decoder state follows */
struct ir_raw_event prev_ev;
struct ir_raw_event this_ev;
+
+#ifdef CONFIG_BPF_RAWIR_EVENT
+ struct bpf_rawir_event bpf_rawir_event;
+ struct list_head progs;
+#endif
struct nec_dec {
int state;
unsigned count;
@@ -126,6 +132,9 @@ struct ir_raw_event_ctrl {
} imon;
};
+/* Mutex for locking raw IR processing and handler change */
+extern struct mutex ir_raw_handler_lock;
+
/* macros for IR decoders */
static inline bool geq_margin(unsigned d1, unsigned d2, unsigned margin)
{
@@ -288,6 +297,7 @@ void ir_lirc_raw_event(struct rc_dev *dev, struct ir_raw_event ev);
void ir_lirc_scancode_event(struct rc_dev *dev, struct lirc_scancode *lsc);
int ir_lirc_register(struct rc_dev *dev);
void ir_lirc_unregister(struct rc_dev *dev);
+struct rc_dev *rc_dev_get_from_fd(int fd);
#else
static inline int lirc_dev_init(void) { return 0; }
static inline void lirc_dev_exit(void) {}
@@ -299,4 +309,18 @@ static inline int ir_lirc_register(struct rc_dev *dev) { return 0; }
static inline void ir_lirc_unregister(struct rc_dev *dev) { }
#endif
+/*
+ * bpf interface
+ */
+#ifdef CONFIG_BPF_RAWIR_EVENT
+void rc_dev_bpf_init(struct rc_dev *dev);
+void rc_dev_bpf_free(struct rc_dev *dev);
+void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev);
+#else
+static inline void rc_dev_bpf_init(struct rc_dev *dev) { }
+static inline void rc_dev_bpf_free(struct rc_dev *dev) { }
+static inline void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev)
+{ }
+#endif
+
#endif /* _RC_CORE_PRIV */
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 374f83105a23..e68cdd4fbf5d 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -14,7 +14,7 @@
static LIST_HEAD(ir_raw_client_list);
/* Used to handle IR raw handler extensions */
-static DEFINE_MUTEX(ir_raw_handler_lock);
+DEFINE_MUTEX(ir_raw_handler_lock);
static LIST_HEAD(ir_raw_handler_list);
static atomic64_t available_protocols = ATOMIC64_INIT(0);
@@ -32,6 +32,7 @@ static int ir_raw_event_thread(void *data)
handler->protocols || !handler->protocols)
handler->decode(raw->dev, ev);
ir_lirc_raw_event(raw->dev, ev);
+ rc_dev_bpf_run(raw->dev, ev);
raw->prev_ev = ev;
}
mutex_unlock(&ir_raw_handler_lock);
@@ -572,6 +573,7 @@ int ir_raw_event_prepare(struct rc_dev *dev)
spin_lock_init(&dev->raw->edge_spinlock);
timer_setup(&dev->raw->edge_handle, ir_raw_edge_handle, 0);
INIT_KFIFO(dev->raw->kfifo);
+ rc_dev_bpf_init(dev);
return 0;
}
@@ -621,9 +623,17 @@ void ir_raw_event_unregister(struct rc_dev *dev)
list_for_each_entry(handler, &ir_raw_handler_list, list)
if (handler->raw_unregister)
handler->raw_unregister(dev);
- mutex_unlock(&ir_raw_handler_lock);
+
+ rc_dev_bpf_free(dev);
ir_raw_event_free(dev);
+
+ /*
+ * A user can be calling bpf(BPF_PROG_{QUERY|ATTACH|DETACH}), so
+ * ensure that the raw member is null on unlock; this is how
+ * "device gone" is checked.
+ */
+ mutex_unlock(&ir_raw_handler_lock);
}
/*
diff --git a/include/linux/bpf_rcdev.h b/include/linux/bpf_rcdev.h
new file mode 100644
index 000000000000..17a30f30436a
--- /dev/null
+++ b/include/linux/bpf_rcdev.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _BPF_RCDEV_H
+#define _BPF_RCDEV_H
+
+#include <linux/bpf.h>
+#include <uapi/linux/bpf.h>
+
+#ifdef CONFIG_BPF_RAWIR_EVENT
+int rc_dev_prog_attach(const union bpf_attr *attr);
+int rc_dev_prog_detach(const union bpf_attr *attr);
+int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr);
+#else
+static inline int rc_dev_prog_attach(const union bpf_attr *attr)
+{
+ return -EINVAL;
+}
+
+static inline int rc_dev_prog_detach(const union bpf_attr *attr)
+{
+ return -EINVAL;
+}
+
+static inline int rc_dev_prog_query(const union bpf_attr *attr,
+ union bpf_attr __user *uattr)
+{
+ return -EINVAL;
+}
+#endif
+
+#endif /* _BPF_RCDEV_H */
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index b67f8793de0d..e2b1b12474d4 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -25,6 +25,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT, raw_tracepoint)
#ifdef CONFIG_CGROUP_BPF
BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
#endif
+#ifdef CONFIG_BPF_RAWIR_EVENT
+BPF_PROG_TYPE(BPF_PROG_TYPE_RAWIR_EVENT, rawir_event)
+#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d94d333a8225..243e141e8a5b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -141,6 +141,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SK_MSG,
BPF_PROG_TYPE_RAW_TRACEPOINT,
BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
+ BPF_PROG_TYPE_RAWIR_EVENT,
};
enum bpf_attach_type {
@@ -158,6 +159,7 @@ enum bpf_attach_type {
BPF_CGROUP_INET6_CONNECT,
BPF_CGROUP_INET4_POST_BIND,
BPF_CGROUP_INET6_POST_BIND,
+ BPF_RAWIR_EVENT,
__MAX_BPF_ATTACH_TYPE
};
@@ -1902,6 +1904,35 @@ union bpf_attr {
* egress otherwise). This is the only flag supported for now.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
+ *
+ * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
+ * Description
+ * Report decoded scancode with toggle value. For use in
+ * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
+ * decoded scancode. This is will generate a keydown event,
+ * and a keyup event once the scancode is no longer repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
+ * protocol (see RC_PROTO_* enum).
+ *
+ * Some protocols include a toggle bit, in case the button
+ * was released and pressed again between consecutive scancodes,
+ * copy this bit into *toggle* if it exists, else set to 0.
+ *
+ * Return
+ * Always return 0 (for now)
+ *
+ * int bpf_rc_repeat(void *ctx)
+ * Description
+ * Repeat the last decoded scancode; some IR protocols like
+ * NEC have a special IR message for repeat last button,
+ * in case user is holding a button down; the scancode is
+ * not repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event.
+ *
+ * Return
+ * Always return 0 (for now)
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -1976,7 +2007,9 @@ union bpf_attr {
FN(fib_lookup), \
FN(sock_hash_update), \
FN(msg_redirect_hash), \
- FN(sk_redirect_hash),
+ FN(sk_redirect_hash), \
+ FN(rc_repeat), \
+ FN(rc_keydown),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
BPF_HDR_START_NET,
};
+/*
+ * user accessible mirror of in-kernel ir_raw_event
+ */
+#define BPF_RAWIR_EVENT_SPACE 0
+#define BPF_RAWIR_EVENT_PULSE 1
+#define BPF_RAWIR_EVENT_TIMEOUT 2
+#define BPF_RAWIR_EVENT_RESET 3
+#define BPF_RAWIR_EVENT_CARRIER 4
+#define BPF_RAWIR_EVENT_DUTY_CYCLE 5
+
+struct bpf_rawir_event {
+ union {
+ __u32 duration;
+ __u32 carrier;
+ __u32 duty_cycle;
+ };
+
+ __u32 type;
+};
+
/* user accessible mirror of in-kernel sk_buff.
* new fields can only be added to the end of this structure
*/
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e2aeb5e89f44..75c089f407c8 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -11,6 +11,7 @@
*/
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
+#include <linux/bpf_rcdev.h>
#include <linux/btf.h>
#include <linux/syscalls.h>
#include <linux/slab.h>
@@ -1567,6 +1568,8 @@ static int bpf_prog_attach(const union bpf_attr *attr)
case BPF_SK_SKB_STREAM_PARSER:
case BPF_SK_SKB_STREAM_VERDICT:
return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, true);
+ case BPF_RAWIR_EVENT:
+ return rc_dev_prog_attach(attr);
default:
return -EINVAL;
}
@@ -1637,6 +1640,8 @@ static int bpf_prog_detach(const union bpf_attr *attr)
case BPF_SK_SKB_STREAM_PARSER:
case BPF_SK_SKB_STREAM_VERDICT:
return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, false);
+ case BPF_RAWIR_EVENT:
+ return rc_dev_prog_detach(attr);
default:
return -EINVAL;
}
@@ -1684,6 +1689,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
case BPF_CGROUP_SOCK_OPS:
case BPF_CGROUP_DEVICE:
break;
+ case BPF_RAWIR_EVENT:
+ return rc_dev_prog_query(attr, uattr);
default:
return -EINVAL;
}
--
2.17.0
^ permalink raw reply related
* [PATCH v3 2/2] bpf: add selftest for rawir_event type program
From: Sean Young @ 2018-05-16 21:04 UTC (permalink / raw)
To: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller, Y Song
In-Reply-To: <cover.1526504511.git.sean@mess.org>
This is simple test over rc-loopback.
Signed-off-by: Sean Young <sean@mess.org>
---
tools/bpf/bpftool/prog.c | 1 +
tools/include/uapi/linux/bpf.h | 57 +++++++-
tools/lib/bpf/libbpf.c | 1 +
tools/testing/selftests/bpf/Makefile | 8 +-
tools/testing/selftests/bpf/bpf_helpers.h | 6 +
tools/testing/selftests/bpf/test_rawir.sh | 37 +++++
.../selftests/bpf/test_rawir_event_kern.c | 26 ++++
.../selftests/bpf/test_rawir_event_user.c | 130 ++++++++++++++++++
8 files changed, 261 insertions(+), 5 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_rawir.sh
create mode 100644 tools/testing/selftests/bpf/test_rawir_event_kern.c
create mode 100644 tools/testing/selftests/bpf/test_rawir_event_user.c
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 9bdfdf2d3fbe..8889a4ee8577 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -71,6 +71,7 @@ static const char * const prog_type_name[] = {
[BPF_PROG_TYPE_SK_MSG] = "sk_msg",
[BPF_PROG_TYPE_RAW_TRACEPOINT] = "raw_tracepoint",
[BPF_PROG_TYPE_CGROUP_SOCK_ADDR] = "cgroup_sock_addr",
+ [BPF_PROG_TYPE_RAWIR_EVENT] = "rawir_event",
};
static void print_boot_time(__u64 nsecs, char *buf, unsigned int size)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1205d86a7a29..243e141e8a5b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -141,6 +141,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SK_MSG,
BPF_PROG_TYPE_RAW_TRACEPOINT,
BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
+ BPF_PROG_TYPE_RAWIR_EVENT,
};
enum bpf_attach_type {
@@ -158,6 +159,7 @@ enum bpf_attach_type {
BPF_CGROUP_INET6_CONNECT,
BPF_CGROUP_INET4_POST_BIND,
BPF_CGROUP_INET6_POST_BIND,
+ BPF_RAWIR_EVENT,
__MAX_BPF_ATTACH_TYPE
};
@@ -1829,7 +1831,6 @@ union bpf_attr {
* Return
* 0 on success, or a negative error in case of failure.
*
- *
* int bpf_fib_lookup(void *ctx, struct bpf_fib_lookup *params, int plen, u32 flags)
* Description
* Do FIB lookup in kernel tables using parameters in *params*.
@@ -1856,6 +1857,7 @@ union bpf_attr {
* Egress device index on success, 0 if packet needs to continue
* up the stack for further processing or a negative error in case
* of failure.
+ *
* int bpf_sock_hash_update(struct bpf_sock_ops_kern *skops, struct bpf_map *map, void *key, u64 flags)
* Description
* Add an entry to, or update a sockhash *map* referencing sockets.
@@ -1902,6 +1904,35 @@ union bpf_attr {
* egress otherwise). This is the only flag supported for now.
* Return
* **SK_PASS** on success, or **SK_DROP** on error.
+ *
+ * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
+ * Description
+ * Report decoded scancode with toggle value. For use in
+ * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
+ * decoded scancode. This is will generate a keydown event,
+ * and a keyup event once the scancode is no longer repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
+ * protocol (see RC_PROTO_* enum).
+ *
+ * Some protocols include a toggle bit, in case the button
+ * was released and pressed again between consecutive scancodes,
+ * copy this bit into *toggle* if it exists, else set to 0.
+ *
+ * Return
+ * Always return 0 (for now)
+ *
+ * int bpf_rc_repeat(void *ctx)
+ * Description
+ * Repeat the last decoded scancode; some IR protocols like
+ * NEC have a special IR message for repeat last button,
+ * in case user is holding a button down; the scancode is
+ * not repeated.
+ *
+ * *ctx* pointer to bpf_rawir_event.
+ *
+ * Return
+ * Always return 0 (for now)
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -1976,7 +2007,9 @@ union bpf_attr {
FN(fib_lookup), \
FN(sock_hash_update), \
FN(msg_redirect_hash), \
- FN(sk_redirect_hash),
+ FN(sk_redirect_hash), \
+ FN(rc_repeat), \
+ FN(rc_keydown),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
BPF_HDR_START_NET,
};
+/*
+ * user accessible mirror of in-kernel ir_raw_event
+ */
+#define BPF_RAWIR_EVENT_SPACE 0
+#define BPF_RAWIR_EVENT_PULSE 1
+#define BPF_RAWIR_EVENT_TIMEOUT 2
+#define BPF_RAWIR_EVENT_RESET 3
+#define BPF_RAWIR_EVENT_CARRIER 4
+#define BPF_RAWIR_EVENT_DUTY_CYCLE 5
+
+struct bpf_rawir_event {
+ union {
+ __u32 duration;
+ __u32 carrier;
+ __u32 duty_cycle;
+ };
+
+ __u32 type;
+};
+
/* user accessible mirror of in-kernel sk_buff.
* new fields can only be added to the end of this structure
*/
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index df54c4c9e48a..372269e9053d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1455,6 +1455,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)
case BPF_PROG_TYPE_CGROUP_DEVICE:
case BPF_PROG_TYPE_SK_MSG:
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
+ case BPF_PROG_TYPE_RAWIR_EVENT:
return false;
case BPF_PROG_TYPE_UNSPEC:
case BPF_PROG_TYPE_KPROBE:
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1eb0fa2aba92..b84e36d05d34 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -24,7 +24,7 @@ urandom_read: urandom_read.c
# Order correspond to 'make run_tests' order
TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
- test_sock test_btf test_sockmap
+ test_sock test_btf test_sockmap test_rawir_event_user
TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \
@@ -33,7 +33,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o test_adjust_tail.o \
test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
- test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o
+ test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
+ test_rawir_event_kern.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -42,7 +43,8 @@ TEST_PROGS := test_kmod.sh \
test_xdp_meta.sh \
test_offload.py \
test_sock_addr.sh \
- test_tunnel.sh
+ test_tunnel.sh \
+ test_rawir.sh
# Compile but not part of 'make run_tests'
TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 8f143dfb3700..26d89b7f9841 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -114,6 +114,12 @@ static int (*bpf_get_stack)(void *ctx, void *buf, int size, int flags) =
static int (*bpf_fib_lookup)(void *ctx, struct bpf_fib_lookup *params,
int plen, __u32 flags) =
(void *) BPF_FUNC_fib_lookup;
+static int (*bpf_rc_repeat)(void *ctx) =
+ (void *) BPF_FUNC_rc_repeat;
+static int (*bpf_rc_keydown)(void *ctx, unsigned int protocol,
+ unsigned int scancode, unsigned int toggle) =
+ (void *) BPF_FUNC_rc_keydown;
+
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/test_rawir.sh b/tools/testing/selftests/bpf/test_rawir.sh
new file mode 100755
index 000000000000..0aa77b043ee1
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_rawir.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# Test bpf_rawir_event over rc-loopback. Steps for the test:
+#
+# 1. Find the /dev/lircN device for rc-loopback
+# 2. Attach bpf_rawir_event program which decodes some IR.
+# 3. Send some IR to the same IR device; since it is loopback, this will
+# end up in the bpf program
+# 4. bpf program should decode IR and report keycode
+# 5. We can read keycode from same /dev/lirc device
+
+GREEN='\033[0;92m'
+RED='\033[0;31m'
+NC='\033[0m' # No Color
+
+modprobe rc-loopback
+
+for i in /sys/class/rc/rc*
+do
+ if grep -q DRV_NAME=rc-loopback $i/uevent
+ then
+ LIRCDEV=$(grep DEVNAME= $i/lirc*/uevent | sed sQDEVNAME=Q/dev/Q)
+ fi
+done
+
+if [ -n $LIRCDEV ];
+then
+ TYPE=rawir_event
+ ./test_rawir_event_user $LIRCDEV
+ ret=$?
+ if [ $ret -ne 0 ]; then
+ echo -e ${RED}"FAIL: $TYPE"${NC}
+ else
+ echo -e ${GREEN}"PASS: $TYPE"${NC}
+ fi
+fi
diff --git a/tools/testing/selftests/bpf/test_rawir_event_kern.c b/tools/testing/selftests/bpf/test_rawir_event_kern.c
new file mode 100644
index 000000000000..33ba5d30af62
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_rawir_event_kern.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+// test ir decoder
+//
+// Copyright (C) 2018 Sean Young <sean@mess.org>
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+SEC("rawir_event")
+int bpf_decoder(struct bpf_rawir_event *e)
+{
+ if (e->type == BPF_RAWIR_EVENT_PULSE) {
+ /*
+ * The lirc interface is microseconds, but here we receive
+ * nanoseconds.
+ */
+ int microseconds = e->duration / 1000;
+
+ if (microseconds & 0x10000)
+ bpf_rc_keydown(e, 0x40, microseconds & 0xffff, 0);
+ }
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_rawir_event_user.c b/tools/testing/selftests/bpf/test_rawir_event_user.c
new file mode 100644
index 000000000000..c3d7f2c68033
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_rawir_event_user.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0
+// test ir decoder
+//
+// Copyright (C) 2018 Sean Young <sean@mess.org>
+
+#include <linux/bpf.h>
+#include <linux/lirc.h>
+#include <assert.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <string.h>
+#include <unistd.h>
+#include <poll.h>
+#include <libgen.h>
+#include <sys/resource.h>
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include "bpf_util.h"
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+int main(int argc, char **argv)
+{
+ struct bpf_object *obj;
+ int ret, lircfd, progfd, mode;
+ int testir = 0x1dead;
+ u32 prog_ids[10], prog_flags[10], prog_cnt;
+
+ if (argc != 2) {
+ printf("Usage: %s /dev/lircN\n", argv[0]);
+ return 2;
+ }
+
+ ret = bpf_prog_load("test_rawir_event_kern.o",
+ BPF_PROG_TYPE_RAWIR_EVENT, &obj, &progfd);
+ if (ret) {
+ printf("Failed to load bpf program\n");
+ return 1;
+ }
+
+ lircfd = open(argv[1], O_RDWR | O_NONBLOCK);
+ if (lircfd == -1) {
+ printf("failed to open lirc device %s: %m\n", argv[1]);
+ return 1;
+ }
+
+ /* Let's try detach it before it was ever attached */
+ ret = bpf_prog_detach2(progfd, lircfd, BPF_RAWIR_EVENT);
+ if (ret != -1 || errno != ENOENT) {
+ printf("bpf_prog_detach2 not attached should fail: %m\n");
+ return 1;
+ }
+
+ mode = LIRC_MODE_SCANCODE;
+ if (ioctl(lircfd, LIRC_SET_REC_MODE, &mode)) {
+ printf("failed to set rec mode: %m\n");
+ return 1;
+ }
+
+ prog_cnt = 10;
+ ret = bpf_prog_query(lircfd, BPF_RAWIR_EVENT, 0, prog_flags, prog_ids,
+ &prog_cnt);
+ if (ret) {
+ printf("Failed to query bpf programs on lirc device: %m\n");
+ return 1;
+ }
+
+ if (prog_cnt != 0) {
+ printf("Expected nothing to be attached\n");
+ return 1;
+ }
+
+ ret = bpf_prog_attach(progfd, lircfd, BPF_RAWIR_EVENT, 0);
+ if (ret) {
+ printf("Failed to attach bpf to lirc device: %m\n");
+ return 1;
+ }
+
+ /* Write raw IR */
+ ret = write(lircfd, &testir, sizeof(testir));
+ if (ret != sizeof(testir)) {
+ printf("Failed to send test IR message: %m\n");
+ return 1;
+ }
+
+ struct pollfd pfd = { .fd = lircfd, .events = POLLIN };
+ struct lirc_scancode lsc;
+
+ poll(&pfd, 1, 100);
+
+ /* Read decoded IR */
+ ret = read(lircfd, &lsc, sizeof(lsc));
+ if (ret != sizeof(lsc)) {
+ printf("Failed to read decoded IR: %m\n");
+ return 1;
+ }
+
+ if (lsc.scancode != 0xdead || lsc.rc_proto != 64) {
+ printf("Incorrect scancode decoded\n");
+ return 1;
+ }
+
+ prog_cnt = 10;
+ ret = bpf_prog_query(lircfd, BPF_RAWIR_EVENT, 0, prog_flags, prog_ids,
+ &prog_cnt);
+ if (ret) {
+ printf("Failed to query bpf programs on lirc device: %m\n");
+ return 1;
+ }
+
+ if (prog_cnt != 1) {
+ printf("Expected one program to be attached\n");
+ return 1;
+ }
+
+ /* Let's try detaching it now it is actually attached */
+ ret = bpf_prog_detach2(progfd, lircfd, BPF_RAWIR_EVENT);
+ if (ret) {
+ printf("bpf_prog_detach2: returned %m\n");
+ return 1;
+ }
+
+ return 0;
+}
--
2.17.0
^ permalink raw reply related
* [PATCH bpf-next] bpf: fix sock hashmap kmalloc warning
From: Yonghong Song @ 2018-05-16 21:06 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
syzbot reported a kernel warning below:
WARNING: CPU: 0 PID: 4499 at mm/slab_common.c:996 kmalloc_slab+0x56/0x70 mm/slab_common.c:996
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 4499 Comm: syz-executor050 Not tainted 4.17.0-rc3+ #9
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1b9/0x294 lib/dump_stack.c:113
panic+0x22f/0x4de kernel/panic.c:184
__warn.cold.8+0x163/0x1b3 kernel/panic.c:536
report_bug+0x252/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:kmalloc_slab+0x56/0x70 mm/slab_common.c:996
RSP: 0018:ffff8801d907fc58 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff8801aeecb280 RCX: ffffffff8185ebd7
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffe1
RBP: ffff8801d907fc58 R08: ffff8801adb5e1c0 R09: ffffed0035a84700
R10: ffffed0035a84700 R11: ffff8801ad423803 R12: ffff8801aeecb280
R13: 00000000fffffff4 R14: ffff8801ad891a00 R15: 00000000014200c0
__do_kmalloc mm/slab.c:3713 [inline]
__kmalloc+0x25/0x760 mm/slab.c:3727
kmalloc include/linux/slab.h:517 [inline]
map_get_next_key+0x24a/0x640 kernel/bpf/syscall.c:858
__do_sys_bpf kernel/bpf/syscall.c:2131 [inline]
__se_sys_bpf kernel/bpf/syscall.c:2096 [inline]
__x64_sys_bpf+0x354/0x4f0 kernel/bpf/syscall.c:2096
do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The test case is against sock hashmap with a key size 0xffffffe1.
Such a large key size will cause the below code in function
sock_hash_alloc() overflowing and produces a smaller elem_size,
hence map creation will be successful.
htab->elem_size = sizeof(struct htab_elem) +
round_up(htab->map.key_size, 8);
Later, when map_get_next_key is called and kernel tries
to allocate the key unsuccessfully, it will issue
the above warning.
Similar to hashtab, ensure the key size is at most
MAX_BPF_STACK for a successful map creation.
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Reported-by: syzbot+e4566d29080e7f3460ff@syzkaller.appspotmail.com
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/sockmap.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 56879c9fd3a4..79f5e8988889 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1990,6 +1990,12 @@ static struct bpf_map *sock_hash_alloc(union bpf_attr *attr)
attr->map_flags & ~SOCK_CREATE_FLAG_MASK)
return ERR_PTR(-EINVAL);
+ if (attr->key_size > MAX_BPF_STACK)
+ /* eBPF programs initialize keys on stack, so they cannot be
+ * larger than max stack size
+ */
+ return ERR_PTR(-E2BIG);
+
err = bpf_tcp_ulp_register();
if (err && err != -EEXIST)
return ERR_PTR(err);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net-next v12 2/7] sch_cake: Add ingress mode
From: Cong Wang @ 2018-05-16 21:09 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <152650254608.25701.5749607607862123240.stgit@alrua-kau>
On Wed, May 16, 2018 at 1:29 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> + if (tb[TCA_CAKE_AUTORATE]) {
> + if (!!nla_get_u32(tb[TCA_CAKE_AUTORATE]))
> + q->rate_flags |= CAKE_FLAG_AUTORATE_INGRESS;
> + else
> + q->rate_flags &= ~CAKE_FLAG_AUTORATE_INGRESS;
> + }
> +
> + if (tb[TCA_CAKE_INGRESS]) {
> + if (!!nla_get_u32(tb[TCA_CAKE_INGRESS]))
> + q->rate_flags |= CAKE_FLAG_INGRESS;
> + else
> + q->rate_flags &= ~CAKE_FLAG_INGRESS;
> + }
> +
> if (tb[TCA_CAKE_MEMORY])
> q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
>
> @@ -1559,6 +1628,14 @@ static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
> if (nla_put_u32(skb, TCA_CAKE_MEMORY, q->buffer_config_limit))
> goto nla_put_failure;
>
> + if (nla_put_u32(skb, TCA_CAKE_AUTORATE,
> + !!(q->rate_flags & CAKE_FLAG_AUTORATE_INGRESS)))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_INGRESS,
> + !!(q->rate_flags & CAKE_FLAG_INGRESS)))
> + goto nla_put_failure;
> +
Why do you want to dump each bit of the rate_flags separately rather than
dumping the whole rate_flags as an integer?
^ permalink raw reply
* Re: [iproute2-next v2 1/1] tipc: fixed node and name table listings
From: David Ahern @ 2018-05-16 21:13 UTC (permalink / raw)
To: Jon Maloy, stephen, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1526392461-21682-1-git-send-email-jon.maloy@ericsson.com>
On 5/15/18 7:54 AM, Jon Maloy wrote:
> We make it easier for users to correlate between 128-bit node
> identities and 32-bit node hash number by extending the 'node list'
> command to also show the hash number.
>
> We also improve the 'nametable show' command to show the node identity
> instead of the node hash number. Since the former potentially is much
> longer than the latter, we make room for it by eliminating the (to the
> user) irrelevant publication key. We also reorder some of the columns so
> that the node id comes last, since this looks nicer and is more logical.
>
> ---
> v2: Fixed compiler warning as per comment from David Ahern
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
> ---
> tipc/misc.c | 18 ++++++++++++++++++
> tipc/misc.h | 1 +
> tipc/nametable.c | 18 ++++++++++--------
> tipc/node.c | 19 ++++++++-----------
> tipc/peer.c | 4 ++++
> 5 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/tipc/misc.c b/tipc/misc.c
> index 16849f1..e8b726f 100644
> --- a/tipc/misc.c
> +++ b/tipc/misc.c
> @@ -13,6 +13,9 @@
> #include <stdint.h>
> #include <linux/tipc.h>
> #include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <errno.h>
> #include "misc.h"
>
> #define IN_RANGE(val, low, high) ((val) <= (high) && (val) >= (low))
> @@ -109,3 +112,18 @@ void nodeid2str(uint8_t *id, char *str)
> for (i = 31; str[i] == '0'; i--)
> str[i] = 0;
> }
> +
> +void hash2nodestr(uint32_t hash, char *str)
> +{
> + struct tipc_sioc_nodeid_req nr = {};
> + int sd;
> +
> + sd = socket(AF_TIPC, SOCK_RDM, 0);
> + if (sd < 0) {
> + fprintf(stderr, "opening TIPC socket: %s\n", strerror(errno));
> + return;
> + }
> + nr.peer = hash;
> + if (!ioctl(sd, SIOCGETNODEID, &nr))
> + nodeid2str((uint8_t *)nr.node_id, str);
> +}
you are leaking sd
^ permalink raw reply
* Re: [PATCH net-next v12 1/7] sched: Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-05-16 21:13 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <CAM_iQpX0mfKJWjDE_2ofKftcr2KgaOkinm70-CdNtNSBj8_cJQ@mail.gmail.com>
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Wed, May 16, 2018 at 1:29 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>> +
>> +static struct Qdisc *cake_leaf(struct Qdisc *sch, unsigned long arg)
>> +{
>> + return NULL;
>> +}
>> +
>> +static unsigned long cake_find(struct Qdisc *sch, u32 classid)
>> +{
>> + return 0;
>> +}
>> +
>> +static void cake_walk(struct Qdisc *sch, struct qdisc_walker *arg)
>> +{
>> +}
>
>
> Thanks for adding the support to other TC filters, it is much better
> now!
You're welcome. Turned out not to be that hard :)
> A quick question: why class_ops->dump_stats is still NULL?
>
> It is supposed to dump the stats of each flow. Is there still any
> difficulty to map it to tc class? I thought you figured it out when
> you added the tcf_classify().
On the classify side, I solved the "multiple sets of queues" problem by
using skb->priority to select the tin (diffserv tier) and the classifier
output to select the queue within that tin. This would not work for
dumping stats; some other way of mapping queues to the linear class
space would be needed. And since we are not actually collecting any
per-flow stats that I could print, I thought it wasn't worth coming up
with a half-baked proposal for this just to add an API hook that no one
in the existing CAKE user base has ever asked for...
-Toke
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix sock hashmap kmalloc warning
From: John Fastabend @ 2018-05-16 21:14 UTC (permalink / raw)
To: Yonghong Song, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180516210626.776403-1-yhs@fb.com>
On 05/16/2018 02:06 PM, Yonghong Song wrote:
> syzbot reported a kernel warning below:
> WARNING: CPU: 0 PID: 4499 at mm/slab_common.c:996 kmalloc_slab+0x56/0x70 mm/slab_common.c:996
> Kernel panic - not syncing: panic_on_warn set ...
>
> CPU: 0 PID: 4499 Comm: syz-executor050 Not tainted 4.17.0-rc3+ #9
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> panic+0x22f/0x4de kernel/panic.c:184
> __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
> report_bug+0x252/0x2d0 lib/bug.c:186
> fixup_bug arch/x86/kernel/traps.c:178 [inline]
> do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> RIP: 0010:kmalloc_slab+0x56/0x70 mm/slab_common.c:996
> RSP: 0018:ffff8801d907fc58 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8801aeecb280 RCX: ffffffff8185ebd7
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00000000ffffffe1
> RBP: ffff8801d907fc58 R08: ffff8801adb5e1c0 R09: ffffed0035a84700
> R10: ffffed0035a84700 R11: ffff8801ad423803 R12: ffff8801aeecb280
> R13: 00000000fffffff4 R14: ffff8801ad891a00 R15: 00000000014200c0
> __do_kmalloc mm/slab.c:3713 [inline]
> __kmalloc+0x25/0x760 mm/slab.c:3727
> kmalloc include/linux/slab.h:517 [inline]
> map_get_next_key+0x24a/0x640 kernel/bpf/syscall.c:858
> __do_sys_bpf kernel/bpf/syscall.c:2131 [inline]
> __se_sys_bpf kernel/bpf/syscall.c:2096 [inline]
> __x64_sys_bpf+0x354/0x4f0 kernel/bpf/syscall.c:2096
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The test case is against sock hashmap with a key size 0xffffffe1.
> Such a large key size will cause the below code in function
> sock_hash_alloc() overflowing and produces a smaller elem_size,
> hence map creation will be successful.
> htab->elem_size = sizeof(struct htab_elem) +
> round_up(htab->map.key_size, 8);
>
> Later, when map_get_next_key is called and kernel tries
> to allocate the key unsuccessfully, it will issue
> the above warning.
>
> Similar to hashtab, ensure the key size is at most
> MAX_BPF_STACK for a successful map creation.
>
> Fixes: 81110384441a ("bpf: sockmap, add hash map support")
> Reported-by: syzbot+e4566d29080e7f3460ff@syzkaller.appspotmail.com
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/bpf/sockmap.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
> index 56879c9fd3a4..79f5e8988889 100644
> --- a/kernel/bpf/sockmap.c
> +++ b/kernel/bpf/sockmap.c
> @@ -1990,6 +1990,12 @@ static struct bpf_map *sock_hash_alloc(union bpf_attr *attr)
> attr->map_flags & ~SOCK_CREATE_FLAG_MASK)
> return ERR_PTR(-EINVAL);
>
> + if (attr->key_size > MAX_BPF_STACK)
> + /* eBPF programs initialize keys on stack, so they cannot be
> + * larger than max stack size
> + */
> + return ERR_PTR(-E2BIG);
> +
> err = bpf_tcp_ulp_register();
> if (err && err != -EEXIST)
> return ERR_PTR(err);
>
Thanks!
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v12 4/7] sch_cake: Add NAT awareness to packet classifier
From: Toke Høiland-Jørgensen @ 2018-05-16 21:14 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <CAM_iQpUCB24x7d7oDYkes_ngQ==8CjiwGqCBit-BK5NxzzZODg@mail.gmail.com>
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Wed, May 16, 2018 at 1:29 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>> When CAKE is deployed on a gateway that also performs NAT (which is a
>> common deployment mode), the host fairness mechanism cannot distinguish
>> internal hosts from each other, and so fails to work correctly.
>>
>> To fix this, we add an optional NAT awareness mode, which will query the
>> kernel conntrack mechanism to obtain the pre-NAT addresses for each packet
>> and use that in the flow and host hashing.
>>
>> When the shaper is enabled and the host is already performing NAT, the cost
>> of this lookup is negligible. However, in unlimited mode with no NAT being
>> performed, there is a significant CPU cost at higher bandwidths. For this
>> reason, the feature is turned off by default.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> ---
>> net/sched/sch_cake.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 73 insertions(+)
>>
>> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
>> index 65439b643c92..e1038a7b6686 100644
>> --- a/net/sched/sch_cake.c
>> +++ b/net/sched/sch_cake.c
>> @@ -71,6 +71,12 @@
>> #include <net/tcp.h>
>> #include <net/flow_dissector.h>
>>
>> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)
>> +#include <net/netfilter/nf_conntrack_core.h>
>> +#include <net/netfilter/nf_conntrack_zones.h>
>> +#include <net/netfilter/nf_conntrack.h>
>> +#endif
>> +
>> #define CAKE_SET_WAYS (8)
>> #define CAKE_MAX_TINS (8)
>> #define CAKE_QUEUES (1024)
>> @@ -514,6 +520,60 @@ static bool cobalt_should_drop(struct cobalt_vars *vars,
>> return drop;
>> }
>>
>> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)
>> +
>> +static void cake_update_flowkeys(struct flow_keys *keys,
>> + const struct sk_buff *skb)
>> +{
>> + const struct nf_conntrack_tuple *tuple;
>> + enum ip_conntrack_info ctinfo;
>> + struct nf_conn *ct;
>> + bool rev = false;
>> +
>> + if (tc_skb_protocol(skb) != htons(ETH_P_IP))
>> + return;
>> +
>> + ct = nf_ct_get(skb, &ctinfo);
>> + if (ct) {
>> + tuple = nf_ct_tuple(ct, CTINFO2DIR(ctinfo));
>> + } else {
>> + const struct nf_conntrack_tuple_hash *hash;
>> + struct nf_conntrack_tuple srctuple;
>> +
>> + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
>> + NFPROTO_IPV4, dev_net(skb->dev),
>> + &srctuple))
>> + return;
>> +
>> + hash = nf_conntrack_find_get(dev_net(skb->dev),
>> + &nf_ct_zone_dflt,
>> + &srctuple);
>> + if (!hash)
>> + return;
>> +
>> + rev = true;
>> + ct = nf_ct_tuplehash_to_ctrack(hash);
>> + tuple = nf_ct_tuple(ct, !hash->tuple.dst.dir);
>> + }
>> +
>> + keys->addrs.v4addrs.src = rev ? tuple->dst.u3.ip : tuple->src.u3.ip;
>> + keys->addrs.v4addrs.dst = rev ? tuple->src.u3.ip : tuple->dst.u3.ip;
>> +
>> + if (keys->ports.ports) {
>> + keys->ports.src = rev ? tuple->dst.u.all : tuple->src.u.all;
>> + keys->ports.dst = rev ? tuple->src.u.all : tuple->dst.u.all;
>> + }
>> + if (rev)
>> + nf_ct_put(ct);
>> +}
>> +#else
>> +static void cake_update_flowkeys(struct flow_keys *keys,
>> + const struct sk_buff *skb)
>> +{
>> + /* There is nothing we can do here without CONNTRACK */
>> +}
>> +#endif
>> +
>> /* Cake has several subtle multiple bit settings. In these cases you
>> * would be matching triple isolate mode as well.
>> */
>> @@ -541,6 +601,9 @@ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
>> skb_flow_dissect_flow_keys(skb, &keys,
>> FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
>>
>> + if (flow_mode & CAKE_FLOW_NAT_FLAG)
>> + cake_update_flowkeys(&keys, skb);
>> +
>> /* flow_hash_from_keys() sorts the addresses by value, so we have
>> * to preserve their order in a separate data structure to treat
>> * src and dst host addresses as independently selectable.
>> @@ -1727,6 +1790,12 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
>> q->flow_mode = (nla_get_u32(tb[TCA_CAKE_FLOW_MODE]) &
>> CAKE_FLOW_MASK);
>>
>> + if (tb[TCA_CAKE_NAT]) {
>> + q->flow_mode &= ~CAKE_FLOW_NAT_FLAG;
>> + q->flow_mode |= CAKE_FLOW_NAT_FLAG *
>> + !!nla_get_u32(tb[TCA_CAKE_NAT]);
>> + }
>
>
> I think it's better to return -EOPNOTSUPP when CONFIG_NF_CONNTRACK
> is not enabled.
Good point, will fix :)
-Toke
^ permalink raw reply
* Re: [PATCH net-next v12 2/7] sch_cake: Add ingress mode
From: Toke Høiland-Jørgensen @ 2018-05-16 21:16 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <CAM_iQpVz9bRDkW_cqj-vo6fapXaG6zV-bWM0Sc2wmHuj-vzWqg@mail.gmail.com>
Cong Wang <xiyou.wangcong@gmail.com> writes:
> On Wed, May 16, 2018 at 1:29 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>> + if (tb[TCA_CAKE_AUTORATE]) {
>> + if (!!nla_get_u32(tb[TCA_CAKE_AUTORATE]))
>> + q->rate_flags |= CAKE_FLAG_AUTORATE_INGRESS;
>> + else
>> + q->rate_flags &= ~CAKE_FLAG_AUTORATE_INGRESS;
>> + }
>> +
>> + if (tb[TCA_CAKE_INGRESS]) {
>> + if (!!nla_get_u32(tb[TCA_CAKE_INGRESS]))
>> + q->rate_flags |= CAKE_FLAG_INGRESS;
>> + else
>> + q->rate_flags &= ~CAKE_FLAG_INGRESS;
>> + }
>> +
>> if (tb[TCA_CAKE_MEMORY])
>> q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
>>
>> @@ -1559,6 +1628,14 @@ static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
>> if (nla_put_u32(skb, TCA_CAKE_MEMORY, q->buffer_config_limit))
>> goto nla_put_failure;
>>
>> + if (nla_put_u32(skb, TCA_CAKE_AUTORATE,
>> + !!(q->rate_flags & CAKE_FLAG_AUTORATE_INGRESS)))
>> + goto nla_put_failure;
>> +
>> + if (nla_put_u32(skb, TCA_CAKE_INGRESS,
>> + !!(q->rate_flags & CAKE_FLAG_INGRESS)))
>> + goto nla_put_failure;
>> +
>
> Why do you want to dump each bit of the rate_flags separately rather than
> dumping the whole rate_flags as an integer?
Well, these were added one at a time, each as a new option. Isn't that
more or less congruent with how netlink attributes are supposed to be
used?
-Toke
^ permalink raw reply
* Re: [PATCH iproute2-next] tc-netem: fix limit description in man page
From: David Ahern @ 2018-05-16 21:17 UTC (permalink / raw)
To: Marcelo Ricardo Leitner, netdev
In-Reply-To: <adc166a5db72cf5c658cbc68a7d733c0793e31d4.1526431697.git.mleitner@redhat.com>
On 5/15/18 6:49 PM, Marcelo Ricardo Leitner wrote:
> As the kernel code says, limit is actually the amount of packets it can
> hold queued at a time, as per:
>
> static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> struct sk_buff **to_free)
> {
> ...
> if (unlikely(sch->q.qlen >= sch->limit))
> return qdisc_drop_all(skb, sch, to_free);
>
> So lets fix the description of the field in the man page.
>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
> man/man8/tc-netem.8 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
applied to iproute2-next. Thanks,
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-05-16 21:23 UTC (permalink / raw)
To: Roman Mashak
Cc: Jamal Hadi Salim, Linux Kernel Network Developers, David Miller,
Cong Wang, Jiri Pirko, pablo, kadlec, fw, ast, Daniel Borkmann,
Eric Dumazet, kliteyn
In-Reply-To: <85efib33kr.fsf@mojatatu.com>
On Wed 16 May 2018 at 17:36, Roman Mashak <mrv@mojatatu.com> wrote:
> Vlad Buslov <vladbu@mellanox.com> writes:
>
>> On Wed 16 May 2018 at 14:38, Roman Mashak <mrv@mojatatu.com> wrote:
>>> On Wed, May 16, 2018 at 2:43 AM, Vlad Buslov <vladbu@mellanox.com> wrote:
>>>>>>>> I'm trying to run tdc, but keep getting following error even on clean
>>>>>>>> branch without my patches:
>>>>>>>
>>>>>>> Vlad, not sure if you saw my email:
>>>>>>> Apply Roman's patch and try again
>>>>>>>
>>>>>>> https://marc.info/?l=linux-netdev&m=152639369112020&w=2
>>>>>>>
>>>>>>> cheers,
>>>>>>> jamal
>>>>>>
>>>>>> With patch applied I get following error:
>>>>>>
>>>>>> Test 7d50: Add skbmod action to set destination mac
>>>>>> exit: 255 0
>>>>>> dst MAC address <11:22:33:44:55:66>
>>>>>> RTNETLINK answers: No such file or directory
>>>>>> We have an error talking to the kernel
>>>>>>
>>>>>
>>>>> You may actually have broken something with your patches in this case.
>>>>
>>>> Results is for net-next without my patches.
>>>
>>> Do you have skbmod compiled in kernel or as a module?
>>
>> Thanks, already figured out that default config has some actions
>> disabled.
>> Have more errors now. Everything related to ife:
>>
>> Test 7682: Create valid ife encode action with mark and pass control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No such file or directory
>> We have an error talking to the kernel
>>
>> Test ef47: Create valid ife encode action with mark and pipe control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No space left on device
>> We have an error talking to the kernel
>>
>> Test df43: Create valid ife encode action with mark and continue control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No space left on device
>> We have an error talking to the kernel
>>
>> Test e4cf: Create valid ife encode action with mark and drop control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No space left on device
>> We have an error talking to the kernel
>>
>> Test ccba: Create valid ife encode action with mark and reclassify control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No space left on device
>> We have an error talking to the kernel
>>
>> Test a1cf: Create valid ife encode action with mark and jump control
>> exit: 255 0
>> IFE type 0xED3E
>> RTNETLINK answers: No space left on device
>> We have an error talking to the kernel
>>
>> ...
>>
>>
>
> Please make sure you have these in your kernel config:
>
> CONFIG_NET_ACT_IFE=y
> CONFIG_NET_IFE_SKBMARK=m
> CONFIG_NET_IFE_SKBPRIO=m
> CONFIG_NET_IFE_SKBTCINDEX=m
>
> For tdc to run all the tests, it is assumed that all the supported tc
> actions/filters are enabled and compiled.
Enabling these options allowed all ife tests to pass. Thanks!
Error in u32 test still appears however:
Test e9a3: Add u32 with source match
-----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 ingress"
-----> prepare stage *** Error message: "Cannot find device "v0p1"
^ permalink raw reply
* Re: [PATCH iproute2-next] tc-netem: fix limit description in man page
From: Stephen Hemminger @ 2018-05-16 21:28 UTC (permalink / raw)
To: David Ahern; +Cc: Marcelo Ricardo Leitner, netdev
In-Reply-To: <700b8303-28df-d991-6410-2f9d06f2b70d@gmail.com>
On Wed, 16 May 2018 15:17:50 -0600
David Ahern <dsahern@gmail.com> wrote:
> On 5/15/18 6:49 PM, Marcelo Ricardo Leitner wrote:
> > As the kernel code says, limit is actually the amount of packets it can
> > hold queued at a time, as per:
> >
> > static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> > struct sk_buff **to_free)
> > {
> > ...
> > if (unlikely(sch->q.qlen >= sch->limit))
> > return qdisc_drop_all(skb, sch, to_free);
> >
> > So lets fix the description of the field in the man page.
> >
> > Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> > ---
> > man/man8/tc-netem.8 | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> applied to iproute2-next. Thanks,
>
Since it is an error, I will put it in master.
^ permalink raw reply
* Re: [PATCH 00/14] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-05-16 21:29 UTC (permalink / raw)
To: Davide Caratti
Cc: Roman Mashak, Jamal Hadi Salim, Linux Kernel Network Developers,
David Miller, Cong Wang, Jiri Pirko, pablo, kadlec, fw, ast,
Daniel Borkmann, Eric Dumazet, kliteyn
In-Reply-To: <bd6cf2d13402fae390e29dba8919a5a388481b5e.camel@redhat.com>
On Wed 16 May 2018 at 18:10, Davide Caratti <dcaratti@redhat.com> wrote:
> On Wed, 2018-05-16 at 13:36 -0400, Roman Mashak wrote:
>> Vlad Buslov <vladbu@mellanox.com> writes:
>>
>> > On Wed 16 May 2018 at 14:38, Roman Mashak <mrv@mojatatu.com> wrote:
>> > > On Wed, May 16, 2018 at 2:43 AM, Vlad Buslov <vladbu@mellanox.com> wrote:
>> > > > > > > > I'm trying to run tdc, but keep getting following error even on clean
>> > > > > > > > branch without my patches:
>> > > > > > >
>> > > > > > > Vlad, not sure if you saw my email:
>> > > > > > > Apply Roman's patch and try again
>> > > > > > >
>> > > > > > > https://marc.info/?l=linux-netdev&m=152639369112020&w=2
>> > > > > > >
>> > > > > > > cheers,
>> > > > > > > jamal
>> > > > > >
>> > > > > > With patch applied I get following error:
>> > > > > >
>> > > > > > Test 7d50: Add skbmod action to set destination mac
>> > > > > > exit: 255 0
>> > > > > > dst MAC address <11:22:33:44:55:66>
>> > > > > > RTNETLINK answers: No such file or directory
>> > > > > > We have an error talking to the kernel
>> > > > > >
>> > > > >
>> > > > > You may actually have broken something with your patches in this case.
>> > > >
>> > > > Results is for net-next without my patches.
>> > >
>> > > Do you have skbmod compiled in kernel or as a module?
>> >
>> > Thanks, already figured out that default config has some actions
>> > disabled.
>> > Have more errors now. Everything related to ife:
>> >
>> > Test 7682: Create valid ife encode action with mark and pass control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No such file or directory
>> > We have an error talking to the kernel
>> >
>> > Test ef47: Create valid ife encode action with mark and pipe control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No space left on device
>> > We have an error talking to the kernel
>> >
>> > Test df43: Create valid ife encode action with mark and continue control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No space left on device
>> > We have an error talking to the kernel
>> >
>> > Test e4cf: Create valid ife encode action with mark and drop control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No space left on device
>> > We have an error talking to the kernel
>> >
>> > Test ccba: Create valid ife encode action with mark and reclassify control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No space left on device
>> > We have an error talking to the kernel
>> >
>> > Test a1cf: Create valid ife encode action with mark and jump control
>> > exit: 255 0
>> > IFE type 0xED3E
>> > RTNETLINK answers: No space left on device
>> > We have an error talking to the kernel
>> >
>> > ...
>> >
>> >
>>
>> Please make sure you have these in your kernel config:
>>
>> CONFIG_NET_ACT_IFE=y
>> CONFIG_NET_IFE_SKBMARK=m
>> CONFIG_NET_IFE_SKBPRIO=m
>> CONFIG_NET_IFE_SKBTCINDEX=m
>>
>> For tdc to run all the tests, it is assumed that all the supported tc
>> actions/filters are enabled and compiled.
> hello,
>
> looking at ife.json, it seems that we have at least 4 typos in
> 'teardown'.
>
> It does
>
> $TC actions flush action skbedit
>
> in place of
>
> $TC actions flush action ife
>
> On my fedora28 (with fedora28 kernel), fixing them made test 7682 return
> 'ok' (and all others in ife category, except ee94, 7ee0 and 0a7d).
>
> regards,
I can confirm that on net-next kernel version that I use, there are also
multiple teardowns of actions type skbedit after actually creating ife
action in file ife.json. However, tests pass when I enabled config
options that Roman suggested:
ok 119 - 7682 # Create valid ife encode action with mark and pass control
^ permalink raw reply
* Re: [PATCH bpf-next v6 1/4] bpf: sockmap, refactor sockmap routines to work with hashmap
From: John Fastabend @ 2018-05-16 21:46 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: netdev, davem
In-Reply-To: <954e3b7f-9001-e528-c6ab-f8f69c84cfd8@iogearbox.net>
On 05/15/2018 12:19 PM, Daniel Borkmann wrote:
> On 05/14/2018 07:00 PM, John Fastabend wrote:
> [...]
[...]
>
> As you say in the comment above the function wrt locking notes that the
> __sock_map_ctx_update_elem() can be called concurrently.
>
> All operations operate on sock_map using cmpxchg and xchg operations to ensure we
> do not get stale references. Any reads into the map must be done with READ_ONCE()
> because of this.
>
> You initially use the READ_ONCE() on the verdict/parse/tx_msg, but later on when
> grabbing the reference you use again progs->bpf_verdict/bpf_parse/bpf_tx_msg which
> would potentially refetch it, but if updates would happen concurrently e.g. to the
> three progs, they could be NULL in the mean-time, no? bpf_prog_inc_not_zero() would
> then crash. Why are not the ones used that you fetched previously via READ_ONCE()
> for taking the ref?
Nice catch. We should use the reference fetched by READ_ONCE in all cases.
>
> The second question I had is that verdict/parse/tx_msg are updated independently
> from each other and each could be NULL or non-NULL. What if, say, parse is NULL
> and verdict as well as tx_msg is non-NULL and the bpf_prog_inc_not_zero() on the
> tx_msg prog fails. Doesn't this cause a use-after-free since a ref on verdict wasn't
> taken earlier but the bpf_prog_put() will cause accidental misbalance/free of the
> progs?
Also good catch. I'll send patches for both now. Thanks.
>
> It would probably help to clarify the locking comment a bit more if indeed the
> above should be okay as is.
>
> Thanks,
> Daniel
>
^ permalink raw reply
* [bpf PATCH 1/2] bpf: sockmap update rollback on error can incorrectly dec prog refcnt
From: John Fastabend @ 2018-05-16 21:46 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
If the user were to only attach one of the parse or verdict programs
then it is possible a subsequent sockmap update could incorrectly
decrement the refcnt on the program. This happens because in the
rollback logic, after an error, we have to decrement the program
reference count when its been incremented. However, we only increment
the program reference count if the user has both a verdict and a
parse program. The reason for this is because, at least at the
moment, both are required for any one to be meaningful. The problem
fixed here is in the rollback path we decrement the program refcnt
even if only one existing. But we never incremented the refcnt in
the first place creating an imbalance.
This patch fixes the error path to handle this case.
Fixes: 2f857d04601a ("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 098eca5..f03aaa8 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1717,10 +1717,10 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
if (tx_msg) {
tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg);
if (IS_ERR(tx_msg)) {
- if (verdict)
- bpf_prog_put(verdict);
- if (parse)
+ if (parse && verdict) {
bpf_prog_put(parse);
+ bpf_prog_put(verdict);
+ }
return PTR_ERR(tx_msg);
}
}
@@ -1805,10 +1805,10 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
out_free:
smap_release_sock(psock, sock);
out_progs:
- if (verdict)
- bpf_prog_put(verdict);
- if (parse)
+ if (parse && verdict) {
bpf_prog_put(parse);
+ bpf_prog_put(verdict);
+ }
if (tx_msg)
bpf_prog_put(tx_msg);
write_unlock_bh(&sock->sk_callback_lock);
^ permalink raw reply related
* [bpf PATCH 2/2] bpf: parse and verdict prog attach may race with bpf map update
From: John Fastabend @ 2018-05-16 21:46 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <20180516214651.6664.62408.stgit@john-Precision-Tower-5810>
In the sockmap design BPF programs (SK_SKB_STREAM_PARSER and
SK_SKB_STREAM_VERDICT) are attached to the sockmap map type and when
a sock is added to the map the programs are used by the socket.
However, sockmap updates from both userspace and BPF programs can
happen concurrently with the attach and detach of these programs.
To resolve this we use the bpf_prog_inc_not_zero and a READ_ONCE()
primitive to ensure the program pointer is not refeched and
possibly NULL'd before the refcnt increment. This happens inside
a RCU critical section so although the pointer reference in the map
object may be NULL (by a concurrent detach operation) the reference
from READ_ONCE will not be free'd until after grace period. This
ensures the object returned by READ_ONCE() is valid through the
RCU criticl section and safe to use as long as we "know" it may
be free'd shortly.
Daniel spotted a case in the sock update API where instead of using
the READ_ONCE() program reference we used the pointer from the
original map, stab->bpf_{verdict|parse}. The problem with this is
the logic checks the object returned from the READ_ONCE() is not
NULL and then tries to reference the object again but using the
above map pointer, which may have already been NULL'd by a parallel
detach operation. If this happened bpf_porg_inc_not_zero could
dereference a NULL pointer.
Fix this by using variable returned by READ_ONCE() that is checked
for NULL.
Fixes: 2f857d04601a ("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index f03aaa8..583c1eb 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1703,11 +1703,11 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
* we increment the refcnt. If this is the case abort with an
* error.
*/
- verdict = bpf_prog_inc_not_zero(stab->bpf_verdict);
+ verdict = bpf_prog_inc_not_zero(verdict);
if (IS_ERR(verdict))
return PTR_ERR(verdict);
- parse = bpf_prog_inc_not_zero(stab->bpf_parse);
+ parse = bpf_prog_inc_not_zero(parse);
if (IS_ERR(parse)) {
bpf_prog_put(verdict);
return PTR_ERR(parse);
^ 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