From owner-freebsd-net@FreeBSD.ORG Wed Feb 4 14:45:49 2015 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 865ADFF0 for ; Wed, 4 Feb 2015 14:45:49 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [IPv6:2001:4f8:3:ffe0:406a:0:50:2]) (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 5C94BFF2 for ; Wed, 4 Feb 2015 14:45:49 +0000 (UTC) Received: from phabric-backend.isc.freebsd.org (phabric-backend.isc.freebsd.org [127.0.1.5]) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9) with ESMTP id t14EjnTC096346 for ; Wed, 4 Feb 2015 14:45:49 GMT (envelope-from root@phabric-backend.isc.freebsd.org) Received: (from root@localhost) by phabric-backend.isc.freebsd.org (8.14.9/8.14.9/Submit) id t14Ejn9F096345; Wed, 4 Feb 2015 14:45:49 GMT (envelope-from root) Date: Wed, 4 Feb 2015 14:45:49 +0000 To: freebsd-net@freebsd.org From: "rrs (Randall Stewart)" Subject: [Differential] [Commented On] D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate thecallout code (and potentially for use by other tests). Message-ID: <1a5308ff90013979069e811ef5a52a74@localhost.localdomain> X-Priority: 3 Thread-Topic: D1711: Changes to the callout code to restore active semantics and also add a test-framework and test to validate the callout code (and potentially for use by other tests). X-Herald-Rules: none X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: X-Phabricator-Cc: In-Reply-To: References: Thread-Index: Y2JjMTcyODJkYzgxM2NkZDFjY2RhOGRmMTlkIFTSMJ0= X-Phabricator-Sent-This-Message: Yes X-Mail-Transport-Agent: MetaMTA X-Auto-Response-Suppress: All X-Phabricator-Mail-Tags: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Feb 2015 14:45:49 -0000 rrs added a comment. Julian: The simple fix is to change the callout_init_rw -> callout_init(c, 1); That makes it so the callout_stop() in this instance would return 0, not 1. Since the callout could not be stopped. This would then cause the reference count to drop to 1 (it was 2 the original and the callout). It would not free here but when the callout runs it would. Now that all being said, I have put that in some netflix code and will test it.. but there is something strange about this whole lle* path that I don't yet grok. The arp code and nd6 code carefully do reference counting. There are comments in in.c and in6.c that mention that the arp/nd6 code are the callers.. thats fine.. However *if* either callout goes off in the then the in/in6 callout runs which does: UNLOCK(lle) free(la) No reference counting, no nothing.. I don't grok how all this is connected and need to study it more but it just feels wrong. If its doing reference counting I would have thought the callout would have done so too. I need to take a bigger and closer view at the lle/arp code and how it is all supposed to inter-work.. it appears to me broken, but that could be just my mis-understanding of the code.. I should get home tomorrow and besides fixing my mail-server I wills see what Robert, George and Kirk have to say about the lle code paths in there new book, and then dig in and try to puzzle it out. I *think* the simple change callout_init_rw -> callout_init(c, 1) will fix this and of course we have to remove the unlock in the actual callout since the callout code won't be locking it any longer... but I am wondering if there is not a deeper issue with this code. Doing the above will not change the behavior and will get the right results on the flush, but this use/not-use references seems wrong so there may be deeper bugs in the lle code! R REVISION DETAIL https://reviews.freebsd.org/D1711 To: rrs, gnn, rwatson, lstewart, jhb, kostikbel, sbruno, imp, adrian, hselasky Cc: julian, hiren, jhb, kostikbel, emaste, delphij, neel, erj, freebsd-net