diff mbox

[RFC] net: Enable vlans and dump for -netdev, too,

Message ID 1432322539-23719-1-git-send-email-thuth@redhat.com
State New
Headers show

Commit Message

Thomas Huth May 22, 2015, 7:22 p.m. UTC
So far, it is not possible to use the network dump interface
with the "-netdev" option yet, it only works with the legacy
"-net" option. To be able to use it with "-netdev", too, this
patch now enables the "vlan" packet distribution for the
"-netdev" option, too, so that you can now dump network packets
like this:

qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
        -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
        -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 net/net.c        | 3 +++
 qapi-schema.json | 1 +
 2 files changed, 4 insertions(+)

 [Note: I'm quite new to this area of the code, please review carefully
  whether this approach makes sense or whether it should be done differently]

Comments

Eric Blake May 22, 2015, 7:42 p.m. UTC | #1
On 05/22/2015 01:22 PM, Thomas Huth wrote:
> So far, it is not possible to use the network dump interface
> with the "-netdev" option yet, it only works with the legacy
> "-net" option. To be able to use it with "-netdev", too, this
> patch now enables the "vlan" packet distribution for the
> "-netdev" option, too, so that you can now dump network packets
> like this:
> 
> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---

> +++ b/qapi-schema.json
> @@ -2512,6 +2512,7 @@
>  { 'struct': 'Netdev',
>    'data': {
>      'id':   'str',
> +    '*vlan': 'int32',
>      'opts': 'NetClientOptions' } }

Needs a documentation blurb for the new parameter, including '#optional'
and '(since 2.4)' markings.
Stefan Hajnoczi May 26, 2015, 12:52 p.m. UTC | #2
On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
> So far, it is not possible to use the network dump interface
> with the "-netdev" option yet, it only works with the legacy
> "-net" option. To be able to use it with "-netdev", too, this
> patch now enables the "vlan" packet distribution for the
> "-netdev" option, too, so that you can now dump network packets
> like this:
> 
> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  net/net.c        | 3 +++
>  qapi-schema.json | 1 +
>  2 files changed, 4 insertions(+)
> 
>  [Note: I'm quite new to this area of the code, please review carefully
>   whether this approach makes sense or whether it should be done differently]

We thought the QEMU "vlan" concept would be dropped completely in the
future, so it was never added to -netdev.  No patches to do that have
been posted over the years, so I think it was more of a conceptual goal
than a concrete requirement.

I'm okay with merging this but will leave some time for discussion
before merging the patch.

"Speak now or forever hold your peace"

Stefan
Thomas Huth May 26, 2015, 1:07 p.m. UTC | #3
On Tue, 26 May 2015 13:52:35 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
> > So far, it is not possible to use the network dump interface
> > with the "-netdev" option yet, it only works with the legacy
> > "-net" option. To be able to use it with "-netdev", too, this
> > patch now enables the "vlan" packet distribution for the
> > "-netdev" option, too, so that you can now dump network packets
> > like this:
> > 
> > qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
> >         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
> >         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  net/net.c        | 3 +++
> >  qapi-schema.json | 1 +
> >  2 files changed, 4 insertions(+)
> > 
> >  [Note: I'm quite new to this area of the code, please review carefully
> >   whether this approach makes sense or whether it should be done differently]
> 
> We thought the QEMU "vlan" concept would be dropped completely in the
> future, so it was never added to -netdev.  No patches to do that have
> been posted over the years, so I think it was more of a conceptual goal
> than a concrete requirement.
> 
> I'm okay with merging this but will leave some time for discussion
> before merging the patch.
> 
> "Speak now or forever hold your peace"

Certainly no objections from my side to keep this "vlan" concept in
QEMU, but thinking about this option again, I'd rather call it "hub=xx"
for the -netdev option instead of "vlan=xx", since this better matches
the "-netdev hubport" naming and it IMHO also describes the
functionality in a better way.
(of course I also need to add proper updates to the documentation files
in my patch in that way)

 Thomas
