kern/130657: ipv6 bugfix, need review.

[Alfred Perlstein]

adding mailing list discussion to audit trail.


----- Forwarded message from "Bjoern A. Zeeb" <bzeeb-lists at lists.zabbadoz.net> -----

From: "Bjoern A. Zeeb" <bzeeb-lists at lists.zabbadoz.net>
To: Alfred Perlstein <alfred at freebsd.org>
cc: net at freebsd.org
Subject: Re: ipv6 bugfix, need review.
Date: Wed, 24 Dec 2008 23:13:17 +0000 (UTC)
Message-ID: <20081224230540.C97918 at maildrop.int.zabbadoz.net>

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.

----- End forwarded message -----

-- 
- Alfred Perlstein