Merge branch 'crux' of github.com:vyos/vyos-1x into crux

6 files changed, 345 insertions, 95 deletions

diff --git a/src/conf_mode/accel_pppoe.py b/src/conf_mode/accel_pppoe.py
index 31f439c68..97e3723f0 100755
--- a/src/conf_mode/accel_pppoe.py
+++ b/src/conf_mode/accel_pppoe.py
@@ -54,7 +54,7 @@ auth_chap_md5
 auth_mschap_v1
 auth_mschap_v2
 #pppd_compat
-#shaper
+shaper
 {% if snmp == 'enable' or snmp == 'enable-ma' %}
 net-snmp
 {% endif %}
@@ -76,7 +76,7 @@ level=5
 {% if snmp == 'enable-ma' %}
 [snmp]
 master=1
-{% endif %}
+{% endif -%}
 
 [client-ip-range]
 disable
@@ -101,24 +101,24 @@ gw-ip-address={{ppp_gw}}
 {% for prfx in client_ipv6_pool['delegate-prefix']: %}
 delegate={{prfx}}
 {% endfor %}
-{% endif %}
+{% endif -%}
 
 {% if dns %}
 [dns]
 {% if dns[0] %}
 dns1={{dns[0]}}
-{% endif %}
+{% endif -%}
 {% if dns[1] %}
 dns2={{dns[1]}}
-{% endif %}
-{% endif %}
+{% endif -%}
+{% endif -%}
 
 {% if dnsv6 %}
 [dnsv6]
 {% for srv in dnsv6: %}
 dns={{srv}}
 {% endfor %}
-{% endif %}
+{% endif -%}
 
 {% if wins %}
 [wins]
@@ -127,13 +127,13 @@ wins1={{wins[0]}}
 {% endif %}
 {% if wins[1] %}
 wins2={{wins[1]}}
-{% endif %}
-{% endif %}
+{% endif -%}
+{% endif -%}
 
 {% if authentication['mode'] == 'local' %}
 [chap-secrets]
 chap-secrets=/etc/accel-ppp/pppoe/chap-secrets
-{% endif %}
+{% endif -%}
 
 {% if authentication['mode'] == 'radius' %}
 [radius]
@@ -156,14 +156,23 @@ nas-identifier={{authentication['radiusopt']['nas-id']}}
 {% endif %}
 {% if authentication['radiusopt']['nas-ip'] %}
 nas-ip-address={{authentication['radiusopt']['nas-ip']}}
-{% endif %}
+{% endif -%}
 {% if authentication['radiusopt']['dae-srv'] %}
 dae-server={{authentication['radiusopt']['dae-srv']['ip-addr']}}:\
 {{authentication['radiusopt']['dae-srv']['port']}},\
 {{authentication['radiusopt']['dae-srv']['secret']}}
-{% endif %}
+{% endif -%}
 gw-ip-address={{ppp_gw}}
 verbose=1
+
+{% if authentication['radiusopt']['shaper'] %}
+[shaper]
+verbose=1
+attr={{authentication['radiusopt']['shaper']['attr']}}
+{% if authentication['radiusopt']['shaper']['vendor'] %}
+vendor={{authentication['radiusopt']['shaper']['vendor']}}
+{% endif -%}
+{% endif -%}
 {% endif %}
 
 [ppp]
@@ -245,11 +254,16 @@ tcp=127.0.0.1:2001
 
 ### pppoe chap secrets
 chap_secrets_conf = '''
-# username  server  password  acceptable local IP addresses
+# username  server  password  acceptable local IP addresses   shaper
 {% for user in authentication['local-users'] %}
 {% if authentication['local-users'][user]['state'] == 'enabled' %}
+{% if (authentication['local-users'][user]['upload']) and (authentication['local-users'][user]['download']) %}
+{{user}}\t*\t{{authentication['local-users'][user]['passwd']}}\t{{authentication['local-users'][user]['ip']}}\t\
+{{authentication['local-users'][user]['download']}}/{{authentication['local-users'][user]['upload']}}
+{% else %}
 {{user}}\t*\t{{authentication['local-users'][user]['passwd']}}\t{{authentication['local-users'][user]['ip']}}
 {% endif %}
+{% endif %}
 {% endfor %}
 '''
 ###
