Re: [PATCH v3] hw/net: fsl_etsec: Reverse the RCTRL.RSF logic

[David Gibson]

On Wed, Feb 10, 2021 at 10:10:21AM +0800, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per MPC8548ERM [1] chapter 14.5.3.4.1:
> 
> When RCTRL.RSF is 1, frames less than 64 bytes are accepted upon
> a DA match. But currently QEMU does the opposite. This commit
> reverses the RCTRL.RSF testing logic to match the manual.
> 
> Due to the reverse of the logic, certain guests may potentially
> break if they don't program eTSEC to have RCTRL.RSF bit set.
> When RCTRL.RSF is 0, short frames are silently dropped, however
> as of today both slirp and tap networking do not pad short frames
> (e.g.: an ARP packet) to the minimum frame size of 60 bytes. So
> ARP requests will be dropped, preventing the guest from becoming
> visible on the network.
> 
> The same issue was reported on e1000 and vmxenet3 before, see:
> 
> commit 78aeb23eded2 ("e1000: Pad short frames to minimum size (60 bytes)")
> commit 40a87c6c9b11 ("vmxnet3: Pad short frames to minimum size (60 bytes)")
> 
> [1] https://www.nxp.com/docs/en/reference-manual/MPC8548ERM.pdf
> 
> Fixes: eb1e7c3e5146 ("Add Enhanced Three-Speed Ethernet Controller (eTSEC)")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Applied to ppc-for-6.0.

> 
> ---
> 
> Changes in v3:
> - remove the slirp/tap networking workaround and only do the reverse
> 
> Changes in v2:
> - rewrite the commit message and reverse the RCTRL.RSF test logic
> 
>  hw/net/fsl_etsec/rings.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 121415a..fe055d3 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -502,7 +502,7 @@ ssize_t etsec_rx_ring_write(eTSEC *etsec, const uint8_t 
> *buf, size_t size)
>          return -1;
>      }
>  
> -    if ((etsec->regs[RCTRL].value & RCTRL_RSF) && (size < 60)) {
> +    if (!(etsec->regs[RCTRL].value & RCTRL_RSF) && (size < 60)) {
>          /* CRC is not in the packet yet, so short frame is below 60 bytes */
>          RING_DEBUG("%s: Drop short frame\n", __func__);
>          return -1;

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature