RE: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field

[Sriram Yagnaraman]

> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Wednesday, 1 February 2023 05:58
> 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 v3 8/9] igb: respect VT_CTL ignore MAC field
> 
> On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> > Also trace out a warning if replication mode is disabled, since we
> > only support replication mode enabled.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > ---
> >   hw/net/igb_core.c   | 9 +++++++++
> >   hw/net/trace-events | 2 ++
> >   2 files changed, 11 insertions(+)
> >
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > c5f9c14f47..8115be2d76 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> >       }
> >
> >       if (core->mac[MRQC] & 1) {
> > +        if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
> > +            trace_igb_rx_vmdq_replication_mode_disabled();
> > +        }
> > +
> >           if (is_broadcast_ether_addr(ehdr->h_dest)) {
> >               for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> >                   if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) { @@
> > -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> >               }
> >           }
> >
> > +        /* assume a full pool list if IGMAC is set */
> > +        if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
> > +            queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
> > +        }
> > +
> 
> This overwrites "queues", but "external_tx" is not overwritten.

Description in section 7.10.3.6 is a bit confusing, I interpreted that packet 
is not sent to network if it matches an exact filter regardless of VT_CTL.IGMAC 
setting.
I think that VT_CTL.IGMAC setting is only for MAC filtering and pool selection, 
and not to determine if a packet must go to external LAN or not.

> 
> >           if (e1000x_vlan_rx_filter_enabled(core->mac)) {
> >               uint16_t mask = 0;
> >
> > diff --git a/hw/net/trace-events b/hw/net/trace-events index
> > e94172e748..9bc7658692 100644
> > --- a/hw/net/trace-events
> > +++ b/hw/net/trace-events
> > @@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t
> > offset, const void* source, uint3
> >
> >   igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
> >
> > +igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication
> mode enabled is supported"
> > +
> >   igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR
> enabled"
> >   igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr)
> "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
> >   igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"