Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping

qemu-devel

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

Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping

From:	Akihiko Odaki
Subject:	Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping
Date:	Tue, 31 Jan 2023 11:36:14 +0900
User-agent:	Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101 Thunderbird/102.7.0

On 2023/01/31 0:47, Michael S. Tsirkin wrote:

On Tue, Jan 31, 2023 at 12:36:38AM +0900, Akihiko Odaki wrote:

On 2023/01/31 0:12, Michael S. Tsirkin wrote:

On Mon, Jan 30, 2023 at 10:47:07PM +0900, Akihiko Odaki wrote:

filter-dump specifiees Ethernet as PCAP LinkType, which does not expect
virtio-net header. Having virtio-net header in such PCAP file breaks
PCAP unconsumable. Unfortunately currently there is no LinkType for
virtio-net so for now strip virtio-net header to convert the output to
Ethernet.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>


Probably means you need to calculate checksums and split gso too right?


It was not necessary in my case as I used Wireshark and it tolerates wrong
checksums and large packets (it even says "Checksum incorrect [maybe caused
by 'TCP checksum offload'?]"). It was even more helpful to have raw packets
instead of transformed packets for debugging purposes. Perhaps an option to
transform packets may be added later if a need arises.


I think we should add LINKTYPE_VIRTIO. Very useful to debug a host of
checksum/segmentation issues. Want to hack it up?

I'd rather like to land this patch as is. I think patching Wireshark sothat it repsects virtio-net flags is a non-trivial task. Even ifWireshark is patched, it takes time until the patched version becomesavailable, and other tooling may not work with the new linktype.

In my opinion, virtio-net header is not worth much when debugging as itonly contains some simple flags and is unlikely to be corrupted. Thelogic to determine whether the flags should be set is also simple (e.g.,if TCP segmentation offload is enabled and it is TCP/IPv4, setVIRTIO_NET_HDR_GSO_TCPV4).

---
   include/net/net.h |  6 ++++++
   net/dump.c        | 11 +++++++----
   net/net.c         | 18 ++++++++++++++++++
   net/tap.c         | 16 ++++++++++++++++
   4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index dc20b31e9f..4b2d72b3fc 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -56,8 +56,10 @@ typedef RxFilterInfo *(QueryRxFilter)(NetClientState *);
   typedef bool (HasUfo)(NetClientState *);
   typedef bool (HasVnetHdr)(NetClientState *);
   typedef bool (HasVnetHdrLen)(NetClientState *, int);
+typedef bool (GetUsingVnetHdr)(NetClientState *);
   typedef void (UsingVnetHdr)(NetClientState *, bool);
   typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
+typedef int (GetVnetHdrLen)(NetClientState *);
   typedef void (SetVnetHdrLen)(NetClientState *, int);
   typedef int (SetVnetLE)(NetClientState *, bool);
   typedef int (SetVnetBE)(NetClientState *, bool);
@@ -84,8 +86,10 @@ typedef struct NetClientInfo {
       HasUfo *has_ufo;
       HasVnetHdr *has_vnet_hdr;
       HasVnetHdrLen *has_vnet_hdr_len;
+    GetUsingVnetHdr *get_using_vnet_hdr;
       UsingVnetHdr *using_vnet_hdr;
       SetOffload *set_offload;
+    GetVnetHdrLen *get_vnet_hdr_len;
       SetVnetHdrLen *set_vnet_hdr_len;
       SetVnetLE *set_vnet_le;
       SetVnetBE *set_vnet_be;
@@ -183,9 +187,11 @@ void qemu_format_nic_info_str(NetClientState *nc, uint8_t 
macaddr[6]);
   bool qemu_has_ufo(NetClientState *nc);
   bool qemu_has_vnet_hdr(NetClientState *nc);
   bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
+bool qemu_get_using_vnet_hdr(NetClientState *nc);
   void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
                         int ecn, int ufo);
