headers
Eric W. Biederman | 1 May 2012 20:47
Favicon

Re: [PATCH v2 00/17] netfilter: add namespace support for netfilter protos

Gao feng <gaofeng <at> cn.fujitsu.com> writes:

> Currently the sysctl of netfilter proto is not isolated, so when
> changing proto's sysctl in container will cause the host's sysctl
> be changed too. it's not expected.
>
> This patch set adds the namespace support for netfilter protos.
>
> impletement four pernet_operations to register sysctl and initial
> pernet data for proto.
>
> -ipv4_net_ops is used to register tcp4(compat),
>  udp4(compat),icmp(compat),ipv4(compat).
> -ipv6_net_ops is used to register tcp6,udp6 and icmpv6.
> -sctp_net_ops is used to register sctp4(compat) and sctp6.
> -udplite_net_ops is used to register udplite4 and udplite6
>
> extern l[3,4]proto (sysctl) register functions to make them support
> namespace.
>
> finailly add namespace support for cttimeout.

I am a bit out of it this week so I could not look at these patches
in the detail that I would like.  However skimming through it looks
like you addressed your review comments, and the changes look like
the kind of changes I would expect from something like this.

I assume you have tested to make certain your code actually works.

So on that basis for the patchset:
(Continue reading)

Permalink | Reply | 1 comment |
headers
Pablo Neira Ayuso | 2 May 2012 02:34
Favicon

Re: [v12 PATCH 2/3] NETFILTER module xt_hmark, new target for HASH based fwmark

Hi Hans,

I have decided to take your patch and give it one spin today.

Please, find it attached. The main things I've done are:

* splitting the code into smaller functions, thus, it becomes more
  maintainable.

* try to put common code into functions, eg. the layer 4 protocol
  parsing to obtain the ports is the same for both IPv4 and IPv6.

* adding the hmark_tuple abstraction, cleaner than using several
  variables to set the address, ports, and so on. Thus, we only pass
  one single pointer to it.

* I have removed most of the comments, they bloat the file and most
  information can be extracted by reading the code. I only left the
  comments that clarify "strange" things.

Regarding ICMP traffic, I think we can use the ID field for the
hashing as well. Thus, we handle ICMP like other protocols.

Please, I'd appreciate if you can test and spot issues after my
rework. I have slightly tested here.

I may make some minor cleanup on it before submission but, in that
case, in that case, I'll post the patch. I would not expect more major
changes in it.
(Continue reading)

Permalink | Reply | 15 comments |
headers
Pablo Neira Ayuso | 2 May 2012 02:40
Favicon

Re: [PATCH v2 00/17] netfilter: add namespace support for netfilter protos

Hi Eric,

On Tue, May 01, 2012 at 11:47:45AM -0700, Eric W. Biederman wrote:
> Gao feng <gaofeng <at> cn.fujitsu.com> writes:
> 
> > Currently the sysctl of netfilter proto is not isolated, so when
> > changing proto's sysctl in container will cause the host's sysctl
> > be changed too. it's not expected.
> >
> > This patch set adds the namespace support for netfilter protos.
> >
> > impletement four pernet_operations to register sysctl and initial
> > pernet data for proto.
> >
> > -ipv4_net_ops is used to register tcp4(compat),
> >  udp4(compat),icmp(compat),ipv4(compat).
> > -ipv6_net_ops is used to register tcp6,udp6 and icmpv6.
> > -sctp_net_ops is used to register sctp4(compat) and sctp6.
> > -udplite_net_ops is used to register udplite4 and udplite6
> >
> > extern l[3,4]proto (sysctl) register functions to make them support
> > namespace.
> >
> > finailly add namespace support for cttimeout.
> 
> I am a bit out of it this week so I could not look at these patches
> in the detail that I would like.  However skimming through it looks
> like you addressed your review comments, and the changes look like
> the kind of changes I would expect from something like this.
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Pablo Neira Ayuso | 2 May 2012 02:51
Favicon

Re: [PATCH RFC 2/2] netfilter: conntrack: replace mutex with cmpxchg

