On Mon, 2008-07-28 at 20:31 +0200, Pablo Neira Ayuso wrote:
> Pablo Neira Ayuso wrote:
> > Pablo Neira Ayuso wrote:
> >> Or much simpler, just call read_rcu_unlock() before the first
> >> nla_nest_start() so that this results in much smaller patch:
> >>
> >> nlmsg_failure:
> >> nla_put_failure:
> >> read_rcu_unlock(); <---
> >> nlmsg_trim(skb, b);
> >> return -1;
>
> Sorry, this is wrong. It should be:
>
> nla_put_failure:
> read_rcu_unlock();
> nlmsg_failure:
> nlmsg_trim(skb, b);
> return -1;
Very true indeed. Thanks for noticing. The nlmsg_failure is kinda hidden
in the macro and the jump targets were in the wrong order. You find the
corrected version below.
nf_ctnetlink: Remove read locks from dump functions to increase
performance in the event notification path
Signed-off-by: Fabian Hugelshofer <hugelshofer2006@xxxxxx>
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 95a7967..06f69a2 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -104,16 +104,14 @@ ctnetlink_dump_tuples(struct sk_buff *skb,
struct nf_conntrack_l3proto *l3proto;
struct nf_conntrack_l4proto *l4proto;
- l3proto = nf_ct_l3proto_find_get(tuple->src.l3num);
+ l3proto = __nf_ct_l3proto_find(tuple->src.l3num);
ret = ctnetlink_dump_tuples_ip(skb, tuple, l3proto);
- nf_ct_l3proto_put(l3proto);
if (unlikely(ret < 0))
return ret;
- l4proto = nf_ct_l4proto_find_get(tuple->src.l3num, tuple->dst.protonum);
+ l4proto = __nf_ct_l4proto_find(tuple->src.l3num, tuple->dst.protonum);
ret = ctnetlink_dump_tuples_proto(skb, tuple, l4proto);
- nf_ct_l4proto_put(l4proto);
return ret;
}
@@ -150,9 +148,8 @@ ctnetlink_dump_protoinfo(struct sk_buff *skb, const struct nf_conn *ct)
struct nlattr *nest_proto;
int ret;
- l4proto = nf_ct_l4proto_find_get(nf_ct_l3num(ct), nf_ct_protonum(ct));
+ l4proto = __nf_ct_l4proto_find(nf_ct_l3num(ct), nf_ct_protonum(ct));
if (!l4proto->to_nlattr) {
- nf_ct_l4proto_put(l4proto);
return 0;
}
@@ -162,14 +159,11 @@ ctnetlink_dump_protoinfo(struct sk_buff *skb, const struct nf_conn *ct)
ret = l4proto->to_nlattr(skb, nest_proto, ct);
- nf_ct_l4proto_put(l4proto);
-
nla_nest_end(skb, nest_proto);
return ret;
nla_put_failure:
- nf_ct_l4proto_put(l4proto);
return -1;
}
@@ -183,7 +177,6 @@ ctnetlink_dump_helpinfo(struct sk_buff *skb, const struct nf_conn *ct)
if (!help)
return 0;
- rcu_read_lock();
helper = rcu_dereference(help->helper);
if (!helper)
goto out;
@@ -198,11 +191,9 @@ ctnetlink_dump_helpinfo(struct sk_buff *skb, const struct nf_conn *ct)
nla_nest_end(skb, nest_helper);
out:
- rcu_read_unlock();
return 0;
nla_put_failure:
- rcu_read_unlock();
return -1;
}
@@ -374,6 +365,8 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
nfmsg->version = NFNETLINK_V0;
nfmsg->res_id = 0;
+ rcu_read_lock();
+
nest_parms = nla_nest_start(skb, CTA_TUPLE_ORIG | NLA_F_NESTED);
if (!nest_parms)
goto nla_put_failure;
@@ -402,11 +395,14 @@ ctnetlink_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
ctnetlink_dump_nat_seq_adj(skb, ct) < 0)
goto nla_put_failure;
+ rcu_read_unlock();
+
nlh->nlmsg_len = skb_tail_pointer(skb) - b;
return skb->len;
-nlmsg_failure:
nla_put_failure:
+ rcu_read_unlock();
+nlmsg_failure:
nlmsg_trim(skb, b);
return -1;
}
@@ -1285,16 +1281,19 @@ ctnetlink_exp_dump_tuple(struct sk_buff *skb,
{
struct nlattr *nest_parms;
+ rcu_read_lock();
nest_parms = nla_nest_start(skb, type | NLA_F_NESTED);
if (!nest_parms)
goto nla_put_failure;
if (ctnetlink_dump_tuples(skb, tuple) < 0)
goto nla_put_failure;
nla_nest_end(skb, nest_parms);
+ rcu_read_unlock();
return 0;
nla_put_failure:
+ rcu_read_unlock();
return -1;
}
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
