* [PATCH iproute2-net] mptcp: limits: fix JSON support
@ 2026-10-29 13:17 Matthieu Baerts (NGI0)
2026-10-29 17:07 ` Stephen Hemminger
0 siblings, 1 reply; 3+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-10-29 13:17 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern; +Cc: netdev, mptcp, Matthieu Baerts (NGI0)
print_mptcp_limit() was trying to open a JSON sub-object and closing it,
but no JSON object was actually created before.
Because of that, JSON support was working with the MPTCP limits:
$ ip -j mptcp limits show
add_addr_accepted 0 subflows 2
Create a plain object before calling print_mptcp_limit: an array is not
needed here because all limits have a different name.
After this modification, we now have:
$ ip -j mptcp limits show | jq
{
"add_addr_accepted": 0,
"subflows": 2
}
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
ip/ipmptcp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 02cf72ce..33906eb8 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -446,7 +446,9 @@ static int mptcp_limit_get_set(int argc, char **argv, int cmd)
ret = 0;
if (do_get) {
+ new_json_obj_plain(json);
ret = print_mptcp_limit(answer, stdout);
+ delete_json_obj_plain();
free(answer);
}
---
base-commit: d1341d07788e8148ce209107813861321a1fb690
change-id: 20261029-mptcp-limits-json-f60c82b687fa
Best regards,
--
Matthieu Baerts (NGI0) <matttbe@kernel.org>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH iproute2-net] mptcp: limits: fix JSON support
2026-10-29 13:17 [PATCH iproute2-net] mptcp: limits: fix JSON support Matthieu Baerts (NGI0)
@ 2026-10-29 17:07 ` Stephen Hemminger
2026-10-29 17:24 ` Matthieu Baerts
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Hemminger @ 2026-10-29 17:07 UTC (permalink / raw)
To: Matthieu Baerts (NGI0); +Cc: David Ahern, netdev, mptcp
On Thu, 29 Oct 2026 14:17:42 +0100
"Matthieu Baerts (NGI0)" <matttbe@kernel.org> wrote:
> print_mptcp_limit() was trying to open a JSON sub-object and closing it,
> but no JSON object was actually created before.
>
> Because of that, JSON support was working with the MPTCP limits:
>
> $ ip -j mptcp limits show
> add_addr_accepted 0 subflows 2
>
> Create a plain object before calling print_mptcp_limit: an array is not
> needed here because all limits have a different name.
>
> After this modification, we now have:
>
> $ ip -j mptcp limits show | jq
> {
> "add_addr_accepted": 0,
> "subflows": 2
> }
>
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
Should be the regular json_obj variants, not the _plain ones
here.
The AI response was:
On Thu, 29 Oct 2026 14:17:42 +0100, Matthieu Baerts (NGI0) wrote:
> print_mptcp_limit() was trying to open a JSON sub-object and closing it,
> but no JSON object was actually created before.
Right, and that part of the diagnosis is correct -- the open_json_object()
and close_json_object() calls in print_mptcp_limit() were silent no-ops
because no writer existed. Thanks for catching it.
> Create a plain object before calling print_mptcp_limit: an array is not
> needed here because all limits have a different name.
This is where I'd push back. An array isn't needed in the sense that there
is only ever one object, but it is what every other "ip" subcommand emits,
including the other three JSON sites in this same file:
ip/ipmptcp.c:317 new_json_obj(json) mptcp_addr_dump()
ip/ipmptcp.c:345 new_json_obj(json) mptcp_addr_show()
ip/ipmptcp.c:611 new_json_obj(json) mptcp_monitor()
So after this patch "ip mptcp" is inconsistent with itself:
$ ip -j mptcp endpoint show
[]
$ ip -j mptcp limits show
{"add_addr_accepted":0,"subflows":2}
The _plain variant exists for the standalone utilities that have their own
output conventions -- dcb, devlink, dpll, nstat, ifstat, lnstat, vdpa. It
is not used anywhere else under ip/, and I'd rather it stayed that way.
> + new_json_obj_plain(json);
> ret = print_mptcp_limit(answer, stdout);
> + delete_json_obj_plain();
Please use the regular pair instead:
new_json_obj(json);
ret = print_mptcp_limit(answer, stdout);
delete_json_obj();
which gives
$ ip -j mptcp limits show
[{"add_addr_accepted":0,"subflows":2}]
$ ip -j -p mptcp limits show
[
{
"add_addr_accepted": 0,
"subflows": 2
}
]
No other change is needed: print_mptcp_limit() already calls
open_json_object(NULL), which is required either way -- under an array it
creates the element object, under _plain it created the root object. Plain
text output is unchanged in both versions.
Worth doing now rather than later: JSON output for the limits never worked,
so nothing can be depending on the current shape yet. I checked the mptcp
selftests as the likely consumer and they parse text only --
pm_netlink.sh:71 does
ip -n "${ns1}" mptcp limits | awk '{ print $2" "$4 }'
with no -j or --json use anywhere under selftests/net/mptcp/ (the only jq
there parses nft output in mptcp_sockopt.sh). So the shape is still free to
change; once it ships in a release it isn't.
One nit in the commit message while you're respinning:
> Because of that, JSON support was working with the MPTCP limits:
Missing a negation -- "was not working". The example output just below makes
the intent clear, so this alone wouldn't be worth a v2.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH iproute2-net] mptcp: limits: fix JSON support
2026-10-29 17:07 ` Stephen Hemminger
@ 2026-10-29 17:24 ` Matthieu Baerts
0 siblings, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2026-10-29 17:24 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, netdev, mptcp
Hi Stephen,
Thank you for the review!
On 29/10/2026 18:07, Stephen Hemminger wrote:
> On Thu, 29 Oct 2026 14:17:42 +0100
> "Matthieu Baerts (NGI0)" <matttbe@kernel.org> wrote:
>
>> print_mptcp_limit() was trying to open a JSON sub-object and closing it,
>> but no JSON object was actually created before.
>>
>> Because of that, JSON support was working with the MPTCP limits:
>>
>> $ ip -j mptcp limits show
>> add_addr_accepted 0 subflows 2
>>
>> Create a plain object before calling print_mptcp_limit: an array is not
>> needed here because all limits have a different name.
>>
>> After this modification, we now have:
>>
>> $ ip -j mptcp limits show | jq
>> {
>> "add_addr_accepted": 0,
>> "subflows": 2
>> }
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>
> Should be the regular json_obj variants, not the _plain ones
> here.
Mmh, can I disagree with AI? :)
> The AI response was:
>
> On Thu, 29 Oct 2026 14:17:42 +0100, Matthieu Baerts (NGI0) wrote:
>> print_mptcp_limit() was trying to open a JSON sub-object and closing it,
>> but no JSON object was actually created before.
>
> Right, and that part of the diagnosis is correct -- the open_json_object()
> and close_json_object() calls in print_mptcp_limit() were silent no-ops
> because no writer existed. Thanks for catching it.
>
>> Create a plain object before calling print_mptcp_limit: an array is not
>> needed here because all limits have a different name.
>
> This is where I'd push back. An array isn't needed in the sense that there
> is only ever one object, but it is what every other "ip" subcommand emits,
> including the other three JSON sites in this same file:
>
> ip/ipmptcp.c:317 new_json_obj(json) mptcp_addr_dump()
> ip/ipmptcp.c:345 new_json_obj(json) mptcp_addr_show()
> ip/ipmptcp.c:611 new_json_obj(json) mptcp_monitor()
>
> So after this patch "ip mptcp" is inconsistent with itself:
>
> $ ip -j mptcp endpoint show
> []
> $ ip -j mptcp limits show
> {"add_addr_accepted":0,"subflows":2}
Mmh, yes that's "inconsistent", but that's a different output, for a
different purpose:
- for the first one, we have a list of endpoints: a dictionary with
the same keys. We need an extra object (an array, but that could have
been a dict with the ID as key) here to get the different endpoints.
- for the limits, we have a bunch of key-values. Adding an extra array
doesn't make sense.
> The _plain variant exists for the standalone utilities that have their own
> output conventions -- dcb, devlink, dpll, nstat, ifstat, lnstat, vdpa. It
> is not used anywhere else under ip/, and I'd rather it stayed that way.
>
>> + new_json_obj_plain(json);
>> ret = print_mptcp_limit(answer, stdout);
>> + delete_json_obj_plain();
Each output has its own convention, no?
> Please use the regular pair instead:
>
> new_json_obj(json);
> ret = print_mptcp_limit(answer, stdout);
> delete_json_obj();
>
> which gives
>
> $ ip -j mptcp limits show
> [{"add_addr_accepted":0,"subflows":2}]
> $ ip -j -p mptcp limits show
> [
> {
> "add_addr_accepted": 0,
> "subflows": 2
> }
> ]
Here, we can see the array doesn't make sense. Are there other similar
output in 'ip' adding an extra array just to present one dict?
> No other change is needed: print_mptcp_limit() already calls
> open_json_object(NULL), which is required either way -- under an array it
> creates the element object, under _plain it created the root object. Plain
> text output is unchanged in both versions.
>
> Worth doing now rather than later: JSON output for the limits never worked,
> so nothing can be depending on the current shape yet. I checked the mptcp
> selftests as the likely consumer and they parse text only --
> pm_netlink.sh:71 does
>
> ip -n "${ns1}" mptcp limits | awk '{ print $2" "$4 }'
>
> with no -j or --json use anywhere under selftests/net/mptcp/ (the only jq
> there parses nft output in mptcp_sockopt.sh). So the shape is still free to
> change; once it ships in a release it isn't.
>
> One nit in the commit message while you're respinning:
>
>> Because of that, JSON support was working with the MPTCP limits:
>
> Missing a negation -- "was not working". The example output just below makes
> the intent clear, so this alone wouldn't be worth a v2.
Indeed, I can add the missing "not" if a v2 is needed.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-29 17:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 13:17 [PATCH iproute2-net] mptcp: limits: fix JSON support Matthieu Baerts (NGI0)
2026-10-29 17:07 ` Stephen Hemminger
2026-10-29 17:24 ` Matthieu Baerts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox