> For the Link ID management API:
 > 
 > dladm_create_linkid()   /* get_new_linkid */
 > dladm_destroy_linkid()  /* delete_linkid */
 > dladm_linkid2name()     /* id2name */
 > dladm_name2linkid()     /* name2id */
 > dladm_change_linkid_mapping()  /* rename */
 > dladm_walk_linkid()  /* The walker mentioned in my reply to Cathy */
 > 
 > For the persistent link configuration API:
 > 
 > dladm_create_conf()     /* get_new_conf */
 > dladm_set_conf_field()  /* set_conf_field */
 > dladm_get_conf_field()  /* get_conf_field */
 > dladm_read_conf()       /* get_conf */
 > dladm_write_conf()      /* commit_conf */
 > dladm_delete_conf()     /* delete_conf */
 > dladm_destroy_conf()    /* free_conf */
 > dladm_walk_conf()       /* walk_confs */

I like those names -- except dladm_change_linkid_mapping(), which feels
a bit bulky.  Maybe:

  dladm_status_t dladm_remap_linkid(dladm_linkid_t id, const char *linkname)

 > I'd prefer to not require class, ID, and name for the create call,
 > because what if in the future we need more required fields in the
 > future, but I see your point.

Do you have any other specific required fields in mind?  Or is more of a
future-proofing of the API?

 > I'll also make the ID and class field read-only so that attempts to set
 > either with dladm_set_conf_field will fail.

Sounds good.

 > > Why couldn't that be detected when the persistence is attempted -- e.g.,
 > > fail the commit/write()?
 >
 > I could add some checks.  We could have the commit check to make sure 
 > the name to ID mapping in the configuration structure matches what the 
 > link ID management API shows.  However, I'm not sure how the 
 > configuration API would tell the difference between this situation and a 
 > rename operation.
 > 
 > For example, we search using an ID to see if there is already an 
 > existing persistent configuration for this ID.  Since net10 has net6's 
 > ID, there is a configuration.  We can tell the names are different, but 
 > how does the API tell the difference between this operation and an 
 > attempt to rename the interface?

My thinking was that the API should never allow a configuration to be
written out that would result in two persisted configurations with the
same linkid.  In the scenario you gave before:

   delete_linkid("net6");
   id = get_new_linkid("net10", class);  /* Could return net6's link ID! */
   create_conf("net10", id, ...)
   write_conf("net10");

... the write_conf() for "net10" would fail because "net6" already has
its persistent linkid.  If someone wanted to do a rename, they'd need to
delete_conf("net6") first.

 > Another problem I thought of as I typed this up is what happens if 
 > devfsadmd crashes and is restarted?  Then we lose all the temporary link 
 > ID to name mappings, unless devfsadmd has some way of storing the 
 > temporary link ID to name mappings.  If we persist them with the 
 > persistence API, the mapping is restored on reboot, which is not what 
 > we'd want.
 > 
 > I think my use of the term persistence could be problematic elsewhere.

This is certainly something worth some thought, but my gut reaction is
that recovery of temporary state across it crashing is no more a
requirement than recovery of temporary state across a kernel crash.

 > Even if we have two processes (or even two threads in the same process) 
 > trying to change the same link's configuration?

There are two separate notions of safety here.  One is that the
configuration is self-consistent and not corrupt.  That is, the API must
not allow a given persistent configuration to appear to be a mongrel
consisting of state that was written by multiple threads competing to
update it.  That is, from the point of view of a thread using the API to
read the persistent configuration, it should always see a single,
consistent view of the persistent configuration.

The other notion of safety is to protect against update collisions (i.e.,
from multiple updaters stomping on one-another) -- e.g., one application
tries to name a link one thing, and the other tries to name a link another
thing.  I think as long as the resulting configuration is self-consistent
(that is, the final link name reflects either one name or the other), this
is not required.

That said, I *think* we'd need to make some changes to the API to satisfy
the first notion of safety.  Specifically, since some changes require
atomically applying changes to several links (e.g., creating an
aggregation with two links), I think read_conf()/write_conf() would need
to operate on the entire persistent configuration, rather than just a
given link.  With that approach, I think it would also be possible to
detect/prevent update collisions -- e.g., you could store a generation
number associated with the configuration that is returned when it is read,
and then bump that number when it's written out.  If the number doesn't
match across the read()/write(), then the writer is forced to go re-read
to pull in the latest changes (and generation number), and then retry the
write() if appropriate.

-- 
meem

Reply via email to