Re: [PATCH v2 5/9] igb: check oversized packets for VMDq

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/01/30 22:23, Sriram Yagnaraman wrote:
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
>   hw/net/igb_core.c | 48 +++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index cea7c036f0..89650fcfd4 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -910,12 +910,27 @@ igb_rx_l4_cso_enabled(IGBCore *core)
>       return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD);
>   }
>   
> +static bool
> +igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
> +{
> +    uint16_t pool = qn % IGB_MAX_VM_POOLS;
> +    bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
> +    int maximum_ethernet_lpe_size =
> +        core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
> +    int maximum_ethernet_vlan_size = 1522;
> +
> +    return (size > maximum_ethernet_lpe_size ||
> +        (size > maximum_ethernet_vlan_size && !lpe));

It will return true if !lpe && size > maximum_ethernet_lpe_size 
(maximum_ethernet_lpe_size can be smaller than 
maximum_ethernet_vlan_size although it is quite unlikely), but it should 
ignore maximum_ethernet_lpe_size in such a case.

Also you don't need the brace around the entire expression.

> +}
> +
>   static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> -                                   E1000E_RSSInfo *rss_info, bool *external_tx)
> +                                   size_t size, E1000E_RSSInfo *rss_info,
> +                                   bool *external_tx)
>   {
>       static const int ta_shift[] = { 4, 3, 2, 0 };
>       uint32_t f, ra[2], *macp, rctl = core->mac[RCTL];
>       uint16_t queues = 0;
> +    uint16_t oversized = 0;
>       uint16_t vid = lduw_be_p(&PKT_GET_VLAN_HDR(ehdr)->h_tci) & VLAN_VID_MASK;
>       bool accepted = false;
>       int i;
> @@ -941,7 +956,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>   
>       if (core->mac[MRQC] & 1) {
>           if (is_broadcast_ether_addr(ehdr->h_dest)) {
> -            for (i = 0; i < 8; i++) {
> +            for (i = 0; i < IGB_MAX_VM_POOLS; i++) {
>                   if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
>                       queues |= BIT(i);
>                   }
> @@ -975,7 +990,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>                   f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
>                   f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
>                   if (macp[f >> 5] & (1 << (f & 0x1f))) {
> -                    for (i = 0; i < 8; i++) {
> +                    for (i = 0; i < IGB_MAX_VM_POOLS; i++) {
>                           if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
>                               queues |= BIT(i);
>                           }
> @@ -998,7 +1013,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>                       }
>                   }
>               } else {
> -                for (i = 0; i < 8; i++) {
> +                for (i = 0; i < IGB_MAX_VM_POOLS; i++) {
>                       if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
>                           mask |= BIT(i);
>                       }
> @@ -1015,9 +1030,26 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>           }
>   
>           queues &= core->mac[VFRE];
> -        igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
> -        if (rss_info->queue & 1) {
> -            queues <<= 8;
> +        if (queues) {
> +            for (i = 0; i < IGB_MAX_VM_POOLS; i++) {
> +                if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size)) {
> +                    oversized |= BIT(i);
> +                }
> +            }
> +            /* 8.19.37 increment ROC if packet is oversized for all queues */
> +            if (oversized == queues) {
> +                trace_e1000x_rx_oversized(size);
> +                e1000x_inc_reg_if_not_full(core->mac, ROC);
> +            }
> +            queues &= ~oversized;
> +        }
> +
> +        if (queues) {
> +            igb_rss_parse_packet(core, core->rx_pkt,
> +                                 external_tx != NULL, rss_info);
> +            if (rss_info->queue & 1) {
> +                queues <<= 8;
> +            }
>           }
>       } else {
>           switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
> @@ -1561,7 +1593,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>                                  e1000x_vlan_enabled(core->mac),
>                                  core->mac[VET] & 0xffff);
>   
> -    queues = igb_receive_assign(core, ehdr, &rss_info, external_tx);
> +    queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
>       if (!queues) {
>           trace_e1000e_rx_flt_dropped();
>           return orig_size;