From 94f886ab0f98207fee57c11e78b94240b1ffff8e Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 15 Jan 2021 19:25:04 +0100 Subject: tunnel: T3173: path MTU discovery option should be valueless --- interface-definitions/interfaces-tunnel.xml.in | 48 ++++---- python/vyos/ifconfig/tunnel.py | 145 ++++++++++++++++--------- src/conf_mode/interfaces-tunnel.py | 23 +++- 3 files changed, 136 insertions(+), 80 deletions(-) diff --git a/interface-definitions/interfaces-tunnel.xml.in b/interface-definitions/interfaces-tunnel.xml.in index 273aa648e..39e274840 100644 --- a/interface-definitions/interfaces-tunnel.xml.in +++ b/interface-definitions/interfaces-tunnel.xml.in @@ -177,44 +177,42 @@ IPv4 specific tunnel parameters - + - Path MTU Discovery parameters - - disable enable - - - disable - Disable Path MTU Discovery for tunnel - - - enable - Enable Path MTU Discovery for tunnel (by default) - - - ^(disable|enable)$ - + Disable path MTU discovery + - Time to live field + Time to live (default: 0) - 0-255 - Time to live (default 255) + 0 + Copy value from original IP header + + + 1-255 + Time to Live TTL must be between 0 and 255 - 255 + 0 - Type of Service (TOS) + Type of Service (default: 0) + + inherit + + + 0 + Copy value from original IP header + - 0-99 + 1-99 Type of Service (TOS) @@ -222,7 +220,7 @@ TOS must be between 0 and 99 - inherit + 0 @@ -234,7 +232,7 @@ - key must be between 0-4294967295 + Key must be in range 0-4294967295 @@ -249,7 +247,7 @@ Encaplimit field 0-255 - Encaplimit (default 4) + Encaplimit (default: 4) diff --git a/python/vyos/ifconfig/tunnel.py b/python/vyos/ifconfig/tunnel.py index 1af4f8e72..7e3f9565a 100644 --- a/python/vyos/ifconfig/tunnel.py +++ b/python/vyos/ifconfig/tunnel.py @@ -48,31 +48,42 @@ class _Tunnel(Interface): }, } + default = { + 'local' : '', + 'remote': '', + 'dev' : '', + 'ttl' : '', + 'tos' : '', + 'key' : '', + } + options = Interface.options + list(default.keys()) + # TODO: This is surely used for more than tunnels # TODO: could be refactored elsewhere - _command_set = {**Interface._command_set, **{ - 'multicast': { - 'validate': lambda v: assert_list(v, ['enable', 'disable']), - 'convert': enable_to_on, - 'shellcmd': 'ip link set dev {ifname} multicast {value}', - }, - 'allmulticast': { - 'validate': lambda v: assert_list(v, ['enable', 'disable']), - 'convert': enable_to_on, - 'shellcmd': 'ip link set dev {ifname} allmulticast {value}', - }, - }} - + _command_set = { + **Interface._command_set, + **{ + 'multicast': { + 'validate': lambda v: assert_list(v, ['enable', 'disable']), + 'convert': enable_to_on, + 'shellcmd': 'ip link set dev {ifname} multicast {value}', + }, + 'allmulticast': { + 'validate': lambda v: assert_list(v, ['enable', 'disable']), + 'convert': enable_to_on, + 'shellcmd': 'ip link set dev {ifname} allmulticast {value}', + }, + } + } _create_cmd = 'ip tunnel add {ifname} mode {type}' - def __init__(self, ifname, **config): - self.config = deepcopy(config) if config else {} - super().__init__(ifname, **config) - def _create(self): # add " option-name option-name-value ..." for all options set - options = " ".join(["{} {}".format(k, self.config[k]) - for k in self.options if k in self.config and self.config[k]]) + options = ' '.join(['{} {}'.format(k, self.config[k]) + for k,v in self.config.items() if v and k not in + ['ifname', 'type', 'raw']]) + if 'raw' in self.config: + options += ' ' + ' '.join(self.config['raw']) self._cmd('{} {}'.format(self._create_cmd.format(**self.config), options)) self.set_admin_state('down') @@ -80,14 +91,13 @@ class _Tunnel(Interface): change = 'ip tunnel change {ifname} mode {type}' # add " option-name option-name-value ..." for all options set - options = " ".join(["{} {}".format(k, self.config[k]) - for k in self.options if k in self.config and self.config[k]]) + options = ' '.join(['{} {}'.format(k, self.config[k]) + for k,v in self.config.items() if v and k not in + ['ifname', 'type', 'raw']]) + if 'raw' in self.config: + options += ' ' + ' '.join(self.config['raw']) self._cmd('{} {}'.format(change.format(**self.config), options)) - @classmethod - def get_config(cls): - return dict(zip(cls.options, ['']*len(cls.options))) - def get_mac(self): """ Get current interface MAC (Media Access Contrl) address used. @@ -141,8 +151,13 @@ class GREIf(_Tunnel): https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/link_gre.c """ - default = {'type': 'gre'} - options = ['local', 'remote', 'dev', 'ttl', 'tos', 'key'] + default = { + **_Tunnel.default, + **{ + 'type': 'gre', + 'raw' : ['pmtudisc'], # parameters that we can pass raw to ip command + }, + } # GreTap also called GRE Bridge class GRETapIf(_Tunnel): @@ -151,18 +166,20 @@ class GRETapIf(_Tunnel): https://en.wikipedia.org/wiki/TUN/TAP """ - # no multicast, ttl or tos for gretap - definition = { **_Tunnel.definition, **{ 'bridgeable': True, }, } - - default = {'type': 'gretap'} - options = ['local', 'remote', 'ttl',] + default = { + 'type': 'gretap', + 'local': '', + 'remote': '', + 'dev': '', + 'raw' : ['pmtudisc'], # parameters that we can pass raw to ip command + } _create_cmd = 'ip link add name {ifname} type {type}' @@ -177,10 +194,16 @@ class IP6GREIf(_Tunnel): https://tools.ietf.org/html/rfc7676 https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/link_gre6.c """ - - default = {'type': 'ip6gre'} - options = ['local', 'remote', 'dev', 'encaplimit', - 'hoplimit', 'tclass', 'flowlabel'] + default = { + **_Tunnel.default, + **{ + 'type': 'ip6gre', + 'encaplimit': '', + 'hoplimit': '', + 'tclass': '', + 'flowlabel': '', + }, + } class IPIPIf(_Tunnel): """ @@ -189,12 +212,15 @@ class IPIPIf(_Tunnel): For more information please refer to: https://tools.ietf.org/html/rfc2003 """ - # IPIP does not allow to pass multicast, unlike GRE # but the interface itself can be set with multicast - - default = {'type': 'ipip'} - options = ['local', 'remote', 'dev', 'ttl', 'tos', 'key'] + default = { + **_Tunnel.default, + **{ + 'type': 'ipip', + 'raw' : ['pmtudisc'], # parameters that we can pass raw to ip command + }, + } class IPIP6If(_Tunnel): """ @@ -203,10 +229,16 @@ class IPIP6If(_Tunnel): For more information please refer to: https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/link_ip6tnl.c """ - - default = {'type': 'ipip6'} - options = ['local', 'remote', 'dev', 'encaplimit', - 'hoplimit', 'tclass', 'flowlabel'] + default = { + **_Tunnel.default, + **{ + 'type': 'ipip6', + 'encaplimit': '', + 'hoplimit': '', + 'tclass': '', + 'flowlabel': '', + }, + } class IP6IP6If(IPIP6If): """ @@ -215,7 +247,12 @@ class IP6IP6If(IPIP6If): For more information please refer to: https://tools.ietf.org/html/rfc2473 """ - default = {'type': 'ip6ip6'} + default = { + **_Tunnel.default, + **{ + 'type': 'ip6ip6', + }, + } class SitIf(_Tunnel): @@ -225,9 +262,12 @@ class SitIf(_Tunnel): For more information please refer to: https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/ip/link_iptnl.c """ - - default = {'type': 'sit'} - options = ['local', 'remote', 'dev', 'ttl', 'tos', 'key'] + default = { + **_Tunnel.default, + **{ + 'type': 'sit', + }, + } class Sit6RDIf(SitIf): """ @@ -236,7 +276,14 @@ class Sit6RDIf(SitIf): https://en.wikipedia.org/wiki/IPv6_rapid_deployment """ # TODO: check if key can really be used with 6RD - options = ['remote', 'ttl', 'tos', 'key', '6rd-prefix', '6rd-relay-prefix'] + default = { + **_Tunnel.default, + **{ + 'type': '6rd', + '6rd_prefix' : '', + '6rd_relay_prefix' : '', + }, + } def _create(self): # do not call _Tunnel.create, building fully here diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index f79405388..d2fcf3121 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -126,10 +126,14 @@ def verify(tunnel): if 'source_interface' in tunnel: verify_interface_exists(tunnel['source_interface']) - # ttl != 0 and nopmtudisc are incompatible - if 'pmtu_discovery' in tunnel['parameters']['ip']: - if 'disable' in tunnel['parameters']['ip']['pmtu_discovery'] and "0" not in tunnel['parameters']['ip']['ttl']: - raise ConfigError('ip ttl should be set to "0" with option "pmtu-discovery disable"') + # TTL != 0 and nopmtudisc are incompatible, parameters and ip use default + # values, thus the keys are always present. + if dict_search('parameters.ip.no_pmtu_discovery', tunnel) != None: + if dict_search('parameters.ip.ttl', tunnel) != '0': + raise ConfigError('Disabled PMTU requires TTL set to "0"!') + if tunnel['encapsulation'] in ['ipip6', 'ip6ip6', 'ip6gre']: + raise ConfigError('Can not disable PMTU discovery for given encapsulation') + def generate(tunnel): return None @@ -169,7 +173,6 @@ def apply(tunnel): 'local_ip' : 'local', 'remote_ip' : 'remote', 'source_interface' : 'dev', - 'parameters.ip.pmtu_discovery' : 'pmtud', 'parameters.ip.ttl' : 'ttl', 'parameters.ip.tos' : 'tos', 'parameters.ip.key' : 'key', @@ -180,7 +183,10 @@ def apply(tunnel): if tunnel['encapsulation'] in ['ipip6', 'ip6ip6', 'ip6gre']: mappingv6 = { # this : get_config() - 'parameters.ipv6.encaplimit' : 'encaplimit' + 'parameters.ipv6.encaplimit' : 'encaplimit', + 'parameters.ipv6.flowlabel' : 'flowlabel', + 'parameters.ipv6.hoplimit' : 'hoplimit', + 'parameters.ipv6.tclass' : 'flowlabel' } mapping.update(mappingv6) @@ -188,6 +194,11 @@ def apply(tunnel): if dict_search(our_key, tunnel) and their_key in conf: conf[their_key] = dict_search(our_key, tunnel) + if dict_search('parameters.ip.no_pmtu_discovery', tunnel) != None: + if 'pmtudisc' in conf['raw']: + conf['raw'].remove('pmtudisc') + conf['raw'].append('nopmtudisc') + tun = klass(tunnel['ifname'], **conf) tun.change_options() tun.update(tunnel) -- cgit v1.2.3