T5729: T5590: T5616: backport to sagita fwall marks, fix on firewall logs parsing, and migration to valueless node for log and state matchers

17 files changed, 309 insertions, 300 deletions

diff --git a/interface-definitions/include/firewall/common-rule-inet.xml.i b/interface-definitions/include/firewall/common-rule-inet.xml.i
index 88c055f52..52721ecc4 100644
--- a/interface-definitions/include/firewall/common-rule-inet.xml.i
+++ b/interface-definitions/include/firewall/common-rule-inet.xml.i
@@ -3,6 +3,7 @@
 #include <include/generic-description.xml.i>
 #include <include/firewall/dscp.xml.i>
 #include <include/firewall/packet-options.xml.i>
+#include <include/firewall/firewall-mark.xml.i>
 #include <include/firewall/connection-mark.xml.i>
 #include <include/firewall/conntrack-helper.xml.i>
 #include <include/firewall/nft-queue.xml.i>
@@ -81,44 +82,7 @@
     </leafNode>
   </children>
 </node>
-<leafNode name="log">
-  <properties>
-    <help>Option to log packets matching rule</help>
-    <completionHelp>
-      <list>enable disable</list>
-    </completionHelp>
-    <valueHelp>
-      <format>enable</format>
-      <description>Enable log</description>
-    </valueHelp>
-    <valueHelp>
-      <format>disable</format>
-      <description>Disable log</description>
-    </valueHelp>
-    <constraint>
-      <regex>(enable|disable)</regex>
-    </constraint>
-  </properties>
-</leafNode>
-<leafNode name="log">
-  <properties>
-    <help>Option to log packets matching rule</help>
-    <completionHelp>
-      <list>enable disable</list>
-    </completionHelp>
-    <valueHelp>
-      <format>enable</format>
-      <description>Enable log</description>
-    </valueHelp>
-    <valueHelp>
-      <format>disable</format>
-      <description>Disable log</description>
-    </valueHelp>
-    <constraint>
-      <regex>(enable|disable)</regex>
-    </constraint>
-  </properties>
-</leafNode>
+#include <include/firewall/log.xml.i>
 #include <include/firewall/rule-log-options.xml.i>
 <node name="connection-status">
   <properties>
@@ -220,89 +184,7 @@
     </leafNode>
   </children>
 </node>
-<node name="state">
-  <properties>
-    <help>Session state</help>
-  </properties>
-  <children>
-    <leafNode name="established">
-      <properties>
-        <help>Established state</help>
-        <completionHelp>
-          <list>enable disable</list>
-        </completionHelp>
-        <valueHelp>
-          <format>enable</format>
-          <description>Enable</description>
-        </valueHelp>
-        <valueHelp>
-          <format>disable</format>
-          <description>Disable</description>
-        </valueHelp>
-        <constraint>
-          <regex>(enable|disable)</regex>
-        </constraint>
-      </properties>
-    </leafNode>
-    <leafNode name="invalid">
-      <properties>
-        <help>Invalid state</help>
-        <completionHelp>
-          <list>enable disable</list>
-        </completionHelp>
-        <valueHelp>
-          <format>enable</format>
-          <description>Enable</description>
-        </valueHelp>
-        <valueHelp>
-          <format>disable</format>
-          <description>Disable</description>
-        </valueHelp>
-        <constraint>
-          <regex>(enable|disable)</regex>
-        </constraint>
-      </properties>
-    </leafNode>
-    <leafNode name="new">
-      <properties>
-        <help>New state</help>
-        <completionHelp>
-          <list>enable disable</list>
-        </completionHelp>
-        <valueHelp>
-          <format>enable</format>
-          <description>Enable</description>
-        </valueHelp>
-        <valueHelp>
-          <format>disable</format>
-          <description>Disable</description>
-        </valueHelp>
-        <constraint>
-          <regex>(enable|disable)</regex>
-        </constraint>
-      </properties>
-    </leafNode>
-    <leafNode name="related">
-      <properties>
-        <help>Related state</help>
-        <completionHelp>
-          <list>enable disable</list>
-        </completionHelp>
-        <valueHelp>
-          <format>enable</format>
-          <description>Enable</description>
-        </valueHelp>
-        <valueHelp>
-          <format>disable</format>
-          <description>Disable</description>
-        </valueHelp>
-        <constraint>
-          <regex>(enable|disable)</regex>
-        </constraint>
-      </properties>
-    </leafNode>
-  </children>
-</node>
+#include <include/firewall/state.xml.i>
 #include <include/firewall/tcp-flags.xml.i>
 <node name="time">
   <properties>
