Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Liping Zhang]

Hi Pablo,

2017-06-06 8:04 GMT+08:00 Pablo Neira Ayuso :
[...]
>> I remembered Pablo told me that the ct helpers "is probably one of
>> the remaining subsystems not having netns support", when I sent
>> patches to fix other issues.
>>
>> So I try to accomplish the netns support for ct helpers.
>> (see https://patchwork.ozlabs.org/patch/740692/).
>
> I was referring to cthelper infrastructure, right? So you add
> possible_net to struct nfnl_cthelper?

Do you mean that we only need to support netns for these user
ct helpers? For these kernel built-in ct helpers, we should keep
them unchanged.

So for __nf_conntrack_helper_find, only when the
NF_CT_HELPER_F_USERSPACE is set, we should check the
netns is equal or not, like this:

static bool
nf_ct_helper_net_eq(struct nf_conntrack_helper *helper, struct net *net)
{
struct nfnl_cthelper *nlhelper;

if (!(helper->flags & NF_CT_HELPER_F_USERSPACE))
return true;

nlhelper = container_of(helper, struct nfnl_cthelper, helper);
return net_eq(net, read_pnet(>net));
}
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Pablo Neira Ayuso]

On Mon, Jun 05, 2017 at 01:11:19PM +0800, Liping Zhang wrote:
> Hi Florian & Pablo,
> 
> 2017-06-05 0:07 GMT+08:00 Florian Westphal :
> > Liping Zhang  wrote:
> >> This patch set aims to add net namespace support for the ct helper,
> >> it is a little large, but I try my best to split them to a relative
> >> smaller patches, which will help to review. Comments are welcome.
> >
> > Why?  Could you explain what kind of functionality is added here, or
> > what problem is fixed?
> >
> > Why do we need per netns complexity for helpers?
> 
> I remembered Pablo told me that the ct helpers "is probably one of
> the remaining subsystems not having netns support", when I sent
> patches to fix other issues.
>
> So I try to accomplish the netns support for ct helpers.
> (see https://patchwork.ozlabs.org/patch/740692/).

I was referring to cthelper infrastructure, right? So you add
possible_net to struct nfnl_cthelper?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Liping Zhang]

Hi Florian & Pablo,

2017-06-05 0:07 GMT+08:00 Florian Westphal :
> Liping Zhang  wrote:
>> This patch set aims to add net namespace support for the ct helper,
>> it is a little large, but I try my best to split them to a relative
>> smaller patches, which will help to review. Comments are welcome.
>
> Why?  Could you explain what kind of functionality is added here, or
> what problem is fixed?
>
> Why do we need per netns complexity for helpers?

I remembered Pablo told me that the ct helpers "is probably one of
the remaining subsystems not having netns support", when I sent
patches to fix other issues.

So I try to accomplish the netns support for ct helpers.
(see https://patchwork.ozlabs.org/patch/740692/).

For these user ct helpers, after per netns support, we can config
different policy to these ct helpers with the same name.(But indeed,
this flexible seems less valuable, we can accomplish it in different
ways).

For these kernel built-in ct helpers, per netns support is indeed
unnecessary. Especially after Florian's patch: "netns: add and
use net_ns_barrier".

Anyway, I have no objection to drop this patch set, as it increased
too much complexity but earned a very little.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Pablo Neira Ayuso]

On Sun, Jun 04, 2017 at 06:07:53PM +0200, Florian Westphal wrote:
> Liping Zhang  wrote:
> > This patch set aims to add net namespace support for the ct helper,
> > it is a little large, but I try my best to split them to a relative
> > smaller patches, which will help to review. Comments are welcome.
> 
> Why?  Could you explain what kind of functionality is added here, or
> what problem is fixed?
> 
> Why do we need per netns complexity for helpers?

I agree. We don't need this now that we have the new nft helper
frontend in place that Florian implemented.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Florian Westphal]

Liping Zhang  wrote:
> This patch set aims to add net namespace support for the ct helper,
> it is a little large, but I try my best to split them to a relative
> smaller patches, which will help to review. Comments are welcome.

Why?  Could you explain what kind of functionality is added here, or
what problem is fixed?

Why do we need per netns complexity for helpers?

I'm fine with patch #1 of course.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH nf-next RFC 0/5] netfilter: add net namespace support for cthelper

[Liping Zhang]

This patch set aims to add net namespace support for the ct helper,
it is a little large, but I try my best to split them to a relative
smaller patches, which will help to review. Comments are welcome.

Actually, I split the whole thing into 4 parts:
1. make nf_conntrack_helper_register become per-net, this is
   done by patch #1 and #2
2. filter ct helper by netns, done by patch #3
3. support netns for the user cthelper by patch #4
4. support netns for the kernel built-in cthelper, done by patch #5

Last, I use the following commands to do test, running about 0.5 hour.
And no exceptions found, i.e. no OOPS or no memory leak reported:

while : ; do
  ip netns add test1
  ip netns exec test1 nfct add helper ssdp inet udp
  ip netns exec test1 iptables -w -t raw -I OUTPUT -p udp -j CT --helper ssdp
  ip netns delete test1
done

while : ; do
  modprobe nf_conntrack_sip "ports=111,222"
  modprobe nf_conntrack_ftp "ports=444,555"
  modprobe nf_conntrack_tftp
  ip netns add test2
  ip netns exec test2 iptables -w -t raw -I OUTPUT -p tcp -j CT --helper ftp-444
  ip netns delete test2
  iptables -w -t raw -I OUTPUT -p udp -j CT --helper sip-0
  iptables -w -t raw -D OUTPUT -p udp -j CT --helper sip-0
  rmmod nf_conntrack_sip
  rmmod nf_conntrack_ftp
  rmmod nf_conntrack_tftp
done

Liping Zhang (5):
  netfilter: use nf_conntrack_helpers_register when possible
  netfilter: make nf_conntrack_helper_register become per-net
  netfilter: make each ct helper belong to a specific netns
  netfilter: complete the netns support for the user cthelpers
  netfilter: complete the netns support for the kernel built-in
cthelpers

 include/net/netfilter/nf_conntrack_helper.h |  44 ++--
 include/net/netns/conntrack.h   |   5 +
 net/ipv4/netfilter/nf_nat_snmp_basic.c  |  19 +++-
 net/netfilter/nf_conntrack_amanda.c |  27 +++--
 net/netfilter/nf_conntrack_ftp.c|  19 +++-
 net/netfilter/nf_conntrack_h323_main.c  |  70 -
 net/netfilter/nf_conntrack_helper.c | 155 ++--
 net/netfilter/nf_conntrack_irc.c|  19 +++-
 net/netfilter/nf_conntrack_netbios_ns.c |  19 +++-
 net/netfilter/nf_conntrack_netlink.c|  15 +--
 net/netfilter/nf_conntrack_pptp.c   |  19 +++-
 net/netfilter/nf_conntrack_sane.c   |  19 +++-
 net/netfilter/nf_conntrack_sip.c|  19 +++-
 net/netfilter/nf_conntrack_snmp.c   |  19 +++-
 net/netfilter/nf_conntrack_tftp.c   |  19 +++-
 net/netfilter/nfnetlink_cthelper.c  |  70 +
 net/netfilter/nft_ct.c  |  12 ++-
 net/netfilter/xt_CT.c   |   3 +-
 net/openvswitch/conntrack.c |   7 +-
 19 files changed, 452 insertions(+), 127 deletions(-)

-- 
2.5.5


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html