netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nft 0/4] json test case fixups
@ 2021-01-21 13:55 Florian Westphal
  2021-01-21 13:55 ` [PATCH 1/4] json: fix icmpv6.t test cases Florian Westphal
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Florian Westphal @ 2021-01-21 13:55 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

json tests fail in several cases because test files were out of data.
This refreshs them.

One bug is genuine: in limit statement, json parser should treat
a missing burst as 5, not 0.

Florian Westphal (4):
  json: fix icmpv6.t test cases
  json: limit: set default burst to 5
  json: ct: add missing rule
  json: icmp: refresh json output

 src/parser_json.c                 |   2 +-
 tests/py/ip/ct.t.json             |  30 ++
 tests/py/ip/icmp.t.json           | 648 ++++++++++++++++++++++++++----
 tests/py/ip6/icmpv6.t.json        |  27 +-
 tests/py/ip6/icmpv6.t.json.output | 586 ++++++++++++++++++++++++++-
 5 files changed, 1196 insertions(+), 97 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] json: fix icmpv6.t test cases
  2021-01-21 13:55 [PATCH nft 0/4] json test case fixups Florian Westphal
@ 2021-01-21 13:55 ` Florian Westphal
  2021-01-21 15:12   ` Phil Sutter
  2021-01-21 13:55 ` [PATCH 2/4] json: limit: set default burst to 5 Florian Westphal
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2021-01-21 13:55 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip6/icmpv6.t.json        |  27 +-
 tests/py/ip6/icmpv6.t.json.output | 586 +++++++++++++++++++++++++++++-
 2 files changed, 597 insertions(+), 16 deletions(-)

diff --git a/tests/py/ip6/icmpv6.t.json b/tests/py/ip6/icmpv6.t.json
index f6cfbf172f56..ffc4931c4e0c 100644
--- a/tests/py/ip6/icmpv6.t.json
+++ b/tests/py/ip6/icmpv6.t.json
@@ -1185,7 +1185,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
 	    "op": "==",
@@ -1203,7 +1203,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
             "op": "!=",
@@ -1221,7 +1221,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
 	    "op": "==",
@@ -1244,7 +1244,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
             "op": "!=",
@@ -1267,7 +1267,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
 	    "op": "==",
@@ -1287,7 +1287,7 @@
             "left": {
                 "payload": {
                     "field": "max-delay",
-                    "name": "icmpv6"
+                    "protocol": "icmpv6"
                 }
             },
             "op": "!=",
@@ -1300,3 +1300,18 @@
     }
 ]
 
+# icmpv6 type packet-too-big icmpv6 mtu 1280
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "mtu",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": 1280
+        }
+    }
+]
diff --git a/tests/py/ip6/icmpv6.t.json.output b/tests/py/ip6/icmpv6.t.json.output
index 3a1066211f56..7b8f5c1909db 100644
--- a/tests/py/ip6/icmpv6.t.json.output
+++ b/tests/py/ip6/icmpv6.t.json.output
@@ -8,7 +8,7 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "mld-listener-done"
         }
     },
@@ -27,7 +27,7 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     "time-exceeded",
@@ -53,7 +53,7 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     "time-exceeded",
@@ -103,7 +103,7 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "port-unreachable"
         }
     }
@@ -119,9 +119,12 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ "addr-unreachable", 66 ]
+                "range": [
+                    "addr-unreachable",
+                    66
+                ]
             }
         }
     }
@@ -137,7 +140,7 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     "policy-fail",
@@ -162,10 +165,15 @@
                     "protocol": "icmpv6"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ "addr-unreachable", 66 ] }
+                    {
+                        "range": [
+                            "addr-unreachable",
+                            66
+                        ]
+                    }
                 ]
             }
         }
