Date: Thu, 11 Oct 2012 02:26:51 +0400 From: Maxim Dounin <mdounin@mdounin.ru> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: net@FreeBSD.org Subject: Re: [CFT/Review] net byte order for AF_INET Message-ID: <20121010222651.GR40452@mdounin.ru> In-Reply-To: <20121010195842.GH34622@FreeBSD.org> References: <20121009154128.GU34622@FreeBSD.org> <20121010195842.GH34622@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121010222651.GR40452>