Re: [PATCH 15/40] e1000x: Take CRC into consideration for size check

[Philippe Mathieu-Daudé]

On 14/4/23 13:37, Akihiko Odaki wrote:
> Section 13.7.15 Receive Length Error Count says:
> >   Packets over 1522 bytes are oversized if LongPacketEnable is 0b
> > (RCTL.LPE). If LongPacketEnable (LPE) is 1b, then an incoming packet
> > is considered oversized if it exceeds 16384 bytes.
>
> > These lengths are based on bytes in the received packet from
> > <Destination Address> through <CRC>, inclusively.
>
> As QEMU processes packets without CRC, the number of bytes for CRC
> need to be subtracted.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   hw/net/e1000x_common.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
> index 6cc23138a8..b4dfc74b66 100644
> --- a/hw/net/e1000x_common.c
> +++ b/hw/net/e1000x_common.c
> @@ -142,10 +142,10 @@ bool e1000x_is_oversized(uint32_t *mac, size_t size)
>   {
>       /* this is the size past which hardware will
>          drop packets when setting LPE=0 */
> -    static const int maximum_ethernet_vlan_size = 1522;
> +    static const int maximum_ethernet_vlan_size = 1522 - 4;
>       /* this is the size past which hardware will
>          drop packets when setting LPE=1 */
> -    static const int maximum_ethernet_lpe_size = 16 * KiB;
> +    static const int maximum_ethernet_lpe_size = 16 * KiB - 4;
>   
>       if ((size > maximum_ethernet_lpe_size ||
>           (size > maximum_ethernet_vlan_size

IMHO this function could be simplified. Something like:

  bool long_packet_enabled = mac[RCTL] & E1000_RCTL_LPE;
  size_t oversize = long_packet_enabled ? 16 * KiB : ETH_VLAN_MAXSIZE;
  size_t crc32_size = sizeof(uint32_t);

  if (mac[RCTL] & E1000_RCTL_SBP) {
    return false;
  }

  if (size + crc32_size > oversize ) {
    ...
    return true;
  }

  return false;
}