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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.
> > 
> 
> 



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