qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/6] tests: Add network filter tests to the chec


From: Zhang Chen
Subject: Re: [Qemu-devel] [PATCH 2/6] tests: Add network filter tests to the check-qtest-s390x list
Date: Fri, 18 Aug 2017 09:49:47 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1



On 08/17/2017 10:02 PM, Thomas Huth wrote:
On 17.08.2017 10:41, Cornelia Huck wrote:
On Thu, 17 Aug 2017 08:25:09 +0200
Thomas Huth <address@hidden> wrote:

With some small modifications, we can also use the the netfilter,
the fiter-mirror and the filter-redirector tests on s390x.
s/fiter/filter/
OK ... could you please fix that when picking up the patch (in case I do
not have to resend)?

Signed-off-by: Thomas Huth <address@hidden>
---
  tests/Makefile.include         |  3 +++
  tests/test-filter-mirror.c     |  9 +++++++--
  tests/test-filter-redirector.c | 22 ++++++++++++++++------
  tests/test-netfilter.c         | 11 ++++++++++-
  4 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
index a1d5865..d569d27 100644
--- a/tests/test-filter-mirror.c
+++ b/tests/test-filter-mirror.c
@@ -25,6 +25,11 @@ static void test_mirror(void)
      char *recv_buf;
      uint32_t size = sizeof(send_buf);
      size = htonl(size);
+    const char *devstr = "e1000";
+
+    if (g_str_equal(qtest_get_arch(), "s390x")) {
+        devstr = "virtio-net-ccw";
+    }
I'm wondering if we could unify selection of the network device
somehow. There's probably two cases:
- Test a specific device. This obviously needs to be decided
   individually.
- Just use a functional network device. For s390x, this will be
   virtio-net-ccw; for other architectures, this test uses e1000, while
   one of the tests below uses rtl8139 (why?). A helper for that may be
   useful.
Maybe ... OTOH, this likely increases also test coverage if we do not
use the same PCI NIC in all the tests...?

ret = socketpair(PF_UNIX, SOCK_STREAM, 0, send_sock);
      g_assert_cmpint(ret, !=, -1);
diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
index 69c663b..3afd411 100644
--- a/tests/test-filter-redirector.c
+++ b/tests/test-filter-redirector.c
@@ -57,6 +57,16 @@
  #include "qemu/error-report.h"
  #include "qemu/main-loop.h"
+static const char *get_devstr(void)
+{
+    if (g_str_equal(qtest_get_arch(), "s390x")) {
+        return "virtio-net-ccw";
+    }
+
+    return "rtl8139";
No problem with your patch, but I'm wondering why this does not use
e1000. Special capabilities of rtl8139?
Maybe Zhang Chen can answer that question? (Now on CC: - forgot to do
that initially, sorry!)
But I guess it's just an arbitrary NIC that works on most of the
platforms (i.e. a normal PCI NIC)...?

Yes, you are right, we can use other NIC here(like e1000).

Reviewed-by: Zhang Chen<address@hidden>


Thanks
Zhang Chen


+}
+
+
  static void test_redirector_tx(void)
  {
      int backend_sock[2], recv_sock;
diff --git a/tests/test-netfilter.c b/tests/test-netfilter.c
index 8b5a9b2..2506473 100644
--- a/tests/test-netfilter.c
+++ b/tests/test-netfilter.c
@@ -182,6 +182,12 @@ static void remove_netdev_with_multi_netfilter(void)
  int main(int argc, char **argv)
  {
      int ret;
+    char *args;
+    const char *devstr = "e1000";
It's our old friend again :)

+
+    if (g_str_equal(qtest_get_arch(), "s390x")) {
+        devstr = "virtio-net-ccw";
+    }
g_test_init(&argc, &argv, NULL);
      qtest_add_func("/netfilter/addremove_one", add_one_netfilter);
@@ -191,10 +197,13 @@ int main(int argc, char **argv)
      qtest_add_func("/netfilter/remove_netdev_multi",
                     remove_netdev_with_multi_netfilter);
- qtest_start("-netdev user,id=qtest-bn0 -device e1000,netdev=qtest-bn0");
+    args = g_strdup_printf("-netdev user,id=qtest-bn0 "
+                           "-device %s,netdev=qtest-bn0", devstr);
+    qtest_start(args);
      ret = g_test_run();
qtest_end();
+    g_free(args);
return ret;
  }
Even though I think we should deal with the questions above, having
more tests for s390x is certainly a good idea. Thus,

Reviewed-by: Cornelia Huck <address@hidden>
Thanks for the review,
  Thomas


.


--
Thanks
Zhang Chen






reply via email to

[Prev in Thread] Current Thread [Next in Thread]