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; > > As said, if you do this in ctnetlink_conntrack_event, I think that you > can also remove the rcu_read_lock in ctnetlink_dump_helpinfo, as then > all dump functions will be invoked under rcu_read_lock. I modified the patch with your suggestions. Read locks have been removed from all dump functions. > In ctnetlink_get_conntrack, I think that ctnetlink_fill_info needs to be > protected with rcu_read_lock. ctnetlink_get_conntrack now contains a read lock to protect the dump functions. Protecting it outside is therefore not necessary. 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..696649e 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_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