Hi,
Sebastien Roy wrote:
> Dan,
>
> Dan Groves wrote:
>> I've posted a new version of the link ID management API document to:
>>
>> http://www.opensolaris.org/os/project/clearview/uv/link_id_management.pdf
>
> This is looking good. A few comments:
>
> Section 2:
>
> * The daemon is called linkmgmtd, but the service should probably have a
> more friendly name or one that is not equal to the daemon's name (which
> is an implementation detail of the service). I don't really feel
> strongly about this, but there seems to be some precedent behind
> separating the service name from the names of the programs used to
> implement it.
>
We plan to change the name of the daemon and/or the service as one of
the last things we do. Cathy and I haven't talked about what new names
would be, but we've had some suggestions already.
I did notice one mistake in this section, the new service is (currently)
network/linkmgmtd, which makes more sense than system/linkmgmtd. I've
made this change in the document.
Once we settle on final names, I'll make those changes in the document.
>
> Section 2.2:
>
> * The table on page 3 still lists the link id as a property
>
>
I've taken care of that, the change will be in the next version of the
document which I will post soon.
> Section 2.3:
>
> * 2nd paragraph: The warning about it being a dangerous operation is a
> bit understated, I think. It's not just that administrators can't
> create links, but devices can't attach either, so opening links for
> devices that haven't attached (such as plumbing an IP interface over a
> device that hasn't been used since the system has booted) won't work
> either. I know this still implicitly constitutes "creating a link", but
> readers might not read it that way.
>
I changed the sentence from:
Note that this is a dangerous operation, without linkmgmtd
running the administrator cannot modify any link con?guration data, the
administrator cannot create links, and there is nothing to respond to
door upcalls from the kernel.
to:
Note that this is a dangerous operation, without linkmgmtd
running the administrator cannot modify any link con?guration data, the
administrator cannot create links, physical devices cannot attach, IP
interfaces cannot be plumbed via ifconfig(1M), and there is nothing to
respond to door upcalls from the kernel.
> * The sentence that starts with "This way, temporary ..." is a bit
> awkward. I know what you meant, but as written, one could interpret
> this as, "In the event of a system crash or reboot, temporary link
> information is restored".
>
I changed the sentence from:
This way, temporary link information is restored and only lost
in the event of a system crash or reboot.
to:
Temporary link information is then restored in the event of a linkmgmtd
crash; however, temporary link information will always be lost in the
event of a system crash or reboot.
>
> Section 3.1.1:
>
> * I'm a confused between the distinction between PHYS, ETHER, and WIFI.
> To me, both WIFI and ETHER are physical classes. I believe the code
> was recently modified to represent this relationship by getting rid of
> PHYS from the enum, and having PHYS simply be a macro consisting of the
> classes that represent physical classes. It's possible that this
> document simply needs to be updated to reflect what's been done with the
> implementation.
>
Yes, I should update this section. I'll keep the list, and change:
This type enumerates the di?erent link classes supported. The
enumeration consists of:
to:
This type identifies different link classes or groups of link classes
supported. The type has the following values:
> Section 3.2.1:
>
> * If the mapping isn't persistent until a configuration is persisted
> using dladm_*_conf(), then what is the purpose of DLADM_OPT_PERSIST in
> dladm_create_datalink_id()?
>
Originally it was going to be for bookkeeping. In other words, can we
reuse the link ID for persistent links when the administrator deletes
the link? But see my response to your next comment.
> * Setting both DLADM_OPT_PERSIST and DLADM_OPT_TEMP is acceptable and
> does not result in DLADM_STATUS_BADARG in the implementation. Does the
> document reflect what will be true in the future for the implementation
> in this case, or does the document need to be changed to reflect the
> implementation?
>
I think this is a mistake. I remember Cathy and I had talked at one
time about dropping DLADM_OPT_TEMP, which indicates that a link id
temporary and won't survive past a reboot, and adding DLADM_OPT_ACTIVE,
which indicates that a link is "active". In that case, setting both
DLADM_OPT_PERSIST and DLADM_OPT_ACTIVE won't return an error. I
remember making that change in one version of the document, which I
don't think I published. When I got the code and the document in synch
before releasing this version, I must have switched it back to
DLADM_OPT_TEMP, because that is what we use in the current version of
the code. DLADM_OPT_ACTIVE makes more sense to me, but I'll wait for
Cathy to get back from vacation before I make any changes in this area.
This might also help with resolving your previous comment.
thanks for the comments,
Dan
> That's all, good stuff.
> -Seb
