Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE

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

On 2023/01/28 22:46, Sriram Yagnaraman wrote:
> RSS for VFs is only enabled if VMOLR[n].RSSE is set.
> 
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
>   hw/net/igb_core.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 1eb7ba168f..e4fd4a1a5f 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -69,7 +69,7 @@ typedef struct IGBTxPktVmdqCallbackContext {
>   
>   static ssize_t
>   igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> -                     bool has_vnet, bool *assigned);
> +                     bool has_vnet, bool *external_tx);

I admit external_tx is somewhat confusing, but it is more than just 
telling if it is assigned to Rx queue or not. If external_tx is not 
NULL, it indicates it is part of Tx packet switching. In that case, a 
bool value which describes whether the packet should be routed to 
external LAN must be assigned. The value can be false even if the packet 
is assigned to Rx queues; it will be always false if it is multicast, 
for example.

>   
>   static inline void
>   igb_set_interrupt_cause(IGBCore *core, uint32_t val);
> @@ -942,7 +942,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_VF_FUNCTIONS; i++) {

I just left it as 8 because VMDq is not specific to VF. Perhaps it is 
better to have another macro to denote the number of VMDq pools, but it 
is not done yet.

>                   if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
>                       queues |= BIT(i);
>                   }
> @@ -976,7 +976,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_VF_FUNCTIONS; i++) {
>                           if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
>                               queues |= BIT(i);
>                           }
> @@ -999,7 +999,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_VF_FUNCTIONS; i++) {
>                       if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
>                           mask |= BIT(i);
>                       }
> @@ -1018,7 +1018,15 @@ 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;
> +            for (i = 0; i < IGB_MAX_VF_FUNCTIONS; i++) {
> +                if (!(queues & BIT(i))) {
> +                    continue;
> +                }
> +                if (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE) {
> +                    queues |= BIT(i + IGB_MAX_VF_FUNCTIONS);
> +                    queues &= ~BIT(i);
> +                }
> +            }
>           }
>       } else {
>           switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {