[PATCH] Add support for netconsole driver used on bridge device with VIF attached

[PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Yuval Shaia]

When starting a VM which has virtual interface attached to the same bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is used for netconsole the
following message appears (after about 60 seconds) and VM creation operation fails.
     Error: Device 0 (vif) could not be connected. Hotplug scripts not working.

Note:
When trying to do the opposite, i.e. first create VM and then run
netconsole we got the error #524 - vif2.0 doesn't support polling,
aborting.

Here is my network setup:
----------                              ----------
| VM A1  |------vif1.0----->|           | Host B |
|--------|                  |--xenbr0   |        |
| Host A |--bond0 (eth0)--->|           |        |
----------          |                   ----------
                    |                        |
                    V                        V
--------------------------------------------------
|                      LAN                       |
--------------------------------------------------

I'm using netconsole to capture logs from  Host-A and send them to Host-B.
Host-A and Host-B are separate hosts (running XEN) which are  connected to the 
same LAN.
src-ip is Host-A address.
dst-ip/mac is Host-B address.
netconsole parameters: netconsole=1111@src-ip/xenbr0,2002@dst-ip/dst-mac

As i see it, netconsole driver requires ndo_poll_controller from the device's controlling driver (function __netpoll_setup in net/core/netpoll.c), a thing that is not supported currently in xen_netback driver which is the driver that runs on dom0 and serve VM's virtual interface.
call flow: init_netconsole() in netconsole.c -> alloc_param_target() -> netpoll_setup() 
in netpoll.c -> __netpoll_setup() which check if ndo_poll_controller()
Per Ian,
Are you sure this is being called for the VIF interface? In your
configuration I'd expect it to be called on the bridge not the vif, or
at least for calling on the VIF to not impact whether netpool was
enabled for the bridge or not.

I think the underlying issue which you are seeing is that
br_netpoll_setup() requires that all members of the bridge support
netpoll before allowing netpoll to be enabled on the bridge itself. 

This seems like an odd restriction in the bridge driver since in
principal only the port over which the netpoll traffic will be going
will need netpoll, but perhaps the bridge can't tell which port that is
or is going to be? I think it is worth discussing this with the bridge
maintainers (who I have CC'd, threads starts at
http://marc.info/?l=linux-netdev&m=135878868112700&w=2)

Hopefully the bridge isn't flooding/broadcasting netpoll to all ports,
at least in the case where DST IP and MAC have been specified. That
would be rather inefficient, especially when most ports go to virtual
machines.

So before I ack this patch I'd like to hear back from the bridge
maintainers about whether the current behaviour in the bridge is
intended and whether it could be fixed in some better way than adding
netpoll to netback.

AFAICT the only reason to actually support netpoll in netback would be
if you wanted host logs to go to a listener running in a domain on the
same host, which sounds like a mad idea to me! If someone actually has a
real need for that use case and can test that it works I'd be happy to
reconsider this patch on that basis (assuming the necessary #ifdefs are
added as mentioned before).

Per Ian,
I can see why the *bridge* device might need an ndo_poll_controller hook
in this setup but I can't see any reason why the netback device would
need one.
Reply:
Please note that without netback driver netconsole runs fine, i.e before trying 
to create VM which attached to the bridge.

The following patch (to latest kernel) fix this bug by adding implementation to ndo_poll_controller.

0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
0 2001
From: Yuval <yuval.shaia@oracle.com>
Date: Tue, 8 Jan 2013 10:08:45 +0200
Subject: [PATCH] Add support for netconsole driver used on bridge device with
 VIF attached

Signed-off-by: Yuval <yuval.shaia@oracle.com>
---
 drivers/net/xen-netback/interface.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/xen-netback/interface.c \
b/drivers/net/xen-netback/interface.c index 601ae2a..10751f5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device *dev, u32 \
features)  return features;
 }
 
+static void xenvif_poll_controller(struct net_device *dev)
+{
+	disable_irq(dev->irq);
+	xenvif_interrupt(dev->irq, dev);
+	enable_irq(dev->irq);
+}
+
 static const struct xenvif_stat {
 	char name[ETH_GSTRING_LEN];
 	u16 offset;
@@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
 	.ndo_stop	= xenvif_close,
 	.ndo_change_mtu	= xenvif_change_mtu,
 	.ndo_fix_features = xenvif_fix_features,
+	.ndo_poll_controller = xenvif_poll_controller,
 };
 
 struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
-- 
1.7.9.5

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Wed, 2013-05-01 at 12:55 +0100, Yuval Shaia wrote:
[... snip regurgitation of the thread...]
> 0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
> 0 2001
> From: Yuval <yuval.shaia@oracle.com>
> Date: Tue, 8 Jan 2013 10:08:45 +0200
> Subject: [PATCH] Add support for netconsole driver used on bridge device with
>  VIF attached

Sorry, but this is not what I was asking for.

Please submit with a coherent changelog based on (i.e. digested from)
the previous discussion which explains why this change is necessary
including the background of why it is being made in this way and the
interaction with the bridging layer. I wasn't asking you to just cut and
paste that discussion and prepend it to the commit like that.

The key point is that we don't think that doing netconsole from dom0 to
a domU on the same host is a useful configuration or something which is
especially desirable to support but that because of how the bridge
handles netconsole netback needs to expose this hook in order that
netconsole can be enabled via a physical device on the same bridge to a
netserver elsewhere.

> 
> Signed-off-by: Yuval <yuval.shaia@oracle.com>
> ---
>  drivers/net/xen-netback/interface.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/xen-netback/interface.c \
> b/drivers/net/xen-netback/interface.c index 601ae2a..10751f5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device *dev, u32 \
> features)  return features;
>  }
>  
> +static void xenvif_poll_controller(struct net_device *dev)
> +{
> +	disable_irq(dev->irq);
> +	xenvif_interrupt(dev->irq, dev);
> +	enable_irq(dev->irq);
> +}
> +
>  static const struct xenvif_stat {
>  	char name[ETH_GSTRING_LEN];
>  	u16 offset;
> @@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
>  	.ndo_stop	= xenvif_close,
>  	.ndo_change_mtu	= xenvif_change_mtu,
>  	.ndo_fix_features = xenvif_fix_features,
> +	.ndo_poll_controller = xenvif_poll_controller,
>  };
>  
>  struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,

Re: [Xen-devel] [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Konrad Rzeszutek Wilk]

On Fri, May 03, 2013 at 10:11:10AM +0100, Ian Campbell wrote:
> On Wed, 2013-05-01 at 12:55 +0100, Yuval Shaia wrote:
> [... snip regurgitation of the thread...]
> > 0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
> > 0 2001
> > From: Yuval <yuval.shaia@oracle.com>
> > Date: Tue, 8 Jan 2013 10:08:45 +0200
> > Subject: [PATCH] Add support for netconsole driver used on bridge device with
> >  VIF attached
> 
> Sorry, but this is not what I was asking for.
> 
> Please submit with a coherent changelog based on (i.e. digested from)
> the previous discussion which explains why this change is necessary
> including the background of why it is being made in this way and the
> interaction with the bridging layer. I wasn't asking you to just cut and
> paste that discussion and prepend it to the commit like that.
> 
> The key point is that we don't think that doing netconsole from dom0 to
> a domU on the same host is a useful configuration or something which is
> especially desirable to support but that because of how the bridge
> handles netconsole netback needs to expose this hook in order that
> netconsole can be enabled via a physical device on the same bridge to a
> netserver elsewhere.

I would have thought that doing netconsole on a domU would be a worthwile
attempt - especially to troubleshoot a guest?

