[PATCH] Revert "hw/net: Fix the transmission return size"

[PATCH] Revert "hw/net: Fix the transmission return size"

[Ilya Repko]

This reverts commit 3a6d374b754b4b345195ff6846eeaffedc96a7c5.

During axienet_eth_rx_notify(), s->rxpos is modified to indicate how much
data was pushed to AXI DMA. eth_rx() would then return this value.
If at 0, network subsystem would consider packet reception as failed
and put the packet in a queue for later.

Before we attempt to push packet data to AXI DMA, the packet is stored
in s->rxmem buffer. If an attempt to push data fails, we will reattempt
to deliver it from s->rxmem buffer once s2mm stream gets a new descriptor.
s->rxmem would not be overwritten by a subsequent eth_rx() call, because
eth_can_rx() protects it in case it has any data at all. Leaving the packet
in a NetQueue though effectively duplicates it.

Therefore, eth_rx() must indicate successful packet reception in case
data push to AXI DMA fails.

Signed-off-by: Ilya Repko <i.repko@syntacore.com>
---
 hw/net/xilinx_axienet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 31e7708082..101b3f260a 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -867,7 +867,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
     axienet_eth_rx_notify(s);
 
     enet_update_irq(s);
-    return s->rxpos;
+    return size;
 }
 
 static size_t
-- 
2.51.1

Re: [PATCH] Revert "hw/net: Fix the transmission return size"

[Edgar E. Iglesias]

On Thu, Nov 06, 2025 at 12:48:37PM +0300, Ilya Repko wrote:
> This reverts commit 3a6d374b754b4b345195ff6846eeaffedc96a7c5.
> 
> During axienet_eth_rx_notify(), s->rxpos is modified to indicate how much
> data was pushed to AXI DMA. eth_rx() would then return this value.
> If at 0, network subsystem would consider packet reception as failed
> and put the packet in a queue for later.
> 
> Before we attempt to push packet data to AXI DMA, the packet is stored
> in s->rxmem buffer. If an attempt to push data fails, we will reattempt
> to deliver it from s->rxmem buffer once s2mm stream gets a new descriptor.
> s->rxmem would not be overwritten by a subsequent eth_rx() call, because
> eth_can_rx() protects it in case it has any data at all. Leaving the packet
> in a NetQueue though effectively duplicates it.
> 
> Therefore, eth_rx() must indicate successful packet reception in case
> data push to AXI DMA fails.

Hi,

Adding Fea since we're reverting his patch, he may have some insights.

What you describe sounds reasonable but I think we've seen issues with
both your version (the original one) and Fea's version.

What machine are you running? Are you running Linux guests?

Cheers,
Edgar




> 
> Signed-off-by: Ilya Repko <i.repko@syntacore.com>
> ---
>  hw/net/xilinx_axienet.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 31e7708082..101b3f260a 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -867,7 +867,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>      axienet_eth_rx_notify(s);
>  
>      enet_update_irq(s);
> -    return s->rxpos;
> +    return size;
>  }
>  
>  static size_t
> -- 
> 2.51.1
> 

Re: [PATCH] Revert "hw/net: Fix the transmission return size"

[Ilya Repko]

Hi,

We encountered this issue with U-Boot driver. It implements a single RX
buffer, which gets resubmitted to s2mm once the network stack is done
handling a packet, so for a while the DMA engine has no buffers to
store incoming packets into, and that's why we're seeing AXI DMA data
push failures, which lead to packets duplicating and NetQueue growing
indefinitely.

Haven't really tested with Linux. What was the problem with the kernel
driver?

Ilya

On Thu, 2025-11-06 at 20:16 +0100, Edgar E. Iglesias wrote:
> On Thu, Nov 06, 2025 at 12:48:37PM +0300, Ilya Repko wrote:
> > This reverts commit 3a6d374b754b4b345195ff6846eeaffedc96a7c5.
> > 
> > During axienet_eth_rx_notify(), s->rxpos is modified to indicate
> > how much
> > data was pushed to AXI DMA. eth_rx() would then return this value.
> > If at 0, network subsystem would consider packet reception as
> > failed
> > and put the packet in a queue for later.
> > 
> > Before we attempt to push packet data to AXI DMA, the packet is
> > stored
> > in s->rxmem buffer. If an attempt to push data fails, we will
> > reattempt
> > to deliver it from s->rxmem buffer once s2mm stream gets a new
> > descriptor.
> > s->rxmem would not be overwritten by a subsequent eth_rx() call,
> > because
> > eth_can_rx() protects it in case it has any data at all. Leaving
> > the packet
> > in a NetQueue though effectively duplicates it.
> > 
> > Therefore, eth_rx() must indicate successful packet reception in
> > case
> > data push to AXI DMA fails.
> 
> Hi,
> 
> Adding Fea since we're reverting his patch, he may have some
> insights.
> 
> What you describe sounds reasonable but I think we've seen issues
> with
> both your version (the original one) and Fea's version.
> 
> What machine are you running? Are you running Linux guests?
> 
> Cheers,
> Edgar
> 
> 
> 
> 
> > 
> > Signed-off-by: Ilya Repko <i.repko@syntacore.com>
> > ---
> >  hw/net/xilinx_axienet.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> > index 31e7708082..101b3f260a 100644
> > --- a/hw/net/xilinx_axienet.c
> > +++ b/hw/net/xilinx_axienet.c
> > @@ -867,7 +867,7 @@ static ssize_t eth_rx(NetClientState *nc, const
> > uint8_t *buf, size_t size)
> >      axienet_eth_rx_notify(s);
> >  
> >      enet_update_irq(s);
> > -    return s->rxpos;
> > +    return size;
> >  }
> >  
> >  static size_t
> > -- 
> > 2.51.1
> > 

[PATCH] Revert "hw/net: Fix the transmission return size"

[Ilya Repko]

This reverts commit 3a6d374b754b4b345195ff6846eeaffedc96a7c5.

During axienet_eth_rx_notify(), s->rxpos is modified to indicate how much
data was pushed to AXI DMA. eth_rx() would then return this value.
If at 0, network subsystem would consider packet reception as failed
and put the packet in a queue for later.

Before we attempt to push packet data to AXI DMA, the packet is stored
in s->rxmem buffer. If an attempt to push data fails, we will reattempt
to deliver it from s->rxmem buffer once s2mm stream gets a new descriptor.
s->rxmem would not be overwritten by a subsequent eth_rx() call, because
eth_can_rx() protects it in case it has any data at all. Leaving the packet
in a NetQueue though effectively duplicates it.

Therefore, eth_rx() must indicate successful packet reception in case
data push to AXI DMA fails.

Signed-off-by: Ilya Repko <i.repko@syntacore.com>
---
 hw/net/xilinx_axienet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 31e7708082..101b3f260a 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -867,7 +867,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
     axienet_eth_rx_notify(s);
 
     enet_update_irq(s);
-    return s->rxpos;
+    return size;
 }
 
 static size_t
-- 
2.51.1