Markus Armbruster May 26, 2015, 2:29 p.m. UTC | #4
Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
>> So far, it is not possible to use the network dump interface
>> with the "-netdev" option yet, it only works with the legacy
>> "-net" option. To be able to use it with "-netdev", too, this
>> patch now enables the "vlan" packet distribution for the
>> "-netdev" option, too, so that you can now dump network packets
>> like this:
>> 
>> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
>>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
>>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
>> 
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  net/net.c        | 3 +++
>>  qapi-schema.json | 1 +
>>  2 files changed, 4 insertions(+)
>> 
>>  [Note: I'm quite new to this area of the code, please review carefully
>>   whether this approach makes sense or whether it should be done differently]
>
> We thought the QEMU "vlan" concept would be dropped completely in the
> future, so it was never added to -netdev.  No patches to do that have
> been posted over the years, so I think it was more of a conceptual goal
> than a concrete requirement.

Well, patches to do that first need to replace the VLAN-only dump
feature.

To fully deprecate -net, we also have to replace -net nic for
configuring onboard NICs.

Prior discussion:
http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html

We haven't really tried either.

Thomas's patch declares defeat on VLANs: it ports them to -netdev,
perpetuating the concept.

> I'm okay with merging this but will leave some time for discussion
> before merging the patch.
>
> "Speak now or forever hold your peace"

Sure we're ready to give up without having tried?
Daniel P. Berrangé May 26, 2015, 2:36 p.m. UTC | #5
On Tue, May 26, 2015 at 04:29:51PM +0200, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
> >> So far, it is not possible to use the network dump interface
> >> with the "-netdev" option yet, it only works with the legacy
> >> "-net" option. To be able to use it with "-netdev", too, this
> >> patch now enables the "vlan" packet distribution for the
> >> "-netdev" option, too, so that you can now dump network packets
> >> like this:
> >> 
> >> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
> >>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
> >>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
> >> 
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>  net/net.c        | 3 +++
> >>  qapi-schema.json | 1 +
> >>  2 files changed, 4 insertions(+)
> >> 
> >>  [Note: I'm quite new to this area of the code, please review carefully
> >>   whether this approach makes sense or whether it should be done differently]
> >
> > We thought the QEMU "vlan" concept would be dropped completely in the
> > future, so it was never added to -netdev.  No patches to do that have
> > been posted over the years, so I think it was more of a conceptual goal
> > than a concrete requirement.
> 
> Well, patches to do that first need to replace the VLAN-only dump
> feature.
> 
> To fully deprecate -net, we also have to replace -net nic for
> configuring onboard NICs.
> 
> Prior discussion:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
> 
> We haven't really tried either.
> 
> Thomas's patch declares defeat on VLANs: it ports them to -netdev,
> perpetuating the concept.
> 
> > I'm okay with merging this but will leave some time for discussion
> > before merging the patch.
> >
> > "Speak now or forever hold your peace"
> 
> Sure we're ready to give up without having tried?

Seems to me that we'd be better off taking the -netdev arg and
adding a new parameter 'dump=filename' that can be used with any
netdev backend. eg with tap

   -netdev tap,id=foo,fd=3,dump=/somefile,dumplen=1024

The use of a dedicated 'dump' network backend always appear rather
strange as a conceptual approach, and I don't think we should
propagate its use into -netdev

Regards,
Daniel
Stefan Hajnoczi May 26, 2015, 4:43 p.m. UTC | #6
On Tue, May 26, 2015 at 3:36 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Tue, May 26, 2015 at 04:29:51PM +0200, Markus Armbruster wrote:
>> Stefan Hajnoczi <stefanha@redhat.com> writes:
>>
>> > On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
>> >> So far, it is not possible to use the network dump interface
>> >> with the "-netdev" option yet, it only works with the legacy
>> >> "-net" option. To be able to use it with "-netdev", too, this
>> >> patch now enables the "vlan" packet distribution for the
>> >> "-netdev" option, too, so that you can now dump network packets
>> >> like this:
>> >>
>> >> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
>> >>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
>> >>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
>> >>
>> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> >> ---
>> >>  net/net.c        | 3 +++
>> >>  qapi-schema.json | 1 +
>> >>  2 files changed, 4 insertions(+)
>> >>
>> >>  [Note: I'm quite new to this area of the code, please review carefully
>> >>   whether this approach makes sense or whether it should be done differently]
>> >
>> > We thought the QEMU "vlan" concept would be dropped completely in the
>> > future, so it was never added to -netdev.  No patches to do that have
>> > been posted over the years, so I think it was more of a conceptual goal
>> > than a concrete requirement.
>>
>> Well, patches to do that first need to replace the VLAN-only dump
>> feature.
>>
>> To fully deprecate -net, we also have to replace -net nic for
>> configuring onboard NICs.
>>
>> Prior discussion:
>> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
>>
>> We haven't really tried either.
>>
>> Thomas's patch declares defeat on VLANs: it ports them to -netdev,
>> perpetuating the concept.
>>
>> > I'm okay with merging this but will leave some time for discussion
>> > before merging the patch.
>> >
>> > "Speak now or forever hold your peace"
>>
>> Sure we're ready to give up without having tried?
>
> Seems to me that we'd be better off taking the -netdev arg and
> adding a new parameter 'dump=filename' that can be used with any
> netdev backend. eg with tap
>
>    -netdev tap,id=foo,fd=3,dump=/somefile,dumplen=1024
>
> The use of a dedicated 'dump' network backend always appear rather
> strange as a conceptual approach, and I don't think we should
> propagate its use into -netdev

