On Wed, Oct 19, 2022 at 07:10:50AM IST, Alexei Starovoitov wrote:
> On Thu, Oct 13, 2022 at 11:52:45AM +0530, Kumar Kartikeya Dwivedi wrote:
> > if (unlikely((map_flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)))
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)))
> > return -EINVAL;
>
> ...
>
> > /* We don't reset or free fields other than timer on uref dropping to zero. */
> > - if (!map_value_has_timer(map))
> > + if (!btf_type_fields_has_field(map->fields_tab, BPF_TIMER))
>
> ...
>
> > - !map_value_has_spin_lock(&smap->map)))
> > + !btf_type_fields_has_field(smap->map.fields_tab, BPF_SPIN_LOCK)))
> > return ERR_PTR(-EINVAL);
>
> ...
>
> > - if (!map_value_has_timer(&htab->map))
> > + if (!btf_type_fields_has_field(htab->map.fields_tab, BPF_TIMER))
> > return;
>
> ...
>
> > if (unlikely(map_flags & BPF_F_LOCK)) {
> > - if (unlikely(!map_value_has_spin_lock(map)))
> > + if (unlikely(!btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)))
> > return -EINVAL;
>
> ...
>
> > - /* We don't reset or free kptr on uref dropping to zero. */
> > - if (!map_value_has_timer(&htab->map))
> > + /* We only free timer on uref dropping to zero */
> > + if (!btf_type_fields_has_field(htab->map.fields_tab, BPF_TIMER))
> > return;
>
> ...
> > if ((elem_map_flags & ~BPF_F_LOCK) ||
> > - ((elem_map_flags & BPF_F_LOCK) && !map_value_has_spin_lock(map)))
> > + ((elem_map_flags & BPF_F_LOCK) && !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)))
> > return -EINVAL;
>
> ...
>
> > if (unlikely((flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)))
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)))
> > return -EINVAL;
>
> ...
>
> > - if (map_value_has_spin_lock(inner_map)) {
> > + if (btf_type_fields_has_field(inner_map->fields_tab, BPF_SPIN_LOCK)) {
> > fdput(f);
> > return ERR_PTR(-ENOTSUPP);
>
> ...
>
> > if ((attr->flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)) {
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)) {
> > err = -EINVAL;
> > goto err_put;
> > }
> > @@ -1440,7 +1428,7 @@ static int map_update_elem(union bpf_attr *attr, bpfptr_t uattr)
> > }
> >
> > if ((attr->flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)) {
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)) {
> > err = -EINVAL;
> > goto err_put;
> > }
> > @@ -1603,7 +1591,7 @@ int generic_map_delete_batch(struct bpf_map *map,
> > return -EINVAL;
> >
> > if ((attr->batch.elem_flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)) {
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)) {
> > return -EINVAL;
> > }
> >
> > @@ -1660,7 +1648,7 @@ int generic_map_update_batch(struct bpf_map *map,
> > return -EINVAL;
> >
> > if ((attr->batch.elem_flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)) {
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)) {
> > return -EINVAL;
> > }
> >
> > @@ -1723,7 +1711,7 @@ int generic_map_lookup_batch(struct bpf_map *map,
> > return -EINVAL;
> >
> > if ((attr->batch.elem_flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map))
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK))
> > return -EINVAL;
> >
> > value_size = bpf_map_value_size(map);
> > @@ -1845,7 +1833,7 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
> > }
> >
> > if ((attr->flags & BPF_F_LOCK) &&
> > - !map_value_has_spin_lock(map)) {
> > + !btf_type_fields_has_field(map->fields_tab, BPF_SPIN_LOCK)) {
>
> All of these btf_type_fields_has_field() is quite an eyesore.
> That was the reason to suggest btf_record_has_field() in the previous email.
I agree, what do you think of calling it btf_type_has_field? You pass in the
btf_type_record and the field type.
