Re: [PATCH bpf-next] bpf: Support default .validate() and .update() behavior for struct_ops links

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/11/23 4:36 PM, David Vernet wrote:
I see, thanks for explaining. This is why sched_ext doesn't really work
with the BPF_F_LINK version of map update. We can't guarantee that a map
can be updated if we can't succeed in ->reg(), because we can also race
with e.g. sysrq unloading the scheduler between ->validate() and
->reg(). In a sense, it feels like ->reg() in "updateable" struct_ops
implementations should be void, whereas in other struct_ops
implementations like scx() it has to be int *. If validate() is meant to
prevent the scenario you outlined, can you help me understand why we
still check the return value of ->reg() in bpf_struct_ops_link_create()?
Or at the very least it seems like we should WARN_ON()?

->regs() can fail if another struct_ops under the same name has already been loaded to the subsystem. If another subsystem needs another return value to support .update, I believe it can be done if that is blocking scx to support "updateable" link.

If it needs to validate struct_ops as a while,

There was a typo: as a /whole/.


1. it must be implemented in .validate instead of .reg. Otherwise, it may
end up having an unusable map.

Some clarity on this point (why we check ->reg() on the ->validate()
path) would help me write this comment more clearly.


hmm... where does it check ->reg() on the ->validate() now?

I was meaning the struct_ops supported subsystem should validate the struct_ops map in '.validate' instead of in the '.reg'.

or I misunderstood the question?


2. if the validation is implemented in '.reg' only, the map update behavior
will be different between BPF_F_LINK map and the non BPF_F_LINK map.

Ack, this is good to document regardless.

I'll send a v3 on Monday with these comments added both to the code, and
to the commit summary.

Thanks,
David





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux