> Hi Cathy,
>
> After the conversation we had on the phone today, I thought about the
> idea I mentioned to you of having two API's, one for link ID management
> and one for persisting configuration.
>
Like we discussed, I like this idea to split the linkid management API and
configuration persistence API.
> Here's how I fleshed it out. Any thoughts would be appreciated,
> especially on what the names should be.
>
> Link ID management:
>
> get_new_linkid(name, class)
>
> Create a link ID to name mapping for the given name.
>
> delete_linkid(...)
>
> Delete a link ID to name mapping. The argument could be either a link
> ID or a name.
>
> id2name(id)
>
> Return the name associated with a given ID
>
> name2id(name)
>
> Return the ID associated with a given name
>
> rename(name1, name2)
>
> Change the link ID to name mapping such that the ID for name1 now refers
> to name2. Note that the link's ID does not change, only the name
> associated with it. The rationale behind the rename call is that the
> persistent configuration API does not affect the link ID to name mapping
> that's currently in memory.
>
One thing regarding the above API is whether the link management module
needs to have the knowledge of whether the link is active or not or the
consumer need to track that knowledge. For example:
step 1. "dladm create-aggr -d bge1 aggr1" to create both the active and the
persistent aggregation, so it calls:
linkid = get_new_link("aggr1", CLASS_AGGR);
configuration persistence functions to persist this link.
step 2. "dladm delete-aggr -t aggr1" to delete it temporarily:
Now the problems comes because the consumer needs to have the knowledge
whether to call delete_linkid() or not.
Therefore, I'd suggest get_new_linkid() and delete_linkid() to have a flag
to indicate to create or delete linkids for which one (or both, active and
persistent links).
> We can add walkers if necessary.
>
The walkers should be iterate the linkids. Again, it can take the
persistent/active flag as an argument.
The difference between the current proposal and the original ones is that we
never need to inform management module other link attributes other than link
id and link names for non-persistent links.
> All changes made with this API are temporary unless persisted. We could
> implement these functions as wrappers around the door calls you've
> already implemented, we'd just need to add one for the rename function.
>
> Any configuration information that is temporary can be stored or
> manipulated in any way the caller desires, except that the caller cannot
> change the link ID to name mapping except through the above API. If the
> caller desires to persist the information across reboots, then the
> caller must use the following API:
>
> Persistent link configuration API
>
> get_new_conf()
>
> Returns a link configuration structure. Does not affect persistent
> storage.
>
> set_conf_field(conf, field_name, value)
>
> Set the given field in the given structure to the given value. Does not
> affect persist storage.
>
> get_conf_field(conf, field_name, value, vallen)
>
> Return the value for the given field in the given buffer.
>
> get_conf(...)
>
> Get the configuration structure for the given link. This is a copy of
> what is in persistent storage. The argument could be either a link ID
> or a link name. I lean towards a link ID.
>
> commit_conf(conf)
>
> Commit the given configuration structure to persistent storage.
>
> delete_conf(link)
>
> Delete the given link's configuration information from persistent storage
>
> free_conf(conf)
>
> Free the memory associated with a configuration structure returned by
> either get_conf or get_new_conf. Does not affect persistent storage
>
> walk_confs()
>
What exactly information it iterates? The linkids? Then it can be achieved
by the walkers in the linkid management API.
> Return a configuration struct for each persistent link. I expect that
> devfsadmd would use this on boot to determine what link ID to name
> mappings have been persisted.
>
In theory, as we are separating linkid management and configuration
persistence APIs, the devfsadmd doesn't need to be informed of all the
persistent link attributes at all, and the libdladm can simply do the
persistence work by itself, and not through devfsadmd. But I can see two
benefit that devfsadmd stores persistent link configuration information:
a. those configuration are looked up from both libdladm and the kernel, so
have the configuration in memory will helps the performance.
b. the devfdadmd will need to be used to store and query the persistent
configuration anyway, as requested from the door-upcall from the kernel, so
it might be easy if the all persistence/query requests to be processed in
the same place.
> Judging by the work you've already done in devfsadmd, we might already
> have the implementation or something that can be adapted to the
> implementation for a few of these functions (such as [get|set]_conf_field).
>
> A few examples of usage:
>
> To configure a temporary link:
>
> id = get_new_linkid(name, class)
> /* configure new link here */
>
> To persist a new link:
>
> id = get_new_linkid(name, class);
> conf = get_new_conf();
> set_conf_field(conf, "class", class);
> set_conf_field(conf, "name", name);
> set_conf_field(conf, "id", id);
> commit_conf(conf);
> free_conf(conf);
>
I think it is better make "id" to be the identifier of a persistent link
configuration entry, and unique from other conf_field, so that when you do
the commit_conf(), you know whether it is a new link or it is to update an
existing link.
> Note that until commit_conf is called, the new link will not survive
> across reboots.
>
> To rename a temporary link:
>
> rename("net0", "net1");
>
> To rename a persistent link:
>
> get_conf("net5", &conf);
> /* NOTE: If get_conf were to take an ID, then replace the above line
> with:
>
> get_conf(name2id("net5"), &conf);
>
> Note that get_conf("net6") AFTER calling rename should have the exact
> same effect as what is shown in the example.
> */
> rename("net5", "net6");
> set_conf_field(conf, "name", "net6");
> commit_conf(conf);
> free_conf(conf);
>
> Note that the rename operation is not persisted until the commit_conf
> call returns.
>
> To delete a link:
>
> delete_linkid("net6");
> delete_conf("net6"); /* If the link has been persisted */
>
> I see this as a potential problem. Both steps have to be done if the
> link has been persisted.
If the consumer want to delete the completely, it needs to call both,
otherwise I would consider it as a bug, but not a problem of our API.
> If the API user only does the delete_linkid,
> then the link ID to name mapping is removed, but it still exists in
> persistent storage. For example:
>
> delete_linkid("net6");
> id = get_new_linkid("net10", class); /* Could return net6's link ID! */
>
> If the user persisted this new link ID, we'd have a link ID collision on
> the next reboot.
>
> This also implies that there is no way to safely and temporarily delete
> the link ID to name mapping for a persistent link. Since this is an
> API, I think we could document this limitation.
>
I don't quite understand why.
> If the API user just calls delete_conf without delete_linkid, that
> situation is less harmful. On reboot, since there is no persistent
> configuration for net6, net6's link ID to name mapping is not restored
> and net6's ID is now available.
>
> One thing I do not address here is locking. We should have a mutex lock
> around the persistent configuration calls. I am not sure we need
> special locking for the link ID management calls, I think the locking
> you already have in place in devfsadmd would be sufficient.
Yes. That is right.
Thanks
- Cathy
> What do you think?
>
> Dan
>
>
> _________________________________
> clearview-discuss mailing list
> clearview-discuss at opensolaris.org