> 
> > 
> > Signed-off-by: Yuval <yuval.shaia@oracle.com>
> > ---
> >  drivers/net/xen-netback/interface.c |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/xen-netback/interface.c \
> > b/drivers/net/xen-netback/interface.c index 601ae2a..10751f5 100644
> > --- a/drivers/net/xen-netback/interface.c
> > +++ b/drivers/net/xen-netback/interface.c
> > @@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device *dev, u32 \
> > features)  return features;
> >  }
> >  
> > +static void xenvif_poll_controller(struct net_device *dev)
> > +{
> > +	disable_irq(dev->irq);
> > +	xenvif_interrupt(dev->irq, dev);
> > +	enable_irq(dev->irq);
> > +}
> > +
> >  static const struct xenvif_stat {
> >  	char name[ETH_GSTRING_LEN];
> >  	u16 offset;
> > @@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
> >  	.ndo_stop	= xenvif_close,
> >  	.ndo_change_mtu	= xenvif_change_mtu,
> >  	.ndo_fix_features = xenvif_fix_features,
> > +	.ndo_poll_controller = xenvif_poll_controller,
> >  };
> >  
> >  struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [Xen-devel] [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Fri, 2013-05-03 at 15:36 +0100, Konrad Rzeszutek Wilk wrote:
> On Fri, May 03, 2013 at 10:11:10AM +0100, Ian Campbell wrote:
> > On Wed, 2013-05-01 at 12:55 +0100, Yuval Shaia wrote:
> > [... snip regurgitation of the thread...]
> > > 0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
> > > 0 2001
> > > From: Yuval <yuval.shaia@oracle.com>
> > > Date: Tue, 8 Jan 2013 10:08:45 +0200
> > > Subject: [PATCH] Add support for netconsole driver used on bridge device with
> > >  VIF attached
> > 
> > Sorry, but this is not what I was asking for.
> > 
> > Please submit with a coherent changelog based on (i.e. digested from)
> > the previous discussion which explains why this change is necessary
> > including the background of why it is being made in this way and the
> > interaction with the bridging layer. I wasn't asking you to just cut and
> > paste that discussion and prepend it to the commit like that.
> > 
> > The key point is that we don't think that doing netconsole from dom0 to
> > a domU on the same host is a useful configuration or something which is
> > especially desirable to support but that because of how the bridge
> > handles netconsole netback needs to expose this hook in order that
> > netconsole can be enabled via a physical device on the same bridge to a
> > netserver elsewhere.
> 
> I would have thought that doing netconsole on a domU would be a worthwile
> attempt - especially to troubleshoot a guest?

Yes. But that's not what I said, nor what this patch does.

What this patch enables is dom0 doing netconsole *to* a guest. i.e.
dom0's console going to a guest running on the same host, which is not
an especially useful thing to do.

AFAIK netfront already supports domU netconsole.

Ian.

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Mon, 2013-04-29 at 11:42 +0100, Yuval Shaia wrote:
> Can you suggest how to continue from here? as i don't see any comment on
> that issue up to now.

Please can you resend the patch with an extended changelog based on the
discussions in this thread and CC the bridge folks as well.

Ian.

> Yuval
> > 
> > AFAICT the only reason to actually support netpoll in netback would be
> > if you wanted host logs to go to a listener running in a domain on the
> > same host, which sounds like a mad idea to me! If someone actually has a
> > real need for that use case and can test that it works I'd be happy to
> > reconsider this patch on that basis (assuming the necessary #ifdefs are
> > added as mentioned before).
> > 
> > > > BTW looking at the patch as it is any use of this hook should be wrapped
> > > > in CONFIG_NET_POLL_CONTROLLER.
> > > > 
> > > > Ian.
> > > > 
> > > 
> > > 
> > 
> > 
> 
> 
> 

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Yuval Shaia]

