> I don't understand why dladm would need to call delete_linkid if you're
> temporarily deleting a link. I think if you're temporarily deleting a
> link, you'd want to keep the link ID to name mapping around so that the
> link ID is not reused. Even if that wasn't a concern (say if guaranteed
> that we would never reuse link IDs even after the mapping was
> deleted), I still don't see why we couldn't bring the link down without
> deleting the mapping.
>
Well. If this linkid was created for a temporary link only, it should be
deleted when that link is destroyed.
> If knowing the state of the link was important, I would say we should
> track that using the configuration structures. We could have a field
> indicating if a link is active. When the system reboots, the link
> becomes active and the field would be changed to true.
>
But this configuration structure is only used for persist data, right? and
if the link is purely temporary, the configuration structure is not involved
at all.
>> 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).
>>
>
> I could do that, but I think it would be better to track that
> information elsewhere.
>
Where is the elsewhere? How?
>>> walk_confs()
>>>
>> What exactly information it iterates? The linkids? Then it can be
>> achieved by the walkers in the linkid management API.
>>
>
> It walks the configuration data structure for each persistent link. I
> don't think it can be achieved by the walkers in the linkid management
> API because at boot time we need some way of loading the persistent link
> ID to name mappings before we can use the link ID management API walker.
>
> I.e.
>
> in devfsadmd at boot
> walk_confs()
> for each conf
> get_conf_field(conf, "id", &id);
> get_conf_field(conf, "name", &name);
> add_map(id, name); /* this function is part of devfsadmd and
> not any of the API's discussed in this mail */
>
I see.
>>> 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.
>>
>
> I agree with everything you say here, but, there's one problem. How do
> you initialize devfsadmd's internal structures at boot? That's what
> this walker is for.
>
Yes, you are right. But what I tried to discuss with you here is whether
devfdadmd needs to be involved as part of the implementation of
configuration persistence API, and it seems you agreed it should.
>>> 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.
>>
>
> I'm not sure that is necessary because the ID is in the conf structure,
> and I could extract it internally.
>
Right, so that your internal processes is different for "key fields" even
users use the same API to set them. I don't see the benefit.
>>> 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 a link always has the same ID, even across reboots, then we will have
> a collision in this case. At the next reboot, net6 is restored (because
> it was persistent and was never removed form persistent storage) and
> net10 comes up (because it was persisted) and both have the same link ID.
>
I would think that When "net10" was persisted, the commit_conf(conf)
function should return failures.
Thanks
- Cathy
