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.
[...]
