[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYgxueymz1xUeQMAkFEr1dW4T=kotw4PssE3cVKh7RMfw@mail.gmail.com>
Date: Thu, 14 May 2020 13:11:39 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jonathan Lemon <bsd@...com>
Cc: Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
Networking <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...com>,
Daniel Borkmann <daniel@...earbox.net>,
Kernel Team <kernel-team@...com>,
"Paul E . McKenney" <paulmck@...nel.org>,
Jonathan Lemon <jonathan.lemon@...il.com>
Subject: Re: [PATCH bpf-next 1/6] bpf: implement BPF ring buffer and verifier
support for it
On Thu, May 14, 2020 at 9:50 AM Jonathan Lemon <bsd@...com> wrote:
>
> On Wed, May 13, 2020 at 12:25:27PM -0700, Andrii Nakryiko wrote:
> > +static struct bpf_ringbuf *bpf_ringbuf_restore_from_rec(void *meta_ptr)
> > +{
> > + unsigned long addr = (unsigned long)meta_ptr;
> > + unsigned long off = *(u32 *)(meta_ptr + 4) << PAGE_SHIFT;
> > +
> > + return (void*)((addr & PAGE_MASK) - off);
> > +}
> > +
> > +static void *__bpf_ringbuf_reserve(struct bpf_ringbuf *rb, u64 size)
> > +{
> > + unsigned long cons_pos, prod_pos, new_prod_pos, flags;
> > + u32 len, pg_off;
> > + void *meta_ptr;
> > +
> > + if (unlikely(size > UINT_MAX))
> > + return NULL;
>
> Size should be 30 bits, not UINT_MAX, since 2 bits are reserved.
Oh, good catch, thanks. Yep, should be (UINT_MAX >> 2), will add a
constant for this.
>
> > +
> > + len = round_up(size + RINGBUF_META_SZ, 8);
> > + cons_pos = READ_ONCE(rb->consumer_pos);
> > +
> > + if (in_nmi()) {
> > + if (!spin_trylock_irqsave(&rb->spinlock, flags))
> > + return NULL;
> > + } else {
> > + spin_lock_irqsave(&rb->spinlock, flags);
> > + }
> > +
> > + prod_pos = rb->producer_pos;
> > + new_prod_pos = prod_pos + len;
> > +
> > + /* check for out of ringbuf space by ensuring producer position
> > + * doesn't advance more than (ringbuf_size - 1) ahead
> > + */
> > + if (new_prod_pos - cons_pos > rb->mask) {
> > + spin_unlock_irqrestore(&rb->spinlock, flags);
> > + return NULL;
> > + }
> > +
> > + meta_ptr = rb->data + (prod_pos & rb->mask);
> > + pg_off = bpf_ringbuf_rec_pg_off(rb, meta_ptr);
> > +
> > + WRITE_ONCE(*(u32 *)meta_ptr, RINGBUF_BUSY_BIT | size);
> > + WRITE_ONCE(*(u32 *)(meta_ptr + 4), pg_off);
>
> Or define a 64bit word in the structure and use:
>
> WRITE_ONCE(*(u64 *)meta_ptr, rec.header);
yep, can do that
>
>
> > +
> > + /* ensure length prefix is written before updating producer positions */
> > + smp_wmb();
> > + WRITE_ONCE(rb->producer_pos, new_prod_pos);
> > +
> > + spin_unlock_irqrestore(&rb->spinlock, flags);
> > +
> > + return meta_ptr + RINGBUF_META_SZ;
> > +}
> > +
> > +BPF_CALL_3(bpf_ringbuf_reserve, struct bpf_map *, map, u64, size, u64, flags)
> > +{
> > + struct bpf_ringbuf_map *rb_map;
> > +
> > + if (unlikely(flags))
> > + return -EINVAL;
> > +
> > + rb_map = container_of(map, struct bpf_ringbuf_map, map);
> > + return (unsigned long)__bpf_ringbuf_reserve(rb_map->rb, size);
> > +}
> > +
>
> --
> Jonathan
Powered by blists - more mailing lists