diff --git a/interface-definitions/include/firewall/firewall-mark.xml.i b/interface-definitions/include/firewall/firewall-mark.xml.i
new file mode 100644
index 000000000..a4cee12d8
--- /dev/null
+++ b/interface-definitions/include/firewall/firewall-mark.xml.i
@@ -0,0 +1,26 @@
+<!-- include start from firewall/firewall-mark.xml.i -->
+<leafNode name="mark">
+  <properties>
+    <help>Firewall mark</help>
+    <valueHelp>
+      <format>u32:0-2147483647</format>
+      <description>Firewall mark to match</description>
+    </valueHelp>
+    <valueHelp>
+      <format>!u32:0-2147483647</format>
+      <description>Inverted Firewall mark to match</description>
+    </valueHelp>
+    <valueHelp>
+      <format>&lt;start-end&gt;</format>
+      <description>Firewall mark range to match</description>
+    </valueHelp>
+    <valueHelp>
+      <format>!&lt;start-end&gt;</format>
+      <description>Firewall mark inverted range to match</description>
+    </valueHelp>
+    <constraint>
+      <validator name="numeric-exclude" argument="--allow-range --range 0-2147483647"/>
+    </constraint>
+  </properties>
+</leafNode>
+<!-- include end -->
diff --git a/interface-definitions/include/firewall/log.xml.i b/interface-definitions/include/firewall/log.xml.i
index 46d20c1df..8f3fae1be 100644
--- a/interface-definitions/include/firewall/log.xml.i
+++ b/interface-definitions/include/firewall/log.xml.i
@@ -1,15 +1,7 @@
 <!-- include start from firewall/log.xml.i -->
-<node name="log">
+<leafNode name="log">
   <properties>
-    <help>Option to log packets</help>
+    <help>Enable log</help>
+    <valueless/>
   </properties>
-  <children>
-    <leafNode name="enable">
-      <properties>
-        <help>Enable logging</help>
-        <valueless/>
-      </properties>
-    </leafNode>
-  </children>
-</node>
-<!-- include end -->
+</leafNode>
\ No newline at end of file
diff --git a/interface-definitions/include/firewall/state.xml.i b/interface-definitions/include/firewall/state.xml.i
new file mode 100644
index 000000000..47ce3c91d
--- /dev/null
+++ b/interface-definitions/include/firewall/state.xml.i
@@ -0,0 +1,30 @@
+<!-- include start from firewall/state.xml.i -->
+<leafNode name="state">
+  <properties>
+    <help>Session state</help>
+    <completionHelp>
+      <list>established invalid new related</list>
+    </completionHelp>
+    <valueHelp>
+      <format>established</format>
+      <description>Established state</description>
+    </valueHelp>
+    <valueHelp>
+      <format>invalid</format>
+      <description>Invalid state</description>
+    </valueHelp>
+    <valueHelp>
+      <format>new</format>
+      <description>New state</description>
+    </valueHelp>
+    <valueHelp>
+      <format>related</format>
+      <description>Related state</description>
+    </valueHelp>
+    <constraint>
+      <regex>(established|invalid|new|related)</regex>
+    </constraint>
+    <multi/>
+  </properties>
+</leafNode>
+<!-- include end -->
\ No newline at end of file
diff --git a/interface-definitions/include/nat-rule.xml.i b/interface-definitions/include/nat-rule.xml.i
index 6234e6195..deb13529d 100644
--- a/interface-definitions/include/nat-rule.xml.i
+++ b/interface-definitions/include/nat-rule.xml.i
@@ -34,12 +34,7 @@
         #include <include/firewall/nat-balance.xml.i>
       </children>
     </node>
-    <leafNode name="log">
-      <properties>
-        <help>NAT rule logging</help>
-        <valueless/>
-      </properties>
-    </leafNode>
+    #include <include/firewall/log.xml.i>
     <leafNode name="packet-type">
       <properties>
         <help>Packet type</help>