I'm happy with this approach too.

Stefan
Thomas Huth May 26, 2015, 6:15 p.m. UTC | #7
On Tue, 26 May 2015 17:43:09 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, May 26, 2015 at 3:36 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> > On Tue, May 26, 2015 at 04:29:51PM +0200, Markus Armbruster wrote:
> >> Stefan Hajnoczi <stefanha@redhat.com> writes:
> >>
> >> > On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
> >> >> So far, it is not possible to use the network dump interface
> >> >> with the "-netdev" option yet, it only works with the legacy
> >> >> "-net" option. To be able to use it with "-netdev", too, this
> >> >> patch now enables the "vlan" packet distribution for the
> >> >> "-netdev" option, too, so that you can now dump network packets
> >> >> like this:
> >> >>
> >> >> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
> >> >>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
> >> >>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
> >> >>
> >> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> >> ---
> >> >>  net/net.c        | 3 +++
> >> >>  qapi-schema.json | 1 +
> >> >>  2 files changed, 4 insertions(+)
> >> >>
> >> >>  [Note: I'm quite new to this area of the code, please review carefully
> >> >>   whether this approach makes sense or whether it should be done differently]
> >> >
> >> > We thought the QEMU "vlan" concept would be dropped completely in the
> >> > future, so it was never added to -netdev.  No patches to do that have
> >> > been posted over the years, so I think it was more of a conceptual goal
> >> > than a concrete requirement.
> >>
> >> Well, patches to do that first need to replace the VLAN-only dump
> >> feature.
> >>
> >> To fully deprecate -net, we also have to replace -net nic for
> >> configuring onboard NICs.
> >>
> >> Prior discussion:
> >> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
> >>
> >> We haven't really tried either.
> >>
> >> Thomas's patch declares defeat on VLANs: it ports them to -netdev,
> >> perpetuating the concept.
> >>
> >> > I'm okay with merging this but will leave some time for discussion
> >> > before merging the patch.
> >> >
> >> > "Speak now or forever hold your peace"
> >>
> >> Sure we're ready to give up without having tried?
> >
> > Seems to me that we'd be better off taking the -netdev arg and
> > adding a new parameter 'dump=filename' that can be used with any
> > netdev backend. eg with tap
> >
> >    -netdev tap,id=foo,fd=3,dump=/somefile,dumplen=1024
> >
> > The use of a dedicated 'dump' network backend always appear rather
> > strange as a conceptual approach, and I don't think we should
> > propagate its use into -netdev
> 
> I'm happy with this approach too.

I'm certainly fine with both two approaches - as long as the -net vs.
-netdev chaos finally goes away.

However, there is one thing to consider: The "vlans" currently emulate
some kind of hub hardware. If we do not support these with -netdev and
finally remove -net one day, QEMU looses this feature, so that could be
considered as a regression. Question is how useful this feature was
beside connecting it to the dump device, and whether anybody really
used it... for example, when running QEMU inside QEMU with nested
virtualization, could the outer QEMU emulate a hub this way to connect
the two contained virtual machines? OTOH, I think such behavior can
also be reached by different means, so it should be ok to loose this
feature, shouldn't it?

 Thomas
Paolo Bonzini May 26, 2015, 6:25 p.m. UTC | #8
On 26/05/2015 15:07, Thomas Huth wrote:
> Certainly no objections from my side to keep this "vlan" concept in
> QEMU, but thinking about this option again, I'd rather call it "hub=xx"
> for the -netdev option instead of "vlan=xx", since this better matches
> the "-netdev hubport" naming and it IMHO also describes the
> functionality in a better way.
> (of course I also need to add proper updates to the documentation files
> in my patch in that way)

