| Message ID | 20230130134715.76604-21-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | e1000x cleanups (preliminary for IGB) | expand |
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? > --- > 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
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. > >> --- >> 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 >
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? > > > > > --- > > > 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 > >
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 so that it repsects virtio-net flags is a non-trivial task. Even if Wireshark is patched, it takes time until the patched version becomes available, and other tooling may not work with the new linktype. In my opinion, virtio-net header is not worth much when debugging as it only contains some simple flags and is unlikely to be corrupted. The logic to determine whether the flags should be set is also simple (e.g., if TCP segmentation offload is enabled and it is TCP/IPv4, set VIRTIO_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 >>> >
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,
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> --- include/net/net.h | 6 ++++++ net/dump.c | 11 +++++++---- net/net.c | 18 ++++++++++++++++++ net/tap.c | 16 ++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-)