diff --git a/interface-definitions/include/policy/route-common.xml.i b/interface-definitions/include/policy/route-common.xml.i
index 216ec9bea..e68c18eef 100644
--- a/interface-definitions/include/policy/route-common.xml.i
+++ b/interface-definitions/include/policy/route-common.xml.i
@@ -1,6 +1,7 @@
 <!-- include start from policy/route-common.xml.i -->

 #include <include/policy/route-rule-action.xml.i>

 #include <include/generic-description.xml.i>

+#include <include/firewall/firewall-mark.xml.i>

 <leafNode name="disable">

   <properties>

     <help>Option to disable firewall rule</help>

@@ -76,25 +77,7 @@
     </leafNode>

   </children>

 </node>

-<leafNode name="log">

-  <properties>

-    <help>Option to log packets matching rule</help>

-    <completionHelp>

-      <list>enable disable</list>

-    </completionHelp>

-    <valueHelp>

-      <format>enable</format>

-      <description>Enable log</description>

-    </valueHelp>

-    <valueHelp>

-      <format>disable</format>

-      <description>Disable log</description>

-    </valueHelp>

-    <constraint>

-      <regex>(enable|disable)</regex>

-    </constraint>

-  </properties>

-</leafNode>

+#include <include/firewall/log.xml.i>

 <leafNode name="protocol">

   <properties>

     <help>Protocol to match (protocol name, number, or "all")</help>

@@ -230,89 +213,7 @@
     </leafNode>

   </children>

 </node>

-<node name="state">

-  <properties>

-    <help>Session state</help>

-  </properties>

-  <children>

-    <leafNode name="established">

-      <properties>

-        <help>Established state</help>

-        <completionHelp>

-          <list>enable disable</list>

-        </completionHelp>

-        <valueHelp>

-          <format>enable</format>

-          <description>Enable</description>

-        </valueHelp>

-        <valueHelp>

-          <format>disable</format>

-          <description>Disable</description>

-        </valueHelp>

-        <constraint>

-          <regex>(enable|disable)</regex>

-        </constraint>

-      </properties>

-    </leafNode>

-    <leafNode name="invalid">

-      <properties>

-        <help>Invalid state</help>

-        <completionHelp>

-          <list>enable disable</list>

-        </completionHelp>

-        <valueHelp>

-          <format>enable</format>

-          <description>Enable</description>

-        </valueHelp>

-        <valueHelp>

-          <format>disable</format>

-          <description>Disable</description>

-        </valueHelp>

-        <constraint>

-          <regex>(enable|disable)</regex>

-        </constraint>

-      </properties>

-    </leafNode>

-    <leafNode name="new">

-      <properties>

-        <help>New state</help>

-        <completionHelp>

-          <list>enable disable</list>

-        </completionHelp>

-        <valueHelp>

-          <format>enable</format>

-          <description>Enable</description>

-        </valueHelp>

-        <valueHelp>

-          <format>disable</format>

-          <description>Disable</description>

-        </valueHelp>

-        <constraint>

-          <regex>(enable|disable)</regex>

-        </constraint>

-      </properties>

-    </leafNode>

-    <leafNode name="related">

-      <properties>

-        <help>Related state</help>

-        <completionHelp>

-          <list>enable disable</list>

-        </completionHelp>

-        <valueHelp>

-          <format>enable</format>

-          <description>Enable</description>

-        </valueHelp>

-        <valueHelp>

-          <format>disable</format>

-          <description>Disable</description>

-        </valueHelp>

-        <constraint>

-          <regex>(enable|disable)</regex>

-        </constraint>

-      </properties>

-    </leafNode>

-  </children>

-</node>

+#include <include/firewall/state.xml.i>

 #include <include/firewall/tcp-flags.xml.i>

 <node name="time">

   <properties>

diff --git a/interface-definitions/include/version/firewall-version.xml.i b/interface-definitions/include/version/firewall-version.xml.i
index 39f0cdcba..299eebb00 100644
--- a/interface-definitions/include/version/firewall-version.xml.i
+++ b/interface-definitions/include/version/firewall-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/firewall-version.xml.i -->
-<syntaxVersion component='firewall' version='12'></syntaxVersion>
+<syntaxVersion component='firewall' version='13'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/include/version/policy-version.xml.i b/interface-definitions/include/version/policy-version.xml.i
index 2c96e0f15..4fbe757f5 100644
--- a/interface-definitions/include/version/policy-version.xml.i
+++ b/interface-definitions/include/version/policy-version.xml.i
@@ -1,3 +1,3 @@
 <!-- include start from include/version/policy-version.xml.i -->
