Re: [PATCH bpf-next v3 1/3] bpf: Add overwrite mode for BPF ring buffer

[Xu Kuohai]

On 10/28/2025 10:47 AM, Andrii Nakryiko wrote:
> On Fri, Oct 17, 2025 at 9:04 PM Xu Kuohai <xukuohai@xxxxxxxxxxxxxxx> wrote:


[...]

> > @@ -72,6 +73,8 @@ struct bpf_ringbuf {
> >           */
> >          unsigned long consumer_pos __aligned(PAGE_SIZE);
> >          unsigned long producer_pos __aligned(PAGE_SIZE);
> > +       /* points to the record right after the last overwritten one */
> > +       unsigned long overwrite_pos;
>
> I moved this after pending_pos, as all these fields are actually
> exposed to the user space, so didn't want to unnecessarily shift
> pending_pos.

OK, that makes sense

> >          unsigned long pending_pos;
> >          char data[] __aligned(PAGE_SIZE);
> >   };
> > @@ -166,7 +169,7 @@ static void bpf_ringbuf_notify(struct irq_work *work)
> >    * considering that the maximum value of data_sz is (4GB - 1), there
> >    * will be no overflow, so just note the size limit in the comments.
> >    */
> > -static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node)
> > +static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node, bool overwrite_mode)
> >   {
> >          struct bpf_ringbuf *rb;
> >
> > @@ -183,17 +186,25 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node)
> >          rb->consumer_pos = 0;
> >          rb->producer_pos = 0;
> >          rb->pending_pos = 0;
> > +       rb->overwrite_mode = overwrite_mode;
> >
> >          return rb;
> >   }
> >
> >   static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
> >   {
> > +       bool overwrite_mode = false;
> >          struct bpf_ringbuf_map *rb_map;
> >
> >          if (attr->map_flags & ~RINGBUF_CREATE_FLAG_MASK)
> >                  return ERR_PTR(-EINVAL);
> >
> > +       if (attr->map_flags & BPF_F_RB_OVERWRITE) {
> > +               if (attr->map_type == BPF_MAP_TYPE_USER_RINGBUF)
>
> this seemed error prone if we ever add another ringbuf type (unlikely,
> but still), so I inverted this all to allow BPF_F_RB_OVERWRITE only
> for BPF_MAP_TYPE_RINGBUF. We should try to be as strict as possible by
> default.

Got it, thanks.

> > +                       return ERR_PTR(-EINVAL);
> > +               overwrite_mode = true;
> > +       }
> > +
> >          if (attr->key_size || attr->value_size ||
> >              !is_power_of_2(attr->max_entries) ||
> >              !PAGE_ALIGNED(attr->max_entries))
> > @@ -205,7 +216,7 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
> >
> >          bpf_map_init_from_attr(&rb_map->map, attr);
> >
> > -       rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node);
> > +       rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node, overwrite_mode);
> >          if (!rb_map->rb) {
> >                  bpf_map_area_free(rb_map);
> >                  return ERR_PTR(-ENOMEM);
> > @@ -293,13 +304,25 @@ static int ringbuf_map_mmap_user(struct bpf_map *map, struct vm_area_struct *vma
> >          return remap_vmalloc_range(vma, rb_map->rb, vma->vm_pgoff + RINGBUF_PGOFF);
> >   }
> >
> > +/* Return an estimate of the available data in the ring buffer.
>
> Fixed up comment style
>
> [...]
>
> >   static u32 ringbuf_total_data_sz(const struct bpf_ringbuf *rb)
> > @@ -402,11 +425,41 @@ bpf_ringbuf_restore_from_rec(struct bpf_ringbuf_hdr *hdr)
> >          return (void*)((addr & PAGE_MASK) - off);
> >   }
> >
> > +static bool bpf_ringbuf_has_space(const struct bpf_ringbuf *rb,
> > +                                 unsigned long new_prod_pos,
> > +                                 unsigned long cons_pos,
> > +                                 unsigned long pend_pos)
> > +{
> > +       /* no space if oldest not yet committed record until the newest
> > +        * record span more than (ringbuf_size - 1).
> > +        */
>
> same, keep in mind that we now use kernel-wide comment style with /*
> on separate line. Fixed up all other places as well.

Thanks for fixing everything! I think I should update my editor config, it’s
still stuck at an 80-character line width.

> > +       if (new_prod_pos - pend_pos > rb->mask)
> > +               return false;
> > +
> > +       /* ok, we have space in overwrite mode */
> > +       if (unlikely(rb->overwrite_mode))
> > +               return true;
> > +
> > +       /* no space if producer position advances more than (ringbuf_size - 1)
> > +        * ahead of consumer position when not in overwrite mode.
> > +        */
> > +       if (new_prod_pos - cons_pos > rb->mask)
> > +               return false;
> > +
> > +       return true;
> > +}
> > +
>
> [...]