On Tue, 2013-01-29 at 10:29 +0000, Ian Campbell wrote:
> On Tue, 2013-01-29 at 08:54 +0000, Yuval Shaia wrote:
> > > Please can you explain the exact code path which results in this new
> > > hook being called.
> > > 
> > init_netconsole() in netconsole.c -> alloc_param_target() -> netpoll_setup() 
> > in netpoll.c -> __netpoll_setup() which check if ndo_poll_controller() 
> > operation is supported.
> 
> Are you sure this is being called for the VIF interface? In your
> configuration I'd expect it to be called on the bridge not the vif, or
> at least for calling on the VIF to not impact whether netpool was
> enabled for the bridge or not.
> 
> I think the underlying issue which you are seeing is that
> br_netpoll_setup() requires that all members of the bridge support
> netpoll before allowing netpoll to be enabled on the bridge itself. 
> 
> This seems like an odd restriction in the bridge driver since in
> principal only the port over which the netpoll traffic will be going
> will need netpoll, but perhaps the bridge can't tell which port that is
> or is going to be? I think it is worth discussing this with the bridge
> maintainers (who I have CC'd, threads starts at
> http://marc.info/?l=linux-netdev&m=135878868112700&w=2)
> 
> Hopefully the bridge isn't flooding/broadcasting netpoll to all ports,
> at least in the case where DST IP and MAC have been specified. That
> would be rather inefficient, especially when most ports go to virtual
> machines.
> 
> So before I ack this patch I'd like to hear back from the bridge
> maintainers about whether the current behaviour in the bridge is
> intended and whether it could be fixed in some better way than adding
> netpoll to netback.
Ian,
Can you suggest how to continue from here? as i don't see any comment on
that issue up to now.
Yuval
> 
> AFAICT the only reason to actually support netpoll in netback would be
> if you wanted host logs to go to a listener running in a domain on the
> same host, which sounds like a mad idea to me! If someone actually has a
> real need for that use case and can test that it works I'd be happy to
> reconsider this patch on that basis (assuming the necessary #ifdefs are
> added as mentioned before).
> 
> > > BTW looking at the patch as it is any use of this hook should be wrapped
> > > in CONFIG_NET_POLL_CONTROLLER.
> > > 
> > > Ian.
> > > 
> > 
> > 
> 
> 

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Tue, 2013-01-29 at 08:54 +0000, Yuval Shaia wrote:
> > Please can you explain the exact code path which results in this new
> > hook being called.
> > 
> init_netconsole() in netconsole.c -> alloc_param_target() -> netpoll_setup() 
> in netpoll.c -> __netpoll_setup() which check if ndo_poll_controller() 
> operation is supported.

Are you sure this is being called for the VIF interface? In your
configuration I'd expect it to be called on the bridge not the vif, or
at least for calling on the VIF to not impact whether netpool was
enabled for the bridge or not.

I think the underlying issue which you are seeing is that
br_netpoll_setup() requires that all members of the bridge support
netpoll before allowing netpoll to be enabled on the bridge itself. 

This seems like an odd restriction in the bridge driver since in
principal only the port over which the netpoll traffic will be going
will need netpoll, but perhaps the bridge can't tell which port that is
or is going to be? I think it is worth discussing this with the bridge
maintainers (who I have CC'd, threads starts at
http://marc.info/?l=linux-netdev&m=135878868112700&w=2)

Hopefully the bridge isn't flooding/broadcasting netpoll to all ports,
at least in the case where DST IP and MAC have been specified. That
would be rather inefficient, especially when most ports go to virtual
machines.

So before I ack this patch I'd like to hear back from the bridge
maintainers about whether the current behaviour in the bridge is
intended and whether it could be fixed in some better way than adding
netpoll to netback.

AFAICT the only reason to actually support netpoll in netback would be
if you wanted host logs to go to a listener running in a domain on the
same host, which sounds like a mad idea to me! If someone actually has a
real need for that use case and can test that it works I'd be happy to
reconsider this patch on that basis (assuming the necessary #ifdefs are
added as mentioned before).

> > BTW looking at the patch as it is any use of this hook should be wrapped
> > in CONFIG_NET_POLL_CONTROLLER.
> > 
> > Ian.
> > 
> 
> 

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Yuval Shaia]

On Tuesday 22 January 2013 11:23:32 am Ian Campbell wrote:
> On Mon, 2013-01-21 at 18:00 +0000, yuval.shaia@oracle.com wrote:
> > On 01/21/2013 07:28 PM, Ian Campbell wrote:
> > > On Mon, 2013-01-21 at 17:17 +0000, Yuval Shaia wrote:
> > >> When starting a VM which has virtual interface attached to the same
> > >> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is
> > >> used for netconsole the
> > >> following message appears (after about 60 seconds) and VM creation
> > >> operation fails.
> > >>        Error: Device 0 (vif) could not be connected. Hotplug scripts
> > >> not working.
> > >
> > > I'm not sure how this can relate netconsole since this happens before
> > > the guest even runs, doesn't it?
> >
> > Running the same scenario without netconsole runs smoothly.
> 
> But why? I can't make sense of your scenario and therefore cannot
> explain why this patch would make any difference. Perhaps your patch is
> just papering over some other issue?
> 
> > >> As i see it, netconsole driver requires ndo_poll_controller from the
> > >> device's controlling driver (function __netpoll_setup in
> > >> net/core/netpoll.c), a thing that is not supported currently in
> > >> xen_netback driver which is the driver that runs on dom0 and serve
> > >> VM's virtual interface.
> > >
> > > Which domain is using netconsole? Is it dom0 or domU? What parameters
> > > do you give it?
> >
> > dom0.
> > parameters: netconsole=1111@src-ip/xenbr0,2002@dst-ip/dst-mac
> 
> You give these parameters to dom0 so that dom0's own logs are sent to
> some other host?
> 
> Are dst-ip and dst-mac a domain on the same host or are they some offbox
> logging service?
Sorry, my mistake, i should have attach network setup at first.
Anyway, here is my setup:
----------                       ----------
| Host A |--bond0--->|           | Host B |
|--------|           |--xenbr0   |        |
| VM A1  |--vif1.0-->|     |     |        |
----------                 |     ----------
                           |         |
                           V         V
