Re: [PATCH] net: Add "info neighbors" command

[Markus Armbruster]

Doug Evans <dje@google.com> writes:

> On Fri, Sep 3, 2021 at 6:08 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Doug Evans <dje@google.com> writes:
>>
>> > This command dumps the ARP and NDP tables maintained within slirp.
>> > One use-case for it is showing the guest's IPv6 address(es).
>> >
>> > Signed-off-by: Doug Evans <dje@google.com>
>> > ---
>> >  hmp-commands-info.hx               | 15 +++++++
>> >  include/net/slirp.h                |  1 +
>> >  net/slirp.c                        | 15 +++++++
>> >  tests/acceptance/info_neighbors.py | 69 ++++++++++++++++++++++++++++++
>> >  4 files changed, 100 insertions(+)
>> >  create mode 100644 tests/acceptance/info_neighbors.py
>>
>> Standard request for new HMP commands without corresponding QMP
>> commands: please state in the commit message why the QMP command is not
>> worthwhile.
>>
>> HMP commands without a QMP equivalent are okay if their functionality
>> makes no sense in QMP, or is of use only for human users.
>>
>> Example for "makes no sense in QMP": setting the current CPU, because a
>> QMP monitor doesn't have a current CPU.
>>
>> Examples for "is of use only for human users": HMP command "help", the
>> integrated pocket calculator.
>>
>> Debugging commands are kind of borderline.  Debugging is commonly a
>> human activity, where HMP is just fine.  However, humans create tools to
>> assist with their activities, and then QMP is useful.  While I wouldn't
>> encourage HMP-only for the debugging use case, I wouldn't veto it.
>>
>
>
> Mostly I was following what I saw for "info usernet".
> I don't see a difference between "info neighbors" and "info usernet" so I
> went with that.
> Both draw their data from libslirp.

I see.

> I'm happy to add QMP support if necessary.
> Note that there is code that parses "info usernet" output, e.g.,
> get_info_usernet_hostfwd_port for python.

Demonstrates "is of use only for human users" is wrong.

> Presumably we don't want to print text in slirp only to parse it in qemu,
> right?

Yes, we'd prefer not to parse.

As long as libslirp can only give us text, we need to parse it
somewhere.

We can parse it right in QEMU, or punt the job to whatever uses QEMU.
The latter can get away with parsing just the part they need.  But we
may end up with multiple parsers.


> That'll change the qemu/slirp interface.
> OTOH, to what extent does libslirp want to export a more formal API for
> this, vs just text?

This is a question for Samuel or Marc-André.

If the answer is "no", then HMP only (so we don't have to parse in QEMU)
is fair, I think.  The commit message should explain this.