On Wed, Apr 21, 2021 at 10:46 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Tue, Apr 20, 2021 at 9:46 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Tue, Apr 20, 2021 at 06:34:11PM -0700, Yonghong Song wrote:
> > > >
> > > > kconfig, typeless ksym, struct_ops and CO-RE are not supported yet.
> > >
> > > Beyond this, currently libbpf has a lot of flexibility between prog open
> > > and load, change program type, key/value size, pin maps, max_entries, reuse
> > > map, etc. it is worthwhile to mention this in the cover letter.
> > > It is possible that these changes may defeat the purpose of signing the
> > > program though.
> >
> > Right. We'd need to decide which ones are ok to change after signature
> > verification. I think max_entries gotta be allowed, since tools
> > actively change it. The other fields selftest change too, but I'm not sure
> > it's a good thing to allow for signed progs. TBD.
> >
>
> [...]
>
> >
> > > > +static void mark_feat_supported(enum kern_feature_id last_feat)
> > > > +{
> > > > + struct kern_feature_desc *feat;
> > > > + int i;
> > > > +
> > > > + for (i = 0; i <= last_feat; i++) {
> > > > + feat = &feature_probes[i];
> > > > + WRITE_ONCE(feat->res, FEAT_SUPPORTED);
> > > > + }
> > >
> > > This assumes all earlier features than FD_IDX are supported. I think this is
> > > probably fine although it may not work for some weird backport.
> > > Did you see any issues if we don't explicitly set previous features
> > > supported?
> >
> > This helper is only used as mark_feat_supported(FEAT_FD_IDX)
> > to tell libbpf that it shouldn't probe anything.
> > Otherwise probing via prog_load screw up gen_trace completely.
> > May be it will be mark_all_feat_supported(void), but that seems less flexible.
>
>
> mark_feat_supported() is changing global state irreversibly, which is
> not great. I think it will be cleaner to just pass bpf_object * into
> kernel_supports() helper, and there return true if obj->gen_trace is
> set. That way it won't affect any other use cases that can happen in
> the same process (not that there are any right now, but still). I
> checked and in all current places there is obj available or it can be
> accessed through prog->obj, so this shouldn't be a problem.
sure. Will use that.
