| Message ID | 20151007042550.GC23203@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Oct 07, 2015 at 01:25:50PM +0900, Ken-ichirou MATSUZAWA wrote: > This patch enables to load nf_conntrack_netlink module if > NFULNL_CFG_F_CONNTRACK config flag is specified. Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 12, 2015 at 07:13:52PM +0200, Pablo Neira Ayuso wrote: > On Wed, Oct 07, 2015 at 01:25:50PM +0900, Ken-ichirou MATSUZAWA wrote: > > This patch enables to load nf_conntrack_netlink module if > > NFULNL_CFG_F_CONNTRACK config flag is specified. > > Applied, thanks. I just noticed we're breaking atomicity when configuring things. I mean, if we fail to configure the flags, we return an error but it's too late, we just create the instance and we have configured everything. I quickly made a couple of patches here, we'll be posting them tomorrow. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 07, 2015 at 01:25:50PM +0900, Ken-ichirou MATSUZAWA wrote: > This patch enables to load nf_conntrack_netlink module if > NFULNL_CFG_F_CONNTRACK config flag is specified. > > Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> > --- > net/netfilter/nfnetlink_log.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c > index e1d1187..f8d9bd8 100644 > --- a/net/netfilter/nfnetlink_log.c > +++ b/net/netfilter/nfnetlink_log.c > @@ -927,7 +927,16 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb, > } > > if (flags & NFULNL_CFG_F_CONNTRACK && > - rcu_access_pointer(nfnl_ct_hook) == NULL) { > + !rcu_access_pointer(nfnl_ct_hook)) { > +#ifdef CONFIG_MODULES > + nfnl_unlock(NFNL_SUBSYS_ULOG); > + request_module("ip_conntrack_netlink"); > + nfnl_lock(NFNL_SUBSYS_ULOG); > + if (rcu_access_pointer(nfnl_ct_hook)) { > + ret = -EAGAIN; > + goto out; > + } > +#endif I know this is not your fault, but could you have a look to see if we can get resolved the broken atomicity problems in first place when applying configuration updates to nfnetlink_queue in a similar way to what I did for nfnetlink_log? I mean, move code that check for things right up the beginning of the functions, then we pass the preparation phase and we're good, we apply the configuration changes. Again not your fault, but there's another nasty thing in that code: We can destroy a queue in NFQNL_CFG_CMD_UNBIND, then we can keep going on configuring it when it is actually destroyed. This is working only by luck because we're using call_rcu so the object is still there, but this is sloppy and it should be fixed. After having that resolved, we can go back to adding module autoload. Let me know, Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sorry for being late. On Fri, Oct 16, 2015 at 07:05:32PM +0200, Pablo Neira Ayuso wrote: > can get resolved the broken atomicity problems in first place when > applying configuration updates to nfnetlink_queue in a similar way to > what I did for nfnetlink_log? I tried it, would you review the following patches? > can destroy a queue in NFQNL_CFG_CMD_UNBIND, then we can keep going on > configuring it when it is actually destroyed. This is working only by I'm wondering how we handle obsolete commands NFQNL_CFG_CMD_PF_BIND / UNBIND, especially with a valid queue_num and configurations. These patches follows current behavior, do nothing, ignores another nlas and just returns success. Should I update patches to handle configuraion nlas with obsolute commands? Thanks, -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index e1d1187..f8d9bd8 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -927,7 +927,16 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb, } if (flags & NFULNL_CFG_F_CONNTRACK && - rcu_access_pointer(nfnl_ct_hook) == NULL) { + !rcu_access_pointer(nfnl_ct_hook)) { +#ifdef CONFIG_MODULES + nfnl_unlock(NFNL_SUBSYS_ULOG); + request_module("ip_conntrack_netlink"); + nfnl_lock(NFNL_SUBSYS_ULOG); + if (rcu_access_pointer(nfnl_ct_hook)) { + ret = -EAGAIN; + goto out; + } +#endif ret = -EOPNOTSUPP; goto out; }
This patch enables to load nf_conntrack_netlink module if NFULNL_CFG_F_CONNTRACK config flag is specified. Signed-off-by: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp> --- net/netfilter/nfnetlink_log.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)