From owner-freebsd-current@FreeBSD.ORG Sat Jan 8 19:10:07 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B7A20106571E; Sat, 8 Jan 2011 19:10:07 +0000 (UTC) (envelope-from bzeeb-lists@lists.zabbadoz.net) Received: from mail.cksoft.de (mail.cksoft.de [IPv6:2001:4068:10::3]) by mx1.freebsd.org (Postfix) with ESMTP id 5ADAF8FC0C; Sat, 8 Jan 2011 19:10:07 +0000 (UTC) Received: from localhost (amavis.fra.cksoft.de [192.168.74.71]) by mail.cksoft.de (Postfix) with ESMTP id B2D5841C707; Sat, 8 Jan 2011 20:10:06 +0100 (CET) X-Virus-Scanned: amavisd-new at cksoft.de Received: from mail.cksoft.de ([192.168.74.103]) by localhost (amavis.fra.cksoft.de [192.168.74.71]) (amavisd-new, port 10024) with ESMTP id MKQXYujFXA8C; Sat, 8 Jan 2011 20:10:06 +0100 (CET) Received: by mail.cksoft.de (Postfix, from userid 66) id E0EA641C6FC; Sat, 8 Jan 2011 20:10:05 +0100 (CET) Received: from maildrop.int.zabbadoz.net (maildrop.int.zabbadoz.net [10.111.66.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.int.zabbadoz.net (Postfix) with ESMTP id 696DE4448F3; Sat, 8 Jan 2011 19:07:04 +0000 (UTC) Date: Sat, 8 Jan 2011 19:07:04 +0000 (UTC) From: "Bjoern A. Zeeb" X-X-Sender: bz@maildrop.int.zabbadoz.net To: Daniel Eischen In-Reply-To: Message-ID: <20110108185914.D14966@maildrop.int.zabbadoz.net> References: <20110107103837.E14966@maildrop.int.zabbadoz.net> X-OpenPGP-Key: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Randall Stewart , freebsd-current@freebsd.org Subject: Re: UDP checksum broken, -head and releng_8 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Jan 2011 19:10:08 -0000 On Sat, 8 Jan 2011, Daniel Eischen wrote: >>>>> When sending multicast packets to a socket that is _not_ >>>>> bound to the multicast address, this generates bad UDP >>>>> checksums. This use to work and was broke sometime between >>>>> the middle of October and late December as far as I can >>>>> tell. >>>> >>>> My very best guess would be: r215110 >>> >>> It doesn't look very harmful, but I'll try backing it out. >> >> Backing this out seems to fix it. I'll have to test it >> more after I get some sleep ;-) > > I've attached what may be a proper patch. Please review. > I'd like to get this fixed in releng_8 too. I would remove the comment from the MC good path about the in_pcbladdr() error but just change the description at the top s,use,prefer, to be more exact. The other question I am not sure what shoud happen is, in the case in_pcbladdr() returns a source address but a given one via MC option/ifp does not find an address (in case outgoing itnerface could be different)? That was never considered in the past. It's probably easiest understood with the slightly modified version of the patch. Otherwise I think it would match both the historic behaviour again and keep the fix for r215110 (that rev. should be mentioned in the commit message). So apart from the 1 line comment change (ignoring my XXX-BZ completely for the moment and this fix) this looks good. /bz PS: I doubt you can make 8.2-R anymore. Index: in_pcb.c =================================================================== --- in_pcb.c (revision 216952) +++ in_pcb.c (working copy) @@ -874,9 +874,10 @@ in_pcbconnect_setup(struct inpcb *inp, struct sock } } if (laddr.s_addr == INADDR_ANY) { + error = in_pcbladdr(inp, &faddr, &laddr, cred); /* * If the destination address is multicast and an outgoing - * interface has been set as a multicast option, use the + * interface has been set as a multicast option, prefer the * address of that interface as our source address. */ if (IN_MULTICAST(ntohl(faddr.s_addr)) && @@ -893,16 +894,23 @@ in_pcbconnect_setup(struct inpcb *inp, struct sock break; if (ia == NULL) { IN_IFADDR_RUNLOCK(); - return (EADDRNOTAVAIL); + /* + * XXX-BZ should we use a possible + * source address from in_pcbladdr() + * and only overwrite the error if + * there was one? + * if (error) + */ + error = EADDRNOTAVAIL; + } else { + laddr = ia->ia_addr.sin_addr; + IN_IFADDR_RUNLOCK(); + error = 0; } - laddr = ia->ia_addr.sin_addr; - IN_IFADDR_RUNLOCK(); } - } else { - error = in_pcbladdr(inp, &faddr, &laddr, cred); - if (error) - return (error); } + if (error) + return (error); } oinp = in_pcblookup_hash(inp->inp_pcbinfo, faddr, fport, laddr, lport, 0, NULL); -- Bjoern A. Zeeb You have to have visions! Going to jail sucks -- All my daemons like it! http://www.freebsd.org/doc/en_US.ISO8859-1/books/handbook/jails.html