netpoll and the bonding driver

netpoll and the bonding driver

[Jeff Moyer]

Hi,

I'm trying to implement a netpoll hook for the bonding driver.  In doing
so, I ran into the following problem:

netpoll_send_skb calls the device's hard_start_xmit routine.  In this case,
it will be one of the bonding driver's xmit routines.  Each of these ends
up calling bond_dev_queue_xmit, which in turn calls dev_queue_xmit.

Now, for netconsole, the code disables interrupts before calling
netpoll_send_udp:

        local_irq_save(flags);

        for(left = len; left; ) {
                frag = min(left, MAX_PRINT_CHUNK);
                netpoll_send_udp(&np, msg, frag);
                msg += frag;
                left -= frag;
        }

        local_irq_restore(flags);

Note that if you did an alt-sysrq-t, then you would enter this code path in
interrupt context as well, and herein lies the problem.  It seems that
dev_queue_xmit is not supposed to be called with interrupts disabled.  The
immediate affect of this is that the WARN_ON in local_bh_enable triggers
(called at the end of dev_queue_xmit), causing us to loop infinitely
printing out stack traces.

So, my question is this: how in the world do we fit the bonding driver into
the generic netpoll infrastructure?  In the case of every other driver,
netpoll simply calls the hard_start_xmit routine[1], and this approach
simply doesn't work for the bonding driver, for the reasons I described
above.  So, one way to make the bonding driver fit into this model is to
modify it to not call dev_queue_xmit when called from netpoll.  This can be
done, I suppose, by adding another start_xmit routine that is specific to
netpoll.  This doesn't feel good to me, but I'm not sure how else you would
solve the problem (and netpoll already gets its own poll interface, so is
one more all that bad?).  The other approach to take is to put bonding
specific logic into netpoll.  I think we can all agree that is a bad idea.

-Jeff

[1] Note that netpoll does not perform any of the checks that dev_queue_xmit
    does.  This either means that a) in the netpoll case, this is an okay
    thing to do (since it's been working for this long), or b) netpoll has a 
    bug.

Re: netpoll and the bonding driver

[Matt Mackall]

On Fri, Jun 17, 2005 at 03:56:35PM -0400, Jeff Moyer wrote:
> Hi,
> 
> I'm trying to implement a netpoll hook for the bonding driver.

My first question would be: does this really make sense to do? Why not
just bind netpoll to one of the underlying devices?

-- 
Mathematics is the supreme nostalgia of our time.

Re: netpoll and the bonding driver

[John W. Linville]

On Sun, Jun 19, 2005 at 11:14:36AM -0700, Matt Mackall wrote:
> On Fri, Jun 17, 2005 at 03:56:35PM -0400, Jeff Moyer wrote:

> > I'm trying to implement a netpoll hook for the bonding driver.
> 
> My first question would be: does this really make sense to do? Why not
> just bind netpoll to one of the underlying devices?

Depending on the bonding mode, this would be very unlikely to work.
The other side of the link will still be expecting to talk to the
bond rather than to an individual link.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: netpoll and the bonding driver

[Jeff Moyer]

==> Regarding Re: netpoll and the bonding driver; "John W. Linville" <linville@tuxdriver.com> adds:

linville> On Sun, Jun 19, 2005 at 11:14:36AM -0700, Matt Mackall wrote:
>> On Fri, Jun 17, 2005 at 03:56:35PM -0400, Jeff Moyer wrote:

>> > I'm trying to implement a netpoll hook for the bonding driver.
>> 
>> My first question would be: does this really make sense to do? Why not
>> just bind netpoll to one of the underlying devices?

linville> Depending on the bonding mode, this would be very unlikely to
linville> work.  The other side of the link will still be expecting to talk
linville> to the bond rather than to an individual link.

Right, and for those drivers which register a netpoll_rx routine, they may
not get all of the packets destined for them.

-Jeff