> -----Original Message-----
> From: Akihiko Odaki <[email protected]>
> Sent: Friday, 14 April 2023 13:37
> Cc: Sriram Yagnaraman <[email protected]>; Jason Wang
> <[email protected]>; Dmitry Fleytman <[email protected]>;
> Michael S. Tsirkin <[email protected]>; Alex Bennée <[email protected]>;
> Philippe Mathieu-Daudé <[email protected]>; Thomas Huth
> <[email protected]>; Wainer dos Santos Moschetta
> <[email protected]>; Beraldo Leal <[email protected]>; Cleber Rosa
> <[email protected]>; Laurent Vivier <[email protected]>; Paolo Bonzini
> <[email protected]>; [email protected]; Akihiko Odaki
> <[email protected]>
> Subject: [PATCH 30/40] igb: Implement igb-specific oversize check
>
> igb has a configurable size limit for LPE, and uses different limits
> depending on
> whether the packet is treated as a VLAN packet.
>
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
> hw/net/igb_core.c | 41 +++++++++++++++++++++++++++--------------
> 1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> 2013a9a53d..569897fb99 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -954,16 +954,21 @@ igb_rx_l4_cso_enabled(IGBCore *core)
> return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD); }
>
> -static bool
The convention in seems to be to declare return value in first line and then
the function name in the next line.
> -igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
> +static bool igb_rx_is_oversized(IGBCore *core, const struct eth_header *ehdr,
> + size_t size, bool lpe, uint16_t rlpml)
> {
> - uint16_t pool = qn % IGB_NUM_VM_POOLS;
> - bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
> - int max_ethernet_lpe_size =
> - core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
> - int max_ethernet_vlan_size = 1522;
> + size += 4;
Is the above 4 CRC bytes?
> +
> + if (lpe) {
> + return size > rlpml;
> + }
> +
> + if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0xffff) &&
> + e1000x_vlan_rx_filter_enabled(core->mac)) {
> + return size > 1522;
> + }
Should a check for 1526 bytes if extended VLAN is present be added?
Maybe in "igb: Strip the second VLAN tag for extended VLAN"?
>
> - return size > (lpe ? max_ethernet_lpe_size : max_ethernet_vlan_size);
> + return size > 1518;
> }
>
> static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
> @@ -976,6 +981,8 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const L2Header *l2_header,
> uint16_t queues = 0;
> uint16_t oversized = 0;
> uint16_t vid = be16_to_cpu(l2_header->vlan[0].h_tci) & VLAN_VID_MASK;
> + bool lpe;
> + uint16_t rlpml;
> int i;
>
> memset(rss_info, 0, sizeof(E1000E_RSSInfo)); @@ -984,6 +991,14 @@
> static uint16_t igb_receive_assign(IGBCore *core, const L2Header *l2_header,
> *external_tx = true;
> }
>
> + lpe = !!(core->mac[RCTL] & E1000_RCTL_LPE);
> + rlpml = core->mac[RLPML];
> + if (!(core->mac[RCTL] & E1000_RCTL_SBP) &&
> + igb_rx_is_oversized(core, ehdr, size, lpe, rlpml)) {
> + trace_e1000x_rx_oversized(size);
> + return queues;
> + }
> +
> if (e1000x_is_vlan_packet(ehdr, core->mac[VET] & 0xffff) &&
> !e1000x_rx_vlan_filter(core->mac, PKT_GET_VLAN_HDR(ehdr))) {
> return queues;
> @@ -1067,7 +1082,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const L2Header *l2_header,
> queues &= core->mac[VFRE];
> if (queues) {
> for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> - if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size))
> {
> + lpe = !!(core->mac[VMOLR0 + i] & E1000_VMOLR_LPE);
> + rlpml = core->mac[VMOLR0 + i] & E1000_VMOLR_RLPML_MASK;
> + if ((queues & BIT(i)) &&
> + igb_rx_is_oversized(core, ehdr, size, lpe, rlpml))
> + {
> oversized |= BIT(i);
> }
> }
> @@ -1609,11 +1627,6 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
> iov_to_buf(iov, iovcnt, iov_ofs, &min_buf,
> sizeof(min_buf.l2_header));
> }
>
> - /* Discard oversized packets if !LPE and !SBP. */
> - if (e1000x_is_oversized(core->mac, size)) {
> - return orig_size;
> - }
> -
> net_rx_pkt_set_packet_type(core->rx_pkt,
> get_eth_packet_type(&min_buf.l2_header.eth));
> net_rx_pkt_set_protocols(core->rx_pkt, iov, iovcnt, iov_ofs);
> --
> 2.40.0
