Message ID | 1461436546.7627.45.camel@edumazet-glaptop3.roam.corp.google.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Sat, Apr 23, 2016 at 9:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > When multiple skb are TX-completed in a row, we might incorrectly keep > a timestamp of a prior skb and cause extra work. > > Fixes: ec693d47010e8 ("net/mlx4_en: Add HW timestamping (TS) support") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Willem de Bruijn <willemb@google.com> Eric, was that supposed to be Reported-by: or Signed-off-by: ... FWIW, Dave, I wanted to use the opportunity and comment that this week many of us gonna be on Passover vacation, so response rate should be lower than usual > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c > index c0d7b7296236..a386f047c1af 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c > +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c > @@ -405,7 +405,6 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev, > u32 packets = 0; > u32 bytes = 0; > int factor = priv->cqe_factor; > - u64 timestamp = 0; > int done = 0; > int budget = priv->tx_work_limit; > u32 last_nr_txbb; > @@ -445,9 +444,12 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev, > new_index = be16_to_cpu(cqe->wqe_index) & size_mask; > > do { > + u64 timestamp = 0; > + > txbbs_skipped += last_nr_txbb; > ring_index = (ring_index + last_nr_txbb) & size_mask; > - if (ring->tx_info[ring_index].ts_requested) > + > + if (unlikely(ring->tx_info[ring_index].ts_requested)) > timestamp = mlx4_en_get_cqe_ts(cqe); > > /* free next descriptor */ > >
On Sat, 2016-04-23 at 23:23 +0300, Or Gerlitz wrote: > On Sat, Apr 23, 2016 at 9:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > When multiple skb are TX-completed in a row, we might incorrectly keep > > a timestamp of a prior skb and cause extra work. > > > > Fixes: ec693d47010e8 ("net/mlx4_en: Add HW timestamping (TS) support") > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > Willem de Bruijn <willemb@google.com> > > Eric, was that supposed to be Reported-by: or Signed-off-by: ... This was meant to be. Cc: Willem de Bruijn <willemb@google.com> Willem was CC on the original bug at Google (Google-Bug-Id: 28356763 ) Thanks.
On Sat, Apr 23, 2016 at 9:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > From: Eric Dumazet <edumazet@google.com> > > When multiple skb are TX-completed in a row, we might incorrectly keep > a timestamp of a prior skb and cause extra work. > > Fixes: ec693d47010e8 ("net/mlx4_en: Add HW timestamping (TS) support") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Willem de Bruijn <willemb@google.com> Other than the commit message issue raised by Or, Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Sat, 23 Apr 2016 11:35:46 -0700 > From: Eric Dumazet <edumazet@google.com> > > When multiple skb are TX-completed in a row, we might incorrectly keep > a timestamp of a prior skb and cause extra work. > > Fixes: ec693d47010e8 ("net/mlx4_en: Add HW timestamping (TS) support") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Willem de Bruijn <willemb@google.com> Applied and queued up for -stable.
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c index c0d7b7296236..a386f047c1af 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c @@ -405,7 +405,6 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev, u32 packets = 0; u32 bytes = 0; int factor = priv->cqe_factor; - u64 timestamp = 0; int done = 0; int budget = priv->tx_work_limit; u32 last_nr_txbb; @@ -445,9 +444,12 @@ static bool mlx4_en_process_tx_cq(struct net_device *dev, new_index = be16_to_cpu(cqe->wqe_index) & size_mask; do { + u64 timestamp = 0; + txbbs_skipped += last_nr_txbb; ring_index = (ring_index + last_nr_txbb) & size_mask; - if (ring->tx_info[ring_index].ts_requested) + + if (unlikely(ring->tx_info[ring_index].ts_requested)) timestamp = mlx4_en_get_cqe_ts(cqe); /* free next descriptor */