From patchwork Thu Jun 2 00:17:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vishwanath Pai X-Patchwork-Id: 628914 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rKnqy0Khsz9t3r for ; Thu, 2 Jun 2016 10:18:06 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=akamai.com header.i=@akamai.com header.b=S+/7d4pB; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752007AbcFBASD (ORCPT ); Wed, 1 Jun 2016 20:18:03 -0400 Received: from prod-mail-xrelay07.akamai.com ([23.79.238.175]:25554 "EHLO prod-mail-xrelay07.akamai.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751935AbcFBASB (ORCPT ); Wed, 1 Jun 2016 20:18:01 -0400 Received: from prod-mail-xrelay07.akamai.com (localhost.localdomain [127.0.0.1]) by postfix.imss70 (Postfix) with ESMTP id 05BC043357E; Thu, 2 Jun 2016 00:18:00 +0000 (GMT) Received: from prod-mail-relay11.akamai.com (prod-mail-relay11.akamai.com [172.27.118.250]) by prod-mail-xrelay07.akamai.com (Postfix) with ESMTP id D9925433414; Thu, 2 Jun 2016 00:17:59 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akamai.com; s=a1; t=1464826679; bh=r70WikG5UQ22V2YaJD0YoypVZEi/fjIOSDQGEwQCGv8=; l=4406; h=Date:From:To:Cc:From; b=S+/7d4pBqr/Zh8kUJnOSr/SrdSA1Vl8T3UFs3QfCxnhOFOYqk9VZc/iK6bkebxqNz c8MV59R+Klx3aKk1oz3PeV8qg4aNM8j3ZuV4WBuQPRBhGhTmQ2xJZPIcM+HQn58HaR I9jkvdP2maqaghkSW2/xY7b162FLb1RDRncJXgr0= Received: from bos-lpqrs.kendall.corp.akamai.com (bos-lpqrs.kendall.corp.akamai.com [172.28.13.81]) by prod-mail-relay11.akamai.com (Postfix) with ESMTP id D5C411FC88; Thu, 2 Jun 2016 00:17:59 +0000 (GMT) Received: from vpai by bos-lpqrs.kendall.corp.akamai.com with local (Exim 4.82) (envelope-from ) id 1b8GKl-0000tJ-QU; Wed, 01 Jun 2016 20:17:59 -0400 Date: Wed, 1 Jun 2016 20:17:59 -0400 From: Vishwanath Pai To: pablo@netfilter.org, kaber@trash.net, kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org Cc: coreteam@netfilter.org, johunt@akamai.com, netdev@vger.kernel.org, pai.vishwain@gmail.com Subject: [PATCH iptables 3/3] libxt_hashlimit: iptables-restore does not work as expected with xt_hashlimit Message-ID: <20160602001759.GF1644@akamai.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org libxt_hashlimit: iptables-restore does not work as expected with xt_hashlimit Add the following iptables rule. $ iptables -A INPUT -m hashlimit --hashlimit-above 200/sec \ --hashlimit-burst 5 --hashlimit-mode srcip --hashlimit-name hashlimit1 \ --hashlimit-htable-expire 30000 -j DROP $ iptables-save > save.txt Edit save.txt and change the value of --hashlimit-above to 300: -A INPUT -m hashlimit --hashlimit-above 300/sec --hashlimit-burst 5 \ --hashlimit-mode srcip --hashlimit-name hashlimit2 \ --hashlimit-htable-expire 30000 -j DROP Now restore save.txt $ iptables-restore < save.txt Now userspace thinks that the value of --hashlimit-above is 300 but it is actually 200 in the kernel. This happens because when we add multiple hash-limit rules with the same name they will share the same hashtable internally. The kernel module tries to re-use the old hashtable without updating the values. There are multiple problems here: 1) We can add two iptables rules with the same name, but kernel does not handle this well, one procfs file cannot work with two rules 2) If the second rule has no effect because the hashtable has values from rule 1 3) hashtable-restore does not work (as described above) To fix this I have made the following design change: 1) If a second rule is added with the same name as an existing rule, append a number when we create the procfs, for example hashlimit_1, hashlimit_2 etc 2) Two rules will not share the same hashtable unless they are similar in every possible way 3) This behavior has to be forced with a new userspace flag: --hashlimit-ehanced-procfs, if this flag is not passed we default to the old behavior. This is to make sure we do not break existing scripts that rely on the existing behavior. Signed-off-by: Vishwanath Pai diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c index 4193464..ac67875 100644 --- a/extensions/libxt_hashlimit.c +++ b/extensions/libxt_hashlimit.c @@ -67,6 +67,7 @@ enum { O_HTABLE_MAX, O_HTABLE_GCINT, O_HTABLE_EXPIRE, + O_PROCFS, F_BURST = 1 << O_BURST, F_UPTO = 1 << O_UPTO, F_ABOVE = 1 << O_ABOVE, @@ -177,6 +178,7 @@ static const struct xt_option_entry hashlimit_mt_opts[] = { {.name = "hashlimit-mode", .id = O_MODE, .type = XTTYPE_STRING}, {.name = "hashlimit-name", .id = O_NAME, .type = XTTYPE_STRING, .flags = XTOPT_MAND | XTOPT_PUT, XTOPT_POINTER(s, name), .min = 1}, + {.name = "hashlimit-enhanced-procfs", .id = O_PROCFS, .type = XTTYPE_NONE}, XTOPT_TABLEEND, }; #undef s @@ -521,6 +523,9 @@ static void hashlimit_mt_parse(struct xt_option_call *cb) case O_DSTMASK: info->cfg.dstmask = cb->val.hlen; break; + case O_PROCFS: + info->cfg.flags |= XT_HASHLIMIT_FLAG_PROCFS; + break; } } @@ -856,6 +861,9 @@ hashlimit_mt_save(const struct hashlimit_cfg2 *cfg, const char* name, unsigned i printf(" --hashlimit-srcmask %u", cfg->srcmask); if (cfg->dstmask != dmask) printf(" --hashlimit-dstmask %u", cfg->dstmask); + + if ((revision == 2) && (cfg->flags & XT_HASHLIMIT_FLAG_PROCFS) ) + printf(" --hashlimit-enhanced-procfs"); } static void diff --git a/extensions/libxt_hashlimit.man b/extensions/libxt_hashlimit.man index 6aac3f2..0434f03 100644 --- a/extensions/libxt_hashlimit.man +++ b/extensions/libxt_hashlimit.man @@ -40,6 +40,9 @@ Like \-\-hashlimit\-srcmask, but for destination addresses. \fB\-\-hashlimit\-name\fP \fIfoo\fP The name for the /proc/net/ipt_hashlimit/foo entry. .TP +\fB\-\-hashlimit\-enhanced\-procfs\fP +Append _number to the procfs file when multiple rules with the same name exist +.TP \fB\-\-hashlimit\-htable\-size\fP \fIbuckets\fP The number of buckets of the hash table .TP diff --git a/include/linux/netfilter/xt_hashlimit.h b/include/linux/netfilter/xt_hashlimit.h index e493fc1..2954381 100644 --- a/include/linux/netfilter/xt_hashlimit.h +++ b/include/linux/netfilter/xt_hashlimit.h @@ -16,6 +16,10 @@ struct xt_hashlimit_htable; enum { + XT_HASHLIMIT_FLAG_PROCFS = 1 +}; + +enum { XT_HASHLIMIT_HASH_DIP = 1 << 0, XT_HASHLIMIT_HASH_DPT = 1 << 1, XT_HASHLIMIT_HASH_SIP = 1 << 2, @@ -74,6 +78,9 @@ struct hashlimit_cfg2 { __u32 expire; /* when do entries expire? */ __u8 srcmask, dstmask; + + __u8 procfs_suffix; + __u32 flags; }; struct xt_hashlimit_mtinfo1 {