Re: [PATCH iproute2-net] mptcp: limits: fix JSON support

[Matthieu Baerts <matttbe@kernel.org>]

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.