+int qemu_get_vnet_hdr_len(NetClientState *nc);
   void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
   int qemu_set_vnet_le(NetClientState *nc, bool is_le);
   int qemu_set_vnet_be(NetClientState *nc, bool is_be);
diff --git a/net/dump.c b/net/dump.c
index 6a63b15359..7d05f16ca7 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -61,12 +61,13 @@ struct pcap_sf_pkthdr {
       uint32_t len;
   };
-static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt)
+static ssize_t dump_receive_iov(DumpState *s, const struct iovec *iov, int cnt,
+                                int offset)
   {
       struct pcap_sf_pkthdr hdr;
       int64_t ts;
       int caplen;
-    size_t size = iov_size(iov, cnt);
+    size_t size = iov_size(iov, cnt) - offset;
       struct iovec dumpiov[cnt + 1];
       /* Early return in case of previous error. */
@@ -84,7 +85,7 @@ static ssize_t dump_receive_iov(DumpState *s, const struct 
iovec *iov, int cnt)
       dumpiov[0].iov_base = &hdr;
       dumpiov[0].iov_len = sizeof(hdr);
-    cnt = iov_copy(&dumpiov[1], cnt, iov, cnt, 0, caplen);
+    cnt = iov_copy(&dumpiov[1], cnt, iov, cnt, offset, caplen);
       if (writev(s->fd, dumpiov, cnt + 1) != sizeof(hdr) + caplen) {
           error_report("network dump write error - stopping dump");
@@ -153,8 +154,10 @@ static ssize_t filter_dump_receive_iov(NetFilterState *nf, 
NetClientState *sndr,
                                          int iovcnt, NetPacketSent *sent_cb)
   {
       NetFilterDumpState *nfds = FILTER_DUMP(nf);
+    int offset = qemu_get_using_vnet_hdr(nf->netdev) ?
+                 qemu_get_vnet_hdr_len(nf->netdev) : 0;
-    dump_receive_iov(&nfds->ds, iov, iovcnt);
+    dump_receive_iov(&nfds->ds, iov, iovcnt, offset);
       return 0;
   }
diff --git a/net/net.c b/net/net.c
index 2d01472998..03f17de5fc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -513,6 +513,15 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len)
       return nc->info->has_vnet_hdr_len(nc, len);
   }