On Fri, Apr 27, 2012 at 02:28:53PM -0400, Benjamin Poirier wrote:
> This mutex protects a single pointer.

You seem to be using an old Linux kernel tree. This doesn't apply.

> ---
>  net/netfilter/nf_conntrack_ecache.c |   38 +++++++++-------------------------
>  1 files changed, 10 insertions(+), 28 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
> index 0134009..603eb69 100644
> --- a/net/netfilter/nf_conntrack_ecache.c
> +++ b/net/netfilter/nf_conntrack_ecache.c
>  <at>  <at>  -25,8 +25,6  <at>  <at> 
>  #include <net/netfilter/nf_conntrack_core.h>
>  #include <net/netfilter/nf_conntrack_extend.h>
>  
> -static DEFINE_MUTEX(nf_ct_ecache_mutex);
> -
>  /* deliver cached events and clear cache entry - must be called with locally
>   * disabled softirqs */
>  void nf_ct_deliver_cached_events(struct nf_conn *ct)
>  <at>  <at>  -80,52 +78,36  <at>  <at>  EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
>  int nf_conntrack_register_notifier(struct net *net,
>  				   struct nf_ct_event_notifier *new)
>  {
> -	int ret = 0;
> -
> -	mutex_lock(&nf_ct_ecache_mutex);
> -	if (net->ct.nf_conntrack_event_cb != NULL)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:24
Picon
Gravatar

[PATCH 04/17] ipvs: WRR scheduler does not need GFP_ATOMIC allocation

From: Julian Anastasov <ja <at> ssi.bg>

	Schedulers are initialized and bound to services only
on commands.

Signed-off-by: Julian Anastasov <ja <at> ssi.bg>
Signed-off-by: Hans Schillstrom <hans <at> schillstrom.com>
Signed-off-by: Simon Horman <horms <at> verge.net.au>
---
 net/netfilter/ipvs/ip_vs_wrr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_wrr.c b/net/netfilter/ipvs/ip_vs_wrr.c
index fd0d4e0..231be7d 100644
--- a/net/netfilter/ipvs/ip_vs_wrr.c
+++ b/net/netfilter/ipvs/ip_vs_wrr.c
 <at>  <at>  -84,7 +84,7  <at>  <at>  static int ip_vs_wrr_init_svc(struct ip_vs_service *svc)
 	/*
 	 *    Allocate the mark variable for WRR scheduling
 	 */
-	mark = kmalloc(sizeof(struct ip_vs_wrr_mark), GFP_ATOMIC);
+	mark = kmalloc(sizeof(struct ip_vs_wrr_mark), GFP_KERNEL);
 	if (mark == NULL)
 		return -ENOMEM;

--

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:25
Picon
Gravatar

[PATCH 17/17] IPVS: ip_vs_proto.c: local functions should not be exposed globally

From: H Hartley Sweeten <hartleys <at> visionengravers.com>

Functions not referenced outside of a source file should be marked
static to prevent it from being exposed globally.

This quiets the sparse warnings:

warning: symbol '__ipvs_proto_data_get' was not declared. Should it be static?

Signed-off-by: H Hartley Sweeten <hsweeten <at> visionengravers.com>
Cc: "David S. Miller" <davem <at> davemloft.net>
Signed-off-by: Simon Horman <horms <at> verge.net.au>
---
 net/netfilter/ipvs/ip_vs_proto.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index 8726488..e3f4bb0 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
 <at>  <at>  -153,7 +153,7  <at>  <at>  EXPORT_SYMBOL(ip_vs_proto_get);
 /*
  *	get ip_vs_protocol object data by netns and proto
  */
-struct ip_vs_proto_data *
+static struct ip_vs_proto_data *
 __ipvs_proto_data_get(struct netns_ipvs *ipvs, unsigned short proto)
 {
 	struct ip_vs_proto_data *pd;
--

--
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:24
Picon
Gravatar

[PATCH 10/17] ipvs: fix ip_vs_try_bind_dest to rebind app and transmitter

From: Julian Anastasov <ja <at> ssi.bg>

	Initially, when the synced connection is created we
use the forwarding method provided by master but once we
bind to destination it can be changed. As result, we must
update the application and the transmitter.

	As ip_vs_try_bind_dest is called always for connections
that require dest binding, there is no need to validate the
cp and dest pointers.

Signed-off-by: Julian Anastasov <ja <at> ssi.bg>
Signed-off-by: Simon Horman <horms <at> verge.net.au>
---
 net/netfilter/ipvs/ip_vs_conn.c |   33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 7647f3b..9d237d7 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
 <at>  <at>  -612,14 +612,33  <at>  <at>  struct ip_vs_dest *ip_vs_try_bind_dest(struct ip_vs_conn *cp)
 {
 	struct ip_vs_dest *dest;

-	if ((cp) && (!cp->dest)) {
-		dest = ip_vs_find_dest(ip_vs_conn_net(cp), cp->af, &cp->daddr,
-				       cp->dport, &cp->vaddr, cp->vport,
-				       cp->protocol, cp->fwmark, cp->flags);
+	dest = ip_vs_find_dest(ip_vs_conn_net(cp), cp->af, &cp->daddr,
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:24
Picon
Gravatar

[PATCH 08/17] ipvs: ignore IP_VS_CONN_F_NOOUTPUT in backup server

From: Julian Anastasov <ja <at> ssi.bg>

	As IP_VS_CONN_F_NOOUTPUT is derived from the
forwarding method we should get it from conn_flags just
like we do it for IP_VS_CONN_F_FWD_MASK bits when binding
to real server.

Signed-off-by: Julian Anastasov <ja <at> ssi.bg>
Signed-off-by: Simon Horman <horms <at> verge.net.au>
---
 net/netfilter/ipvs/ip_vs_conn.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 4a09b78..f562e63 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
 <at>  <at>  -567,7 +567,7  <at>  <at>  ip_vs_bind_dest(struct ip_vs_conn *cp, struct ip_vs_dest *dest)
 		if (!(cp->flags & IP_VS_CONN_F_TEMPLATE))
 			conn_flags &= ~IP_VS_CONN_F_INACTIVE;
 		/* connections inherit forwarding method from dest */
-		cp->flags &= ~IP_VS_CONN_F_FWD_MASK;
+		cp->flags &= ~(IP_VS_CONN_F_FWD_MASK | IP_VS_CONN_F_NOOUTPUT);
 	}
 	cp->flags |= conn_flags;
 	cp->dest = dest;
--

-- 
1.7.9.5

--
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:24
Picon
Gravatar

[PATCH 01/17] ipvs: timeout tables do not need GFP_ATOMIC allocation

From: Julian Anastasov <ja <at> ssi.bg>

	They are called only on initialization.

Signed-off-by: Julian Anastasov <ja <at> ssi.bg>
Signed-off-by: Hans Schillstrom <hans <at> schillstrom.com>
Signed-off-by: Simon Horman <horms <at> verge.net.au>
---
 net/netfilter/ipvs/ip_vs_proto.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index 6eda11d..a981b7c 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
 <at>  <at>  -196,7 +196,7  <at>  <at>  void ip_vs_protocol_timeout_change(struct netns_ipvs *ipvs, int flags)
 int *
 ip_vs_create_timeout_table(int *table, int size)
 {
-	return kmemdup(table, size, GFP_ATOMIC);
+	return kmemdup(table, size, GFP_KERNEL);
 }

 
--

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
(Continue reading)

Permalink | Reply | 0 comments |
headers
Simon Horman | 2 May 2012 03:19
Picon
Gravatar

Re: [PATCH v3 2/2] netfilter: ipvs: use GFP_KERNEL allocation where possible

On Sat, Apr 14, 2012 at 12:37:47PM -0400, Sasha Levin wrote:
> Use GFP_KERNEL instead of GFP_ATOMIC when registering an ipvs protocol.
> 
> This is safe since it will always run from a process context.

For the benefit of patchwork, this patch is in ipvs-next.

Thanks again for the good work.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |

Gmane