@@ -185,7 +193,565 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ "addr-unreachable", 66 ] }
+                    {
+                        "range": [
+                            "addr-unreachable",
+                            66
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id 33-45
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    33,
+                    45
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id != 33-45
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "range": [
+                    33,
+                    45
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id {33, 55, 67, 88}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    33,
+                    55,
+                    67,
+                    88
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id != {33, 55, 67, 88}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "set": [
+                    33,
+                    55,
+                    67,
+                    88
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id {33-55}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 id != {33-55}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "set": [
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence 2
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": 2
+        }
+    }
+]
+
+# icmpv6 sequence {3, 4, 5, 6, 7} accept
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    3,
+                    4,
+                    5,
+                    6,
+                    7
+                ]
+            }
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
+# icmpv6 sequence {2, 4}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    2,
+                    4
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence != {2, 4}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "set": [
+                    2,
+                    4
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence 2-4
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "range": [
+                    2,
+                    4
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence != 2-4
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "range": [
+                    2,
+                    4
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence { 2-4}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    {
+                        "range": [
+                            2,
+                            4
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
+# icmpv6 sequence != { 2-4}
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-request",
+                    "echo-reply"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmpv6"
+                }
+            },
+            "op": "!=",
+            "right": {
+                "set": [
+                    {
+                        "range": [
+                            2,
+                            4
+                        ]
+                    }
                 ]
             }
         }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] json: limit: set default burst to 5
  2021-01-21 13:55 [PATCH nft 0/4] json test case fixups Florian Westphal
  2021-01-21 13:55 ` [PATCH 1/4] json: fix icmpv6.t test cases Florian Westphal
@ 2021-01-21 13:55 ` Florian Westphal
  2021-01-21 14:44   ` Phil Sutter
  2021-01-21 13:55 ` [PATCH 3/4] json: ct: add missing rule Florian Westphal
  2021-01-21 13:55 ` [PATCH 4/4] json: icmp: refresh json output Florian Westphal
  3 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2021-01-21 13:55 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

The tests fail because json printing omits a burst of 5 and
the parser treats that as 'burst 0'.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index f0486b77a225..2d132caf529c 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1784,7 +1784,7 @@ static struct stmt *json_parse_limit_stmt(struct json_ctx *ctx,
 					  const char *key, json_t *value)
 {
 	struct stmt *stmt;
-	uint64_t rate, burst = 0;
+	uint64_t rate, burst = 5;
 	const char *rate_unit = "packets", *time, *burst_unit = "bytes";
 	int inv = 0;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] json: ct: add missing rule
  2021-01-21 13:55 [PATCH nft 0/4] json test case fixups Florian Westphal
  2021-01-21 13:55 ` [PATCH 1/4] json: fix icmpv6.t test cases Florian Westphal
  2021-01-21 13:55 ` [PATCH 2/4] json: limit: set default burst to 5 Florian Westphal
@ 2021-01-21 13:55 ` Florian Westphal
  2021-01-21 15:13   ` Phil Sutter
  2021-01-21 13:55 ` [PATCH 4/4] json: icmp: refresh json output Florian Westphal
  3 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2021-01-21 13:55 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

ERROR: did not find JSON equivalent for rule 'meta mark set ct original ip daddr map { 1.1.1.1 : 0x00000011 }'

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip/ct.t.json | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/py/ip/ct.t.json b/tests/py/ip/ct.t.json
index 881cd4c942c1..d942649a550f 100644
--- a/tests/py/ip/ct.t.json
+++ b/tests/py/ip/ct.t.json
@@ -216,3 +216,33 @@
     }
 ]
 
+# meta mark set ct original ip daddr map { 1.1.1.1 : 0x00000011 }
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "map": {
+                    "data": {
+                        "set": [
+                            [
+                                "1.1.1.1",
+                                17
+                            ]
+                        ]
+                    },
+                    "key": {
+                        "ct": {
+                            "dir": "original",
+                            "key": "ip daddr"
+                        }
+                    }
+                }
+            }
+        }
+    }
+]
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] json: icmp: refresh json output
  2021-01-21 13:55 [PATCH nft 0/4] json test case fixups Florian Westphal
                   ` (2 preceding siblings ...)
  2021-01-21 13:55 ` [PATCH 3/4] json: ct: add missing rule Florian Westphal
@ 2021-01-21 13:55 ` Florian Westphal
  2021-01-21 15:04   ` Phil Sutter
  3 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2021-01-21 13:55 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

