[CFT/Review] net byte order for AF_INET

[Gleb Smirnoff]

  Maxim,

On Thu, Oct 11, 2012 at 02:26:51AM +0400, Maxim Dounin wrote:
M> > --- sys/netinet/raw_ip.c	(revision 241405)
M> > +++ sys/netinet/raw_ip.c	(working copy)
M> > @@ -292,7 +292,7 @@
M> >  	 * not modify the packet except for some
M> >  	 * byte order swaps.
M> >  	 */
M> > -	ip->ip_len += off;
M> > +	ip->ip_len = htons(ip->ip_len) + off;
M> 
M> This one is certainly wrong as it uses arithmetic with a number in 
M> network byte order.

Ah, this is a typo certainly. Should be ntohs(). Code did
work just because functions are symmetrical.

Regenerated diff attached.

M> >  
M> >  	hash = INP_PCBHASH_RAW(proto, ip->ip_src.s_addr,
M> >  	    ip->ip_dst.s_addr, V_ripcbinfo.ipi_hashmask);
M> > @@ -449,11 +449,11 @@
M> >  		ip = mtod(m, struct ip *);
M> >  		ip->ip_tos = inp->inp_ip_tos;
M> >  		if (inp->inp_flags & INP_DONTFRAG)
M> > -			ip->ip_off = IP_DF;
M> > +			ip->ip_off = htons(IP_DF);
M> >  		else
M> >  			ip->ip_off = 0;
M> >  		ip->ip_p = inp->inp_ip_p;
M> > -		ip->ip_len = m->m_pkthdr.len;
M> > +		ip->ip_len = htons(m->m_pkthdr.len);
M> >  		ip->ip_src = inp->inp_laddr;
M> >  		if (jailed(inp->inp_cred)) {
M> >  			/*
M> > @@ -504,6 +504,9 @@
M> >  		if (ip->ip_id == 0)
M> >  			ip->ip_id = ip_newid();
M> >  
M> > +		ip->ip_len = htons(ip->ip_len);
M> > +		ip->ip_off = htons(ip->ip_off);
M> > +
M> 
M> So the packet is expected to come into rip_output() from caller 
M> with ip_len/ip_off in host byte order, right?  As already 
M> suggested - it would be good to add a comment explaining this.

This is de facto standard for raw sockets in most OS-es. Byte order
in raw socket is host. And this is the same behavior we had before
the patch. So no reason for extra comments.

-- 
Totus tuus, Glebius.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: IPv4.net-byte-order.diff
Type: text/x-diff
Size: 37810 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-net/attachments/20121011/9cfcfe9f/attachment.diff>