+bool qemu_get_using_vnet_hdr(NetClientState *nc)
+{
+    if (!nc || !nc->info->get_using_vnet_hdr) {
+        return false;
+    }
+
+    return nc->info->get_using_vnet_hdr(nc);
+}
+
   void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
   {
       if (!nc || !nc->info->using_vnet_hdr) {
@@ -532,6 +541,15 @@ void qemu_set_offload(NetClientState *nc, int csum, int 
tso4, int tso6,
       nc->info->set_offload(nc, csum, tso4, tso6, ecn, ufo);
   }
+int qemu_get_vnet_hdr_len(NetClientState *nc)
+{
+    if (!nc || !nc->info->get_vnet_hdr_len) {
+        return 0;
+    }
+
+    return nc->info->get_vnet_hdr_len(nc);
+}
+
   void qemu_set_vnet_hdr_len(NetClientState *nc, int len)
   {
       if (!nc || !nc->info->set_vnet_hdr_len) {
diff --git a/net/tap.c b/net/tap.c
index 7d7bc1dc5f..1bf085d422 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -255,6 +255,13 @@ static bool tap_has_vnet_hdr_len(NetClientState *nc, int 
len)
       return !!tap_probe_vnet_hdr_len(s->fd, len);
   }
+static int tap_get_vnet_hdr_len(NetClientState *nc)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+    return s->host_vnet_hdr_len;
+}
+
   static void tap_set_vnet_hdr_len(NetClientState *nc, int len)
   {
       TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -268,6 +275,13 @@ static void tap_set_vnet_hdr_len(NetClientState *nc, int 
len)
       s->host_vnet_hdr_len = len;
   }
+static bool tap_get_using_vnet_hdr(NetClientState *nc)
+{
+    TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+    return s->using_vnet_hdr;
+}
+
   static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
   {
       TAPState *s = DO_UPCAST(TAPState, nc, nc);
@@ -372,8 +386,10 @@ static NetClientInfo net_tap_info = {
       .has_ufo = tap_has_ufo,
       .has_vnet_hdr = tap_has_vnet_hdr,
       .has_vnet_hdr_len = tap_has_vnet_hdr_len,
+    .get_using_vnet_hdr = tap_get_using_vnet_hdr,
       .using_vnet_hdr = tap_using_vnet_hdr,
       .set_offload = tap_set_offload,
+    .get_vnet_hdr_len = tap_get_vnet_hdr_len,
       .set_vnet_hdr_len = tap_set_vnet_hdr_len,
       .set_vnet_le = tap_set_vnet_le,
       .set_vnet_be = tap_set_vnet_be,
--
2.39.1

[Prev in Thread]

Current Thread

[Next in Thread]

[PATCH v4 13/28] e1000: Configure ResettableClass, (continued)
- [PATCH v4 13/28] e1000: Configure ResettableClass, Akihiko Odaki, 2023/01/30
- [PATCH v4 14/28] e1000e: Configure ResettableClass, Akihiko Odaki, 2023/01/30
- [PATCH v4 15/28] e1000e: Introduce e1000_rx_desc_union, Akihiko Odaki, 2023/01/30
- [PATCH v4 16/28] e1000e: Set MII_ANER_NWAY, Akihiko Odaki, 2023/01/30
- [PATCH v4 17/28] e1000e: Remove extra pointer indirection, Akihiko Odaki, 2023/01/30
- [PATCH v4 18/28] net: Check L4 header size, Akihiko Odaki, 2023/01/30
- [PATCH v4 20/28] net: Strip virtio-net header when dumping, Akihiko Odaki, 2023/01/30
  - Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping, Michael S. Tsirkin, 2023/01/30
    - Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping, Akihiko Odaki, 2023/01/30
    - Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping, Michael S. Tsirkin, 2023/01/30
    - Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping, Akihiko Odaki <=
- [PATCH v4 19/28] e1000x: Alter the signature of e1000x_is_vlan_packet, Akihiko Odaki, 2023/01/30
- [PATCH v4 22/28] hw/net/net_rx_pkt: Remove net_rx_pkt_has_virt_hdr, Akihiko Odaki, 2023/01/30
- [PATCH v4 25/28] hw/net/net_tx_pkt: Check the payload length, Akihiko Odaki, 2023/01/30
- [PATCH v4 28/28] MAINTAINERS: Add e1000e test files, Akihiko Odaki, 2023/01/30
- [PATCH v4 24/28] hw/net/net_tx_pkt: Implement TCP segmentation, Akihiko Odaki, 2023/01/30
- [PATCH v4 27/28] MAINTAINERS: Add Akihiko Odaki as a e1000e reviewer, Akihiko Odaki, 2023/01/30
- [PATCH v4 21/28] hw/net/net_tx_pkt: Automatically determine if virtio-net header is used, Akihiko Odaki, 2023/01/30
- [PATCH v4 23/28] e1000e: Perform software segmentation for loopback, Akihiko Odaki, 2023/01/30
- [PATCH v4 26/28] e1000e: Do not assert when MSI-X is disabled later, Akihiko Odaki, 2023/01/30

Prev by Date: Re: [PATCH v3 08/14] RISC-V: Adding T-Head MemPair extension
Next by Date: [PATCH v2] hw/timer/hpet: Fix expiration time overflow
Previous by thread: Re: [PATCH v4 20/28] net: Strip virtio-net header when dumping
Next by thread: [PATCH v4 19/28] e1000x: Alter the signature of e1000x_is_vlan_packet
Index(es):
- Date
- Thread