(Cc'ed Lennart)
On Thu, Jul 31, 2014 at 05:40:53PM +0200, Kay Sievers wrote:
> On Wed, Jul 23, 2014 at 6:34 PM, Djalal Harouni <tix...@opendz.org> wrote:
> > This series adds the infrastructure to test and upload multiple
> > policies.
> >
> > The last #5 patch allows to upload multiple policies per connection
>
> What is the reason for this? A policy holding connection (which
> matches a busname unit) shouldn't only be in charge of one single
> name?
Hmm, I thought that what you describe is for activators?
I was following the current kdbus spec/code here!
>From kdbus.h source:
"@KDBUS_HELLO_POLICY_HOLDER: Special-purpose connection which registers
policy entries for one or multiple names. The provided names are not
activated, and are not registered with the name database"
And the logic in policy.c is set that multiple policy entries can have
the same owner which is the policy holding connection:
struct kdbus_policy_db_entry {
char *name;
struct hlist_node hentry;
struct list_head access_list;
const void *owner;
bool wildcard:1;
};
This should allow one single connection to handle multiple policy
entries, I think the design is good, since policy entries are then
handled internally by kdbus, so only one single connection resources
to achieve this, which can be reliable on private domains,buses...
The policy holding connection acts like create policy entries, and
invalidate them when it is closed!
Still I see three points here from how much pressure and job should
the policy holding connection do!
1) Register policy entries (handled internally), no communication
2) Register policy entries + do basic communication based on ID
3) Register policy entries + acquire name or names + do communication
based on names...
All of these points are from the perspective: if the policy holding
connection dies, all the registered policy entries will go away, thus
invalidating all communications to the registered names...
Should we care ?
And for that patch change, it will block activators from having
multiple names, it will fail, but it will succeed for policy holders.
Hope I'm not out of scope!
> > The todo for the policy holders is:
> >
> > * Should we set a maximum value for how many names/policies a policy
> > holder is allowed to upload. This is needed since we do not want to
> > consume all the entries (kdbus_policy_db->entries_hash).
>
> Yes, everything that consumes kernel memory needs to be limited.
Ok, so this needs a proper discussion, will send about it and Cc Lennart
too.
So perhaps do not apply this series until we have a proper limit.
> > * Update/test the kdbus_policy_set() to work with multiple names.
>
> This is meant for custom endpoints, not for policy-holder connections?
Currently it is aimed to work for both of them!
I've tested for policy-holder connections and it works with this series
applied, but only when we create new connections. It will not work when
we try to update or register new properties, policies or names.
So from the source:
connection.c:1978 it will fail, but with this patch series it will
succeed.
connection.c:1801 it will fail in kdbus_cmd_conn_update()
Since it allows only to add/update one policy entry. So this one will be
a todo item.
And for custom endpoints, I'm sure that kdbus_policy_set() is broken, and
it was perhaps never tested since custom endpoints are still broken, we
can't create them, at least that was the case the last time I checked.
Kay I'm trying to have a kdbus_policy_set() version only for
connections, so it will work for policy holding connections that handle
multiple names! this will help to fix the sending cache issue I've been
talking about.
So please, do confirm that a policy holding connection should be able to
register multiple policies for multiple names, before I give it more
time. Thank you!
The kdbus_policy_set() for custom endpoints can be worked out later...
> Thanks,
> Kay
--
Djalal Harouni
http://opendz.org
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