---------------------------------------------
|                   LAN                     |
---------------------------------------------

I'm using netconsole to capture logs from  Host-A and send them to Host-B.
Host-A and Host-B are separate hosts (running XEN) which are  connected to the 
same LAN.
src-ip is Host-A address.
dst-ip/mac is Host-B address.

> 
> I can see why the *bridge* device might need an ndo_poll_controller hook
> in this setup but I can't see any reason why the netback device would
> need one.
> 
Please note that without netback driver netconsole runs fine, i.e before trying 
to create VM which attached to the bridge.
> > > I assume it is the domU but if that's the case I don't see why a
> > > netback rather than netfront patch would be required.
> > >
> > > Or is dom0 doing netconsole where the log receiver is a domU?
> >
> > Log received at dom0 as output of xm cr vm.cfg command.
> > netconsole is used on dom0 to monitor its log
> 
> Are you now saying that logging goes from domU to dom0? Which is it?
netback comes to play as soon as i'm attaching virtual interface to a bridge 
and since this bridge is under netconsole monitoring the function is 
triggered. Up to that point there is no issue getting logs in Host-B so i 
assume the problem is not with the bridge driver.
> 
> In this scenario there is really no reason I can see that netback would
> need an ndo_poll_controller hook -- that would be needed in the domU in
> the netfront driver.
> 
> Please can you explain the exact code path which results in this new
> hook being called.
> 
init_netconsole() in netconsole.c -> alloc_param_target() -> netpoll_setup() 
in netpoll.c -> __netpoll_setup() which check if ndo_poll_controller() 
operation is supported.
> BTW looking at the patch as it is any use of this hook should be wrapped
> in CONFIG_NET_POLL_CONTROLLER.
> 
> Ian.
> 

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[yuval.shaia]

[-- Attachment #1.1: Type: text/plain, Size: 3650 bytes --]


On 01/22/2013 11:23 AM, Ian Campbell wrote:
> On Mon, 2013-01-21 at 18:00 +0000, yuval.shaia@oracle.com wrote:
>> On 01/21/2013 07:28 PM, Ian Campbell wrote:
>>> On Mon, 2013-01-21 at 17:17 +0000, Yuval Shaia wrote:
>>>
>>>> When starting a VM which has virtual interface attached to the same
>>>> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is
>>>> used for netconsole the
>>>> following message appears (after about 60 seconds) and VM creation
>>>> operation fails.
>>>>         Error: Device 0 (vif) could not be connected. Hotplug scripts not
>>>> working.
>>> I'm not sure how this can relate netconsole since this happens before
>>> the guest even runs, doesn't it?
>> Running the same scenario without netconsole runs smoothly.
> But why? I can't make sense of your scenario and therefore cannot
> explain why this patch would make any difference. Perhaps your patch is
> just papering over some other issue?
>
>>>> As i see it, netconsole driver requires ndo_poll_controller from the
>>>> device's controlling driver (function __netpoll_setup in
>>>> net/core/netpoll.c), a thing that is not supported currently in
>>>> xen_netback driver which is the driver that runs on dom0 and serve
>>>> VM's virtual interface.
>>> Which domain is using netconsole? Is it dom0 or domU? What parameters do
>>> you give it?
>> dom0.
>> parameters: netconsole=1111@src-ip/xenbr0,2002@dst-ip/dst-mac
> You give these parameters to dom0 so that dom0's own logs are sent to
> some other host?
>
> Are dst-ip and dst-mac a domain on the same host or are they some offbox
> logging service?

