Dan Groves wrote:
> Hi,
>
> I thought about the proposals below. I'd like to flesh them out a bit.
>
> Comments and suggestions welcome. Names and data types subject to change.
>
> All configuration data is stored in a configuration store, accessed
> through this library. The data can be either persistent or temporary.
> Persistent data will survive through a reboot. Temporary data will not.
>
I thought we agreed that the APIs are only responsible to persist link
configuration, aside from allocating and releasing linkids for temporary links.
Note that today that temporary link configuration is manipulated by sending
various ioctls to the dld and aggr driver. Are we going to replace that?
Further, if we really use this API to manipulate both active and persist
configuration, the following proposal is not sufficient: the current
linkprop releated dladm subcommands already allow explicitly setting and
getting *one* persist linkprop or a active linkprop, but the granulation in
your proposal is per-link, not per-linkprop.
> dladm_get_empty_dlconf(dladm_dlconf_t *conf)
>
> Gets a blank dlconf structure. No changes are made to the configuration
> store. The returned structure must be freed with dladm_free_dlconf.
>
> dladm_set_dlconf_field(dladm_dlconf_t conf, const char *field, char
> *value);
>
> Set the given field in the given link configuration structure to the
> given value
>
the link name and link class are also set by this function? Although I
personally think that those are first-class "fields" of a link compared to
other fields, I am fine to use the same interface to set them.
> dladm_get_dlconf_field(dladm_dlconf_t conf, const char* field, char
> *value, size_t vallen);
>
Same question as above.
> Get the value of the given field from the given link configuration
> object. Store the data in value.
>
> dladm_get_dlconf(dladm_linkid_t id, dladm_dlconf_t *conf, uint_t flags)
>
> Get a copy of the link configuration structure for the given link ID.
> flags indicates if we get the persistent or temporary configuration.
Again, I am not sure whether we will use this interface to get temporary
(active) configuration (other than linkname and link class). But at the same
time, we need an API to get linkname from a linkid for a temporary link,
this might be a reason why we need to differentiate linkname from other
second-class link fields.
> Note this structure is a copy, if the administrator wants any changes to
> take affect or be persisted, they must be commited using
> dladm_commit_dlconf. The link configuration structure must be freed
> with dladm_free_dlconf when the structure is no longer needed.
>
> dladm_delete_dlconf(dladm_linkid_t id, uint_t flags);
>
> Delete the link configuration from the configuration store and the link
> ID to name mapping for the given ID. flags indicates if we delete the
> persistent or the temporary configuration. If the caller wants to
> delete the temporary configuration, and both a temporary and persistent
> configuration for this link exists, then the persistent configuration as
> well as the name to ID mapping remains. Also note that the caller can
> delete both the persistent and temporary configuration if both exist.
>
I think this function name is confusing, because there is no dladm_dlconf_t
involved. Otherwise it is fine, and I can understand the need for the flags
argument of this function.
> dladm_free_dlconf(dladm_dlconf_t conf)
>
> Free any dynamically allocated memory associated with the given link
> configuration structure. This does not affect the configuration store.
>
> dladm_commit_dlconf(dladm_dlconf_t conf, uint_t flags);
>
> Commit a copy of the given structure to the configuration storage. flags
> indicates if the data should persist across reboots. It is possible to
> have both persistent and temporary configurations for a given link. A
> commit will overwrite whatever data is currently in the configuration
> store for the given link at the same persistence level. For example, if
> a persistent configuration exists for link A in the configuration store,
> and dladm_commit_dlconf is called for link A for a temporary
> configuration, then the temporary configuration will be stored and the
> persistent configuration will not be changed.
>
> Note that if no name to link ID mapping exists for this link, then one
> is created.
>
> dladm_get_linkid(const char* name, linkid_t *id);
>
> Get the link ID for the given name.
>
> dladm_lock_dlconf(uint_t flags);
>
> Lock the configuration store to allow atomic access. Flags indicates if
> it is a reader/writer lock.
>
> dladm_unlock_dlconf();
>
> Unlock the configuration store.
>
> A few pseudocode examples of how to use the API:
>
> Create a link:
>
> dladm_get_empty_dlconf(&conf);
> dladm_set_dlconf_field(conf, "name", "net0");
> dladm_set_dlconf_field(conf, "phys", "bge0");
> dladm_set_dlconf_field(conf, "class", "aggr");
> dladm_lock_dlconf(WRITE);
> dladm_commit_dlconf(conf, PERSIST);
> dladm_unlock_dlconf();
>
> A little more involved than my original proposal, but notice that if we
> need to know more options at create time, it is very easy to add them
> (just change the caller to call dladm_set_dlconf_field for any of the
> new options). We also only have one place where we specify if a link is
> persistent or temporary.
>
Does the above example only create the persist link but not the active one?
I would assume that if the link is both active and persist, then the second
argument of dladm_commit_dlconf() is (PERSIST | ACTIVE).
Further, in the new proposal, what's the step to allocate linkid for a
temporary link?
Thanks
- Cathy
> Rename a net0 to net1:
>
> dladm_lock_dlconf(WRITE);
>
> dladm_get_linkid("net1", &id);
> if (id != INVALID_LINKID && !force)
> dladm_unlock_dlconf();
> return (EEXISTS);
>
> dladm_get_linkid("net0", &id);
>
> dladm_get_dlconf(id, &conf, PERSIST);
> dladm_delete_dlconf(id, PERSIST|TEMP);
> dladm_set_dlconf_field(conf, "name", "net1");
> dladm_commit_dlconf(conf, PERSIST);
> dladm_unlock_dlconf();
> dladm_free_dlconf(conf);
>
> First we check to see if net1 exists. If net1 exists, we only perform
> the rename operation if the administrator really wants to. Then we get
> a copy of net0's configuration. We remove net0's configuration, and
> make its ID available for use. We give net0's configuration a new name,
> and commit it.
>
> In the case where net1 does not exist, when we commit, we would reuse
> net0's ID. Because the store is locked, no one else can commit a new
> configuration, so no one else can use the ID. If net1 exists, but we're
> performing the operation anyway, we would simply overwrite net1's
> configuration with the new configuration, and update the name to ID
> mapping.
>
> Delete a link's temporary configuration:
>
> dladm_lock_dlconf(WRITE);
> dladm_get_linkid("net1", &id);
> dladm_delete_dlconf(id, TEMP);
> dladm_unlock_dlconf();
>
> Thoughts?
>
>
>
> Dan Groves wrote:
>> Hi Cathy,
>>
>> Cathy Zhou wrote:
>>> Hi Dan,
>>>
>>> I am done with the simple case of the the kernel door upcall and
>>> start to
>>> work on the libdladm door calls.
>>>
>>> First, do we agree that dladm_datalink_xxx() function will only be
>>> used to
>>> persistent link configuration, with the exception that we also needs
>>> function to allocate and free linkids for temporary links? If so, to
>>> get the linkid of a temporarily created links, is the below process
>>> right?
>>>
>>> dladm_datalink_create(name, class, &dh);
>>> dladm_datalink_commit(dh);
>>> dladm_datalink_get_id(name, &linkid);
>>>
>>> It seems complicated.
>>>
>>
>> Yes it does seem complicated. Maybe we could do something like this
>> instead:
>>
>> dladm_get_empty_dlconf(&dh);
>> dladm_set_dlconf_field(dh, "name", "net0");
>> dladm_set_dlconf_field(dh, "device", "bge0");
>> dladm_set_dlconf_field(dh, "class", "aggr");
>> dladm_commit_dlconf(dh, TEMP);
>> dladm_get_linkid("net0", &linkid);
>>
>> There's more function calls, but I think this approach solves a few
>> different problems.
>>
>> Let me explain first what's going with this approach.
>>
>> This approach is similar to how X programming works (or used to work,
>> it's been many years since I've done X programming). When you want to
>> draw something on the screen, you create a Widget. The Widget is a
>> data structure that holds information about what it is you want to
>> draw. When you create the Widget, the X server doesn't draw anything
>> on the screen. You set options on the Widget, and then you tell the X
>> server to draw the Widget.
>>
>> The first thing in my example is dladm_get_empty_dlconf. This get you
>> an empty data link configuration structure. At this point there has
>> been no link name to link id mapping created.
>>
>> Then you set the fields of the structure. At a minimum for this
>> version, when you want to create a new link you need to set name and
>> class (and possibly physical device). Right now we're talking about
>> having different ID spaces for different classes of links, i.e.
>> aggregations have a particular range, and other links have another
>> range. So right now with the create function, you need to know the
>> link's name and class. What if we need to know more in the future?
>> Would we change the create function? This method we don't need to
>> change the function signature, we just add more dladm_set_dlconf_field
>> calls to the API's user.
>>
>> I think this also provides a solution to the persistent vs. temporary
>> link problem. When dladm_commit_dlconf is called in my example, two
>> things happen. The link ID to name mapping is created (if one does
>> not exist for this link), and the link configuration structure is
>> stored. The mapping and the storage are either persistent or temporary
>> based on the argument to the function. If it is temporary, neither
>> the mapping nor the configuration persist across reboots. If it is
>> persistent, then they survive across reboots.
>>
>> What happens if an administrator wants to have both a persistent and a
>> temporary configuration? We could assume the temporary configuration
>> takes precedence over the persistent configuration when both exist.
>>
>> While this is a radical change, I think it makes more sense, and is
>> more extensible. What do you think?
>>
>>> Second, I'd like to add an argument to indicate whether the lock is
>>> read or
>>> write in the dladm_datalink_lock() function.
>>>
>>
>> Sure.
>>
>>
>>> Third, I am not sure what's conclusion that whether the
>>> dladm_datalink_create() takes the argument to indicate a link is
>>> persistent or temporary, or dladm_datalink_commit() takes that argument?
>>>
>>> Please let me know.
>>>
>>
>> Yesterday I was working on incorporating the feedback I received from
>> you and Meem before the break. I came up with a solution to the
>> problem, but now I don't think it is a good solution. I think the
>> solution above is a better one. Let me know what you think. If you
>> want me to pursue this new idea, I will.
>>
>> thanks,
>> Dan
>>
>>
>> _________________________________
>> clearview-discuss mailing list
>> clearview-discuss at opensolaris.org
>
