* [PATCH 0/2] netlink: netlink_dump_start takes pointer to data
@ 2012-02-24 22:14 pablo
2012-02-24 22:14 ` [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks pablo
2012-02-24 22:14 ` [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings pablo
0 siblings, 2 replies; 7+ messages in thread
From: pablo @ 2012-02-24 22:14 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, davem
From: Pablo Neira Ayuso <pablo@netfilter.org>
Hi,
This patch modifies netlink_dump_start to take a pointer to data.
All caller are modified to use the new interface. This pointer can
be accessed via cb->data from the callback.
The follow-up patch for ctnetlink uses this change to pass the
filter structure, which allows filtering the dump by the ct
mark/mask.
Let me know if you are OK with this change, I can included it
in the next netfilter update or you may want to directly apply
it.
Thanks!
Pablo Neira Ayuso (2):
netlink: netlink_dump_start may take data pointer for callbacks
netfilter: ctnetlink: support kernel-space dump filterings
crypto/crypto_user.c | 2 +-
drivers/infiniband/core/netlink.c | 2 +-
include/linux/netfilter/nfnetlink_conntrack.h | 1 +
include/linux/netlink.h | 2 +
net/core/rtnetlink.c | 2 +-
net/ipv4/inet_diag.c | 4 +-
net/netfilter/ipset/ip_set_core.c | 2 +-
net/netfilter/nf_conntrack_netlink.c | 37 +++++++++++++++++++++++--
net/netfilter/nfnetlink_acct.c | 2 +-
net/netlink/af_netlink.c | 2 +
net/netlink/genetlink.c | 2 +-
net/unix/diag.c | 2 +-
net/xfrm/xfrm_user.c | 2 +-
13 files changed, 49 insertions(+), 13 deletions(-)
--
1.7.7.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks
2012-02-24 22:14 [PATCH 0/2] netlink: netlink_dump_start takes pointer to data pablo
@ 2012-02-24 22:14 ` pablo
2012-02-24 22:47 ` David Miller
2012-02-24 22:14 ` [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings pablo
1 sibling, 1 reply; 7+ messages in thread
From: pablo @ 2012-02-24 22:14 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, davem
From: Pablo Neira Ayuso <pablo@netfilter.org>
This patch modifies the netlink_dump_start function to take one
generic pointer to data. This pointer can be used inside the
dump() and done() callbacks via cb->data.
Netfilter is going to use this patch to provide filtered dumps
to user-space. This is specifically interesting in ctnetlink that
may handle lots of conntrack entries. We can save precious
cycles by skipping the conversion to TLV format of conntrack
entries that are not interesting for user-space.
More specifically, ctnetlink will include one operation to allow
to filter the dumping of conntrack entries by ctmark values.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
crypto/crypto_user.c | 2 +-
drivers/infiniband/core/netlink.c | 2 +-
include/linux/netlink.h | 2 ++
net/core/rtnetlink.c | 2 +-
net/ipv4/inet_diag.c | 4 ++--
net/netfilter/ipset/ip_set_core.c | 2 +-
net/netfilter/nf_conntrack_netlink.c | 4 ++--
net/netfilter/nfnetlink_acct.c | 2 +-
net/netlink/af_netlink.c | 2 ++
net/netlink/genetlink.c | 2 +-
net/unix/diag.c | 2 +-
net/xfrm/xfrm_user.c | 2 +-
12 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 16f8693..231d28a 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -391,7 +391,7 @@ static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return -EINVAL;
return netlink_dump_start(crypto_nlsk, skb, nlh,
- link->dump, link->done, 0);
+ link->dump, link->done, NULL, 0);
}
err = nlmsg_parse(nlh, crypto_msg_min[type], attrs, CRYPTOCFGA_MAX,
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index d1c8196..13e8098 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -149,7 +149,7 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return -EINVAL;
return netlink_dump_start(nls, skb, nlh,
client->cb_table[op].dump,
- NULL, 0);
+ NULL, NULL, 0);
}
}
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index a390e9d..9f233ec 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -225,6 +225,7 @@ struct netlink_callback {
int (*dump)(struct sk_buff * skb,
struct netlink_callback *cb);
int (*done)(struct netlink_callback *cb);
+ void *data;
u16 family;
u16 min_dump_alloc;
unsigned int prev_seq, seq;
@@ -252,6 +253,7 @@ extern int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
int (*dump)(struct sk_buff *skb, struct netlink_callback*),
int (*done)(struct netlink_callback*),
+ void *data,
u16 min_dump_alloc);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 65aebd4..7de8b24 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1982,7 +1982,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
__rtnl_unlock();
rtnl = net->rtnl;
err = netlink_dump_start(rtnl, skb, nlh, dumpit,
- NULL, min_dump_alloc);
+ NULL, NULL, min_dump_alloc);
rtnl_lock();
return err;
}
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index fcf2818..c1ff5dd 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -962,7 +962,7 @@ static int inet_diag_rcv_msg_compat(struct sk_buff *skb, struct nlmsghdr *nlh)
}
return netlink_dump_start(sock_diag_nlsk, skb, nlh,
- inet_diag_dump_compat, NULL, 0);
+ inet_diag_dump_compat, NULL, NULL, 0);
}
return inet_diag_get_exact_compat(skb, nlh);
@@ -987,7 +987,7 @@ static int inet_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
}
return netlink_dump_start(sock_diag_nlsk, skb, h,
- inet_diag_dump, NULL, 0);
+ inet_diag_dump, NULL, NULL, 0);
}
return inet_diag_get_exact(skb, h, (struct inet_diag_req_v2 *)NLMSG_DATA(h));
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index aeee9bd..7b63c16 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1164,7 +1164,7 @@ ip_set_dump(struct sock *ctnl, struct sk_buff *skb,
return netlink_dump_start(ctnl, skb, nlh,
ip_set_dump_start,
- ip_set_dump_done, 0);
+ ip_set_dump_done, NULL, 0);
}
/* Add, del and test */
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 33a7bb4..404b317 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -979,7 +979,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP)
return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
- ctnetlink_done, 0);
+ ctnetlink_done, NULL, 0);
err = ctnetlink_parse_zone(cda[CTA_ZONE], &zone);
if (err < 0)
@@ -1883,7 +1883,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
return netlink_dump_start(ctnl, skb, nlh,
ctnetlink_exp_dump_table,
- ctnetlink_exp_done, 0);
+ ctnetlink_exp_done, NULL, 0);
}
err = ctnetlink_parse_zone(cda[CTA_EXPECT_ZONE], &zone);
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 11ba013..b7ea475 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -172,7 +172,7 @@ nfnl_acct_get(struct sock *nfnl, struct sk_buff *skb,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
return netlink_dump_start(nfnl, skb, nlh, nfnl_acct_dump,
- NULL, 0);
+ NULL, NULL, 0);
}
if (!tb[NFACCT_NAME])
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 4d751e3..ff08c89 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1739,6 +1739,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
int (*dump)(struct sk_buff *skb,
struct netlink_callback *),
int (*done)(struct netlink_callback *),
+ void *data,
u16 min_dump_alloc)
{
struct netlink_callback *cb;
@@ -1754,6 +1755,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
cb->done = done;
cb->nlh = nlh;
cb->min_dump_alloc = min_dump_alloc;
+ cb->data = data;
atomic_inc(&skb->users);
cb->skb = skb;
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index a115471..1a5cbca 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -564,7 +564,7 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
genl_unlock();
err = netlink_dump_start(net->genl_sock, skb, nlh,
- ops->dumpit, ops->done, 0);
+ ops->dumpit, ops->done, NULL, 0);
genl_lock();
return err;
}
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 6b7697f..8ad0962 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -303,7 +303,7 @@ static int unix_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
if (h->nlmsg_flags & NLM_F_DUMP)
return netlink_dump_start(sock_diag_nlsk, skb, h,
- unix_diag_dump, NULL, 0);
+ unix_diag_dump, NULL, NULL, 0);
else
return unix_diag_get_exact(skb, h, (struct unix_diag_req *)NLMSG_DATA(h));
}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 66b84fb..eb86719 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2300,7 +2300,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
return -EINVAL;
return netlink_dump_start(net->xfrm.nlsk, skb, nlh,
- link->dump, link->done, 0);
+ link->dump, link->done, NULL, 0);
}
err = nlmsg_parse(nlh, xfrm_msg_min[type], attrs, XFRMA_MAX,
--
1.7.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings
2012-02-24 22:14 [PATCH 0/2] netlink: netlink_dump_start takes pointer to data pablo
2012-02-24 22:14 ` [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks pablo
@ 2012-02-24 22:14 ` pablo
2012-02-25 1:09 ` Jan Engelhardt
1 sibling, 1 reply; 7+ messages in thread
From: pablo @ 2012-02-24 22:14 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, davem
From: Pablo Neira Ayuso <pablo@netfilter.org>
This patch adds CTA_MARK_MASK which, together with CTA_MARK, allows
you to selectively send conntrack entries to user-space by
returning those that match mark & mask.
With this, we can save cycles in the building and the parsing of
the entries that may be later on filtered out in user-space by using
the mark & mask.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/netfilter/nfnetlink_conntrack.h | 1 +
net/netfilter/nf_conntrack_netlink.c | 35 +++++++++++++++++++++++-
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/include/linux/netfilter/nfnetlink_conntrack.h b/include/linux/netfilter/nfnetlink_conntrack.h
index 5ec1abc..e58e4b9 100644
--- a/include/linux/netfilter/nfnetlink_conntrack.h
+++ b/include/linux/netfilter/nfnetlink_conntrack.h
@@ -43,6 +43,7 @@ enum ctattr_type {
CTA_ZONE,
CTA_SECCTX,
CTA_TIMESTAMP,
+ CTA_MARK_MASK,
__CTA_MAX
};
#define CTA_MAX (__CTA_MAX - 1)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 404b317..cdd21f7 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -691,9 +691,18 @@ static int ctnetlink_done(struct netlink_callback *cb)
{
if (cb->args[1])
nf_ct_put((struct nf_conn *)cb->args[1]);
+ if (cb->data)
+ kfree(cb->data);
return 0;
}
+struct ctnetlink_dump_filter {
+ struct {
+ u_int32_t value;
+ u_int32_t mask;
+ } mark;
+};
+
static int
ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
{
@@ -703,6 +712,7 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
struct hlist_nulls_node *n;
struct nfgenmsg *nfmsg = nlmsg_data(cb->nlh);
u_int8_t l3proto = nfmsg->nfgen_family;
+ const struct ctnetlink_dump_filter *filter = cb->data;
spin_lock_bh(&nf_conntrack_lock);
last = (struct nf_conn *)cb->args[1];
@@ -723,6 +733,10 @@ restart:
continue;
cb->args[1] = 0;
}
+ if (filter->mark.mask &&
+ !(filter->mark.value ==
+ (ct->mark & filter->mark.mask)))
+ continue;
if (ctnetlink_fill_info(skb, NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq,
NFNL_MSG_TYPE(
@@ -894,6 +908,7 @@ static const struct nla_policy ct_nla_policy[CTA_MAX+1] = {
[CTA_NAT_DST] = { .type = NLA_NESTED },
[CTA_TUPLE_MASTER] = { .type = NLA_NESTED },
[CTA_ZONE] = { .type = NLA_U16 },
+ [CTA_MARK_MASK] = { .type = NLA_U32 },
};
static int
@@ -977,9 +992,25 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
u16 zone;
int err;
- if (nlh->nlmsg_flags & NLM_F_DUMP)
+ if (nlh->nlmsg_flags & NLM_F_DUMP) {
+ struct ctnetlink_dump_filter *filter = NULL;
+
+#if defined(CONFIG_NF_CONNTRACK_MARK)
+ filter = kzalloc(sizeof(struct ctnetlink_dump_filter),
+ GFP_KERNEL);
+ if (filter == NULL)
+ return -ENOMEM;
+
+ if (cda[CTA_MARK])
+ filter->mark.value = ntohl(nla_get_be32(cda[CTA_MARK]));
+ if (cda[CTA_MARK_MASK]) {
+ filter->mark.mask =
+ ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
+ }
+#endif
return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
- ctnetlink_done, NULL, 0);
+ ctnetlink_done, filter, 0);
+ }
err = ctnetlink_parse_zone(cda[CTA_ZONE], &zone);
if (err < 0)
--
1.7.7.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks
2012-02-24 22:14 ` [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks pablo
@ 2012-02-24 22:47 ` David Miller
2012-02-24 23:18 ` Pablo Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2012-02-24 22:47 UTC (permalink / raw)
To: pablo; +Cc: netdev, netfilter-devel
From: pablo@netfilter.org
Date: Fri, 24 Feb 2012 23:14:07 +0100
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> This patch modifies the netlink_dump_start function to take one
> generic pointer to data. This pointer can be used inside the
> dump() and done() callbacks via cb->data.
>
> Netfilter is going to use this patch to provide filtered dumps
> to user-space. This is specifically interesting in ctnetlink that
> may handle lots of conntrack entries. We can save precious
> cycles by skipping the conversion to TLV format of conntrack
> entries that are not interesting for user-space.
>
> More specifically, ctnetlink will include one operation to allow
> to filter the dumping of conntrack entries by ctmark values.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This isn't really your fault but netlink_dump_start() has an
enormous number of arguments.
Several of them are zero or NULL in all except one special situation.
An entire argument is a lot of overhead for one situation to impose on
all the others.
I have no objection to the data callback scheme, it's just that
the argument list of this interface is getting out of control.
Usually, in situations like this, we have some control structure
that holds all the control state and we pass that in instead.
struct netlink_dump_control c = { .dump = dump, .done = done, ... };
netlink_dump_start(..., &c);
It could be perhaps used here to get things back under control.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks
2012-02-24 22:47 ` David Miller
@ 2012-02-24 23:18 ` Pablo Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2012-02-24 23:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev, netfilter-devel
On Fri, Feb 24, 2012 at 05:47:26PM -0500, David Miller wrote:
> From: pablo@netfilter.org
> Date: Fri, 24 Feb 2012 23:14:07 +0100
>
> > From: Pablo Neira Ayuso <pablo@netfilter.org>
> >
> > This patch modifies the netlink_dump_start function to take one
> > generic pointer to data. This pointer can be used inside the
> > dump() and done() callbacks via cb->data.
> >
> > Netfilter is going to use this patch to provide filtered dumps
> > to user-space. This is specifically interesting in ctnetlink that
> > may handle lots of conntrack entries. We can save precious
> > cycles by skipping the conversion to TLV format of conntrack
> > entries that are not interesting for user-space.
> >
> > More specifically, ctnetlink will include one operation to allow
> > to filter the dumping of conntrack entries by ctmark values.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> This isn't really your fault but netlink_dump_start() has an
> enormous number of arguments.
>
> Several of them are zero or NULL in all except one special situation.
>
> An entire argument is a lot of overhead for one situation to impose on
> all the others.
>
> I have no objection to the data callback scheme, it's just that
> the argument list of this interface is getting out of control.
>
> Usually, in situations like this, we have some control structure
> that holds all the control state and we pass that in instead.
>
> struct netlink_dump_control c = { .dump = dump, .done = done, ... };
>
> netlink_dump_start(..., &c);
>
> It could be perhaps used here to get things back under control.
OK, I'll send a patch to make it like this.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings
2012-02-24 22:14 ` [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings pablo
@ 2012-02-25 1:09 ` Jan Engelhardt
2012-02-25 13:26 ` Pablo Neira Ayuso
0 siblings, 1 reply; 7+ messages in thread
From: Jan Engelhardt @ 2012-02-25 1:09 UTC (permalink / raw)
To: pablo; +Cc: netdev, netfilter-devel, davem
On Friday 2012-02-24 23:14, pablo@netfilter.org wrote:
>@@ -977,9 +992,25 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
> u16 zone;
> int err;
>
>- if (nlh->nlmsg_flags & NLM_F_DUMP)
>+ if (nlh->nlmsg_flags & NLM_F_DUMP) {
>+ struct ctnetlink_dump_filter *filter = NULL;
>+
>+#if defined(CONFIG_NF_CONNTRACK_MARK)
>+ filter = kzalloc(sizeof(struct ctnetlink_dump_filter),
>+ GFP_KERNEL);
>+ if (filter == NULL)
>+ return -ENOMEM;
>+
>+ if (cda[CTA_MARK])
>+ filter->mark.value = ntohl(nla_get_be32(cda[CTA_MARK]));
>+ if (cda[CTA_MARK_MASK]) {
>+ filter->mark.mask =
>+ ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
>+ }
>+#endif
> return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
>- ctnetlink_done, NULL, 0);
>+ ctnetlink_done, filter, 0);
>+ }
I had thought of the following before your patch came up:
ctnl_dump_any(skb,cb)
{
...loop over CTs...
}
ctnl_dump_foo(skb,cb)
{
if (cb->args[0] == NULL) {
cb->args[0] = filter = kzalloc(sizeof(struct ctnl_dump_filter));
if (cb->nlh has CTA_MARK) /* [1] */
filter->mark.value = ...
}
return ctnl_dump_any(skb,cb);
}
ctnl_dump_bar(skb,cb)
{
if (cb->args[0] == NULL) {
cb->args[0] = somethingelse;
}
return ctnl_dump_any(skb,cb);
}
ctnetlink_get_foo(ctnl,skb,...)
{
netlink_dump_start(ctnl,skb,nlh,ctnl_dump_foo,ctnl_done,0);
}
ctnetlink_get_bar(ctnl,skb,...)
{
netlink_dump_start(ctnl,skb,nlh,ctnl_dump_bar,ctnl_done,0);
}
[1]: Arguably needs a way to put cda into cb.
==
Either way, netlink is gathering up a lot of arguments and has come
to the point where I would suggest to put it all into a struct,
like we have done for struct xt_action_param.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings
2012-02-25 1:09 ` Jan Engelhardt
@ 2012-02-25 13:26 ` Pablo Neira Ayuso
0 siblings, 0 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2012-02-25 13:26 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netdev, netfilter-devel, davem
On Sat, Feb 25, 2012 at 02:09:09AM +0100, Jan Engelhardt wrote:
>
> On Friday 2012-02-24 23:14, pablo@netfilter.org wrote:
> >@@ -977,9 +992,25 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
> > u16 zone;
> > int err;
> >
> >- if (nlh->nlmsg_flags & NLM_F_DUMP)
> >+ if (nlh->nlmsg_flags & NLM_F_DUMP) {
> >+ struct ctnetlink_dump_filter *filter = NULL;
> >+
> >+#if defined(CONFIG_NF_CONNTRACK_MARK)
> >+ filter = kzalloc(sizeof(struct ctnetlink_dump_filter),
> >+ GFP_KERNEL);
> >+ if (filter == NULL)
> >+ return -ENOMEM;
> >+
> >+ if (cda[CTA_MARK])
> >+ filter->mark.value = ntohl(nla_get_be32(cda[CTA_MARK]));
> >+ if (cda[CTA_MARK_MASK]) {
> >+ filter->mark.mask =
> >+ ntohl(nla_get_be32(cda[CTA_MARK_MASK]));
> >+ }
> >+#endif
> > return netlink_dump_start(ctnl, skb, nlh, ctnetlink_dump_table,
> >- ctnetlink_done, NULL, 0);
> >+ ctnetlink_done, filter, 0);
> >+ }
>
> I had thought of the following before your patch came up:
>
> ctnl_dump_any(skb,cb)
> {
> ...loop over CTs...
> }
> ctnl_dump_foo(skb,cb)
> {
> if (cb->args[0] == NULL) {
> cb->args[0] = filter = kzalloc(sizeof(struct ctnl_dump_filter));
> if (cb->nlh has CTA_MARK) /* [1] */
> filter->mark.value = ...
I thought about this at first instance, but I still think that
allowing to pass data to the dump callback makes sense. It's quite
common to provide some interface to pass data pointer to callbacks.
> }
> return ctnl_dump_any(skb,cb);
> }
> ctnl_dump_bar(skb,cb)
> {
> if (cb->args[0] == NULL) {
> cb->args[0] = somethingelse;
> }
> return ctnl_dump_any(skb,cb);
> }
> ctnetlink_get_foo(ctnl,skb,...)
> {
> netlink_dump_start(ctnl,skb,nlh,ctnl_dump_foo,ctnl_done,0);
> }
> ctnetlink_get_bar(ctnl,skb,...)
> {
> netlink_dump_start(ctnl,skb,nlh,ctnl_dump_bar,ctnl_done,0);
> }
>
> [1]: Arguably needs a way to put cda into cb.
The main problem is that cda is allocated in the stack. We'd need to
allocate the array in the heap instead. Then, pass it to the dump_cb.
Then, we would need to retrieve the value from the attribute, that
would be slowier than this approach.
Thanks for your comments.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-02-25 13:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-24 22:14 [PATCH 0/2] netlink: netlink_dump_start takes pointer to data pablo
2012-02-24 22:14 ` [PATCH 1/2] netlink: netlink_dump_start may take data pointer for callbacks pablo
2012-02-24 22:47 ` David Miller
2012-02-24 23:18 ` Pablo Neira Ayuso
2012-02-24 22:14 ` [PATCH 2/2] netfilter: ctnetlink: support kernel-space dump filterings pablo
2012-02-25 1:09 ` Jan Engelhardt
2012-02-25 13:26 ` 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).