On Thu, 29 Oct 2020 21:20:55 +0000
"Sharma, Puneet" <pusha...@akamai.com> wrote:
> I did provide an example to better explain what patch is doing.
>
> Sorry for long paste.
>
> So, with current implementation output of command:
> $ tc -s -d -j filter show dev <eth_name> ingress
>
> would contain:
> [{
> "protocol": "ip",
> "pref": 20000,
> "kind": "basic",
> "chain": 0
> },{
> "protocol": "ip",
> "pref": 20000,
> "kind": "basic",
> "chain": 0,
> "options": {handle 0x1
> (
> cmp(u8 at 9 layer 1 eq 6)
> OR cmp(u8 at 9 layer 1 eq 17)
> ) AND ipset(sg-test-ipv4 src)
>
> "actions": [{
> "order": 1,
> "kind": "gact",
> "control_action": {
> "type": "pass"
> },
> "prob": {
> "random_type": "none",
> "control_action": {
> "type": "pass"
> },
> "val": 0
> },
> "index": 1,
> "ref": 1,
> "bind": 1,
> "installed": 2633,
> "last_used": 2633,
> "stats": {
> "bytes": 0,
> "packets": 0,
> "drops": 0,
> "overlimits": 0,
> "requeues": 0,
> "backlog": 0,
> "qlen": 0
> }
> }]
> }
> }
> ]
>
> Clearly this is an invalid JSON. Look at “options"
>
>
> With patch it would look like:
> [{
> "protocol": "ip",
> "pref": 20000,
> "kind": "basic",
> "chain": 0
> },{
> "protocol": "ip",
> "pref": 20000,
> "kind": "basic",
> "chain": 0,
> "options": {
> "handle": 1,
> "ematch": "(cmp(u8 at 9 layer 1 eq 6)OR cmp(u8 at 9 layer 1 eq
> 17)) AND ipset(sg-test-ipv4 src)",
> "actions": [{
> "order": 1,
> "kind": "gact",
> "control_action": {
> "type": "pass"
> },
> "prob": {
> "random_type": "none",
> "control_action": {
> "type": "pass"
> },
> "val": 0
> },
> "index": 1,
> "ref": 1,
> "bind": 1,
> "installed": 297829723,
> "last_used": 297829723,
> "stats": {
> "bytes": 0,
> "packets": 0,
> "drops": 0,
> "overlimits": 0,
> "requeues": 0,
> "backlog": 0,
> "qlen": 0
> }
> }]
> }
> }
> ]
>
> Now it’s handling the “handle” and “ematch” inside “options" depending on
> context.
>
> Hope it’s more clear now.
>
> Thanks,
> ~Puneet.
>
> > On Oct 29, 2020, at 4:17 PM, Stephen Hemminger <step...@networkplumber.org>
> > wrote:
> >
> > On Wed, 28 Oct 2020 14:35:54 -0400
> > Puneet Sharma <pusha...@akamai.com> wrote:
> >
> >> Currently, json for basic rules output does not produce correct json
> >> syntax. The following fixes were done to correct it for extended
> >> matches for use with "basic" filters.
> >>
> >> tc/f_basic.c: replace fprintf with print_uint to support json output.
> >> fixing this prints "handle" tag correctly in json output.
> >>
> >> tc/m_ematch.c: replace various fprintf with correct print.
> >> add new "ematch" tag for json output which represents
> >> "tc filter add ... basic match '()'" string. Added print_raw_string
> >> to print raw string instead of key value for json.
> >>
> >> lib/json_writer.c: add jsonw_raw_string to print raw text in json.
> >>
> >> lib/json_print.c: add print_color_raw_string to print string
> >> depending on context.
> >>
> >> example:
> >> $ tc -s -d -j filter show dev <eth_name> ingress
> >> Before:
> >> ...
> >> "options": {handle 0x2
> >> (
> >> cmp(u8 at 9 layer 1 eq 6)
> >> OR cmp(u8 at 9 layer 1 eq 17)
> >> ) AND ipset(test-ipv4 src)
> >>
> >> "actions": [{
> >> ...
> >>
> >> After:
> >> [{
> >> ...
> >> "options": {
> >> "handle": 1,
> >> "ematch": "(cmp(u8 at 9 layer 1 eq 6)OR cmp(u8 at 9 layer 1 eq 17)) AND
> >> ipset(test-ipv4 src)",
> >> ...
> >> ]
> >>
> >> Signed-off-by: Puneet Sharma <pusha...@akamai.com>
> >> ---
> >
> > What is the point of introducing raw string?
> > The JSON standard says that string fields must use proper escapes.
> >
> > Please don't emit invalid JSON. It will break consumption by other
> > libraries.
>
I agree that the existing output is wrong. But your patch introduces
+void jsonw_raw_string(json_writer_t *self, const char *value);
Why?
You should just use jsonw_string() which already handles things like special
characters in the string. In theory, there could be quotes or other characters
in the ematch string.
