| Message ID | 20231010025924.14593-2-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-net RSS/hash report fixes | expand |
10.10.2023 05:59, Akihiko Odaki wrote: > The largest possible virtio-net header is struct virtio_net_hdr_v1_hash. > > Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > net/tap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index c6639d9f20..ea46feeaa8 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -118,7 +118,7 @@ static ssize_t tap_receive_iov(NetClientState *nc, const struct iovec *iov, > TAPState *s = DO_UPCAST(TAPState, nc, nc); > const struct iovec *iovp = iov; > struct iovec iov_copy[iovcnt + 1]; > - struct virtio_net_hdr_mrg_rxbuf hdr = { }; > + struct virtio_net_hdr_v1_hash hdr = { }; BTW, can we get rid of (implicit) memzero() here and in similar places, initializing only the actually used fields? Not that his particular structure is very large (and this change makes it 8 bytes larger), but still.. Thanks, /mjt
On 2023/10/10 16:56, Michael Tokarev wrote: > 10.10.2023 05:59, Akihiko Odaki wrote: >> The largest possible virtio-net header is struct virtio_net_hdr_v1_hash. >> >> Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> net/tap.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/net/tap.c b/net/tap.c >> index c6639d9f20..ea46feeaa8 100644 >> --- a/net/tap.c >> +++ b/net/tap.c >> @@ -118,7 +118,7 @@ static ssize_t tap_receive_iov(NetClientState *nc, >> const struct iovec *iov, >> TAPState *s = DO_UPCAST(TAPState, nc, nc); >> const struct iovec *iovp = iov; >> struct iovec iov_copy[iovcnt + 1]; >> - struct virtio_net_hdr_mrg_rxbuf hdr = { }; >> + struct virtio_net_hdr_v1_hash hdr = { }; > > BTW, can we get rid of (implicit) memzero() here and in > similar places, initializing only the actually used fields? > Not that his particular structure is very large (and this > change makes it 8 bytes larger), but still.. These variables are rarely used so I think it's fine. In particular, these variables are used only when QEMU sends an ARP packet while virtio-net is enabled. Regards, Akihiko Odaki
On 2023/10/11 15:43, Akihiko Odaki wrote: > On 2023/10/10 16:56, Michael Tokarev wrote: >> 10.10.2023 05:59, Akihiko Odaki wrote: >>> The largest possible virtio-net header is struct virtio_net_hdr_v1_hash. >>> >>> Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") >>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >>> --- >>> net/tap.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/net/tap.c b/net/tap.c >>> index c6639d9f20..ea46feeaa8 100644 >>> --- a/net/tap.c >>> +++ b/net/tap.c >>> @@ -118,7 +118,7 @@ static ssize_t tap_receive_iov(NetClientState >>> *nc, const struct iovec *iov, >>> TAPState *s = DO_UPCAST(TAPState, nc, nc); >>> const struct iovec *iovp = iov; >>> struct iovec iov_copy[iovcnt + 1]; >>> - struct virtio_net_hdr_mrg_rxbuf hdr = { }; >>> + struct virtio_net_hdr_v1_hash hdr = { }; >> >> BTW, can we get rid of (implicit) memzero() here and in >> similar places, initializing only the actually used fields? >> Not that his particular structure is very large (and this >> change makes it 8 bytes larger), but still.. > > These variables are rarely used so I think it's fine. In particular, > these variables are used only when QEMU sends an ARP packet while > virtio-net is enabled. My previous statement was not correct. These variables are used also when QEMU emulates a network interface that doesn't expose virtio-net headers. I split the code path for these two cases in v3. The header size is shrunken now for the normal code path. Regards, Akihiko Odaki
diff --git a/net/tap.c b/net/tap.c index c6639d9f20..ea46feeaa8 100644 --- a/net/tap.c +++ b/net/tap.c @@ -118,7 +118,7 @@ static ssize_t tap_receive_iov(NetClientState *nc, const struct iovec *iov, TAPState *s = DO_UPCAST(TAPState, nc, nc); const struct iovec *iovp = iov; struct iovec iov_copy[iovcnt + 1]; - struct virtio_net_hdr_mrg_rxbuf hdr = { }; + struct virtio_net_hdr_v1_hash hdr = { }; if (s->host_vnet_hdr_len && !s->using_vnet_hdr) { iov_copy[0].iov_base = &hdr; @@ -136,7 +136,7 @@ static ssize_t tap_receive_raw(NetClientState *nc, const uint8_t *buf, size_t si TAPState *s = DO_UPCAST(TAPState, nc, nc); struct iovec iov[2]; int iovcnt = 0; - struct virtio_net_hdr_mrg_rxbuf hdr = { }; + struct virtio_net_hdr_v1_hash hdr = { }; if (s->host_vnet_hdr_len) { iov[iovcnt].iov_base = &hdr;
The largest possible virtio-net header is struct virtio_net_hdr_v1_hash. Fixes: fbbdbddec0 ("tap: allow extended virtio header with hash info") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- net/tap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)