I agree.  It makes sense, because vlans have other uses besides dump
(e.g. connecting VMs via the socket or l2tpv3 backends, where VMs have
multiple NICs).

Paolo
Markus Armbruster May 27, 2015, 7:22 a.m. UTC | #9
Thomas Huth <thuth@redhat.com> writes:

> On Tue, 26 May 2015 17:43:09 +0100
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
>> On Tue, May 26, 2015 at 3:36 PM, Daniel P. Berrange
>> <berrange@redhat.com> wrote:
>> > On Tue, May 26, 2015 at 04:29:51PM +0200, Markus Armbruster wrote:
>> >> Stefan Hajnoczi <stefanha@redhat.com> writes:
>> >>
>> >> > On Fri, May 22, 2015 at 09:22:19PM +0200, Thomas Huth wrote:
>> >> >> So far, it is not possible to use the network dump interface
>> >> >> with the "-netdev" option yet, it only works with the legacy
>> >> >> "-net" option. To be able to use it with "-netdev", too, this
>> >> >> patch now enables the "vlan" packet distribution for the
>> >> >> "-netdev" option, too, so that you can now dump network packets
>> >> >> like this:
>> >> >>
>> >> >> qemu... -device e1000,netdev=myhub -netdev hubport,hubid=1,id=myhub \
>> >> >>         -netdev user,id=n1,vlan=1,tftp=/tmp/tftp,bootfile=zImage \
>> >> >>         -netdev dump,id=n2,vlan=1,file=/tmp/dump.dat
>> >> >>
>> >> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> >> >> ---
>> >> >>  net/net.c        | 3 +++
>> >> >>  qapi-schema.json | 1 +
>> >> >>  2 files changed, 4 insertions(+)
>> >> >>
>> >> >>  [Note: I'm quite new to this area of the code, please review carefully
>> >> >>   whether this approach makes sense or whether it should be done differently]
>> >> >
>> >> > We thought the QEMU "vlan" concept would be dropped completely in the
>> >> > future, so it was never added to -netdev.  No patches to do that have
>> >> > been posted over the years, so I think it was more of a conceptual goal
>> >> > than a concrete requirement.
>> >>
>> >> Well, patches to do that first need to replace the VLAN-only dump
>> >> feature.
>> >>
>> >> To fully deprecate -net, we also have to replace -net nic for
>> >> configuring onboard NICs.
>> >>
>> >> Prior discussion:
>> >> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
>> >>
>> >> We haven't really tried either.
>> >>
>> >> Thomas's patch declares defeat on VLANs: it ports them to -netdev,
>> >> perpetuating the concept.
>> >>
>> >> > I'm okay with merging this but will leave some time for discussion
>> >> > before merging the patch.
>> >> >
>> >> > "Speak now or forever hold your peace"
>> >>
>> >> Sure we're ready to give up without having tried?
>> >
>> > Seems to me that we'd be better off taking the -netdev arg and
>> > adding a new parameter 'dump=filename' that can be used with any
>> > netdev backend. eg with tap
>> >
>> >    -netdev tap,id=foo,fd=3,dump=/somefile,dumplen=1024
>> >
>> > The use of a dedicated 'dump' network backend always appear rather
>> > strange as a conceptual approach, and I don't think we should
>> > propagate its use into -netdev
>> 
>> I'm happy with this approach too.
>
> I'm certainly fine with both two approaches - as long as the -net vs.
> -netdev chaos finally goes away.
>
> However, there is one thing to consider: The "vlans" currently emulate
> some kind of hub hardware. If we do not support these with -netdev and
> finally remove -net one day, QEMU looses this feature, so that could be
> considered as a regression. Question is how useful this feature was
> beside connecting it to the dump device, and whether anybody really
> used it... for example, when running QEMU inside QEMU with nested
> virtualization, could the outer QEMU emulate a hub this way to connect
> the two contained virtual machines? OTOH, I think such behavior can
> also be reached by different means, so it should be ok to loose this
> feature, shouldn't it?

I wouldn't mind losing the "VLAN" / hub feature.  But if there's a
compelling use for it, I wouldn't mind keeping it, as long as it's out
of the way of users who don't need this niche feature.

So my goal isn't so much "get rid of hubs", it's "deprecate -net", and
"avoid dragging in a hub just for dumping packets".

I want us to deprecate -net, because:

