[v2] netfilter: nf_tables: delete table via table handle

Message ID 20180108175818.23100-1-harshasharmaiitr@gmail.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series
  • [v2] netfilter: nf_tables: delete table via table handle
Related show

Commit Message

Harsha Sharma Jan. 8, 2018, 5:58 p.m.
This patch add code to delete table via unique table handle and table
family.

Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
---
Changes in v2:
 - Remove nf_tables_afinfo_lookup_byhandle
 - Change log message

 net/netfilter/nf_tables_api.c | 45 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso Jan. 8, 2018, 7:23 p.m. | #1
On Mon, Jan 08, 2018 at 11:28:18PM +0530, Harsha Sharma wrote:
> This patch add code to delete table via unique table handle and table
> family.
> 
> Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
> ---
> Changes in v2:
>  - Remove nf_tables_afinfo_lookup_byhandle
>  - Change log message
> 
>  net/netfilter/nf_tables_api.c | 45 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index ba6065c39674..1f1f3be37034 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -400,6 +400,28 @@ static struct nft_table *nft_table_lookup(const struct nft_af_info *afi,
>  	return NULL;
>  }
>  
> +static struct nft_table *__nft_table_lookup_byhandle(const struct nft_af_info *afi,
> +						     u64 handle, u8 genmask)

Please, rename this __nft_table_lookup_byhandle to nft_table_lookup_byhandle.

> +{
> +	struct nft_table *table;
> +
> +	list_for_each_entry(table, &afi->tables, list) {
> +		if (handle == table->handle &&
> +		    nft_active_genmask(table, genmask))
> +			return table;
> +	}
> +	return NULL;
> +}
> +
> +static struct nft_table *nft_table_lookup_byhandle(const struct nft_af_info *afi,
> +						   const struct nlattr *nla,
> +						   u8 genmask)
> +{
> +	return __nft_table_lookup_byhandle(afi,
> +					   be64_to_cpu(nla_get_be64(nla)),
> +					   genmask);
> +}

So you can remove this function above.

> +
>  static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi,
>  						const struct nlattr *nla,
>  						u8 genmask)
> @@ -416,6 +438,22 @@ static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi,
>  	return ERR_PTR(-ENOENT);
>  }
>  
> +static struct nft_table *nf_tables_table_lookup_byhandle(const struct nft_af_info *afi,
> +							 const struct nlattr *nla,
> +							 u8 genmask)
> +{
> +	struct nft_table *table;
> +
> +	if (nla == NULL)
> +		return ERR_PTR(-EINVAL);
> +
> +	table = nft_table_lookup_byhandle(afi, nla, genmask);
> +	if (table != NULL)
> +		return table;
> +
> +	return ERR_PTR(-ENOENT);
> +}
> +
>  static inline u64 nf_tables_alloc_handle(struct nft_table *table)
>  {
>  	return ++table->hgenerator;
> @@ -893,14 +931,17 @@ static int nf_tables_deltable(struct net *net, struct sock *nlsk,
>  	struct nft_ctx ctx;
>  
>  	nft_ctx_init(&ctx, net, skb, nlh, NULL, NULL, NULL, nla);
> -	if (family == AF_UNSPEC || nla[NFTA_TABLE_NAME] == NULL)
> +	if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))

We have to break lines at 80 chars, so I suggest:

	if (family == AF_UNSPEC ||
            (!nla[NFTA_TABLE_NAME] && !nla[NFTA_TABLE_HANDLE]))

>  		return nft_flush(&ctx, family);
>  
>  	afi = nf_tables_afinfo_lookup(net, family, false);
>  	if (IS_ERR(afi))
>  		return PTR_ERR(afi);
> +	if (nla[NFTA_TABLE_HANDLE])
> +		table = nf_tables_table_lookup_byhandle(afi, nla[NFTA_TABLE_HANDLE], genmask);
> +	else
> +		table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
>  
> -	table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
>  	if (IS_ERR(table))
>  		return PTR_ERR(table);

Other than that, this looks good.

Once you send v3 for this, please, follow up with chain handles.