@@ -389,9 +403,11 @@ def get_config():
         config_data['authentication']['local-users'].update(
           {
             usr : {
-              'passwd' : '',
-              'state'  : 'enabled',
-              'ip'     : '*'
+              'passwd'    : None,
+              'state'     : 'enabled',
+              'ip'        : '*',
+              'upload'    : None,
+              'download'  : None
             }
           }
         )
@@ -401,7 +417,11 @@ def get_config():
           config_data['authentication']['local-users'][usr]['state'] = 'disable'
         if c.exists('authentication local-users username ' + usr + ' static-ip'):
           config_data['authentication']['local-users'][usr]['ip'] = c.return_value('authentication local-users username ' + usr + ' static-ip')
-   
+        if c.exists('authentication local-users username ' + usr + ' rate-limit download'):
+          config_data['authentication']['local-users'][usr]['download'] = c.return_value('authentication local-users username ' + usr + ' rate-limit download')
+        if c.exists('authentication local-users username ' + usr + ' rate-limit upload'):
+          config_data['authentication']['local-users'][usr]['upload'] = c.return_value('authentication local-users username ' + usr + ' rate-limit upload')
+
     ### authentication mode radius servers and settings
 
   if c.exists('authentication mode radius'):
@@ -426,28 +446,42 @@ def get_config():
         }
       )
 