nft inserts dependencies for icmp header types, but I forgot to
update the json test files to reflect this change.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip/icmp.t.json | 648 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 568 insertions(+), 80 deletions(-)

diff --git a/tests/py/ip/icmp.t.json b/tests/py/ip/icmp.t.json
index 965eb10be9ed..480740afb525 100644
--- a/tests/py/ip/icmp.t.json
+++ b/tests/py/ip/icmp.t.json
@@ -8,7 +8,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "echo-reply"
         }
     },
@@ -27,7 +27,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "destination-unreachable"
         }
     },
@@ -46,7 +46,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "source-quench"
         }
     },
@@ -65,7 +65,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "redirect"
         }
     },
@@ -84,7 +84,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "echo-request"
         }
     },
@@ -103,7 +103,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "time-exceeded"
         }
     },
@@ -122,7 +122,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "parameter-problem"
         }
     },
@@ -141,7 +141,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "timestamp-request"
         }
     },
@@ -160,7 +160,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "timestamp-reply"
         }
     },
@@ -179,7 +179,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "info-request"
         }
     },
@@ -198,7 +198,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "info-reply"
         }
     },
@@ -217,7 +217,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "address-mask-request"
         }
     },
@@ -236,7 +236,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "address-mask-reply"
         }
     },
@@ -255,7 +255,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "router-advertisement"
         }
     },
@@ -274,7 +274,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": "router-solicitation"
         }
     },
@@ -293,7 +293,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     "echo-reply",
@@ -301,6 +301,8 @@
                     "source-quench",
                     "redirect",
                     "echo-request",
+                    "router-advertisement",
+                    "router-solicitation",
                     "time-exceeded",
                     "parameter-problem",
                     "timestamp-request",
@@ -308,9 +310,7 @@
                     "info-request",
                     "info-reply",
                     "address-mask-request",
-                    "address-mask-reply",
-                    "router-advertisement",
-                    "router-solicitation"
+                    "address-mask-reply"
                 ]
             }
         }
@@ -352,7 +352,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 111
         }
     },
@@ -390,9 +390,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 33, 55 ]
+                "range": [
+                    33,
+                    55
+                ]
             }
         }
     }
@@ -410,7 +413,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 33, 55 ]
+                "range": [
+                    33,
+                    55
+                ]
             }
         }
     }
@@ -426,10 +432,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -449,7 +460,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -466,11 +482,11 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    2,
-                    4,
+                    "prot-unreachable",
+                    "frag-needed",
                     33,
                     54,
                     56
@@ -514,7 +530,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 12343
         }
     },
@@ -552,9 +568,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 11, 343 ]
+                "range": [
+                    11,
+                    343
+                ]
             }
         }
     },
@@ -575,7 +594,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 11, 343 ]
+                "range": [
+                    11,
+                    343
+                ]
             }
         }
     },
@@ -594,10 +616,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 11, 343 ] }
+                    {
+                        "range": [
+                            11,
+                            343
+                        ]
+                    }
                 ]
             }
         }
@@ -620,7 +647,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 11, 343 ] }
+                    {
+                        "range": [
+                            11,
+                            343
+                        ]
+                    }
                 ]
             }
         }
@@ -640,12 +672,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    1111,
                     222,
-                    343
+                    343,
+                    1111
                 ]
             }
         }
@@ -668,9 +700,9 @@
             "op": "!=",
             "right": {
                 "set": [
-                    1111,
                     222,
-                    343
+                    343,
+                    1111
                 ]
             }
         }
@@ -682,6 +714,23 @@
 
 # icmp id 1245 log
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -690,7 +739,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 1245
         }
     },
@@ -701,6 +750,23 @@
 
 # icmp id 22
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -709,7 +775,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 22
         }
     }
@@ -717,6 +783,23 @@
 
 # icmp id != 233
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -733,6 +816,23 @@
 
 # icmp id 33-45
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -741,9 +841,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -751,6 +854,23 @@
 
 # icmp id != 33-45
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -761,7 +881,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -769,6 +892,23 @@
 
 # icmp id { 33-55}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -777,10 +917,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -789,6 +934,23 @@
 
 # icmp id != { 33-55}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -800,7 +962,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -809,6 +976,23 @@
 
 # icmp id { 22, 34, 333}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -817,7 +1001,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     22,