* It forces a hub, which isn't what you want in the common case.

  Related: the default configuration gives you a hub, which is
  sub-optimal for most users.  Needs fixing.  The easiest way to
  configure networking also results in a hub.  Needs fixing, too.

* It's conceptually redundant, thanks to Stefan's hubport work.
  Unfortunately, it still dominates the user interface.

  -net boils down to a network device connected to a hubport, but our
  user interface doesn't let you do the boiling.  It does for -device's
  vlan parameter: you can always replace it by a netdev parameter
  referring to a suitable hubport created with -netdev.  I want the same
  for -net.

* -net nic adds another complication.  Like -serial, -drive if=ide and
  other similarly old options, it doesn't actually do anything, it
  merely sends a message to the board code, which may do with it
  whatever it wants.  Different boards to different things.  Separate
  rabit hole, better avoid it in this thread.

Thomas's patch brings half of what we have in -device to -netdev.
Unfortunately, it's the magic half: the vlan=N parameter to connect to
the hub with hubid=N, magically creating it if it doesn't exist already.
It doesn't bring the non-magic netdev=ID part to connect to an existing
hubport with id=ID.  That's the half I want if we can't get rid of hubs
outright.

-device needs the magic half for backward compatibility.  -netdev
doesn't.  An argument could be made to want it for consistency with
-device.  As far as I'm concerned: no thanks.

Thomas's patch does not address -net nic issues, and that's just fine.

Thomas's patch does not address "avoid dragging in a hub just for
dumping packets".  Dan sketched a way to do that.  It could make sense
even if we decide we want to keep hubs around for other, complicated
applications.
Thomas Huth July 17, 2015, 6:53 a.m. UTC | #10
On 05/26/2015 04:29 PM, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
[...]
>> We thought the QEMU "vlan" concept would be dropped completely in the
>> future, so it was never added to -netdev.  No patches to do that have
>> been posted over the years, so I think it was more of a conceptual goal
>> than a concrete requirement.
> 
> Well, patches to do that first need to replace the VLAN-only dump
> feature.
> 
> To fully deprecate -net, we also have to replace -net nic for
> configuring onboard NICs.
> 
> Prior discussion:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
> 
> We haven't really tried either.

Ok, assuming that my "Network traffic dumping for -netdev devices" patch
series is going to solve the dumping-for-netdev problem, how do we
tackle the remaining problems that we have to solve before we can
deprecate -net? Does anybody have a survey of the (onboard) NICs that
can only be configured with -net but not with -device? Could they
nowadays be changed to work with -device, too, or are there still major
obstacles to solve first?

 Thomas
Peter Maydell July 17, 2015, 7:25 a.m. UTC | #11
On 17 July 2015 at 07:53, Thomas Huth <thuth@redhat.com> wrote:
> Ok, assuming that my "Network traffic dumping for -netdev devices" patch
> series is going to solve the dumping-for-netdev problem, how do we
> tackle the remaining problems that we have to solve before we can
> deprecate -net? Does anybody have a survey of the (onboard) NICs that
> can only be configured with -net but not with -device? Could they
> nowadays be changed to work with -device, too, or are there still major
> obstacles to solve first?

The problem is that "-device" says "create a new device and
configure it like this". But onboard NICs are created by
the board, so we want let the user say how to configure
those devices, not create new ones...

-- PMM
Thomas Huth July 17, 2015, 7:48 a.m. UTC | #12
On 07/17/2015 09:25 AM, Peter Maydell wrote:
> On 17 July 2015 at 07:53, Thomas Huth <thuth@redhat.com> wrote:
>> Ok, assuming that my "Network traffic dumping for -netdev devices" patch
>> series is going to solve the dumping-for-netdev problem, how do we
>> tackle the remaining problems that we have to solve before we can
>> deprecate -net? Does anybody have a survey of the (onboard) NICs that
>> can only be configured with -net but not with -device? Could they
>> nowadays be changed to work with -device, too, or are there still major
>> obstacles to solve first?
> 
> The problem is that "-device" says "create a new device and
> configure it like this". But onboard NICs are created by
> the board, so we want let the user say how to configure
> those devices, not create new ones...

Ok, I see ... maybe it makes sense to simply keep "-net nic" to be able
to configure the default/onboard NIC, and only to remove all the other
-net options instead ("-net user" etc.). The disliked vlan/hub concept
could then be removed, too, since "-net nic" can be used together with
"-netdev" nowadays by using something like "-net nic,netdev=xxx" as far
as I know. That would clean up most points of confusion, I think, and
would not cause too much code churn for the onboard NICs. Does that
sound feasible?

 Thomas
