From owner-freebsd-net@FreeBSD.ORG Thu Jan 2 17:46:04 2014 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A9BB4BFA for ; Thu, 2 Jan 2014 17:46:04 +0000 (UTC) Received: from mail-qe0-x236.google.com (mail-qe0-x236.google.com [IPv6:2607:f8b0:400d:c02::236]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 682F51005 for ; Thu, 2 Jan 2014 17:46:04 +0000 (UTC) Received: by mail-qe0-f54.google.com with SMTP id cy11so14666491qeb.13 for ; Thu, 02 Jan 2014 09:46:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=UQqRzlgDKxzn5Wnj3l0L4JqFlou/RPMXL1DuhVHbq5A=; b=MyKOP6DHggxQ9IjdFg3GbrQIuLF2OZ8uSiHBt22m+sgVNxeAgIVs7qP3cbB7JGWeVd vKlyujfAtzptJjVBqq/XUXKuzWimBbVm8Cvx5D1EjBEJC4VVnEwiDNi4muc59BsyaH/k 82JPcXSFonjl+l5ca6bWNPubj+q7gAu4VqFR8onF4PEkW58B29D2C9rKPgYUjKLqOisQ Fv8MTy/gSsY3tFjgzM3Dh3A8uuHvDmBe4irkMZ2r/R2QCKKpJnT25LpBwVMtttaHq+Lt iDFYB2yDxcH/O2oLSFBDdGFvCgNuwR/OOMUAcQhzuI7XWzKoC2+4rsvYsP+fontInHaL sdvQ== MIME-Version: 1.0 X-Received: by 10.224.14.132 with SMTP id g4mr34232324qaa.26.1388684763573; Thu, 02 Jan 2014 09:46:03 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.224.52.8 with HTTP; Thu, 2 Jan 2014 09:46:03 -0800 (PST) In-Reply-To: <52C5A020.9010404@vangyzen.net> References: <52C5A020.9010404@vangyzen.net> Date: Thu, 2 Jan 2014 09:46:03 -0800 X-Google-Sender-Auth: r86__bW_PpGZaIqxsuWlK3D8MbA Message-ID: Subject: Re: ipv6 lock contention with parallel socket io From: Adrian Chadd To: Eric van Gyzen Content-Type: text/plain; charset=ISO-8859-1 Cc: FreeBSD Net X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jan 2014 17:46:04 -0000 On 2 January 2014 09:21, Eric van Gyzen wrote: > On 12/31/2013 11:53, Adrian Chadd wrote: >> On 30 December 2013 23:35, Adrian Chadd wrote: >>> Hi, >>> >>> I've noticed a bunch of lock contention occurs when doing highly >>> parallel (> 4096 sockets) TCP IPv6 traffic. >>> >>> The contention is here: >>> >> [snip] >> >>> .. it's the IF_ADATA lock surrounding the lla_lookup() calls. >>> >>> Now: >>> >>> * is there any reason this isn't an rmlock? >>> * the instance early on in nd6_output_lle() isn't taking the read >>> lock, it's taking the full lock. Is there any reason for this? >>> >>> I don't have much experience or time to spend on optimising the ipv6 >>> code to scale better but this seems like one of those things that'll >>> bite us in the ass as the amount of ipv6 deployed increases. >>> >>> Does anyone have any ideas/suggestions on how we could improve things? >> This improves things quite a bit - from 1.9gbyte/sec @ 100% cpu usage >> to 2.7gbyte/sec @ 85% CPU usage. It's not perfect - the lock >> contention is still there - but it's much less of an issue now. >> >> Are there any issues with it? >> >> Index: sys/netinet6/nd6.c >> =================================================================== >> --- sys/netinet6/nd6.c (revision 259475) >> +++ sys/netinet6/nd6.c (working copy) >> @@ -1891,9 +1891,9 @@ >> flags = ((m != NULL) || (lle != NULL)) ? LLE_EXCLUSIVE : 0; >> if (ln == NULL) { >> retry: >> - IF_AFDATA_LOCK(ifp); >> + IF_AFDATA_RLOCK(ifp); >> ln = lla_lookup(LLTABLE6(ifp), flags, (struct sockaddr *)dst); >> - IF_AFDATA_UNLOCK(ifp); >> + IF_AFDATA_RUNLOCK(ifp); >> if ((ln == NULL) && nd6_is_addr_neighbor(dst, ifp)) { >> /* >> * Since nd6_is_addr_neighbor() internally >> calls nd6_lookup(), > > This change looks safe, since flags can only contain LLE_EXCLUSIVE. > However, the assertion in in6_lltable_lookup() seems insufficient. It > asserts any lock on afdata. I think it should also assert a write-lock > in the LLE_CREATE case. > > Granted, this is not strictly related to your change. Would you mind doing up a quick patch to demonstrate? I've heard mumblings at work that ipv6 panics if you change the routing tables during active traffic so if we're missing lock assertions I'd like to add them now and try to pick up the problem as it happens in testing. Thanks! -adrian