Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Mon, Mar 22, 2021 at 01:48:07PM +0800, Bin Meng wrote: > Hi David, > > On Mon, Mar 22, 2021 at 1:24 PM David Gibson > wrote: > > > > On Mon, Mar 22, 2021 at 12:33:06PM +0800, Bin Meng wrote: > > > Hi David, > > > > > > On Mon, Mar 22, 2021 at 12:11 PM David Gibson > > > wrote: > > > > > > > > On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote: > > > > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > > > > > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > > > > > > > Sorry, your reasoning still isn't clear to me. If qemu is not adding > > > > the CRC, what is? > > > > > > No one is padding CRC in QEMU. QEMU network backends pass payload > > > without CRC in between. > > > > Ok, but the CRCs must be added if the packets are bridged onto a real > > device, yes? Where does that happen? > > I've never used it like that before. What's the command line to test that? > > > > > > > > Will it always append a CRC after this padding is complete? > > > > > > No. > > > > If that's true, then won't the packets still be shorter than expected > > if we only pad to 60 bytes? > > In QEMU packets are transmitted without CRC between network backends, > and when a NIC receives a packet, the minimum required payload length > is 60 bytes without a CRC. Ok, I see what you're saying, and indeed it already pads to 60 bytes, rather than 64 on the Rx side. -- 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
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote:
> As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU",
> min_frame_len should excluce CRC, so it should be 60 instead of 64.
>
> Signed-off-by: Bin Meng
Applied to ppc-for-6.0, thanks.
> ---
>
> 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 d6be0d7d18..8f08446415 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -259,7 +259,7 @@ static void process_tx_bd(eTSEC *etsec,
> || etsec->regs[MACCFG2].value & MACCFG2_PADCRC) {
>
> /* Padding and CRC (Padding implies CRC) */
> -tx_padding_and_crc(etsec, 64);
> +tx_padding_and_crc(etsec, 60);
>
> } else if (etsec->first_bd.flags & BD_TX_TC
> || etsec->regs[MACCFG2].value & MACCFG2_CRC_EN) {
--
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
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
Hi David, On Mon, Mar 22, 2021 at 1:24 PM David Gibson wrote: > > On Mon, Mar 22, 2021 at 12:33:06PM +0800, Bin Meng wrote: > > Hi David, > > > > On Mon, Mar 22, 2021 at 12:11 PM David Gibson > > wrote: > > > > > > On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote: > > > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > > > > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > > > > > Sorry, your reasoning still isn't clear to me. If qemu is not adding > > > the CRC, what is? > > > > No one is padding CRC in QEMU. QEMU network backends pass payload > > without CRC in between. > > Ok, but the CRCs must be added if the packets are bridged onto a real > device, yes? Where does that happen? I've never used it like that before. What's the command line to test that? > > > > > Will it always append a CRC after this padding is complete? > > > > No. > > If that's true, then won't the packets still be shorter than expected > if we only pad to 60 bytes? In QEMU packets are transmitted without CRC between network backends, and when a NIC receives a packet, the minimum required payload length is 60 bytes without a CRC. Regards, Bin
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Mon, Mar 22, 2021 at 12:33:06PM +0800, Bin Meng wrote: > Hi David, > > On Mon, Mar 22, 2021 at 12:11 PM David Gibson > wrote: > > > > On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote: > > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > > > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > > > Sorry, your reasoning still isn't clear to me. If qemu is not adding > > the CRC, what is? > > No one is padding CRC in QEMU. QEMU network backends pass payload > without CRC in between. Ok, but the CRCs must be added if the packets are bridged onto a real device, yes? Where does that happen? > > > Will it always append a CRC after this padding is complete? > > No. If that's true, then won't the packets still be shorter than expected if we only pad to 60 bytes? -- 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
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
Hi David, On Mon, Mar 22, 2021 at 12:11 PM David Gibson wrote: > > On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote: > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > Sorry, your reasoning still isn't clear to me. If qemu is not adding > the CRC, what is? No one is padding CRC in QEMU. QEMU network backends pass payload without CRC in between. > Will it always append a CRC after this padding is complete? No. Regards, Bin
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Tue, Mar 16, 2021 at 04:15:05PM +0800, Bin Meng wrote:
> As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU",
> min_frame_len should excluce CRC, so it should be 60 instead of 64.
Sorry, your reasoning still isn't clear to me. If qemu is not adding
the CRC, what is? Will it always append a CRC after this padding is
complete?
>
> Signed-off-by: Bin Meng
> ---
>
> 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 d6be0d7d18..8f08446415 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -259,7 +259,7 @@ static void process_tx_bd(eTSEC *etsec,
> || etsec->regs[MACCFG2].value & MACCFG2_PADCRC) {
>
> /* Padding and CRC (Padding implies CRC) */
> -tx_padding_and_crc(etsec, 64);
> +tx_padding_and_crc(etsec, 60);
>
> } else if (etsec->first_bd.flags & BD_TX_TC
> || etsec->regs[MACCFG2].value & MACCFG2_CRC_EN) {
--
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
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Mon, Mar 22, 2021 at 09:29:12AM +0800, Bin Meng wrote: > On Tue, Mar 16, 2021 at 4:15 PM Bin Meng wrote: > > > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > > > Signed-off-by: Bin Meng > > --- > > > > hw/net/fsl_etsec/rings.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Ping? Sorry, I was away on holiday last week. -- 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
Re: [PATCH] hw/net: fsl_etsec: Tx padding length should exclude CRC
On Tue, Mar 16, 2021 at 4:15 PM Bin Meng wrote: > > As the comment of tx_padding_and_crc() says: "Never add CRC in QEMU", > min_frame_len should excluce CRC, so it should be 60 instead of 64. > > Signed-off-by: Bin Meng > --- > > hw/net/fsl_etsec/rings.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Ping?