Sorry, my mistake, i should have attach network setup at first.

Anyway, here is my setup:
---------- ----------
| Host A |--bond0 -- | Host B |
|--------| |--xenbr0 | |
| VM A1 |--vif1.0 -- | | |
---------- | ----------
| |
---------------------------------------------
| LAN |
---------------------------------------------


I'm using netconsole to capture logs from Host-A and send them to Host-B.
Host-A and Host-B are separate hosts (running XEN) which are connected 
to the same LAN.

src-ip is Host-A address.
dst-ip/mac is Host-B address.

>
> I can see why the *bridge* device might need an ndo_poll_controller hook
> in this setup but I can't see any reason why the netback device would
> need one.
Please note that without netback driver netconsole runs fine, i.e before 
trying to create VM which attached to the bridge.
>
>>> I assume it is the domU but if that's the case I don't see why a netback
>>> rather than netfront patch would be required.
>>>
>>> Or is dom0 doing netconsole where the log receiver is a domU?
>> Log received at dom0 as output of xm cr vm.cfg command.
>> netconsole is used on dom0 to monitor its log
> Are you now saying that logging goes from domU to dom0? Which is it?
>
> In this scenario there is really no reason I can see that netback would
> need an ndo_poll_controller hook -- that would be needed in the domU in
> the netfront driver.
netback comes to play as soon as i'm attaching virtual interface to a 
bridge and since this bridge is under netconsole monitoring the function 
is triggered. Up to that point there is no issue getting logs in Host-B 
so i assume the problem is not with the bridge driver.
>
> Please can you explain the exact code path which results in this new
> hook being called.
init_netconsole() in netconsole.c -> alloc_param_target() -> 
netpoll_setup() in netpoll.c -> __netpoll_setup() which check if 
ndo_poll_controller() operation is supported.
>
> BTW looking at the patch as it is any use of this hook should be wrapped
> in CONFIG_NET_POLL_CONTROLLER.
>
> Ian.
>


[-- Attachment #1.2: Type: text/html, Size: 6935 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Mon, 2013-01-21 at 18:00 +0000, yuval.shaia@oracle.com wrote:
> On 01/21/2013 07:28 PM, Ian Campbell wrote:
> > On Mon, 2013-01-21 at 17:17 +0000, Yuval Shaia wrote:
> >
> >> When starting a VM which has virtual interface attached to the same
> >> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is
> >> used for netconsole the
> >> following message appears (after about 60 seconds) and VM creation
> >> operation fails.
> >>        Error: Device 0 (vif) could not be connected. Hotplug scripts not
> >> working.
> > I'm not sure how this can relate netconsole since this happens before
> > the guest even runs, doesn't it?
> Running the same scenario without netconsole runs smoothly.

But why? I can't make sense of your scenario and therefore cannot
explain why this patch would make any difference. Perhaps your patch is
just papering over some other issue?

> >> As i see it, netconsole driver requires ndo_poll_controller from the
> >> device's controlling driver (function __netpoll_setup in
> >> net/core/netpoll.c), a thing that is not supported currently in
> >> xen_netback driver which is the driver that runs on dom0 and serve
> >> VM's virtual interface.
> > Which domain is using netconsole? Is it dom0 or domU? What parameters do
> > you give it?
> dom0.
> parameters: netconsole=1111@src-ip/xenbr0,2002@dst-ip/dst-mac

You give these parameters to dom0 so that dom0's own logs are sent to
some other host? 

Are dst-ip and dst-mac a domain on the same host or are they some offbox
logging service?

I can see why the *bridge* device might need an ndo_poll_controller hook
in this setup but I can't see any reason why the netback device would
need one.

> >
> > I assume it is the domU but if that's the case I don't see why a netback
> > rather than netfront patch would be required.
> >
> > Or is dom0 doing netconsole where the log receiver is a domU?
> Log received at dom0 as output of xm cr vm.cfg command.
> netconsole is used on dom0 to monitor its log

Are you now saying that logging goes from domU to dom0? Which is it?

In this scenario there is really no reason I can see that netback would
need an ndo_poll_controller hook -- that would be needed in the domU in
the netfront driver.

Please can you explain the exact code path which results in this new
hook being called.

