From patchwork Wed May 22 22:36:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phil Oester X-Patchwork-Id: 245778 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id C163E2C00A5 for ; Thu, 23 May 2013 08:36:39 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756992Ab3EVWgh (ORCPT ); Wed, 22 May 2013 18:36:37 -0400 Received: from mail-ob0-f177.google.com ([209.85.214.177]:62316 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756967Ab3EVWgg (ORCPT ); Wed, 22 May 2013 18:36:36 -0400 Received: by mail-ob0-f177.google.com with SMTP id wn6so3155998obc.36 for ; Wed, 22 May 2013 15:36:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:user-agent:x-gm-message-state; bh=f+iTo2l8VT1r6pltrCtv1ahQdMhVuaI+MzNFHxD/0mY=; b=EvAatDC5NDHvAKOqevOHZFl2zwdod2UaTb3lFiPnbZ9j35Yy8zVoBR1mRg11+FGfKW N/RYLQyIJoqUIGjm4Q4OktR42CVufElM8F/ChLm9pqq0E7QddYfKmbrftVobuyrH0rgW 6qRkPXgJiRHnkIfkauPMy5fnMrethjoX72e1lBsFxcU8k6R71AaaPSZ5JX+TNYkrJ6oi GK+Ag6WyRRojLNmC0HUvpPHrxyhc9YijlQ+DdHImU4oUM6olaho3e6CuM6Yfwt1vOgH/ /atzvoJpUV2dt207QRe1xHAAkhHj093O2+bUwUAYpwtEMRjmciznv4xTbfSjRgrypCEI Ss7A== X-Received: by 10.182.237.50 with SMTP id uz18mr6525976obc.51.1369262196217; Wed, 22 May 2013 15:36:36 -0700 (PDT) Received: from gmail.com (wsip-24-249-192-237.pn.at.cox.net. [24.249.192.237]) by mx.google.com with ESMTPSA id h4sm9921960oel.2.2013.05.22.15.36.34 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 22 May 2013 15:36:35 -0700 (PDT) Date: Wed, 22 May 2013 18:36:34 -0400 From: Phil Oester To: netfilter-devel@vger.kernel.org Cc: pablo@netfilter.org, kaber@trash.net Subject: [PATCH v2] xtables: Add locking to prevent concurrent instances Message-ID: <20130522223455.GA4665@gmail.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQnCO3LuMfXMriENHvPLGbzmfxFepuXlGKQp1h7/Wcw7T34Bjv7tE1K98EVUPlS1zbZX0vDk Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org There have been numerous complaints and bug reports over the years when admins attempt to run more than one instance of iptables simultaneously. Currently open bug reports which are related: 325: Parallel execution of the iptables is impossible 758: Retry iptables command on transient failure 764: Doing -Z twice in parallel breaks counters 822: iptables shows negative or other bad packet/byte counts As Patrick notes in 325: "Since this has been a problem people keep running into, I'd suggest to simply add some locking to iptables to catch the most common case." I started looking into alternatives to add locking, and of course the most common/obvious solution is to use a pidfile. But this has various downsides, such as if the application is terminated abnormally and the pidfile isn't cleaned up. And this also requires a writable filesystem. Using a UNIX domain socket file (e.g. in /var/run) has similar issues. Starting in 2.2, Linux added support for abstract sockets. These sockets require no filesystem, and automatically disappear once the application terminates. This is the locking solution I chose to implement in xtables-multi. As an added bonus, since each network namespace has its own socket pool, an iptables instance running in one namespace will not lock out an iptables instance running in another namespace. A filesystem approach would have to recognize and handle multiple network namespaces. Changes from v1: - Addressed Patrick's comments - locking attempts will be made indefinitely until successful. Phil Signed-off-by: Phil Oester diff --git a/iptables/xtables-multi.c b/iptables/xtables-multi.c index 8014d5f..5a57375 100644 --- a/iptables/xtables-multi.c +++ b/iptables/xtables-multi.c @@ -1,8 +1,12 @@ #include #include #include +#include +#include +#include #include "xshared.h" +#include "xtables.h" #include "xtables-multi.h" #ifdef ENABLE_IPV4 @@ -35,7 +39,31 @@ static const struct subcommand multi_subcommands[] = { {NULL}, }; +#define XTMSOCKET_NAME "xtables_multi" +#define XTMSOCKET_LEN 14 + int main(int argc, char **argv) { + int i = 0, ret, xtm_socket; + struct sockaddr_un xtm_addr; + + memset(&xtm_addr, 0, sizeof(xtm_addr)); + xtm_addr.sun_family = AF_UNIX; + strcpy(xtm_addr.sun_path+1, XTMSOCKET_NAME); + xtm_socket = socket(AF_UNIX, SOCK_STREAM, 0); + /* If we can't even create a socket, just revert to prior (lockless) behavior */ + if (xtm_socket < 0) + return subcmd_main(argc, argv, multi_subcommands); + + while (1) { + ret = bind(xtm_socket, (struct sockaddr*)&xtm_addr, + offsetof(struct sockaddr_un, sun_path)+XTMSOCKET_LEN); + if (ret == 0) + break; + if (++i % 5 == 0) + fprintf(stderr, "Waiting for lock, standby...\n"); + sleep(1); + } + return subcmd_main(argc, argv, multi_subcommands); }