Re: [virtio-comment] [PATCH v10] virtio-net: Clarify VLAN filter table configuration

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jan 26, 2023 at 04:27:47PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, January 26, 2023 12:08 AM
> > 
> > On Wed, Jan 25, 2023 at 11:46:58PM +0000, Parav Pandit wrote:
> > > > > but vlans are different aren't they? Anyway changing that will
> > > > > need a new feature bit.
> > > > I do not the history for about "Note" on why it was made best effort.
> > > > To best of my knowledge, it should not be best effort.
> > > > Device should not overflow the table. It should fail the ADD call.
> > 
> > For the MAC table?  Failing commands generally is a bad way to communicate
> > to drivers. 
> Failing the command when device cannot do the job is right way to tell the driver to follow the contract in general terms.

There's no contract though. My point is that table size is not
in the spec. As long as that is the case failing because it's too big
is a bad idea.

> A command could be to disable a vq, delete entry, add entry, encrypt data, LSO packets and more.
> Again in general terms. :)
> Device telling driver that it did the job and not doing it in device only brings inferior user experience.
> 
> May be QEMU has some history...
> 
> > If we do there needs to also be a way to report our limits. 
> Yes, I agree. Reporting limits is good approach we should follow.
> 
> > In this case
> > it's enough to look at qemu to see an implementation that just sets a "table
> > overflow" flag and stops filtering.
> > 
> > Similarly a populat vhost+bridge+tap combo does not keep the copy of the mac
> > table at all and instead realies on guest sending announcement packets and
> > bridge learning from these.
> >
> How does this work in a sane way?
> Driver set the mac filter table and it doesn't expect random packets to come in?
> If the device doesn't maintain the filter table, it is "SHOULD" requirement as stated today on best effort basis.

exaactly, that is why it's SHOULD.

> > 
> > > > If the above claim of "Note" is not strong enough, we can probably
> > > > run a different issue to make it "MUST".
> > > > I am not sure we really need feature bit it we fix the note.
> > >
> > > I clicked too early while typing.
> > > Since DEL operation doesn't synchronize with the data path of device and
> > driver, at some point a packet in pipe can be still received with removed VLAN.
> > > So may be the note was added.
> > > But for sure ADD should not be best effort basis.
> > 
> > So for VLANs this was added by Tiwei. I guess it looked sane then but now I
> > don't remember the motivation. Tiwei CC'd.
> > --
> > MST