-<syntaxVersion component='policy' version='6'></syntaxVersion>
+<syntaxVersion component='policy' version='7'></syntaxVersion>
 <!-- include end -->
diff --git a/interface-definitions/nat66.xml.in b/interface-definitions/nat66.xml.in
index a657535ba..2fd95e03a 100644
--- a/interface-definitions/nat66.xml.in
+++ b/interface-definitions/nat66.xml.in
@@ -32,12 +32,7 @@
                 </properties>
               </leafNode>
               #include <include/nat-exclude.xml.i>
-              <leafNode name="log">
-                <properties>
-                  <help>NAT66 rule logging</help>
-                  <valueless/>
-                </properties>
-              </leafNode>
+              #include <include/firewall/log.xml.i>
               #include <include/firewall/outbound-interface-no-group.xml.i>
               #include <include/nat/protocol.xml.i>
               <node name="destination">
diff --git a/python/vyos/firewall.py b/python/vyos/firewall.py
index 2ad93a85d..06b58d4ed 100644
--- a/python/vyos/firewall.py
+++ b/python/vyos/firewall.py
@@ -87,7 +87,6 @@ def nft_action(vyos_action):
 
 def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
     output = []
-    #def_suffix = '6' if ip_name == 'ip6' else ''
     if ip_name == 'ip6':
         def_suffix = '6'
         family = 'ipv6'
@@ -96,7 +95,7 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
         family = 'bri' if ip_name == 'bri' else 'ipv4'
 
     if 'state' in rule_conf and rule_conf['state']:
-        states = ",".join([s for s, v in rule_conf['state'].items() if v == 'enable'])
+        states = ",".join([s for s in rule_conf['state']])
 
         if states:
             output.append(f'ct state {{{states}}}')
@@ -262,29 +261,6 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
 
                     output.append(f'{proto} {prefix}port {operator} @P_{group_name}')
 
-    if 'log' in rule_conf and rule_conf['log'] == 'enable':
-        action = rule_conf['action'] if 'action' in rule_conf else 'accept'
-        #output.append(f'log prefix "[{fw_name[:19]}-{rule_id}-{action[:1].upper()}]"')
-        output.append(f'log prefix "[{family}-{hook}-{fw_name}-{rule_id}-{action[:1].upper()}]"')
-
-        if 'log_options' in rule_conf:
-
-            if 'level' in rule_conf['log_options']:
-                log_level = rule_conf['log_options']['level']
-                output.append(f'log level {log_level}')
-
-            if 'group' in rule_conf['log_options']:
-                log_group = rule_conf['log_options']['group']
-                output.append(f'log group {log_group}')
-
-                if 'queue_threshold' in rule_conf['log_options']:
-                    queue_threshold = rule_conf['log_options']['queue_threshold']
-                    output.append(f'queue-threshold {queue_threshold}')
-
-                if 'snapshot_length' in rule_conf['log_options']:
-                    log_snaplen = rule_conf['log_options']['snapshot_length']
-                    output.append(f'snaplen {log_snaplen}')
-
     if 'hop_limit' in rule_conf:
         operators = {'eq': '==', 'gt': '>', 'lt': '<'}
         for op, operator in operators.items():
@@ -400,6 +376,43 @@ def parse_rule(rule_conf, hook, fw_name, rule_id, ip_name):
         conn_mark_str = ','.join(rule_conf['connection_mark'])
         output.append(f'ct mark {{{conn_mark_str}}}')
 