-  #### advanced radius-setting
-      if c.exists('authentication radius-settings'): 
-        if c.exists('authentication radius-settings acct-timeout'):
-          config_data['authentication']['radiusopt']['acct-timeout'] = c.return_value('authentication radius-settings acct-timeout')
-        if c.exists('authentication radius-settings max-try'):
-          config_data['authentication']['radiusopt']['max-try'] = c.return_value('authentication radius-settings max-try') 
-        if c.exists('authentication radius-settings timeout'):
-          config_data['authentication']['radiusopt']['timeout'] = c.return_value('authentication radius-settings timeout')
-        if c.exists('authentication radius-settings nas-identifier'):
-          config_data['authentication']['radiusopt']['nas-id'] = c.return_value('authentication radius-settings nas-identifier')
-        if c.exists('authentication radius-settings nas-ip-address'):
-          config_data['authentication']['radiusopt']['nas-ip'] = c.return_value('authentication radius-settings nas-ip-address')
-        if c.exists('authentication radius-settings dae-server'):
-          config_data['authentication']['radiusopt'].update(
-            {
-              'dae-srv' : {
-                'ip-addr' : c.return_value('authentication radius-settings dae-server ip-address'),
-                'port'    : c.return_value('authentication radius-settings dae-server port'), 
-                'secret'  : str(c.return_value('authentication radius-settings dae-server secret'))
-              }
+    #### advanced radius-setting
+    if c.exists('authentication radius-settings'):
+      if c.exists('authentication radius-settings acct-timeout'):
+        config_data['authentication']['radiusopt']['acct-timeout'] = c.return_value('authentication radius-settings acct-timeout')
+      if c.exists('authentication radius-settings max-try'):
+        config_data['authentication']['radiusopt']['max-try'] = c.return_value('authentication radius-settings max-try')
+      if c.exists('authentication radius-settings timeout'):
+        config_data['authentication']['radiusopt']['timeout'] = c.return_value('authentication radius-settings timeout')
+      if c.exists('authentication radius-settings nas-identifier'):
+        config_data['authentication']['radiusopt']['nas-id'] = c.return_value('authentication radius-settings nas-identifier')
+      if c.exists('authentication radius-settings nas-ip-address'):
+        config_data['authentication']['radiusopt']['nas-ip'] = c.return_value('authentication radius-settings nas-ip-address')
+      if c.exists('authentication radius-settings dae-server'):
+        config_data['authentication']['radiusopt'].update(
+          {
+            'dae-srv' : {
+              'ip-addr' : c.return_value('authentication radius-settings dae-server ip-address'),
+              'port'    : c.return_value('authentication radius-settings dae-server port'),
+              'secret'  : str(c.return_value('authentication radius-settings dae-server secret'))
             }
-          )
+          }
+        )
+      #### filter-id is the internal accel default if attribute is empty
+      #### set here as default for visibility which may change in the future
+      if c.exists('authentication radius-settings rate-limit enable'):
+        if not c.exists('authentication radius-settings rate-limit attribute'):
+          config_data['authentication']['radiusopt']['shaper'] = {
+            'attr'  : 'Filter-Id'
+          }
+        else:
+          config_data['authentication']['radiusopt']['shaper'] = {
+          'attr'  : c.return_value('authentication radius-settings rate-limit attribute')
+          }
+        if c.exists('authentication radius-settings rate-limit vendor'):
+          config_data['authentication']['radiusopt']['shaper']['vendor'] = c.return_value('authentication radius-settings rate-limit vendor')
+
 
   if c.exists('mtu'):
     config_data['mtu'] = c.return_value('mtu')
@@ -496,10 +530,17 @@ def verify(c):
   if c['authentication']['mode'] == 'local':
     if not c['authentication']['local-users']:
       raise ConfigError('pppoe-server authentication local-users required')
-  
+
     for usr in c['authentication']['local-users']:
       if not c['authentication']['local-users'][usr]['passwd']:
         raise ConfigError('user ' + usr + ' requires a password')
+      ### if up/download is set, check that both have a value
+      if c['authentication']['local-users'][usr]['upload']:
+        if not c['authentication']['local-users'][usr]['download']:
+          raise ConfigError('user ' + usr + ' requires download speed value')
+      if c['authentication']['local-users'][usr]['download']:
+        if not c['authentication']['local-users'][usr]['upload']:
+          raise ConfigError('user ' + usr + ' requires upload speed value')
 
   if c['authentication']['mode'] == 'radius':
     if len(c['authentication']['radiussrv']) == 0:
diff --git a/src/conf_mode/igmp_proxy.py b/src/conf_mode/igmp_proxy.py
index b994369af..cd0704124 100755
--- a/src/conf_mode/igmp_proxy.py
+++ b/src/conf_mode/igmp_proxy.py
@@ -20,6 +20,7 @@ import sys
 import os
 import jinja2
 
+from netifaces import interfaces
 from vyos.config import Config
 from vyos import ConfigError
 
@@ -50,17 +51,17 @@ config_tmpl = """
 quickleave
 {% endif -%}
 
-{% for i in interface %}
-# Configuration for {{ i.interface }} ({{ i.role }} interface)
-{% if i.role == 'disabled' -%}
-phyint {{ i.interface }} disabled
+{% for interface in interfaces %}
+# Configuration for {{ interface.name }} ({{ interface.role }} interface)
+{% if interface.role == 'disabled' -%}
+phyint {{ interface.name }} disabled
 {%- else -%}
-phyint {{ i.interface }} {{ i.role }} ratelimit 0 threshold {{ i.threshold }}
+phyint {{ interface.name }} {{ interface.role }} ratelimit 0 threshold {{ interface.threshold }}
 {%- endif -%}
-{%- for subnet in i.alt_subnet %}
+{%- for subnet in interface.alt_subnet %}
         altnet {{ subnet }}
 {%- endfor %}
-{%- for subnet in i.whitelist %}
+{%- for subnet in interface.whitelist %}
         whitelist {{ subnet }}
 {%- endfor %}
 {% endfor %}
@@ -69,7 +70,7 @@ phyint {{ i.interface }} {{ i.role }} ratelimit 0 threshold {{ i.threshold }}
 default_config_data = {
     'disable': False,
     'disable_quickleave': False,
-    'interface': [],
+    'interfaces': [],
 }
 
 def get_config():
@@ -91,7 +92,7 @@ def get_config():
     for intf in conf.list_nodes('interface'):
         conf.set_level('protocols igmp-proxy interface {0}'.format(intf))
         interface = {
-            'interface': intf,
+            'name': intf,
             'alt_subnet': [],
             'role': 'downstream',
             'threshold': '1',
@@ -111,7 +112,7 @@ def get_config():
             interface['whitelist'] = conf.return_values('whitelist')
 
         # Append interface configuration to global configuration list
-        igmp_proxy['interface'].append(interface)
+        igmp_proxy['interfaces'].append(interface)
 
     return igmp_proxy
 
@@ -125,12 +126,14 @@ def verify(igmp_proxy):
         return None
 
     # at least two interfaces are required, one upstream and one downstream
-    if len(igmp_proxy['interface']) < 2:
+    if len(igmp_proxy['interfaces']) < 2:
         raise ConfigError('Must define an upstream and at least 1 downstream interface!')
 
     upstream = 0
-    for i in igmp_proxy['interface']:
-        if "upstream" == i['role']:
+    for interface in igmp_proxy['interfaces']:
+        if interface['name'] not in interfaces():
+            raise ConfigError('Interface "{}" does not exist'.format(interface['name']))
+        if "upstream" == interface['role']:
             upstream += 1
 
     if upstream == 0:
diff --git a/src/conf_mode/vrrp.py b/src/conf_mode/vrrp.py
index a08493309..04bce9d39 100755
--- a/src/conf_mode/vrrp.py
+++ b/src/conf_mode/vrrp.py
@@ -27,7 +27,7 @@ import vyos.keepalived
 
 from vyos import ConfigError
 
-
+daemon_file = "/etc/default/keepalived"
 config_file = "/etc/keepalived/keepalived.conf"
 
 config_tmpl = """
@@ -136,6 +136,14 @@ vrrp_sync_group {{ sync_group.name }} {
 
 """
 
+daemon_tmpl = """
+# Autogenerated by VyOS
+# Options to pass to keepalived
+
+# DAEMON_ARGS are appended to the keepalived command-line
+DAEMON_ARGS="--snmp"
+"""
+
 def get_config():
     vrrp_groups = []
     sync_groups = []
@@ -304,9 +312,12 @@ def generate(data):
 
     tmpl = jinja2.Template(config_tmpl)
     config_text = tmpl.render({"groups": vrrp_groups, "sync_groups": sync_groups})
-    
     with open(config_file, 'w') as f:
         f.write(config_text)
+
+    with open(daemon_file, 'w') as f:
+        f.write(daemon_tmpl)
+
     return None
 
 def apply(data):
diff --git a/src/migration-scripts/quagga/3-to-4 b/src/migration-scripts/quagga/3-to-4
new file mode 100755
index 000000000..be3528391
--- /dev/null
+++ b/src/migration-scripts/quagga/3-to-4
@@ -0,0 +1,76 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 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
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+#
+
+# Between 1.2.3 and 1.2.4, FRR added per-neighbor enforce-first-as option.
+# Unfortunately they also removed the global enforce-first-as option,
+# which broke all old configs that used to have it.
+#
+# To emulate the effect of the original option, we insert it in every neighbor
+# if the config used to have the original global option
+
+import sys
+
+from vyos.configtree import ConfigTree
+
+
+if (len(sys.argv) < 1):
+    print("Must specify file name!")
+    sys.exit(1)
+
+file_name = sys.argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+config = ConfigTree(config_file)
+
+if not config.exists(['protocols', 'bgp']):
+    # Nothing to do
+    sys.exit(0)
+else:
+    # Check if BGP is actually configured and obtain the ASN
+    asn_list = config.list_nodes(['protocols', 'bgp'])
+    if asn_list:
+        # There's always just one BGP node, if any
+        asn = asn_list[0]
+    else:
+        # There's actually no BGP, just its empty shell
+        sys.exit(0)
+
+    # Check if BGP enforce-first-as option is set
+    enforce_first_as_path = ['protocols', 'bgp', asn, 'parameters', 'enforce-first-as']
+    if config.exists(enforce_first_as_path):
+        # Delete the obsolete option
+        config.delete(enforce_first_as_path)
+
+        # Now insert it in every peer
+        peers = config.list_nodes(['protocols', 'bgp', asn, 'neighbor'])
+        for p in peers:
+            config.set(['protocols', 'bgp', asn, 'neighbor', p, 'enforce-first-as'])
+    else:
+        # Do nothing
+        sys.exit(0)
+
+    # Save a new configuration file
+    try:
+        with open(file_name, 'w') as f:
+            f.write(config.to_string())
+    except OSError as e:
+        print("Failed to save the modified config: {}".format(e))
+        sys.exit(1)
+
diff --git a/src/op_mode/show_dhcp.py b/src/op_mode/show_dhcp.py
index 4c4ee6355..c2a05f516 100755
--- a/src/op_mode/show_dhcp.py
+++ b/src/op_mode/show_dhcp.py
@@ -20,6 +20,9 @@ import argparse
 import ipaddress
 import tabulate
 import sys
+import collections
+import os
+from datetime import datetime
 
 from vyos.config import Config
 from isc_dhcp_leases import Lease, IscDhcpLeases
@@ -27,6 +30,18 @@ from isc_dhcp_leases import Lease, IscDhcpLeases
 lease_file = "/config/dhcpd.leases"
 pool_key = "shared-networkname"
 
+lease_display_fields = collections.OrderedDict()
+lease_display_fields['ip'] = 'IP address'
+lease_display_fields['hardware_address'] = 'Hardware address'
+lease_display_fields['state'] = 'State'
+lease_display_fields['start'] = 'Lease start'
+lease_display_fields['end'] = 'Lease expiration'
+lease_display_fields['remaining'] = 'Remaining'
+lease_display_fields['pool'] = 'Pool'
+lease_display_fields['hostname'] = 'Hostname'
+
+lease_valid_states = ['all', 'active', 'free', 'expired', 'released', 'abandoned', 'reset', 'backup']
+
 def in_pool(lease, pool):
     if pool_key in lease.sets:
         if lease.sets[pool_key] == pool:
@@ -34,17 +49,47 @@ def in_pool(lease, pool):
 
     return False
 
+def utc_to_local(utc_dt):
+    return datetime.fromtimestamp((utc_dt - datetime(1970,1,1)).total_seconds())
+
 def get_lease_data(lease):
     data = {}
 
-    # End time may not be present in backup leases
+    # isc-dhcp lease times are in UTC so we need to convert them to local time to display
+    try:
+        data["start"] = utc_to_local(lease.start).strftime("%Y/%m/%d %H:%M:%S")
+    except:
+        data["start"] = ""
+
+    try:
+        data["end"] = utc_to_local(lease.end).strftime("%Y/%m/%d %H:%M:%S")
+    except:
+        data["end"] = ""
+
     try:
-        data["expires"] = lease.end.strftime("%Y/%m/%d %H:%M:%S")
+        data["remaining"] = lease.end - datetime.utcnow()
+        # negative timedelta prints wrong so bypass it
+        if (data["remaining"].days >= 0):
+            # substraction gives us a timedelta object which can't be formatted with strftime
+            # so we use str(), split gets rid of the microseconds
+            data["remaining"] = str(data["remaining"]).split('.')[0]
+        else:
+            data["remaining"] = ""
     except:
-        data["expires"] = ""
+        data["remaining"] = ""
+
+    # currently not used but might come in handy
+    # todo: parse into datetime string
+    for prop in ['tstp', 'tsfp', 'atsfp', 'cltt']:
+        if prop in lease.data:
+            data[prop] = lease.data[prop]
+        else:
+            data[prop] = ''
 
     data["hardware_address"] = lease.ethernet
     data["hostname"] = lease.hostname
+
+    data["state"] = lease.binding_state
     data["ip"] = lease.ip
 
     try:
@@ -54,26 +99,54 @@ def get_lease_data(lease):
 
     return data
 
-def get_leases(leases, state=None, pool=None):
+def get_leases(leases, state, pool=None, sort='ip'):
+    # get leases from file
     leases = IscDhcpLeases(lease_file).get()
 
-    if state is not None:
-        leases = list(filter(lambda x: x.binding_state == 'active', leases))
+    # filter leases by state
+    if 'all' not in state:
+        leases = list(filter(lambda x: x.binding_state in state, leases))
 
+    # filter leases by pool name
     if pool is not None:
-        leases = list(filter(lambda x: in_pool(x, pool), leases))
+        if config.exists_effective("service dhcp-server shared-network-name {0}".format(pool)):
+            leases = list(filter(lambda x: in_pool(x, pool), leases))
+        else:
+            print("Pool {0} does not exist.".format(pool))
+            sys.exit(0)
 
-    return list(map(get_lease_data, leases))
+    # should maybe filter all state=active by lease.valid here?
 
-def show_leases(leases):
-    headers = ["IP address", "Hardware address", "Lease expiration", "Pool", "Client Name"]
+    # sort by start time to dedupe (newest lease overrides older)
+    leases = sorted(leases, key = lambda lease: lease.start)
+
+    # dedupe by converting to dict
+    leases_dict = {}
+    for lease in leases:
+        # dedupe by IP
+        leases_dict[lease.ip] = lease
 
+    # convert the lease data
+    leases = list(map(get_lease_data, leases_dict.values()))
+
+    # apply output/display sort
+    if sort == 'ip':
+        leases = sorted(leases, key = lambda lease: int(ipaddress.ip_address(lease['ip'])))
+    else:
+        leases = sorted(leases, key = lambda lease: lease[sort])
+
+    return leases
+
+def show_leases(leases):
     lease_list = []
     for l in leases:
-        lease_list.append([l["ip"], l["hardware_address"], l["expires"], l["pool"], l["hostname"]])
+        lease_list_params = []
+        for k in lease_display_fields.keys():
+            lease_list_params.append(l[k])
+        lease_list.append(lease_list_params)
+
+    output = tabulate.tabulate(lease_list, lease_display_fields.values())
 
-    output = tabulate.tabulate(lease_list, headers)
-    
     print(output)
 
 def get_pool_size(config, pool):
@@ -85,7 +158,7 @@ def get_pool_size(config, pool):
             start = config.return_effective_value("service dhcp-server shared-network-name {0} subnet {1} range {2} start".format(pool, s, r))
             stop = config.return_effective_value("service dhcp-server shared-network-name {0} subnet {1} range {2} stop".format(pool, s, r))
 
-            size += int(ipaddress.IPv4Address(stop)) - int(ipaddress.IPv4Address(start))
+            size += int(ipaddress.ip_address(stop)) - int(ipaddress.ip_address(start))
 
     return size
 
@@ -101,35 +174,33 @@ if __name__ == '__main__':
     group = parser.add_mutually_exclusive_group()
     group.add_argument("-l", "--leases", action="store_true", help="Show DHCP leases")
     group.add_argument("-s", "--statistics", action="store_true", help="Show DHCP statistics")
+    group.add_argument("--allowed", type=str, choices=["pool", "sort", "state"], help="Show allowed values for argument")
 
-    parser.add_argument("-e", "--expired", action="store_true", help="Show expired leases")
-    parser.add_argument("-p", "--pool", type=str, action="store", help="Show lease for specific pool")
-    parser.add_argument("-j", "--json", action="store_true", default=False, help="Product JSON output")
+    parser.add_argument("-p", "--pool", type=str, help="Show lease for specific pool")
+    parser.add_argument("-S", "--sort", type=str, choices=lease_display_fields.keys(), default='ip', help="Sort by")
+    parser.add_argument("-t", "--state", type=str, nargs="+", choices=lease_valid_states, default="active", help="Lease state to show (can specify multiple with spaces)")
+    parser.add_argument("-j", "--json", action="store_true", default=False, help="Produce JSON output")
 
     args = parser.parse_args()
 
     # Do nothing if service is not configured
     config = Config()
     if not config.exists_effective('service dhcp-server'):
-        print("DHCP service is not configured")
+        print("DHCP service is not configured.")
         sys.exit(0)
 
+    # if dhcp server is down, inactive leases may still be shown as active, so warn the user.
+    if os.system('systemctl -q is-active isc-dhcp-server.service') != 0:
+        print("WARNING: DHCP server is configured but not started. Data may be stale.")
+
     if args.leases:
-        if args.expired:
-            if args.pool:
-                leases = get_leases(lease_file, state='free', pool=args.pool)
-            else:
-                leases = get_leases(lease_file, state='free')
-        else:
-            if args.pool:
-                leases = get_leases(lease_file, state='active', pool=args.pool)
-            else:
-                leases = get_leases(lease_file, state='active')
+        leases = get_leases(lease_file, args.state, args.pool, args.sort)
 
         if args.json:
             print(json.dumps(leases, indent=4))
         else:
             show_leases(leases)
+
     elif args.statistics:
         pools = []
 
@@ -143,10 +214,10 @@ if __name__ == '__main__':
         stats = []
         for p in pools:
             size = get_pool_size(config, p)
-            leases = len(get_leases(lease_file, state='active', pool=args.pool))
+            leases = len(get_leases(lease_file, state='active', pool=p))
 
             if size != 0:
-                use_percentage = round(leases / size) * 100
+                use_percentage = round(leases / size * 100)
             else:
                 use_percentage = 0
 
@@ -163,5 +234,12 @@ if __name__ == '__main__':
             print(json.dumps(stats, indent=4))
         else:
             show_pool_stats(stats)
+
+    elif args.allowed == 'pool':
+        print(' '.join(config.list_effective_nodes("service dhcp-server shared-network-name")))
+    elif args.allowed == 'sort':
+        print(' '.join(lease_display_fields.keys()))
+    elif args.allowed == 'state':
+        print(' '.join(lease_valid_states))
     else:
-        print("Use either --leases or --statistics option")
+        parser.print_help()
diff --git a/src/op_mode/show_dhcpv6.py b/src/op_mode/show_dhcpv6.py
index f1f5a6a55..1a6ee62e6 100755
--- a/src/op_mode/show_dhcpv6.py
+++ b/src/op_mode/show_dhcpv6.py
@@ -21,6 +21,8 @@ import ipaddress
 import tabulate
 import sys
 import collections
+import os
+from datetime import datetime
 
 from vyos.config import Config
 from isc_dhcp_leases import Lease, IscDhcpLeases
@@ -33,6 +35,7 @@ lease_display_fields['ip'] = 'IPv6 address'
 lease_display_fields['state'] = 'State'
 lease_display_fields['last_comm'] = 'Last communication'
 lease_display_fields['expires'] = 'Lease expiration'
+lease_display_fields['remaining'] = 'Remaining'
 lease_display_fields['type'] = 'Type'
 lease_display_fields['pool'] = 'Pool'
 lease_display_fields['iaid_duid'] = 'IAID_DUID'
@@ -57,20 +60,35 @@ def format_hex_string(in_str):
 
     return out_str
 
+def utc_to_local(utc_dt):
+    return datetime.fromtimestamp((utc_dt - datetime(1970,1,1)).total_seconds())
+
 def get_lease_data(lease):
     data = {}
 
-    # End time may not be present in backup leases
+    # isc-dhcp lease times are in UTC so we need to convert them to local time to display
     try:
-        data["expires"] = lease.end.strftime("%Y/%m/%d %H:%M:%S")
+        data["expires"] = utc_to_local(lease.end).strftime("%Y/%m/%d %H:%M:%S")
     except:
         data["expires"] = ""
 
     try:
-        data["last_comm"] = lease.last_communication.strftime("%Y/%m/%d %H:%M:%S")
+        data["last_comm"] = utc_to_local(lease.last_communication).strftime("%Y/%m/%d %H:%M:%S")
     except:
         data["last_comm"] = ""
 
+    try:
+        data["remaining"] = lease.end - datetime.utcnow()
+        # negative timedelta prints wrong so bypass it
+        if (data["remaining"].days >= 0):
+            # substraction gives us a timedelta object which can't be formatted with strftime
+            # so we use str(), split gets rid of the microseconds
+            data["remaining"] = str(data["remaining"]).split('.')[0]
+        else:
+            data["remaining"] = ""
+    except:
+        data["remaining"] = ""
+
     # isc-dhcp records lease declarations as ia_{na|ta|pd} IAID_DUID {...}
     # where IAID_DUID is the combined IAID and DUID
     data["iaid_duid"] = format_hex_string(lease.host_identifier_string)
@@ -91,16 +109,35 @@ def get_lease_data(lease):
 def get_leases(leases, state, pool=None, sort='ip'):
     leases = IscDhcpLeases(lease_file).get()
 
-    if state != 'all':
-        leases = list(filter(lambda x: x.binding_state == state, leases))
+    # filter leases by state
+    if 'all' not in state:
+        leases = list(filter(lambda x: x.binding_state in state, leases))
 
-    # filter lease by pool name
+    # filter leases by pool name
     if pool is not None:
-        leases = list(filter(lambda x: in_pool(x, pool), leases))
+        if config.exists_effective("service dhcp-server shared-network-name {0}".format(pool)):
+            leases = list(filter(lambda x: in_pool(x, pool), leases))
+        else:
+            print("Pool {0} does not exist.".format(pool))
+            sys.exit(0)
 
-    leases = list(map(get_lease_data, leases))
+    # should maybe filter all state=active by lease.valid here?
+
+    # sort by last_comm time to dedupe (newest lease overrides older)
+    leases = sorted(leases, key = lambda lease: lease.last_communication)
+
+    # dedupe by converting to dict
+    leases_dict = {}
+    for lease in leases:
+        # dedupe by IP
+        leases_dict[lease.ip] = lease
+
+    # convert the lease data
+    leases = list(map(get_lease_data, leases_dict.values()))
+
+    # apply output/display sort
     if sort == 'ip':
-        leases = sorted(leases, key = lambda k: int(ipaddress.IPv6Address(k['ip'])))
+        leases = sorted(leases, key = lambda k: int(ipaddress.ip_address(k['ip'])))
     else:
         leases = sorted(leases, key = lambda k: k[sort])
 
@@ -128,7 +165,7 @@ if __name__ == '__main__':
 
     parser.add_argument("-p", "--pool", type=str, help="Show lease for specific pool")
     parser.add_argument("-S", "--sort", type=str, choices=lease_display_fields.keys(), default='ip', help="Sort by")
-    parser.add_argument("-t", "--state", type=str, choices=lease_valid_states, default="active", help="Lease state to show")
+    parser.add_argument("-t", "--state", type=str, nargs="+", choices=lease_valid_states, default="active", help="Lease state to show (can specify multiple with spaces)")
     parser.add_argument("-j", "--json", action="store_true", default=False, help="Produce JSON output")
 
     args = parser.parse_args()
@@ -139,6 +176,10 @@ if __name__ == '__main__':
         print("DHCPv6 service is not configured")
         sys.exit(0)
 
+    # if dhcp server is down, inactive leases may still be shown as active, so warn the user.
+    if os.system('systemctl -q is-active isc-dhcpv6-server.service') != 0:
+        print("WARNING: DHCPv6 server is configured but not started. Data may be stale.")
+
     if args.leases:
         leases = get_leases(lease_file, args.state, args.pool, args.sort)