[pve-devel] [PATCH ifupdown2 2/3] fix ipforwarding
Thomas Lamprecht
t.lamprecht at proxmox.com
Wed May 8 14:28:44 CEST 2019
On 5/6/19 6:25 AM, Alexandre Derumier wrote:
> Currently ifupdown2 disable forwarding if not defined with ip-forward
> on interfaces, or if bridge don't have an address (breaking inet dhcp)
>
> We need to keep classic ifupdown behaviour
is there any upstream issue/discussion regarding this?
>
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
> ...e-ipforward-interface-value-if-not-define.patch | 100 +++++++++++++++++++++
> ...autoconfiguration-of-forwarding-of-bridge.patch | 31 +++++++
> debian/patches/series | 2 +
> 3 files changed, 133 insertions(+)
> create mode 100644 debian/patches/pve/0006-don-t-change-ipforward-interface-value-if-not-define.patch
> create mode 100644 debian/patches/pve/0007-disable-autoconfiguration-of-forwarding-of-bridge.patch
>
> diff --git a/debian/patches/pve/0006-don-t-change-ipforward-interface-value-if-not-define.patch b/debian/patches/pve/0006-don-t-change-ipforward-interface-value-if-not-define.patch
> new file mode 100644
> index 0000000..facd462
> --- /dev/null
> +++ b/debian/patches/pve/0006-don-t-change-ipforward-interface-value-if-not-define.patch
> @@ -0,0 +1,100 @@
> +From 053252ec9cc5f60db1cdcb44800690708bb9c11b Mon Sep 17 00:00:00 2001
> +From: Alexandre Derumier <aderumier at odiso.com>
> +Date: Mon, 6 May 2019 06:04:48 +0200
> +Subject: [PATCH] don't change ipforward interface value if not defined
> +
> +Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> +---
> + ifupdown2/addons/address.py | 73 ++++++++++++++++++++-------------------------
> + 1 file changed, 33 insertions(+), 40 deletions(-)
> +
> +diff --git a/ifupdown2/addons/address.py b/ifupdown2/addons/address.py
> +index d3a76cf..a2f86f3 100644
> +--- a/ifupdown2/addons/address.py
> ++++ b/ifupdown2/addons/address.py
> +@@ -715,49 +715,42 @@ class address(moduleBase):
> + self.log_error('%s: \'ip6-forward\' is not supported for '
> + 'bridge port' %ifaceobj.name)
> + return
> +- setting_default_value = False
> +- if not ipforward:
> +- setting_default_value = True
> +- ipforward = (self.ipforward or
> +- self.get_mod_subattr('ip-forward', 'default'))
why not just change the default? I.e.,
----8<----
diff --git a/ifupdown2/addons/address.py b/ifupdown2/addons/address.py
index d3a76cf..96afcb6 100644
--- a/ifupdown2/addons/address.py
+++ b/ifupdown2/addons/address.py
@@ -99,12 +99,12 @@ class address(moduleBase):
'ip-forward' :
{ 'help': 'ip forwarding flag',
'validvals': ['on', 'off', 'yes', 'no', '0', '1'],
- 'default' : 'off',
+ 'default' : 'on',
'example' : ['ip-forward off']},
'ip6-forward' :
{ 'help': 'ipv6 forwarding flag',
'validvals': ['on', 'off', 'yes', 'no', '0', '1'],
- 'default' : 'off',
+ 'default' : 'on',
'example' : ['ip6-forward off']},
'mpls-enable' :
{ 'help': 'mpls enable flag',
--
> +- ipforward = utils.boolean_support_binary(ipforward)
> +- # File read has been used for better performance
> +- # instead of using sysctl command
> +- running_ipforward = self.read_file_oneline(
> +- '/proc/sys/net/ipv4/conf/%s/forwarding'
> +- %ifaceobj.name)
> +- if ipforward != running_ipforward:
> +- try:
> +- self.sysctl_set('net.ipv4.conf.%s.forwarding'
> +- %('/'.join(ifaceobj.name.split("."))),
> +- ipforward)
> +- except Exception as e:
> +- if not setting_default_value:
> ++
> ++ if ipforward:
> ++ ipforward = utils.boolean_support_binary(ipforward)
> ++ # File read has been used for better performance
> ++ # instead of using sysctl command
> ++ running_ipforward = self.read_file_oneline(
> ++ '/proc/sys/net/ipv4/conf/%s/forwarding'
> ++ %ifaceobj.name)
> ++
> ++ if ipforward != running_ipforward:
> ++ try:
> ++
> ++ self.sysctl_set('net.ipv4.conf.%s.forwarding'
> ++ %('/'.join(ifaceobj.name.split("."))),
> ++ ipforward)
> ++ except Exception as e:
> + ifaceobj.status = ifaceStatus.ERROR
> + self.logger.error('%s: %s' %(ifaceobj.name, str(e)))
> +
> +- setting_default_value = False
> +- if not ip6forward:
> +- setting_default_value = True
> +- ip6forward = (self.ip6forward or
> +- self.get_mod_subattr('ip6-forward', 'default'))
> +- ip6forward = utils.boolean_support_binary(ip6forward)
> +- # File read has been used for better performance
> +- # instead of using sysctl command
> +- running_ip6forward = self.read_file_oneline(
> +- '/proc/sys/net/ipv6/conf/%s/forwarding'
> +- %ifaceobj.name)
> +- if ip6forward != running_ip6forward:
> +- try:
> +- self.sysctl_set('net.ipv6.conf.%s.forwarding'
> +- %('/'.join(ifaceobj.name.split("."))),
> +- ip6forward)
> +- except Exception as e:
> +- # There is chance of ipv6 being removed because of,
> +- # for example, setting mtu < 1280
> +- # In such cases, log error only if user has configured
> +- # ip6-forward
> +- if not setting_default_value:
> ++ if ip6forward:
> ++ ip6forward = utils.boolean_support_binary(ip6forward)
> ++ # File read has been used for better performance
> ++ # instead of using sysctl command
> ++ running_ip6forward = self.read_file_oneline(
> ++ '/proc/sys/net/ipv6/conf/%s/forwarding'
> ++ %ifaceobj.name)
> ++ if ip6forward != running_ip6forward:
> ++ try:
> ++ self.sysctl_set('net.ipv6.conf.%s.forwarding'
> ++ %('/'.join(ifaceobj.name.split("."))),
> ++ ip6forward)
> ++ except Exception as e:
> ++ # There is chance of ipv6 being removed because of,
> ++ # for example, setting mtu < 1280
> ++ # In such cases, log error only if user has configured
> ++ # ip6-forward
> + ifaceobj.status = ifaceStatus.ERROR
> + self.logger.error('%s: %s' %(ifaceobj.name, str(e)))
> +
> +--
> +2.11.0
> +
> diff --git a/debian/patches/pve/0007-disable-autoconfiguration-of-forwarding-of-bridge.patch b/debian/patches/pve/0007-disable-autoconfiguration-of-forwarding-of-bridge.patch
> new file mode 100644
> index 0000000..c8f67fb
> --- /dev/null
> +++ b/debian/patches/pve/0007-disable-autoconfiguration-of-forwarding-of-bridge.patch
> @@ -0,0 +1,31 @@
> +From baf59fa021b197fffe9679a15e51a92dd9450e38 Mon Sep 17 00:00:00 2001
> +From: Alexandre Derumier <aderumier at odiso.com>
> +Date: Mon, 6 May 2019 06:13:07 +0200
> +Subject: [PATCH] disable autoconfiguration of forwarding of bridge.
> +
> +Current ifupdwon2 behaviour of ifupdown2 is to disable forwarding
> +on a bridge if no address is defined.
> +
> +This is breaking dhcp interfaces
> +
> +Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> +---
> + ifupdown2/addons/address.py | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/ifupdown2/addons/address.py b/ifupdown2/addons/address.py
> +index a2f86f3..05aaa64 100644
> +--- a/ifupdown2/addons/address.py
> ++++ b/ifupdown2/addons/address.py
> +@@ -690,7 +690,7 @@ class address(moduleBase):
> + self.logger.error('%s: %s' %(ifaceobj.name, str(e)))
> +
> + if (ifaceobj.link_kind & ifaceLinkKind.BRIDGE):
> +- self._set_bridge_forwarding(ifaceobj)
> ++ #self._set_bridge_forwarding(ifaceobj)
hmm, why not changing it in the definition of _set_bridge_forwarding ?
So forwarding would still get enabled if it was disabled in the other case,
where addresses are configured.
> + return
> + if not self.syntax_check_sysctls(ifaceobj):
> + return
> +--
> +2.11.0
> +
> diff --git a/debian/patches/series b/debian/patches/series
> index 5975ab1..e03c55a 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -3,4 +3,6 @@ pve/0002-don-t-remove-tap-veth-fwpr-interfaces-from-bridge-on.patch
> pve/0003-add-dummy-mtu-bridgevlanport-modules.patch
> pve/0004-debian-fixup-networking.service.patch
> pve/0005-allow-vlan-subinterface-in-a-vlan-aware-bridge.patch
> +pve/0006-don-t-change-ipforward-interface-value-if-not-define.patch
> +pve/0007-disable-autoconfiguration-of-forwarding-of-bridge.patch
> extra/0001-addons-bridge-fix-NoneType-object-has-no-attribute-_.patch
>
More information about the pve-devel
mailing list