br_netfilter: brnf_net structure for sysctl setting
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]
RFC PATCH v2 1/11 br_netfilter: brnf_net structure for sysctl setting
--- net/bridge/br_private.h | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
net/bridge/br_private.h +13 -0
@@ -312,6 +312,19 @@ struct br_input_skb_cb {# define BR_INPUT_SKB_CB_MROUTERS_ONLY (__skb) (0)#endif +#if definedCONFIG_BRIDGE_NETFILTER && definedCONFIG_SYSCTL +struct brnf_net { +struct net *net; +struct ctl_table_header *hdr; +int call_arptables; +int call_iptables; +int call_ip6tables; +int filter_vlan_tagged; +int filter_pppoe_tagged; +int pass_vlan_indev; +}; +#endif + #define br_printk(level, br, format, args...) \ printk(level "%s: " format, (br)->dev->name, ##args)
Signature · 1 line
1.7.5.4
RFC PATCH v2 2/11 br_netfilter: default sysctl settings in init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
net/bridge/br_netfilter.c +12 -0
@@ -64,6 +64,18 @@ static int brnf_pass_vlan_indev __read_mostly = 0;#define brnf_pass_vlan_indev 0#endif +#ifdef CONFIG_SYSCTL +static struct brnf_net init_brnf_net = { + .hdr =NULL , + .call_arptables = brnf_call_arptables, + .call_iptables = brnf_call_iptables, + .call_ip6tables = brnf_call_ip6tables, + .filter_vlan_tagged = brnf_filter_vlan_tagged, + .filter_pppoe_tagged = brnf_filter_pppoe_tagged, + .pass_vlan_indev = brnf_pass_vlan_indev, +}; +#endif +#define IS_IP (skb) \ (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_IP))
Signature · 1 line
1.7.5.4
Re: [PATCH RFC v2 02/11] br_netfilter: default sysctl settings in init_brnf_net
Quoted 15 lines · Vasily Averin
Signed-off-by Vasily Averin <[email protected]>--- net/bridge/br_netfilter.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)net/bridge/br_netfilter.c +1
-0@@ -64,6 +64,18 @@ static int brnf_pass_vlan_indev __read_mostly = 0;#define brnf_pass_vlan_indev 0#endif +#ifdef CONFIG_SYSCTL
net/bridge/br_netfilter.c static int brnf_pass_vlan_indev __read_mostly = 0; · line 77
Quoted 10 lines
net/bridge/br_netfilter.c +10
-0+static struct brnf_net init_brnf_net = { + .hdr =NULL , + .call_arptables = brnf_call_arptables, + .call_iptables = brnf_call_iptables, + .call_ip6tables = brnf_call_ip6tables, + .filter_vlan_tagged = brnf_filter_vlan_tagged, + .filter_pppoe_tagged = brnf_filter_pppoe_tagged, + .pass_vlan_indev = brnf_pass_vlan_indev, +}; +#endif
These patches are split in an unnecessary excessive fashion and are hard to review. The rule is one patch per logical change, not one patch per file. Introducing a structure and not using it might be acceptable, but adding a structure definition and not using it is just stupid.
Also introducing all these init_brnf_net conversion that are completely replaced afterwards is just useless noise. Please combine them in a more reasonable fashion and resend.
v3 cover per-netns sysctl for br_netfilter
Dear Patrick,
thank you for feedback.
Frankly speaking I still badly understand how it's better to split this patch set. Finally I've decided to combine v2 patches to 2 parts (1-8 and 9-11). Could you please explain how to it better?
This patch set enables per network namespace managemnt for br_netfiltes sysctls, it allows to enable processing br-nf-call hooks in ones network namespaces and keep it disabled in another ones.
v3: patches are merged into more large chunks
v2: removed extra overhead for CONFIG_SYSCTL=n
Vasily Averin (2):
br_netfilter: common structure for sysctl flags
br_netfilter: per-netns copy of structure for sysctl flags
net/bridge/br_netfilter.c | 155 ++++++++++++++++++++++++++++++++++----------- net/bridge/br_private.h | 13 ++++ 2 files changed, 130 insertions(+), 38 deletions(-)
Signature · 1 line
1.7.5.4
Pablo Neira Ayuso
maintainerRe: [PATCH RFC v3 0/2] per-netns sysctl for br_netfilter
Hi Vasily,
Quoted 6 lines · Vasily Averin
Dear Patrick,
thank you for feedback.
Frankly speaking I still badly understand how it's better to split this patch set. Finally I've decided to combine v2 patches to 2 parts (1-8 and 9-11). Could you please explain how to it better?
Quoted 6 lines
This patch set enables per network namespace managemnt for
br_netfiltessysctls, it allows to enable processing br-nf-call hooks in ones network namespaces and keep it disabled in another ones.v3: patches are merged into more large chunks
v2: removed extra overhead forCONFIG_SYSCTL=n
Any further concern with this v3 patchset? Thanks.
Re: [PATCH RFC v3 0/2] per-netns sysctl for br_netfilter
Quoted 11 lines · Pablo Neira Ayuso
Hi Vasily,
Quoted 6 lines · Vasily Averin
This patch set enables per network namespace managemnt for
br_netfiltessysctls, it allows to enable processing br-nf-call hooks in ones network namespaces and keep it disabled in another ones.v3: patches are merged into more large chunks
v2: removed extra overhead forCONFIG_SYSCTL=nAny further concern with this v3 patchset? Thanks.
Dear Pablo,
I'm going to re-make this patchset to add ability to inherit settings from new parent's net-namespace. Unfortunately right now I'm quite busy by relocation, therefore this task is delayed. Also I would like say thanks to Bart and Maciej Zenczykowski for feedback.
A message that is not in the archive (sent off-list or not yet received): [email protected]
RFC PATCH v3 1/2 br_netfilter: common structure for sysctl flags
Introduced common structure for sysctl flags
--- net/bridge/br_netfilter.c | 55 ++++++++++++++++++++++++++------------------ net/bridge/br_private.h | 13 ++++++++++ 2 files changed, 45 insertions(+), 23 deletions(-)
net/bridge/br_netfilter.c +32 -23
@@ -49,19 +49,28 @@#ifdef CONFIG_SYSCTL static struct ctl_table_header *brnf_sysctl_header; -static int brnf_call_iptables__read_mostly =1 ; -static int brnf_call_ip6tables__read_mostly =1 ; -static int brnf_call_arptables__read_mostly =1 ; -static int brnf_filter_vlan_tagged__read_mostly =0 ; -static int brnf_filter_pppoe_tagged__read_mostly =0 ; -static int brnf_pass_vlan_indev__read_mostly =0 ; -#else +#endif +#define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1 -#define brnf_call_arptables 1#define brnf_filter_vlan_tagged 0#define brnf_filter_pppoe_tagged 0#define brnf_pass_vlan_indev 0 + +#ifdef CONFIG_SYSCTL +static struct brnf_net init_brnf_net = { + .hdr =NULL , + .call_arptables = brnf_call_arptables, + .call_iptables = brnf_call_iptables, + .call_ip6tables = brnf_call_ip6tables, + .filter_vlan_tagged = brnf_filter_vlan_tagged, + .filter_pppoe_tagged = brnf_filter_pppoe_tagged, + .pass_vlan_indev = brnf_pass_vlan_indev, +}; + +#define brnf_flag (skb, flag) init_brnf_net.flag +#else +#define brnf_flag (skb, flag) brnf_##flag#endif #define IS_IP (skb) \
@@ -85,15 +94,15 @@ static inline __be16 vlan_proto(const struct sk_buff *skb)#define IS_VLAN_IP (skb) \ (vlan_proto(skb) == htons(ETH_P_IP) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))#define IS_VLAN_IPV6 (skb) \ (vlan_proto(skb) == htons(ETH_P_IPV6) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))#define IS_VLAN_ARP (skb) \ (vlan_proto(skb) == htons(ETH_P_ARP) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))static inline __be16 pppoe_proto (const struct sk_buff *skb) {
@@ -104,12 +113,12 @@ static inline __be16 pppoe_proto(const struct sk_buff *skb)#define IS_PPPOE_IP (skb) \ (skb->protocol == htons(ETH_P_PPP_SES) && \ pppoe_proto(skb) == htons(PPP_IP) && \ - brnf_filter_pppoe_tagged) + brnf_flag(skb, filter_pppoe_tagged))#define IS_PPPOE_IPV6 (skb) \ (skb->protocol == htons(ETH_P_PPP_SES) && \ pppoe_proto(skb) == htons(PPP_IPV6) && \ - brnf_filter_pppoe_tagged) + brnf_flag(skb, filter_pppoe_tagged))static void fake_update_pmtu (struct dst_entry *dst,struct sock *sk,struct sk_buff *skb,u32 mtu)
@@ -532,7 +541,7 @@ static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const structstruct net_device *vlan, *br; br =bridge_parent (dev); -if (brnf_pass_vlan_indev ==0 || !vlan_tx_tag_present (skb)) +if (brnf_flag (skb, pass_vlan_indev) ==0 || !vlan_tx_tag_present (skb))return br; vlan = __vlan_find_dev_deep(br, skb->vlan_proto,
@@ -690,14 +699,14 @@ static unsigned int br_nf_pre_routing(const struct nf_hook_ops *ops, br = p->br;if (IS_IPV6 (skb) ||IS_VLAN_IPV6 (skb) ||IS_PPPOE_IPV6 (skb)) { -if (!brnf_call_ip6tables && !br->nf_call_ip6tables) +if (!brnf_flag (skb, call_ip6tables) && !br->nf_call_ip6tables)return NF_ACCEPT ;nf_bridge_pull_encap_header_rcsum (skb);return br_nf_pre_routing_ipv6 (ops, skb, in, out, okfn); } -if (!brnf_call_iptables && !br->nf_call_iptables) +if (!brnf_flag (skb, call_iptables) && !br->nf_call_iptables)return NF_ACCEPT ;if (!IS_IP (skb) && !IS_VLAN_IP (skb) && !IS_PPPOE_IP (skb))
@@ -838,7 +847,7 @@ static unsigned int br_nf_forward_arp(const struct nf_hook_ops *ops,return NF_ACCEPT ; br = p->br; -if (!brnf_call_arptables && !br->nf_call_arptables) +if (!brnf_flag (skb, call_arptables) && !br->nf_call_arptables)return NF_ACCEPT ;if (!IS_ARP (skb)) {
@@ -1015,42 +1024,42 @@ int brnf_sysctl_call_tables(struct ctl_table *ctl, int write,static struct ctl_table brnf_table[] = { { .procname ="bridge-nf-call-arptables" , - .data = &brnf_call_arptables, + .data = &init_brnf_net.call_arptables, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables, }, { .procname ="bridge-nf-call-iptables" , - .data = &brnf_call_iptables, + .data = &init_brnf_net.call_iptables, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables, }, { .procname ="bridge-nf-call-ip6tables" , - .data = &brnf_call_ip6tables, + .data = &init_brnf_net.call_ip6tables, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables, }, { .procname ="bridge-nf-filter-vlan-tagged" , - .data = &brnf_filter_vlan_tagged, + .data = &init_brnf_net.filter_vlan_tagged, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables, }, { .procname ="bridge-nf-filter-pppoe-tagged" , - .data = &brnf_filter_pppoe_tagged, + .data = &init_brnf_net.filter_pppoe_tagged, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables, }, { .procname ="bridge-nf-pass-vlan-input-dev" , - .data = &brnf_pass_vlan_indev, + .data = &init_brnf_net.pass_vlan_indev, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables,
net/bridge/br_private.h +13 -0
@@ -312,6 +312,19 @@ struct br_input_skb_cb {# define BR_INPUT_SKB_CB_MROUTERS_ONLY (__skb) (0)#endif +#if definedCONFIG_BRIDGE_NETFILTER && definedCONFIG_SYSCTL +struct brnf_net { +struct net *net; +struct ctl_table_header *hdr; +int call_arptables; +int call_iptables; +int call_ip6tables; +int filter_vlan_tagged; +int filter_pppoe_tagged; +int pass_vlan_indev; +}; +#endif + #define br_printk(level, br, format, args...) \ printk(level "%s: " format, (br)->dev->name, ##args)
Signature · 1 line
1.7.5.4
RFC PATCH v3 2/2 br_netfilter: per-netns copy of structure for sysctl flags
In reply to a message not in the archivepernet_operations creates per-netns copy of common structure for sysctl flags and initialize it values taken from init_brnf_net.
--- net/bridge/br_netfilter.c | 104 +++++++++++++++++++++++++++++++++++++------- 1 files changed, 87 insertions(+), 17 deletions(-)
net/bridge/br_netfilter.c +87 -17
@@ -40,6 +40,7 @@#include "br_private.h" #ifdef CONFIG_SYSCTL #include <linux/sysctl.h> +#include <net/netns/generic.h> #endif #define skb_origaddr (skb) (((struct bridge_skb_cb *) \
@@ -47,9 +48,6 @@#define store_orig_dstaddr (skb) (skb_origaddr(skb) = ip_hdr(skb)->daddr)#define dnat_took_place (skb) (skb_origaddr(skb) != ip_hdr(skb)->daddr) -#ifdef CONFIG_SYSCTL -static struct ctl_table_header *brnf_sysctl_header; -#endif #define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1
@@ -58,6 +56,7 @@ static struct ctl_table_header *brnf_sysctl_header;#define brnf_pass_vlan_indev 0#ifdef CONFIG_SYSCTL +static int brnf_net_id__read_mostly ;static struct brnf_net init_brnf_net = { .hdr =NULL , .call_arptables = brnf_call_arptables,
@@ -68,7 +67,13 @@ static struct brnf_net init_brnf_net = { .pass_vlan_indev = brnf_pass_vlan_indev, }; -#define brnf_flag (skb, flag) init_brnf_net.flag +static inline struct brnf_net *brnf_net (const struct net *net) +{ +return net_generic (net, brnf_net_id); +} + +#define skb_netns (skb) net((skb)->dev) +#define brnf_flag (skb, flag) brnf_net(skb_netns(skb))->flag#else #define brnf_flag (skb, flag) brnf_##flag#endif
@@ -1066,6 +1071,70 @@ static struct ctl_table brnf_table[] = { }, { } }; + +static int brnf_sysctl_net_register (struct brnf_net *bn) +{ +struct ctl_table *table; +struct ctl_table_header *hdr; +int i; + + table = brnf_table; +if (!net_eq (bn->net, &init_net)) { + + table =kmemdup (table,sizeof (brnf_table),GFP_KERNEL ); +if (!table) + goto err_alloc; + } +for (i =0 ; table[i].data; i++) + table[i].data += (char *)bn - (char *)&init_brnf_net; + + hdr =register_net_sysctl (bn->net,"net/bridge" , table); +if (!hdr) + goto err_reg; + + bn->hdr = hdr; +return 0 ; + +err_reg: +if (!net_eq (bn->net, &init_net)) +kfree (table); +err_alloc: +return -ENOMEM ; +} + +static void brnf_sysctl_net_unregister (struct brnf_net *bn) +{ +struct ctl_table *table; + +if (bn->hdr ==NULL ) +return ; + + table = bn->hdr->ctl_table_arg; +unregister_net_sysctl_table (bn->hdr); +if (!net_eq (bn->net, &init_net)) +kfree (table); +} + +static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return brnf_sysctl_net_register (bn); +} + +static void __net_exit brnf_net_exit(struct net *net) +{ +brnf_sysctl_net_unregister (brnf_net (net)); +} + +static struct pernet_operations __net_initdata brnf_net_ops = { + .init = brnf_net_init, + .exit = brnf_net_exit, + .id = &brnf_net_id, + .size =sizeof (struct brnf_net ), +};#endif int __init br_netfilter_init (void )
@@ -1074,32 +1143,33 @@ int __init br_netfilter_init(void) ret =dst_entries_init (&fake_dst_ops);if (ret <0 ) -return ret; + goto err_dst; ret =nf_register_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); -if (ret <0 ) { -dst_entries_destroy (&fake_dst_ops); -return ret; - } +if (ret <0 ) + goto err_nf; +#ifdef CONFIG_SYSCTL - brnf_sysctl_header =register_net_sysctl (&init_net,"net/bridge" , brnf_table); -if (brnf_sysctl_header ==NULL ) { -printk (KERN_WARNING -"br_netfilter: can't register to sysctl.\n" ); + ret =register_pernet_subsys (&brnf_net_ops); +if (ret <0 ) {nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); -dst_entries_destroy (&fake_dst_ops); -return -ENOMEM ; + goto err_nf; }#endif printk (KERN_NOTICE "Bridge firewalling registered\n" );return 0 ; + +err_nf: +dst_entries_destroy (&fake_dst_ops); +err_dst: +return ret; }void br_netfilter_fini (void ) { -nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops));#ifdef CONFIG_SYSCTL -unregister_net_sysctl_table (brnf_sysctl_header); +unregister_pernet_subsys (&brnf_net_ops);#endif +nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops));dst_entries_destroy (&fake_dst_ops); }
Signature · 1 line
1.7.5.4
Re: [PATCH RFC v3 2/2] br_netfilter: per-netns copy of structure for sysctl flags
Vasily Averin schreef op 12/05/2014 18:32:
net/bridge/br_netfilter.c static struct ctl_table brnf_table[] = { · line 1124
Quoted 12 lines
pernet_operationscreates per-netns copy of common structure for sysctl flags and initialize it values taken frominit_brnf_net.Signed-off-by Vasily Averin <[email protected]>net/bridge/br_netfilter.c +7
-0+static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return brnf_sysctl_net_register (bn);
This does introduce a bit of backwards incompatibility (easily fixed by adapting scripts), but this is really unavoidable when transforming an existing global configuration to a per-netns configuration. I'm ok with it.
cheers,
Bart
Re: [PATCH RFC v3 2/2] br_netfilter: per-netns copy of structure for sysctl flags
Quoted 6 lines · Bart De Schuymer
Vasily Averin schreef op 12/05/2014 18:32:
Quoted 4 lines
pernet_operationscreates per-netns copy of common structure for sysctl flags and initialize it values taken frominit_brnf_net.Signed-off-by Vasily Averin <[email protected]>
Quoted 12 lines
net/bridge/br_netfilter.c static struct ctl_table brnf_table[] = { · line 1124
Quoted 7 lines
net/bridge/br_netfilter.c +7
-0+static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return brnf_sysctl_net_register (bn);This does introduce a bit of backwards incompatibility (easily fixed by adapting scripts), but this is really unavoidable when transforming an existing global configuration to a per-netns configuration. I'm ok with it.
Could you please explain, which backward incompatibility you mean here? Nobody changes values init_brnf_net,init_net have own copy, like any other network namespaces.
Re: [PATCH RFC v3 2/2] br_netfilter: per-netns copy of structure for sysctl flags
Vasily Averin schreef op 12/05/2014 22:11:
Quoted 20 lines · Bart De Schuymer
Quoted 6 lines
Vasily Averin schreef op 12/05/2014 18:32:
Quoted 4 lines
pernet_operationscreates per-netns copy of common structure for sysctl flags and initialize it values taken frominit_brnf_net.Signed-off-by Vasily Averin <[email protected]>Quoted 12 lines
net/bridge/br_netfilter.c static struct ctl_table brnf_table[] = { · line 1124
Quoted 7 lines
net/bridge/br_netfilter.c +7
-0+static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return brnf_sysctl_net_register (bn);This does introduce a bit of backwards incompatibility (easily fixed by adapting scripts), but this is really unavoidable when transforming an existing global configuration to a per-netns configuration. I'm ok with it.
Quoted 3 lines
Could you please explain, which backward incompatibility you mean here? Nobody changes values
init_brnf_net,init_nethave own copy, like any other network namespaces.
Well, init_brnf_net is never written to, so it keeps the default flags.
If a new netns is created, a copy of the contents of init_brnf_net is made. So, whenever a netns is created, it starts with the default flags (e.g. brnf_call_iptables is always 1 for a newly created netns).
In the current kernel, when a new netns is created, the configuration of the main netns is used (the proc system doesn't even show the flags in the created netns): if brnf_call_iptables is 0 before the new netns is created, iptables won't see bridged IP traffic in the new netns. With your patch, this behaviour will change.
It's possible to alter your patch to keep the same behaviour as before at netns creation, but always starting from the same defaults is cleaner.
cheers,
Bart
A message that is not in the archive (sent off-list or not yet received): [email protected]
question about default values for per-namespace settings
Dear Tejun,
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
I'm preparing patch that makes per-netns sysctls in br_netfilter,
to be able to enable/disable br-nf-call processing in each network namespace independently.
I've initialized sysctl values in each netns by system defaults, like it was done in similar cases. However Bart pointed that "this does introduce a bit of backwards incompatibility":
currently all netns shares the br_netfilter sysctl settings applied in init_net.
From OpenVz point of view containers should be properly isolated, should have predictable initial configuration
and should not depend on settings applied in another containers. On the other hand independent containers is only one of possible usecases, and I have no strong objections against Bart's proposal. Frankly speaking initially I've planned to copy setting from init_net too.
To make possible both variants I can introduce one more setting, it allows to specify desired behavior:
to use system defaults or to copy current settings from init_net.
However probably the same dilemma was observed in another subsystems? Perhaps this selector already exists?
If isn't, how do you think, should I introduce some new global parameter, or may be it should be some local bridge-only-related setting?
Thank you,
Vasily Averin
you can find some more details about my patch in netfilter-devel@ [PATCH RFC v3 2/2] br_netfilter: per-netns copy of structure for sysctl flags
Quoted 32 lines · Bart De Schuymer
Vasily Averin schreef op 12/05/2014 22:11:
Quoted 20 lines · Bart De Schuymer
Quoted 6 lines
Vasily Averin schreef op 12/05/2014 18:32:
Quoted 4 lines
pernet_operationscreates per-netns copy of common structure for sysctl flags and initialize it values taken frominit_brnf_net.Signed-off-by Vasily Averin <[email protected]>Quoted 12 lines
net/bridge/br_netfilter.c static struct ctl_table brnf_table[] = { · line 1124
Quoted 7 lines
net/bridge/br_netfilter.c +7
-0+static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return brnf_sysctl_net_register (bn);This does introduce a bit of backwards incompatibility (easily fixed by adapting scripts), but this is really unavoidable when transforming an existing global configuration to a per-netns configuration. I'm ok with it.
Quoted 3 lines
Could you please explain, which backward incompatibility you mean here? Nobody changes values
init_brnf_net,init_nethave own copy, like any other network namespaces.Well,
init_brnf_netis never written to, so it keeps the default flags.
If a new netns is created, a copy of the contents ofinit_brnf_netis made. So, whenever a netns is created, it starts with the default flags (e.g.brnf_call_iptablesis always 1 for a newly created netns).In the current kernel, when a new netns is created, the configuration of the main netns is used (the proc system doesn't even show the flags in the created netns): if
brnf_call_iptablesis 0 before the new netns is created, iptables won't see bridged IP traffic in the new netns. With your patch, this behaviour will change.It's possible to alter your patch to keep the same behaviour as before at netns creation, but always starting from the same defaults is cleaner.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: question about default values for per-namespace settings
Quoted 9 lines · Vasily Averin
Dear Tejun,
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
I'm preparing patch that makes per-netns sysctls in
br_netfilter,
to be able to enable/disable br-nf-call processing in each network namespace independently.
Quoted 7 lines
I've initialized sysctl values in each netns by system defaults, like it was done in similar cases. However Bart pointed that "this does introduce a bit of backwards incompatibility":
currently all netns shares thebr_netfiltersysctl settings applied ininit_net.From OpenVz point of view containers should be properly isolated, should have predictable initial configuration
and should not depend on settings applied in another containers.
In 'another container' no, but if you are starting a nested container, then the from the parent container, yes. Not from init_net.
Quoted 57 lines
On the other hand independent containers is only one of possible usecases, and I have no strong objections against Bart's proposal. Frankly speaking initially I've planned to copy setting from
init_nettoo.To make possible both variants I can introduce one more setting, it allows to specify desired behavior:
to use system defaults or to copy current settings frominit_net.However probably the same dilemma was observed in another subsystems? Perhaps this selector already exists?
If isn't, how do you think, should I introduce some new global parameter, or may be it should be some local bridge-only-related setting?
Thank you,
Vasily Averinyou can find some more details about my patch in netfilter-devel@ [PATCH RFC v3 2/2]
br_netfilter: per-netns copy of structure for sysctl flagsQuoted 25 lines · Bart De Schuymer
Vasily Averin schreef op 12/05/2014 22:11:
Quoted 20 lines · Bart De Schuymer
Quoted 6 lines
Vasily Averin schreef op 12/05/2014 18:32:
Quoted 4 lines
pernet_operationscreates per-netns copy of common structure for sysctl flags and initialize it values taken frominit_brnf_net.Signed-off-by Vasily Averin <[email protected]>Quoted 12 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+static int __net_init brnf_net_init(struct net *net) +{ + struct brnf_net *bn = brnf_net(net); + + memcpy(bn, &init_brnf_net, sizeof(struct brnf_net)); + bn->net = net; + return brnf_sysctl_net_register(bn);This does introduce a bit of backwards incompatibility (easily fixed by adapting scripts), but this is really unavoidable when transforming an existing global configuration to a per-netns configuration. I'm ok with it.
Quoted 3 lines
Could you please explain, which backward incompatibility you mean here? Nobody changes values
init_brnf_net,init_nethave own copy, like any other network namespaces.Quoted 7 lines
Well,
init_brnf_netis never written to, so it keeps the default flags.
If a new netns is created, a copy of the contents ofinit_brnf_netis made. So, whenever a netns is created, it starts with the default flags (e.g.brnf_call_iptablesis always 1 for a newly created netns).In the current kernel, when a new netns is created, the configuration of the main netns is used (the proc system doesn't even show the flags in the created netns): if
brnf_call_iptablesis 0 before the new netns is created, iptables won't see bridged IP traffic in the new netns. With your patch, this behaviour will change.It's possible to alter your patch to keep the same behaviour as before at netns creation, but always starting from the same defaults is cleaner.
_______________________________________________
Containers mailing list Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
Re: question about default values for per-namespace settings
In reply to a message not in the archiveHello,
Quoted 3 lines · Vasily Averin
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
Unfortunately, I don't have much idea on the subject. Serge and others on the containers list should know a lot better than me.
Thanks.
Signature · 1 line
tejun
Re: question about default values for per-namespace settings
In reply to a message not in the archiveVasily Averin schreef op 15/05/2014 11:01:
Quoted 13 lines
Dear Tejun,
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
I'm preparing patch that makes per-netns sysctls in
br_netfilter,
to be able to enable/disable br-nf-call processing in each network namespace independently.I've initialized sysctl values in each netns by system defaults, like it was done in similar cases. However Bart pointed that "this does introduce a bit of backwards incompatibility":
currently all netns shares thebr_netfiltersysctl settings applied ininit_net.
Quoted 6 lines
Quoted 1 line
From OpenVz point of view containers should be properly isolated,
should have predictable initial configuration
and should not depend on settings applied in another containers. On the other hand independent containers is only one of possible usecases, and I have no strong objections against Bart's proposal. Frankly speaking initially I've planned to copy setting frominit_nettoo.
You misread my mail. I stated that I'm ok with always starting from the defaults (as your patch does). As pointed out by Maciej, always starting from init_net isn't really an option in case of nested namespaces (start from the parent's namespace instead). There'll always be pros and cons to whatever you choose here. Complete backwards compatibility isn't possible either way.
The only way to keep backwards compatibility is to introduce new proc file names and keep the old behavior for the old names (but I'm not in favor of that).
cheers,
Bart
Re: question about default values for per-namespace settings
In reply to Vasily AverinHello,
Quoted 3 lines · Vasily Averin
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
Unfortunately, I don't have much idea on the subject. Serge and others on the containers list should know a lot better than me.
Thanks.
Signature · 1 line
tejun
A message that is not in the archive (sent off-list or not yet received): 20140515174850.GB20738-9pTldWuhBndy/[email protected]
Re: question about default values for per-namespace settings
Isn't pulling current settings from init_net an isolation violation if init_net isn't the namespace you are in at the time you are creating the new namespace?
The way I see it there are 2 possibilities:
(a) you use some kernel (probably compile time) defaults (ie. what init_net gets when you boot machine)
(b) you inherit from current namespace
I'm not sure what the right choice is.
For something like 'iptables configuration' it seems (a) is correct (come up with no firewall).
For something like 'tcp socket memory limits' or 'bindv6only' or 'v6.default_use_tempaddr) it does seem like (b) is possibly more appropriate.
That said I think there are cases where (a) is clearly correct and (b) is clearly not desirable (iptables conf being a prime example). After all a new namespace doesn't inherit interfaces from the namespace we're in when we create it.
I can't think of any cases where (b) is clearly correct and (a) is clearly not desirable.
[I guess this is less than clear for settings which auto scale at boot with available ram and/or number of cpus in the machine]
Based on that doing (a) for everything may be the right choice (consistency trump...).
This would imply network namespace you are in should have no effect on the new network namespace you are creating.
OTOH, if I want to change some tcp mem tuning sysctl (or something like net.ipv6.conf.default.use_tempaddr = 2)
it would be annoying if /etc/sysctl.conf didn't apply to non-init namespace. But perhaps this is better solved in userspace by loading some /etc/sysctls-for-new-network-namespaces.conf settings in some network namespace creating libraries.
- Maciej
Re: question about default values for per-namespace settings
In reply to Vasily AverinVasily Averin schreef op 15/05/2014 11:01:
Quoted 13 lines
Dear Tejun,
how do you think, which defaults should be used for per-namespace settings in general case and for per-netns sysctls especially? Do we have some common position about this or perhaps we already have some setting that allows to select desired behavior?
I'm preparing patch that makes per-netns sysctls in
br_netfilter,
to be able to enable/disable br-nf-call processing in each network namespace independently.I've initialized sysctl values in each netns by system defaults, like it was done in similar cases. However Bart pointed that "this does introduce a bit of backwards incompatibility":
currently all netns shares thebr_netfiltersysctl settings applied ininit_net.
Quoted 6 lines
Quoted 1 line
From OpenVz point of view containers should be properly isolated,
should have predictable initial configuration
and should not depend on settings applied in another containers. On the other hand independent containers is only one of possible usecases, and I have no strong objections against Bart's proposal. Frankly speaking initially I've planned to copy setting frominit_nettoo.
You misread my mail. I stated that I'm ok with always starting from the defaults (as your patch does). As pointed out by Maciej, always starting from init_net isn't really an option in case of nested namespaces (start from the parent's namespace instead). There'll always be pros and cons to whatever you choose here. Complete backwards compatibility isn't possible either way.
The only way to keep backwards compatibility is to introduce new proc file names and keep the old behavior for the old names (but I'm not in favor of that).
cheers,
Bart
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: question about default values for per-namespace settings
Quoted 3 lines · Bart De Schuymer
As pointed out by Maciej, always
starting frominit_netisn't really an option in case of nested namespaces (start from the parent's namespace instead).
Dear Bart, Serge, Maciej
thank you very much for your feedback!
I've analyzed possibility to inherit settings from parent net-namespace, discovered problems described below and finally decided to follow Maciej's way (a) "use some kernel defaults", with adding an ability to change pre-compiled kernel defaults.
Below you can found more detailed description of discovered problems.
1) there are no (easy) ways to find parent of given network namespace.
Network namespaces in kernel are not hierarchical but flat, "struct net" have no reference to parent netns, and my collegians expect that Eric Biederman will likely object to adding a parent netns pointer.
Without this reference I do not see any good ways to copy parents settings.
2) settings inheriting does not work if subsystem module is loaded after creation of network namespace.
In this case all namespaces get pre-compiled defaults settings, and seems there are no ways to apply "adjusted" setting to all already existing netns.
Moreover there is curious situation: to apply required sysctl settings during module loading, Red Hat recommends to force "sysctl -p" execution via install command in modprobe.conf
https://bugzilla.redhat.com/show_bug.cgi?id=634735#c7
However if module is loaded from inside one of network namespaces it does not work!
In this case sysctl is executed inside netns.
If assigned sysctl key is not virtualized -- sysctl command can fail if key is virtualized -- setting in current netns will be adjusted, but not -- in init_net, that looks unexpected for me.
I believe initial subsystem settings of newly created namespace should not differ from initial settings of newly created subsystem in already existing namespace. In case in-kernel setting inheriting this behavior cannot be reached, additional subsystem tuning is required anyway.
Therefore Maceiej's variant (a) "use some kernel defaults" looks like right choice for me. If parent wants to assign some adjusted settings in child environments -- it can only force loading of required modules and apply required settings directly.
At the same time I would like to have an ability to change pre-compiled defaults somehow. In my patch I'm going to add new module options, that allows node owner to specify wished "safe" settings before module loading, and change them via sysfs after this.
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: question about default values for per-namespace settings
Quoted 7 lines · Vasily Averin
Quoted 3 lines · Bart De Schuymer
As pointed out by Maciej, always
starting frominit_netisn't really an option in case of nested namespaces (start from the parent's namespace instead).Dear Bart, Serge, Maciej
thank you very much for your feedback!
I am missing the context which makes raises this issue.
Quoted 7 lines
I've analyzed possibility to inherit settings from parent net-namespace, discovered problems described below and finally decided to follow Maciej's way (a) "use some kernel defaults", with adding an ability to change pre-compiled kernel defaults.
Below you can found more detailed description of discovered problems.
Quoted 8 lines
1) there are no (easy) ways to find parent of given network namespace.
Network namespaces in kernel are not hierarchical but flat, "struct net" have no reference to parent netns, and my collegians expect that Eric Biederman will likely object to adding a parent netns pointer.
Without this reference I do not see any good ways to copy parents settings.
Copying settings can easily happen at netowkr namespace creation time. Copying at any other time is too weird to even think about. So no you don't need a parent network namespace pointer to enable copying.
Quoted 6 lines
2) settings inheriting does not work if subsystem module is loaded after creation of network namespace.
In this case all namespaces get pre-compiled defaults settings, and seems there are no ways to apply "adjusted" setting to all already existing netns.
setns.
Quoted 7 lines
Moreover there is curious situation: to apply required sysctl settings during module loading, Red Hat recommends to force "sysctl -p" execution via install command in modprobe.conf
https://bugzilla.redhat.com/show_bug.cgi?id=634735#c7However if module is loaded from inside one of network namespaces it does not work!
Why not? The appropriate events should fire globally. And note in most use cases participants in a network namespace won't have permissions to load modules.
Quoted 4 lines
In this case sysctl is executed inside netns.
If assigned sysctl key is not virtualized -- sysctl command can fail if key is virtualized -- setting in current netns will be adjusted, but not -- ininit_net, that looks unexpected for me.
From what little you have said. This sounds like a don't do that then situation. Certainly if a module is the kernel triggers request module the module will be loaded in the initial set of namespaces.
Quoted 4 lines
I believe initial subsystem settings of newly created namespace should not differ from initial settings of newly created subsystem in already existing namespace. In case in-kernel setting inheriting this behavior cannot be reached, additional subsystem tuning is required anyway.
You are arguing that creation of a network namespace should use the kernel's default values for sysctls? That is a fairly reasonable position to take.
Quoted 9 lines
Therefore Maceiej's variant (a) "use some kernel defaults" looks like right choice for me. If parent wants to assign some adjusted settings in child environments -- it can only force loading of required modules and apply required settings directly.
At the same time I would like to have an ability to change pre-compiled defaults somehow. In my patch I'm going to add new module options, that allows node owner to specify wished "safe" settings before module loading, and change them via sysfs after this.
Why sysfs and not sysctl?
It is not clear to me what is going on, from the limited details I see in this message it sounds like there may be a bit of overdesign and tackling problems that do not matter in the real world going on.
For any kernel settings that apply to a network namespace we have two very basic choices.
- Set them to default values when a namespace is initialized.
- Copy them from somewhere when the namespace is created.
Last I looked at that code we were copying sysctl values from the initial network namespace instead of the creators network namespace. Which has always seemed a bit silly to me.
In general most people don't care and this does not cause an issue for most folks, or we could not have gone 5+ years without addressing it.
For most things any practical program at this point is going to have to set the sysctls it cares about because it is going to have to run on existing kernels.
Beyond that I don't have a strong opinion but we could either set values to well expected defaults, or copy them from the creators previous network namespace. Both would give deterministic results without any significant chance of breaking userspace today.
Is their a compelling use case in this conversation that could weigh the decision of which semantics make the most sense?
Adding sysfs entries or module parameters to change the action of sysctls sounds like there is something broken somewhere. Unfortunately it is not clear to me where that somewhere is.
Eric
RFC PATCH v2 3/11 br_netfilter: brnf_flag macro
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
net/bridge/br_netfilter.c +6 -0
@@ -76,6 +76,12 @@ static struct brnf_net init_brnf_net = { };#endif +#ifdef CONFIG_SYSCTL +#define brnf_flag (skb, flag) init_brnf_net.flag +#else +#define brnf_flag (skb, flag) brnf_##flag +#endif +#define IS_IP (skb) \ (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_IP))
Signature · 1 line
1.7.5.4
RFC PATCH v2 4/11 br_netfilter: switch sysctl call_arptables to init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
net/bridge/br_netfilter.c +3 -4
@@ -51,18 +51,17 @@static struct ctl_table_header *brnf_sysctl_header;static int brnf_call_iptables__read_mostly =1 ;static int brnf_call_ip6tables__read_mostly =1 ; -static int brnf_call_arptables__read_mostly =1 ;static int brnf_filter_vlan_tagged__read_mostly =0 ;static int brnf_filter_pppoe_tagged__read_mostly =0 ;static int brnf_pass_vlan_indev__read_mostly =0 ;#else #define brnf_call_iptables 1#define brnf_call_ip6tables 1 -#define brnf_call_arptables 1#define brnf_filter_vlan_tagged 0#define brnf_filter_pppoe_tagged 0#define brnf_pass_vlan_indev 0#endif +#define brnf_call_arptables 1#ifdef CONFIG_SYSCTL static struct brnf_net init_brnf_net = {
@@ -856,7 +855,7 @@ static unsigned int br_nf_forward_arp(const struct nf_hook_ops *ops,return NF_ACCEPT ; br = p->br; -if (!brnf_call_arptables && !br->nf_call_arptables) +if (!brnf_flag (skb, call_arptables) && !br->nf_call_arptables)return NF_ACCEPT ;if (!IS_ARP (skb)) {
@@ -1033,7 +1032,7 @@ int brnf_sysctl_call_tables(struct ctl_table *ctl, int write,static struct ctl_table brnf_table[] = { { .procname ="bridge-nf-call-arptables" , - .data = &brnf_call_arptables, + .data = &init_brnf_net.call_arptables, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = brnf_sysctl_call_tables,
Signature · 1 line
1.7.5.4
RFC PATCH v2 5/11 br_netfilter: switch sysctls call_iptables and call_ip6tables to init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 14 ++++++-------- 1 files changed, 6 insertions(+), 8 deletions(-)
net/bridge/br_netfilter.c +6 -8
@@ -49,19 +49,17 @@#ifdef CONFIG_SYSCTL static struct ctl_table_header *brnf_sysctl_header; -static int brnf_call_iptables__read_mostly =1 ; -static int brnf_call_ip6tables__read_mostly =1 ;static int brnf_filter_vlan_tagged__read_mostly =0 ;static int brnf_filter_pppoe_tagged__read_mostly =0 ;static int brnf_pass_vlan_indev__read_mostly =0 ;#else -#define brnf_call_iptables 1 -#define brnf_call_ip6tables 1#define brnf_filter_vlan_tagged 0#define brnf_filter_pppoe_tagged 0#define brnf_pass_vlan_indev 0#endif #define brnf_call_arptables 1 +#define brnf_call_iptables 1 +#define brnf_call_ip6tables 1#ifdef CONFIG_SYSCTL static struct brnf_net init_brnf_net = {
@@ -707,14 +705,14 @@ static unsigned int br_nf_pre_routing(const struct nf_hook_ops *ops, br = p->br;if (IS_IPV6 (skb) ||IS_VLAN_IPV6 (skb) ||IS_PPPOE_IPV6 (skb)) { -if (!brnf_call_ip6tables && !br->nf_call_ip6tables) +if (!brnf_flag (skb, call_ip6tables) && !br->nf_call_ip6tables)return NF_ACCEPT ;nf_bridge_pull_encap_header_rcsum (skb);return br_nf_pre_routing_ipv6 (ops, skb, in, out, okfn); } -if (!brnf_call_iptables && !br->nf_call_iptables) +if (!brnf_flag (skb, call_iptables) && !br->nf_call_iptables)return NF_ACCEPT ;if (!IS_IP (skb) && !IS_VLAN_IP (skb) && !IS_PPPOE_IP (skb))
@@ -1039,14 +1037,14 @@ static struct ctl_table brnf_table[] = { }, { .procname = "bridge-nf-call-iptables", - .data = &brnf_call_iptables, + .data = &init_brnf_net.call_iptables, .maxlen = sizeof(int), .mode = 0644, .proc_handler = brnf_sysctl_call_tables, }, { .procname = "bridge-nf-call-ip6tables", - .data = &brnf_call_ip6tables, + .data = &init_brnf_net.call_ip6tables, .maxlen = sizeof(int), .mode = 0644, .proc_handler = brnf_sysctl_call_tables,
Signature · 1 line
1.7.5.4
RFC PATCH v2 6/11 br_netfilter: switch sysctl filter_vlan_tagged to init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
net/bridge/br_netfilter.c +5 -6
@@ -49,17 +49,16 @@#ifdef CONFIG_SYSCTL static struct ctl_table_header *brnf_sysctl_header; -static int brnf_filter_vlan_tagged__read_mostly =0 ;static int brnf_filter_pppoe_tagged__read_mostly =0 ;static int brnf_pass_vlan_indev__read_mostly =0 ;#else -#define brnf_filter_vlan_tagged 0#define brnf_filter_pppoe_tagged 0#define brnf_pass_vlan_indev 0#endif #define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1 +#define brnf_filter_vlan_tagged 0#ifdef CONFIG_SYSCTL static struct brnf_net init_brnf_net = {
@@ -100,15 +99,15 @@ static inline __be16 vlan_proto(const struct sk_buff *skb)#define IS_VLAN_IP (skb) \ (vlan_proto(skb) == htons(ETH_P_IP) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))#define IS_VLAN_IPV6 (skb) \ (vlan_proto(skb) == htons(ETH_P_IPV6) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))#define IS_VLAN_ARP (skb) \ (vlan_proto(skb) == htons(ETH_P_ARP) && \ - brnf_filter_vlan_tagged) + brnf_flag(skb, filter_vlan_tagged))static inline __be16 pppoe_proto (const struct sk_buff *skb) {
@@ -1051,7 +1050,7 @@ static struct ctl_table brnf_table[] = { }, { .procname = "bridge-nf-filter-vlan-tagged", - .data = &brnf_filter_vlan_tagged, + .data = &init_brnf_net.filter_vlan_tagged, .maxlen = sizeof(int), .mode = 0644, .proc_handler = brnf_sysctl_call_tables,
Signature · 1 line
1.7.5.4
RFC PATCH v2 7/11 br_netfilter: switch sysctl filter_pppoe_tagged to init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-)
net/bridge/br_netfilter.c +4 -5
@@ -49,16 +49,15 @@#ifdef CONFIG_SYSCTL static struct ctl_table_header *brnf_sysctl_header; -static int brnf_filter_pppoe_tagged__read_mostly =0 ;static int brnf_pass_vlan_indev__read_mostly =0 ;#else -#define brnf_filter_pppoe_tagged 0#define brnf_pass_vlan_indev 0#endif #define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1#define brnf_filter_vlan_tagged 0 +#define brnf_filter_pppoe_tagged 0#ifdef CONFIG_SYSCTL static struct brnf_net init_brnf_net = {
@@ -118,12 +117,12 @@ static inline __be16 pppoe_proto(const struct sk_buff *skb)#define IS_PPPOE_IP (skb) \ (skb->protocol == htons(ETH_P_PPP_SES) && \ pppoe_proto(skb) == htons(PPP_IP) && \ - brnf_filter_pppoe_tagged) + brnf_flag(skb, filter_pppoe_tagged))#define IS_PPPOE_IPV6 (skb) \ (skb->protocol == htons(ETH_P_PPP_SES) && \ pppoe_proto(skb) == htons(PPP_IPV6) && \ - brnf_filter_pppoe_tagged) + brnf_flag(skb, filter_pppoe_tagged))static void fake_update_pmtu (struct dst_entry *dst,struct sock *sk,struct sk_buff *skb,u32 mtu)
@@ -1057,7 +1056,7 @@ static struct ctl_table brnf_table[] = { }, { .procname = "bridge-nf-filter-pppoe-tagged", - .data = &brnf_filter_pppoe_tagged, + .data = &init_brnf_net.filter_pppoe_tagged, .maxlen = sizeof(int), .mode = 0644, .proc_handler = brnf_sysctl_call_tables,
Signature · 1 line
1.7.5.4
RFC PATCH v2 8/11 br_netfilter: switch sysctl pass_vlan_indev to init_brnf_net
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
net/bridge/br_netfilter.c +3 -5
@@ -49,15 +49,13 @@#ifdef CONFIG_SYSCTL static struct ctl_table_header *brnf_sysctl_header; -static int brnf_pass_vlan_indev__read_mostly =0 ; -#else -#define brnf_pass_vlan_indev 0#endif #define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1#define brnf_filter_vlan_tagged 0#define brnf_filter_pppoe_tagged 0 +#define brnf_pass_vlan_indev 0#ifdef CONFIG_SYSCTL static struct brnf_net init_brnf_net = {
@@ -545,7 +543,7 @@ static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const structstruct net_device *vlan, *br; br =bridge_parent (dev); -if (brnf_pass_vlan_indev ==0 || !vlan_tx_tag_present (skb)) +if (brnf_flag (skb, pass_vlan_indev) ==0 || !vlan_tx_tag_present (skb))return br; vlan = __vlan_find_dev_deep(br, skb->vlan_proto,
@@ -1063,7 +1061,7 @@ static struct ctl_table brnf_table[] = { }, { .procname = "bridge-nf-pass-vlan-input-dev", - .data = &brnf_pass_vlan_indev, + .data = &init_brnf_net.pass_vlan_indev, .maxlen = sizeof(int), .mode = 0644, .proc_handler = brnf_sysctl_call_tables,
Signature · 1 line
1.7.5.4
RFC PATCH v2 9/11 br_netfilter: added pernet_operations without sysctl registration
In reply to a message not in the archiveadded registration of per-netns operation without sysctl registration also reworked rollback in br_netfilter_init()
--- net/bridge/br_netfilter.c | 50 +++++++++++++++++++++++++++++++++++++++----- 1 files changed, 44 insertions(+), 6 deletions(-)
net/bridge/br_netfilter.c +44 -6
@@ -40,6 +40,7 @@#include "br_private.h" #ifdef CONFIG_SYSCTL #include <linux/sysctl.h> +#include <net/netns/generic.h> #endif #define skb_origaddr (skb) (((struct bridge_skb_cb *) \
@@ -58,6 +59,7 @@ static struct ctl_table_header *brnf_sysctl_header;#define brnf_pass_vlan_indev 0#ifdef CONFIG_SYSCTL +static int brnf_net_id__read_mostly ;static struct brnf_net init_brnf_net = { .hdr =NULL , .call_arptables = brnf_call_arptables,
@@ -67,6 +69,11 @@ static struct brnf_net init_brnf_net = { .filter_pppoe_tagged = brnf_filter_pppoe_tagged, .pass_vlan_indev = brnf_pass_vlan_indev, }; + +static inline struct brnf_net *brnf_net (const struct net *net) +{ +return net_generic (net, brnf_net_id); +}#endif #ifdef CONFIG_SYSCTL
@@ -1068,6 +1075,26 @@ static struct ctl_table brnf_table[] = { }, { } }; + +static int __net_init brnf_net_init (struct net *net) +{ +struct brnf_net *bn =brnf_net (net); + +memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); + bn->net = net; +return 0 ; +} + +static void __net_exit brnf_net_exit(struct net *net) +{ +} + +static struct pernet_operations __net_initdata brnf_net_ops = { + .init = brnf_net_init, + .exit = brnf_net_exit, + .id = &brnf_net_id, + .size =sizeof (struct brnf_net ), +};#endif int __init br_netfilter_init (void )
@@ -1076,13 +1103,12 @@ int __init br_netfilter_init(void) ret =dst_entries_init (&fake_dst_ops);if (ret <0 ) -return ret; + goto err_dst; ret =nf_register_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); -if (ret <0 ) { -dst_entries_destroy (&fake_dst_ops); -return ret; - } +if (ret <0 ) + goto err_nf; +#ifdef CONFIG_SYSCTL brnf_sysctl_header =register_net_sysctl (&init_net,"net/bridge" , brnf_table);if (brnf_sysctl_header ==NULL ) {
@@ -1092,16 +1118,28 @@ int __init br_netfilter_init(void)dst_entries_destroy (&fake_dst_ops); return -ENOMEM; } + ret =register_pernet_subsys (&brnf_net_ops); +if (ret <0 ) { +unregister_net_sysctl_table (brnf_sysctl_header); +nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); + goto err_nf; + }#endif printk (KERN_NOTICE "Bridge firewalling registered\n" );return 0 ; + +err_nf: +dst_entries_destroy (&fake_dst_ops); +err_dst: +return ret; }void br_netfilter_fini (void ) { -nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops));#ifdef CONFIG_SYSCTL +unregister_pernet_subsys (&brnf_net_ops);unregister_net_sysctl_table (brnf_sysctl_header);#endif +nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops));dst_entries_destroy (&fake_dst_ops); }
Signature · 1 line
1.7.5.4
RFC PATCH v2 10/11 br_netfilter: per-netns sysctl registration
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 56 +++++++++++++++++++++++++++++++++----------- 1 files changed, 42 insertions(+), 14 deletions(-)
net/bridge/br_netfilter.c +42 -14
@@ -48,9 +48,6 @@#define store_orig_dstaddr (skb) (skb_origaddr(skb) = ip_hdr(skb)->daddr)#define dnat_took_place (skb) (skb_origaddr(skb) != ip_hdr(skb)->daddr) -#ifdef CONFIG_SYSCTL -static struct ctl_table_header *brnf_sysctl_header; -#endif #define brnf_call_arptables 1#define brnf_call_iptables 1#define brnf_call_ip6tables 1
@@ -1076,17 +1073,58 @@ static struct ctl_table brnf_table[] = { { } }; +static int brnf_sysctl_net_register (struct brnf_net *bn) +{ +struct ctl_table *table; +struct ctl_table_header *hdr; +int i; + + table = brnf_table; +if (!net_eq (bn->net, &init_net)) { + + table =kmemdup (table,sizeof (brnf_table),GFP_KERNEL ); +if (!table) + goto err_alloc; + } + hdr =register_net_sysctl (bn->net,"net/bridge" , table); +if (!hdr) + goto err_reg; + + bn->hdr = hdr; +return 0 ; + +err_reg: +if (!net_eq (bn->net, &init_net)) +kfree (table); +err_alloc: +return -ENOMEM ; +} + +static void brnf_sysctl_net_unregister (struct brnf_net *bn) +{ +struct ctl_table *table; + +if (bn->hdr ==NULL ) +return ; + + table = bn->hdr->ctl_table_arg; +unregister_net_sysctl_table (bn->hdr); +if (!net_eq (bn->net, &init_net)) +kfree (table); +} +static int __net_init brnf_net_init (struct net *net) {struct brnf_net *bn =brnf_net (net);memcpy (bn, &init_brnf_net,sizeof (struct brnf_net )); bn->net = net; -return 0 ; +return brnf_sysctl_net_register (bn); } static void __net_exit brnf_net_exit(struct net *net) { +brnf_sysctl_net_unregister (brnf_net (net)); }static struct pernet_operations __net_initdata brnf_net_ops = {
@@ -1110,17 +1148,8 @@ int __init br_netfilter_init(void) goto err_nf;#ifdef CONFIG_SYSCTL - brnf_sysctl_header =register_net_sysctl (&init_net,"net/bridge" , brnf_table); -if (brnf_sysctl_header ==NULL ) { -printk (KERN_WARNING -"br_netfilter: can't register to sysctl.\n" ); -nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); -dst_entries_destroy (&fake_dst_ops); -return -ENOMEM ; - } ret =register_pernet_subsys (&brnf_net_ops);if (ret <0 ) { -unregister_net_sysctl_table (brnf_sysctl_header);nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops)); goto err_nf; }
@@ -1138,7 +1167,6 @@ void br_netfilter_fini(void) {#ifdef CONFIG_SYSCTL unregister_pernet_subsys (&brnf_net_ops); -unregister_net_sysctl_table (brnf_sysctl_header);#endif nf_unregister_hooks (br_nf_ops,ARRAY_SIZE (br_nf_ops));dst_entries_destroy (&fake_dst_ops);
Signature · 1 line
1.7.5.4
RFC PATCH v2 11/11 br_netfilter: switch all sysctls to per-netns processing
In reply to a message not in the archive--- net/bridge/br_netfilter.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
net/bridge/br_netfilter.c +5 -3
@@ -71,10 +71,9 @@ static inline struct brnf_net *brnf_net(const struct net *net) {return net_generic (net, brnf_net_id); } -#endif -#ifdef CONFIG_SYSCTL -#define brnf_flag (skb, flag) init_brnf_net.flag +#define skb_netns (skb) dev_net((skb)->dev) +#define brnf_flag (skb, flag) brnf_net(skb_netns(skb))->flag#else #define brnf_flag (skb, flag) brnf_##flag#endif
@@ -1086,6 +1085,9 @@ static int brnf_sysctl_net_register(struct brnf_net *bn)if (!table) goto err_alloc; } +for (i =0 ; table[i].data; i++) + table[i].data += (char *)bn - (char *)&init_brnf_net; + hdr =register_net_sysctl (bn->net,"net/bridge" , table);if (!hdr) goto err_reg;
Signature · 1 line
1.7.5.4
v2 cover per-netns sysctl for br_netfilter
In reply to a message not in the archiveDear Bart,
thank you for feedback, I've reworked patch set to fix your notes.
This patch set enables per network namespace managemnt for br_netfiltes sysctls, it allows to enable processing br-nf-call hooks in one network namespace and keep it disabled in another ones.
v2: removed extra overhead for CONFIG_SYSCTL=n
Vasily Averin (11):
1 br_netfilter: brnf_net structure for sysctl setting
2 br_netfilter: default sysctl settings in init_brnf_net
3 br_netfilter: brnf_flag macro
4 br_netfilter: switch sysctl call_arptables to init_brnf_net
5 br_netfilter: switch sysctls call_iptables call_ip6tables to init_brnf_net 6 br_netfilter: switch sysctl filter_vlan_tagged to init_brnf_net 7 br_netfilter: switch sysctl filter_pppoe_tagged to init_brnf_net 8 br_netfilter: switch sysctl pass_vlan_indev to init_brnf_net
9 br_netfilter: pernet_operations brnf_net_opts without sysctl registration
10 br_netfilter: per-netns sysctl registration
11 br_netfilter: switch all sysctls to per-netns processing
net/bridge/br_netfilter.c | 155 ++++++++++++++++++++++++++++++++++----------- net/bridge/br_private.h | 13 ++++ 2 files changed, 130 insertions(+), 38 deletions(-)
Signature · 1 line
1.7.5.4