| Message ID | 20230201033539.30049-22-akihiko.odaki@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | e1000x cleanups (preliminary for IGB) | expand |
在 2023/2/1 11:35, Akihiko Odaki 写道: > The new function qemu_get_using_vnet_hdr() allows to automatically > determine if virtio-net header is used. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > hw/net/e1000e_core.c | 3 +-- > hw/net/net_tx_pkt.c | 19 ++++++++++--------- > hw/net/net_tx_pkt.h | 3 +-- > hw/net/vmxnet3.c | 6 ++---- > 4 files changed, 14 insertions(+), 17 deletions(-) > > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c > index 38d374fba3..954a007151 100644 > --- a/hw/net/e1000e_core.c > +++ b/hw/net/e1000e_core.c > @@ -3376,8 +3376,7 @@ e1000e_core_pci_realize(E1000ECore *core, > qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); > > for (i = 0; i < E1000E_NUM_QUEUES; i++) { > - net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, > - E1000E_MAX_TX_FRAGS, core->has_vnet); > + net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, E1000E_MAX_TX_FRAGS); > } > > net_rx_pkt_init(&core->rx_pkt, core->has_vnet); > diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c > index 8a23899a4d..cf46c8457f 100644 > --- a/hw/net/net_tx_pkt.c > +++ b/hw/net/net_tx_pkt.c > @@ -35,7 +35,6 @@ struct NetTxPkt { > PCIDevice *pci_dev; > > struct virtio_net_hdr virt_hdr; > - bool has_virt_hdr; So this requires implicit coupling of NetTxPkt and a NetClientState (not self contained). This may work now but probably not the future e.g when two packets were queued in a list when one packet has a vnet header but another doesn't? Thanks > > struct iovec *raw; > uint32_t raw_frags; > @@ -59,7 +58,7 @@ struct NetTxPkt { > }; > > void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, > - uint32_t max_frags, bool has_virt_hdr) > + uint32_t max_frags) > { > struct NetTxPkt *p = g_malloc0(sizeof *p); > > @@ -71,10 +70,8 @@ void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, > > p->max_payload_frags = max_frags; > p->max_raw_frags = max_frags; > - p->has_virt_hdr = has_virt_hdr; > p->vec[NET_TX_PKT_VHDR_FRAG].iov_base = &p->virt_hdr; > - p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = > - p->has_virt_hdr ? sizeof p->virt_hdr : 0; > + p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = sizeof p->virt_hdr; > p->vec[NET_TX_PKT_L2HDR_FRAG].iov_base = &p->l2_hdr; > p->vec[NET_TX_PKT_L3HDR_FRAG].iov_base = &p->l3_hdr; > > @@ -617,9 +614,11 @@ static bool net_tx_pkt_do_sw_fragmentation(struct NetTxPkt *pkt, > > bool net_tx_pkt_send(struct NetTxPkt *pkt, NetClientState *nc) > { > + bool using_vnet_hdr = qemu_get_using_vnet_hdr(nc->peer); > + > assert(pkt); > > - if (!pkt->has_virt_hdr && > + if (!using_vnet_hdr && > pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { > net_tx_pkt_do_sw_csum(pkt); > } > @@ -636,11 +635,13 @@ bool net_tx_pkt_send(struct NetTxPkt *pkt, NetClientState *nc) > } > } > > - if (pkt->has_virt_hdr || > + if (using_vnet_hdr || > pkt->virt_hdr.gso_type == VIRTIO_NET_HDR_GSO_NONE) { > + int index = using_vnet_hdr ? > + NET_TX_PKT_VHDR_FRAG : NET_TX_PKT_L2HDR_FRAG; > net_tx_pkt_fix_ip6_payload_len(pkt); > - net_tx_pkt_sendv(pkt, nc, pkt->vec, > - pkt->payload_frags + NET_TX_PKT_PL_START_FRAG); > + net_tx_pkt_sendv(pkt, nc, pkt->vec + index, > + pkt->payload_frags + NET_TX_PKT_PL_START_FRAG - index); > return true; > } > > diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h > index 2e38a5fa69..8d3faa42fb 100644 > --- a/hw/net/net_tx_pkt.h > +++ b/hw/net/net_tx_pkt.h > @@ -32,10 +32,9 @@ struct NetTxPkt; > * @pkt: packet pointer > * @pci_dev: PCI device processing this packet > * @max_frags: max tx ip fragments > - * @has_virt_hdr: device uses virtio header. > */ > void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, > - uint32_t max_frags, bool has_virt_hdr); > + uint32_t max_frags); > > /** > * Clean all tx packet resources. > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c > index c63bbb59bd..8c3f5d6e14 100644 > --- a/hw/net/vmxnet3.c > +++ b/hw/net/vmxnet3.c > @@ -1521,8 +1521,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) > > /* Preallocate TX packet wrapper */ > VMW_CFPRN("Max TX fragments is %u", s->max_tx_frags); > - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), > - s->max_tx_frags, s->peer_has_vhdr); > + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); > net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); > > /* Read rings memory locations for RX queues */ > @@ -2402,8 +2401,7 @@ static int vmxnet3_post_load(void *opaque, int version_id) > { > VMXNET3State *s = opaque; > > - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), > - s->max_tx_frags, s->peer_has_vhdr); > + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); > net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); > > if (s->msix_used) {
On 2023/02/21 12:38, Jason Wang wrote: > > 在 2023/2/1 11:35, Akihiko Odaki 写道: >> The new function qemu_get_using_vnet_hdr() allows to automatically >> determine if virtio-net header is used. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> hw/net/e1000e_core.c | 3 +-- >> hw/net/net_tx_pkt.c | 19 ++++++++++--------- >> hw/net/net_tx_pkt.h | 3 +-- >> hw/net/vmxnet3.c | 6 ++---- >> 4 files changed, 14 insertions(+), 17 deletions(-) >> >> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c >> index 38d374fba3..954a007151 100644 >> --- a/hw/net/e1000e_core.c >> +++ b/hw/net/e1000e_core.c >> @@ -3376,8 +3376,7 @@ e1000e_core_pci_realize(E1000ECore *core, >> qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); >> for (i = 0; i < E1000E_NUM_QUEUES; i++) { >> - net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, >> - E1000E_MAX_TX_FRAGS, core->has_vnet); >> + net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, >> E1000E_MAX_TX_FRAGS); >> } >> net_rx_pkt_init(&core->rx_pkt, core->has_vnet); >> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c >> index 8a23899a4d..cf46c8457f 100644 >> --- a/hw/net/net_tx_pkt.c >> +++ b/hw/net/net_tx_pkt.c >> @@ -35,7 +35,6 @@ struct NetTxPkt { >> PCIDevice *pci_dev; >> struct virtio_net_hdr virt_hdr; >> - bool has_virt_hdr; > > > So this requires implicit coupling of NetTxPkt and a NetClientState (not > self contained). This may work now but probably not the future e.g when > two packets were queued in a list when one packet has a vnet header but > another doesn't? > > Thanks This patch is actually intended to remove coupling of NetTxPkt and NetClientState. e1000e and igb have loopback mode, and in this mode, NetTxPkt needs to perform segmentation by itself even if the peer accepts vnet header. However, before this patch, has_virt_hdr flag is fixed in net_tx_pkt_init() so it couldn't handle a case where one packet needs vnet header and another doesn't. This patch fixes such a case by deferring the decision whether to have vnet header (and to offload segmentation) to the point when actually sending the packet. This allows NetTxPkt to add a vnet header and not to do so, depending on the situation. Patch "e1000e: Perform software segmentation for loopback" further decouples NetTxPkt and NetClientState by introducing a new function, net_tx_pkt_send_custom(). Unlike net_tx_pkt_send(), net_tx_pkt_send_custom() do not need NetClientState, and it is totally up to the caller whether to have vnet header or to offload segmentation. Regards, Akihiko Odaki > > >> struct iovec *raw; >> uint32_t raw_frags; >> @@ -59,7 +58,7 @@ struct NetTxPkt { >> }; >> void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, >> - uint32_t max_frags, bool has_virt_hdr) >> + uint32_t max_frags) >> { >> struct NetTxPkt *p = g_malloc0(sizeof *p); >> @@ -71,10 +70,8 @@ void net_tx_pkt_init(struct NetTxPkt **pkt, >> PCIDevice *pci_dev, >> p->max_payload_frags = max_frags; >> p->max_raw_frags = max_frags; >> - p->has_virt_hdr = has_virt_hdr; >> p->vec[NET_TX_PKT_VHDR_FRAG].iov_base = &p->virt_hdr; >> - p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = >> - p->has_virt_hdr ? sizeof p->virt_hdr : 0; >> + p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = sizeof p->virt_hdr; >> p->vec[NET_TX_PKT_L2HDR_FRAG].iov_base = &p->l2_hdr; >> p->vec[NET_TX_PKT_L3HDR_FRAG].iov_base = &p->l3_hdr; >> @@ -617,9 +614,11 @@ static bool net_tx_pkt_do_sw_fragmentation(struct >> NetTxPkt *pkt, >> bool net_tx_pkt_send(struct NetTxPkt *pkt, NetClientState *nc) >> { >> + bool using_vnet_hdr = qemu_get_using_vnet_hdr(nc->peer); >> + >> assert(pkt); >> - if (!pkt->has_virt_hdr && >> + if (!using_vnet_hdr && >> pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { >> net_tx_pkt_do_sw_csum(pkt); >> } >> @@ -636,11 +635,13 @@ bool net_tx_pkt_send(struct NetTxPkt *pkt, >> NetClientState *nc) >> } >> } >> - if (pkt->has_virt_hdr || >> + if (using_vnet_hdr || >> pkt->virt_hdr.gso_type == VIRTIO_NET_HDR_GSO_NONE) { >> + int index = using_vnet_hdr ? >> + NET_TX_PKT_VHDR_FRAG : NET_TX_PKT_L2HDR_FRAG; >> net_tx_pkt_fix_ip6_payload_len(pkt); >> - net_tx_pkt_sendv(pkt, nc, pkt->vec, >> - pkt->payload_frags + NET_TX_PKT_PL_START_FRAG); >> + net_tx_pkt_sendv(pkt, nc, pkt->vec + index, >> + pkt->payload_frags + NET_TX_PKT_PL_START_FRAG - index); >> return true; >> } >> diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h >> index 2e38a5fa69..8d3faa42fb 100644 >> --- a/hw/net/net_tx_pkt.h >> +++ b/hw/net/net_tx_pkt.h >> @@ -32,10 +32,9 @@ struct NetTxPkt; >> * @pkt: packet pointer >> * @pci_dev: PCI device processing this packet >> * @max_frags: max tx ip fragments >> - * @has_virt_hdr: device uses virtio header. >> */ >> void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, >> - uint32_t max_frags, bool has_virt_hdr); >> + uint32_t max_frags); >> /** >> * Clean all tx packet resources. >> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c >> index c63bbb59bd..8c3f5d6e14 100644 >> --- a/hw/net/vmxnet3.c >> +++ b/hw/net/vmxnet3.c >> @@ -1521,8 +1521,7 @@ static void vmxnet3_activate_device(VMXNET3State >> *s) >> /* Preallocate TX packet wrapper */ >> VMW_CFPRN("Max TX fragments is %u", s->max_tx_frags); >> - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), >> - s->max_tx_frags, s->peer_has_vhdr); >> + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); >> net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); >> /* Read rings memory locations for RX queues */ >> @@ -2402,8 +2401,7 @@ static int vmxnet3_post_load(void *opaque, int >> version_id) >> { >> VMXNET3State *s = opaque; >> - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), >> - s->max_tx_frags, s->peer_has_vhdr); >> + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); >> net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); >> if (s->msix_used) { >
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 38d374fba3..954a007151 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -3376,8 +3376,7 @@ e1000e_core_pci_realize(E1000ECore *core, qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); for (i = 0; i < E1000E_NUM_QUEUES; i++) { - net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, - E1000E_MAX_TX_FRAGS, core->has_vnet); + net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, E1000E_MAX_TX_FRAGS); } net_rx_pkt_init(&core->rx_pkt, core->has_vnet); diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c index 8a23899a4d..cf46c8457f 100644 --- a/hw/net/net_tx_pkt.c +++ b/hw/net/net_tx_pkt.c @@ -35,7 +35,6 @@ struct NetTxPkt { PCIDevice *pci_dev; struct virtio_net_hdr virt_hdr; - bool has_virt_hdr; struct iovec *raw; uint32_t raw_frags; @@ -59,7 +58,7 @@ struct NetTxPkt { }; void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, - uint32_t max_frags, bool has_virt_hdr) + uint32_t max_frags) { struct NetTxPkt *p = g_malloc0(sizeof *p); @@ -71,10 +70,8 @@ void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, p->max_payload_frags = max_frags; p->max_raw_frags = max_frags; - p->has_virt_hdr = has_virt_hdr; p->vec[NET_TX_PKT_VHDR_FRAG].iov_base = &p->virt_hdr; - p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = - p->has_virt_hdr ? sizeof p->virt_hdr : 0; + p->vec[NET_TX_PKT_VHDR_FRAG].iov_len = sizeof p->virt_hdr; p->vec[NET_TX_PKT_L2HDR_FRAG].iov_base = &p->l2_hdr; p->vec[NET_TX_PKT_L3HDR_FRAG].iov_base = &p->l3_hdr; @@ -617,9 +614,11 @@ static bool net_tx_pkt_do_sw_fragmentation(struct NetTxPkt *pkt, bool net_tx_pkt_send(struct NetTxPkt *pkt, NetClientState *nc) { + bool using_vnet_hdr = qemu_get_using_vnet_hdr(nc->peer); + assert(pkt); - if (!pkt->has_virt_hdr && + if (!using_vnet_hdr && pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { net_tx_pkt_do_sw_csum(pkt); } @@ -636,11 +635,13 @@ bool net_tx_pkt_send(struct NetTxPkt *pkt, NetClientState *nc) } } - if (pkt->has_virt_hdr || + if (using_vnet_hdr || pkt->virt_hdr.gso_type == VIRTIO_NET_HDR_GSO_NONE) { + int index = using_vnet_hdr ? + NET_TX_PKT_VHDR_FRAG : NET_TX_PKT_L2HDR_FRAG; net_tx_pkt_fix_ip6_payload_len(pkt); - net_tx_pkt_sendv(pkt, nc, pkt->vec, - pkt->payload_frags + NET_TX_PKT_PL_START_FRAG); + net_tx_pkt_sendv(pkt, nc, pkt->vec + index, + pkt->payload_frags + NET_TX_PKT_PL_START_FRAG - index); return true; } diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h index 2e38a5fa69..8d3faa42fb 100644 --- a/hw/net/net_tx_pkt.h +++ b/hw/net/net_tx_pkt.h @@ -32,10 +32,9 @@ struct NetTxPkt; * @pkt: packet pointer * @pci_dev: PCI device processing this packet * @max_frags: max tx ip fragments - * @has_virt_hdr: device uses virtio header. */ void net_tx_pkt_init(struct NetTxPkt **pkt, PCIDevice *pci_dev, - uint32_t max_frags, bool has_virt_hdr); + uint32_t max_frags); /** * Clean all tx packet resources. diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index c63bbb59bd..8c3f5d6e14 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -1521,8 +1521,7 @@ static void vmxnet3_activate_device(VMXNET3State *s) /* Preallocate TX packet wrapper */ VMW_CFPRN("Max TX fragments is %u", s->max_tx_frags); - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), - s->max_tx_frags, s->peer_has_vhdr); + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); /* Read rings memory locations for RX queues */ @@ -2402,8 +2401,7 @@ static int vmxnet3_post_load(void *opaque, int version_id) { VMXNET3State *s = opaque; - net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), - s->max_tx_frags, s->peer_has_vhdr); + net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s), s->max_tx_frags); net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr); if (s->msix_used) {
The new function qemu_get_using_vnet_hdr() allows to automatically determine if virtio-net header is used. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- hw/net/e1000e_core.c | 3 +-- hw/net/net_tx_pkt.c | 19 ++++++++++--------- hw/net/net_tx_pkt.h | 3 +-- hw/net/vmxnet3.c | 6 ++---- 4 files changed, 14 insertions(+), 17 deletions(-)