| Message ID | 1522844653-37136-1-git-send-email-yuvalm@mellanox.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | stephen hemminger |
| Headers | show |
| Series | [iproute2-next] tc: Correct json output for actions | expand |
On 4/4/18 6:24 AM, Yuval Mintz wrote: > Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON > support for tc-actions at the expense of breaking other use cases that > reach tc_print_action(), as the latter don't expect the 'actions' array > to be a new object. > > Consider the following taken duringrun of tc_chain.sh selftest, > and see the latter command output is broken: > > $ ./tc/tc -j -p actions list action gact | grep -C 3 actions > [ { > "total acts": 1 > },{ > "actions": [ { > "order": 0, > > $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions > }, > "skip_hw": true, > "not_in_hw": true,{ > "actions": [ { > "order": 1, > "kind": "gact", > "control_action": { > > Relocate the open/close of the JSON object to declare the object only > for the case that needs it. > > Signed-off-by: Yuval Mintz <yuvalm@mellanox.com> > --- > tc/m_action.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) This is a bug fix so it should go through master. Added Roman so he is aware of the mistake.
Yuval Mintz <yuvalm@mellanox.com> writes: > Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON > support for tc-actions at the expense of breaking other use cases that > reach tc_print_action(), as the latter don't expect the 'actions' array > to be a new object. > > Consider the following taken duringrun of tc_chain.sh selftest, > and see the latter command output is broken: > > $ ./tc/tc -j -p actions list action gact | grep -C 3 actions > [ { > "total acts": 1 > },{ > "actions": [ { > "order": 0, > > $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions > }, > "skip_hw": true, > "not_in_hw": true,{ > "actions": [ { > "order": 1, > "kind": "gact", > "control_action": { > > Relocate the open/close of the JSON object to declare the object only > for the case that needs it. > > Signed-off-by: Yuval Mintz <yuvalm@mellanox.com> [...] Good catch, thanks Yuval. Tested-by: Roman Mashak <mrv@mojatatu.com>
On Wed, 04 Apr 2018 17:09:04 -0400 Roman Mashak <mrv@mojatatu.com> wrote: > Yuval Mintz <yuvalm@mellanox.com> writes: > > > Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON > > support for tc-actions at the expense of breaking other use cases that > > reach tc_print_action(), as the latter don't expect the 'actions' array > > to be a new object. > > > > Consider the following taken duringrun of tc_chain.sh selftest, > > and see the latter command output is broken: > > > > $ ./tc/tc -j -p actions list action gact | grep -C 3 actions > > [ { > > "total acts": 1 > > },{ > > "actions": [ { > > "order": 0, > > > > $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions > > }, > > "skip_hw": true, > > "not_in_hw": true,{ > > "actions": [ { > > "order": 1, > > "kind": "gact", > > "control_action": { > > > > Relocate the open/close of the JSON object to declare the object only > > for the case that needs it. > > > > Signed-off-by: Yuval Mintz <yuvalm@mellanox.com> > > [...] > > > Good catch, thanks Yuval. > > Tested-by: Roman Mashak <mrv@mojatatu.com> Applied
diff --git a/tc/m_action.c b/tc/m_action.c index 2f85d35..8993b93 100644 --- a/tc/m_action.c +++ b/tc/m_action.c @@ -366,7 +366,6 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts) if (tab_flush && NULL != tb[0] && NULL == tb[1]) return tc_print_action_flush(f, tb[0]); - open_json_object(NULL); open_json_array(PRINT_JSON, "actions"); for (i = 0; i <= tot_acts; i++) { if (tb[i]) { @@ -383,7 +382,6 @@ tc_print_action(FILE *f, const struct rtattr *arg, unsigned short tot_acts) } close_json_array(PRINT_JSON, NULL); - close_json_object(); return 0; } @@ -439,8 +437,9 @@ int print_action(const struct sockaddr_nl *who, } } - + open_json_object(NULL); tc_print_action(fp, tb[TCA_ACT_TAB], tot_acts ? *tot_acts:0); + close_json_object(); return 0; }
Commit 9fd3f0b255d9 ("tc: enable json output for actions") added JSON support for tc-actions at the expense of breaking other use cases that reach tc_print_action(), as the latter don't expect the 'actions' array to be a new object. Consider the following taken duringrun of tc_chain.sh selftest, and see the latter command output is broken: $ ./tc/tc -j -p actions list action gact | grep -C 3 actions [ { "total acts": 1 },{ "actions": [ { "order": 0, $ ./tc/tc -p -j -s filter show dev enp3s0np2 ingress | grep -C 3 actions }, "skip_hw": true, "not_in_hw": true,{ "actions": [ { "order": 1, "kind": "gact", "control_action": { Relocate the open/close of the JSON object to declare the object only for the case that needs it. Signed-off-by: Yuval Mintz <yuvalm@mellanox.com> --- tc/m_action.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)