Thanks.
--
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
kbuild test robot Jan. 11, 2018, 12:37 p.m. | #2
Hi Harsha,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf/master]
[also build test ERROR on v4.15-rc7 next-20180110]
[cannot apply to nf-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Harsha-Sharma/netfilter-nf_tables-delete-table-via-table-handle/20180111-153748
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: x86_64-randconfig-u0-01111920 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net//netfilter/nf_tables_api.c: In function '__nft_table_lookup_byhandle':
   net//netfilter/nf_tables_api.c:375:22: error: 'struct nft_table' has no member named 'handle'
      if (handle == table->handle &&
                         ^
   net//netfilter/nf_tables_api.c: In function 'nf_tables_deltable':
>> net//netfilter/nf_tables_api.c:890:66: error: 'NFTA_TABLE_HANDLE' undeclared (first use in this function)
     if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
                                                                     ^
   net//netfilter/nf_tables_api.c:890:66: note: each undeclared identifier is reported only once for each function it appears in

vim +/NFTA_TABLE_HANDLE +890 net//netfilter/nf_tables_api.c

   876	
   877	static int nf_tables_deltable(struct net *net, struct sock *nlsk,
   878				      struct sk_buff *skb, const struct nlmsghdr *nlh,
   879				      const struct nlattr * const nla[],
   880				      struct netlink_ext_ack *extack)
   881	{
   882		const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
   883		u8 genmask = nft_genmask_next(net);
   884		struct nft_af_info *afi;
   885		struct nft_table *table;
   886		int family = nfmsg->nfgen_family;
   887		struct nft_ctx ctx;
   888	
   889		nft_ctx_init(&ctx, net, skb, nlh, NULL, NULL, NULL, nla);
 > 890		if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
   891			return nft_flush(&ctx, family);
   892	
   893		afi = nf_tables_afinfo_lookup(net, family, false);
   894		if (IS_ERR(afi))
   895			return PTR_ERR(afi);
   896		if (nla[NFTA_TABLE_HANDLE])
   897			table = nf_tables_table_lookup_byhandle(afi, nla[NFTA_TABLE_HANDLE], genmask);
   898		else
   899			table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
   900	
   901		if (IS_ERR(table))
   902			return PTR_ERR(table);
   903	
   904		if (nlh->nlmsg_flags & NLM_F_NONREC &&
   905		    table->use > 0)
   906			return -EBUSY;
   907	
   908		ctx.afi = afi;
   909		ctx.table = table;
   910	
   911		return nft_flush_table(&ctx);
   912	}
   913	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Pablo Neira Ayuso Jan. 11, 2018, 12:39 p.m. | #3
Harsha,

Ignore this, kbuild test robot is getting confused because you have
added table->handle in a separated patch.

On Thu, Jan 11, 2018 at 08:37:13PM +0800, kbuild test robot wrote:
> Hi Harsha,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on nf/master]
> [also build test ERROR on v4.15-rc7 next-20180110]
> [cannot apply to nf-next/master]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Harsha-Sharma/netfilter-nf_tables-delete-table-via-table-handle/20180111-153748
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
> config: x86_64-randconfig-u0-01111920 (attached as .config)
> compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    net//netfilter/nf_tables_api.c: In function '__nft_table_lookup_byhandle':
>    net//netfilter/nf_tables_api.c:375:22: error: 'struct nft_table' has no member named 'handle'
>       if (handle == table->handle &&
>                          ^
>    net//netfilter/nf_tables_api.c: In function 'nf_tables_deltable':
> >> net//netfilter/nf_tables_api.c:890:66: error: 'NFTA_TABLE_HANDLE' undeclared (first use in this function)
>      if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
>                                                                      ^
>    net//netfilter/nf_tables_api.c:890:66: note: each undeclared identifier is reported only once for each function it appears in
> 
> vim +/NFTA_TABLE_HANDLE +890 net//netfilter/nf_tables_api.c
> 
>    876	
>    877	static int nf_tables_deltable(struct net *net, struct sock *nlsk,
>    878				      struct sk_buff *skb, const struct nlmsghdr *nlh,
>    879				      const struct nlattr * const nla[],
>    880				      struct netlink_ext_ack *extack)
>    881	{
>    882		const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
>    883		u8 genmask = nft_genmask_next(net);
>    884		struct nft_af_info *afi;
>    885		struct nft_table *table;
>    886		int family = nfmsg->nfgen_family;
>    887		struct nft_ctx ctx;
>    888	
>    889		nft_ctx_init(&ctx, net, skb, nlh, NULL, NULL, NULL, nla);
>  > 890		if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
>    891			return nft_flush(&ctx, family);
>    892	
>    893		afi = nf_tables_afinfo_lookup(net, family, false);
>    894		if (IS_ERR(afi))
>    895			return PTR_ERR(afi);
>    896		if (nla[NFTA_TABLE_HANDLE])
>    897			table = nf_tables_table_lookup_byhandle(afi, nla[NFTA_TABLE_HANDLE], genmask);
>    898		else
>    899			table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
>    900	
>    901		if (IS_ERR(table))
>    902			return PTR_ERR(table);
>    903	
>    904		if (nlh->nlmsg_flags & NLM_F_NONREC &&
>    905		    table->use > 0)
>    906			return -EBUSY;
>    907	
>    908		ctx.afi = afi;
>    909		ctx.table = table;
>    910	
>    911		return nft_flush_table(&ctx);
>    912	}
>    913	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


--
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
kbuild test robot Jan. 11, 2018, 2:13 p.m. | #4
Hi Harsha,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on nf/master]
[also build test ERROR on v4.15-rc7 next-20180110]
[cannot apply to nf-next/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Harsha-Sharma/netfilter-nf_tables-delete-table-via-table-handle/20180111-153748
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net//netfilter/nf_tables_api.c: In function '__nft_table_lookup_byhandle':
>> net//netfilter/nf_tables_api.c:375:22: error: 'struct nft_table' has no member named 'handle'
      if (handle == table->handle &&
                         ^~
   net//netfilter/nf_tables_api.c: In function 'nf_tables_deltable':
>> net//netfilter/nf_tables_api.c:890:66: error: 'NFTA_TABLE_HANDLE' undeclared (first use in this function); did you mean 'NFTA_RULE_HANDLE'?
     if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
                                                                     ^~~~~~~~~~~~~~~~~
                                                                     NFTA_RULE_HANDLE
   net//netfilter/nf_tables_api.c:890:66: note: each undeclared identifier is reported only once for each function it appears in

vim +375 net//netfilter/nf_tables_api.c

   368	
   369	static struct nft_table *__nft_table_lookup_byhandle(const struct nft_af_info *afi,
   370							     u64 handle, u8 genmask)
   371	{
   372		struct nft_table *table;
   373	
   374		list_for_each_entry(table, &afi->tables, list) {
 > 375			if (handle == table->handle &&
   376			    nft_active_genmask(table, genmask))
   377				return table;
   378		}
   379		return NULL;
   380	}
   381	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ba6065c39674..1f1f3be37034 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -400,6 +400,28 @@  static struct nft_table *nft_table_lookup(const struct nft_af_info *afi,
 	return NULL;
 }
 
+static struct nft_table *__nft_table_lookup_byhandle(const struct nft_af_info *afi,
+						     u64 handle, u8 genmask)
+{
+	struct nft_table *table;
+
+	list_for_each_entry(table, &afi->tables, list) {
+		if (handle == table->handle &&
+		    nft_active_genmask(table, genmask))
+			return table;
+	}
+	return NULL;
+}
+
+static struct nft_table *nft_table_lookup_byhandle(const struct nft_af_info *afi,
+						   const struct nlattr *nla,
+						   u8 genmask)
+{
+	return __nft_table_lookup_byhandle(afi,
+					   be64_to_cpu(nla_get_be64(nla)),
+					   genmask);
+}
+
 static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi,
 						const struct nlattr *nla,
 						u8 genmask)
@@ -416,6 +438,22 @@  static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi,
 	return ERR_PTR(-ENOENT);
 }
 
+static struct nft_table *nf_tables_table_lookup_byhandle(const struct nft_af_info *afi,
+							 const struct nlattr *nla,
+							 u8 genmask)
+{
+	struct nft_table *table;
+
+	if (nla == NULL)
+		return ERR_PTR(-EINVAL);
+
+	table = nft_table_lookup_byhandle(afi, nla, genmask);
+	if (table != NULL)
+		return table;
+
+	return ERR_PTR(-ENOENT);
+}
+
 static inline u64 nf_tables_alloc_handle(struct nft_table *table)
 {
 	return ++table->hgenerator;
@@ -893,14 +931,17 @@  static int nf_tables_deltable(struct net *net, struct sock *nlsk,
 	struct nft_ctx ctx;
 
 	nft_ctx_init(&ctx, net, skb, nlh, NULL, NULL, NULL, nla);
-	if (family == AF_UNSPEC || nla[NFTA_TABLE_NAME] == NULL)
+	if (family == AF_UNSPEC || (nla[NFTA_TABLE_NAME] == NULL && nla[NFTA_TABLE_HANDLE] == NULL))
 		return nft_flush(&ctx, family);
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
+	if (nla[NFTA_TABLE_HANDLE])
+		table = nf_tables_table_lookup_byhandle(afi, nla[NFTA_TABLE_HANDLE], genmask);
+	else
+		table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
 
-	table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME], genmask);
 	if (IS_ERR(table))
 		return PTR_ERR(table);