Re: [PATCH RFC net-next 1/2] drivers: net: virtio_net: Add tx_timeout stats field

[Julio Faracco]

Em seg, 7 de out de 2019 Ãs 11:15, Julian Wiedmann <jwi@xxxxxxxxxxxxx> escreveu:
>
> On 06.10.19 20:45, jcfaracco@xxxxxxxxx wrote:
> > From: Julio Faracco <jcfaracco@xxxxxxxxx>
> >
> > For debug purpose of TX timeout events, a tx_timeout entry was added to
> > monitor this special case: when dev_watchdog identifies a tx_timeout and
> > throw an exception. We can both consider this event as an error, but
> > driver should report as a tx_timeout statistic.
> >
>
> Hi Julio,
> dev_watchdog() updates txq->trans_timeout, why isn't that sufficient?

Hi Julian,
Good catch! This case (this patch) it would be useful only for ethtool stats.
This is not so relevant as the method implementation itself.
But, on the other hand, I think it should be relevant if we split into
tx_timeout per queue.
Anyway, suggestions are welcome.

>
>
> > Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx>
> > Signed-off-by: Daiane Mendes <dnmendes76@xxxxxxxxx>
> > Cc: Jason Wang <jasowang@xxxxxxxxxx>
> > ---
> >  drivers/net/virtio_net.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4f3de0ac8b0b..27f9b212c9f5 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -75,6 +75,7 @@ struct virtnet_sq_stats {
> >       u64 xdp_tx;
> >       u64 xdp_tx_drops;
> >       u64 kicks;
> > +     u64 tx_timeouts;
> >  };
> >
> >  struct virtnet_rq_stats {
> > @@ -98,6 +99,7 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = {
> >       { "xdp_tx",             VIRTNET_SQ_STAT(xdp_tx) },
> >       { "xdp_tx_drops",       VIRTNET_SQ_STAT(xdp_tx_drops) },
> >       { "kicks",              VIRTNET_SQ_STAT(kicks) },
> > +     { "tx_timeouts",        VIRTNET_SQ_STAT(tx_timeouts) },
> >  };
> >
> >  static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = {
> > @@ -1721,7 +1723,7 @@ static void virtnet_stats(struct net_device *dev,
> >       int i;
> >
> >       for (i = 0; i < vi->max_queue_pairs; i++) {
> > -             u64 tpackets, tbytes, rpackets, rbytes, rdrops;
> > +             u64 tpackets, tbytes, terrors, rpackets, rbytes, rdrops;
> >               struct receive_queue *rq = &vi->rq[i];
> >               struct send_queue *sq = &vi->sq[i];
> >
> > @@ -1729,6 +1731,7 @@ static void virtnet_stats(struct net_device *dev,
> >                       start = u64_stats_fetch_begin_irq(&sq->stats.syncp);
> >                       tpackets = sq->stats.packets;
> >                       tbytes   = sq->stats.bytes;
> > +                     terrors  = sq->stats.tx_timeouts;
> >               } while (u64_stats_fetch_retry_irq(&sq->stats.syncp, start));
> >
> >               do {
> > @@ -1743,6 +1746,7 @@ static void virtnet_stats(struct net_device *dev,
> >               tot->rx_bytes   += rbytes;
> >               tot->tx_bytes   += tbytes;
> >               tot->rx_dropped += rdrops;
> > +             tot->tx_errors  += terrors;
> >       }
> >
> >       tot->tx_dropped = dev->stats.tx_dropped;
> >
>