Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> writes:
Overall logic seems right to me, a few comments below.
[...]
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 5b25d278409b..d298746f4dcc 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1538,6 +1538,41 @@ struct btf_mod_pair {
>
> struct bpf_kfunc_desc_tab;
>
> +enum bpf_stream_id {
> + BPF_STDOUT = 1,
> + BPF_STDERR = 2,
> +};
> +
> +struct bpf_stream_elem {
> + struct llist_node node;
> + int total_len;
> + int consumed_len;
> + char str[];
> +};
> +
> +struct bpf_stream_elem_batch {
> + struct llist_node *node;
> +};
This type is not used anymore.
> +
> +enum {
> + BPF_STREAM_MAX_CAPACITY = (4 * 1024U * 1024U),
> +};
> +
> +struct bpf_stream {
> + enum bpf_stream_id stream_id;
Nit: `stream_id` is never read, as streams are identified by a position
in the bpf_prog_aux->stream.
> + atomic_t capacity;
> + struct llist_head log;
> +
> + rqspinlock_t lock;
> + struct llist_node *backlog_head;
> + struct llist_node *backlog_tail;
Nit: maybe add comments describing what kind of data is in the llist_{head,node}? E.g.:
atomic_t capacity;
struct llist_head log; /* list of struct bpf_stream_elem in LIFO order */
rqspinlock_t lock; /* backlog_{head,tail} lock */
struct llist_node *backlog_head; /* list of struct bpf_stream_elem in FIFO order */
struct llist_node *backlog_tail;
> +};
> +
> +struct bpf_stream_stage {
> + struct llist_head log;
> + int len;
> +};
> +
[...]
> diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> new file mode 100644
> index 000000000000..b9e6f7a43b1b
> --- /dev/null
> +++ b/kernel/bpf/stream.c
[...]
> +int bpf_stream_stage_commit(struct bpf_stream_stage *ss, struct bpf_prog *prog,
> + enum bpf_stream_id stream_id)
> +{
> + struct llist_node *list, *head, *tail;
> + struct bpf_stream *stream;
> + int ret;
> +
> + stream = bpf_stream_get(stream_id, prog->aux);
> + if (!stream)
> + return -EINVAL;
> +
> + ret = bpf_stream_consume_capacity(stream, ss->len);
> + if (ret)
> + return ret;
> +
> + list = llist_del_all(&ss->log);
> + head = list;
> +
> + if (!list)
> + return 0;
> + while (llist_next(list)) {
> + tail = llist_next(list);
> + list = tail;
> + }
> + llist_add_batch(head, tail, &stream->log);
If `llist_next(list) == NULL` at entry `tail` is never assigned?
> + return 0;
> +}
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d5807d2efc92..5ab8742d2c00 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13882,10 +13882,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> regs[BPF_REG_0].type = PTR_TO_BTF_ID;
> regs[BPF_REG_0].btf_id = ptr_type_id;
>
> - if (meta.func_id == special_kfunc_list[KF_bpf_get_kmem_cache])
> + if (meta.func_id == special_kfunc_list[KF_bpf_get_kmem_cache]) {
> regs[BPF_REG_0].type |= PTR_UNTRUSTED;
> -
> - if (is_iter_next_kfunc(&meta)) {
> + } else if (is_iter_next_kfunc(&meta)) {
Nit: unrelated change?
> struct bpf_reg_state *cur_iter;
>
> cur_iter = get_iter_from_state(env->cur_state, &meta);
[...]
