On Thu, May 14, 2020 at 9:50 AM Jonathan Lemon [off-list ref] wrote:

On Wed, May 13, 2020 at 12:25:27PM -0700, Andrii Nakryiko wrote:

quoted

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

quoted

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

quoted

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