@@ -831,6 +1015,23 @@
 
 # icmp id != { 22, 34, 333}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -853,6 +1054,23 @@
 
 # icmp sequence 22
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -861,7 +1079,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 22
         }
     }
@@ -869,6 +1087,23 @@
 
 # icmp sequence != 233
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -885,6 +1120,23 @@
 
 # icmp sequence 33-45
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -893,9 +1145,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -903,6 +1158,23 @@
 
 # icmp sequence != 33-45
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -913,7 +1185,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -921,6 +1196,23 @@
 
 # icmp sequence { 33, 55, 67, 88}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -929,7 +1221,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     33,
@@ -944,6 +1236,23 @@
 
 # icmp sequence != { 33, 55, 67, 88}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -967,6 +1276,23 @@
 
 # icmp sequence { 33-55}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -975,10 +1301,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -987,6 +1318,23 @@
 
 # icmp sequence != { 33-55}
 [
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
     {
         "match": {
             "left": {
@@ -998,10 +1346,105 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
+# icmp id 1 icmp sequence 2
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
+                ]
+            }
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": 2
+        }
+    }
+]
+
+# icmp type { echo-reply, echo-request} icmp id 1 icmp sequence 2
+[
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "type",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": {
+                "set": [
+                    "echo-reply",
+                    "echo-request"
                 ]
             }
         }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "id",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": 1
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "payload": {
+                    "field": "sequence",
+                    "protocol": "icmp"
+                }
+            },
+            "op": "==",
+            "right": 2
+        }
     }
 ]
 
@@ -1015,7 +1458,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 33
         }
     }
@@ -1031,9 +1474,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 22, 33 ]
+                "range": [
+                    22,
+                    33
+                ]
             }
         }
     }
@@ -1049,10 +1495,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 22, 33 ] }
+                    {
+                        "range": [
+                            22,
+                            33
+                        ]
+                    }
                 ]
             }
         }
@@ -1072,7 +1523,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 22, 33 ] }
+                    {
+                        "range": [
+                            22,
+                            33
+                        ]
+                    }
                 ]
             }
         }
@@ -1089,7 +1545,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 22
         }
     }
@@ -1121,9 +1577,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -1141,7 +1600,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -1157,7 +1619,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     33,
@@ -1203,10 +1665,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -1226,7 +1693,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -1243,7 +1715,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": 22
         }
     }
@@ -1275,9 +1747,12 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -1295,7 +1770,10 @@
             },
             "op": "!=",
             "right": {
-                "range": [ 33, 45 ]
+                "range": [
+                    33,
+                    45
+                ]
             }
         }
     }
@@ -1311,7 +1789,7 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
                     33,
@@ -1357,10 +1835,15 @@
                     "protocol": "icmp"
                 }
             },
-	    "op": "==",
+            "op": "==",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
@@ -1380,7 +1863,12 @@
             "op": "!=",
             "right": {
                 "set": [
-                    { "range": [ 33, 55 ] }
+                    {
+                        "range": [
+                            33,
+                            55
+                        ]
+                    }
                 ]
             }
         }
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] json: limit: set default burst to 5
  2021-01-21 13:55 ` [PATCH 2/4] json: limit: set default burst to 5 Florian Westphal
@ 2021-01-21 14:44   ` Phil Sutter
  2021-01-21 14:57     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 12+ messages in thread
From: Phil Sutter @ 2021-01-21 14:44 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi!

On Thu, Jan 21, 2021 at 02:55:08PM +0100, Florian Westphal wrote:
> The tests fail because json printing omits a burst of 5 and
> the parser treats that as 'burst 0'.

While this patch is correct in that it aligns json and bison parser
behaviours, I think omitting burst value in JSON output is a bug by
itself: We don't care about output length and users are supposed to
parse (and thus filter) the information anyway, so there's no gain from
omitting such info. I'll address this in a separate patch, though.

