From owner-svn-src-head@freebsd.org Sat Aug 20 16:49:24 2016 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EB599BC0EEE; Sat, 20 Aug 2016 16:49:24 +0000 (UTC) (envelope-from bms@fastmail.net) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id AD1C7197A; Sat, 20 Aug 2016 16:49:24 +0000 (UTC) (envelope-from bms@fastmail.net) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id 5EE752025C; Sat, 20 Aug 2016 12:49:23 -0400 (EDT) Received: from frontend1 ([10.202.2.160]) by compute2.internal (MEProxy); Sat, 20 Aug 2016 12:49:23 -0400 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=fastmail.net; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-sasl-enc :x-sasl-enc; s=mesmtp; bh=HD4EHIlt6M4xzmvuJDV950U3/Mw=; b=EJ0msq 4yGWrPS5BJ/0Gxd0QT/ifUtfOCY7xxDAKIrDz8h4E3argkAQLbh+H921tqVhTPpX d1RBSjBLaNdX+/aHLZLJE38H8QadqEJ/edNUG4gFjvI5M0oeZ80v7SJGbTlcB+RY mdzRIlzsaOyU3Ao1TXzC9ualTYaDjgeitp4cI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=HD4EHIlt6M4xzmv uJDV950U3/Mw=; b=QCub3SnybNF/WP7abOJGJ4Rbjz2qnrDx/Tl6lt/K7s1cDo+ fFgp4zXRujBTIKZoY6fqQG9CbO3mgM6w3AOVLMLc0CIcrZ+AhBhpQJvxDiAQHjnK KuQ6/hAfQ5NXOPsAsa+PlFaDnoaJ6aRu+uI9NJAy8elf9q2SsOkGySvjg2x8= X-Sasl-enc: 4VKkddHAUxSSovpPlkHKDtLaq7PR3qKCTOlsY1n9GefT 1471711763 Received: from pion.local (cpc96954-walt26-2-0-cust843.13-2.cable.virginm.net [82.31.91.76]) by mail.messagingengine.com (Postfix) with ESMTPA id 6AD24F285F; Sat, 20 Aug 2016 12:49:22 -0400 (EDT) Subject: Re: svn commit: r304436 - in head: . sys/netinet To: Ryan Stone References: <201608182259.u7IMx5oW002018@repo.freebsd.org> <4fbc2e1d-3a62-5963-83d5-f9c931503e51@fastmail.net> <3806700d-ed27-7915-4818-c2d64f7b806d@fastmail.net> <6f4449f2-d145-8b49-c3f0-433e8ff4d2a2@fastmail.net> Cc: Ryan Stone , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" , Adrian Chadd From: Bruce Simpson Message-ID: <990e003f-6180-b016-3b5f-6bdf579d073f@fastmail.net> Date: Sat, 20 Aug 2016 17:49:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 20 Aug 2016 16:49:25 -0000 On 20/08/16 17:36, Ryan Stone wrote: > + adrian@, who prompted me to look at UDP in the first place > > I'm really not sure what my next step should be. I'm willing to revert > r304436, but I really don't want to revert r304437 because we've seen > crashes in the real world due to the missing locking. Unfortunately, > reverting r304436 would mean that every UDP packet would incur the > overhead of an additional rlock/runlock call, which is what I've been > trying to avoid. I don't see a particularly good path forward. Your changes have the same motivations as my historical change to move group-level IP multicast lookups (and locking) out of the output path. Source-specific multicast (SSM) requires support for reception filters on individual sockets, so I moved those multicast-specific input checks into the socket layer. > - The if_addr_lock would appear to be an excellent candidate to be > converted into an rmlock, but unfortunately we made the mistake of > exposing the lock through the ifnet KPI. Fixing that would require > rototilling every single Ethernet/WiFi/etc driver in the tree. Oops... > - Providing a mechanism for ip_input() to signal to udp_input() that the > packet was addressed to an L3 broadcast address would require > rototilling the pr_input interface, and I'd have to carefully ensure > that if anything might interpose itself between the two layers (IPSec?) > that the flag would have to be passed through correctly. > > - mbuf flags are far too precious to allocate a new one for such a > narrow use-case Agree. Then it may be that slipping the definition of M_BCAST to mean 'And IP identified that this is network-layer broadcast' is the most expedient solution. A quick look around suggests you may be able to get away with it. You'd essentially be widening the definition of the existing M_BCAST flag. Given the ultimate consumer of the mbuf in the case you are addressing in the code (bad pun) is udp_input(), that may be fine. (We had to do something similar for ILNPv6, because of how IPv6 input works, so it allocates an unused bit from the IPv6 flow label.) But it has to be qualified very carefully. If L2 is re-entered from IP (e.g. a netgraph IP-level hook), it may have unexpected side-effects. I have not given this the KScope treatment, just a quick fxr.watson.org.