Subject: Re: getting rid of NTOHS() in ip_input()
To: Jun-ichiro itojun Hagino <itojun@iijlab.net>
From: Jason R Thorpe <thorpej@wasabisystems.com>
List: tech-net
Date: 08/12/2002 20:31:33
On Tue, Aug 13, 2002 at 07:57:34AM +0900, Jun-ichiro itojun Hagino wrote:

 > Index: netinet/igmp.c
 > ===================================================================
 > RCS file: /cvsroot/syssrc/sys/netinet/igmp.c,v
 > retrieving revision 1.30
 > diff -u -1 -r1.30 igmp.c
 > --- netinet/igmp.c	2002/06/30 22:40:33	1.30
 > +++ netinet/igmp.c	2002/08/12 22:57:49
 > @@ -162,3 +162,3 @@
 >  	minlen = iphlen + IGMP_MINLEN;
 > -	if (ip->ip_len < minlen) {
 > +	if (ntohs(ip->ip_len) < minlen) {
 >  		++igmpstat.igps_rcv_tooshort;
 > @@ -183,3 +183,3 @@
 >  	/* No need to assert alignment here. */
 > -	if (in_cksum(m, ip->ip_len - iphlen)) {
 > +	if (in_cksum(m, ntohs(ip->ip_len) - iphlen)) {
 >  		++igmpstat.igps_rcv_badsum;

I think in igmp_input(), you can do something like:

	uint16_t ip_len = ntohs(ip->ip_len);

just once and use the computed value again, right?

 > Index: netinet/ip_input.c
 > ===================================================================
 > RCS file: /cvsroot/syssrc/sys/netinet/ip_input.c,v
 > retrieving revision 1.154
 > diff -u -1 -r1.154 ip_input.c
 > --- netinet/ip_input.c	2002/06/30 22:40:34	1.154
 > +++ netinet/ip_input.c	2002/08/12 22:57:50
 > @@ -574,8 +574,2 @@
 >  	/*
 > -	 * Convert fields to host representation.
 > -	 */
 > -	NTOHS(ip->ip_len);
 > -	NTOHS(ip->ip_off);
 > -
 > -	/*
 >  	 * Process options and, if not destined for us,
 > @@ -724,3 +718,3 @@
 >  	 */
 > -	if (ip->ip_off & ~(IP_DF|IP_RF)) {
 > +	if (ntohs(ip->ip_off) & ~(IP_DF|IP_RF)) {

Byte-swap the constant here, instead of the variable.  The compiler will
optimize it for you.

 >  		/*
 > @@ -744,4 +738,4 @@
 >  		 */
 > -		ip->ip_len -= hlen;
 > -		mff = (ip->ip_off & IP_MF) != 0;
 > +		ip->ip_len = htons(ntohs(ip->ip_len) - hlen);
 > +		mff = (ntohs(ip->ip_off) & IP_MF) != 0;

Byte swap the constant here.

 >  		if (mff) {
 > @@ -751,3 +745,4 @@
 >  		         */
 > -			if (ip->ip_len == 0 || (ip->ip_len & 0x7) != 0) {
 > +			if (ntohs(ip->ip_len) == 0 ||
 > +			    (ntohs(ip->ip_len) & 0x7) != 0) {

Byte swap constants.

 >  				ipstat.ips_badfrags++;
 > @@ -757,3 +752,3 @@
 >  		}
 > -		ip->ip_off <<= 3;
 > +		ip->ip_off = htons((ntohs(ip->ip_off) & 0x1fff) << 3);
 >  
 > @@ -764,3 +759,3 @@
 >  		 */
 > -		if (mff || ip->ip_off) {
 > +		if (mff || ntohs(ip->ip_off)) {

Don't need to byte-swap if you're testing != 0.

 > Index: netinet/raw_ip.c
 > ===================================================================
 > RCS file: /cvsroot/syssrc/sys/netinet/raw_ip.c,v
 > retrieving revision 1.61
 > diff -u -1 -r1.61 raw_ip.c
 > --- netinet/raw_ip.c	2002/06/09 16:33:43	1.61
 > +++ netinet/raw_ip.c	2002/08/12 22:57:50
 > @@ -165,5 +165,7 @@
 >  	 * XXX Compatibility: programs using raw IP expect ip_len
 > -	 * XXX to have the header length subtracted.
 > +	 * XXX to have the header length subtracted, and in host order.
 > +	 * XXX ip_off is also expected to be host order.
 >  	 */
 > -	ip->ip_len -= ip->ip_hl << 2;
 > +	ip->ip_len = ntohs(ip->ip_len) - (ip->ip_hl << 2);
 > +	NTOHS(ip->ip_off);

You're converting from host-to-net, so use htons() and HTONS(), right?

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>