Re: [Qemu-devel] [PATCH v3 2/2] net: introduce command to query rx-filter information

[Markus Armbruster <armbru@redhat.com>]

Amos Kong <akong@redhat.com> writes:

> On Thu, Jul 04, 2013 at 08:28:59AM +0200, Markus Armbruster wrote:
>> Amos Kong <akong@redhat.com> writes:
>> 
>> > On Tue, Jul 02, 2013 at 03:27:12PM +0200, Markus Armbruster wrote:
>> >> Amos Kong <akong@redhat.com> writes:
>> >> 
>> >> > On Tue, Jul 02, 2013 at 11:05:56AM +0200, Markus Armbruster wrote:
>> >> >> Amos Kong <akong@redhat.com> writes:
>> >> [...]
>> >> >> >> This interface is abstract in the sense that it applies to
>> >> >> >> all NICs.  At
>> >> >> >> this time, it's implemented only virtio-net implements it.  I'm
>> >> >> >> habitually wary of abstractions based on just one concrete instance,
>> >> >> >> which makes me ask:
>> >> >> >> 
>> >> >> >> 1. Ignorant question first: could the feature make sense
>> >> >> >> for other NICs,
>> >> >> >> too, or is it specific to virtio-net?
>> >> >> >
>> >> >> > We will not. 
>> >> >> >
>> >> >> > It's ugly to check if nic is virtio-net nic in net/net.c, so I
>> >> >> > register the query function to NetClientInfo. Traversal the net
>> >> >> > client list in net/net.c, and execute query of each virtio-net
>> >> >> > instance in virtio-net.c
>> >> >> 
>> >> >> Implementing the feature as an optional callback is fine.
>> >> >> 
>> >> >> Let me rephrase my question: could this feature be implemented for other
>> >> >> NICs?  I'm *not* asking you to do that, just whether it would be
>> >> >> possible.
>> >> >> 
>> >> >> I'm asking because my review of the QAPI schema depends on the answer.
>> >> >> 
>> >> >> >> 2. If the former, are you reasonably sure this object will
>> >> >> >> do for other
>> >> >> >> NICs?
>> >> >> >
>> >> >> > No.
>> >> >> 
>> >> >> I'm not sure I understand you.  Do you mean to say that the feature
>> >> >> could be implemented for other NICs, but RxFilterInfo would probably not
>> >> >> fit for them?
>> >> >
>> >> > We will not implement the feature to other NICs, no request.
>> >> >
>> >> > We notify the management of virtio-net rx-filter change, because
>> >> > we want to sync the the rx-filter change to macvtap device.
>> >> 
>> >> I understand there are no plans to implement this feature for other
>> >> NICs.  But I'm not asking whether we *want* to implement it for other
>> >> NICs, I'm asking whether we *could*.
>> >  
>> > In theory, we can.
>> >
>> >> Or rephrased yet another way: what exactly makes this feature applicable
>> >> to virtio-net only?
>> >
>> > Macvtap can only be used by virtio-net, not other emulated nic.
>> > It's meaningless for management to know the rx-filter change of
>> > non-virtio-net NICs.
>> 
>> I'm having trouble squaring "in theory, we can" with "meaningless".  So
>> I'm rephrasing my question yet again.
>> 
>> Do NICs other than virtio-net have rx-filters?
>  
> Yes.
>
> Talked with Jason, upstream kernel fixed some bugs, then we can also
> use e1000 with macvtap. In this case, we should also implement a
> .query_rx_filter function for e1000. We can do it by another patchset.

Yes.  Just to avoid misunderstandings: I'm not asking you for that.  I
merely asked whether it's possible, and you answered that.

>> If yes, what have these NIC rx-filters in common, and how do they
>> differ?
>> 
>> Why would anybody want to query rx-filters?  Use cases, please.
>
> It's a way to tell management the rx-filter setup in guest nic.
> Management query the rx-filter setup of guest, then change the
> setup of macvtap device, macvtap uses same rx-filter setup as
> virtual nic.

So this use case is "mirror the virtual NIC's rx-filter setup to
macvtap".  Makes sense.

This leads me to the question I've been aiming for all along: will your
definition of RxFilterInfo do for devices other than virtio-net?

It should do if it contains only stuff that all NICs with an rx-filter
have.  Is that the case?

[...]