[RTnet-developers] race condition in e1000_xmit_frame ?

[<fre...@tr...>]

Hallo,

Is it possible that there is a a race condition in the e1000 driver of
rtnet ?

At the end of the e1000_xmit_frame function in e1000_main.c, there is the
following code :

     rtdm_lock_put_irqrestore(&tx_ring->tx_lock, context);
     first = tx_ring->next_to_use;

     if (likely(e1000_tx_csum(adapter, tx_ring, skb)))
       tx_flags |= E1000_TX_FLAGS_CSUM;

     e1000_tx_queue(adapter, tx_ring, tx_flags,
                    e1000_tx_map(adapter, tx_ring, skb, first,
                                 max_per_txd, nr_frags, mss),
                    skb->xmit_stamp);

First, the tx_ring->tx_lock is released, then the tx_ring->next_to_use
variable is incremented at the end of the e1000_tx_queue. This means that
if there is a context switch in between, 2 processes are using the same
tx_ring->buffer, which means that one of the rtskb's is not freed. This is
what I noticed after adding some logging in the driver.

According to me, the rtdm_lock_put_irqrestore should be done after the
e1000_tx_queue. This is also what happens on in the e1000 driver in the
linux source tree :

     e1000_tx_queue(adapter, tx_ring, tx_flags,
                    e1000_tx_map(adapter, tx_ring, skb, first,
                                 max_per_txd, nr_frags, mss));

     netdev->trans_start = jiffies;

     /* Make sure there is space in the ring for the next send. */
     e1000_maybe_stop_tx(netdev, tx_ring, MAX_SKB_FRAGS + 2);

     spin_unlock_irqrestore(&tx_ring->tx_lock, flags);

Can the responsible for the rtnet e1000 driver give some feedback about
this ?

Frederik