BTW looking at the patch as it is any use of this hook should be wrapped
in CONFIG_NET_POLL_CONTROLLER.

Ian.

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[yuval.shaia]

On 01/21/2013 07:28 PM, Ian Campbell wrote:
> On Mon, 2013-01-21 at 17:17 +0000, Yuval Shaia wrote:
>
>> When starting a VM which has virtual interface attached to the same
>> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is
>> used for netconsole the
>> following message appears (after about 60 seconds) and VM creation
>> operation fails.
>>        Error: Device 0 (vif) could not be connected. Hotplug scripts not
>> working.
> I'm not sure how this can relate netconsole since this happens before
> the guest even runs, doesn't it?
Running the same scenario without netconsole runs smoothly.
>
>> As i see it, netconsole driver requires ndo_poll_controller from the
>> device's controlling driver (function __netpoll_setup in
>> net/core/netpoll.c), a thing that is not supported currently in
>> xen_netback driver which is the driver that runs on dom0 and serve
>> VM's virtual interface.
> Which domain is using netconsole? Is it dom0 or domU? What parameters do
> you give it?
dom0.
parameters: netconsole=1111@src-ip/xenbr0,2002@dst-ip/dst-mac
>
> I assume it is the domU but if that's the case I don't see why a netback
> rather than netfront patch would be required.
>
> Or is dom0 doing netconsole where the log receiver is a domU?
Log received at dom0 as output of xm cr vm.cfg command.
netconsole is used on dom0 to monitor its log
>
> Ian.
>

Re: [PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Ian Campbell]

On Mon, 2013-01-21 at 17:17 +0000, Yuval Shaia wrote:

> When starting a VM which has virtual interface attached to the same 
> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is 
> used for netconsole the
> following message appears (after about 60 seconds) and VM creation 
> operation fails.
>       Error: Device 0 (vif) could not be connected. Hotplug scripts not 
> working.

I'm not sure how this can relate netconsole since this happens before
the guest even runs, doesn't it?

> As i see it, netconsole driver requires ndo_poll_controller from the
> device's controlling driver (function __netpoll_setup in
> net/core/netpoll.c), a thing that is not supported currently in
> xen_netback driver which is the driver that runs on dom0 and serve
> VM's virtual interface.

Which domain is using netconsole? Is it dom0 or domU? What parameters do
you give it?

I assume it is the domU but if that's the case I don't see why a netback
rather than netfront patch would be required.

Or is dom0 doing netconsole where the log receiver is a domU?

Ian.

[PATCH] Add support for netconsole driver used on bridge device with VIF attached

[Yuval Shaia]

When starting a VM which has virtual interface attached to the same bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is used for netconsole the
following message appears (after about 60 seconds) and VM creation operation fails.
     Error: Device 0 (vif) could not be connected. Hotplug scripts not working.

Note:
When trying to do the opposite, i.e. first create VM and then run
netconsole we got the error #524 - vif2.0 doesn't support polling,
aborting.

As i see it, netconsole driver requires ndo_poll_controller from the device's controlling driver (function __netpoll_setup in net/core/netpoll.c), a thing that is not supported currently in xen_netback driver which is the driver that runs on dom0 and serve VM's virtual interface.

The following patch (to latest kernel) fix this bug by adding implementation to ndo_poll_controller.

0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
0 2001
From: Yuval <yuval.shaia@oracle.com>
Date: Tue, 8 Jan 2013 10:08:45 +0200
Subject: [PATCH] Add support for netconsole driver used on bridge device with
 VIF attached

Signed-off-by: Yuval <yuval.shaia@oracle.com>
---
 drivers/net/xen-netback/interface.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 601ae2a..10751f5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device *dev, u32 features)
 	return features;
 }
 
+static void xenvif_poll_controller(struct net_device *dev)
+{
+	disable_irq(dev->irq);
+	xenvif_interrupt(dev->irq, dev);
+	enable_irq(dev->irq);
+}
+
 static const struct xenvif_stat {
 	char name[ETH_GSTRING_LEN];
 	u16 offset;
@@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
 	.ndo_stop	= xenvif_close,
 	.ndo_change_mtu	= xenvif_change_mtu,
 	.ndo_fix_features = xenvif_fix_features,
+	.ndo_poll_controller = xenvif_poll_controller,
 };
 
 struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