Stefan Hajnoczi July 17, 2015, 8:16 a.m. UTC | #13
On Fri, Jul 17, 2015 at 08:53:08AM +0200, Thomas Huth wrote:
> On 05/26/2015 04:29 PM, Markus Armbruster wrote:
> > Stefan Hajnoczi <stefanha@redhat.com> writes:
> [...]
> >> We thought the QEMU "vlan" concept would be dropped completely in the
> >> future, so it was never added to -netdev.  No patches to do that have
> >> been posted over the years, so I think it was more of a conceptual goal
> >> than a concrete requirement.
> > 
> > Well, patches to do that first need to replace the VLAN-only dump
> > feature.
> > 
> > To fully deprecate -net, we also have to replace -net nic for
> > configuring onboard NICs.
> > 
> > Prior discussion:
> > http://lists.nongnu.org/archive/html/qemu-devel/2013-02/msg03743.html
> > 
> > We haven't really tried either.
> 
> Ok, assuming that my "Network traffic dumping for -netdev devices" patch
> series is going to solve the dumping-for-netdev problem, how do we
> tackle the remaining problems that we have to solve before we can
> deprecate -net? Does anybody have a survey of the (onboard) NICs that
> can only be configured with -net but not with -device? Could they
> nowadays be changed to work with -device, too, or are there still major
> obstacles to solve first?

Take a look at nd_table[] and nb_nics.  That's the array of -net nic
devices.  The boards look into the array to grab NICs.

The default NIC is set in net_init_clients() as a "-net nic" option.

The weird thing about "-net nic" is that the device is not created in
net/net.c by the "-net nic" command-line option.  It just adds the
information to the nd_table[] array.

It's the board that has to instantiate nd_table[] entries.  For example,
pc_nic_init() adds devices for x86 guests.

That's about all I know or have thought about so far.  It would be nice
to get rid of -net but it will take some work and is a QEMU 3.0 feature
since it breaks backwards compatibility.
Markus Armbruster July 22, 2015, 1:40 p.m. UTC | #14
Copying Andreas just in case.

Thomas Huth <thuth@redhat.com> writes:

> On 07/17/2015 09:25 AM, Peter Maydell wrote:
>> On 17 July 2015 at 07:53, Thomas Huth <thuth@redhat.com> wrote:
>>> Ok, assuming that my "Network traffic dumping for -netdev devices" patch
>>> series is going to solve the dumping-for-netdev problem, how do we
>>> tackle the remaining problems that we have to solve before we can
>>> deprecate -net? Does anybody have a survey of the (onboard) NICs that
>>> can only be configured with -net but not with -device? Could they
>>> nowadays be changed to work with -device, too, or are there still major
>>> obstacles to solve first?
>> 
>> The problem is that "-device" says "create a new device and
>> configure it like this". But onboard NICs are created by
>> the board, so we want let the user say how to configure
>> those devices, not create new ones...

The more general problem is lack of a uniform way to configure onboard
devices.

We have a bunch of ways to configure onboard devices: -net nic, -serial,
-parallel, -drive, ...  These all deposit configuration requests in
well-known places for the board code to pick up.  A request can apply

(a) to a mandatory onboard device, modifying its configuration, or

(b) to an optional onboard device, triggering its creation, or

(c) to nothing in particular.

It all depends on the board code.

For qdevified devices, you can replace (b) with -device, but not (a), as
Peter points out.

To likewise replace (a), we'd need means to change an *existing*
device's properties.  Complication: how to address the device.  Onboard
devices don't have a qdev ID...  QOM path?

Aside: you can sometimes use -global to replace (a), but it's not
general, because -global applies to all devices of a certain type, not
just the one you're actually targeting.

> Ok, I see ... maybe it makes sense to simply keep "-net nic" to be able
> to configure the default/onboard NIC, and only to remove all the other
> -net options instead ("-net user" etc.). The disliked vlan/hub concept
> could then be removed, too, since "-net nic" can be used together with
> "-netdev" nowadays by using something like "-net nic,netdev=xxx" as far
> as I know. That would clean up most points of confusion, I think, and
> would not cause too much code churn for the onboard NICs. Does that
> sound feasible?

