From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pablo Neira Ayuso Subject: Re: [PATCH net-next] netfilter: xt_quota: fix the behavior of xt_quota module Date: Tue, 2 Oct 2018 12:51:25 +0200 Message-ID: <20181002105125.uv7mcitvaalpjueo@salvia> References: <1538443388-6881-1-git-send-email-chenbofeng.kernel@gmail.com> <1538443388-6881-3-git-send-email-chenbofeng.kernel@gmail.com> <20181002075903.3wpgej3j6dttbqck@salvia> <20181002101119.tyljwzqpdj7qoe6f@salvia> <20181002101556.lpvn4kz7xgv2at3f@salvia> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Chenbo Feng , Linux NetDev , netfilter-devel@vger.kernel.org, kernel-team@android.com, Lorenzo Colitti , Chenbo Feng To: Maciej =?utf-8?Q?=C5=BBenczykowski?= Return-path: Received: from mail.us.es ([193.147.175.20]:33026 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727160AbeJBReK (ORCPT ); Tue, 2 Oct 2018 13:34:10 -0400 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id E50DB1A098D for ; Tue, 2 Oct 2018 12:51:27 +0200 (CEST) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id D37F0DA87A for ; Tue, 2 Oct 2018 12:51:27 +0200 (CEST) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Tue, Oct 02, 2018 at 03:38:24AM -0700, Maciej Żenczykowski wrote: > > Well, you will need a kernel + userspace update anyway, right? > > It's true you need new iptables userspace to *see* during dump and/or > manually *set* during restore the remain counter. > > However, (and I believe Chenbo tested this) just a new kernel is > enough to fix the problem of modifications within the table resetting > the counter. > This is because the data gets copied out of kernel and back into > kernel by old iptables without any further modifications. > ie. the new kernel not clearing the field on copy to userspace and > honouring it on copy to kernel is sufficient. I see, Willem removed this behaviour in newer kernels. The private area is now zeroed, is that what you mean right? So I guess this cannot be done transparently. Anyway, I think the --remain approach to fix this longstanding problem from iptables :-). > So iptables-save | iptables-restore doesn't work, but iptables -A foo does. > > (currently iptables -t X -{A,D} foo clears all xt_quota counters in > table X even when foo is utterly unrelated) > > >> I mean: Instead of using atomic64_set() to set the counter to 1 once > >> we went over quota, > > > > incomplete sentence, sorry: > > > > I mean: Instead of using atomic64_set() to set the counter to 1 once > > we go overquota, we just keep updating 'consumed' bytes. > > I guess it's a fair point that with a u64 we won't ever realistically > overflow the number of sent bytes, so this could be a running counter > of matched bytes... > > and we don't even need to update it if it was over the quota when we > first looked at it, so we'll go over by at most # of cpus * max size > of gso packet bytes. > > > ie. we don't express things in 'remaining bytes' logic, but we account > > for 'bytes we already consumed'. So we never go negative - I know > > understand what you mean about -1... I think we are each other > > thinking from our respective approach proposal. > > I guess our decision was probably driven by xt_quota2 use on android > where infinite quota is often used as a temporary placeholder. I see, thanks for explaining. Thanks.