* [PATCH WIP nf-next] netfilter: nf_tables: Introduce stateful object update operation
@ 2019-08-19 11:19 Fernando Fernandez Mancera
2019-08-19 11:55 ` Florian Westphal
0 siblings, 1 reply; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2019-08-19 11:19 UTC (permalink / raw)
To: netfilter-devel; +Cc: Fernando Fernandez Mancera
This is a WIP patch version. I still having some issues in userspace but I
would like to get feedback about the kernel-side patch. Thanks!
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
include/net/netfilter/nf_tables.h | 6 +++
net/netfilter/nf_tables_api.c | 73 ++++++++++++++++++++++++++++---
2 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index dc301e3d6739..dc4e32040ea9 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1123,6 +1123,9 @@ struct nft_object_ops {
int (*dump)(struct sk_buff *skb,
struct nft_object *obj,
bool reset);
+ int (*update)(const struct nft_ctx *ctx,
+ const struct nlattr *const tb[],
+ struct nft_object *obj);
const struct nft_object_type *type;
};
@@ -1405,10 +1408,13 @@ struct nft_trans_elem {
struct nft_trans_obj {
struct nft_object *obj;
+ bool update;
};
#define nft_trans_obj(trans) \
(((struct nft_trans_obj *)trans->data)->obj)
+#define nft_trans_obj_update(trans) \
+ (((struct nft_trans_obj *)trans->data)->update)
struct nft_trans_flowtable {
struct nft_flowtable *flowtable;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index fe3b7b0c6c66..d7b94904599c 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5122,6 +5122,48 @@ nft_obj_type_get(struct net *net, u32 objtype)
return ERR_PTR(-ENOENT);
}
+static int nf_tables_updobj(const struct nft_ctx *ctx,
+ const struct nft_object_type *type,
+ const struct nlattr *attr,
+ struct nft_object *obj)
+{
+ struct nft_trans *trans;
+ struct nlattr **tb;
+ int err = -ENOMEM;
+
+ trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ,
+ sizeof(struct nft_trans_obj));
+ if (!trans)
+ return -ENOMEM;
+
+ tb = kmalloc_array(type->maxattr + 1, sizeof(*tb), GFP_KERNEL);
+ if (!tb)
+ goto err;
+
+ if (attr) {
+ err = nla_parse_nested_deprecated(tb, type->maxattr, attr,
+ type->policy, NULL);
+ if (err < 0)
+ goto err;
+ } else {
+ memset(tb, 0, sizeof(tb[0]) * (type->maxattr + 1));
+ }
+
+ err = obj->ops->update(ctx, (const struct nlattr * const *)tb, obj);
+ if (err < 0)
+ goto err;
+
+ nft_trans_obj_update(trans) = true;
+ list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+
+ kfree(tb);
+ return 0;
+
+err:
+ nft_trans_destroy(trans);
+ return err;
+}
+
static int nf_tables_newobj(struct net *net, struct sock *nlsk,
struct sk_buff *skb, const struct nlmsghdr *nlh,
const struct nlattr * const nla[],
@@ -5161,7 +5203,13 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
NL_SET_BAD_ATTR(extack, nla[NFTA_OBJ_NAME]);
return -EEXIST;
}
- return 0;
+ if (nlh->nlmsg_flags & NLM_F_REPLACE)
+ return -EOPNOTSUPP;
+
+ type = nft_obj_type_get(net, objtype);
+ nft_ctx_init(&ctx, net, skb, nlh, family, table, NULL, nla);
+
+ return nf_tables_updobj(&ctx, type, nla[NFTA_OBJ_DATA], obj);
}
nft_ctx_init(&ctx, net, skb, nlh, family, table, NULL, nla);
@@ -6786,10 +6834,17 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
te->set->ndeact--;
break;
case NFT_MSG_NEWOBJ:
- nft_clear(net, nft_trans_obj(trans));
- nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans),
- NFT_MSG_NEWOBJ);
- nft_trans_destroy(trans);
+ if (nft_trans_obj_update(trans)) {
+ nf_tables_obj_notify(&trans->ctx,
+ nft_trans_obj(trans),
+ NFT_MSG_NEWOBJ);
+ } else {
+ nft_clear(net, nft_trans_obj(trans));
+ nf_tables_obj_notify(&trans->ctx,
+ nft_trans_obj(trans),
+ NFT_MSG_NEWOBJ);
+ nft_trans_destroy(trans);
+ }
break;
case NFT_MSG_DELOBJ:
nft_obj_del(nft_trans_obj(trans));
@@ -6936,8 +6991,12 @@ static int __nf_tables_abort(struct net *net)
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWOBJ:
- trans->ctx.table->use--;
- nft_obj_del(nft_trans_obj(trans));
+ if (nft_trans_obj_update(trans)) {
+ nft_trans_destroy(trans);
+ } else {
+ trans->ctx.table->use--;
+ nft_obj_del(nft_trans_obj(trans));
+ }
break;
case NFT_MSG_DELOBJ:
trans->ctx.table->use++;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH WIP nf-next] netfilter: nf_tables: Introduce stateful object update operation
2019-08-19 11:19 [PATCH WIP nf-next] netfilter: nf_tables: Introduce stateful object update operation Fernando Fernandez Mancera
@ 2019-08-19 11:55 ` Florian Westphal
2019-08-19 13:16 ` Fernando Fernandez Mancera
0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2019-08-19 11:55 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel
Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:
> This is a WIP patch version. I still having some issues in userspace but I
> would like to get feedback about the kernel-side patch. Thanks!
>
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> ---
> include/net/netfilter/nf_tables.h | 6 +++
> net/netfilter/nf_tables_api.c | 73 ++++++++++++++++++++++++++++---
> 2 files changed, 72 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index dc301e3d6739..dc4e32040ea9 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -1123,6 +1123,9 @@ struct nft_object_ops {
> int (*dump)(struct sk_buff *skb,
> struct nft_object *obj,
> bool reset);
> + int (*update)(const struct nft_ctx *ctx,
> + const struct nlattr *const tb[],
> + struct nft_object *obj);
> const struct nft_object_type *type;
> };
>
> @@ -1405,10 +1408,13 @@ struct nft_trans_elem {
>
> struct nft_trans_obj {
> struct nft_object *obj;
> + bool update;
> };
>
> #define nft_trans_obj(trans) \
> (((struct nft_trans_obj *)trans->data)->obj)
> +#define nft_trans_obj_update(trans) \
> + (((struct nft_trans_obj *)trans->data)->update)
>
> struct nft_trans_flowtable {
> struct nft_flowtable *flowtable;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index fe3b7b0c6c66..d7b94904599c 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5122,6 +5122,48 @@ nft_obj_type_get(struct net *net, u32 objtype)
> return ERR_PTR(-ENOENT);
> }
>
> +static int nf_tables_updobj(const struct nft_ctx *ctx,
> + const struct nft_object_type *type,
> + const struct nlattr *attr,
> + struct nft_object *obj)
> +{
> + struct nft_trans *trans;
> + struct nlattr **tb;
> + int err = -ENOMEM;
> +
> + trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ,
> + sizeof(struct nft_trans_obj));
> + if (!trans)
> + return -ENOMEM;
> +
> + tb = kmalloc_array(type->maxattr + 1, sizeof(*tb), GFP_KERNEL);
You can use kcalloc here and then remove the memset().
> + err = obj->ops->update(ctx, (const struct nlattr * const *)tb, obj);
> + if (err < 0)
> + goto err;
This looks wrong, see below.
> @@ -5161,7 +5203,13 @@ static int nf_tables_newobj(struct net *net, struct sock *nlsk,
> NL_SET_BAD_ATTR(extack, nla[NFTA_OBJ_NAME]);
> return -EEXIST;
> }
> - return 0;
> + if (nlh->nlmsg_flags & NLM_F_REPLACE)
> + return -EOPNOTSUPP;
> +
> + type = nft_obj_type_get(net, objtype);
> + nft_ctx_init(&ctx, net, skb, nlh, family, table, NULL, nla);
> +
> + return nf_tables_updobj(&ctx, type, nla[NFTA_OBJ_DATA], obj);
> }
>
> case NFT_MSG_NEWOBJ:
> - nft_clear(net, nft_trans_obj(trans));
> - nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans),
> - NFT_MSG_NEWOBJ);
> - nft_trans_destroy(trans);
> + if (nft_trans_obj_update(trans)) {
> + nf_tables_obj_notify(&trans->ctx,
> + nft_trans_obj(trans),
> + NFT_MSG_NEWOBJ);
I would have expected the ->update() here, when committing the batch.
Under what conditions can an update() fail?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH WIP nf-next] netfilter: nf_tables: Introduce stateful object update operation
2019-08-19 11:55 ` Florian Westphal
@ 2019-08-19 13:16 ` Fernando Fernandez Mancera
0 siblings, 0 replies; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2019-08-19 13:16 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
Hi Florian,
El 19 de agosto de 2019 13:55:27 CEST, Florian Westphal <fw@strlen.de> escribió:
>Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:
>> This is a WIP patch version. I still having some issues in userspace
>but I
>> would like to get feedback about the kernel-side patch. Thanks!
>>
>> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
>> ---
>> include/net/netfilter/nf_tables.h | 6 +++
>> net/netfilter/nf_tables_api.c | 73
>++++++++++++++++++++++++++++---
>> 2 files changed, 72 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/net/netfilter/nf_tables.h
>b/include/net/netfilter/nf_tables.h
>> index dc301e3d6739..dc4e32040ea9 100644
>> --- a/include/net/netfilter/nf_tables.h
>> +++ b/include/net/netfilter/nf_tables.h
>> @@ -1123,6 +1123,9 @@ struct nft_object_ops {
>> int (*dump)(struct sk_buff *skb,
>> struct nft_object *obj,
>> bool reset);
>> + int (*update)(const struct nft_ctx *ctx,
>> + const struct nlattr *const tb[],
>> + struct nft_object *obj);
>> const struct nft_object_type *type;
>> };
>>
>> @@ -1405,10 +1408,13 @@ struct nft_trans_elem {
>>
>> struct nft_trans_obj {
>> struct nft_object *obj;
>> + bool update;
>> };
>>
>> #define nft_trans_obj(trans) \
>> (((struct nft_trans_obj *)trans->data)->obj)
>> +#define nft_trans_obj_update(trans) \
>> + (((struct nft_trans_obj *)trans->data)->update)
>>
>> struct nft_trans_flowtable {
>> struct nft_flowtable *flowtable;
>> diff --git a/net/netfilter/nf_tables_api.c
>b/net/netfilter/nf_tables_api.c
>> index fe3b7b0c6c66..d7b94904599c 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -5122,6 +5122,48 @@ nft_obj_type_get(struct net *net, u32 objtype)
>> return ERR_PTR(-ENOENT);
>> }
>>
>> +static int nf_tables_updobj(const struct nft_ctx *ctx,
>> + const struct nft_object_type *type,
>> + const struct nlattr *attr,
>> + struct nft_object *obj)
>> +{
>> + struct nft_trans *trans;
>> + struct nlattr **tb;
>> + int err = -ENOMEM;
>> +
>> + trans = nft_trans_alloc(ctx, NFT_MSG_NEWOBJ,
>> + sizeof(struct nft_trans_obj));
>> + if (!trans)
>> + return -ENOMEM;
>> +
>> + tb = kmalloc_array(type->maxattr + 1, sizeof(*tb), GFP_KERNEL);
>
>You can use kcalloc here and then remove the memset().
>p
>> + err = obj->ops->update(ctx, (const struct nlattr * const *)tb,
>obj);
>> + if (err < 0)
>> + goto err;
>
>This looks wrong, see below.
>
>> @@ -5161,7 +5203,13 @@ static int nf_tables_newobj(struct net *net,
>struct sock *nlsk,
>> NL_SET_BAD_ATTR(extack, nla[NFTA_OBJ_NAME]);
>> return -EEXIST;
>> }
>> - return 0;
>> + if (nlh->nlmsg_flags & NLM_F_REPLACE)
>> + return -EOPNOTSUPP;
>> +
>> + type = nft_obj_type_get(net, objtype);
>> + nft_ctx_init(&ctx, net, skb, nlh, family, table, NULL, nla);
>> +
>> + return nf_tables_updobj(&ctx, type, nla[NFTA_OBJ_DATA], obj);
>> }
>>
>> case NFT_MSG_NEWOBJ:
>> - nft_clear(net, nft_trans_obj(trans));
>> - nf_tables_obj_notify(&trans->ctx, nft_trans_obj(trans),
>> - NFT_MSG_NEWOBJ);
>> - nft_trans_destroy(trans);
>> + if (nft_trans_obj_update(trans)) {
>> + nf_tables_obj_notify(&trans->ctx,
>> + nft_trans_obj(trans),
>> + NFT_MSG_NEWOBJ);
>
>I would have expected the ->update() here, when committing the batch.
>Under what conditions can an update() fail?
It depends on the object type. In the quota case it can fail if the quota have invalid values.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-19 13:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 11:19 [PATCH WIP nf-next] netfilter: nf_tables: Introduce stateful object update operation Fernando Fernandez Mancera
2019-08-19 11:55 ` Florian Westphal
2019-08-19 13:16 ` Fernando Fernandez Mancera
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.