pppoe: T5630: make MRU default to MTU if unspecified

This fixes the implementation in e062a8c11 ("pppoe: T5630: allow to specify MRU
in addition to already configurable MTU") and restores the bahavior that MRU
defaults to MTU if MRU is not explicitly set.

This was the behavior in VyOS 1.3.3 and below before we added ability to define
the MRU value.

(cherry picked from commit ffd7339e2ea3eafdd97ac0763ca4a3913fe71bf3)

3 files changed, 51 insertions, 11 deletions

diff --git a/interface-definitions/interfaces-pppoe.xml.in b/interface-definitions/interfaces-pppoe.xml.in
index aa2965c65..cdecc6540 100644
--- a/interface-definitions/interfaces-pppoe.xml.in
+++ b/interface-definitions/interfaces-pppoe.xml.in
@@ -116,7 +116,7 @@
           </leafNode>
           <leafNode name="mru">
             <properties>
-              <help>Maximum Receive Unit (MRU)</help>
+              <help>Maximum Receive Unit (MRU) (default: MTU value)</help>
               <valueHelp>
                 <format>u32:128-16384</format>
                 <description>Maximum Receive Unit in byte</description>
@@ -126,7 +126,6 @@
               </constraint>
               <constraintErrorMessage>MRU must be between 128 and 16384</constraintErrorMessage>
             </properties>
-            <defaultValue>1492</defaultValue>
           </leafNode>
           <leafNode name="no-peer-dns">
             <properties>
diff --git a/smoketest/scripts/cli/test_interfaces_pppoe.py b/smoketest/scripts/cli/test_interfaces_pppoe.py
index 2aaccbb13..e46354aee 100755
--- a/smoketest/scripts/cli/test_interfaces_pppoe.py
+++ b/smoketest/scripts/cli/test_interfaces_pppoe.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 #
-# Copyright (C) 2019-2020 VyOS maintainers and contributors
+# Copyright (C) 2019-2023 VyOS maintainers and contributors
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 2 or later as
@@ -58,13 +58,11 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
             user = 'VyOS-user-' + interface
             passwd = 'VyOS-passwd-' + interface
             mtu = '1400'
-            mru = '1300'
 
             self.cli_set(base_path + [interface, 'authentication', 'user', user])
             self.cli_set(base_path + [interface, 'authentication', 'password', passwd])
             self.cli_set(base_path + [interface, 'default-route', 'auto'])
             self.cli_set(base_path + [interface, 'mtu', mtu])
-            self.cli_set(base_path + [interface, 'mru', '9000'])
             self.cli_set(base_path + [interface, 'no-peer-dns'])
 
             # check validate() - a source-interface is required
@@ -72,11 +70,6 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
                 self.cli_commit()
             self.cli_set(base_path + [interface, 'source-interface', self._source_interface])
 
-            # check validate() - MRU needs to be less or equal then MTU
-            with self.assertRaises(ConfigSessionError):
-                self.cli_commit()
-            self.cli_set(base_path + [interface, 'mru', mru])
-
         # commit changes
         self.cli_commit()
 
@@ -88,7 +81,7 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
             tmp = get_config_value(interface, 'mtu')[1]
             self.assertEqual(tmp, mtu)
             tmp = get_config_value(interface, 'mru')[1]
-            self.assertEqual(tmp, mru)
+            self.assertEqual(tmp, mtu)
             tmp = get_config_value(interface, 'user')[1].replace('"', '')
             self.assertEqual(tmp, user)
             tmp = get_config_value(interface, 'password')[1].replace('"', '')
@@ -233,5 +226,47 @@ class PPPoEInterfaceTest(VyOSUnitTestSHIM.TestCase):
             tmp = get_config_value(interface, 'host-uniq')[1]
             self.assertEqual(tmp, f'"{host_uniq}"')
 
+    def test_pppoe_mtu_mru(self):
+        # Check if PPPoE dialer can be configured and runs
+        for interface in self._interfaces:
+            user = f'VyOS-user-{interface}'
+            passwd = f'VyOS-passwd-{interface}'
+            mtu = '1400'
+            mru = '1300'
+
+            self.cli_set(base_path + [interface, 'authentication', 'user', user])
+            self.cli_set(base_path + [interface, 'authentication', 'password', passwd])
+            self.cli_set(base_path + [interface, 'mtu', mtu])
+            self.cli_set(base_path + [interface, 'mru', '9000'])
+
+            # check validate() - a source-interface is required
+            with self.assertRaises(ConfigSessionError):
+                self.cli_commit()
+            self.cli_set(base_path + [interface, 'source-interface', self._source_interface])
+
+            # check validate() - MRU needs to be less or equal then MTU
+            with self.assertRaises(ConfigSessionError):
+                self.cli_commit()
+            self.cli_set(base_path + [interface, 'mru', mru])
+
+        # commit changes
+        self.cli_commit()
+
+        # verify configuration file(s)
+        for interface in self._interfaces:
+            user = f'VyOS-user-{interface}'
+            passwd = f'VyOS-passwd-{interface}'
+
+            tmp = get_config_value(interface, 'mtu')[1]
+            self.assertEqual(tmp, mtu)
+            tmp = get_config_value(interface, 'mru')[1]
+            self.assertEqual(tmp, mru)
+            tmp = get_config_value(interface, 'user')[1].replace('"', '')
+            self.assertEqual(tmp, user)
+            tmp = get_config_value(interface, 'password')[1].replace('"', '')
+            self.assertEqual(tmp, passwd)
+            tmp = get_config_value(interface, 'ifname')[1]
+            self.assertEqual(tmp, interface)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/interfaces-pppoe.py b/src/conf_mode/interfaces-pppoe.py
index 49714c558..7434c5b3b 100755
--- a/src/conf_mode/interfaces-pppoe.py
+++ b/src/conf_mode/interfaces-pppoe.py
@@ -44,6 +44,12 @@ def get_config(config=None):
     base = ['interfaces', 'pppoe']
     pppoe = get_interface_dict(conf, base)
 
+    if 'deleted' not in pppoe:
+        # We always set the MRU value to the MTU size. This code path only re-creates
+        # the old behavior if MRU is not set on the CLI.
+        if 'mru' not in pppoe:
+            pppoe['mru'] = pppoe['mtu']
+
     return pppoe
 
 def verify(pppoe):