+    if 'mark' in rule_conf:
+        mark = rule_conf['mark']
+        operator = ''
+        if mark[0] == '!':
+            operator = '!='
+            mark = mark[1:]
+        output.append(f'meta mark {operator} {{{mark}}}')
+
+    if 'vlan' in rule_conf:
+        if 'id' in rule_conf['vlan']:
+            output.append(f'vlan id {rule_conf["vlan"]["id"]}')
+        if 'priority' in rule_conf['vlan']:
+            output.append(f'vlan pcp {rule_conf["vlan"]["priority"]}')
+
+    if 'log' in rule_conf:
+        action = rule_conf['action'] if 'action' in rule_conf else 'accept'
+        #output.append(f'log prefix "[{fw_name[:19]}-{rule_id}-{action[:1].upper()}]"')
+        output.append(f'log prefix "[{family}-{hook}-{fw_name}-{rule_id}-{action[:1].upper()}]"')
+                        ##{family}-{hook}-{fw_name}-{rule_id}
+        if 'log_options' in rule_conf:
+
+            if 'level' in rule_conf['log_options']:
+                log_level = rule_conf['log_options']['level']
+                output.append(f'log level {log_level}')
+
+            if 'group' in rule_conf['log_options']:
+                log_group = rule_conf['log_options']['group']
+                output.append(f'log group {log_group}')
+
+                if 'queue_threshold' in rule_conf['log_options']:
+                    queue_threshold = rule_conf['log_options']['queue_threshold']
+                    output.append(f'queue-threshold {queue_threshold}')
+
+                if 'snapshot_length' in rule_conf['log_options']:
+                    log_snaplen = rule_conf['log_options']['snapshot_length']
+                    output.append(f'snaplen {log_snaplen}')
+
     output.append('counter')
 
     if 'set' in rule_conf:
diff --git a/smoketest/scripts/cli/test_firewall.py b/smoketest/scripts/cli/test_firewall.py
index 5f842727d..c51592ec2 100755
--- a/smoketest/scripts/cli/test_firewall.py
+++ b/smoketest/scripts/cli/test_firewall.py
@@ -213,13 +213,13 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'action', 'accept'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'source', 'address', '172.16.20.10'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'destination', 'address', '172.16.10.10'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'log', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'log'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'log-options', 'level', 'debug'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'ttl', 'eq', '15'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'action', 'reject'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'protocol', 'tcp'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'destination', 'port', '8888'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'log', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'log'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'log-options', 'level', 'err'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'tcp', 'flags', 'syn'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'tcp', 'flags', 'not', 'ack'])
@@ -231,7 +231,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '3', 'protocol', 'tcp'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '3', 'destination', 'port', '22'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '3', 'limit', 'rate', '5/minute'])
-        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '3', 'log', 'disable'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '3', 'log'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '4', 'action', 'drop'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '4', 'protocol', 'tcp'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '4', 'destination', 'port', '22'])
@@ -299,7 +299,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'packet-length', '1024'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'dscp', '17'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'dscp', '52'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'log', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'log'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'log-options', 'group', '66'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'log-options', 'snapshot-length', '6666'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '6', 'log-options', 'queue-threshold','32000'])
@@ -312,10 +312,12 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'default-action', 'drop'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'source', 'address', '198.51.100.1'])
+        self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'mark', '1010'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'action', 'jump'])
         self.cli_set(['firewall', 'ipv4', 'forward', 'filter', 'rule', '1', 'jump-target', name])
 
         self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'protocol', 'tcp'])
+        self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'mark', '!98765'])
         self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'action', 'queue'])
         self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '2', 'queue', '3'])
         self.cli_set(['firewall', 'ipv4', 'input', 'filter', 'rule', '3', 'protocol', 'udp'])
@@ -329,11 +331,11 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         nftables_search = [
             ['chain VYOS_FORWARD_filter'],
             ['type filter hook forward priority filter; policy accept;'],
-            ['ip saddr 198.51.100.1', f'jump NAME_{name}'],
+            ['ip saddr 198.51.100.1', 'meta mark 0x000003f2', f'jump NAME_{name}'],
             ['FWD-filter default-action drop', 'drop'],
             ['chain VYOS_INPUT_filter'],
             ['type filter hook input priority filter; policy accept;'],
-            [f'meta l4proto tcp','queue to 3'],
+            ['meta mark != 0x000181cd', 'meta l4proto tcp','queue to 3'],
             ['meta l4proto udp','queue flags bypass,fanout to 0-15'],
             ['INP-filter default-action accept', 'accept'],
             [f'chain NAME_{name}'],
@@ -386,7 +388,7 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'action', 'accept'])
         self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'source', 'address', '2002::1'])
         self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'destination', 'address', '2002::1:1'])
-        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'log', 'enable'])
+        self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'log'])
         self.cli_set(['firewall', 'ipv6', 'name', name, 'rule', '1', 'log-options', 'level', 'crit'])
 
         self.cli_set(['firewall', 'ipv6', 'forward', 'filter', 'default-action', 'accept'])
