| Message ID | 20230113130833.81458-1-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | net: Strip virtio-net header when dumping | expand |
On 13/1/23 14:08, 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> > --- > include/net/net.h | 6 ++++++ > net/dump.c | 11 +++++++---- > net/net.c | 18 ++++++++++++++++++ > net/netmap.c | 1 + > net/tap.c | 16 ++++++++++++++++ > 5 files changed, 48 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); > @@ -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..3a95c3ba6a 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); Per [*], is get_using_vnet_hdr() really useful? Can't we just check for using_vnet_hdr != NULL as: return nc->info->using_vnet_hdr != NULL; > +} > + > 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 -ENOSYS; Does -ENOSYS provides any value? Otherwise we could return 0. > + } > + > + return nc->info->get_vnet_hdr_len(nc); > +} > diff --git a/net/netmap.c b/net/netmap.c > index 9e0cec58d3..ed9c7ec948 100644 > --- a/net/netmap.c > +++ b/net/netmap.c > @@ -393,6 +393,7 @@ static NetClientInfo net_netmap_info = { > .has_ufo = netmap_has_vnet_hdr, > .has_vnet_hdr = netmap_has_vnet_hdr, > .has_vnet_hdr_len = netmap_has_vnet_hdr_len, > + .get_using_vnet_hdr = netmap_get_using_vnet_hdr, On what is this patch based? I don't have netmap_get_using_vnet_hdr(). > .using_vnet_hdr = netmap_using_vnet_hdr, > .set_offload = netmap_set_offload, > .set_vnet_hdr_len = netmap_set_vnet_hdr_len,
On 2023/01/13 22:40, Philippe Mathieu-Daudé wrote: > On 13/1/23 14:08, 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> >> --- >> include/net/net.h | 6 ++++++ >> net/dump.c | 11 +++++++---- >> net/net.c | 18 ++++++++++++++++++ >> net/netmap.c | 1 + >> net/tap.c | 16 ++++++++++++++++ >> 5 files changed, 48 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); > >> @@ -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..3a95c3ba6a 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); > > Per [*], is get_using_vnet_hdr() really useful? Can't we just > check for using_vnet_hdr != NULL as: tap allows to disable vnet_hdr by passing false to using_vnet_hdr() so we need to ask it whether vnet_hdr is currently enabled or not. > > return nc->info->using_vnet_hdr != NULL; > >> +} >> + >> 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 -ENOSYS; > > Does -ENOSYS provides any value? Otherwise we could return 0. I have just sent v2, which returns 0 here instead. tap returns 0 in case vnet_hdr is disabled so this should also return 0. > >> + } >> + >> + return nc->info->get_vnet_hdr_len(nc); >> +} > >> diff --git a/net/netmap.c b/net/netmap.c >> index 9e0cec58d3..ed9c7ec948 100644 >> --- a/net/netmap.c >> +++ b/net/netmap.c >> @@ -393,6 +393,7 @@ static NetClientInfo net_netmap_info = { >> .has_ufo = netmap_has_vnet_hdr, >> .has_vnet_hdr = netmap_has_vnet_hdr, >> .has_vnet_hdr_len = netmap_has_vnet_hdr_len, >> + .get_using_vnet_hdr = netmap_get_using_vnet_hdr, > > On what is this patch based? I don't have netmap_get_using_vnet_hdr(). This is a mistake; I removed it in v2. Regards, Akihiko Odaki > >> .using_vnet_hdr = netmap_using_vnet_hdr, >> .set_offload = netmap_set_offload, >> .set_vnet_hdr_len = netmap_set_vnet_hdr_len, >
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..3a95c3ba6a 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 -ENOSYS; + } + + 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/netmap.c b/net/netmap.c index 9e0cec58d3..ed9c7ec948 100644 --- a/net/netmap.c +++ b/net/netmap.c @@ -393,6 +393,7 @@ static NetClientInfo net_netmap_info = { .has_ufo = netmap_has_vnet_hdr, .has_vnet_hdr = netmap_has_vnet_hdr, .has_vnet_hdr_len = netmap_has_vnet_hdr_len, + .get_using_vnet_hdr = netmap_get_using_vnet_hdr, .using_vnet_hdr = netmap_using_vnet_hdr, .set_offload = netmap_set_offload, .set_vnet_hdr_len = netmap_set_vnet_hdr_len, diff --git a/net/tap.c b/net/tap.c index e28ceb078f..daa7e0a042 100644 --- a/net/tap.c +++ b/net/tap.c @@ -257,6 +257,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); @@ -270,6 +277,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); @@ -374,8 +388,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/netmap.c | 1 + net/tap.c | 16 ++++++++++++++++ 5 files changed, 48 insertions(+), 4 deletions(-)