Thanks, Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] json: limit: set default burst to 5
  2021-01-21 14:44   ` Phil Sutter
@ 2021-01-21 14:57     ` Pablo Neira Ayuso
  2021-01-21 14:59       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2021-01-21 14:57 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

Hi Phil,

On Thu, Jan 21, 2021 at 03:44:14PM +0100, Phil Sutter wrote:
> Hi!
> 
> On Thu, Jan 21, 2021 at 02:55:08PM +0100, Florian Westphal wrote:
> > The tests fail because json printing omits a burst of 5 and
> > the parser treats that as 'burst 0'.
> 
> While this patch is correct in that it aligns json and bison parser
> behaviours, I think omitting burst value in JSON output is a bug by
> itself: We don't care about output length and users are supposed to
> parse (and thus filter) the information anyway, so there's no gain from
> omitting such info. I'll address this in a separate patch, though.

The listing of:

nft list ruleset

is already omitting this. Would you prefer this is also exposed there?

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] json: limit: set default burst to 5
  2021-01-21 14:57     ` Pablo Neira Ayuso
@ 2021-01-21 14:59       ` Pablo Neira Ayuso
  2021-01-21 15:10         ` Phil Sutter
  0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2021-01-21 14:59 UTC (permalink / raw)
  To: Phil Sutter, Florian Westphal, netfilter-devel

On Thu, Jan 21, 2021 at 03:57:59PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Thu, Jan 21, 2021 at 03:44:14PM +0100, Phil Sutter wrote:
> > Hi!
> > 
> > On Thu, Jan 21, 2021 at 02:55:08PM +0100, Florian Westphal wrote:
> > > The tests fail because json printing omits a burst of 5 and
> > > the parser treats that as 'burst 0'.
> > 
> > While this patch is correct in that it aligns json and bison parser
> > behaviours, I think omitting burst value in JSON output is a bug by
> > itself: We don't care about output length and users are supposed to
> > parse (and thus filter) the information anyway, so there's no gain from
> > omitting such info. I'll address this in a separate patch, though.
> 
> The listing of:
> 
> nft list ruleset
> 
> is already omitting this. Would you prefer this is also exposed there?

I mean, IIRC for json it makes sense to display every field (not omit
anything), so my question is whether you think the native syntax
should omit this or it's fine as it is.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] json: icmp: refresh json output
  2021-01-21 13:55 ` [PATCH 4/4] json: icmp: refresh json output Florian Westphal
@ 2021-01-21 15:04   ` Phil Sutter
  0 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2021-01-21 15:04 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

Hi,

On Thu, Jan 21, 2021 at 02:55:10PM +0100, Florian Westphal wrote:
> nft inserts dependencies for icmp header types, but I forgot to
> update the json test files to reflect this change.

For asymmetric JSON output, there are *.t.json.output files. Please add
the missing dependency expressions there.

In general, *.t.json files should contain JSON equivalents for rules as
they are *input* into nft. So we want them to be as close to the
introductory standard syntax comment as possible. This patch "breaks" a
few cases, e.g.:

[...]
> @@ -301,6 +301,8 @@
>                      "source-quench",
>                      "redirect",
>                      "echo-request",
> +                    "router-advertisement",
> +                    "router-solicitation",
>                      "time-exceeded",
>                      "parameter-problem",
>                      "timestamp-request",
> @@ -308,9 +310,7 @@
>                      "info-request",
>                      "info-reply",
>                      "address-mask-request",
> -                    "address-mask-reply",
> -                    "router-advertisement",
> -                    "router-solicitation"
> +                    "address-mask-reply"
>                  ]
>              }
>          }

Input is indeed sorted as prior to this patch. The reordered output is
found in icmp.t.json.output. The benefit from being picky here is minor,
but here's a better example:

[...]
> @@ -466,11 +482,11 @@
>                      "protocol": "icmp"
>                  }
>              },
> -	    "op": "==",
> +            "op": "==",
>              "right": {
>                  "set": [
> -                    2,
> -                    4,
> +                    "prot-unreachable",
> +                    "frag-needed",
>                      33,
>                      54,
>                      56

We test that icmp code values 2 and 4 are accepted. Standard syntax test
covers the asymmetric output containing the names. JSON should do the
same. OTOH, names as input is tested in the negated form of the same
test which follows this one.

Thanks, Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] json: limit: set default burst to 5
  2021-01-21 14:59       ` Pablo Neira Ayuso
