On 2019/02/01 5:15, Jesper Dangaard Brouer wrote: > On Thu, 31 Jan 2019 09:45:23 -0800 (PST) > David Miller <davem@xxxxxxxxxxxxx> wrote: > >> From: "Michael S. Tsirkin" <mst@xxxxxxxxxx> >> Date: Thu, 31 Jan 2019 10:25:17 -0500 >> >>> On Thu, Jan 31, 2019 at 08:40:30PM +0900, Toshiaki Makita wrote: >>>> Previously virtnet_xdp_xmit() did not account for device tx counters, >>>> which caused confusions. >>>> To be consistent with SKBs, account them on freeing xdp_frames. >>>> >>>> Reported-by: David Ahern <dsahern@xxxxxxxxx> >>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@xxxxxxxxxxxxx> >>> >>> Well we count them on receive so I guess it makes sense for consistency >>> >>> Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx> >>> >>> however, I really wonder whether adding more and more standard net stack >>> things like this will end up costing most of XDP its speed. >>> >>> Should we instead make sure *not* to account XDP packets >>> in any counters at all? XDP programs can use maps >>> to do their own counting... >> >> This has been definitely a discussion point, and something we should >> develop a clear, strong, policy on. >> >> David, Jesper, care to chime in where we ended up in that last thread >> discussion this? > > IHMO packets RX and TX on a device need to be accounted, in standard > counters, regardless of XDP. For XDP RX the packet is counted as RX, > regardless if XDP choose to XDP_DROP. On XDP TX which is via > XDP_REDIRECT or XDP_TX, the driver that transmit the packet need to > account the packet in a TX counter (this if often delayed to DMA TX > completion handling). We cannot break the expectation that RX and TX > counter are visible to userspace stats tools. XDP should not make these > packets invisible. > > Performance wise, I don't see an issue. As updating these counters > (packets and bytes) can be done as a bulk, either when driver NAPI RX > func ends, or in TX DMA completion, like most drivers already do today. > Further more, most drivers save this in per RX ring data-area, which > are only summed when userspace read these. Agreed. > A separate question (and project) raised by David Ahern, was if we > should have more detailed stats on the different XDP action return > codes, as an easy means for sysadms to diagnose running XDP programs. > That is something that require more discussions, as it can impact > performance, and likely need to be opt-in. My opinion is yes we should > do this for the sake of better User eXperience, BUT *only* if we can find > a technical solution that does not hurt performance. Basically the situation for the detailed stats is the same as standard stats, at least in virtio_net. Stats are updated as a bulk, and the counters reside in RX/TX ring structures. Probably this way of implementation would be ok performance-wise? But as other drivers may have different situations, if it is generally difficult to avoid performance penalty I'm OK with making them opt-in as a standard way. -- Toshiaki Makita _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
