From owner-freebsd-arch Sat Dec 4 6:49: 4 1999 Delivered-To: freebsd-arch@freebsd.org Received: from ns1.yes.no (ns1.yes.no [195.204.136.10]) by hub.freebsd.org (Postfix) with ESMTP id 117E1153DB for ; Sat, 4 Dec 1999 06:48:53 -0800 (PST) (envelope-from eivind@bitbox.follo.net) Received: from bitbox.follo.net (bitbox.follo.net [195.204.143.218]) by ns1.yes.no (8.9.3/8.9.3) with ESMTP id PAA29121 for ; Sat, 4 Dec 1999 15:48:46 +0100 (CET) Received: (from eivind@localhost) by bitbox.follo.net (8.8.8/8.8.6) id PAA97420 for freebsd-arch@freebsd.org; Sat, 4 Dec 1999 15:48:46 +0100 (MET) Received: from smtp02.wxs.nl (smtp02.wxs.nl [195.121.6.60]) by hub.freebsd.org (Postfix) with ESMTP id AEB5314FBE; Sat, 4 Dec 1999 06:48:30 -0800 (PST) (envelope-from asmodai@wxs.nl) Received: from daemon.ninth-circle.org ([195.121.197.22]) by smtp02.wxs.nl (Netscape Messaging Server 3.61) with ESMTP id AAA23BD; Sat, 4 Dec 1999 15:48:27 +0100 Received: (from asmodai@localhost) by daemon.ninth-circle.org (8.9.3/8.9.3) id PAA03642; Sat, 4 Dec 1999 15:48:07 +0100 (CET) (envelope-from asmodai) Date: Sat, 4 Dec 1999 15:48:07 +0100 From: Jeroen Ruigrok/Asmodai To: Yoshinobu Inoue Cc: freebsd-arch@freebsd.org, cvs-committers@freebsd.org Subject: Re: [Solicite review for KAME 3rd patch] Message-ID: <19991204154807.G711@daemon.ninth-circle.org> References: <19991203050711F.shin@nd.net.fujitsu.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Mailer: Mutt 1.0i In-Reply-To: <19991203050711F.shin@nd.net.fujitsu.co.jp>; from shin@nd.net.fujitsu.co.jp on Fri, Dec 03, 1999 at 05:07:11AM +0900 Organisation: Ninth-Circle Enterprises Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Hello Inoue-san, -On [19991203 00:00], Yoshinobu Inoue (shin@nd.net.fujitsu.co.jp) wrote: >I prepared the 3rd KAME patch at the URL below, and would like >to get comments for them. Here are my comments: [NOTE: all based on a clean extracted src from 09:00 GMT+1 cvsup sources] All patches succeeded on a fresh CURRENT source tree except for: - sbin/ifconfig/Makefile no .rej file was made curiously enough. Patched by hand. - sbin/route/Makefile no .rej file was made. Patched by hand. - usr.bin/netstat/Makefile also no .rej file. Patched by hand. sys/conf/files: I like the rearranging of the netinet options, it certainly clarifies things. sys/i386/conf/LINT: COMPATIBILITY OPTIONS seems to be a whitespace fix. Not needed IMHO. Does the gif comment imply that IPv4 in IPv4, IPv4 in IPv6, IPv6 in IPv4, IPv6 in IPv6 tunneling is supported? I think it does, but I wonder if others might not understand the notation used. One more comment line to clarify things from the start wouldn't hurt and would save questions to -questions. Also, use `gif' instead of `gif`, minor nitpick to remain consistent. The `faith` pseudo-device catches those packets which sent to it, and forwards then to IPv4 and IPv6 translating daemon. Should this be: The `faith' pseudo-device captures packets sent to it and forwards them to the IPv4/IPv6 translation daemon. Although I wonder if forwards could be changed to divert. Something which I wonder about is the removal of opt_inet.h in if.c. All other source files seem to retain their opt_inet.h alongside the opt_inet6.h. Was this intentional? Does faith replace loop? If so, what is against adding the IPv6 functionality to loop? if_fddisubr.c: contains a whitespace fix for netipx/ipx.h, not needed IMHO. Also contains whitespace fixes for llc_snap_*. Not needed. Same goes for senderr(), the else statement, the empty line `fix', register struct sockaddr_dl *sdl, the printf(" len 0x%x, the #endif, the #ifdef IPX and following #endif, #ifdef NETATALK, case LLC_ISO_LSAP, the other empty line `fix', fh->fddi_dhost, and the last empty line `fix'. Also, wouldn't it be better to follow INET6 directly after INET, since both are effectively the same protocol, except a different version? Just like you did with the #ifdef INET6 later on in this file. Later on with the ETHERTYPE cases, you now position yourself between NS and DECNET, why not follow directly after INET? if_gif.h: Numerous white space changes which are not needed and sometimes even revert the previous style(9) compliant code. Removal of the $FreeBSD$ doesn't seem needed. if_spppsubr.c: #define MAXALIVECNT is a whitespace change, the #define whitespace changes are not needed. Same goes for the */ `fix'. And another empty line `fix'. The IFF_CISCO whitespace fixes are also not needed. Some more #define and empty line whitespace `fixes'. if_atm.c: No need to touch the disclaimer for a whitespace fix. And aside fine the new #include does only whitespace fixes which are not needed. in_pcb.c: More tab/whitespace `fixes' aside from the new includes added and the #ifdef INET6's. in_pcb.h: Again, aside from one change, you shouldn't do whitespace fixes. in_proto.c: again a whitespace fix which shouldn't be in there. ip_fw.c: aside from changed function calls and the likes, more whitespace fixes. tcp_input.c: suffer from whitespace fixes. And the file suffers from continuous whitespace fixes. Find an update of your patch at http://home.wxs.nl/~asmodai/udp-for-ipv6-fixed.19991203 This update only fixes the whitespace problems for a couple files. I grew weary after file 16 or so. I compiled a world and got: cc -O -pipe -DKERNEL -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Winline - Wcast-qual -fformat-extensions -ansi -DKLD_MODULE -nostdinc -I- -I/usr/obj/usr/src/sys/modules/if_disc -I/usr/obj/usr/src/sys/modules/if_disc/@ -mpreferred-stack-boundary=2 -c /usr/src/sys/modules/if_disc/../../net/if_disc.c /usr/src/sys/modules/if_disc/../../net/if_disc.c:55: opt_inet6.h: No such file or directory *** Error code 1 Stop in /usr/src/sys/modules/if_disc. My suggestion: checkout a new source tree and only modify that what you want to change and ignore tab line-ups and concentrate on the functionality. White space corrections are best done in cosmestic commits. Hope this helps, -- Jeroen Ruigrok van der Werven/Asmodai asmodai(at)wxs.nl The BSD Programmer's Documentation Project Network/Security Specialist BSD: Technical excellence at its best Learn e-mail netiquette: http://www.lemis.com/email.html ...fools rush in where Daemons fear to tread. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message