-- 
1.7.9.5

Re: [PATCH] Add support for netconsole driver used on bridge device with, VIF attached

[Ian Campbell]

On Mon, 2013-01-21 at 16:34 +0000, yuval.shaia@oracle.com wrote:
> When starting a VM which has virtual interface attached to the same 
> bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is 
> used for netconsole the
> following message appears (after about 60 seconds) and VM creation 
> operation fails.
>       Error: Device 0 (vif) could not be connected. Hotplug scripts not 
> working.

I'm not sure how this can relate netconsole since this happens before
the guest even runs, doesn't it?

> Note:
> When trying to do the opposite, i.e. first create VM and then run
> netconsole we got the error #524 - vif2.0 doesn't support polling,
> aborting.
> 
> The following patch (to latest kernel) fix this bug by adding 
> implementation to ndo_poll_controller.

Surely this is needed in netfront not netback?

BT, you patch is whitespace damanged, please see
Documentation/SubmittingPatches and Documentation/email-clients.txt for
tips. Please also run checkpatch.pl.

Ian.

> 00
> 01-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
> 0 2001
> From: Yuval <yuval.shaia@oracle.com>
> Date: Tue, 8 Jan 2013 10:08:45 +0200
> Subject: [PATCH] Add support for netconsole driver used on bridge device 
> with
>   VIF attached
> 
> Signed-off-by: Yuval <yuval.shaia@oracle.com>
> ---
>   drivers/net/xen-netback/interface.c |    8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/xen-netback/interface.c 
> b/drivers/net/xen-netback/interface.c
> index 601ae2a..10751f5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device 
> *dev, u32 features)
>       return features;
>   }
> 
> +static void xenvif_poll_controller(struct net_device *dev)
> +{
> +    disable_irq(dev->irq);
> +    xenvif_interrupt(dev->irq, dev);
> +    enable_irq(dev->irq);
> +}
> +
>   static const struct xenvif_stat {
>       char name[ETH_GSTRING_LEN];
>       u16 offset;
> @@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
>       .ndo_stop    = xenvif_close,
>       .ndo_change_mtu    = xenvif_change_mtu,
>       .ndo_fix_features = xenvif_fix_features,
> +    .ndo_poll_controller = xenvif_poll_controller,
>   };
> 
>   struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,

[PATCH] Add support for netconsole driver used on bridge device with, VIF attached

[yuval.shaia]

When starting a VM which has virtual interface attached to the same 
bridge (i.e vif = [type=netfront,bridge=xenbr0'] in vm.cfg) which is 
used for netconsole the
following message appears (after about 60 seconds) and VM creation 
operation fails.
      Error: Device 0 (vif) could not be connected. Hotplug scripts not 
working.

Note:
When trying to do the opposite, i.e. first create VM and then run
netconsole we got the error #524 - vif2.0 doesn't support polling,
aborting.

The following patch (to latest kernel) fix this bug by adding 
implementation to ndo_poll_controller.

0001-Add-support-for-netconsole-driver-used-on-bridge-dev.patch
0 2001
From: Yuval <yuval.shaia@oracle.com>
Date: Tue, 8 Jan 2013 10:08:45 +0200
Subject: [PATCH] Add support for netconsole driver used on bridge device 
with
  VIF attached

Signed-off-by: Yuval <yuval.shaia@oracle.com>
---
  drivers/net/xen-netback/interface.c |    8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index 601ae2a..10751f5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -179,6 +179,13 @@ static u32 xenvif_fix_features(struct net_device 
*dev, u32 features)
      return features;
  }

+static void xenvif_poll_controller(struct net_device *dev)
+{
+    disable_irq(dev->irq);
+    xenvif_interrupt(dev->irq, dev);
+    enable_irq(dev->irq);
+}
+
  static const struct xenvif_stat {
      char name[ETH_GSTRING_LEN];
      u16 offset;
@@ -237,6 +244,7 @@ static const struct net_device_ops xenvif_netdev_ops = {
      .ndo_stop    = xenvif_close,
      .ndo_change_mtu    = xenvif_change_mtu,
      .ndo_fix_features = xenvif_fix_features,
+    .ndo_poll_controller = xenvif_poll_controller,
  };

  struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
-- 
1.7.9.5