Hello,
Rough take after skimming:
* I don't have an overall objection. In terms of behavior, the only thing
which stood out was input rejection depending on the current usage. The
preferred way of handling that is rejecting future allocations rather than
failing configuration as that makes it impossible e.g. to lower limit and
drain existing usages from outside the container.
* However, the boilerplate to usefulness ratio doesn't look too good and I
wonder whether what we should do is adding a generic "misc" controller
which can host this sort of static hierarchical counting. I'll think more
on it.
Thanks.
--
tejun
On 09.12.20 21:58, Tejun Heo wrote:
> Hello,
>
> Rough take after skimming:
>
> * I don't have an overall objection. In terms of behavior, the only thing
> which stood out was input rejection depending on the current usage. The
> preferred way of handling that is rejecting future allocations rather than
> failing configuration as that makes it impossible e.g. to lower limit and
> drain existing usages from outside the container.
>
> * However, the boilerplate to usefulness ratio doesn't look too good and I
> wonder whether what we should do is adding a generic "misc" controller
> which can host this sort of static hierarchical counting. I'll think more
> on it.
We first dicussed to have
encryption_ids.stat
encryption_ids.max
encryption_ids.current
and we added the sev in later, so that we can also have tdx, seid, sgx or whatever.
Maybe also 2 or more things at the same time.
Right now this code has
encryption_ids.sev.stat
encryption_ids.sev.max
encryption_ids.sev.current
And it would be trivial to extend it to have
encryption_ids.seid.stat
encryption_ids.seid.max
encryption_ids.seid.current
on s390 instead (for our secure guests).
So in the end this is almost already a misc controller, the only thing that we
need to change is the capability to also define things other than encryption.*.*
And of course we would need to avoid adding lots of random garbage to such a thing.
But if you feel ok with the burden to keep things kind of organized a misc
controller would certainly work for the encryption ID usecase as well.
So I would be fine with the thing as is or a misc controlĺer.
Christian
On Thu, 10 Dec 2020, Christian Borntraeger wrote:
> > * However, the boilerplate to usefulness ratio doesn't look too good and I
> > wonder whether what we should do is adding a generic "misc" controller
> > which can host this sort of static hierarchical counting. I'll think more
> > on it.
>
> We first dicussed to have
> encryption_ids.stat
> encryption_ids.max
> encryption_ids.current
>
> and we added the sev in later, so that we can also have tdx, seid, sgx or whatever.
> Maybe also 2 or more things at the same time.
>
> Right now this code has
>
> encryption_ids.sev.stat
> encryption_ids.sev.max
> encryption_ids.sev.current
>
> And it would be trivial to extend it to have
> encryption_ids.seid.stat
> encryption_ids.seid.max
> encryption_ids.seid.current
> on s390 instead (for our secure guests).
>
> So in the end this is almost already a misc controller, the only thing that we
> need to change is the capability to also define things other than encryption.*.*
> And of course we would need to avoid adding lots of random garbage to such a thing.
>
> But if you feel ok with the burden to keep things kind of organized a misc
> controller would certainly work for the encryption ID usecase as well.
> So I would be fine with the thing as is or a misc controlĺer.
>
Yeah, I think generalization of this would come in the form of either (1)
the dumping ground of an actual "misc" controller, that you elude to, or
(2) a kernel abstraction so you can spin up your own generic controller
that has the {current, max, stat} support. In the case of the latter,
encryption IDs becomes a user of that abstraction.
Concern with a single misc controller would be that any subsystem that
wants to use it has to exactly fit this support: current, max, stat,
nothing more. The moment a controller needs some additional support, and
its controller is already implemented in previous kernel versionv as a
part of "misc," we face a problem.
On the other hand, a kernel abstraction that provides just the basic
{current, max, stat} support might be interesting if it can be extended by
the subsystem instance using it.