@@ -508,19 +510,19 @@ class TestFirewall(VyOSUnitTestSHIM.TestCase):
 
         self.cli_set(['firewall', 'ipv4', 'name', name, 'default-action', 'drop'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'state', 'established', 'enable'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'state', 'related', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'state', 'established'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '1', 'state', 'related'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'action', 'reject'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'state', 'invalid', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '2', 'state', 'invalid'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '3', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '3', 'state', 'new', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '3', 'state', 'new'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '3', 'connection-status', 'nat', 'destination'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'state', 'new', 'enable'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'state', 'established', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'state', 'new'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'state', 'established'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '4', 'connection-status', 'nat', 'source'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '5', 'action', 'accept'])
-        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '5', 'state', 'related', 'enable'])
+        self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '5', 'state', 'related'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '5', 'conntrack-helper', 'ftp'])
         self.cli_set(['firewall', 'ipv4', 'name', name, 'rule', '5', 'conntrack-helper', 'pptp'])
 
diff --git a/smoketest/scripts/cli/test_policy_route.py b/smoketest/scripts/cli/test_policy_route.py
index 118b1d3a2..40fff03e6 100755
--- a/smoketest/scripts/cli/test_policy_route.py
+++ b/smoketest/scripts/cli/test_policy_route.py
@@ -191,25 +191,29 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
     def test_pbr_matching_criteria(self):
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'protocol', 'udp'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'action', 'drop'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '1', 'mark', '2020'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '2', 'protocol', 'tcp'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '2', 'tcp', 'flags', 'syn'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '2', 'tcp', 'flags', 'not', 'ack'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '2', 'mark', '2-3000'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '2', 'set', 'table', table_id])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'source', 'address', '198.51.100.0/24'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'protocol', 'tcp'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'destination', 'port', '22'])
-        self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'state', 'new', 'enable'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'state', 'new'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'ttl', 'gt', '2'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'mark', '!456'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '3', 'set', 'table', table_id])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'protocol', 'icmp'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'icmp', 'type-name', 'echo-request'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'packet-length', '128'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'packet-length', '1024-2048'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'packet-type', 'other'])
-        self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'log', 'enable'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'log'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '4', 'set', 'table', table_id])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '5', 'dscp', '41'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '5', 'dscp', '57-59'])
+        self.cli_set(['policy', 'route', 'smoketest', 'rule', '5', 'mark', '!456-500'])
         self.cli_set(['policy', 'route', 'smoketest', 'rule', '5', 'set', 'table', table_id])
 
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '1', 'protocol', 'udp'])
@@ -221,7 +225,7 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'source', 'address', '2001:db8::0/64'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'protocol', 'tcp'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'destination', 'port', '22'])
-        self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'state', 'new', 'enable'])
+        self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'state', 'new'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'hop-limit', 'gt', '2'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '3', 'set', 'table', table_id])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'protocol', 'icmpv6'])
@@ -230,7 +234,7 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'packet-length-exclude', '1024-2048'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'packet-type', 'multicast'])
 
-        self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'log', 'enable'])
+        self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'log'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '4', 'set', 'table', table_id])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '5', 'dscp-exclude', '61'])
         self.cli_set(['policy', 'route6', 'smoketest6', 'rule', '5', 'dscp-exclude', '14-19'])
@@ -247,11 +251,11 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
         # IPv4
         nftables_search = [
             ['iifname { "' + interface + '", "' + interface_wc + '" }', 'jump VYOS_PBR_UD_smoketest'],
-            ['meta l4proto udp', 'drop'],
-            ['tcp flags syn / syn,ack', 'meta mark set ' + mark_hex],
-            ['ct state new', 'tcp dport 22', 'ip saddr 198.51.100.0/24', 'ip ttl > 2', 'meta mark set ' + mark_hex],
-            ['meta l4proto icmp', 'log prefix "[ipv4-route-smoketest-4-A]"', 'icmp type echo-request', 'ip length { 128, 1024-2048 }', 'meta pkttype other', 'meta mark set ' + mark_hex],
-            ['ip dscp { 0x29, 0x39-0x3b }', 'meta mark set ' + mark_hex]
+            ['meta l4proto udp', 'meta mark 0x000007e4', 'drop'],
+            ['tcp flags syn / syn,ack', 'meta mark 0x00000002-0x00000bb8', 'meta mark set ' + mark_hex],
+            ['ct state new', 'tcp dport 22', 'ip saddr 198.51.100.0/24', 'ip ttl > 2', 'meta mark != 0x000001c8', 'meta mark set ' + mark_hex],
+            ['log prefix "[ipv4-route-smoketest-4-A]"', 'icmp type echo-request', 'ip length { 128, 1024-2048 }', 'meta pkttype other', 'meta mark set ' + mark_hex],
+            ['ip dscp { 0x29, 0x39-0x3b }', 'meta mark != 0x000001c8-0x000001f4', 'meta mark set ' + mark_hex]
         ]
 
         self.verify_nftables(nftables_search, 'ip vyos_mangle')
