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>