[PATCH v2] e1000e: set RX desc status with DD flag in a separate operation

Ding Hui posted 1 patch 3 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220914121647.11585-1-dinghui@sangfor.com.cn
Maintainers: Dmitry Fleytman <dmitry.fleytman@gmail.com>, Jason Wang <jasowang@redhat.com>
hw/net/e1000e_core.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
[PATCH v2] e1000e: set RX desc status with DD flag in a separate operation
Posted by Ding Hui 3 years, 4 months ago
Like commit 034d00d48581 ("e1000: set RX descriptor status in
a separate operation"), there is also same issue in e1000e, which
would cause lost packets or stop sending packets to VM with DPDK.

Do similar fix in e1000e.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/402
Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>
---
 hw/net/e1000e_core.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

---
v2: use uint8_t/uint32_t directly instead of typeof

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 208e3e0d79..a570b366b2 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1364,6 +1364,57 @@ struct NetRxPkt *pkt, const E1000E_RSSInfo *rss_info,
     }
 }
 
+static inline void
+e1000e_pci_dma_write_rx_desc(E1000ECore *core, dma_addr_t addr,
+                             uint8_t *desc, dma_addr_t len)
+{
+    PCIDevice *dev = core->owner;
+
+    if (e1000e_rx_use_legacy_descriptor(core)) {
+        struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
+        size_t offset = offsetof(struct e1000_rx_desc, status);
+        uint8_t status = d->status;
+
+        d->status &= ~E1000_RXD_STAT_DD;
+        pci_dma_write(dev, addr, desc, len);
+
+        if (status & E1000_RXD_STAT_DD) {
+            d->status = status;
+            pci_dma_write(dev, addr + offset, &status, sizeof(status));
+        }
+    } else {
+        if (core->mac[RCTL] & E1000_RCTL_DTYP_PS) {
+            union e1000_rx_desc_packet_split *d =
+                (union e1000_rx_desc_packet_split *) desc;
+            size_t offset = offsetof(union e1000_rx_desc_packet_split,
+                wb.middle.status_error);
+            uint32_t status = d->wb.middle.status_error;
+
+            d->wb.middle.status_error &= ~E1000_RXD_STAT_DD;
+            pci_dma_write(dev, addr, desc, len);
+
+            if (status & E1000_RXD_STAT_DD) {
+                d->wb.middle.status_error = status;
+                pci_dma_write(dev, addr + offset, &status, sizeof(status));
+            }
+        } else {
+            union e1000_rx_desc_extended *d =
+                (union e1000_rx_desc_extended *) desc;
+            size_t offset = offsetof(union e1000_rx_desc_extended,
+                wb.upper.status_error);
+            uint32_t status = d->wb.upper.status_error;
+
+            d->wb.upper.status_error &= ~E1000_RXD_STAT_DD;
+            pci_dma_write(dev, addr, desc, len);
+
+            if (status & E1000_RXD_STAT_DD) {
+                d->wb.upper.status_error = status;
+                pci_dma_write(dev, addr + offset, &status, sizeof(status));
+            }
+        }
+    }
+}
+
 typedef struct e1000e_ba_state_st {
     uint16_t written[MAX_PS_BUFFERS];
     uint8_t cur_idx;
@@ -1600,7 +1651,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
 
         e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL,
                            rss_info, do_ps ? ps_hdr_len : 0, &bastate.written);
-        pci_dma_write(d, base, &desc, core->rx_desc_len);
+        e1000e_pci_dma_write_rx_desc(core, base, desc, core->rx_desc_len);
 
         e1000e_ring_advance(core, rxi,
                             core->rx_desc_len / E1000_MIN_RX_DESC_LEN);
-- 
2.17.1
Re: [PATCH v2] e1000e: set RX desc status with DD flag in a separate operation
Posted by Jason Wang 3 years, 4 months ago
在 2022/9/14 20:16, Ding Hui 写道:
> Like commit 034d00d48581 ("e1000: set RX descriptor status in
> a separate operation"), there is also same issue in e1000e, which
> would cause lost packets or stop sending packets to VM with DPDK.
>
> Do similar fix in e1000e.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/402
> Signed-off-by: Ding Hui <dinghui@sangfor.com.cn>


Applied.

Thanks


> ---
>   hw/net/e1000e_core.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 52 insertions(+), 1 deletion(-)
>
> ---
> v2: use uint8_t/uint32_t directly instead of typeof
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 208e3e0d79..a570b366b2 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -1364,6 +1364,57 @@ struct NetRxPkt *pkt, const E1000E_RSSInfo *rss_info,
>       }
>   }
>   
> +static inline void
> +e1000e_pci_dma_write_rx_desc(E1000ECore *core, dma_addr_t addr,
> +                             uint8_t *desc, dma_addr_t len)
> +{
> +    PCIDevice *dev = core->owner;
> +
> +    if (e1000e_rx_use_legacy_descriptor(core)) {
> +        struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
> +        size_t offset = offsetof(struct e1000_rx_desc, status);
> +        uint8_t status = d->status;
> +
> +        d->status &= ~E1000_RXD_STAT_DD;
> +        pci_dma_write(dev, addr, desc, len);
> +
> +        if (status & E1000_RXD_STAT_DD) {
> +            d->status = status;
> +            pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +        }
> +    } else {
> +        if (core->mac[RCTL] & E1000_RCTL_DTYP_PS) {
> +            union e1000_rx_desc_packet_split *d =
> +                (union e1000_rx_desc_packet_split *) desc;
> +            size_t offset = offsetof(union e1000_rx_desc_packet_split,
> +                wb.middle.status_error);
> +            uint32_t status = d->wb.middle.status_error;
> +
> +            d->wb.middle.status_error &= ~E1000_RXD_STAT_DD;
> +            pci_dma_write(dev, addr, desc, len);
> +
> +            if (status & E1000_RXD_STAT_DD) {
> +                d->wb.middle.status_error = status;
> +                pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +            }
> +        } else {
> +            union e1000_rx_desc_extended *d =
> +                (union e1000_rx_desc_extended *) desc;
> +            size_t offset = offsetof(union e1000_rx_desc_extended,
> +                wb.upper.status_error);
> +            uint32_t status = d->wb.upper.status_error;
> +
> +            d->wb.upper.status_error &= ~E1000_RXD_STAT_DD;
> +            pci_dma_write(dev, addr, desc, len);
> +
> +            if (status & E1000_RXD_STAT_DD) {
> +                d->wb.upper.status_error = status;
> +                pci_dma_write(dev, addr + offset, &status, sizeof(status));
> +            }
> +        }
> +    }
> +}
> +
>   typedef struct e1000e_ba_state_st {
>       uint16_t written[MAX_PS_BUFFERS];
>       uint8_t cur_idx;
> @@ -1600,7 +1651,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt,
>   
>           e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL,
>                              rss_info, do_ps ? ps_hdr_len : 0, &bastate.written);
> -        pci_dma_write(d, base, &desc, core->rx_desc_len);
> +        e1000e_pci_dma_write_rx_desc(core, base, desc, core->rx_desc_len);
>   
>           e1000e_ring_advance(core, rxi,
>                               core->rx_desc_len / E1000_MIN_RX_DESC_LEN);