Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-17 Thread Y Song
On Thu, May 17, 2018 at 2:45 PM, Sean Young  wrote:
> Hi,
>
> Again thanks for a thoughtful review. This will definitely will improve
> the code.
>
> On Thu, May 17, 2018 at 10:02:52AM -0700, Y Song wrote:
>> On Wed, May 16, 2018 at 2:04 PM, Sean Young  wrote:
>> > 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 
>> > ---
>> >  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 ..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 
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#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 

Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-17 Thread Sean Young
Hi,

Again thanks for a thoughtful review. This will definitely will improve
the code.

On Thu, May 17, 2018 at 10:02:52AM -0700, Y Song wrote:
> On Wed, May 16, 2018 at 2:04 PM, Sean Young  wrote:
> > 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 
> > ---
> >  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 ..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 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#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:
> > +   

Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-17 Thread Y Song
On Wed, May 16, 2018 at 2:04 PM, Sean Young  wrote:
> 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 
> ---
>  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 ..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 
> +
> +#include 
> +#include 
> +#include 
> +#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 _repeat_proto;
> +   case BPF_FUNC_rc_keydown:
> +   return _keydown_proto;
> +   case BPF_FUNC_map_lookup_elem:
> +   return _map_lookup_elem_proto;
> +   case BPF_FUNC_map_update_elem:
> +   return _map_update_elem_proto;
> +   case BPF_FUNC_map_delete_elem:
> +   return _map_delete_elem_proto;
> +   case BPF_FUNC_ktime_get_ns:
> 

Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-17 Thread Sean Young
Hi Quentin,

On Thu, May 17, 2018 at 01:10:56PM +0100, Quentin Monnet wrote:
> 2018-05-16 22:04 UTC+0100 ~ Sean Young 
> > 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 
> > ---
> >  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
> > 
> 
> [...]
> 
> Hi Sean,
> 
> Please find below some nitpicks on the documentation for the two helpers.

I agree with all your points. I will reword and fix this for v4.

Thanks,

Sean
> 
> > 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
> 
> [...]
> 
> > @@ -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
> 
> Could you please use bold RST markup for constants and function names?
> Typically for BPF_PROG_TYPE_RAWIR_EVENT here and the enum below.
> 
> > + * decoded scancode. This is will generate a keydown event,
> 
> s/This is will/This will/?
> 
> > + * 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).
> 
> This documentation is intended to be compiled as a man page. Could you
> please use a complete sentence here?
> Also, this could do with additional markup as well: **struct
> bpf_rawir_event**.
> 
> > + *
> > + * 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
> 
> The "Return" lines here and in the second helper use space indent
> instead as tabs (as all other lines do). Would you mind fixing it for
> consistency?
> 
> > + * Always return 0 (for now)
> 
> Other helpers use just "0" in that case, but I do not really mind.
> Out of curiosity, do you have anything specific in mind for changing the
> return value here in the future?

I don't expect this is to change, so I should just "0".

> 
> > + *
> > + * 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,
> 
> s/repeat/repeating/?
> 
> > + * in case user is holding a button down; the scancode is
> > + * not repeated.
> > + *
> > + * *ctx* pointer to bpf_rawir_event.
> 
> Please use a complete sentence here as well, if you do not mind.
> 
> > + *
> > + * Return
> > + * Always return 0 (for now)
> >   */
> Thanks,
> Quentin


Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-17 Thread Quentin Monnet
2018-05-16 22:04 UTC+0100 ~ Sean Young 
> 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 
> ---
>  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
> 

[...]

Hi Sean,

Please find below some nitpicks on the documentation for the two helpers.

> 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

[...]

> @@ -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

Could you please use bold RST markup for constants and function names?
Typically for BPF_PROG_TYPE_RAWIR_EVENT here and the enum below.

> + *   decoded scancode. This is will generate a keydown event,

s/This is will/This will/?

> + *   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).

This documentation is intended to be compiled as a man page. Could you
please use a complete sentence here?
Also, this could do with additional markup as well: **struct
bpf_rawir_event**.

> + *
> + *   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

The "Return" lines here and in the second helper use space indent
instead as tabs (as all other lines do). Would you mind fixing it for
consistency?

> + *   Always return 0 (for now)

Other helpers use just "0" in that case, but I do not really mind.
Out of curiosity, do you have anything specific in mind for changing the
return value here in the future?

> + *
> + * 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,

s/repeat/repeating/?

> + *   in case user is holding a button down; the scancode is
> + *   not repeated.
> + *
> + *   *ctx* pointer to bpf_rawir_event.

Please use a complete sentence here as well, if you do not mind.

> + *
> + * Return
> + *   Always return 0 (for now)
>   */
Thanks,
Quentin


[PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT

2018-05-16 Thread Sean Young
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 
---
 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 ..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 
+
+#include 
+#include 
+#include 
+#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 _repeat_proto;
+   case BPF_FUNC_rc_keydown:
+   return _keydown_proto;
+   case BPF_FUNC_map_lookup_elem:
+   return _map_lookup_elem_proto;
+   case BPF_FUNC_map_update_elem:
+   return _map_update_elem_proto;
+   case BPF_FUNC_map_delete_elem:
+   return _map_delete_elem_proto;
+   case BPF_FUNC_ktime_get_ns:
+   return _ktime_get_ns_proto;
+   case BPF_FUNC_tail_call:
+   return _tail_call_proto;
+   case BPF_FUNC_get_prandom_u32:
+   return _get_prandom_u32_proto;
+   case BPF_FUNC_trace_printk:
+   if (capable(CAP_SYS_ADMIN))
+   return