|
Replies:
26
-
Last Post:
Sep 25, 2009 10:00 AM
by: sowmini
|
|
|
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 20, 2009 9:17 AM
|
|
|
|
|
Posts:
601
From:
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 20, 2009 9:17 AM
in response to: Guest
|
|
>
> On Mon, Jul 06, 2009 at 03:36:11PM -0700, Michael Hunter wrote:
> > This is a request for people to review the NWAM Phase 1 code.
> >
> > The code can be found at http://cr.opensolaris.org/~mph/nwam1_cr/
> >
> > For those inside sun there is a cscope database
> > in /net/coupe.eng/builds/mph/nwam1_cr/usr/src (sorry to those external
> > to SWAN).
> >
> > Documentation can both be found in the source tree and on the project
> > web page at http://opensolaris.org/os/project/nwam/
> >
> > We request code review be finished by Monday July 20th.
> >
> > Michael Hunter
I reviewed the libnwam and netcfgd changes.. overall looks good- the
addition of these features is a big step forward for nwam! here are my
comments.
--Sowmini
---------------------------------------------------------------------------
General:
- what is the difference between the netadm and netcfg uids? The
Brussels II project is considering setting up /sbin/ipadm as owned
by some dladm-like uid, and we had assumed this would be netadm.
Should it be netcfg?
- why is netcfgd under $SRC/cmd/cmd-inet/lib/netcfgd? Seems like this is
parallel to dlmgmtd for layer 3, so to be symmetric
with dlmgtd, shouldn't it be under $SRC/cmd/netcfgd/*? at the very least,
it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
$SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
I don't think these apps get installed in /lib), but I think the
cleaner solution is
$SRC/cmd/nwam/netcfgd
$SRC/cmd/nwam/nwamd
$SRC/cmd/nwam/common /* for common themes like daemonize() */
- it's hard to figure out which libnwam interfaces are "truly" exported,
and which ones are there because they are shared between some daemon
and libnwam. The nwam_backend_init is an example of the latter, which
ends up in mapfile-vers as globally exported just so that the daemon
can also use it. We need to find a better solution for the latter case
(see comments for libnwam.h)
----------------------------------------------------------------------------- --
Reviewed with no comments:
usr/src/cmd/cmd-inet/lib/netcfgd/util.h
usr/src/lib/libnwam/amd64/Makefile
usr/src/lib/libnwam/i386/Makefile
usr/src/lib/libnwam/sparc/Makefile
usr/src/lib/libnwam/sparcv9/Makefile
usr/src/lib/libnwam/Makefile.com
usr/src/lib/libnwam/common/mapfile-vers (except as noted in comments for
libnwam.h)
usr/src/lib/libnwam/common/door.c (deleted file)
usr/src/lib/libnwam/common/libnwam_error.c
usr/src/lib/libnwam/common/libnwam_events.c
usr/src/lib/libnwam/common/libnwam_values.c
usr/src/lib/libnwam/common/libnwam_wlan.c
usr/src/cmd/cmd-inet/lib/Makefile
usr/src/lib/libnwam/README
----------------------------------------------------------------------------- --
usr/src/cmd/cmd-inet/lib/netcfgd/Makefile
- dlmgmtd runs as OWNER dladm, group sys. What are the owner/group
of netcftd (I'd guess that the owner would be netcfg- but inside the
code it seems to use netadm, and invokes getpwam etc. - why the
divergence from dlmgmtd)?
- do we need to define ROOTCFGDIR/<configfile> permissions?
usr/src/cmd/cmd-inet/lib/netcfgd/logging.c
- see relevant comments for netcfgd/main.c
usr/src/cmd/cmd-inet/lib/netcfgd/main.c
- fg can be a local variable to main() - it is not used outside the main()
context.
- debug doesn't need to be extern global, it can be static to logging.c
but perhaps it is not needed at all? How is one supposed to set it?
(One suggestion, always have syslog support, but use SIGUSR1/SIGUSR2
handlers to make it more/less verbose by incr/decr debug)
- zonename does not need to be a global var: it's only usage is for
lookup_zonename, and there it is passed as an argument.
usr/src/lib/libinetcfg/Makefile
- not sure I understand why this is necessary? According to
$SRC/lib/README.Makefiles, after you define HDRS and HDRDIR, you one need
check: $(CHECKHDRS)
Could you explain why this is needed>
usr/src/lib/libinetcfg/common/inetcfg.c
- icfg_get_linkinfo is a dead function - can be removed
- Not your introduction, but icfg_get_flags(), icfg_get_metric(),
icfg_get_mtu(), icfg_get_index() should bzero the lifr
as a first step. Also, these are exported functions that could be
called with a NULL second argument, so they should verify that
the flags/metric/etc. is not null before trying to set them.
- line 1854: list == NULL, can happen if there are no links
available, so I think this should only return ICFG_NO_MEMORY
if lwp->lw_err = ENOMEM. also it may be that we ran out of
memory half way through the walk, so we may still need to free
memory. i.e., we shoudl goto done, and do
for (i = 0; i < lwp->lw_num; i++) {
free(..);
}
- line 1863: why only AF_INET?
- icfg_plumb(): does this need to verify zone_check_datalink()?
(ifconfig does this in inetplumb before calling ifplumb)
- Comments at line 2333-2334 are incorrect - after the fix for
CR 6243060, the kernel only allows modifications to IFF_IPv4, IFF_IPV6,
IFF_BROADCAST, IFF_XRESOLV. Thus you could skip lines 2339-2345, and
instead start with lifr_flags of 0, before doing lines 2348-2354.
- lines 2368-2374: call icfg_get_flags instead of inlining this code.
- is it necessary to expose errors like ICFG_NO_UNPLUMB_ARP/ICFG_NO_PLUMB_IP
etc? How is the caller supposed to make sense of these errors?
i.e., collapse lines 86-90 to two errors, ICFG_IF_CREATE_ERR and
ICFG_IF_DESTROY_ERR (or some such pair of error codes)
- Question: it looks like we need separate icfg_handle structures for
IPv4 and IPv6 interfaces with nwamd_plumb_unplumb_interface being passed
an af to plumb. How is the af determined (seems like the
nwamd_ncu_state_machine figures this out. Also, I would have expected
the code to do something like
icfg_open()
plumb interface
add addresses
... other stuff with the interface..
and maybe only do the icfg_close when the interface is unplumbed
(but even that would result in needless creation of the ifh_sock), but
I see an icfg_open/icfg_close pair for each operation like icfg_plumb,
icfg_add_ipaddr etc. which would give rise to 6745288-like problems?
Ideally we should have a single handle for ipv4 and ipv6 (with
ifh_sock4, ifh_sock6), and then use the address itself to figure out
which af was targetted (as needed). If that's not possible (because
there are a lot of functions like icfg_set_mtu/icfg_set_metric which,
though they are dead code with questionable semantics, are not
getting cleaned up by this proposal) we should be restricting nwam
usage of icfg_open to retain handle(s) for as long as possible, instead
of doing an open/close per operation.
- icfg_unplumb: if called with an ipmp interface, do we want to bail?
(you would notice IFF_IPMP at line 2541)
- If the PUNLINK of the arp stream fails at
2570 } else {
2571 ret = ICFG_NO_UNPLUMB_ARP;
2572 goto done;
2573 }
the code bails out and doesn't try to unplumb the IP stream, whereas
onnv seems to- was this difference intentional?
usr/src/lib/libinetcfg/common/mapfile-vers
- see coments for inetcfg.h - looks like only icfg_plumb, icfg_remove_ipaddr
amd icfg_unplumb need to be added.
usr/src/lib/libinetcfg/common/inetcfg.h
- icfg_is_logical is not exported, so it can be made static to inetcfg.c
Similarly icfg_is_loopback, icfg_if_protocol.
usr/src/lib/libinetcfg/common/llib-linetcfg
- no changes to this file?
usr/src/lib/libnwam/Makefile
- ok except for similar question about the check target as for inetcfg/Makefile
usr/src/lib/libnwam/common/libnwam_audit.c
- line 69: do you need to adt_free_event() and adt_end_session()?
- line 49: do you need to adt_end_session()?
usr/src/lib/libnwam/common/libnwam_backend.c
- how does nwam_backend_door_server work (doesn't it end up doing
door_return 2 times?): on receiving some NWAM_BACKEND*REQ, it sets up
cmd = NWAM_BACKEND_*RES, and then goes to nwam_create_backend_door_arg
which will do a door_return at line 181. Then nwam_backend_door_server()
will again do a door_return at line 325, right?
- nwam_backend_init(): lines 349-360. dlmgmtd had some issues with the
filesystem not being writable early in boot. How does nwamd work
around that problem?
- if fattach() fails at line 382, we should door_revoke() the DOOR_FILE.
- nwam_backend_exit() is something that is only used by netcfgd,
and it registers an atexit() callback, so it doesn't look like it belongs
in a general "libnwam/common" file.. this should be made local to
netcfgd/*.c itself.
- should this be 0755
358 (chmod(NWAM_DOOR_DIR, 07555) < 0 ||
- make_door_call() is a rather generic function name (and libscf also
has this). Suggest nwam_make_door_call() or nwam_door_call() instead.
usr/s]http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
2,205
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 7:13 AM
in response to: sowmini
|
|
On Mon, 2009-07-20 at 12:17 -0400, Sowmini dot Varadhan at Sun dot COM wrote:
> - why is netcfgd under $SRC/cmd/cmd-inet/lib/netcfgd? Seems like this is
> parallel to dlmgmtd for layer 3, so to be symmetric
> with dlmgtd, shouldn't it be under $SRC/cmd/netcfgd/*? at the very least,
> it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
> $SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
> I don't think these apps get installed in /lib), but I think the
> cleaner solution is
> $SRC/cmd/nwam/netcfgd
> $SRC/cmd/nwam/nwamd
> $SRC/cmd/nwam/common /* for common themes like daemonize() */
I thought that netcfgd was going to evolve to be something that is not
NWAM specific.
In any case, I provided a similar comment regarding the location of
nwamd. I personally don't want to see any more strange hierarchies
under usr/src/cmd, so I'd suggest that each be placed directly under
usr/src/cmd. If we want to share a daemonize function, then it should
go in a library, since there aren't the only two daemons that could use
it. There are gobs of daemonize() and daemon() functions strewn about
usr/src/cmd that all do mostly the same thing.
-Seb
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
420
From:
IE
Registered:
6/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 8:06 AM
in response to: seb
|
|
On 22/07/2009 15:13, Sebastien Roy wrote:
>
> On Mon, 2009-07-20 at 12:17 -0400, Sowmini dot Varadhan at Sun dot COM wrote:
>> - why is netcfgd under $SRC/cmd/cmd-inet/lib/netcfgd? Seems like this is
>> parallel to dlmgmtd for layer 3, so to be symmetric
>> with dlmgtd, shouldn't it be under $SRC/cmd/netcfgd/*? at the very least,
>> it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
>> $SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
>> I don't think these apps get installed in /lib), but I think the
>> cleaner solution is
>> $SRC/cmd/nwam/netcfgd
>> $SRC/cmd/nwam/nwamd
>> $SRC/cmd/nwam/common /* for common themes like daemonize() */
>
> I thought that netcfgd was going to evolve to be something that is not
> NWAM specific.
>
> In any case, I provided a similar comment regarding the location of
> nwamd. I personally don't want to see any more strange hierarchies
> under usr/src/cmd, so I'd suggest that each be placed directly under
> usr/src/cmd. If we want to share a daemonize function, then it should
> go in a library, since there aren't the only two daemons that could use
> it. There are gobs of daemonize() and daemon() functions strewn about
> usr/src/cmd that all do mostly the same thing.
>
Which is where the is a libdaemon.so that comes from the opensource community
and is used in apps/programs that need such functionality there.
This is already delivered into Nevada through the desktop gate.
Darren.
[1] - http://0pointer.de/lennart/projects/libdaemon/
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
6,935
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 8:37 AM
in response to: dkenny
|
|
Darren Kenny wrote:
> On 22/07/2009 15:13, Sebastien Roy wrote:
>> In any case, I provided a similar comment regarding the location of
>> nwamd. I personally don't want to see any more strange hierarchies
>> under usr/src/cmd, so I'd suggest that each be placed directly under
>> usr/src/cmd. If we want to share a daemonize function, then it should
>> go in a library, since there aren't the only two daemons that could use
>> it. There are gobs of daemonize() and daemon() functions strewn about
>> usr/src/cmd that all do mostly the same thing.
>>
>
> Which is where the is a libdaemon.so that comes from the opensource community
> and is used in apps/programs that need such functionality there.
>
> This is already delivered into Nevada through the desktop gate.
>
> Darren.
>
> [1] - http://0pointer.de/lennart/projects/libdaemon/
Ick. I filed a CR a long time ago on putting that into libc -- 4471189.
We shouldn't be reaching off into left field for such a basic function.
--
James Carlson 42.703N 71.076W <carlsonj at workingcode dot com>
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
2,205
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 9:05 AM
in response to: carlsonj
|
|
On Wed, 2009-07-22 at 11:37 -0400, James Carlson wrote:
> Darren Kenny wrote:
> > On 22/07/2009 15:13, Sebastien Roy wrote:
> >> In any case, I provided a similar comment regarding the location of
> >> nwamd. I personally don't want to see any more strange hierarchies
> >> under usr/src/cmd, so I'd suggest that each be placed directly under
> >> usr/src/cmd. If we want to share a daemonize function, then it should
> >> go in a library, since there aren't the only two daemons that could use
> >> it. There are gobs of daemonize() and daemon() functions strewn about
> >> usr/src/cmd that all do mostly the same thing.
> >>
> >
> > Which is where the is a libdaemon.so that comes from the opensource community
> > and is used in apps/programs that need such functionality there.
> >
> > This is already delivered into Nevada through the desktop gate.
> >
> > Darren.
> >
> > [1] - http://0pointer.de/lennart/projects/libdaemon/
>
> Ick. I filed a CR a long time ago on putting that into libc -- 4471189.
>
> We shouldn't be reaching off into left field for such a basic function.
Agreed. Linux and the BSDs all appear to have a daemon() function in
their respective libc's. At least for portability's sake, we should do
the same thing.
-Seb
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
409
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 11:58 AM
in response to: seb
|
|
On Wed, Jul 22, 2009 at 12:05:26PM -0400, Sebastien Roy wrote:
>
> On Wed, 2009-07-22 at 11:37 -0400, James Carlson wrote:
> > Darren Kenny wrote:
> > > On 22/07/2009 15:13, Sebastien Roy wrote:
> > >> In any case, I provided a similar comment regarding the location of
> > >> nwamd. I personally don't want to see any more strange hierarchies
> > >> under usr/src/cmd, so I'd suggest that each be placed directly under
> > >> usr/src/cmd. If we want to share a daemonize function, then it should
> > >> go in a library, since there aren't the only two daemons that could use
> > >> it. There are gobs of daemonize() and daemon() functions strewn about
> > >> usr/src/cmd that all do mostly the same thing.
> > >>
> > >
> > > Which is where the is a libdaemon.so that comes from the opensource community
> > > and is used in apps/programs that need such functionality there.
> > >
> > > This is already delivered into Nevada through the desktop gate.
> > >
> > > Darren.
> > >
> > > [1] - http://0pointer.de/lennart/projects/libdaemon/
> >
> > Ick. I filed a CR a long time ago on putting that into libc -- 4471189.
> >
> > We shouldn't be reaching off into left field for such a basic function.
>
> Agreed. Linux and the BSDs all appear to have a daemon() function in
> their respective libc's. At least for portability's sake, we should do
> the same thing.
I agree that a daemon() library function makes a lot of sense. However,
that's extra scope that we just can't take on in nwam phase 1 at this
point.
Sadly, it looks like Jim's RFE (filed *8* years ago) has only been updated
to say "yeah, we need that, but it needs to do x, y, and z as well". So
there's one reason it hasn't been done yet...
-renee
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
2,205
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 12:03 PM
in response to: okie
|
|
On Wed, 2009-07-22 at 11:58 -0700, Renee Danson Sommerfeld wrote:
> On Wed, Jul 22, 2009 at 12:05:26PM -0400, Sebastien Roy wrote:
> >
> > On Wed, 2009-07-22 at 11:37 -0400, James Carlson wrote:
> > > Darren Kenny wrote:
> > > > On 22/07/2009 15:13, Sebastien Roy wrote:
> > > >> In any case, I provided a similar comment regarding the location of
> > > >> nwamd. I personally don't want to see any more strange hierarchies
> > > >> under usr/src/cmd, so I'd suggest that each be placed directly under
> > > >> usr/src/cmd. If we want to share a daemonize function, then it should
> > > >> go in a library, since there aren't the only two daemons that could use
> > > >> it. There are gobs of daemonize() and daemon() functions strewn about
> > > >> usr/src/cmd that all do mostly the same thing.
> > > >>
> > > >
> > > > Which is where the is a libdaemon.so that comes from the opensource community
> > > > and is used in apps/programs that need such functionality there.
> > > >
> > > > This is already delivered into Nevada through the desktop gate.
> > > >
> > > > Darren.
> > > >
> > > > [1] - http://0pointer.de/lennart/projects/libdaemon/
> > >
> > > Ick. I filed a CR a long time ago on putting that into libc -- 4471189.
> > >
> > > We shouldn't be reaching off into left field for such a basic function.
> >
> > Agreed. Linux and the BSDs all appear to have a daemon() function in
> > their respective libc's. At least for portability's sake, we should do
> > the same thing.
>
> I agree that a daemon() library function makes a lot of sense. However,
> that's extra scope that we just can't take on in nwam phase 1 at this
> point.
Right, my main point being that shoving a daemonize() function into a
common source area under a new usr/src/cmd/nwam/ directory adds no
value. We might as well keep the two functions you have now until
someone replaces the whole lot of them under usr/src/cmd with a proper
libc implementation.
>
> Sadly, it looks like Jim's RFE (filed *8* years ago) has only been updated
> to say "yeah, we need that, but it needs to do x, y, and z as well". So
> there's one reason it hasn't been done yet...
Yeah, not to mention the whole mess that is Solaris-specific contracts,
and that sometimes you want a new one, but sometimes you don't.
-Seb
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
3,083
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 12:04 PM
in response to: okie
|
|
> I agree that a daemon() library function makes a lot of sense. However,
> that's extra scope that we just can't take on in nwam phase 1 at this
> point.
>
> Sadly, it looks like Jim's RFE (filed *8* years ago) has only been updated
> to say "yeah, we need that, but it needs to do x, y, and z as well". So
> there's one reason it hasn't been done yet...
That and the endless bickering that's sure to happen at PSARC :-/
--
meem
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
6,935
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 1:23 PM
in response to: meem
|
|
Peter Memishian wrote:
> > I agree that a daemon() library function makes a lot of sense. However,
> > that's extra scope that we just can't take on in nwam phase 1 at this
> > point.
> >
> > Sadly, it looks like Jim's RFE (filed *8* years ago) has only been updated
> > to say "yeah, we need that, but it needs to do x, y, and z as well". So
> > there's one reason it hasn't been done yet...
>
> That and the endless bickering that's sure to happen at PSARC :-/
>
I suppose something as simple as daemon() could be subjected to a
bike-shed moment, but I'm not so sure it's inevitable at PSARC.
I no longer have access to the full text of CRs, but I think meem had
most of the x/y/z commentary on that one.
I'd like to see us have plain old BSD daemon(). It'd be cool if there
were extensions for the strange Solaricisms that we all have to deal
with when becoming a "real" daemon (particularly contracts), but at
least getting the POSIX basics right looks quite straightforward. I
don't know why it was picked up and then left to rot ...
--
James Carlson 42.703N 71.076W <carlsonj at workingcode dot com>
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
320
From:
US
Registered:
4/14/06
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 5:58 PM
in response to: carlsonj
|
|
I've only done a very superficial code review but it seems like the
support for /etc/nwam/ulp scripting has gone away. This feature was used
by Trusted Extensions to execute some special bringup and commands, like
associating the all-zones ifconfig property with the new interface, and
setting a default label template based on the location and/or domain. Is
the ulp bringup functionality still supported? If not, how do we do this
in NWAM Phase 1?
--Glenn
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
304
From:
Registered:
10/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 11:40 PM
in response to: gfaden
|
|
hi Glenn
Glenn Faden wrote:
> I've only done a very superficial code review but it seems like the
> support for /etc/nwam/ulp scripting has gone away. This feature was
> used by Trusted Extensions to execute some special bringup and
> commands, like associating the all-zones ifconfig property with the
> new interface, and setting a default label template based on the
> location and/or domain. Is the ulp bringup functionality still
> supported? If not, how do we do this in NWAM Phase 1?
The way to do this for phase 1 would be to
create an ENM (external network modifier).
An ENM can specify start/stop scripts or an
SMF FMRI to be activated given a set
of network conditions (or manually if required).
See the draft nwamcfg manpage for the ENM
properties that can be set:
http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamcfg_1m/
If, for example, you wanted to create an
ENM that does something once an IP
address is assigned (the equivalent time
to when a ULP is activated in phase 0),
you could create an ENM with the following
properties
activation-mode = conditional-all
condition = "ip-address is-not 0.0.0.0"
start = /my/start/script
stop = /my/stop/script
This can be done interactively or via
a command file (using nwamcfg's
"-f" subcommand).
Hope this helps,
Alan
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
320
From:
US
Registered:
4/14/06
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 23, 2009 7:13 AM
in response to: amaguire
|
|
Alan Maguire wrote:
> hi Glenn
>
> Glenn Faden wrote:
>> I've only done a very superficial code review but it seems like the
>> support for /etc/nwam/ulp scripting has gone away. This feature was
>> used by Trusted Extensions to execute some special bringup and
>> commands, like associating the all-zones ifconfig property with the
>> new interface, and setting a default label template based on the
>> location and/or domain. Is the ulp bringup functionality still
>> supported? If not, how do we do this in NWAM Phase 1?
> The way to do this for phase 1 would be to
> create an ENM (external network modifier).
> An ENM can specify start/stop scripts or an
> SMF FMRI to be activated given a set
> of network conditions (or manually if required).
> See the draft nwamcfg manpage for the ENM
> properties that can be set:
>
> http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamcfg_1m/
>
> If, for example, you wanted to create an
> ENM that does something once an IP
> address is assigned (the equivalent time
> to when a ULP is activated in phase 0),
> you could create an ENM with the following
> properties
>
> activation-mode = conditional-all
> condition = "ip-address is-not 0.0.0.0"
> start = /my/start/script
> stop = /my/stop/script
>
> This can be done interactively or via
> a command file (using nwamcfg's
> "-f" subcommand).
>
Alan,
Thanks for the info. It isn't clear from the man page whether any
parameters or environment can be passed to the start/stop commands.
Atthe vary least we would need the domain and the interface name. How is
this supposed to work?
--Glenn
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
304
From:
Registered:
10/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 23, 2009 2:18 PM
in response to: gfaden
|
|
Glenn Faden wrote:
> Alan Maguire wrote:
>> hi Glenn
>>
>> Glenn Faden wrote:
>>> I've only done a very superficial code review but it seems like the
>>> support for /etc/nwam/ulp scripting has gone away. This feature was
>>> used by Trusted Extensions to execute some special bringup and
>>> commands, like associating the all-zones ifconfig property with the
>>> new interface, and setting a default label template based on the
>>> location and/or domain. Is the ulp bringup functionality still
>>> supported? If not, how do we do this in NWAM Phase 1?
>> The way to do this for phase 1 would be to
>> create an ENM (external network modifier).
>> An ENM can specify start/stop scripts or an
>> SMF FMRI to be activated given a set
>> of network conditions (or manually if required).
>> See the draft nwamcfg manpage for the ENM
>> properties that can be set:
>>
>> http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamcfg_1m/
>>
>> If, for example, you wanted to create an
>> ENM that does something once an IP
>> address is assigned (the equivalent time
>> to when a ULP is activated in phase 0),
>> you could create an ENM with the following
>> properties
>>
>> activation-mode = conditional-all
>> condition = "ip-address is-not 0.0.0.0"
>> start = /my/start/script
>> stop = /my/stop/script
>>
>> This can be done interactively or via
>> a command file (using nwamcfg's
>> "-f" subcommand).
>>
>
> Alan,
>
> Thanks for the info. It isn't clear from the man page whether any
> parameters or environment can be passed to the start/stop commands.
> Atthe vary least we would need the domain and the interface name. How
> is this supposed to work?
>
That's a good question. We don't support passing
of parameters to start/stop scripts at present.
One approach to determine which interface(s)
are active (in phase 1, depending on the policy,
multiple links and interfaces can be active
simultaneously) would be to use "nwamadm list"
to determine link/interface states. It displays
the states of the various configuration objects
nwamd manages, which include links and interfaces.
See
http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamadm_1m/
for details on nwamadm usage. If a link or interface
is active, it's state will show "online".
Alan
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
320
From:
US
Registered:
4/14/06
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 24, 2009 10:29 AM
in response to: amaguire
|
|
Alan Maguire wrote:
> Glenn Faden wrote:
>> Alan Maguire wrote:
>>> hi Glenn
>>>
>>> Glenn Faden wrote:
>>>> I've only done a very superficial code review but it seems like the
>>>> support for /etc/nwam/ulp scripting has gone away. This feature was
>>>> used by Trusted Extensions to execute some special bringup and
>>>> commands, like associating the all-zones ifconfig property with the
>>>> new interface, and setting a default label template based on the
>>>> location and/or domain. Is the ulp bringup functionality still
>>>> supported? If not, how do we do this in NWAM Phase 1?
>>> The way to do this for phase 1 would be to
>>> create an ENM (external network modifier).
>>> An ENM can specify start/stop scripts or an
>>> SMF FMRI to be activated given a set
>>> of network conditions (or manually if required).
>>> See the draft nwamcfg manpage for the ENM
>>> properties that can be set:
>>>
>>> http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamcfg_1m/
>>>
>>> If, for example, you wanted to create an
>>> ENM that does something once an IP
>>> address is assigned (the equivalent time
>>> to when a ULP is activated in phase 0),
>>> you could create an ENM with the following
>>> properties
>>>
>>> activation-mode = conditional-all
>>> condition = "ip-address is-not 0.0.0.0"
>>> start = /my/start/script
>>> stop = /my/stop/script
>>>
>>> This can be done interactively or via
>>> a command file (using nwamcfg's
>>> "-f" subcommand).
>>>
>>
>> Alan,
>>
>> Thanks for the info. It isn't clear from the man page whether any
>> parameters or environment can be passed to the start/stop commands.
>> Atthe vary least we would need the domain and the interface name. How
>> is this supposed to work?
>>
> That's a good question. We don't support passing
> of parameters to start/stop scripts at present.
> One approach to determine which interface(s)
> are active (in phase 1, depending on the policy,
> multiple links and interfaces can be active
> simultaneously) would be to use "nwamadm list"
> to determine link/interface states. It displays
> the states of the various configuration objects
> nwamd manages, which include links and interfaces.
>
> See
>
> http://opensolaris.org/os/project/nwam/p1spec/manpages/nwamadm_1m/
>
> for details on nwamadm usage. If a link or interface
> is active, it's state will show "online".
This is not optimal in my opinion because the start/stop commands have
to infer the conditions for which they are being invoked. It would be
more straight-forward if that state were passed in somehow. Of course,
the same problem exists with the current ULP scripts which have to
figure out (using dladm or ifconfig) and buy reading /etc/resolv.conf,
what the relevant interface and domain is. Please consider passing in
these values as parameters to the start/stop commands.
--Glenn
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
420
From:
IE
Registered:
6/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Jul 22, 2009 11:07 PM
in response to: seb
|
|
On 22/07/2009 17:05, Sebastien Roy wrote:
>
> On Wed, 2009-07-22 at 11:37 -0400, James Carlson wrote:
>> Darren Kenny wrote:
>>> On 22/07/2009 15:13, Sebastien Roy wrote:
>>>> In any case, I provided a similar comment regarding the location of
>>>> nwamd. I personally don't want to see any more strange hierarchies
>>>> under usr/src/cmd, so I'd suggest that each be placed directly under
>>>> usr/src/cmd. If we want to share a daemonize function, then it should
>>>> go in a library, since there aren't the only two daemons that could use
>>>> it. There are gobs of daemonize() and daemon() functions strewn about
>>>> usr/src/cmd that all do mostly the same thing.
>>>>
>>>
>>> Which is where the is a libdaemon.so that comes from the opensource community
>>> and is used in apps/programs that need such functionality there.
>>>
>>> This is already delivered into Nevada through the desktop gate.
>>>
>>> Darren.
>>>
>>> [1] - http://0pointer.de/lennart/projects/libdaemon/
>>
>> Ick. I filed a CR a long time ago on putting that into libc -- 4471189.
>>
>> We shouldn't be reaching off into left field for such a basic function.
>
> Agreed. Linux and the BSDs all appear to have a daemon() function in
> their respective libc's. At least for portability's sake, we should do
> the same thing.
>
> -Seb
>
Oh, that would be my preference too, I just making the point that even Linux has
something like this, for a while now, you would think with all processes we
follow here we would have spotted this a long time ago.
And for the record, libc would make a lot more sense to me too.
Darren.
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
304
From:
Registered:
10/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 2, 2009 2:26 AM
in response to: sowmini
|
|
hi Sowmini
many thanks for the careful review! I'm going to
try and address the netcfgd and libnwam comments
here. I'll defer on a few of the issues wrt uids etc
to Renee if that's okay - I've specified these below.
Apologies for the delay in responding.
Sowmini dot Varadhan at Sun dot COM wrote:
>> On Mon, Jul 06, 2009 at 03:36:11PM -0700, Michael Hunter wrote:
>>
>>> This is a request for people to review the NWAM Phase 1 code.
>>>
>>> The code can be found at http://cr.opensolaris.org/~mph/nwam1_cr/
>>>
>>> For those inside sun there is a cscope database
>>> in /net/coupe.eng/builds/mph/nwam1_cr/usr/src (sorry to those external
>>> to SWAN).
>>>
>>> Documentation can both be found in the source tree and on the project
>>> web page at http://opensolaris.org/os/project/nwam/
>>>
>>> We request code review be finished by Monday July 20th.
>>>
>>> Michael Hunter
>>>
>
> I reviewed the libnwam and netcfgd changes.. overall looks good- the
> addition of these features is a big step forward for nwam! here are my
> comments.
>
> --Sowmini
>
> ---------------------------------------------------------------------------
>
> General:
> - what is the difference between the netadm and netcfg uids? The
> Brussels II project is considering setting up /sbin/ipadm as owned
> by some dladm-like uid, and we had assumed this would be netadm.
> Should it be netcfg?
>
>
I'll defer to Renee on this one.
> - why is netcfgd under $SRC/cmd/cmd-inet/lib/netcfgd? Seems like this is
> parallel to dlmgmtd for layer 3, so to be symmetric
> with dlmgtd, shouldn't it be under $SRC/cmd/netcfgd/*? at the very least,
> it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
> $SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
> I don't think these apps get installed in /lib), but I think the
> cleaner solution is
> $SRC/cmd/nwam/netcfgd
> $SRC/cmd/nwam/nwamd
> $SRC/cmd/nwam/common /* for common themes like daemonize() */
>
nwamd and netcfgd are installed under /lib, in /lib/inet.
Since nwamd already lived under cmd-inet/lib,
we went with the same location for netcfgd. I'm not sure
if it makes sense to move netcfgd under an "nwam"
subdirectory, given that it may be used in non-NWAM
scenarios in the future.
On the topic of comon code, the amount of duplicated
code has been reduced greatly now, with the
daemonization being the only significant duplication.
Given the current ARC case to support daemonize() in libc,
we'll probably be revisiting that post-integration, so it's
probably not worth eliminating the redundancy right now.
> - it's hard to figure out which libnwam interfaces are "truly" exported,
> and which ones are there because they are shared between some daemon
> and libnwam. The nwam_backend_init is an example of the latter, which
> ends up in mapfile-vers as globally exported just so that the daemon
> can also use it. We need to find a better solution for the latter case
> (see comments for libnwam.h)
>
Since there's likely to be a bit of flux with
these interfaces in the future, and given that
libnwam isn't needed by any other consumers
other than NWAM-specific tools, we've made
libnwam project-private. As such, there's limited
scope to denote that certain functions are more
private than others. What I've tried to do is to
indicate this by splitting out the structures and
functions that are only used by nwamd and netcfgd for
backend door initialization/removal and event
queue management into libnwam_priv.h, utilized
by nwamd and netcfgd.
The functions in question are:
nwam_backend_init()
nwam_backend_fini()
nwam_event_send()
nwam_event_send_fini()
nwam_event_queue_init()
nwam_event_queue_fini()
> -------------------------------------------------------------------------------
> Reviewed with no comments:
>
> usr/src/cmd/cmd-inet/lib/netcfgd/util.h
> usr/src/lib/libnwam/amd64/Makefile
> usr/src/lib/libnwam/i386/Makefile
> usr/src/lib/libnwam/sparc/Makefile
> usr/src/lib/libnwam/sparcv9/Makefile
> usr/src/lib/libnwam/Makefile.com
> usr/src/lib/libnwam/common/mapfile-vers (except as noted in comments for
> libnwam.h)
> usr/src/lib/libnwam/common/door.c (deleted file)
> usr/src/lib/libnwam/common/libnwam_error.c
> usr/src/lib/libnwam/common/libnwam_events.c
> usr/src/lib/libnwam/common/libnwam_values.c
> usr/src/lib/libnwam/common/libnwam_wlan.c
> usr/src/cmd/cmd-inet/lib/Makefile
> usr/src/lib/libnwam/README
> -------------------------------------------------------------------------------
>
> usr/src/cmd/cmd-inet/lib/netcfgd/Makefile
>
> - dlmgmtd runs as OWNER dladm, group sys. What are the owner/group
> of netcftd (I'd guess that the owner would be netcfg- but inside the
> code it seems to use netadm, and invokes getpwam etc. - why the
> divergence from dlmgmtd)?
>
>
Again I'll pass this one to Renee.
> - do we need to define ROOTCFGDIR/<configfile> permissions?
>
>
I don't think so - where did you think this is needed?
> usr/src/cmd/cmd-inet/lib/netcfgd/logging.c
>
> - see relevant comments for netcfgd/main.c
>
> usr/src/cmd/cmd-inet/lib/netcfgd/main.c
>
> - fg can be a local variable to main() - it is not used outside the main()
> context.
>
We now use it in nlog() to determine if logging messages
goes to stderr (if running in foreground) or
to syslog, so it needs to be static global. I've consolidated
the source files for netcfgd into a single file - netcfgd.c.
> - debug doesn't need to be extern global, it can be static to logging.c
> but perhaps it is not needed at all? How is one supposed to set it?
The presence of debug here is cruft from a copy-and-paste
from nwamd I'm afraid. I've removed it now.
>
> (One suggestion, always have syslog support, but use SIGUSR1/SIGUSR2
> handlers to make it more/less verbose by incr/decr debug)
> - zonename does not need to be a global var: it's only usage is for
> lookup_zonename, and there it is passed as an argument.
>
>
This has been removed as again it's copy-and-paste
cruft not needed by netcfgd.
> usr/src/lib/libnwam/Makefile
>
> - ok except for similar question about the check target as for inetcfg/Makefile
>
> usr/src/lib/libnwam/common/libnwam_audit.c
>
> - line 69: do you need to adt_free_event() and adt_end_session()?
> - line 49: do you need to adt_end_session()?
>
>
We do indeed, good catch. I've fixed this.
> usr/src/lib/libnwam/common/libnwam_backend.c
> - how does nwam_backend_door_server work (doesn't it end up doing
> door_return 2 times?): on receiving some NWAM_BACKEND*REQ, it sets up
> cmd = NWAM_BACKEND_*RES, and then goes to nwam_create_backend_door_arg
> which will do a door_return at line 181. Then nwam_backend_door_server()
> will again do a door_return at line 325, right?
>
I've rewritten the door code as there was a lot of
unneeded complexity, particularly in the allocation
of memory. Now the door client simple passes in
a static, maximally-sized buffer and the server fills
in the portion of it that is needed.
http://defect.opensolaris.org/bz/show_bug.cgi?id=10245
has the background to this and how it was addressed.
> - nwam_backend_init(): lines 349-360. dlmgmtd had some issues with the
> filesystem not being writable early in boot. How does nwamd work
> around that problem?
>
There are a few different aspects to this:
- the door files are located under /etc/svc/volatile to allow
creation early in boot (before the root fs is writable)
- in nwamd, when we create or modify configuration
(which we will do principally on first boot post-upgrade),
there is retry logic that ensures we retry every couple
of seconds, dealing with the scenario of a readonly root
early in the boot process.
> - if fattach() fails at line 382, we should door_revoke() the DOOR_FILE.
>
Done.
> - nwam_backend_exit() is something that is only used by netcfgd,
> and it registers an atexit() callback, so it doesn't look like it belongs
> in a general "libnwam/common" file.. this should be made local to
> netcfgd/*.c itself.
>
It does make sense to move the atexit() call into
netcfgd.c, but I'd rather localize the backend door
interactions in libnwam for now to prevent leakage
of backend implementation details.
> - should this be 0755
> 358 (chmod(NWAM_DOOR_DIR, 07555) < 0 ||
>
Yep, thanks for catching that!
> - make_door_call() is a rather generic function name (and libscf also
> has this). Suggest nwam_make_door_call() or nwam_door_call() instead.
>
>
Accepted.
> usr/src/lib/libnwam/common/libnwam_impl.h
> - please use unique prefixes for fields to aid cscope. e.g, use nwh_ as the
> prefix for fields in struct nwam_handle, nwv_ for struct nwam_value fields.
> Just the simple "name" field is currently used in 5 different structures
> even withing libnwam_impl.h itself.
>
Done - I've used nwh_ for nwam handles, nwv_ for
nwam values, nwe_ for nwam events etc.
> - nit:
> 56 struct nwam_value
> 57 {
> should be collapsed to one line:
> 56 struct nwam_value {
>
Accepted.
> - please add some comments explaining what the various fields in
> nwam_backend_door_arg are (e.g., arg_parent, arg_object etc.) and
> what the layout of the data passed between client and server looks like
Done. The parent is the filename, the object the configuration
entity within that file etc. I'll change "parent" to "dbname" as
you suggest.
> - nit: s/ist/list in
> 370 * A ist of all object files is returned. For ENMs
>
>
Fixed.
> usr/src/lib/libnwam/common/libnwam_files.c
> - value_add_escapes(): instead of repeatedly doing malloc/free around this
> function, the caller can pass in the output array with its size. another
> alternative is for value_add_escapes() to create out by doing a strdup(in)
>
>
Accepted - value_add_escapes() now takes "in" and "out" strings
as arguments, filling in the latter with the escaped version
of the former. We pass a char array to the function for "out" so
no malloc is needed to add escapes now.
> similarly for value_remove_escapes()
>
>
For value_remove_escapes() we need to allocate memory
since the value is returned to the user in the value list string
array. I've modified it to strdup() the "in" value since we're
guaranteed that the value will be as short or shorter than the
"in" value, since it is just the "in" value with escapes removed.
> usr/src/lib/libnwam/common/libnwam_enm.c
> - nwam_enm_handle_create: is this function live? it's not exported via
> mapfile-vers or libnwam.h, and there are no internal calls to it?
>
> Functions that seem to need it (e.g., nwam_enm_read) seem to eventually
> create the handle by calling nwam_handle_create itself.
>
> If it's not needed, nwam_enm_handle_create should be removed.
>
>
I've removed it.
> - nwam_enm_set_name(), nwam_enm_get_prop_type() also appear dead.
>
>
They aren't used in onnv, but are by the GUI.
> - How does nwam_enm_handle_create verify its args (e.g., that name is null
> terminated, non-null etc)? Same question applies to nwam_enm_read()
> which *is* exported, so could get anything for its args. Minimally it should
> take a name_size argument in addition to the char *name. Sanity checking of
> flags wold also be a good thing.
>
>
>
I've beefed up the validation somewhat - rather
than passing in a name size, I'm using strnlen()
to validate name size. In addition all pointer
values that must be specified are assert()-validated,
and flags are now validated for read/create/walk/commit
and destroy functions.
> usr/src/lib/libnwam/common/libnwam_known_wlan.c
> - nwam_known_wlan_handle_create()
>
>
Removed it.
> usr/src/lib/libnwam/common/libnwam_loc.c
> - nwam_loc_handle_create(): is this function live? not exported, and no
> calls to it? See comments for libnwam_enm.c
>
>
Removed it.
> - nwam_loc_validate(): where is char *name freed?
>
>
It wasn't, I've fixed that now.
> - lines 325-331: please move these definitions to the start of the file-
> they seem to be sprouting in randomp laces.
>
>
>
Done.
> usr/src/lib/libnwam/common/libnwam_ncp.c
> - nwam_ncp_get_read_only() leaks char *name
>
>
Fixed.
> usr/src/lib/libnwam/common/libnwam_object.c
> - please add comments before each function, explaining input/output
> and expected returns from these functions. The "parent" term, for example,
> is not inutitive- it seems like this is the name of the configuration
> file/db from which the backend is supposed to read/write- is that correct?
> If yes, it would be more obvious to rename "parent" to "dbname" or
> "conf_file". Using the term "parent" for this makes it sound like
> there would be a related "child", which is not the case here (for file-store).
>
>
I've used dbname for places where the configuration container
is specified and added comments before each function.
> - lines 1445-1460 please move these definitions to the start of the file
>
>
Done.
> - lines 1768-1769: why have NWAM_IP_VERSION_IPV4/NWAM_IP_VERSION_IPV6
> when IPV4_VERSION and IPV6_VERSION are available?
>
>
Good idea, I've fixed this.
> - doesn't valid_link_mtu have to check if the mtu is within the acceptable
> range for the link? More generally, I would think that the signature
> of the prop_validate function would need more args to verify against
> libdladm, etc.?
>
>
>
There's a deliberate approach in libnwam - patterned
on libzonecfg - in that when we create configuration
and set property values, we do not attempt to validate
them for runtime usability. So, for example, a user can create
an NCU for a link that doesn't exist on the system at
the time, and create an ENM script that references
a service that isn't installed on the system. The idea behind
this is that runtime characteristics are evaluated by nwamd,
and if application of those parameters fails, the object
is placed in the "maintenance" state (analogous to what
happens for SMF services). By separating configuration
validation from runtime validation, we allow users
to create configurations that may be valid elsewhere.
So in the case of link MTU, I don't think we can validate
it against a specific link's MTU as the link may not exist on
the system.
> usr/src/lib/libnwam/common/libnwam.h
> - what is NWAM_FLAG_GLOBAL_MASK used for? Should this just be "-1"?
>
The idea here is to separate global flags (e.g. NWAM_FLAG_BLOCKING)
from local, object-specific flags used in walk functions (e.g.
NWAM_FLAG_NCU_CLASS_PHYS). The walk filter mask is used in
the walk functions to mask off any flag bits pertaining to
global flag settings (e.g. NWAM_FLAG_DO_NOT_FREE). As I mentioned
above, I've done more flag validation. I've also moved the global, local
and walk filter masks into libnwam_impl.h - no need for
consumers to know about these. I've added a comment to libnwam_impl.h
explaining this.
> - why the extra level of indirect for NWAM_WALK_FILTER_MASK? Can't we just
> have
> #define NWAM_WALK_FILTER_MASK (0xFFFFFFFFULL << 32)
>
>
Walks use local flags only - I think the idea here was to
separate global from local during the walk as I've described above.
I think as long as these definitions aren't exposed to the
user, it's not too problematic.
> - use parentheses around NWAM_FLAG_NCU_TYPE_LINK, NWAM_FLAG_NCU_TYPE_INTERFACE
>
Done.
> But a bigger question is - why are these being shifted out by 32 (i.e.,
> what's in the low order bits, and why is it so complex?)
>
>
See above - separation between local and global flag values.
> - need parentheses around NWAM_FLAG_NCU_CLASS_* definitions. But the
> class definitions are confusing: on the one hand PHY and IPTUN
> seem to be a subset of datalink_class_t (and therefore mutually exclusive),
> but then there's CLASS_IP (which applies to any datalink_class_t).
> How are distinctions drawn here? At the very least, some comments would
> be needed to explain when/how these should be specified.
>
Sure. The distinction is between datalink and interface
objects - types of NCU - and classes of datalink and interface
(vnics, tunnels, IP interfaces etc). So types distinguish
between L2/L3 and classes distinguish between the
various sets of objects at that layer.
> - it's unfortunate that nwam_backend_init and nwam_backend_fini are exposed
> as global interfaces to any application that links libnwam, when
> really, all that's needed is to share them between libnwam and
> netcfgd. I'm not sure if SUNWprivate can be used here (?), but one other
> option is to pull the related functions into a file (seems to be
> libnwam_backend.c in this case) and link the file separately into
> libnwam and netcfgd (e.g., see the usage of $SRC/common/net/patricia/radix.c
> in $SRC/cmd/ipf/tools/Makefile.tools where the same code is shared
> between the kernel ip module and ipfilter . This will allow the functions
> themselves to be declared static, but bug fixes in the code will be
> automatically picked up by both users, without exposing the interface
> to other library callers.
>
>
The whole of libnwam is SUNWprivate at present, so there's
not much we can do at a library level I don't think. The
idea of shared code is a good one, but I'm a bit wary
of introducing such a change, especially when
- the set of consumers of libnwam is limited to nwamd, netcfgd,
nwamadm, nwamcfg and the GUI
- I believe the backend functionality will ultimately be moved to
a more generic libnetcfg that will cover nwam config and
other networking config data, so given that this area
will be changing anyway, the exposure of these functions
will not be long-term.
To denote the different levels of privacy, I've factored
the function definitions that are used by nwamd and
netcfgd into libnwam_priv.h as described above.
> - nwam_addrsrc_t: fyi, Brussels is introducing similar definitions.
> Perhaps it would be useful to extract this to a common file that can
> be shared by both libraries (and without any NWAM_* prefix)?
>
>
Definitely. I suppose it'll have to stay here for now
though.
> - NWAM_STATE*STRING definitions do not have to be exposed via
> libnwam.h: should be mapped using a {code, string} array line nwam_errors
> that's used by nwam_state_to_string()
>
> see related comments about libnwam_util.c. The same thing applies to
> nwam_aux_state_t, NWAM_AUX_STATE*STRING,
> nwam_activation_mode_t, NWAM_ACTIVATION_MODE_*STRING,
> nwam_condition_t, NWAM_CONDITION*STRING,
> nwam_condition_object_type_t, NWAM_CONDITION_OBJECT_TYPE*STRING
> nwam_configsrc_t, NWAM_CONFIGSRC_*STRING
> nwam_nameservices_t, NWAM_LOC_PROP*
> nwam_ncu_type_t, NWAM_NCU_TYPE*STRING
> nwam_ncu_class_t, NWAM_NCU_CLASS_*STRING
> nwam_iptun_type_t, NWAM_IPTUN_TYPE*STRING
> nwam_addrsrc_t, NWAM_ADDRSRC*STRING,
> nwam_priority_mode_t, NWAM_PRIORITY_MODE*STRING
>
>
I've moved all the string definitions that need to be
#define'd into libnwam_impl.h so they don't leak to
consumers. Some of these (like aux state and state)
don't need to be #defined since they are only used
in the nwam_XXX_to_string() function. However,
the strings that represent string versions of
uint64/enum values (e.g. class, type, addrsrc) need to
be #define'd since they appear in value/string
mapping arrays in libnwam_values.c.
> - nwam_iptun_type_t can just use clearviews' iptun_type_t?
>
it can indeed, but since nwamd doesn't currently support
tunnels, we've removed references to tunnel support from
libnwam since the code review was sent out, so this
enum typedef has been removed.
>
>
> usr/src/lib/libnwam/common/libnwam_util.c
>
> - the nwam*_to_string functions should use {code, string} arrays like
> nwam_errors[]. In fact, there can be one nwam_code_to_string_internal()
> that takes the {code, string} array and code as args, and returns the
> const char * as the result. Then the nwam_code_to_string_internal()
> (which would be a generalized version of nwam_strerror()) could be shared
> by nwam_object_type_to_string etc. and this would also consolidate
> the dgettext calls.
The problem with that approach is that some of the
enumerated types will have overlapping values, so
it's impossible to uniquify on the code alone at present.
We do something similar to this for nwam values (such
as activation modes, ncu types etc) which are
stored as uint64s. We provide the API function
nwam_uint64_get_value_string(), which takes
a property name and enum value as args, mapping
them to a particular string value.
I've deferred making the more general change for now
given time pressure but it's a great idea for the future.
Thanks again for the review!
Alan
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
601
From:
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 2, 2009 10:03 AM
in response to: amaguire
|
|
On (09/02/09 10:26), Alan Maguire wrote:
> many thanks for the careful review! I'm going to
> try and address the netcfgd and libnwam comments
> here. I'll defer on a few of the issues wrt uids etc
> to Renee if that's okay - I've specified these below.
> Apologies for the delay in responding.
sure.. no problem. In order to help me navigate through the responses,
could you generate a webrev.delta with changes, please?
--Sowmini
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
304
From:
Registered:
10/17/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 2, 2009 10:55 AM
in response to: sowmini
|
|
Sowmini dot Varadhan at Sun dot COM wrote:
> On (09/02/09 10:26), Alan Maguire wrote:
>
>> many thanks for the careful review! I'm going to
>> try and address the netcfgd and libnwam comments
>> here. I'll defer on a few of the issues wrt uids etc
>> to Renee if that's okay - I've specified these below.
>> Apologies for the delay in responding.
>>
>
> sure.. no problem. In order to help me navigate through the responses,
> could you generate a webrev.delta with changes, please?
>
>
A webrev versus the latest NWAM gate
that includes the majority of the changes
is at:
http://zhadum.east.sun.com/export/ws/amaguire/nwam1-fixes/webrev/
(a few of the codereview changes were made
prior to this). There was a massive volume of
change between the code review webrev and now,
so I'm afraid a delta between it and the
current changes would be overwhelmed
by noise from bugfixes. Sorry about that...
Alan
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
409
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 11, 2009 3:55 PM
in response to: sowmini
|
|
Hi Sowmini,
Thanks very much for the review! My apologies for the long delay in
getting back to you on this. I'll try to address some of your general
comments here.
On Mon, Jul 20, 2009 at 12:17:31PM -0400, sowmini dot varadhan at sun dot com wrote:
>
> General:
> - what is the difference between the netadm and netcfg uids? The
> Brussels II project is considering setting up /sbin/ipadm as owned
> by some dladm-like uid, and we had assumed this would be netadm.
> Should it be netcfg?
The difference is in what those two uids can do--what authorizations
and profiles they get.
netadm is able to do more actual management of the system: it has the
Network Management, Network Autoconf, Network IPsec Management, Service
Management profiles, which allow it to make the changes to system
configuration that nwam needs to make. nwamd runs as the netadm user.
netcfg is geared toward managing the repository: it can read/write/
refresh nwam configuration (by virtue of having the solaris.network.
autoconf.* authorizations). netcfgd, which manages access to the
repository, runs as the netcfg user.
Ownership of /sbin/ipadm should probably just be root. Ownership of
the back-end files, on the other hand, should probably be user netcfg,
group netadm (we're introducing users netadm and netcfg, and group
netadm). You might want to consider creating your own authorizations
that cover your repository, and give those to the netcfg user.
> - why is netcfgd under $SRC/cmd/cmd-inet/lib/netcfgd? Seems like this is
> parallel to dlmgmtd for layer 3, so to be symmetric
> with dlmgtd, shouldn't it be under $SRC/cmd/netcfgd/*?
That's one argument; on the other hand, dlmgmtd is kind of the
outlier. I can understand Seb's point about not liking the
separate cmd-inet hierarchy; but it's my opinion that as long
as $SRC/cmd/cmd-inet exists, it's more confusing to have some
network-related things there, and some elsewhere.
> at the very least,
> it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
> $SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
> I don't think these apps get installed in /lib),
Actually, they do; they're in /lib/inet. /lib is the recommended
place for daemons, as it should be out of root's path. /lib/inet
aggregates network-related daemons to reduce clutter at the /lib
level.
> but I think the
> cleaner solution is
> $SRC/cmd/nwam/netcfgd
> $SRC/cmd/nwam/nwamd
> $SRC/cmd/nwam/common /* for common themes like daemonize() */
I think a libc daemonize() is coming, and is the right answer for
that. Other than that, as Alan mentioned, there's not a lot of
common code left. I would also not be enthusiastic about relocating
the nwamd source, as that's been in the ON gate in its current
location for quite some time now. Finally, I don't think now is
the time to be re-working the source file structure without far
more compelling reasons.
> usr/src/cmd/cmd-inet/lib/netcfgd/Makefile
>
> - dlmgmtd runs as OWNER dladm, group sys. What are the owner/group
> of netcftd (I'd guess that the owner would be netcfg- but inside the
> code it seems to use netadm, and invokes getpwam etc. - why the
> divergence from dlmgmtd)?
netcfgd runs as user netcfg/group netadm; you can see this in the
service manifest (network-netcfg.xml).
I think the code you're talking about is where it creates the door.
In that code, it gets the passwd entry for user netadm, and then uses
that uid and gid to set the owner of the nwam door *directory*. I
think making that netadm is appropriate; there are things in the dir
owned by both netcfg and netadm. I do think there's a bug here, though:
we're setting the directory mode to 0755, and I think it should be
0775, as netadm group members should be able to modify it.
When the backend door itself is created, though, it is created with
netcfgd's own uid/gid, which is netcfg/netadm.
I think the remainder of your comments fell into either the libnwam
section (which Alan is working on, and I think has replied to you
about) or the libinetcfg section, which I'll be getting to shortly.
Thanks,
renee
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
601
From:
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 13, 2009 7:13 PM
in response to: okie
|
|
On (09/11/09 15:55), Renee Danson Sommerfeld wrote:
> > at the very least,
> > it would make sense to have $SRC/cmd/cmd-inet/nwam/netcfgd and
> > $SRC/cmd/cmd-inet/nwam/nwamd ($SRC/cmd/cmd-inet/lib is misleading, since
> > I don't think these apps get installed in /lib),
>
> Actually, they do; they're in /lib/inet. /lib is the recommended
> place for daemons, as it should be out of root's path. /lib/inet
> aggregates network-related daemons to reduce clutter at the /lib
> level.
Right, Alan had pointed this out as well. But in that case, shouldn't
this join the other daemons like in.ndpd, in.mpathd etc and
reside in $SRC/cmd/cmd-inet/usr.lib/?
BTW, while I was mostly satisfied with the responses to the code
review comments, I had sent some other responses to Alan's mail privately
to individuals in the nwam team (but not to nwam-discuss, since the latter
was triggering a spammed response). Please let me know, if anyone
who wishes to do so has not received a copy.
--Sowmini
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
95
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 13, 2009 7:24 PM
in response to: sowmini
|
|
sowmini> ... I had sent some other responses ... privately
sowmini> to individuals in the nwam team (but not to nwam-discuss,
sowmini> since the latter was triggering a spammed response)...
Renee and I looked at this a few days ago and found a suspicious address on
the list that resembled the spam source. So we unsubscribed it, and things
seem better since, AFAWK. We'll keep an eye out for further unusual activity.
-- John
http://blogs.sun.com/jbeck
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
3,083
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] [networking-discuss] NWAM Phase I code review
Posted:
Sep 13, 2009 8:47 PM
in response to: sowmini
|
|
> Right, Alan had pointed this out as well. But in that case, shouldn't
> this join the other daemons like in.ndpd, in.mpathd etc and
> reside in $SRC/cmd/cmd-inet/usr.lib/?
in.mpathd is in cmd-inet/usr.lib because it historically installed in
/usr/lib/inet rather than /lib/inet, and in.ndpd has always installed into
/usr/lib/inet. Given that nwamd installs just into /lib/inet,
cmd-inet/lib seems correct.
--
meem
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
409
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] [networking-discuss] NWAM Phase I code review
Posted:
Sep 14, 2009 2:37 PM
in response to: meem
|
|
On Sun, Sep 13, 2009 at 08:47:55PM -0700, Peter Memishian wrote:
>
> > Right, Alan had pointed this out as well. But in that case, shouldn't
> > this join the other daemons like in.ndpd, in.mpathd etc and
> > reside in $SRC/cmd/cmd-inet/usr.lib/?
>
> in.mpathd is in cmd-inet/usr.lib because it historically installed in
> /usr/lib/inet rather than /lib/inet, and in.ndpd has always installed into
> /usr/lib/inet. Given that nwamd installs just into /lib/inet,
> cmd-inet/lib seems correct.
Yup, this is why the source for nwamd and netcfgd is where it
is.
And to answer the next question: nwamd and netcfgd must be in
/lib/inet, not /usr/lib/inet, because they have to be able to
run before /usr is mounted (that is, filesystem/usr depends on
a long chain of services that ultimately leads to a dependency
on network/physical).
-renee
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
409
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 22, 2009 10:27 PM
in response to: sowmini
|
|
Hi Sowmini,
Thanks for the review! I'm responding here to your comments on libinetcfg;
I think this should be the last section of comments from you to be addressed.
I've posted a webrev with my changes to libinetcfg for both your comments
and Seb's; it's at
file:///net/muskogee/export/hg/nwam/nwamp1/webrev/index.html
My responses to specific comments are in-line below.
-renee
On Mon, Jul 20, 2009 at 12:17:31PM -0400, sowmini dot varadhan at sun dot com wrote:
>
> usr/src/lib/libinetcfg/Makefile
>
> - not sure I understand why this is necessary? According to
> $SRC/lib/README.Makefiles, after you define HDRS and HDRDIR, you one need
> check: $(CHECKHDRS)
> Could you explain why this is needed>
I suspect this was added at some point to be a little more careful
during development; though that's just a guess. I don't believe
it's needed, I'll remove it (both here and from the libnwam Makefile).
> usr/src/lib/libinetcfg/common/inetcfg.c
>
> - icfg_get_linkinfo is a dead function - can be removed
Accept.
> - Not your introduction, but icfg_get_flags(), icfg_get_metric(),
> icfg_get_mtu(), icfg_get_index() should bzero the lifr
> as a first step. Also, these are exported functions that could be
> called with a NULL second argument, so they should verify that
> the flags/metric/etc. is not null before trying to set them.
Accept.
> - line 1854: list == NULL, can happen if there are no links
> available, so I think this should only return ICFG_NO_MEMORY
> if lwp->lw_err = ENOMEM. also it may be that we ran out of
> memory half way through the walk, so we may still need to free
> memory. i.e., we shoudl goto done, and do
> for (i = 0; i < lwp->lw_num; i++) {
> free(..);
> }
Accept/discuss: I'll add a check to return immediately if lw_num is
0 after returning from the dlpi_walk; and I'll check for lw_err set
to ENOMEM and set ret accordingly. But I think the partial freeing
already happens: all the exit paths go through done, which walks lw_list,
freeing each entry. Or is there a different case I'm missing?
> - line 1863: why only AF_INET?
Actually, I think address family is irrelevant here, which is why
AF_UNSPEC was the correct choice. This is a list of datalinks,
ip address family is not a meaningful attribute. AF_UNSPEC is
still a little goofy, but better than INET or INET6.
> - icfg_plumb(): does this need to verify zone_check_datalink()?
> (ifconfig does this in inetplumb before calling ifplumb)
Accept.
> - Comments at line 2333-2334 are incorrect - after the fix for
> CR 6243060, the kernel only allows modifications to IFF_IPv4, IFF_IPV6,
> IFF_BROADCAST, IFF_XRESOLV. Thus you could skip lines 2339-2345, and
> instead start with lifr_flags of 0, before doing lines 2348-2354.
Accept.
> - lines 2368-2374: call icfg_get_flags instead of inlining this code.
Reject. Yes, it would save a few lines here; but it would also redo work
we've already done here to set up the buffer (that's needed whether we
leave as-is or call icfg_get_flags()). I don't see a clear advantage.
> - is it necessary to expose errors like ICFG_NO_UNPLUMB_ARP/ICFG_NO_PLUMB_IP
> etc? How is the caller supposed to make sense of these errors?
> i.e., collapse lines 86-90 to two errors, ICFG_IF_CREATE_ERR and
> ICFG_IF_DESTROY_ERR (or some such pair of error codes)
I would be more worried about this if we were preparing this library to
be used more broadly--but that's definitely not the case.
Seb made a similar point in his comments, and I don't disagree with it;
but given the very narrow use of libinetcfg, and the fact that it will
be made obsolete by libipadm, I don't think fussing with this return
values is very important.
> - Question: it looks like we need separate icfg_handle structures for
> IPv4 and IPv6 interfaces with nwamd_plumb_unplumb_interface being passed
> an af to plumb. How is the af determined (seems like the
> nwamd_ncu_state_machine figures this out.
As we only plumb one IP version at a time, and we confine the use of
the handle to that spot, I don't see why we need separate handles.
If we were going to do something like what you suggest below, and had
to carry the handle through the code, then yes, we would need to track
both a v4 and a v6 handle.
> Also, I would have expected
> the code to do something like
> icfg_open()
> plumb interface
> add addresses
> ... other stuff with the interface..
> and maybe only do the icfg_close when the interface is unplumbed
> (but even that would result in needless creation of the ifh_sock), but
> I see an icfg_open/icfg_close pair for each operation like icfg_plumb,
> icfg_add_ipaddr etc. which would give rise to 6745288-like problems?
>
> Ideally we should have a single handle for ipv4 and ipv6 (with
> ifh_sock4, ifh_sock6), and then use the address itself to figure out
> which af was targetted (as needed). If that's not possible (because
> there are a lot of functions like icfg_set_mtu/icfg_set_metric which,
> though they are dead code with questionable semantics, are not
> getting cleaned up by this proposal) we should be restricting nwam
> usage of icfg_open to retain handle(s) for as long as possible, instead
> of doing an open/close per operation.
Reject. We're not in the right place to be exploring optimizations such
as this. We have working code, based on a library that's not going to be
around a lot longer, and we need to wrap up.
I don't think this is quite as extreme as the case cited in 6745288,
either. The icfg_open()s will be confined to the case where we're
bringing ip ncus up or down; if that's happening a lot, you're going
to have other issues besides the number of open/close pairs.
> - icfg_unplumb: if called with an ipmp interface, do we want to bail?
> (you would notice IFF_IPMP at line 2541)
I think we actually need to pull in the special handling that meem added
into ifconfig with his IPMP rearchitecture project. I initially thought
we might need to pull in *all* of what he added; but I think the unplumb
case is the only one we need to handle at this point, given that NWAM is
the only consumer and NWAM does not manage IPMP groups at this point. We
do need to be able to unplumb just in case we need to clean something up
(that we didn't create) at some point along the way.
This is the one bit of changes that's not in my webrev; I wavered on
this one, so it's not in the bits I've built and tested yet--but I wanted
to go ahead and get the bulk of the work out for people to take a look at.
I'll update the webrev and let everyone know when I've merged in that part.
> - If the PUNLINK of the arp stream fails at
> 2570 } else {
> 2571 ret = ICFG_NO_UNPLUMB_ARP;
> 2572 goto done;
> 2573 }
> the code bails out and doesn't try to unplumb the IP stream, whereas
> onnv seems to- was this difference intentional?
I don't think so; I'll correct that.
> usr/src/lib/libinetcfg/common/mapfile-vers
>
> - see coments for inetcfg.h - looks like only icfg_plumb, icfg_remove_ipaddr
> amd icfg_unplumb need to be added.
>
> usr/src/lib/libinetcfg/common/inetcfg.h
>
> - icfg_is_logical is not exported, so it can be made static to inetcfg.c
> Similarly icfg_is_loopback, icfg_if_protocol.
This project added 7 global interfaces to libinetcfg:
icfg_add_ipaddr
icfg_if_name
icfg_if_protocol
icfg_is_loopback
icfg_plumb
icfg_remove_ipaddr
icfg_unplumb
nwamd consumes all of those except icfg_is_loopback and icfg_if_protocol.
I will remove those two from the mapfile-vers file, and make them static
to inetcfg.c.
icfg_is_logical existed before our changes, and was global; I'm not going
to make it local to the library now, even though nwamd does not consume it.
> usr/src/lib/libinetcfg/common/llib-linetcfg
>
> - no changes to this file?
No, no changes.
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
409
From:
US
Registered:
3/9/05
|
|
|
|
Re: [nwam-discuss] NWAM Phase I code review
Posted:
Sep 24, 2009 10:59 PM
in response to: okie
|
|
On Tue, Sep 22, 2009 at 10:27:51PM -0700, Renee Danson Sommerfeld wrote:
> Hi Sowmini,
>
> Thanks for the review! I'm responding here to your comments on libinetcfg;
> I think this should be the last section of comments from you to be addressed.
>
> On Mon, Jul 20, 2009 at 12:17:31PM -0400, sowmini dot varadhan at sun dot com wrote:
> >
> > - icfg_unplumb: if called with an ipmp interface, do we want to bail?
> > (you would notice IFF_IPMP at line 2541)
>
> I think we actually need to pull in the special handling that meem added
> into ifconfig with his IPMP rearchitecture project. I initially thought
> we might need to pull in *all* of what he added; but I think the unplumb
> case is the only one we need to handle at this point, given that NWAM is
> the only consumer and NWAM does not manage IPMP groups at this point. We
> do need to be able to unplumb just in case we need to clean something up
> (that we didn't create) at some point along the way.
>
> This is the one bit of changes that's not in my webrev; I wavered on
> this one, so it's not in the bits I've built and tested yet--but I wanted
> to go ahead and get the bulk of the work out for people to take a look at.
> I'll update the webrev and let everyone know when I've merged in that part.
I've completed this final bit. The updated webrev is available at
http://jurassic.eng/~okie/webrev/index.html
-renee
_______________________________________________
nwam-discuss mailing list
nwam-discuss at opensolaris dot org
http://mail.opensolaris.org/mailman/listinfo/nwam-discuss
|
|
|
|
|
Posts:
601
From:
Registered:
3/9/05
|
|
|
|
|
|