Deprecating -net except for -net nic sounds like a fine step forward to
me.
Michael S. Tsirkin July 22, 2015, 4:20 p.m. UTC | #15
On Wed, Jul 22, 2015 at 03:40:55PM +0200, Markus Armbruster wrote:
> Copying Andreas just in case.
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
> > On 07/17/2015 09:25 AM, Peter Maydell wrote:
> >> On 17 July 2015 at 07:53, Thomas Huth <thuth@redhat.com> wrote:
> >>> Ok, assuming that my "Network traffic dumping for -netdev devices" patch
> >>> series is going to solve the dumping-for-netdev problem, how do we
> >>> tackle the remaining problems that we have to solve before we can
> >>> deprecate -net? Does anybody have a survey of the (onboard) NICs that
> >>> can only be configured with -net but not with -device? Could they
> >>> nowadays be changed to work with -device, too, or are there still major
> >>> obstacles to solve first?
> >> 
> >> The problem is that "-device" says "create a new device and
> >> configure it like this". But onboard NICs are created by
> >> the board, so we want let the user say how to configure
> >> those devices, not create new ones...
> 
> The more general problem is lack of a uniform way to configure onboard
> devices.
> 
> We have a bunch of ways to configure onboard devices: -net nic, -serial,
> -parallel, -drive, ...  These all deposit configuration requests in
> well-known places for the board code to pick up.  A request can apply
> 
> (a) to a mandatory onboard device, modifying its configuration, or
> 
> (b) to an optional onboard device, triggering its creation, or
> 
> (c) to nothing in particular.
> 
> It all depends on the board code.
> 
> For qdevified devices, you can replace (b) with -device, but not (a), as
> Peter points out.
> 
> To likewise replace (a), we'd need means to change an *existing*
> device's properties.  Complication: how to address the device.  Onboard
> devices don't have a qdev ID...  QOM path?
> 
> Aside: you can sometimes use -global to replace (a), but it's not
> general, because -global applies to all devices of a certain type, not
> just the one you're actually targeting.
> 
> > Ok, I see ... maybe it makes sense to simply keep "-net nic" to be able
> > to configure the default/onboard NIC, and only to remove all the other
> > -net options instead ("-net user" etc.). The disliked vlan/hub concept
> > could then be removed, too, since "-net nic" can be used together with
> > "-netdev" nowadays by using something like "-net nic,netdev=xxx" as far
> > as I know. That would clean up most points of confusion, I think, and
> > would not cause too much code churn for the onboard NICs. Does that
> > sound feasible?
> 
> Deprecating -net except for -net nic sounds like a fine step forward to
> me.

-net dump is also useful, we'll need some solution for that if we
want to deprecate vlans.
Thomas Huth July 22, 2015, 4:45 p.m. UTC | #16
On 22/07/15 18:20, Michael S. Tsirkin wrote:
> On Wed, Jul 22, 2015 at 03:40:55PM +0200, Markus Armbruster wrote:
...
>> Deprecating -net except for -net nic sounds like a fine step forward to
>> me.
> 
> -net dump is also useful, we'll need some solution for that if we
> want to deprecate vlans.

*cough* see my patch series at:
  http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg02771.html

 Thomas
diff mbox

Patch

diff --git a/net/net.c b/net/net.c
index 7427f6a..3595910 100644
--- a/net/net.c
+++ b/net/net.c
@@ -872,6 +872,7 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
 #ifdef CONFIG_L2TPV3
         case NET_CLIENT_OPTIONS_KIND_L2TPV3:
 #endif
+        case NET_CLIENT_OPTIONS_KIND_DUMP:
             break;
 
         default:
@@ -895,6 +896,8 @@  static int net_client_init1(const void *object, int is_netdev, Error **errp)
             (opts->kind != NET_CLIENT_OPTIONS_KIND_NIC ||
              !opts->nic->has_netdev)) {
             peer = net_hub_add_port(u.net->has_vlan ? u.net->vlan : 0, NULL);
+        } else if (is_netdev && u.netdev->has_vlan) {
+            peer = net_hub_add_port(u.netdev->vlan, NULL);
         }
 
         if (net_client_init_fun[opts->kind](opts, name, peer) < 0) {
diff --git a/qapi-schema.json b/qapi-schema.json
index f97ffa1..adb0722 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2512,6 +2512,7 @@ 
 { 'struct': 'Netdev',
   'data': {
     'id':   'str',
+    '*vlan': 'int32',
     'opts': 'NetClientOptions' } }
 
 ##