I apologize for not having review all of the files touched by Volo. What
follows is what I have reviewed.
Thanks,
Dan
===================== (Cut up to and including here.) =====================
Reviewer Name: Dan McDonald
Document/Module Title: Volo
Document/Module Author: Rao Shoaib, Anders Persson, and friends
Document/Module Version/Date: 6 October 2008
Reviewer Preparation Time: 6+ hours
Unfortunately, these files did not receive any review cycles.
usr/src/uts/common/fs/sockfs/sockcommon.c
usr/src/uts/common/fs/sockfs/sockcommon.h
usr/src/uts/common/fs/sockfs/sockcommon_sops.c
usr/src/uts/common/fs/sockfs/sockcommon_subr.c
usr/src/uts/common/fs/sockfs/sockcommon_vnops.c
usr/src/uts/common/fs/sockfs/socknotify.c
usr/src/uts/common/fs/sockfs/sockparams.c
usr/src/uts/common/fs/sockfs/socktpi_impl.h
usr/src/uts/common/fs/sockfs/sockvnops.c
usr/src/uts/common/inet/ip/icmp.c
usr/src/uts/common/inet/ksocket/ksocket.c
usr/src/uts/common/inet/ksocket/ksocket_impl.h
usr/src/uts/common/inet/ksocket/ksocket_mod.c
usr/src/uts/common/inet/sctp/sctp.c
usr/src/uts/common/inet/sctp/sctp_bind.c
usr/src/uts/common/inet/sctp/sctp_common.c
usr/src/uts/common/inet/sctp/sctp_conn.c
usr/src/uts/common/inet/sctp/sctp_cookie.c
usr/src/uts/common/inet/sctp/sctp_impl.h
usr/src/uts/common/inet/sctp/sctp_input.c
usr/src/uts/common/inet/sctp/sctp_notify.c
usr/src/uts/common/inet/sctp/sctp_opt_data.c
usr/src/uts/common/inet/sctp/sctp_output.c
usr/src/uts/common/inet/sctp/sctp_shutdown.c
usr/src/uts/common/inet/sctp_itf.h
usr/src/uts/common/inet/sockmods/sockmod_sctp.c
usr/src/uts/common/inet/sockmods/sockmod_sdp.c
usr/src/uts/common/inet/sockmods/socksctp.c
usr/src/uts/common/fs/sockfs/socksctp.c
usr/src/uts/common/inet/sockmods/socksctp.h
usr/src/uts/common/fs/sockfs/socksctp.h
usr/src/uts/common/inet/sockmods/socksctpsubr.c
usr/src/uts/common/fs/sockfs/socksctpsubr.c
usr/src/uts/common/inet/sockmods/socksdp.c
usr/src/uts/common/fs/sockfs/socksdp.c
usr/src/uts/common/inet/sockmods/socksdp.h
usr/src/uts/common/fs/sockfs/socksdp.h
usr/src/uts/common/inet/sockmods/socksdpsubr.c
usr/src/uts/common/inet/squeue.c
usr/src/uts/common/io/ib/clients/rds/rds_opt.c
usr/src/uts/common/io/ib/clients/rds/rdsddi.c
usr/src/uts/common/io/ib/clients/sdp/sdpddi.c
usr/src/uts/common/io/sock_conf.c
usr/src/uts/common/io/strplumb.c
usr/src/uts/common/netinet/icmp6.h
usr/src/uts/common/os/fio.c
usr/src/uts/common/os/modconf.c
usr/src/uts/common/os/strsubr.c
usr/src/uts/common/smbsrv/smb_kproto.h
usr/src/uts/common/smbsrv/smb_ktypes.h
usr/src/uts/common/sys/Makefile
usr/src/uts/common/sys/ksocket.h
usr/src/uts/common/sys/modctl.h
usr/src/uts/common/sys/socket.h
usr/src/uts/common/sys/socket_proto.h
usr/src/uts/common/sys/socketvar.h
usr/src/uts/common/sys/sockio.h
usr/src/uts/common/sys/squeue.h
usr/src/uts/common/sys/squeue_impl.h
usr/src/uts/common/sys/stream.h
usr/src/uts/common/sys/strsubr.h
usr/src/uts/common/syscall/sendfile.c
usr/src/uts/intel/Makefile.intel.shared
usr/src/uts/intel/ia32/ml/modstubs.s
usr/src/uts/intel/icmp/Makefile
usr/src/uts/intel/icmp/icmp.global-objs.debug64
usr/src/uts/intel/ip/ip.global-objs.debug64
usr/src/uts/intel/ip/ip.global-objs.obj64
usr/src/uts/intel/ksocket/Makefile
usr/src/uts/intel/rts/Makefile
usr/src/uts/intel/rts/rts.global-objs.debug64
usr/src/uts/intel/smbsrv/Makefile
usr/src/uts/intel/socksctp/Makefile
usr/src/uts/intel/socksdp/Makefile
usr/src/uts/intel/tcp/Makefile
usr/src/uts/intel/udp/Makefile
usr/src/uts/sparc/Makefile
usr/src/uts/sparc/Makefile.sparc.shared
usr/src/uts/sparc/icmp/Makefile
usr/src/uts/sparc/icmp/icmp.global-objs.debug64
usr/src/uts/sparc/ip/ip.global-objs.debug64
usr/src/uts/sparc/ip/ip.global-objs.obj64
usr/src/uts/sparc/ksocket/Makefile
usr/src/uts/sparc/ml/modstubs.s
usr/src/uts/sparc/rts/Makefile
usr/src/uts/sparc/rts/rts.global-objs.debug64
usr/src/uts/sparc/smbsrv/Makefile
usr/src/uts/sparc/socksctp/Makefile
usr/src/uts/sparc/socksdp/Makefile
usr/src/uts/sparc/tcp/Makefile
usr/src/uts/sparc/udp/Makefile
usr/src/uts/common/inet/tcp/tcp.c
usr/src/uts/common/inet/tcp/tcp_fusion.c
usr/src/uts/common/inet/tcp/tcp_opt_data.c
usr/src/uts/common/inet/tcp/tcpddi.c
usr/src/uts/common/inet/tcp_impl.h
usr/src/uts/common/inet/tcp_stack.h
. . .
usr/src/cmd/cmd-inet/usr.bin/netstat/unix.c
usr/src/cmd/cmd-inet/usr.sbin/soconfig.c
usr/src/cmd/mdb/Makefile.common
usr/src/cmd/mdb/common/modules/genunix/vfs.c
usr/src/cmd/mdb/common/modules/sockfs/sockfs.c
usr/src/cmd/mdb/intel/amd64/sockfs/Makefile
usr/src/cmd/mdb/sparc/v9/Makefile
usr/src/pkgdefs/SUNWhea/prototype_com
usr/src/pkgdefs/SUNWibsdp/postinstall
usr/src/pkgdefs/SUNWibsdp/preremove
usr/src/pkgdefs/SUNWibsdp/prototype_i386
usr/src/pkgdefs/SUNWibsdp/prototype_sparc
usr/src/pkgdefs/SUNWmdb/prototype_i386
usr/src/pkgdefs/SUNWmdb/prototype_sparc
usr/src/pkgdefs/SUNWmdbr/prototype_i386
usr/src/pkgdefs/SUNWmdbr/prototype_sparc
usr/src/pkgdefs/common_files/i.sock2path
usr/src/uts/Makefile.uts
usr/src/uts/common/Makefile.files
usr/src/uts/common/Makefile.rules
usr/src/uts/common/fs/sockfs/nl7c.c
usr/src/uts/common/fs/sockfs/nl7c.h
usr/src/uts/common/fs/sockfs/nl7chttp.c
usr/src/uts/common/fs/sockfs/nl7curi.c
usr/src/uts/common/fs/sockfs/socksctpvnops.c
usr/src/uts/common/fs/sockfs/socksdpsubr.c
usr/src/uts/common/fs/sockfs/socksdpvnops.c
usr/src/uts/common/fs/sockfs/sockssl.c
usr/src/uts/common/fs/sockfs/socktpi.h
usr/src/uts/common/fs/sockfs/sockstr.c
usr/src/uts/common/fs/sockfs/socksubr.c
usr/src/uts/common/fs/sockfs/socksyscalls.c
usr/src/uts/common/inet/inetddi.c
usr/src/uts/common/inet/ip.h
usr/src/uts/common/inet/ip/icmp_opt_data.c
usr/src/uts/common/inet/ip/icmpddi.c
usr/src/uts/common/inet/ip/ip_mroute.c
usr/src/uts/common/inet/ip/ip_multi.c
usr/src/uts/common/inet/ip/ip_opt_data.c
usr/src/uts/common/inet/ip/ip_rts.c
usr/src/uts/common/inet/ip/ipclassifier.c
usr/src/uts/common/inet/ip/keysock.c
usr/src/uts/common/inet/ip/spdsock.c
usr/src/uts/common/inet/ip6.h
usr/src/uts/common/inet/ip_if.h
usr/src/uts/common/inet/ip_impl.h
usr/src/uts/common/inet/ip_rts.h
usr/src/uts/common/inet/ipclassifier.h
usr/src/uts/common/inet/rawip_impl.h
usr/src/uts/common/inet/spdsock.h
usr/src/uts/common/inet/tcp.h
usr/src/uts/common/inet/udp/udp.c
usr/src/uts/common/inet/udp/udp_opt_data.c
usr/src/uts/common/inet/udp/udpddi.c
usr/src/uts/common/inet/udp_impl.h
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-0 Looks good, no comment needed.
usr/src/cmd/mdb/sparc/v9/sockfs/Makefile
usr/src/cmd/mdb/intel/ia32/ip/Makefile
usr/src/cmd/mdb/intel/ia32/sockfs/Makefile
usr/src/uts/common/fs/smbsrv/smb_negotiate.c
usr/src/uts/common/fs/smbsrv/smb_session.c
usr/src/uts/common/fs/smbsrv/smb_signing.c
usr/src/uts/common/inet/proto_set.c
usr/src/uts/common/inet/proto_set.h
usr/src/uts/common/inet/rts_impl.h
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-1 General E1 ident string should be removed, otherwise
these files are okay.
usr/src/cmd/cmd-inet/etc/sock2path
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-2 Raw IP T4 Perhaps further files will explain this,
but I thought I could send actual raw IP
packets with sockets. Is this actually not
the case and "rawip" was just a poor name?
usr/src/cmd/mdb/common/modules/genunix/net.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-3 Lines 885-902 T3 What about PF_ROUTE, PF_KEY, and PF_POLICY
sockets?
usr/src/pkgdefs/SUNWckr/prototype_com
usr/src/pkgdefs/SUNWckr/prototype_i386
usr/src/pkgdefs/SUNWckr/prototype_sparc
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-4 General T5/E5 Dumb question --> did your ARC case
say you'd be creating a new "socketmod"
directory under .../kernel ? If so, cool,
if not, please make sure this is public.
DM-5 Platform-specific T5 Could some grinning weirdo create
/platform/.../kernel/socketmod/ also?
usr/src/uts/Makefile.targ
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-6 Lines 384-7, T5 Why the refactoring of RELINK and friends?
420-429 Is it to save the 'dirname' fork for only if
it's a ROOTLINK?
You just need to answer this question, you
don't need to slap a comment in the code, but
it might not hurt... ;)
usr/src/uts/common/c2/audit_event.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-7 Lines 4158-63, T4 I'll have to look into socket_getsockname(),
4348-4353, but it appears you'll now be entering/exiting
4544-4549, the so_lock TWICE instead of once for the
5383-5388, data in question. Perhaps with cache
5519-5524 locality that's not so bad, but if you notice
things, check out stuff like this.
usr/src/uts/common/fs/smbsrv/smb_net.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-8 smb_sorecv() T2/E1 What's up with VOLO_FIX?!? I don't see any
comments as to why it's there!
usr/src/uts/common/fs/sockfs/socktpi.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-9 Lines 446-584 E2 A stylistic rant. You indent a
lot of code if error == 0, instead of doing
a if (error != 0 && stp != NULL && ...)
with the open-failed-partway handling, and
then a duplicate trace-and-return in the
non-stp error case.
DM-10 Lines 4561-7 E1/T3 Seems strange NL7C code would appear in the
middle of a socktpi function like this
without much in the way of comment or other
indication. As a first-time viewer of this
code, my reaction is, "HUH?!?"
Can you explain this a bit?
usr/src/uts/common/inet/ip/ip.c
usr/src/uts/common/inet/ip/ip6.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-11 General T2 I think you've run IKE-STC on these bits, and
I know you've run as a punchin client, but
have you run the IPsec per-socket tests?
This will test any
latching/inheriting/etc. behavior explicitly.
DM-12 Line 2783 E1 What's with the XXX?!? Can you say why
ip6.c there's an XXX there?
usr/src/uts/common/inet/ip/ip6_if.c
usr/src/uts/common/inet/ip/ip_if.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-13 *_addr_exists* T3 Why scan the ill list? Aren't there
route lookups you can do that would be
faster? At least justify why you're
scanning the ill list instead of using
ire lookups.
usr/src/uts/common/inet/ip/rts.c
usr/src/uts/common/inet/ip/rts_opt_data.c
usr/src/uts/common/inet/ip/rtsddi.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-14 General T5/E5 Questions: Have you "Volo-ized" rts?
Would you point to this as a good example for
keysock and spdsock, modulo that both of the
latter do not need the
synchronous-error-return semantics that rts
demands?
Also, is there any reason why you didn't just
get rid of the TPI infrastructure for rts?
It's not like the open of /dev/rts is
supported save for ndd(1M) operations, OTOH,
is it a binary-compatibility issue?
usr/src/uts/common/inet/ip/ip_helper_stream.c
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-15 General E3 Perhaps a bit more documentation here about
how these functions actually help?
usr/src/uts/common/inet/mi.c
usr/src/uts/common/inet/mi.h
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-16 *_hiwat() T3 You've altered almost all of the callers to
these... are you only keeping them around
because of third-party drivers?
DM-17 New functions T2 Why are you putting these in this file?
I thought mi.c was supposed to be all Mentat
swill!
usr/src/uts/common/inet/optcom.c
usr/src/uts/common/inet/optcom.h
------- --------------- ------- -----------------------------------------------
No. Location Sev. Comment
------- --------------- ------- -----------------------------------------------
DM-18 New functions T2 See DM-17.
_______________________________________________
networking-discuss mailing list
[email protected]