@@ -262,7 +266,7 @@ class TestPolicyRoute(VyOSUnitTestSHIM.TestCase):
             ['meta l4proto udp', 'drop'],
             ['tcp flags syn / syn,ack', 'meta mark set ' + mark_hex],
             ['ct state new', 'tcp dport 22', 'ip6 saddr 2001:db8::/64', 'ip6 hoplimit > 2', 'meta mark set ' + mark_hex],
-            ['meta l4proto ipv6-icmp', 'log prefix "[ipv6-route6-smoketest6-4-A]"', 'icmpv6 type echo-request', 'ip6 length != { 128, 1024-2048 }', 'meta pkttype multicast', 'meta mark set ' + mark_hex],
+            ['log prefix "[ipv6-route6-smoketest6-4-A]"', 'icmpv6 type echo-request', 'ip6 length != { 128, 1024-2048 }', 'meta pkttype multicast', 'meta mark set ' + mark_hex],            
             ['ip6 dscp != { 0x0e-0x13, 0x3d }', 'meta mark set ' + mark_hex]
         ]
 
diff --git a/src/conf_mode/firewall.py b/src/conf_mode/firewall.py
index da22fad68..c4974d249 100755
--- a/src/conf_mode/firewall.py
+++ b/src/conf_mode/firewall.py
@@ -249,7 +249,7 @@ def verify_rule(firewall, rule_conf, ipv6):
                 raise ConfigError(f'{side} port-group and port cannot both be defined')
 
     if 'log_options' in rule_conf:
-        if 'log' not in rule_conf or 'enable' not in rule_conf['log']:
+        if 'log' not in rule_conf:
             raise ConfigError('log-options defined, but log is not enable')
 
         if 'snapshot_length' in rule_conf['log_options'] and 'group' not in rule_conf['log_options']:
diff --git a/src/migration-scripts/firewall/11-to-12 b/src/migration-scripts/firewall/11-to-12
index 51b2fa860..ba8374d66 100755
--- a/src/migration-scripts/firewall/11-to-12
+++ b/src/migration-scripts/firewall/11-to-12
@@ -46,7 +46,6 @@ if not config.exists(base):
     # Nothing to do
     exit(0)
 
-## FORT
 ## Migration from base chains
 #if config.exists(base + ['interface', iface, direction]):
 for family in ['ipv4', 'ipv6']:
