* [PATCH 0/2] [RFC] Get MASQUERADE target to handle routing changes @ 2012-11-13 20:17 Jozsef Kadlecsik 2012-11-13 20:17 ` [PATCH 1/2] Introduce notification chain for " Jozsef Kadlecsik 2012-11-13 20:17 ` [PATCH 2/2] Handle the routing changes in the MASQUERADE target Jozsef Kadlecsik 0 siblings, 2 replies; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-13 20:17 UTC (permalink / raw) To: netfilter-devel The MASQUERADE target does not handle the cases when the routing changes. (See thread "UDP packets sent with wrong source address after routing change [AV#3431]"). The first patch introduces a new in-kernel notification chain for the routing changes. The second one registers the MASQUERADE target to this events and adds the new "--route-dependent" flag (actually, the value of the flag) and conntrack flag to mark conntrack entries which may be affected by routing changes. As the first step, when routing changes, marked entries are simply deleted. Best regards, Jozsef Jozsef Kadlecsik (2): Introduce notification chain for routing changes Handle the routing changes in the MASQUERADE target include/linux/inetdevice.h | 2 + include/linux/netdevice.h | 1 + include/uapi/linux/netfilter/nf_conntrack_common.h | 4 ++ include/uapi/linux/netfilter/nf_nat.h | 1 + net/ipv4/fib_trie.c | 18 +++++++++ net/ipv4/netfilter/ipt_MASQUERADE.c | 40 ++++++++++++++++++++ 6 files changed, 66 insertions(+), 0 deletions(-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Introduce notification chain for routing changes 2012-11-13 20:17 [PATCH 0/2] [RFC] Get MASQUERADE target to handle routing changes Jozsef Kadlecsik @ 2012-11-13 20:17 ` Jozsef Kadlecsik 2012-11-13 22:15 ` David Miller 2012-11-13 20:17 ` [PATCH 2/2] Handle the routing changes in the MASQUERADE target Jozsef Kadlecsik 1 sibling, 1 reply; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-13 20:17 UTC (permalink / raw) To: netfilter-devel; +Cc: Jozsef Kadlecsik Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> --- include/linux/inetdevice.h | 2 ++ include/linux/netdevice.h | 1 + net/ipv4/fib_trie.c | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 0 deletions(-) diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h index d032780..cf16dab 100644 --- a/include/linux/inetdevice.h +++ b/include/linux/inetdevice.h @@ -170,6 +170,8 @@ struct in_ifaddr { extern int register_inetaddr_notifier(struct notifier_block *nb); extern int unregister_inetaddr_notifier(struct notifier_block *nb); +extern int register_iproute_notifier(struct notifier_block *nb); +extern int unregister_iproute_notifier(struct notifier_block *nb); extern struct net_device *__ip_dev_find(struct net *net, __be32 addr, bool devref); static inline struct net_device *ip_dev_find(struct net *net, __be32 addr) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index f8eda02..f61b5f4 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1540,6 +1540,7 @@ struct packet_type { #define NETDEV_RELEASE 0x0012 #define NETDEV_NOTIFY_PEERS 0x0013 #define NETDEV_JOIN 0x0014 +#define NETDEV_ROUTE_CHANGED 0x0015 extern int register_netdevice_notifier(struct notifier_block *nb); extern int unregister_netdevice_notifier(struct notifier_block *nb); diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c index 31d771c..5caf26b 100644 --- a/net/ipv4/fib_trie.c +++ b/net/ipv4/fib_trie.c @@ -73,6 +73,7 @@ #include <linux/slab.h> #include <linux/prefetch.h> #include <linux/export.h> +#include <linux/notifier.h> #include <net/net_namespace.h> #include <net/ip.h> #include <net/protocol.h> @@ -178,6 +179,8 @@ static const int sync_pages = 128; static struct kmem_cache *fn_alias_kmem __read_mostly; static struct kmem_cache *trie_leaf_kmem __read_mostly; +static BLOCKING_NOTIFIER_HEAD(iproute_chain); + /* * caller must hold RTNL */ @@ -1337,6 +1340,8 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg) rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen, tb->tb_id, &cfg->fc_nlinfo, 0); succeeded: + blocking_notifier_call_chain(&iproute_chain, + NETDEV_ROUTE_CHANGED, fi); return 0; out_free_new_fa: @@ -1713,6 +1718,8 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg) if (fa->fa_state & FA_S_ACCESSED) rt_cache_flush(cfg->fc_nlinfo.nl_net); + blocking_notifier_call_chain(&iproute_chain, + NETDEV_ROUTE_CHANGED, fa->fa_info); fib_release_info(fa->fa_info); alias_free_mem_rcu(fa); return 0; @@ -1979,6 +1986,17 @@ void __init fib_trie_init(void) 0, SLAB_PANIC, NULL); } +int register_iproute_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_register(&iproute_chain, nb); +} +EXPORT_SYMBOL(register_iproute_notifier); + +int unregister_iproute_notifier(struct notifier_block *nb) +{ + return blocking_notifier_chain_unregister(&iproute_chain, nb); +} +EXPORT_SYMBOL(unregister_iproute_notifier); struct fib_table *fib_trie_table(u32 id) { -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Introduce notification chain for routing changes 2012-11-13 20:17 ` [PATCH 1/2] Introduce notification chain for " Jozsef Kadlecsik @ 2012-11-13 22:15 ` David Miller 2012-11-14 7:57 ` Jozsef Kadlecsik 0 siblings, 1 reply; 11+ messages in thread From: David Miller @ 2012-11-13 22:15 UTC (permalink / raw) To: kadlec; +Cc: netfilter-devel From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> Date: Tue, 13 Nov 2012 21:17:36 +0100 > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > --- > include/linux/inetdevice.h | 2 ++ > include/linux/netdevice.h | 1 + > net/ipv4/fib_trie.c | 18 ++++++++++++++++++ > 3 files changed, 21 insertions(+), 0 deletions(-) A change like this needs to be reviewed on netdev, not netfilter-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Introduce notification chain for routing changes 2012-11-13 22:15 ` David Miller @ 2012-11-14 7:57 ` Jozsef Kadlecsik 0 siblings, 0 replies; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-14 7:57 UTC (permalink / raw) To: David Miller; +Cc: netfilter-devel On Tue, 13 Nov 2012, David Miller wrote: > From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > Date: Tue, 13 Nov 2012 21:17:36 +0100 > > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > --- > > include/linux/inetdevice.h | 2 ++ > > include/linux/netdevice.h | 1 + > > net/ipv4/fib_trie.c | 18 ++++++++++++++++++ > > 3 files changed, 21 insertions(+), 0 deletions(-) > > A change like this needs to be reviewed on netdev, not > netfilter-devel Yes, of course. I just wanted to put together a possible real solution which we could discuss further. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-13 20:17 [PATCH 0/2] [RFC] Get MASQUERADE target to handle routing changes Jozsef Kadlecsik 2012-11-13 20:17 ` [PATCH 1/2] Introduce notification chain for " Jozsef Kadlecsik @ 2012-11-13 20:17 ` Jozsef Kadlecsik 2012-11-13 23:49 ` Jan Engelhardt 2012-11-15 11:44 ` Pablo Neira Ayuso 1 sibling, 2 replies; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-13 20:17 UTC (permalink / raw) To: netfilter-devel; +Cc: Jozsef Kadlecsik When the routing changes, MASQUERADE should delete the conntrack entries where the source NATed address changes due to the routing change. As a first approximation, delete all entries which are marked with the new "--route-dependent" flag of the MASQUERADE target. Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> --- include/uapi/linux/netfilter/nf_conntrack_common.h | 4 ++ include/uapi/linux/netfilter/nf_nat.h | 1 + net/ipv4/netfilter/ipt_MASQUERADE.c | 40 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 0 deletions(-) diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h index 1644cdd..1c698b5 100644 --- a/include/uapi/linux/netfilter/nf_conntrack_common.h +++ b/include/uapi/linux/netfilter/nf_conntrack_common.h @@ -87,6 +87,10 @@ enum ip_conntrack_status { /* Conntrack got a helper explicitly attached via CT target. */ IPS_HELPER_BIT = 13, IPS_HELPER = (1 << IPS_HELPER_BIT), + + /* Conntrack must be deleted when routing changed (NAT) */ + IPS_ROUTING_DEPENDENT_BIT = 14, + IPS_ROUTING_DEPENDENT = (1 << IPS_ROUTING_DEPENDENT_BIT), }; /* Connection tracking event types */ diff --git a/include/uapi/linux/netfilter/nf_nat.h b/include/uapi/linux/netfilter/nf_nat.h index bf0cc37..a0dfac7 100644 --- a/include/uapi/linux/netfilter/nf_nat.h +++ b/include/uapi/linux/netfilter/nf_nat.h @@ -8,6 +8,7 @@ #define NF_NAT_RANGE_PROTO_SPECIFIED 2 #define NF_NAT_RANGE_PROTO_RANDOM 4 #define NF_NAT_RANGE_PERSISTENT 8 +#define NF_NAT_ROUTING_DEPENDENT 16 struct nf_nat_ipv4_range { unsigned int flags; diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c index 5d5d4d1..ecf3063 100644 --- a/net/ipv4/netfilter/ipt_MASQUERADE.c +++ b/net/ipv4/netfilter/ipt_MASQUERADE.c @@ -19,6 +19,7 @@ #include <net/ip.h> #include <net/checksum.h> #include <net/route.h> +#include <net/ip_fib.h> #include <linux/netfilter_ipv4.h> #include <linux/netfilter/x_tables.h> #include <net/netfilter/nf_nat.h> @@ -88,6 +89,9 @@ masquerade_tg(struct sk_buff *skb, const struct xt_action_param *par) newrange.min_proto = mr->range[0].min; newrange.max_proto = mr->range[0].max; + if (mr->range[0].flags & NF_NAT_ROUTING_DEPENDENT) + set_bit(IPS_ROUTING_DEPENDENT, &ct->status); + /* Hand modified range to generic setup. */ return nf_nat_setup_info(ct, &newrange, NF_NAT_MANIP_SRC); } @@ -132,6 +136,35 @@ static int masq_inet_event(struct notifier_block *this, return masq_device_event(this, event, dev); } +static int +route_cmp(struct nf_conn *i, const struct fib_info *fi) +{ + const struct nf_conn_nat *nat = nfct_nat(i); + + if (!nat) + return 0; + if (nf_ct_l3num(i) != NFPROTO_IPV4) + return 0; + if (!test_bit(IPS_ROUTING_DEPENDENT, &ct->status)) + return 0; + + /* We should check whether the SNAT address changed. */ + return 1; +} + +static int masq_route_event(struct notifier_block *this, + unsigned long event, + void *ptr) +{ + if (event == NETDEV_ROUTE_CHANGED) { + /* Routing changed, delete marked entries */ + nf_ct_iterate_cleanup(net, route_cmp, + (const struct fib_info)ptr); + } + + return NOTIFY_DONE; +} + static struct notifier_block masq_dev_notifier = { .notifier_call = masq_device_event, }; @@ -140,6 +173,10 @@ static struct notifier_block masq_inet_notifier = { .notifier_call = masq_inet_event, }; +static struct notifier_block masq_route_notifier = { + .notifier_call = masq_route_event, +}; + static struct xt_target masquerade_tg_reg __read_mostly = { .name = "MASQUERADE", .family = NFPROTO_IPV4, @@ -162,6 +199,8 @@ static int __init masquerade_tg_init(void) register_netdevice_notifier(&masq_dev_notifier); /* Register IP address change reports */ register_inetaddr_notifier(&masq_inet_notifier); + /* Register route change reports */ + register_iproute_notifier(&masq_route_notifier); } return ret; @@ -172,6 +211,7 @@ static void __exit masquerade_tg_exit(void) xt_unregister_target(&masquerade_tg_reg); unregister_netdevice_notifier(&masq_dev_notifier); unregister_inetaddr_notifier(&masq_inet_notifier); + unregister_iproute_notifier(&masq_route_notifier); } module_init(masquerade_tg_init); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-13 20:17 ` [PATCH 2/2] Handle the routing changes in the MASQUERADE target Jozsef Kadlecsik @ 2012-11-13 23:49 ` Jan Engelhardt 2012-11-14 7:55 ` Jozsef Kadlecsik 2012-11-15 11:44 ` Pablo Neira Ayuso 1 sibling, 1 reply; 11+ messages in thread From: Jan Engelhardt @ 2012-11-13 23:49 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: netfilter-devel On Tuesday 2012-11-13 21:17, Jozsef Kadlecsik wrote: >+static int >+route_cmp(struct nf_conn *i, const struct fib_info *fi) >+{ [...] >+} >+ >+static int masq_route_event(struct notifier_block *this, >+ unsigned long event, >+ void *ptr) >+{ >+ if (event == NETDEV_ROUTE_CHANGED) { >+ /* Routing changed, delete marked entries */ >+ nf_ct_iterate_cleanup(net, route_cmp, >+ (const struct fib_info)ptr); It would seem you forget a '*' near fib_info)ptr. The cast is pointless though, since ptr is already of type void * which nf_ct_iterate_cleanup expects. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-13 23:49 ` Jan Engelhardt @ 2012-11-14 7:55 ` Jozsef Kadlecsik 0 siblings, 0 replies; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-14 7:55 UTC (permalink / raw) To: Jan Engelhardt; +Cc: netfilter-devel On Wed, 14 Nov 2012, Jan Engelhardt wrote: > On Tuesday 2012-11-13 21:17, Jozsef Kadlecsik wrote: > > >+static int > >+route_cmp(struct nf_conn *i, const struct fib_info *fi) > >+{ [...] > >+} > >+ > >+static int masq_route_event(struct notifier_block *this, > >+ unsigned long event, > >+ void *ptr) > >+{ > >+ if (event == NETDEV_ROUTE_CHANGED) { > >+ /* Routing changed, delete marked entries */ > >+ nf_ct_iterate_cleanup(net, route_cmp, > >+ (const struct fib_info)ptr); > > It would seem you forget a '*' near fib_info)ptr. > The cast is pointless though, since ptr is already of type void * > which nf_ct_iterate_cleanup expects. You are right and it was not tested at all. Just a quick code to discuss not only in theory but something real. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-13 20:17 ` [PATCH 2/2] Handle the routing changes in the MASQUERADE target Jozsef Kadlecsik 2012-11-13 23:49 ` Jan Engelhardt @ 2012-11-15 11:44 ` Pablo Neira Ayuso 2012-11-15 14:42 ` Jozsef Kadlecsik 1 sibling, 1 reply; 11+ messages in thread From: Pablo Neira Ayuso @ 2012-11-15 11:44 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: netfilter-devel Hi Jozsef, Two comments on this patch: On Tue, Nov 13, 2012 at 09:17:37PM +0100, Jozsef Kadlecsik wrote: > When the routing changes, MASQUERADE should delete the conntrack > entries where the source NATed address changes due to the routing > change. As a first approximation, delete all entries which are > marked with the new "--route-dependent" flag of the MASQUERADE > target. > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > --- > include/uapi/linux/netfilter/nf_conntrack_common.h | 4 ++ > include/uapi/linux/netfilter/nf_nat.h | 1 + > net/ipv4/netfilter/ipt_MASQUERADE.c | 40 ++++++++++++++++++++ > 3 files changed, 45 insertions(+), 0 deletions(-) > > diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h > index 1644cdd..1c698b5 100644 > --- a/include/uapi/linux/netfilter/nf_conntrack_common.h > +++ b/include/uapi/linux/netfilter/nf_conntrack_common.h > @@ -87,6 +87,10 @@ enum ip_conntrack_status { > /* Conntrack got a helper explicitly attached via CT target. */ > IPS_HELPER_BIT = 13, > IPS_HELPER = (1 << IPS_HELPER_BIT), > + > + /* Conntrack must be deleted when routing changed (NAT) */ > + IPS_ROUTING_DEPENDENT_BIT = 14, > + IPS_ROUTING_DEPENDENT = (1 << IPS_ROUTING_DEPENDENT_BIT), This seems to me a bit too specific for the masquerade module. I've been checking the struct nf_conn_nat to squash that information there, but I don't find the way to make it without increasing the length of the NAT area. > }; > > /* Connection tracking event types */ > diff --git a/include/uapi/linux/netfilter/nf_nat.h b/include/uapi/linux/netfilter/nf_nat.h > index bf0cc37..a0dfac7 100644 > --- a/include/uapi/linux/netfilter/nf_nat.h > +++ b/include/uapi/linux/netfilter/nf_nat.h > @@ -8,6 +8,7 @@ > #define NF_NAT_RANGE_PROTO_SPECIFIED 2 > #define NF_NAT_RANGE_PROTO_RANDOM 4 > #define NF_NAT_RANGE_PERSISTENT 8 > +#define NF_NAT_ROUTING_DEPENDENT 16 > > struct nf_nat_ipv4_range { > unsigned int flags; > diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c > index 5d5d4d1..ecf3063 100644 > --- a/net/ipv4/netfilter/ipt_MASQUERADE.c > +++ b/net/ipv4/netfilter/ipt_MASQUERADE.c We now have IPv6 NAT support, so I guess you need to patch /net/ipv6/netfilter/ip6t_MASQUERADE.c Regards. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-15 11:44 ` Pablo Neira Ayuso @ 2012-11-15 14:42 ` Jozsef Kadlecsik 2012-11-16 10:09 ` Pablo Neira Ayuso 0 siblings, 1 reply; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-15 14:42 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel On Thu, 15 Nov 2012, Pablo Neira Ayuso wrote: > Two comments on this patch: > > On Tue, Nov 13, 2012 at 09:17:37PM +0100, Jozsef Kadlecsik wrote: > > When the routing changes, MASQUERADE should delete the conntrack > > entries where the source NATed address changes due to the routing > > change. As a first approximation, delete all entries which are > > marked with the new "--route-dependent" flag of the MASQUERADE > > target. > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > --- > > include/uapi/linux/netfilter/nf_conntrack_common.h | 4 ++ > > include/uapi/linux/netfilter/nf_nat.h | 1 + > > net/ipv4/netfilter/ipt_MASQUERADE.c | 40 ++++++++++++++++++++ > > 3 files changed, 45 insertions(+), 0 deletions(-) > > > > diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h > > index 1644cdd..1c698b5 100644 > > --- a/include/uapi/linux/netfilter/nf_conntrack_common.h > > +++ b/include/uapi/linux/netfilter/nf_conntrack_common.h > > @@ -87,6 +87,10 @@ enum ip_conntrack_status { > > /* Conntrack got a helper explicitly attached via CT target. */ > > IPS_HELPER_BIT = 13, > > IPS_HELPER = (1 << IPS_HELPER_BIT), > > + > > + /* Conntrack must be deleted when routing changed (NAT) */ > > + IPS_ROUTING_DEPENDENT_BIT = 14, > > + IPS_ROUTING_DEPENDENT = (1 << IPS_ROUTING_DEPENDENT_BIT), > > This seems to me a bit too specific for the masquerade module. I've > been checking the struct nf_conn_nat to squash that information there, > but I don't find the way to make it without increasing the length of > the NAT area. I know, we waste a status bit. But I couldn't find a better way to store the information in conntrack. > > }; > > > > /* Connection tracking event types */ > > diff --git a/include/uapi/linux/netfilter/nf_nat.h b/include/uapi/linux/netfilter/nf_nat.h > > index bf0cc37..a0dfac7 100644 > > --- a/include/uapi/linux/netfilter/nf_nat.h > > +++ b/include/uapi/linux/netfilter/nf_nat.h > > @@ -8,6 +8,7 @@ > > #define NF_NAT_RANGE_PROTO_SPECIFIED 2 > > #define NF_NAT_RANGE_PROTO_RANDOM 4 > > #define NF_NAT_RANGE_PERSISTENT 8 > > +#define NF_NAT_ROUTING_DEPENDENT 16 > > > > struct nf_nat_ipv4_range { > > unsigned int flags; > > diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c > > index 5d5d4d1..ecf3063 100644 > > --- a/net/ipv4/netfilter/ipt_MASQUERADE.c > > +++ b/net/ipv4/netfilter/ipt_MASQUERADE.c > > We now have IPv6 NAT support, so I guess you need to patch > /net/ipv6/netfilter/ip6t_MASQUERADE.c Ohh, yes, I missed that. I'll add the required code there. Currently I'm trying to find a way to purge just the entries which are affected by the routing change (for example when there are muliple VPN tunnels). However that requires a new conntrack extension and it's nontrivial (at least for me) to figure out the required data from struct fib_info. If the conntrack extension is used then of course the status bit is not required. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-15 14:42 ` Jozsef Kadlecsik @ 2012-11-16 10:09 ` Pablo Neira Ayuso 2012-11-16 21:38 ` Jozsef Kadlecsik 0 siblings, 1 reply; 11+ messages in thread From: Pablo Neira Ayuso @ 2012-11-16 10:09 UTC (permalink / raw) To: Jozsef Kadlecsik; +Cc: netfilter-devel Hi Jozsef, On Thu, Nov 15, 2012 at 03:42:14PM +0100, Jozsef Kadlecsik wrote: > On Thu, 15 Nov 2012, Pablo Neira Ayuso wrote: > > > Two comments on this patch: > > > > On Tue, Nov 13, 2012 at 09:17:37PM +0100, Jozsef Kadlecsik wrote: > > > When the routing changes, MASQUERADE should delete the conntrack > > > entries where the source NATed address changes due to the routing > > > change. As a first approximation, delete all entries which are > > > marked with the new "--route-dependent" flag of the MASQUERADE > > > target. > > > > > > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> > > > --- > > > include/uapi/linux/netfilter/nf_conntrack_common.h | 4 ++ > > > include/uapi/linux/netfilter/nf_nat.h | 1 + > > > net/ipv4/netfilter/ipt_MASQUERADE.c | 40 ++++++++++++++++++++ > > > 3 files changed, 45 insertions(+), 0 deletions(-) > > > > > > diff --git a/include/uapi/linux/netfilter/nf_conntrack_common.h b/include/uapi/linux/netfilter/nf_conntrack_common.h > > > index 1644cdd..1c698b5 100644 > > > --- a/include/uapi/linux/netfilter/nf_conntrack_common.h > > > +++ b/include/uapi/linux/netfilter/nf_conntrack_common.h > > > @@ -87,6 +87,10 @@ enum ip_conntrack_status { > > > /* Conntrack got a helper explicitly attached via CT target. */ > > > IPS_HELPER_BIT = 13, > > > IPS_HELPER = (1 << IPS_HELPER_BIT), > > > + > > > + /* Conntrack must be deleted when routing changed (NAT) */ > > > + IPS_ROUTING_DEPENDENT_BIT = 14, > > > + IPS_ROUTING_DEPENDENT = (1 << IPS_ROUTING_DEPENDENT_BIT), > > > > This seems to me a bit too specific for the masquerade module. I've > > been checking the struct nf_conn_nat to squash that information there, > > but I don't find the way to make it without increasing the length of > > the NAT area. > > I know, we waste a status bit. But I couldn't find a better way to store > the information in conntrack. > > > > }; > > > > > > /* Connection tracking event types */ > > > diff --git a/include/uapi/linux/netfilter/nf_nat.h b/include/uapi/linux/netfilter/nf_nat.h > > > index bf0cc37..a0dfac7 100644 > > > --- a/include/uapi/linux/netfilter/nf_nat.h > > > +++ b/include/uapi/linux/netfilter/nf_nat.h > > > @@ -8,6 +8,7 @@ > > > #define NF_NAT_RANGE_PROTO_SPECIFIED 2 > > > #define NF_NAT_RANGE_PROTO_RANDOM 4 > > > #define NF_NAT_RANGE_PERSISTENT 8 > > > +#define NF_NAT_ROUTING_DEPENDENT 16 > > > > > > struct nf_nat_ipv4_range { > > > unsigned int flags; > > > diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c > > > index 5d5d4d1..ecf3063 100644 > > > --- a/net/ipv4/netfilter/ipt_MASQUERADE.c > > > +++ b/net/ipv4/netfilter/ipt_MASQUERADE.c > > > > We now have IPv6 NAT support, so I guess you need to patch > > /net/ipv6/netfilter/ip6t_MASQUERADE.c > > Ohh, yes, I missed that. I'll add the required code there. > > Currently I'm trying to find a way to purge just the entries which are > affected by the routing change (for example when there are muliple VPN > tunnels). However that requires a new conntrack extension and it's > nontrivial (at least for me) to figure out the required data from struct > fib_info. > > If the conntrack extension is used then of course the status bit is not > required. We can register now variable length conntrack extensions. I think we can use that feature to extend nf_conn_nat to allocate extra information for all working modes of MASQUERADE. It may require changing the nf_nat_setup_info interface to pass some new flags. Regarding the variable length conntrack extensions, please check nf_ct_ext_add_length in net/netfilter/nf_conntrack_helper.c for instance. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] Handle the routing changes in the MASQUERADE target 2012-11-16 10:09 ` Pablo Neira Ayuso @ 2012-11-16 21:38 ` Jozsef Kadlecsik 0 siblings, 0 replies; 11+ messages in thread From: Jozsef Kadlecsik @ 2012-11-16 21:38 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel Hi Pablo, On Fri, 16 Nov 2012, Pablo Neira Ayuso wrote: > > Currently I'm trying to find a way to purge just the entries which are > > affected by the routing change (for example when there are muliple VPN > > tunnels). However that requires a new conntrack extension and it's > > nontrivial (at least for me) to figure out the required data from struct > > fib_info. > > > > If the conntrack extension is used then of course the status bit is not > > required. > > We can register now variable length conntrack extensions. I think we > can use that feature to extend nf_conn_nat to allocate extra > information for all working modes of MASQUERADE. It may require > changing the nf_nat_setup_info interface to pass some new flags. > > Regarding the variable length conntrack extensions, please check > nf_ct_ext_add_length in net/netfilter/nf_conntrack_helper.c for > instance. I realized that there's no point to store the extra information in an extension: changing for example the weight of another routing rule may effect the conntrack entry. So we have to recheck the routing. Sigh. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-11-16 21:38 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-13 20:17 [PATCH 0/2] [RFC] Get MASQUERADE target to handle routing changes Jozsef Kadlecsik 2012-11-13 20:17 ` [PATCH 1/2] Introduce notification chain for " Jozsef Kadlecsik 2012-11-13 22:15 ` David Miller 2012-11-14 7:57 ` Jozsef Kadlecsik 2012-11-13 20:17 ` [PATCH 2/2] Handle the routing changes in the MASQUERADE target Jozsef Kadlecsik 2012-11-13 23:49 ` Jan Engelhardt 2012-11-14 7:55 ` Jozsef Kadlecsik 2012-11-15 11:44 ` Pablo Neira Ayuso 2012-11-15 14:42 ` Jozsef Kadlecsik 2012-11-16 10:09 ` Pablo Neira Ayuso 2012-11-16 21:38 ` Jozsef Kadlecsik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).