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
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to