diff --git a/src/migration-scripts/firewall/12-to-13 b/src/migration-scripts/firewall/12-to-13
new file mode 100755
index 000000000..4eaae779b
--- /dev/null
+++ b/src/migration-scripts/firewall/12-to-13
@@ -0,0 +1,83 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 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
+# 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/>.
+
+# T5729: Switch to valueless whenever is possible.
+# From
+    # set firewall ... rule <rule> log enable
+    # set firewall ... rule <rule> state <state> enable
+    # set firewall ... rule <rule> log disable
+    # set firewall ... rule <rule> state <state> disable
+# To
+    # set firewall ... rule <rule> log
+    # set firewall ... rule <rule> state <state>
+    # Remove command if log=disable or <state>=disable
+
+import re
+
+from sys import argv
+from sys import exit
+
+from vyos.configtree import ConfigTree
+from vyos.ifconfig import Section
+
+if len(argv) < 2:
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+base = ['firewall']
+config = ConfigTree(config_file)
+
+if not config.exists(base):
+    # Nothing to do
+    exit(0)
+
+for family in ['ipv4', 'ipv6', 'bridge']:
+    if config.exists(base + [family]):
+        for hook in ['forward', 'input', 'output', 'name']:
+            if config.exists(base + [family, hook]):
+                for priority in config.list_nodes(base + [family, hook]):
+                    if config.exists(base + [family, hook, priority, 'rule']):
+                        for rule in config.list_nodes(base + [family, hook, priority, 'rule']):
+                            # Log
+                            if config.exists(base + [family, hook, priority, 'rule', rule, 'log']):
+                                log_value = config.return_value(base + [family, hook, priority, 'rule', rule, 'log'])
+                                config.delete(base + [family, hook, priority, 'rule', rule, 'log'])
+                                if log_value == 'enable':
+                                    config.set(base + [family, hook, priority, 'rule', rule, 'log'])
+                            # State
+                            if config.exists(base + [family, hook, priority, 'rule', rule, 'state']):
+                                flag_enable = 'False'
+                                for state in ['established', 'invalid', 'new', 'related']:
+                                    if config.exists(base + [family, hook, priority, 'rule', rule, 'state', state]):
+                                        state_value = config.return_value(base + [family, hook, priority, 'rule', rule, 'state', state])
+                                        config.delete(base + [family, hook, priority, 'rule', rule, 'state', state])
+                                        if state_value == 'enable':
+                                            config.set(base + [family, hook, priority, 'rule', rule, 'state'], value=state, replace=False)
+                                            flag_enable = 'True'
+                                if flag_enable == 'False':
+                                    config.delete(base + [family, hook, priority, 'rule', rule, 'state'])
+
+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))
+    exit(1)
\ No newline at end of file
diff --git a/src/migration-scripts/policy/6-to-7 b/src/migration-scripts/policy/6-to-7
new file mode 100755
index 000000000..e07822ffd
--- /dev/null
+++ b/src/migration-scripts/policy/6-to-7
@@ -0,0 +1,79 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 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
+# 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/>.
+
+# T5729: Switch to valueless whenever is possible.
+# From
+    # set policy [route | route6] ... rule <rule> log enable
+    # set policy [route | route6] ... rule <rule> log disable
+# To
+    # set policy [route | route6] ... rule <rule> log
+    # Remove command if log=disable
+
+import re
+
+from sys import argv
+from sys import exit
+
+from vyos.configtree import ConfigTree
+from vyos.ifconfig import Section
+
+if len(argv) < 2:
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+base = ['policy']
+config = ConfigTree(config_file)
+
+if not config.exists(base):
+    # Nothing to do
+    exit(0)
+
+for family in ['route', 'route6']:
+    if config.exists(base + [family]):
+
+        for policy_name in config.list_nodes(base + [family]):
+            if config.exists(base + [family, policy_name, 'rule']):
+                for rule in config.list_nodes(base + [family, policy_name, 'rule']):
+                    # Log
+                    if config.exists(base + [family, policy_name, 'rule', rule, 'log']):
+                        log_value = config.return_value(base + [family, policy_name, 'rule', rule, 'log'])
+                        config.delete(base + [family, policy_name, 'rule', rule, 'log'])
+                        if log_value == 'enable':
+                            config.set(base + [family, policy_name, 'rule', rule, 'log'])
+                    # State
+                    if config.exists(base + [family, policy_name, 'rule', rule, 'state']):
+                        flag_enable = 'False'
+                        for state in ['established', 'invalid', 'new', 'related']:
+                            if config.exists(base + [family, policy_name, 'rule', rule, 'state', state]):
+                                state_value = config.return_value(base + [family, policy_name, 'rule', rule, 'state', state])
+                                config.delete(base + [family, policy_name, 'rule', rule, 'state', state])
+                                if state_value == 'enable':
+                                    config.set(base + [family, policy_name, 'rule', rule, 'state'], value=state, replace=False)
+                                    flag_enable = 'True'
+                        if flag_enable == 'False':
+                            config.delete(base + [family, policy_name, 'rule', rule, 'state'])
+
+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))
+    exit(1)
\ No newline at end of file
diff --git a/src/validators/numeric-exclude b/src/validators/numeric-exclude
new file mode 100755
index 000000000..d693ad16d
--- /dev/null
+++ b/src/validators/numeric-exclude
@@ -0,0 +1,8 @@
+#!/bin/sh
+path=$(dirname "$0")
+num="${@: -1}"
+if [ "${num:0:1}" != "!" ]; then
+   ${path}/numeric $@
+else
+    ${path}/numeric ${@:1:$#-1} ${num:1}
+fi
\ No newline at end of file