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);
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++) {
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)) {
--
2.34.1On 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)) {
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Sunday, 29 January 2023 08:25
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
>
> 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.
Yes, I undestand the purpose of external_tx. I merely changed the parameter name in the function declaration to match it's defintion. Anyhow, I will remove this change since it was purely comsetic.
>
> >
> > 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.
Ok, I agree. I will introduce a new IGB_MAX_VM_POOLS macro instead.
>
> > 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)) {
On 2023/01/30 21:07, Sriram Yagnaraman wrote:
>> -----Original Message-----
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Sent: Sunday, 29 January 2023 08:25
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
>> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Subject: Re: [PATCH 5/9] igb: respect E1000_VMOLR_RSSE
>>
>> 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.
>
> Yes, I undestand the purpose of external_tx. I merely changed the parameter name in the function declaration to match it's defintion. Anyhow, I will remove this change since it was purely comsetic.
The definition is wrong then. I'll submit the new version with it fixed.
>
>>
>>>
>>> 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.
>
> Ok, I agree. I will introduce a new IGB_MAX_VM_POOLS macro instead.
You don't need "MAX" as the number of pools is fixed if I understand it
correctly.
>
>>
>>> 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)) {
© 2016 - 2026 Red Hat, Inc.