From owner-freebsd-net@FreeBSD.ORG Wed Oct 10 22:26:54 2012 Return-Path: Delivered-To: net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 21CAE795; Wed, 10 Oct 2012 22:26:54 +0000 (UTC) (envelope-from mdounin@mdounin.ru) Received: from mdounin.cust.ramtel.ru (mdounin.cust.ramtel.ru [81.19.69.81]) by mx1.freebsd.org (Postfix) with ESMTP id CACDF8FC0A; Wed, 10 Oct 2012 22:26:53 +0000 (UTC) Received: from mdounin.ru (mdounin.cust.ramtel.ru [81.19.69.81]) by mdounin.cust.ramtel.ru (Postfix) with ESMTP id EC4081703D; Thu, 11 Oct 2012 02:26:51 +0400 (MSK) Date: Thu, 11 Oct 2012 02:26:51 +0400 From: Maxim Dounin To: Gleb Smirnoff Subject: Re: [CFT/Review] net byte order for AF_INET Message-ID: <20121010222651.GR40452@mdounin.ru> References: <20121009154128.GU34622@FreeBSD.org> <20121010195842.GH34622@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121010195842.GH34622@FreeBSD.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: net@FreeBSD.org X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Oct 2012 22:26:54 -0000 Hello! On Wed, Oct 10, 2012 at 11:58:42PM +0400, Gleb Smirnoff wrote: > On Tue, Oct 09, 2012 at 07:41:28PM +0400, Gleb Smirnoff wrote: > T> this is a patch that switches entire IPv4 stack to network > T> byte order. That means, that at any layer any module should > T> expect IP header in network byte order. Any host byte order > T> values can be stored in local variables only and are never stored > T> into a packet itself. > > Update: > > - fresh patch against r241405, should apply to r241405 > - fixed raw sockets` as Luigi and Max Dounin noticed, also fixed > another issue with raw sockets, now work fine - tested with ospfd > - fixed gre(4) > - took into account Luigi's comments about using host byte order > in any ==, <, > comparisons. > > Decided not to touch ip_reass(). > > -- > Totus tuus, Glebius. [...] > --- sys/netinet/raw_ip.c (revision 241405) > +++ sys/netinet/raw_ip.c (working copy) > @@ -292,7 +292,7 @@ > * not modify the packet except for some > * byte order swaps. > */ > - ip->ip_len += off; > + ip->ip_len = htons(ip->ip_len) + off; This one is certainly wrong as it uses arithmetic with a number in network byte order. > > hash = INP_PCBHASH_RAW(proto, ip->ip_src.s_addr, > ip->ip_dst.s_addr, V_ripcbinfo.ipi_hashmask); > @@ -449,11 +449,11 @@ > ip = mtod(m, struct ip *); > ip->ip_tos = inp->inp_ip_tos; > if (inp->inp_flags & INP_DONTFRAG) > - ip->ip_off = IP_DF; > + ip->ip_off = htons(IP_DF); > else > ip->ip_off = 0; > ip->ip_p = inp->inp_ip_p; > - ip->ip_len = m->m_pkthdr.len; > + ip->ip_len = htons(m->m_pkthdr.len); > ip->ip_src = inp->inp_laddr; > if (jailed(inp->inp_cred)) { > /* > @@ -504,6 +504,9 @@ > if (ip->ip_id == 0) > ip->ip_id = ip_newid(); > > + ip->ip_len = htons(ip->ip_len); > + ip->ip_off = htons(ip->ip_off); > + So the packet is expected to come into rip_output() from caller with ip_len/ip_off in host byte order, right? As already suggested - it would be good to add a comment explaining this. [...] -- Maxim Dounin