Patchwork [nftables-kernel] netfilter: nf_tables: Fixes how a table is checked to be in use

login
register
mail settings
Submitter Tomasz Bursztyka
Date Aug. 30, 2013, 9:43 a.m.
Message ID <1377855812-15251-1-git-send-email-tomasz.bursztyka@linux.intel.com>
Download mbox | patch
Permalink /patch/271202/
State Not Applicable
Headers show

Comments

Tomasz Bursztyka - Aug. 30, 2013, 9:43 a.m.
Let's check table's chain list emptyness instead so struct nft_table
does not grow and we avoid the (unlikely to happen) overflow.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/net/netfilter/nf_tables.h | 1 -
 net/netfilter/nf_tables_api.c     | 7 +------
 2 files changed, 1 insertion(+), 7 deletions(-)
Pablo Neira - Aug. 30, 2013, 10:37 p.m.
On Fri, Aug 30, 2013 at 12:43:32PM +0300, Tomasz Bursztyka wrote:
> Let's check table's chain list emptyness instead so struct nft_table
> does not grow and we avoid the (unlikely to happen) overflow.

I prefer if you send me a patch to export the table use counter. I
think it can be useful from userspace to know if the table is used at
all and how many times it's used. I'm neither worry about the 2^32
chains limit and the extra 4 bytes per table (we'll have a small
number of tables object in memory).
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Bursztyka - Sept. 3, 2013, 1:25 p.m.
Hi Pablo,

>> Let's check table's chain list emptyness instead so struct nft_table
>> does not grow and we avoid the (unlikely to happen) overflow.
> I prefer if you send me a patch to export the table use counter. I
> think it can be useful from userspace to know if the table is used at
> all and how many times it's used. I'm neither worry about the 2^32
> chains limit and the extra 4 bytes per table (we'll have a small
> number of tables object in memory).

What's the use case for it? I mean: knowing that the table is in use, I 
can see some use case (and it could be done much simpler).
The number of chains, not really. At least right now, nothing is using 
it so what could be the future need for that?
Same for chains actually.

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - Sept. 4, 2013, 10:48 a.m.
On Tue, Sep 03, 2013 at 04:25:40PM +0300, Tomasz Bursztyka wrote:
[...]
> What's the use case for it? I mean: knowing that the table is in
> use, I can see some use case (and it could be done much simpler).
> The number of chains, not really. At least right now, nothing is
> using it so what could be the future need for that?
> Same for chains actually.

We need that use counter for iptables-nftables for chains. See xtables
-L for non-base chains.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Bursztyka - Sept. 4, 2013, 11:51 a.m.
Hi Pablo,

> We need that use counter for iptables-nftables for chains. See xtables
> -L for non-base chains.

Oh you mean the references!
So semantically the 'use' attribute is not really proper (could it be 
changed? we need to update the documentation of the netlink API anyway: 
many attributes are not described),
moreover if you want to add the same attribute name in table which would 
have a different sense.

So on chain I see the point. Not on table though :)
(or a simpler one, knowing that "there are chains in it" instead of a 
counter)

Tomasz

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 215edf5..e4306a4 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -450,7 +450,6 @@  struct nft_table {
 	struct list_head		chains;
 	struct list_head		sets;
 	u64				hgenerator;
-	u32				use;
 	u16				flags;
 	char				name[];
 };
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c5d0129..68f90da 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -448,7 +448,7 @@  static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
-	if (table->use)
+	if (!list_empty(&table->chains))
 		return -EBUSY;
 
 	list_del(&table->list);
@@ -835,9 +835,6 @@  static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
-	if (table->use == UINT_MAX)
-		return -EOVERFLOW;
-
 	chain = NULL;
 	name = nla[NFTA_CHAIN_NAME];
 
@@ -992,7 +989,6 @@  static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		}
 	}
 	list_add_tail(&chain->list, &table->chains);
-	table->use++;
 notify:
 	nf_tables_chain_notify(skb, nlh, table, chain, NFT_MSG_NEWCHAIN,
 			       family);
@@ -1038,7 +1034,6 @@  static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 		return -EBUSY;
 
 	list_del(&chain->list);
-	table->use--;
 
 	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
 	    chain->flags & NFT_BASE_CHAIN)