| Message ID | 20220803085754.185730-1-rjarry@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] netdev-dpdk: fix tx_dropped counters value | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Wed, Aug 3, 2022 at 4:58 AM Robin Jarry <rjarry@redhat.com> wrote: > > Packets that could not be transmitted because the TXQ are full should be > taken into account in the global ovs_tx_failure_drops as it was the case > before commit 29b94e12d57d ("netdev-dpdk: Refactor the DPDK transmit > path."). > > Also, the global tx_dropped should take all ovs_*_tx_drops counters into > account. > > Fixes: 29b94e12d57d ("netdev-dpdk: Refactor the DPDK transmit path.") > Signed-off-by: Robin Jarry <rjarry@redhat.com> > --- > v1 -> v2: fixed build error (last minute copy paste without testing...) > > lib/netdev-dpdk.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 0dd655507b50..84f0fbf24355 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2883,8 +2883,12 @@ netdev_dpdk_eth_send(struct netdev *netdev, int qid, > cnt = netdev_dpdk_common_send(netdev, batch, &stats); > > dropped = batch_cnt - cnt; > - > dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, cnt); > + stats.tx_failure_drops += dropped; Hello Robin, I think this will double count tx_failure_drops if there were any in netdev_dpdk_common_send(). > + dropped = stats.tx_mtu_exceeded_drops + > + stats.tx_qos_drops + > + stats.tx_failure_drops + > + stats.tx_invalid_hwol_drops; I'm not too clear on why we have to recalculate this, what case would ccase dropped to be incorrect here. Thank you, M > if (OVS_UNLIKELY(dropped)) { > struct netdev_dpdk_sw_stats *sw_stats = dev->sw_stats; > > -- > 2.37.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Hi Mike, Mike Pattrick, Aug 31, 2022 at 18:09: > On Wed, Aug 3, 2022 at 4:58 AM Robin Jarry <rjarry@redhat.com> wrote: > > dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, cnt); > > + stats.tx_failure_drops += dropped; > > Hello Robin, > > I think this will double count tx_failure_drops if there were any in > netdev_dpdk_common_send(). You are correct, I will fix this. > > + dropped = stats.tx_mtu_exceeded_drops + > > + stats.tx_qos_drops + > > + stats.tx_failure_drops + > > + stats.tx_invalid_hwol_drops; > > I'm not too clear on why we have to recalculate this, what case would > ccase dropped to be incorrect here. I'll change the logic a bit. This is awkward. Thanks for reviewing!
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 0dd655507b50..84f0fbf24355 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2883,8 +2883,12 @@ netdev_dpdk_eth_send(struct netdev *netdev, int qid, cnt = netdev_dpdk_common_send(netdev, batch, &stats); dropped = batch_cnt - cnt; - dropped += netdev_dpdk_eth_tx_burst(dev, qid, pkts, cnt); + stats.tx_failure_drops += dropped; + dropped = stats.tx_mtu_exceeded_drops + + stats.tx_qos_drops + + stats.tx_failure_drops + + stats.tx_invalid_hwol_drops; if (OVS_UNLIKELY(dropped)) { struct netdev_dpdk_sw_stats *sw_stats = dev->sw_stats;
Packets that could not be transmitted because the TXQ are full should be taken into account in the global ovs_tx_failure_drops as it was the case before commit 29b94e12d57d ("netdev-dpdk: Refactor the DPDK transmit path."). Also, the global tx_dropped should take all ovs_*_tx_drops counters into account. Fixes: 29b94e12d57d ("netdev-dpdk: Refactor the DPDK transmit path.") Signed-off-by: Robin Jarry <rjarry@redhat.com> --- v1 -> v2: fixed build error (last minute copy paste without testing...) lib/netdev-dpdk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)