ipv6 bugfix, need review.

[Bjoern A. Zeeb]

On Mon, 22 Dec 2008, Alfred Perlstein wrote:

Hi,

> Hey guys, we found a bug at Juniper and it resolves an issue
> for us.  I've been asked to forward this to FreeBSD, I honestly
> am not that clear on the issue so I'm hoping someone can step
> up to review this.
>
> Synopsis is:
>
>  The traffic class byte is set to 0x00000000 in the header of some
>  BGP packets sent between interfaces that have IPv6 addresses,
>  instead of the correct setting 0xc0 (INTERNETCONTROL).
>
> Fix is small and attached.  One thing I am wondering, do we
> need to check "if (inp)" ?  I don't think so.

I am not that concerned about the inp at the moment; there are a few
other things:

1 FreeBSD to my knowledge has neither IPV6_GET_CLASS nor IPV6_SET_CLASS
   nor IPV6_CLASS_MASK

2 To the best I can see this currently ignores the upper 4 TC bits that
   go with the `version field' ("vcf"), so it's a hack good enough for
   now, but not a proper fix?

3 I am assuming that we'd need to fix at least one more place.


Tha said I planned to look at the in6p_flowinfo (inp_flow) field in
the not too distant future anyway; I should perhaps combine this
looking into the entire TC thing as well.


> Index: bsd/sys/netinet/tcp_syncache.c
> ===================================================================
> RCS file: /cvs/junos-2008/bsd/sys/netinet/tcp_syncache.c,v
> retrieving revision 1.24
> diff -p -u -r1.24 tcp_syncache.c
> --- bsd/sys/netinet/tcp_syncache.c	29 Jul 2008 17:07:43 -0000	1.24
> +++ bsd/sys/netinet/tcp_syncache.c	16 Dec 2008 19:23:31 -0000
> @@ -1271,6 +1271,7 @@ syncache_respond(sc, m)
> 	struct inpcb *inp;
> #ifdef INET6
> 	struct ip6_hdr *ip6 = NULL;
> +	int inp_tclass;
> #endif
> 	struct rt_nexthop *minmtu_nh;
> 	struct route_table *rtb = NULL;
> @@ -1387,6 +1388,12 @@ syncache_respond(sc, m)
> 		/* ip6_hlim is set after checksum */
> 		ip6->ip6_flow &= ~IPV6_FLOWLABEL_MASK;
> 		ip6->ip6_flow |= sc->sc_flowlabel;
> +		/* Set the TC for IPv6 just like TOS for IPv4 */
> +		ip6->ip6_flow &= ~IPV6_CLASS_MASK;
> +		if (inp) {
> +		    inp_tclass = IPV6_GET_CLASS(inp->in6p_flowinfo);
> +		    ip6->ip6_flow |= IPV6_SET_CLASS(inp_tclass);
> +		}
>
> 		th = (struct tcphdr *)(ip6 + 1);
> 	} else
>
>
>

-- 
Bjoern A. Zeeb                      The greatest risk is not taking one.