* [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args
@ 2022-02-04 12:11 Florian Westphal
2022-02-09 10:43 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2022-02-04 12:11 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
netlink_dump structure has a union of 'long args[6]' and a context
buffer as scratch space.
Convert ctnetlink to use a structure, its easier to read than the
raw 'args' usage which comes with no type checks and no readable names.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_netlink.c | 36 ++++++++++++++++++----------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index ac438370f94a..3d9f9ee50294 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -58,6 +58,12 @@
MODULE_LICENSE("GPL");
+struct ctnetlink_list_dump_ctx {
+ struct nf_conn *last;
+ unsigned int cpu;
+ bool done;
+};
+
static int ctnetlink_dump_tuples_proto(struct sk_buff *skb,
const struct nf_conntrack_tuple *tuple,
const struct nf_conntrack_l4proto *l4proto)
@@ -1694,14 +1700,18 @@ static int ctnetlink_get_conntrack(struct sk_buff *skb,
static int ctnetlink_done_list(struct netlink_callback *cb)
{
- if (cb->args[1])
- nf_ct_put((struct nf_conn *)cb->args[1]);
+ struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
+
+ if (ctx->last)
+ nf_ct_put(ctx->last);
+
return 0;
}
static int
ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying)
{
+ struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
struct nf_conn *ct, *last;
struct nf_conntrack_tuple_hash *h;
struct hlist_nulls_node *n;
@@ -1712,12 +1722,12 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
struct hlist_nulls_head *list;
struct net *net = sock_net(skb->sk);
- if (cb->args[2])
+ if (ctx->done)
return 0;
- last = (struct nf_conn *)cb->args[1];
+ last = ctx->last;
- for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) {
+ for (cpu = ctx->cpu; cpu < nr_cpu_ids; cpu++) {
struct ct_pcpu *pcpu;
if (!cpu_possible(cpu))
@@ -1731,10 +1741,10 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
ct = nf_ct_tuplehash_to_ctrack(h);
if (l3proto && nf_ct_l3num(ct) != l3proto)
continue;
- if (cb->args[1]) {
+ if (ctx->last) {
if (ct != last)
continue;
- cb->args[1] = 0;
+ ctx->last = NULL;
}
/* We can't dump extension info for the unconfirmed
@@ -1751,19 +1761,19 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
if (res < 0) {
if (!refcount_inc_not_zero(&ct->ct_general.use))
continue;
- cb->args[0] = cpu;
- cb->args[1] = (unsigned long)ct;
+ ctx->cpu = cpu;
+ ctx->last = ct;
spin_unlock_bh(&pcpu->lock);
goto out;
}
}
- if (cb->args[1]) {
- cb->args[1] = 0;
+ if (ctx->last) {
+ ctx->last = NULL;
goto restart;
}
spin_unlock_bh(&pcpu->lock);
}
- cb->args[2] = 1;
+ ctx->done = true;
out:
if (last)
nf_ct_put(last);
@@ -3877,6 +3887,8 @@ static int __init ctnetlink_init(void)
{
int ret;
+ BUILD_BUG_ON(sizeof(struct ctnetlink_list_dump_ctx) > sizeof_field(struct netlink_callback, ctx));
+
ret = nfnetlink_subsys_register(&ctnl_subsys);
if (ret < 0) {
pr_err("ctnetlink_init: cannot register with nfnetlink.\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args
2022-02-04 12:11 [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args Florian Westphal
@ 2022-02-09 10:43 ` Pablo Neira Ayuso
2022-02-09 10:46 ` Florian Westphal
0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2022-02-09 10:43 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Fri, Feb 04, 2022 at 01:11:45PM +0100, Florian Westphal wrote:
> netlink_dump structure has a union of 'long args[6]' and a context
> buffer as scratch space.
>
> Convert ctnetlink to use a structure, its easier to read than the
> raw 'args' usage which comes with no type checks and no readable names.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/netfilter/nf_conntrack_netlink.c | 36 ++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index ac438370f94a..3d9f9ee50294 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -58,6 +58,12 @@
>
> MODULE_LICENSE("GPL");
>
> +struct ctnetlink_list_dump_ctx {
> + struct nf_conn *last;
> + unsigned int cpu;
> + bool done;
> +};
> +
> static int ctnetlink_dump_tuples_proto(struct sk_buff *skb,
> const struct nf_conntrack_tuple *tuple,
> const struct nf_conntrack_l4proto *l4proto)
> @@ -1694,14 +1700,18 @@ static int ctnetlink_get_conntrack(struct sk_buff *skb,
>
> static int ctnetlink_done_list(struct netlink_callback *cb)
> {
> - if (cb->args[1])
> - nf_ct_put((struct nf_conn *)cb->args[1]);
> + struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
Where is cb->ctx initialized?
> + if (ctx->last)
> + nf_ct_put(ctx->last);
> +
> return 0;
> }
>
> static int
> ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying)
> {
> + struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
> struct nf_conn *ct, *last;
> struct nf_conntrack_tuple_hash *h;
> struct hlist_nulls_node *n;
> @@ -1712,12 +1722,12 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
> struct hlist_nulls_head *list;
> struct net *net = sock_net(skb->sk);
>
> - if (cb->args[2])
> + if (ctx->done)
> return 0;
>
> - last = (struct nf_conn *)cb->args[1];
> + last = ctx->last;
>
> - for (cpu = cb->args[0]; cpu < nr_cpu_ids; cpu++) {
> + for (cpu = ctx->cpu; cpu < nr_cpu_ids; cpu++) {
> struct ct_pcpu *pcpu;
>
> if (!cpu_possible(cpu))
> @@ -1731,10 +1741,10 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
> ct = nf_ct_tuplehash_to_ctrack(h);
> if (l3proto && nf_ct_l3num(ct) != l3proto)
> continue;
> - if (cb->args[1]) {
> + if (ctx->last) {
> if (ct != last)
> continue;
> - cb->args[1] = 0;
> + ctx->last = NULL;
> }
>
> /* We can't dump extension info for the unconfirmed
> @@ -1751,19 +1761,19 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying
> if (res < 0) {
> if (!refcount_inc_not_zero(&ct->ct_general.use))
> continue;
> - cb->args[0] = cpu;
> - cb->args[1] = (unsigned long)ct;
> + ctx->cpu = cpu;
> + ctx->last = ct;
> spin_unlock_bh(&pcpu->lock);
> goto out;
> }
> }
> - if (cb->args[1]) {
> - cb->args[1] = 0;
> + if (ctx->last) {
> + ctx->last = NULL;
> goto restart;
> }
> spin_unlock_bh(&pcpu->lock);
> }
> - cb->args[2] = 1;
> + ctx->done = true;
> out:
> if (last)
> nf_ct_put(last);
> @@ -3877,6 +3887,8 @@ static int __init ctnetlink_init(void)
> {
> int ret;
>
> + BUILD_BUG_ON(sizeof(struct ctnetlink_list_dump_ctx) > sizeof_field(struct netlink_callback, ctx));
> +
> ret = nfnetlink_subsys_register(&ctnl_subsys);
> if (ret < 0) {
> pr_err("ctnetlink_init: cannot register with nfnetlink.\n");
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args
2022-02-09 10:43 ` Pablo Neira Ayuso
@ 2022-02-09 10:46 ` Florian Westphal
2022-02-09 11:06 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2022-02-09 10:46 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > static int ctnetlink_done_list(struct netlink_callback *cb)
> > {
> > - if (cb->args[1])
> > - nf_ct_put((struct nf_conn *)cb->args[1]);
> > + struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
>
> Where is cb->ctx initialized?
Its zeroed in __netlink_dump_start, cb->ctx and cb->args are aliased
via union inside netlink_callback struct.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args
2022-02-09 10:46 ` Florian Westphal
@ 2022-02-09 11:06 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2022-02-09 11:06 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Wed, Feb 09, 2022 at 11:46:59AM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > static int ctnetlink_done_list(struct netlink_callback *cb)
> > > {
> > > - if (cb->args[1])
> > > - nf_ct_put((struct nf_conn *)cb->args[1]);
> > > + struct ctnetlink_list_dump_ctx *ctx = (void *)cb->ctx;
> >
> > Where is cb->ctx initialized?
>
> Its zeroed in __netlink_dump_start, cb->ctx and cb->args are aliased
> via union inside netlink_callback struct.
Ah I see, it's all zero initially.
Thanks for explaining.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-09 12:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-04 12:11 [PATCH nf-next] netfilter: ctnetlink: use dump structure instead of raw args Florian Westphal
2022-02-09 10:43 ` Pablo Neira Ayuso
2022-02-09 10:46 ` Florian Westphal
2022-02-09 11:06 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).