From owner-freebsd-net@FreeBSD.ORG Fri Dec 12 20:27:59 2008 Return-Path: Delivered-To: freebsd-net@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 353C8106567B; Fri, 12 Dec 2008 20:27:59 +0000 (UTC) (envelope-from rrs@lakerest.net) Received: from lakerest.net (unknown [IPv6:2001:240:585:2:203:6dff:fe1a:4ddc]) by mx1.freebsd.org (Postfix) with ESMTP id AC3ED8FC1F; Fri, 12 Dec 2008 20:27:57 +0000 (UTC) (envelope-from rrs@lakerest.net) Received: from [10.1.1.54] ([10.1.1.54]) (authenticated bits=0) by lakerest.net (8.14.1/8.14.1) with ESMTP id mBCKRujJ026508 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Fri, 12 Dec 2008 15:27:56 -0500 (EST) (envelope-from rrs@lakerest.net) DKIM-Signature: a=rsa-sha1; c=simple/simple; d=lakerest.net; s=mail; t=1229113676; h=Cc:Message-Id:From:To:In-Reply-To:Content-Type: Mime-Version:Subject:Date:References:X-Mailer; b=080zG2ujyr9WSheoHQ gUDaUROUtG7uG/VqoaP1N5E2pZzzXevfMTlEDSGHazpWHZcvi/IVKY4HZ25h18Wb9eN w== Message-Id: From: Randall Stewart To: Bruce Evans In-Reply-To: <20081213030449.F2405@delplex.bde.org> Content-Type: multipart/mixed; boundary=Apple-Mail-90--449110523 Mime-Version: 1.0 (Apple Message framework v929.2) Date: Fri, 12 Dec 2008 15:27:56 -0500 References: <49249443.8050707@elischer.org> <76CF7D15-251F-4E43-86BE-AD96F48AF123@lakerest.net> <200811201450.30016.max@love2party.net> <24BD4A21-E10D-4E09-8C33-3FCF930A0495@lakerest.net> <494157DF.6030802@FreeBSD.org> <13C9478E-CBF6-4EDA-8E78-AD76549EB844@lakerest.net> <20081213030449.F2405@delplex.bde.org> X-Mailer: Apple Mail (2.929.2) Cc: freebsd-net , "Bruce M. Simpson" Subject: Re: Heads up --- Thinking about UDP and tunneling X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Dec 2008 20:27:59 -0000 --Apple-Mail-90--449110523 Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit Bruce: So lets see: 1) I went ahead and fixed the comments.. even added a ! instead of :-( 2) No problem using func_t.. changed to that.. seems nicer :-D 3) Removed an extra cr or two you pointed out.. hopefully got them all. 4) I disagree with you on the cast... its not ugly.. it prevents us from having to have a per_pcb structure for UDP when all we need is one pointer. As I said in my first post.. it seemed like to much overhead, creating a zone for a single pointer... I am not adverse to casts .. but of course I guess I am just an old fart who has been around to many years writing code :-) 5) I ran s9indent.. and of course it found a lot of other things you missed.. but that is the way of s9indent I have found :-D R --Apple-Mail-90--449110523 Content-Disposition: attachment; filename=diff_with_s9 Content-Type: application/octet-stream; x-unix-mode=0644; name="diff_with_s9" Content-Transfer-Encoding: 7bit Index: netinet/udp_usrreq.c =================================================================== --- netinet/udp_usrreq.c (revision 185928) +++ netinet/udp_usrreq.c (working copy) @@ -97,7 +97,8 @@ */ #ifdef VIMAGE_GLOBALS -int udp_blackhole; +int udp_blackhole; + #endif /* @@ -106,11 +107,13 @@ * packets that would otherwise be discarded due to bad checksums, and may * cause problems (especially for NFS data blocks). */ -static int udp_cksum = 1; +static int udp_cksum = 1; + SYSCTL_INT(_net_inet_udp, UDPCTL_CHECKSUM, checksum, CTLFLAG_RW, &udp_cksum, 0, "compute udp checksum"); -int udp_log_in_vain = 0; +int udp_log_in_vain = 0; + SYSCTL_INT(_net_inet_udp, OID_AUTO, log_in_vain, CTLFLAG_RW, &udp_log_in_vain, 0, "Log all incoming UDP packets"); @@ -118,26 +121,28 @@ CTLFLAG_RW, udp_blackhole, 0, "Do not send port unreachables for refused connects"); -u_long udp_sendspace = 9216; /* really max datagram size */ - /* 40 1K datagrams */ +u_long udp_sendspace = 9216; /* really max datagram size */ + + /* 40 1K datagrams */ SYSCTL_ULONG(_net_inet_udp, UDPCTL_MAXDGRAM, maxdgram, CTLFLAG_RW, &udp_sendspace, 0, "Maximum outgoing UDP datagram size"); -u_long udp_recvspace = 40 * (1024 + +u_long udp_recvspace = 40 * (1024 + #ifdef INET6 - sizeof(struct sockaddr_in6) + sizeof(struct sockaddr_in6) #else - sizeof(struct sockaddr_in) + sizeof(struct sockaddr_in) #endif - ); +); SYSCTL_ULONG(_net_inet_udp, UDPCTL_RECVSPACE, recvspace, CTLFLAG_RW, &udp_recvspace, 0, "Maximum space for incoming UDP datagrams"); #ifdef VIMAGE_GLOBALS -struct inpcbhead udb; /* from udp_var.h */ -struct inpcbinfo udbinfo; -struct udpstat udpstat; /* from udp_var.h */ +struct inpcbhead udb; /* from udp_var.h */ +struct inpcbinfo udbinfo; +struct udpstat udpstat; /* from udp_var.h */ + #endif #ifndef UDBHASHSIZE @@ -148,9 +153,10 @@ CTLFLAG_RW, udpstat, udpstat, "UDP statistics (struct udpstat, netinet/udp_var.h)"); -static void udp_detach(struct socket *so); -static int udp_output(struct inpcb *, struct mbuf *, struct sockaddr *, - struct mbuf *, struct thread *); +static void udp_detach(struct socket *so); +static int +udp_output(struct inpcb *, struct mbuf *, struct sockaddr *, + struct mbuf *, struct thread *); static void udp_zone_change(void *tag) @@ -204,8 +210,10 @@ struct sockaddr *append_sa; struct socket *so; struct mbuf *opts = 0; + #ifdef INET6 struct sockaddr_in6 udp_in6; + #endif INP_RLOCK_ASSERT(inp); @@ -218,7 +226,7 @@ V_ipsec4stat.in_polvio++; return; } -#endif /* IPSEC */ +#endif /* IPSEC */ #ifdef MAC if (mac_inpcb_check_deliver(inp, n) != 0) { m_freem(n); @@ -271,8 +279,10 @@ int len; struct ip save_ip; struct sockaddr_in udp_in; + #ifdef IPFIREWALL_FORWARD struct m_tag *fwd_tag; + #endif ifp = m->m_pkthdr.rcvif; @@ -283,11 +293,10 @@ * user, and use on returned packets, but we don't yet have a way to * check the checksum with options still present. */ - if (iphlen > sizeof (struct ip)) { + if (iphlen > sizeof(struct ip)) { ip_stripoptions(m, (struct mbuf *)0); iphlen = sizeof(struct ip); } - /* * Get IP and UDP header together in first mbuf. */ @@ -330,7 +339,6 @@ m_adj(m, len - ip->ip_len); /* ip->ip_len = len; */ } - /* * Save a copy of the IP header in case we want restore it for * sending an ICMP error message in response. @@ -360,7 +368,7 @@ bcopy(((struct ipovly *)ip)->ih_x1, b, 9); bzero(((struct ipovly *)ip)->ih_x1, 9); ((struct ipovly *)ip)->ih_len = uh->uh_ulen; - uh_sum = in_cksum(m, len + sizeof (struct ip)); + uh_sum = in_cksum(m, len + sizeof(struct ip)); bcopy(b, ((struct ipovly *)ip)->ih_x1, 9); } if (uh_sum) { @@ -387,7 +395,8 @@ uh->uh_dport = ntohs(next_hop->sin_port); /* - * Remove the tag from the packet. We don't need it anymore. + * Remove the tag from the packet. We don't need it + * anymore. */ m_tag_delete(m, fwd_tag); } @@ -414,10 +423,11 @@ inp->inp_faddr.s_addr != ip->ip_src.s_addr) continue; /* - * XXX: Do not check source port of incoming datagram - * unless inp_connect() has been called to bind the - * fport part of the 4-tuple; the source could be - * trying to talk to us with an ephemeral port. + * XXX: Do not check source port of incoming + * datagram unless inp_connect() has been called to + * bind the fport part of the 4-tuple; the source + * could be trying to talk to us with an ephemeral + * port. */ if (inp->inp_fport != 0 && inp->inp_fport != uh->uh_sport) @@ -426,16 +436,16 @@ INP_RLOCK(inp); /* - * Handle socket delivery policy for any-source - * and source-specific multicast. [RFC3678] + * Handle socket delivery policy for any-source and + * source-specific multicast. [RFC3678] */ imo = inp->inp_moptions; if (IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) && imo != NULL) { - struct sockaddr_in sin; - struct in_msource *ims; - int blocked, mode; - size_t idx; + struct sockaddr_in sin; + struct in_msource *ims; + int blocked, mode; + size_t idx; bzero(&sin, sizeof(struct sockaddr_in)); sin.sin_len = sizeof(struct sockaddr_in); @@ -447,27 +457,29 @@ (struct sockaddr *)&sin); if (idx == -1) { /* - * No group membership for this socket. - * Do not bump udps_noportbcast, as - * this will happen further down. + * No group membership for this + * socket. Do not bump + * udps_noportbcast, as this will + * happen further down. */ blocked++; } else { /* - * Check for a multicast source filter - * entry on this socket for this group. - * MCAST_EXCLUDE is the default - * behaviour. It means default accept; - * entries, if present, denote sources - * to be excluded from delivery. + * Check for a multicast source + * filter entry on this socket for + * this group. MCAST_EXCLUDE is the + * default behaviour. It means + * default accept; entries, if + * present, denote sources to be + * excluded from delivery. */ ims = imo_match_source(imo, idx, (struct sockaddr *)&udp_in); mode = imo->imo_mfilters[idx].imf_fmode; if ((ims != NULL && - mode == MCAST_EXCLUDE) || + mode == MCAST_EXCLUDE) || (ims == NULL && - mode == MCAST_INCLUDE)) { + mode == MCAST_INCLUDE)) { #ifdef DIAGNOSTIC if (bootverbose) { printf("%s: blocked by" @@ -488,41 +500,72 @@ struct mbuf *n; n = m_copy(m, 0, M_COPYALL); - if (n != NULL) - udp_append(last, ip, n, iphlen + - sizeof(struct udphdr), &udp_in); - INP_RUNLOCK(last); + if (last->inp_ppcb == NULL) { + if (n != NULL) + udp_append(last, ip, n, iphlen + + sizeof(struct udphdr), &udp_in); + INP_RUNLOCK(last); + } else { + /* + * Engage the tunneling protocol we + * will have to leave the info_lock + * up, since we are hunting through + * multiple UDP inp's hope we don't + * break! + */ + udp_tunnel_func_t tunnel_func; + + INP_RUNLOCK(last); + tunnel_func = (udp_tunnel_func_t) last->inp_ppcb; + INP_RUNLOCK(last); + tunnel_func(m, iphlen); + } } last = inp; /* - * Don't look for additional matches if this one does - * not have either the SO_REUSEPORT or SO_REUSEADDR - * socket options set. This heuristic avoids - * searching through all pcbs in the common case of a - * non-shared port. It assumes that an application - * will never clear these options after setting them. + * Don't look for additional matches if this one + * does not have either the SO_REUSEPORT or + * SO_REUSEADDR socket options set. This heuristic + * avoids searching through all pcbs in the common + * case of a non-shared port. It assumes that an + * application will never clear these options after + * setting them. */ if ((last->inp_socket->so_options & - (SO_REUSEPORT|SO_REUSEADDR)) == 0) + (SO_REUSEPORT | SO_REUSEADDR)) == 0) break; } if (last == NULL) { /* - * No matching pcb found; discard datagram. (No need - * to send an ICMP Port Unreachable for a broadcast - * or multicast datgram.) + * No matching pcb found; discard datagram. (No + * need to send an ICMP Port Unreachable for a + * broadcast or multicast datgram.) */ V_udpstat.udps_noportbcast++; goto badheadlocked; } - udp_append(last, ip, m, iphlen + sizeof(struct udphdr), - &udp_in); - INP_RUNLOCK(last); - INP_INFO_RUNLOCK(&V_udbinfo); + if (last->inp_ppcb == NULL) { + udp_append(last, ip, m, iphlen + sizeof(struct udphdr), + &udp_in); + INP_RUNLOCK(last); + INP_INFO_RUNLOCK(&V_udbinfo); + } else { + /* + * Engage the tunneling protocol we must make sure + * all locks are released when we call the tunneling + * protocol. + */ + udp_tunnel_func_t tunnel_func; + + INP_RUNLOCK(last); + INP_INFO_RUNLOCK(&V_udbinfo); + tunnel_func = (udp_tunnel_func_t) last->inp_ppcb; + INP_RUNLOCK(last); + tunnel_func(m, iphlen); + } return; } - /* * Locate pcb for datagram. */ @@ -530,7 +573,7 @@ ip->ip_dst, uh->uh_dport, 1, ifp); if (inp == NULL) { if (udp_log_in_vain) { - char buf[4*sizeof "123"]; + char buf[4 * sizeof "123"]; strcpy(buf, inet_ntoa(ip->ip_dst)); log(LOG_INFO, @@ -553,7 +596,6 @@ INP_INFO_RUNLOCK(&V_udbinfo); return; } - /* * Check the minimum TTL for socket. */ @@ -563,6 +605,18 @@ INP_RUNLOCK(inp); goto badunlocked; } + if (inp->inp_ppcb) { + /* + * Engage the tunneling protocol we must make sure all locks + * are released when we call the tunneling protocol. + */ + udp_tunnel_func_t tunnel_func; + + tunnel_func = (udp_tunnel_func_t) inp->inp_ppcb; + INP_RUNLOCK(inp); + tunnel_func(m, iphlen); + return; + } udp_append(inp, ip, m, iphlen + sizeof(struct udphdr), &udp_in); INP_RUNLOCK(inp); return; @@ -586,8 +640,8 @@ /* * While udp_ctlinput() always calls udp_notify() with a read lock * when invoking it directly, in_pcbnotifyall() currently uses write - * locks due to sharing code with TCP. For now, accept either a read - * or a write lock, but a read lock is sufficient. + * locks due to sharing code with TCP. For now, accept either a + * read or a write lock, but a read lock is sufficient. */ INP_LOCK_ASSERT(inp); @@ -618,7 +672,7 @@ /* * Hostdead is ugly because it goes linearly through all PCBs. - * + * * XXX: We never get this from ICMP, otherwise it makes an excellent * DoS attack on machines with many connections. */ @@ -660,10 +714,9 @@ if (req->oldptr == 0) { n = V_udbinfo.ipi_count; req->oldidx = 2 * (sizeof xig) - + (n + n/8) * sizeof(struct xinpcb); + + (n + n / 8) * sizeof(struct xinpcb); return (0); } - if (req->newptr != 0) return (EPERM); @@ -676,7 +729,7 @@ INP_INFO_RUNLOCK(&V_udbinfo); error = sysctl_wire_old_buffer(req, 2 * (sizeof xig) - + n * sizeof(struct xinpcb)); + + n * sizeof(struct xinpcb)); if (error != 0) return (error); @@ -694,7 +747,7 @@ INP_INFO_RLOCK(&V_udbinfo); for (inp = LIST_FIRST(V_udbinfo.ipi_listhead), i = 0; inp && i < n; - inp = LIST_NEXT(inp, inp_list)) { + inp = LIST_NEXT(inp, inp_list)) { INP_RLOCK(inp); if (inp->inp_gencnt <= gencnt && cr_canseeinpcb(req->td->td_ucred, inp) == 0) @@ -710,6 +763,7 @@ INP_RLOCK(inp); if (inp->inp_gencnt <= gencnt) { struct xinpcb xi; + bzero(&xi, sizeof(xi)); xi.xi_len = sizeof xi; /* XXX should avoid extra copy */ @@ -725,9 +779,9 @@ if (!error) { /* * Give the user an updated idea of our state. If the - * generation differs from what we told her before, she knows - * that something happened while we were processing this - * request, and it might be necessary to retry. + * generation differs from what we told her before, she + * knows that something happened while we were processing + * this request, and it might be necessary to retry. */ INP_INFO_RLOCK(&V_udbinfo); xig.xig_gen = V_udbinfo.ipi_gencnt; @@ -760,7 +814,7 @@ return (error); INP_INFO_RLOCK(&V_udbinfo); inp = in_pcblookup_hash(&V_udbinfo, addrs[1].sin_addr, addrs[1].sin_port, - addrs[0].sin_addr, addrs[0].sin_port, 1, NULL); + addrs[0].sin_addr, addrs[0].sin_port, 1, NULL); if (inp != NULL) { INP_RLOCK(inp); INP_INFO_RUNLOCK(&V_udbinfo); @@ -781,7 +835,7 @@ } SYSCTL_PROC(_net_inet_udp, OID_AUTO, getcred, - CTLTYPE_OPAQUE|CTLFLAG_RW|CTLFLAG_PRISON, 0, 0, + CTLTYPE_OPAQUE | CTLFLAG_RW | CTLFLAG_PRISON, 0, 0, udp_getcred, "S,xucred", "Get the xucred of a UDP connection"); static int @@ -811,7 +865,6 @@ m_freem(m); return (EMSGSIZE); } - src.sin_family = 0; if (control != NULL) { /* @@ -863,20 +916,19 @@ m_freem(m); return (error); } - /* - * Depending on whether or not the application has bound or connected - * the socket, we may have to do varying levels of work. The optimal - * case is for a connected UDP socket, as a global lock isn't - * required at all. - * - * In order to decide which we need, we require stability of the - * inpcb binding, which we ensure by acquiring a read lock on the - * inpcb. This doesn't strictly follow the lock order, so we play - * the trylock and retry game; note that we may end up with more + * Depending on whether or not the application has bound or + * connected the socket, we may have to do varying levels of work. + * The optimal case is for a connected UDP socket, as a global lock + * isn't required at all. + * + * In order to decide which we need, we require stability of the inpcb + * binding, which we ensure by acquiring a read lock on the inpcb. + * This doesn't strictly follow the lock order, so we play the + * trylock and retry game; note that we may end up with more * conservative locks than required the second time around, so later - * assertions have to accept that. Further analysis of the number of - * misses under contention is required. + * assertions have to accept that. Further analysis of the number + * of misses under contention is required. */ sin = (struct sockaddr_in *)addr; INP_RLOCK(inp); @@ -887,10 +939,10 @@ INP_WLOCK(inp); unlock_udbinfo = 2; } else if ((sin != NULL && ( - (sin->sin_addr.s_addr == INADDR_ANY) || - (sin->sin_addr.s_addr == INADDR_BROADCAST) || - (inp->inp_laddr.s_addr == INADDR_ANY) || - (inp->inp_lport == 0))) || + (sin->sin_addr.s_addr == INADDR_ANY) || + (sin->sin_addr.s_addr == INADDR_BROADCAST) || + (inp->inp_laddr.s_addr == INADDR_ANY) || + (inp->inp_lport == 0))) || (src.sin_family == AF_INET)) { if (!INP_INFO_TRY_RLOCK(&V_udbinfo)) { INP_RUNLOCK(inp); @@ -912,7 +964,7 @@ INP_INFO_LOCK_ASSERT(&V_udbinfo); if ((lport == 0) || (laddr.s_addr == INADDR_ANY && - src.sin_addr.s_addr == INADDR_ANY)) { + src.sin_addr.s_addr == INADDR_ANY)) { error = EINVAL; goto release; } @@ -921,11 +973,10 @@ if (error) goto release; } - /* - * If a UDP socket has been connected, then a local address/port will - * have been selected and bound. - * + * If a UDP socket has been connected, then a local address/port + * will have been selected and bound. + * * If a UDP socket has not been connected to, then an explicit * destination address must be used, in which case a local * address/port may not have been selected and bound. @@ -936,7 +987,6 @@ error = EISCONN; goto release; } - /* * Jail may rewrite the destination address, so let it do * that before we use it. @@ -945,17 +995,17 @@ error = EINVAL; goto release; } - /* - * If a local address or port hasn't yet been selected, or if - * the destination address needs to be rewritten due to using - * a special INADDR_ constant, invoke in_pcbconnect_setup() - * to do the heavy lifting. Once a port is selected, we - * commit the binding back to the socket; we also commit the - * binding of the address if in jail. - * - * If we already have a valid binding and we're not - * requesting a destination address rewrite, use a fast path. + * If a local address or port hasn't yet been selected, or + * if the destination address needs to be rewritten due to + * using a special INADDR_ constant, invoke + * in_pcbconnect_setup() to do the heavy lifting. Once a + * port is selected, we commit the binding back to the + * socket; we also commit the binding of the address if in + * jail. + * + * If we already have a valid binding and we're not requesting + * a destination address rewrite, use a fast path. */ if (inp->inp_laddr.s_addr == INADDR_ANY || inp->inp_lport == 0 || @@ -1007,8 +1057,8 @@ /* * Calculate data length and get a mbuf for UDP, IP, and possible - * link-layer headers. Immediate slide the data pointer back forward - * since we won't use that space at this layer. + * link-layer headers. Immediate slide the data pointer back + * forward since we won't use that space at this layer. */ M_PREPEND(m, sizeof(struct udpiphdr) + max_linkhdr, M_DONTWAIT); if (m == NULL) { @@ -1020,8 +1070,8 @@ m->m_pkthdr.len -= max_linkhdr; /* - * Fill in mbuf with extended UDP header and addresses and length put - * into network format. + * Fill in mbuf with extended UDP header and addresses and length + * put into network format. */ ui = mtod(m, struct udpiphdr *); bzero(ui->ui_x1, sizeof(ui->ui_x1)); /* XXX still needed? */ @@ -1041,7 +1091,6 @@ ip = (struct ip *)&ui->ui_i; ip->ip_off |= IP_DF; } - ipflags = 0; if (inp->inp_socket->so_options & SO_DONTROUTE) ipflags |= IP_ROUTETOIF; @@ -1066,7 +1115,7 @@ m->m_pkthdr.csum_data = offsetof(struct udphdr, uh_sum); } else ui->ui_sum = 0; - ((struct ip *)ui)->ip_len = sizeof (struct udpiphdr) + len; + ((struct ip *)ui)->ip_len = sizeof(struct udpiphdr) + len; ((struct ip *)ui)->ip_ttl = inp->inp_ip_ttl; /* XXX */ ((struct ip *)ui)->ip_tos = inp->inp_ip_tos; /* XXX */ V_udpstat.udps_opackets++; @@ -1133,15 +1182,42 @@ INP_INFO_WUNLOCK(&V_udbinfo); return (error); } - inp = (struct inpcb *)so->so_pcb; INP_INFO_WUNLOCK(&V_udbinfo); inp->inp_vflag |= INP_IPV4; inp->inp_ip_ttl = V_ip_defttl; + /* + * UDP does not have a per-protocol pcb (inp->inp_ppcb). We use this + * pointer for kernel tunneling pointer. If we ever need to have a + * protocol block we will need to move this function pointer there. + * Null in this pointer means "do the normal thing". + */ + inp->inp_ppcb = NULL; INP_WUNLOCK(inp); return (0); } +int +udp_set_kernel_tunneling(struct socket *so, udp_tunnel_func_t f) +{ + struct inpcb *inp; + + inp = (struct inpcb *)so->so_pcb; + + if (so->so_type != SOCK_DGRAM) { + /* Not UDP socket... sorry */ + return (ENOTSUP); + } + if (inp == NULL) { + /* NULL INP? */ + return (EINVAL); + } + INP_WLOCK(inp); + inp->inp_ppcb = f; + INP_WUNLOCK(inp); + return (0); +} + static int udp_bind(struct socket *so, struct sockaddr *nam, struct thread *td) { @@ -1241,11 +1317,10 @@ INP_INFO_WUNLOCK(&V_udbinfo); return (ENOTCONN); } - in_pcbdisconnect(inp); inp->inp_laddr.s_addr = INADDR_ANY; SOCK_LOCK(so); - so->so_state &= ~SS_ISCONNECTED; /* XXX */ + so->so_state &= ~SS_ISCONNECTED; /* XXX */ SOCK_UNLOCK(so); INP_WUNLOCK(inp); INP_INFO_WUNLOCK(&V_udbinfo); @@ -1277,19 +1352,19 @@ } struct pr_usrreqs udp_usrreqs = { - .pru_abort = udp_abort, - .pru_attach = udp_attach, - .pru_bind = udp_bind, - .pru_connect = udp_connect, - .pru_control = in_control, - .pru_detach = udp_detach, - .pru_disconnect = udp_disconnect, - .pru_peeraddr = in_getpeeraddr, - .pru_send = udp_send, - .pru_soreceive = soreceive_dgram, - .pru_sosend = sosend_dgram, - .pru_shutdown = udp_shutdown, - .pru_sockaddr = in_getsockaddr, - .pru_sosetlabel = in_pcbsosetlabel, - .pru_close = udp_close, + .pru_abort = udp_abort, + .pru_attach = udp_attach, + .pru_bind = udp_bind, + .pru_connect = udp_connect, + .pru_control = in_control, + .pru_detach = udp_detach, + .pru_disconnect = udp_disconnect, + .pru_peeraddr = in_getpeeraddr, + .pru_send = udp_send, + .pru_soreceive = soreceive_dgram, + .pru_sosend = sosend_dgram, + .pru_shutdown = udp_shutdown, + .pru_sockaddr = in_getsockaddr, + .pru_sosetlabel = in_pcbsosetlabel, + .pru_close = udp_close, }; Index: netinet/udp_var.h =================================================================== --- netinet/udp_var.h (revision 185928) +++ netinet/udp_var.h (working copy) @@ -38,9 +38,10 @@ * UDP kernel structures and variables. */ struct udpiphdr { - struct ipovly ui_i; /* overlaid ip structure */ - struct udphdr ui_u; /* udp header */ + struct ipovly ui_i; /* overlaid ip structure */ + struct udphdr ui_u; /* udp header */ }; + #define ui_x1 ui_i.ih_x1 #define ui_pr ui_i.ih_pr #define ui_len ui_i.ih_len @@ -52,23 +53,23 @@ #define ui_sum ui_u.uh_sum struct udpstat { - /* input statistics: */ - u_long udps_ipackets; /* total input packets */ - u_long udps_hdrops; /* packet shorter than header */ - u_long udps_badsum; /* checksum error */ - u_long udps_nosum; /* no checksum */ - u_long udps_badlen; /* data length larger than packet */ - u_long udps_noport; /* no socket on port */ - u_long udps_noportbcast; /* of above, arrived as broadcast */ - u_long udps_fullsock; /* not delivered, input socket full */ - u_long udpps_pcbcachemiss; /* input packets missing pcb cache */ - u_long udpps_pcbhashmiss; /* input packets not for hashed pcb */ - /* output statistics: */ - u_long udps_opackets; /* total output packets */ - u_long udps_fastout; /* output packets on fast path */ + /* input statistics: */ + u_long udps_ipackets; /* total input packets */ + u_long udps_hdrops; /* packet shorter than header */ + u_long udps_badsum; /* checksum error */ + u_long udps_nosum; /* no checksum */ + u_long udps_badlen; /* data length larger than packet */ + u_long udps_noport; /* no socket on port */ + u_long udps_noportbcast;/* of above, arrived as broadcast */ + u_long udps_fullsock; /* not delivered, input socket full */ + u_long udpps_pcbcachemiss; /* input packets missing pcb cache */ + u_long udpps_pcbhashmiss; /* input packets not for hashed pcb */ + /* output statistics: */ + u_long udps_opackets; /* total output packets */ + u_long udps_fastout; /* output packets on fast path */ /* of no socket on port, arrived as multicast */ - u_long udps_noportmcast; - u_long udps_filtermcast; /* blocked by multicast filter */ + u_long udps_noportmcast; + u_long udps_filtermcast;/* blocked by multicast filter */ }; /* @@ -93,20 +94,25 @@ #ifdef _KERNEL SYSCTL_DECL(_net_inet_udp); -extern struct pr_usrreqs udp_usrreqs; -extern struct inpcbhead udb; -extern struct inpcbinfo udbinfo; -extern u_long udp_sendspace; -extern u_long udp_recvspace; -extern struct udpstat udpstat; -extern int udp_blackhole; -extern int udp_log_in_vain; +extern struct pr_usrreqs udp_usrreqs; +extern struct inpcbhead udb; +extern struct inpcbinfo udbinfo; +extern u_long udp_sendspace; +extern u_long udp_recvspace; +extern struct udpstat udpstat; +extern int udp_blackhole; +extern int udp_log_in_vain; -void udp_ctlinput(int, struct sockaddr *, void *); -void udp_init(void); -void udp_input(struct mbuf *, int); -struct inpcb *udp_notify(struct inpcb *inp, int errno); -int udp_shutdown(struct socket *so); +void udp_ctlinput(int, struct sockaddr *, void *); +void udp_init(void); +void udp_input(struct mbuf *, int); +struct inpcb *udp_notify(struct inpcb *inp, int errno); +int udp_shutdown(struct socket *so); + + +typedef void (*udp_tunnel_func_t) (struct mbuf *, int off); +int udp_set_kernel_tunneling(struct socket *so, udp_tunnel_function_t f); + #endif #endif Index: netinet6/udp6_usrreq.c =================================================================== --- netinet6/udp6_usrreq.c (revision 185928) +++ netinet6/udp6_usrreq.c (working copy) @@ -115,7 +115,7 @@ #ifdef IPSEC #include #include -#endif /* IPSEC */ +#endif /* IPSEC */ #include @@ -124,8 +124,8 @@ * Per RFC 768, August, 1980. */ -extern struct protosw inetsw[]; -static void udp6_detach(struct socket *so); +extern struct protosw inetsw[]; +static void udp6_detach(struct socket *so); static void udp6_append(struct inpcb *inp, struct mbuf *n, int off, @@ -145,7 +145,7 @@ V_ipsec6stat.in_polvio++; return; } -#endif /* IPSEC */ +#endif /* IPSEC */ #ifdef MAC if (mac_inpcb_check_deliver(inp, n) != 0) { m_freem(n); @@ -186,12 +186,11 @@ ip6 = mtod(m, struct ip6_hdr *); - if (faithprefix_p != NULL && (*faithprefix_p)(&ip6->ip6_dst)) { + if (faithprefix_p != NULL && (*faithprefix_p) (&ip6->ip6_dst)) { /* XXX send icmp6 host/port unreach? */ m_freem(m); return (IPPROTO_DONE); } - #ifndef PULLDOWN_TEST IP6_EXTHDR_CHECK(m, off, sizeof(struct udphdr), IPPROTO_DONE); ip6 = mtod(m, struct ip6_hdr *); @@ -217,7 +216,6 @@ V_udpstat.udps_badlen++; goto badunlocked; } - /* * Checksum extended UDP header and data. */ @@ -229,7 +227,6 @@ V_udpstat.udps_badsum++; goto badunlocked; } - /* * Construct sockaddr format source address. */ @@ -243,8 +240,8 @@ /* * In the event that laddr should be set to the link-local * address (this happens in RIPng), the multicast address - * specified in the received packet will not match laddr. To - * handle this situation, matching is relaxed if the + * specified in the received packet will not match laddr. + * To handle this situation, matching is relaxed if the * receiving interface is the same as one specified in the * socket and if the destination multicast address matches * one of the multicast groups specified in the socket. @@ -262,54 +259,72 @@ if (inp->in6p_lport != uh->uh_dport) continue; /* - * XXX: Do not check source port of incoming datagram - * unless inp_connect() has been called to bind the - * fport part of the 4-tuple; the source could be - * trying to talk to us with an ephemeral port. + * XXX: Do not check source port of incoming + * datagram unless inp_connect() has been called to + * bind the fport part of the 4-tuple; the source + * could be trying to talk to us with an ephemeral + * port. */ if (inp->inp_fport != 0 && inp->inp_fport != uh->uh_sport) continue; if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr)) { if (!IN6_ARE_ADDR_EQUAL(&inp->in6p_laddr, - &ip6->ip6_dst)) + &ip6->ip6_dst)) continue; } if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) { if (!IN6_ARE_ADDR_EQUAL(&inp->in6p_faddr, - &ip6->ip6_src) || + &ip6->ip6_src) || inp->in6p_fport != uh->uh_sport) continue; } - if (last != NULL) { struct mbuf *n; if ((n = m_copy(m, 0, M_COPYALL)) != NULL) { - INP_RLOCK(last); - udp6_append(last, n, off, &fromsa); - INP_RUNLOCK(last); + if (last->inp_ppcb) { + /* + * Engage the tunneling + * protocol we will have to + * leave the info_lock up, + * since we are hunting + * through multiple UDP + * inp's hope we don't break + * :-( + */ + udp_tunnel_func tunnel_func; + + tunnel_func = (udp_tunnel_func_t) last->inp_ppcb; + INP_RUNLOCK(last); + tunnel_func(m, off); + } else { + INP_RLOCK(last); + udp6_append(last, n, off, &fromsa); + INP_RUNLOCK(last); + } } } last = inp; /* - * Don't look for additional matches if this one does - * not have either the SO_REUSEPORT or SO_REUSEADDR - * socket options set. This heuristic avoids - * searching through all pcbs in the common case of a - * non-shared port. It assumes that an application - * will never clear these options after setting them. + * Don't look for additional matches if this one + * does not have either the SO_REUSEPORT or + * SO_REUSEADDR socket options set. This heuristic + * avoids searching through all pcbs in the common + * case of a non-shared port. It assumes that an + * application will never clear these options after + * setting them. */ if ((last->inp_socket->so_options & - (SO_REUSEPORT|SO_REUSEADDR)) == 0) + (SO_REUSEPORT | SO_REUSEADDR)) == 0) break; } if (last == NULL) { /* - * No matching pcb found; discard datagram. (No need - * to send an ICMP Port Unreachable for a broadcast - * or multicast datgram.) + * No matching pcb found; discard datagram. (No + * need to send an ICMP Port Unreachable for a + * broadcast or multicast datgram.) */ V_udpstat.udps_noport++; V_udpstat.udps_noportmcast++; @@ -317,6 +332,19 @@ } INP_RLOCK(last); INP_INFO_RUNLOCK(&V_udbinfo); + if (last->inp_ppcb) { + /* + * Engage the tunneling protocol we must make sure + * all locks are released when we call the tunneling + * protocol. + */ + udp_tunnel_func_t tunnel_func; + + tunnel_func = (udp_tunnel_func_t) inp->inp_ppcb; + INP_RUNLOCK(last); + tunnel_func(m, off); + return (IPPROTO_DONE); + } udp6_append(last, m, off, &fromsa); INP_RUNLOCK(last); return (IPPROTO_DONE); @@ -354,6 +382,18 @@ } INP_RLOCK(inp); INP_INFO_RUNLOCK(&V_udbinfo); + if (inp->inp_ppcb) { + /* + * Engage the tunneling protocol we must make sure all locks + * are released when we call the tunneling protocol. + */ + udp_tunnel_func_t tunnel_func; + + tunnel_func = (udp_tunnel_func_t) inp->inp_ppcb; + INP_RUNLOCK(inp); + tunnel_func(m, off); + return (IPPROTO_DONE); + } udp6_append(inp, m, off, &fromsa); INP_RUNLOCK(inp); return (IPPROTO_DONE); @@ -377,11 +417,11 @@ struct ip6ctlparam *ip6cp = NULL; const struct sockaddr_in6 *sa6_src = NULL; void *cmdarg; - struct inpcb *(*notify)(struct inpcb *, int) = udp_notify; + struct inpcb *(*notify) (struct inpcb *, int)= udp_notify; struct udp_portonly { u_int16_t uh_sport; u_int16_t uh_dport; - } *uhp; + } *uhp; if (sa->sa_family != AF_INET6 || sa->sa_len != sizeof(struct sockaddr_in6)) @@ -413,8 +453,8 @@ if (ip6) { /* - * XXX: We assume that when IPV6 is non NULL, - * M and OFF are valid. + * XXX: We assume that when IPV6 is non NULL, M and OFF are + * valid. */ /* Check if we can safely examine src and dst ports. */ @@ -424,11 +464,11 @@ bzero(&uh, sizeof(uh)); m_copydata(m, off, sizeof(*uhp), (caddr_t)&uh); - (void) in6_pcbnotify(&V_udbinfo, sa, uh.uh_dport, + (void)in6_pcbnotify(&V_udbinfo, sa, uh.uh_dport, (struct sockaddr *)ip6cp->ip6c_src, uh.uh_sport, cmd, cmdarg, notify); } else - (void) in6_pcbnotify(&V_udbinfo, sa, 0, + (void)in6_pcbnotify(&V_udbinfo, sa, 0, (const struct sockaddr *)sa6_src, 0, cmd, cmdarg, notify); } @@ -481,7 +521,7 @@ return (error); } -SYSCTL_PROC(_net_inet6_udp6, OID_AUTO, getcred, CTLTYPE_OPAQUE|CTLFLAG_RW, 0, +SYSCTL_PROC(_net_inet6_udp6, OID_AUTO, getcred, CTLTYPE_OPAQUE | CTLFLAG_RW, 0, 0, udp6_getcred, "S,xucred", "Get the xucred of a UDP6 connection"); static int @@ -520,15 +560,15 @@ * default zone IDs should be enabled. Unfortunately, some * applications do not behave as it should, so we need a * workaround. Even if an appropriate ID is not determined, - * we'll see if we can determine the outgoing interface. If we - * can, determine the zone ID based on the interface below. + * we'll see if we can determine the outgoing interface. If + * we can, determine the zone ID based on the interface + * below. */ if (sin6->sin6_scope_id == 0 && !V_ip6_use_defzone) scope_ambiguous = 1; if ((error = sa6_embedscope(sin6, V_ip6_use_defzone)) != 0) return (error); } - if (control) { if ((error = ip6_setpktopts(control, &opt, inp->in6p_outputopts, td->td_ucred, IPPROTO_UDP)) != 0) @@ -541,37 +581,35 @@ faddr = &sin6->sin6_addr; /* - * IPv4 version of udp_output calls in_pcbconnect in this case, - * which needs splnet and affects performance. - * Since we saw no essential reason for calling in_pcbconnect, - * we get rid of such kind of logic, and call in6_selectsrc - * and in6_pcbsetport in order to fill in the local address - * and the local port. + * IPv4 version of udp_output calls in_pcbconnect in this + * case, which needs splnet and affects performance. Since + * we saw no essential reason for calling in_pcbconnect, we + * get rid of such kind of logic, and call in6_selectsrc and + * in6_pcbsetport in order to fill in the local address and + * the local port. */ if (sin6->sin6_port == 0) { error = EADDRNOTAVAIL; goto release; } - if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_faddr)) { /* how about ::ffff:0.0.0.0 case? */ error = EISCONN; goto release; } + fport = sin6->sin6_port; /* allow 0 port */ - fport = sin6->sin6_port; /* allow 0 port */ - if (IN6_IS_ADDR_V4MAPPED(faddr)) { if ((inp->in6p_flags & IN6P_IPV6_V6ONLY)) { /* - * I believe we should explicitly discard the - * packet when mapped addresses are disabled, - * rather than send the packet as an IPv6 one. - * If we chose the latter approach, the packet - * might be sent out on the wire based on the - * default route, the situation which we'd - * probably want to avoid. - * (20010421 jinmei@kame.net) + * I believe we should explicitly discard + * the packet when mapped addresses are + * disabled, rather than send the packet as + * an IPv6 one. If we chose the latter + * approach, the packet might be sent out on + * the wire based on the default route, the + * situation which we'd probably want to + * avoid. (20010421 jinmei@kame.net) */ error = EINVAL; goto release; @@ -579,18 +617,16 @@ if (!IN6_IS_ADDR_UNSPECIFIED(&inp->in6p_laddr) && !IN6_IS_ADDR_V4MAPPED(&inp->in6p_laddr)) { /* - * when remote addr is an IPv4-mapped address, - * local addr should not be an IPv6 address, - * since you cannot determine how to map IPv6 - * source address to IPv4. + * when remote addr is an IPv4-mapped + * address, local addr should not be an IPv6 + * address, since you cannot determine how + * to map IPv6 source address to IPv4. */ error = EINVAL; goto release; } - af = AF_INET; } - if (!IN6_IS_ADDR_V4MAPPED(faddr)) { laddr = in6_selectsrc(sin6, optp, inp, NULL, td->td_ucred, &oifp, &error); @@ -619,9 +655,9 @@ /* * XXX: this case would happen when the * application sets the V6ONLY flag after - * connecting the foreign address. - * Such applications should be fixed, - * so we bark here. + * connecting the foreign address. Such + * applications should be fixed, so we bark + * here. */ log(LOG_INFO, "udp6_output: IPV6_V6ONLY " "option was set for a connected socket\n"); @@ -639,20 +675,19 @@ hlen = sizeof(struct ip); /* - * Calculate data length and get a mbuf - * for UDP and IP6 headers. + * Calculate data length and get a mbuf for UDP and IP6 headers. */ M_PREPEND(m, hlen + sizeof(struct udphdr), M_DONTWAIT); if (m == 0) { error = ENOBUFS; goto release; } - /* * Stuff checksum and output datagram. */ - udp6 = (struct udphdr *)(mtod(m, caddr_t) + hlen); - udp6->uh_sport = inp->in6p_lport; /* lport is always set in the PCB */ + udp6 = (struct udphdr *)(mtod(m, caddr_t)+hlen); + udp6->uh_sport = inp->in6p_lport; /* lport is always set in the + * PCB */ udp6->uh_dport = fport; if (plen <= 0xffff) udp6->uh_ulen = htons((u_short)plen); @@ -663,22 +698,21 @@ switch (af) { case AF_INET6: ip6 = mtod(m, struct ip6_hdr *); - ip6->ip6_flow = inp->in6p_flowinfo & IPV6_FLOWINFO_MASK; - ip6->ip6_vfc &= ~IPV6_VERSION_MASK; - ip6->ip6_vfc |= IPV6_VERSION; + ip6->ip6_flow = inp->in6p_flowinfo & IPV6_FLOWINFO_MASK; + ip6->ip6_vfc &= ~IPV6_VERSION_MASK; + ip6->ip6_vfc |= IPV6_VERSION; #if 0 /* ip6_plen will be filled in ip6_output. */ - ip6->ip6_plen = htons((u_short)plen); + ip6->ip6_plen = htons((u_short)plen); #endif - ip6->ip6_nxt = IPPROTO_UDP; - ip6->ip6_hlim = in6_selecthlim(inp, NULL); - ip6->ip6_src = *laddr; - ip6->ip6_dst = *faddr; + ip6->ip6_nxt = IPPROTO_UDP; + ip6->ip6_hlim = in6_selecthlim(inp, NULL); + ip6->ip6_src = *laddr; + ip6->ip6_dst = *faddr; if ((udp6->uh_sum = in6_cksum(m, IPPROTO_UDP, - sizeof(struct ip6_hdr), plen)) == 0) { + sizeof(struct ip6_hdr), plen)) == 0) { udp6->uh_sum = 0xffff; } - flags = 0; V_udpstat.udps_opackets++; @@ -716,7 +750,7 @@ struct pr_usrreqs *pru; pru = inetsw[ip_protox[IPPROTO_UDP]].pr_usrreqs; - (*pru->pru_abort)(so); + (*pru->pru_abort) (so); return; } #endif @@ -761,10 +795,9 @@ inp->in6p_hops = -1; /* use kernel default */ inp->in6p_cksum = -1; /* just to be sure */ /* - * XXX: ugly!! - * IPv4 TTL initialization is necessary for an IPv6 socket as well, - * because the socket may be bound to an IPv6 wildcard address, - * which may match an IPv4-mapped IPv6 address. + * XXX: ugly!! IPv4 TTL initialization is necessary for an IPv6 + * socket as well, because the socket may be bound to an IPv6 + * wildcard address, which may match an IPv4-mapped IPv6 address. */ inp->inp_ip_ttl = V_ip_defttl; INP_WUNLOCK(inp); @@ -803,7 +836,6 @@ goto out; } } - error = in6_pcbbind(inp, nam, td->td_ucred); out: INP_WUNLOCK(inp); @@ -825,7 +857,7 @@ struct pr_usrreqs *pru; pru = inetsw[ip_protox[IPPROTO_UDP]].pr_usrreqs; - (*pru->pru_disconnect)(so); + (*pru->pru_disconnect) (so); return; } #endif @@ -886,6 +918,7 @@ } if (td && jailed(td->td_ucred)) { struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)nam; + if (prison_remote_ip6(td->td_ucred, &sin6->sin6_addr) != 0) { error = EAFNOSUPPORT; goto out; @@ -940,7 +973,7 @@ struct pr_usrreqs *pru; pru = inetsw[ip_protox[IPPROTO_UDP]].pr_usrreqs; - error = (*pru->pru_disconnect)(so); + error = (*pru->pru_disconnect) (so); goto out; } #endif @@ -949,11 +982,10 @@ error = ENOTCONN; goto out; } - in6_pcbdisconnect(inp); inp->in6p_laddr = in6addr_any; SOCK_LOCK(so); - so->so_state &= ~SS_ISCONNECTED; /* XXX */ + so->so_state &= ~SS_ISCONNECTED; /* XXX */ SOCK_UNLOCK(so); out: INP_WUNLOCK(inp); @@ -984,7 +1016,6 @@ goto bad; } } - #ifdef INET if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0) { int hasv4addr; @@ -1005,18 +1036,17 @@ /* * When remote addr is IPv4-mapped address, * local addr should not be an IPv6 address; - * since you cannot determine how to map IPv6 - * source address to IPv4. + * since you cannot determine how to map + * IPv6 source address to IPv4. */ error = EINVAL; goto out; } - /* * XXXRW: We release UDP-layer locks before calling * udp_send() in order to avoid recursion. However, - * this does mean there is a short window where inp's - * fields are unstable. Could this lead to a + * this does mean there is a short window where + * inp's fields are unstable. Could this lead to a * potential race in which the factors causing us to * select the UDPv4 output routine are invalidated? */ @@ -1026,7 +1056,7 @@ in6_sin6_2_sin_in_sock(addr); pru = inetsw[ip_protox[IPPROTO_UDP]].pr_usrreqs; /* addr will just be freed in sendit(). */ - return ((*pru->pru_send)(so, flags, m, addr, control, + return ((*pru->pru_send) (so, flags, m, addr, control, td)); } } @@ -1048,19 +1078,19 @@ } struct pr_usrreqs udp6_usrreqs = { - .pru_abort = udp6_abort, - .pru_attach = udp6_attach, - .pru_bind = udp6_bind, - .pru_connect = udp6_connect, - .pru_control = in6_control, - .pru_detach = udp6_detach, - .pru_disconnect = udp6_disconnect, - .pru_peeraddr = in6_mapped_peeraddr, - .pru_send = udp6_send, - .pru_shutdown = udp_shutdown, - .pru_sockaddr = in6_mapped_sockaddr, - .pru_soreceive = soreceive_dgram, - .pru_sosend = sosend_dgram, - .pru_sosetlabel = in_pcbsosetlabel, - .pru_close = udp6_close + .pru_abort = udp6_abort, + .pru_attach = udp6_attach, + .pru_bind = udp6_bind, + .pru_connect = udp6_connect, + .pru_control = in6_control, + .pru_detach = udp6_detach, + .pru_disconnect = udp6_disconnect, + .pru_peeraddr = in6_mapped_peeraddr, + .pru_send = udp6_send, + .pru_shutdown = udp_shutdown, + .pru_sockaddr = in6_mapped_sockaddr, + .pru_soreceive = soreceive_dgram, + .pru_sosend = sosend_dgram, + .pru_sosetlabel = in_pcbsosetlabel, + .pru_close = udp6_close }; --Apple-Mail-90--449110523 Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Content-Transfer-Encoding: 7bit On Dec 12, 2008, at 11:47 AM, Bruce Evans wrote: > On Fri, 12 Dec 2008, Randall Stewart wrote: > >> Here is an updated patch it: >> >> 1) Fixes style9 issues (I hope.. I went back to vi and tried >> tabs :-0.. sigh one of >> these doys I will figure out why my .emacs settings just never cut >> it :-() > > Fraid not. > > % Index: netinet/udp_usrreq.c > % =================================================================== > % --- netinet/udp_usrreq.c (revision 185928) > % +++ netinet/udp_usrreq.c (working copy) > % @@ -488,10 +488,25 @@ > % struct mbuf *n; > % % n = m_copy(m, 0, M_COPYALL); > % - if (n != NULL) > % - udp_append(last, ip, n, iphlen + > % - sizeof(struct udphdr), &udp_in); > % - INP_RUNLOCK(last); > % + > > Extra blank line. > > % + if (last->inp_ppcb == NULL) { > % + if (n != NULL) > % + udp_append(last, ip, n, iphlen + > % + sizeof(struct udphdr), &udp_in); > > Line too long. > > % + INP_RUNLOCK(last); > % + } else { > % + /* Engage the tunneling protocol > > "/*" not on a line by itself. > > % + * we will have to leave the info_lock > % + * up, since we are hunting through % + * multiple UDP > inp's hope we don't break :-( > > Line too long. Defeats reasonable indentation of the rest of the > comment. > > Missing punctuation after ":-(". > > % + */ > % + udp_tunnel_function_t tunnel_func; > > Nested declaration. > > % + > % + INP_RUNLOCK(last); > % + tunnel_func = (udp_tunnel_function_t)last->inp_ppcb; > > Line too long. > > Typedef names involving functions normally use `func_t', not > `function_t'. > This is useful for reducing verboseness and resulting long lines but > wouldn't > fix the long line in the above, since everything in it is too verbose > (in the middle there is an ugly cast whose only effect can be to > avoid a > warning ...). > > % @@ -516,10 +531,25 @@ > % V_udpstat.udps_noportbcast++; > % goto badheadlocked; > % } > % - udp_append(last, ip, m, iphlen + sizeof(struct udphdr), > % - &udp_in); > % - INP_RUNLOCK(last); > % - INP_INFO_RUNLOCK(&V_udbinfo); > % + if (last->inp_ppcb == NULL) { > % + udp_append(last, ip, m, iphlen + sizeof(struct udphdr), > % + &udp_in); > % + INP_RUNLOCK(last); > % + INP_INFO_RUNLOCK(&V_udbinfo); > % + } else { > % + /* Engage the tunneling protocol > > "/*" not on a line by itself. No comment on further instances of this > > % + * we must make sure all locks > % + * are released when we call the > % + * tunneling protocol. > % + */ > > Long lines are ones longer than 80 characters. Splitting at 48 > characters > as in the above is not normal. > > % + udp_tunnel_function_t tunnel_func; > > Nested declaration. > > % @@ -563,6 +593,18 @@ > % INP_RUNLOCK(inp); > % goto badunlocked; > % } > % + if (inp->inp_ppcb) { > % + /* Engage the tunneling protocol > % + * we must make sure all locks > % + * are released when we call the > % + * tunneling protocol. > % + */ > > More formatting for 48-column terminals. > > % + udp_tunnel_function_t tunnel_func; > > Nested declaration. > > Missing blank line after declarations. > > % ... > % +int > % +udp_set_kernel_tunneling(struct socket *so, udp_tunnel_function_t > f) > % +{ > % + struct inpcb *inp; > % + inp = (struct inpcb *)so->so_pcb; > > Initialization in declaration. Not too bad here, but you don't do > it for > similar tunnel function pointer conversions. > > % + > % + if (so->so_type != SOCK_DGRAM) { > % + /* Not UDP socket... sorry */ > > Missing punctuation. > > % Index: netinet/udp_var.h > % =================================================================== > % --- netinet/udp_var.h (revision 185928) > % +++ netinet/udp_var.h (working copy) > % @@ -107,6 +107,10 @@ > % void udp_input(struct mbuf *, int); > % struct inpcb *udp_notify(struct inpcb *inp, int errno); > % int udp_shutdown(struct socket *so); > % + > % + > % +typedef void(*udp_tunnel_function_t)(struct mbuf *, int off); > % +int udp_set_kernel_tunneling(struct socket *so, > udp_tunnel_function_t f); > > I noticed this first in the initial patch. It has a style bug > density of > about 5 per line !-(: > > - 2 extra blank lines > - typedef in the middle of (non-pointer non-typedef) prototypes > - unsorted prototypes (at least the old 3 visible on the above are > sorted) > - new prototypes not indented normally though visible old ones all are > - some parameters have names, some not. > - style(9) says to always have names in the kernel, but this rule > is usually > violated > - the first of the 3 visible old prototypes violates this rule > - the first of the new prototypes violates this rule for the first > of its > 2 parameters only > > % #endif > % % #endif > % Index: netinet6/udp6_usrreq.c > % =================================================================== > % --- netinet6/udp6_usrreq.c (revision 185928) > % +++ netinet6/udp6_usrreq.c (working copy) > % @@ -286,9 +286,21 @@ > % struct mbuf *n; > % % if ((n = m_copy(m, 0, M_COPYALL)) != NULL) { > % - INP_RLOCK(last); > % - udp6_append(last, n, off, &fromsa); > % - INP_RUNLOCK(last); > % + if (last->inp_ppcb) { > % + /* Engage the tunneling protocol > % + * we will have to leave the info_lock > % + * up, since we are hunting through % + * multiple > UDP inp's hope we don't break :-( > % + */ > > Lines too long -- now formatting for 94-column terminals instead of > 48-column ones using cut&pasted&indent. > > Missing punctuation (cut&paste). > > % + udp_tunnel_function_t tunnel_func; > > Nested declaration. > > Line too long. > > Missing blank line after declarations. > > % + tunnel_func = (udp_tunnel_function_t)last->inp_ppcb; > > Line too long. > > % + INP_RUNLOCK(last); > % + tunnel_func(m, off); > % + } else { > % + INP_RLOCK(last); > % + udp6_append(last, n, off, &fromsa); > > Line too long. > > % @@ -317,6 +329,19 @@ > % } > % INP_RLOCK(last); > % INP_INFO_RUNLOCK(&V_udbinfo); > % + if (last->inp_ppcb) { > % + /* Engage the tunneling protocol > % + * we must make sure all locks > % + * are released when we call the > % + * tunneling protocol. > % + */ > > Now formatting for 56-column terminals. > > % @@ -354,6 +379,18 @@ > % } > % INP_RLOCK(inp); > % INP_INFO_RUNLOCK(&V_udbinfo); > % + if (inp->inp_ppcb) { > % + /* Engage the tunneling protocol > % + * we must make sure all locks > % + * are released when we call the > % + * tunneling protocol. > % + */ > > Back to formatting for 48-column terminals. > > There are 6 near-copies of this comment. > > % + udp_tunnel_function_t tunnel_func; > > Nested declaration. > > Missing blank line after declaration. > > % + tunnel_func = (udp_tunnel_function_t)inp->inp_ppcb; > % + INP_RUNLOCK(inp); > % + tunnel_func(m, off); > > Do you have to hold the lock to access inp->inp_ppcb? Otherwise you > can > avoid all the nested declarations and just use the pointer once. If > the > lock is needed, then what happens if the pointer changes after it is > accessed? > > % + return (IPPROTO_DONE); > % + } > % udp6_append(inp, m, off, &fromsa); > % INP_RUNLOCK(inp); > % return (IPPROTO_DONE); > > Bruce > ------------------------------ Randall Stewart 803-317-4952 (cell) 803-345-0391(direct) --Apple-Mail-90--449110523--