@ 2021-01-21 15:10         ` Phil Sutter
  0 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2021-01-21 15:10 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Hi Pablo,

On Thu, Jan 21, 2021 at 03:59:34PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Jan 21, 2021 at 03:57:59PM +0100, Pablo Neira Ayuso wrote:
> > Hi Phil,
> > 
> > On Thu, Jan 21, 2021 at 03:44:14PM +0100, Phil Sutter wrote:
> > > Hi!
> > > 
> > > On Thu, Jan 21, 2021 at 02:55:08PM +0100, Florian Westphal wrote:
> > > > The tests fail because json printing omits a burst of 5 and
> > > > the parser treats that as 'burst 0'.
> > > 
> > > While this patch is correct in that it aligns json and bison parser
> > > behaviours, I think omitting burst value in JSON output is a bug by
> > > itself: We don't care about output length and users are supposed to
> > > parse (and thus filter) the information anyway, so there's no gain from
> > > omitting such info. I'll address this in a separate patch, though.
> > 
> > The listing of:
> > 
> > nft list ruleset
> > 
> > is already omitting this. Would you prefer this is also exposed there?
> 
> I mean, IIRC for json it makes sense to display every field (not omit
> anything), so my question is whether you think the native syntax
> should omit this or it's fine as it is.

You hit the bull's eye: I have a ticket about this behaviour already,
claiming that having a non-trivial default value and omitting it from
output is not a good idea. In practice, reporter created a limit
statement which doesn't work with default burst value (limit rate 1).

I'm not against omitting the burst, but it must not become a problem
then. So my idea to keep the benefits of both was to implement an "auto
burst value" which adjusts to the rate value. Do you think that's
feasible? Maybe something simple like 'burst = max(1, rate / 10)' (for
packets).

Cheers, Phil

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] json: fix icmpv6.t test cases
  2021-01-21 13:55 ` [PATCH 1/4] json: fix icmpv6.t test cases Florian Westphal
@ 2021-01-21 15:12   ` Phil Sutter
  0 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2021-01-21 15:12 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Thu, Jan 21, 2021 at 02:55:07PM +0100, Florian Westphal wrote:
> Signed-off-by: Florian Westphal <fw@strlen.de>

Acked-by: Phil Sutter <phil@nwl.cc>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] json: ct: add missing rule
  2021-01-21 13:55 ` [PATCH 3/4] json: ct: add missing rule Florian Westphal
@ 2021-01-21 15:13   ` Phil Sutter
  0 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2021-01-21 15:13 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Thu, Jan 21, 2021 at 02:55:09PM +0100, Florian Westphal wrote:
> ERROR: did not find JSON equivalent for rule 'meta mark set ct original ip daddr map { 1.1.1.1 : 0x00000011 }'
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Fixes: 8b043938e77b1 ("evaluate: disallow ct original {s,d}ddr from
maps")
Acked-by: Phil Sutter <phil@nwl.cc>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-21 15:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 13:55 [PATCH nft 0/4] json test case fixups Florian Westphal
2021-01-21 13:55 ` [PATCH 1/4] json: fix icmpv6.t test cases Florian Westphal
2021-01-21 15:12   ` Phil Sutter
2021-01-21 13:55 ` [PATCH 2/4] json: limit: set default burst to 5 Florian Westphal
2021-01-21 14:44   ` Phil Sutter
2021-01-21 14:57     ` Pablo Neira Ayuso
2021-01-21 14:59       ` Pablo Neira Ayuso
2021-01-21 15:10         ` Phil Sutter
2021-01-21 13:55 ` [PATCH 3/4] json: ct: add missing rule Florian Westphal
2021-01-21 15:13   ` Phil Sutter
2021-01-21 13:55 ` [PATCH 4/4] json: icmp: refresh json output Florian Westphal
2021-01-21 15:04   ` Phil Sutter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).