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

Luigi Rizzo rizzo at iet.unipi.it
Wed Jan 16 01:31:39 UTC 2013


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


More information about the freebsd-net mailing list