| Message ID | 43374bda3e77484e8f3158e7844624ea@jd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] netdev-linux: remove sum of vport stats and kernel netdev stats | expand |
On Thu, Apr 23, 2020 at 05:35:14AM +0000, 姜立东 via dev wrote: > From df9ff3b67f11e467928ca0873031d81b87f3d0c5 Mon Sep 17 00:00:00 2001 > From: Jiang Lidong <jianglidong3@jd.com> > Date: Thu, 23 Apr 2020 11:07:28 +0800 > Subject: [PATCH] netdev-linux: remove sum of vport stats and kernel netdev stats > When using kernel veth as OVS interface, doubled drop counter > value is shown when veth drops packets due to traffic overrun. > > In netdev_linux_get_stats, it reads both vport stats and kernel > netdev stats, in case vport stats retrieve failure. If both of > them success, error counters are added to include errors from > different layers. But implementation of ovs_vport_get_stats in > kernel data path has included kernel netdev stats by calling > dev_get_stats. When drop or other error counters is not zero, > its value is doubled by netdev_linux_get_stats. > > In this change, adding kernel netdev stats into vport stats > is removed, since vport stats includes all information of > kernel netdev stats. > > Signed-off-by: Jiang Lidong <jianglidong3@jd.com> > --- > lib/netdev-linux.c | 27 +-------------------------- > 1 file changed, 1 insertion(+), 26 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index ff045cb..6d139d0 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2207,33 +2207,8 @@ netdev_linux_get_stats(const struct netdev *netdev_, > } else if (netdev->vport_stats_error) { > /* stats not available from OVS then use netdev stats. */ > *stats = dev_stats; > - } else { > - /* Use kernel netdev's packet and byte counts since vport's counters > - * do not reflect packet counts on the wire when GSO, TSO or GRO are > - * enabled. */ > - stats->rx_packets = dev_stats.rx_packets; > - stats->rx_bytes = dev_stats.rx_bytes; > - stats->tx_packets = dev_stats.tx_packets; > - stats->tx_bytes = dev_stats.tx_bytes; > - > - stats->rx_errors += dev_stats.rx_errors; > - stats->tx_errors += dev_stats.tx_errors; > - stats->rx_dropped += dev_stats.rx_dropped; > - stats->tx_dropped += dev_stats.tx_dropped; Thanks for reporting the issue, looking at get_stats_via_vport get_stats_via_vport__ netdev_stats_from_ovs_vport_stats // only set the ovs_vport_stats, which has 8 fields above But with your patch it will remove all the reset of stats below > - stats->multicast += dev_stats.multicast; > - stats->collisions += dev_stats.collisions; > - stats->rx_length_errors += dev_stats.rx_length_errors; > - stats->rx_over_errors += dev_stats.rx_over_errors; > - stats->rx_crc_errors += dev_stats.rx_crc_errors; > - stats->rx_frame_errors += dev_stats.rx_frame_errors; > - stats->rx_fifo_errors += dev_stats.rx_fifo_errors; > - stats->rx_missed_errors += dev_stats.rx_missed_errors; > - stats->tx_aborted_errors += dev_stats.tx_aborted_errors; > - stats->tx_carrier_errors += dev_stats.tx_carrier_errors; > - stats->tx_fifo_errors += dev_stats.tx_fifo_errors; > - stats->tx_heartbeat_errors += dev_stats.tx_heartbeat_errors; > - stats->tx_window_errors += dev_stats.tx_window_errors; > } How about we just over write the error and drop counts? diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index c6e46f1885aa..c169c52611c4 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2220,10 +2220,11 @@ netdev_linux_get_stats(const struct netdev *netdev_, stats->tx_packets = dev_stats.tx_packets; stats->tx_bytes = dev_stats.tx_bytes; - stats->rx_errors += dev_stats.rx_errors; - stats->tx_errors += dev_stats.tx_errors; - stats->rx_dropped += dev_stats.rx_dropped; - stats->tx_dropped += dev_stats.tx_dropped; + stats->rx_errors = dev_stats.rx_errors; + stats->tx_errors = dev_stats.tx_errors; + stats->rx_dropped = dev_stats.rx_dropped; + stats->tx_dropped = dev_stats.tx_dropped; + stats->multicast += dev_stats.multicast; stats->collisions += dev_stats.collisions; stats->rx_length_errors += dev_stats.rx_length_errors; > + > ovs_mutex_unlock(&netdev->mutex); > > return error; > -- > 1.8.3.1 > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Apr 23, 2020 at 05:35:14AM +0000, 姜立东 via dev wrote: > From df9ff3b67f11e467928ca0873031d81b87f3d0c5 Mon Sep 17 00:00:00 2001 > From: Jiang Lidong <jianglidong3@jd.com> > Date: Thu, 23 Apr 2020 11:07:28 +0800 > Subject: [PATCH] netdev-linux: remove sum of vport stats and kernel netdev stats > When using kernel veth as OVS interface, doubled drop counter > value is shown when veth drops packets due to traffic overrun. > > In netdev_linux_get_stats, it reads both vport stats and kernel > netdev stats, in case vport stats retrieve failure. If both of > them success, error counters are added to include errors from > different layers. But implementation of ovs_vport_get_stats in > kernel data path has included kernel netdev stats by calling > dev_get_stats. When drop or other error counters is not zero, > its value is doubled by netdev_linux_get_stats. > > In this change, adding kernel netdev stats into vport stats > is removed, since vport stats includes all information of > kernel netdev stats. > > Signed-off-by: Jiang Lidong <jianglidong3@jd.com> > --- > lib/netdev-linux.c | 27 +-------------------------- > 1 file changed, 1 insertion(+), 26 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index ff045cb..6d139d0 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -2207,33 +2207,8 @@ netdev_linux_get_stats(const struct netdev *netdev_, > } else if (netdev->vport_stats_error) { > /* stats not available from OVS then use netdev stats. */ > *stats = dev_stats; > - } else { > - /* Use kernel netdev's packet and byte counts since vport's counters > - * do not reflect packet counts on the wire when GSO, TSO or GRO are > - * enabled. */ > - stats->rx_packets = dev_stats.rx_packets; > - stats->rx_bytes = dev_stats.rx_bytes; > - stats->tx_packets = dev_stats.tx_packets; > - stats->tx_bytes = dev_stats.tx_bytes; > - > - stats->rx_errors += dev_stats.rx_errors; > - stats->tx_errors += dev_stats.tx_errors; > - stats->rx_dropped += dev_stats.rx_dropped; > - stats->tx_dropped += dev_stats.tx_dropped; > - stats->multicast += dev_stats.multicast; > - stats->collisions += dev_stats.collisions; > - stats->rx_length_errors += dev_stats.rx_length_errors; > - stats->rx_over_errors += dev_stats.rx_over_errors; > - stats->rx_crc_errors += dev_stats.rx_crc_errors; > - stats->rx_frame_errors += dev_stats.rx_frame_errors; > - stats->rx_fifo_errors += dev_stats.rx_fifo_errors; > - stats->rx_missed_errors += dev_stats.rx_missed_errors; > - stats->tx_aborted_errors += dev_stats.tx_aborted_errors; > - stats->tx_carrier_errors += dev_stats.tx_carrier_errors; > - stats->tx_fifo_errors += dev_stats.tx_fifo_errors; > - stats->tx_heartbeat_errors += dev_stats.tx_heartbeat_errors; > - stats->tx_window_errors += dev_stats.tx_window_errors; > } > + > ovs_mutex_unlock(&netdev->mutex); > > return error; > -- > 1.8.3.1 btw, I always had a hard time applying your patch. Maybe your git is too old, can you upgrade to a newer version? Thanks William
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index ff045cb..6d139d0 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2207,33 +2207,8 @@ netdev_linux_get_stats(const struct netdev *netdev_, } else if (netdev->vport_stats_error) { /* stats not available from OVS then use netdev stats. */ *stats = dev_stats; - } else { - /* Use kernel netdev's packet and byte counts since vport's counters - * do not reflect packet counts on the wire when GSO, TSO or GRO are - * enabled. */ - stats->rx_packets = dev_stats.rx_packets; - stats->rx_bytes = dev_stats.rx_bytes; - stats->tx_packets = dev_stats.tx_packets; - stats->tx_bytes = dev_stats.tx_bytes; - - stats->rx_errors += dev_stats.rx_errors; - stats->tx_errors += dev_stats.tx_errors; - stats->rx_dropped += dev_stats.rx_dropped; - stats->tx_dropped += dev_stats.tx_dropped; - stats->multicast += dev_stats.multicast; - stats->collisions += dev_stats.collisions; - stats->rx_length_errors += dev_stats.rx_length_errors; - stats->rx_over_errors += dev_stats.rx_over_errors; - stats->rx_crc_errors += dev_stats.rx_crc_errors; - stats->rx_frame_errors += dev_stats.rx_frame_errors; - stats->rx_fifo_errors += dev_stats.rx_fifo_errors; - stats->rx_missed_errors += dev_stats.rx_missed_errors; - stats->tx_aborted_errors += dev_stats.tx_aborted_errors; - stats->tx_carrier_errors += dev_stats.tx_carrier_errors; - stats->tx_fifo_errors += dev_stats.tx_fifo_errors; - stats->tx_heartbeat_errors += dev_stats.tx_heartbeat_errors; - stats->tx_window_errors += dev_stats.tx_window_errors; } + ovs_mutex_unlock(&netdev->mutex); return error;