two problems in dev/e1000/if_lem.c::lem_handle_rxtx()

[Jack Vogel]

OK, will look at this as soon as I can.

Jack


On Tue, Jan 15, 2013 at 5:23 PM, Luigi Rizzo <rizzo at iet.unipi.it> wrote:

> Hi,
> i found a couple of problems in
>         dev/e1000/if_lem.c::lem_handle_rxtx() ,
> (compare with dev/e1000/if_em.c::em_handle_que() for better understanding):
>
> 1. in if_em.c::em_handle_que(), when em_rxeof() exceeds the
>   rx_process_limit, the task is rescheduled so it can complete the work.
>   Conversely, in if_lem.c::lem_handle_rxtx() the lem_rxeof() is
>   only run once, and if there are more pending packets the only
>   chance to drain them is to receive (many) more interrupts.
>
>   This is a relatively serious problem, because the receiver has
>   a hard time recovering.
>
>   I'd like to commit a fix to this same as it is done in e1000.
>
> 2. in if_em.c::em_handle_que(), interrupts are reenabled unconditionally,
>    whereas lem_handle_rxtx() only enables them if IFF_DRV_RUNNING is set.
>
>    I cannot really tell what is the correct way here, so I'd like
>    to put a comment there unless there is a good suggestion on
>    what to do.
>
>    Accesses to the intr register are race-prone anyways
>    (disabled in fastintr, enabled in the rxtx task without
>    holding any lock, and generally accessed under EM_CORE_LOCK
>    in other places), and presumably enabling/disabling the
>    interrupts around activations of the taks is just an
>    optimization (and on a VM, it is actually a pessimization
>    due to the huge cost of VM exits).
>
> cheers
> luigi
>