| Message ID | 20210902212015.1303865-1-dje@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: Add "info neighbors" command | expand |
On Thu, Sep 2, 2021 at 2:21 PM Doug Evans <dje@google.com> wrote: > 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> > Reviewed-by: Patrick Venture <venture@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 > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > index 27206ac049..386f09f163 100644 > --- a/hmp-commands-info.hx > +++ b/hmp-commands-info.hx > @@ -512,6 +512,21 @@ SRST > Show user network stack connection states. > ERST > > +#if defined(CONFIG_SLIRP) > + { > + .name = "neighbors", > + .args_type = "", > + .params = "", > + .help = "show the ARP and NDP tables", > + .cmd = hmp_info_neighbors, > + }, > +#endif > + > +SRST > + ``info neighbors`` > + Show the ARP and NDP tables. > +ERST > + > { > .name = "migrate", > .args_type = "", > diff --git a/include/net/slirp.h b/include/net/slirp.h > index bad3e1e241..b9ccfda1e7 100644 > --- a/include/net/slirp.h > +++ b/include/net/slirp.h > @@ -31,6 +31,7 @@ void hmp_hostfwd_add(Monitor *mon, const QDict *qdict); > void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict); > > void hmp_info_usernet(Monitor *mon, const QDict *qdict); > +void hmp_info_neighbors(Monitor *mon, const QDict *qdict); > > #endif > > diff --git a/net/slirp.c b/net/slirp.c > index ad3a838e0b..29a4cd3fe1 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -1028,6 +1028,21 @@ void hmp_info_usernet(Monitor *mon, const QDict > *qdict) > } > } > > +void hmp_info_neighbors(Monitor *mon, const QDict *qdict) > +{ > + SlirpState *s; > + > + QTAILQ_FOREACH(s, &slirp_stacks, entry) { > + int id; > + bool got_hub_id = net_hub_id_for_client(&s->nc, &id) == 0; > + char *info = slirp_neighbor_info(s->slirp); > + monitor_printf(mon, "Hub %d (%s):\n%s", > + got_hub_id ? id : -1, > + s->nc.name, info); > + g_free(info); > + } > +} > + > static void > net_init_slirp_configs(const StringList *fwd, int flags) > { > diff --git a/tests/acceptance/info_neighbors.py > b/tests/acceptance/info_neighbors.py > new file mode 100644 > index 0000000000..ff79ec3ff3 > --- /dev/null > +++ b/tests/acceptance/info_neighbors.py > @@ -0,0 +1,69 @@ > +# Test for the hmp command "info neighbors" > +# > +# Copyright 2021 Google LLC > +# > +# This work is licensed under the terms of the GNU GPL, version 2 or > +# later. See the COPYING file in the top-level directory. > + > +import re > + > +from avocado_qemu import LinuxTest > +from avocado_qemu import Test > + > +VNET_HUB_HEADER = 'Hub -1 (vnet):' > +NEIGHBOR_HEADER_REGEX = '^ *Table *MacAddr *IP Address$' > + > +def trim(text): > + return " ".join(text.split()) > + > +def hmc(test, cmd): > + return test.vm.command('human-monitor-command', command_line=cmd) > + > +def get_neighbors(test): > + output = hmc(test, 'info neighbors').splitlines() > + if len(output) < 2: > + test.fail("Insufficient output from 'info neighbors'") > + test.assertEquals(output[0], VNET_HUB_HEADER) > + test.assertTrue(re.fullmatch(NEIGHBOR_HEADER_REGEX, output[1])) > + return output[2:] > + > +class InfoNeighborsNone(Test): > + > + def test_no_neighbors(self): > + self.vm.add_args('-nodefaults', > + '-netdev', 'user,id=vnet', > + '-device', 'virtio-net,netdev=vnet') > + self.vm.launch() > + neighbors = get_neighbors(self) > + self.assertEquals(len(neighbors), 0) > + > +class InfoNeighbors(LinuxTest): > + > + def test_neighbors(self): > + """ > + :avocado: tags=arch:x86_64 > + :avocado: tags=machine:pc > + :avocado: tags=accel:kvm > + """ > + self.require_accelerator('kvm') > + self.vm.add_args("-accel", "kvm") > + self.vm.add_args('-nographic', > + '-m', '1024') > + self.launch_and_wait() > + > + # Ensure there's some packets to the guest and back. > + self.ssh_command('pwd') > + > + # We should now be aware of the guest as a neighbor. > + expected_ipv4_neighbor = 'ARP 52:54:00:12:34:56 10.0.2.15' > + # The default ipv6 net is fec0. Both fe80 and fec0 can appear. > + expected_ipv6_neighbors = [ > + 'NDP 52:54:00:12:34:56 fe80::5054:ff:fe12:3456', > + 'NDP 52:54:00:12:34:56 fec0::5054:ff:fe12:3456' > + ] > + neighbors = get_neighbors(self) > + self.assertTrue(len(neighbors) >= 2 and len(neighbors) <= 3) > + # IPv4 is output first. > + self.assertEquals(trim(neighbors[0]), expected_ipv4_neighbor) > + for neighbor in neighbors[1:]: > + self.assertTrue(trim(neighbor) in expected_ipv6_neighbors) > -- > 2.33.0.153.gba50c8fa24-goog > > >
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.
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'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. Presumably we don't want to print text in slirp only to parse it in qemu, right? 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?
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.
Hi On Sat, Sep 4, 2021 at 10:26 AM Markus Armbruster <armbru@redhat.com> wrote: > 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. > > Until now, libslirp (& QEMU HMP) provided inner state details as textual form. But since there is a need for a programming-friendly API, it could be added to libslirp and exported as QAPI/QMP by QEMU. Let's try to design the API in a way that can be easily extended (via getters and/or keys etc). Alternatively, libslirp could dump its state as JSON, but this wouldn't fit so nicely with QMP machinery/introspection etc.
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index 27206ac049..386f09f163 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -512,6 +512,21 @@ SRST Show user network stack connection states. ERST +#if defined(CONFIG_SLIRP) + { + .name = "neighbors", + .args_type = "", + .params = "", + .help = "show the ARP and NDP tables", + .cmd = hmp_info_neighbors, + }, +#endif + +SRST + ``info neighbors`` + Show the ARP and NDP tables. +ERST + { .name = "migrate", .args_type = "", diff --git a/include/net/slirp.h b/include/net/slirp.h index bad3e1e241..b9ccfda1e7 100644 --- a/include/net/slirp.h +++ b/include/net/slirp.h @@ -31,6 +31,7 @@ void hmp_hostfwd_add(Monitor *mon, const QDict *qdict); void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict); void hmp_info_usernet(Monitor *mon, const QDict *qdict); +void hmp_info_neighbors(Monitor *mon, const QDict *qdict); #endif diff --git a/net/slirp.c b/net/slirp.c index ad3a838e0b..29a4cd3fe1 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -1028,6 +1028,21 @@ void hmp_info_usernet(Monitor *mon, const QDict *qdict) } } +void hmp_info_neighbors(Monitor *mon, const QDict *qdict) +{ + SlirpState *s; + + QTAILQ_FOREACH(s, &slirp_stacks, entry) { + int id; + bool got_hub_id = net_hub_id_for_client(&s->nc, &id) == 0; + char *info = slirp_neighbor_info(s->slirp); + monitor_printf(mon, "Hub %d (%s):\n%s", + got_hub_id ? id : -1, + s->nc.name, info); + g_free(info); + } +} + static void net_init_slirp_configs(const StringList *fwd, int flags) { diff --git a/tests/acceptance/info_neighbors.py b/tests/acceptance/info_neighbors.py new file mode 100644 index 0000000000..ff79ec3ff3 --- /dev/null +++ b/tests/acceptance/info_neighbors.py @@ -0,0 +1,69 @@ +# Test for the hmp command "info neighbors" +# +# Copyright 2021 Google LLC +# +# This work is licensed under the terms of the GNU GPL, version 2 or +# later. See the COPYING file in the top-level directory. + +import re + +from avocado_qemu import LinuxTest +from avocado_qemu import Test + +VNET_HUB_HEADER = 'Hub -1 (vnet):' +NEIGHBOR_HEADER_REGEX = '^ *Table *MacAddr *IP Address$' + +def trim(text): + return " ".join(text.split()) + +def hmc(test, cmd): + return test.vm.command('human-monitor-command', command_line=cmd) + +def get_neighbors(test): + output = hmc(test, 'info neighbors').splitlines() + if len(output) < 2: + test.fail("Insufficient output from 'info neighbors'") + test.assertEquals(output[0], VNET_HUB_HEADER) + test.assertTrue(re.fullmatch(NEIGHBOR_HEADER_REGEX, output[1])) + return output[2:] + +class InfoNeighborsNone(Test): + + def test_no_neighbors(self): + self.vm.add_args('-nodefaults', + '-netdev', 'user,id=vnet', + '-device', 'virtio-net,netdev=vnet') + self.vm.launch() + neighbors = get_neighbors(self) + self.assertEquals(len(neighbors), 0) + +class InfoNeighbors(LinuxTest): + + def test_neighbors(self): + """ + :avocado: tags=arch:x86_64 + :avocado: tags=machine:pc + :avocado: tags=accel:kvm + """ + self.require_accelerator('kvm') + self.vm.add_args("-accel", "kvm") + self.vm.add_args('-nographic', + '-m', '1024') + self.launch_and_wait() + + # Ensure there's some packets to the guest and back. + self.ssh_command('pwd') + + # We should now be aware of the guest as a neighbor. + expected_ipv4_neighbor = 'ARP 52:54:00:12:34:56 10.0.2.15' + # The default ipv6 net is fec0. Both fe80 and fec0 can appear. + expected_ipv6_neighbors = [ + 'NDP 52:54:00:12:34:56 fe80::5054:ff:fe12:3456', + 'NDP 52:54:00:12:34:56 fec0::5054:ff:fe12:3456' + ] + neighbors = get_neighbors(self) + self.assertTrue(len(neighbors) >= 2 and len(neighbors) <= 3) + # IPv4 is output first. + self.assertEquals(trim(neighbors[0]), expected_ipv4_neighbor) + for neighbor in neighbors[1:]: + self.assertTrue(trim(neighbor) in expected_ipv6_neighbors)
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