On 2/17/20 7:51 AM, Jesper Dangaard Brouer wrote: > On Mon, 17 Feb 2020 14:05:15 +0100 > Andrew Lunn <andrew@xxxxxxx> wrote: > >> On Mon, Feb 17, 2020 at 11:32:09AM +0100, Jesper Dangaard Brouer wrote: >>> On Mon, 17 Feb 2020 11:25:50 +0100 >>> Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> wrote: >>> > [...] >>>> >>>> yes, I think it is definitely better. So to follow up: >>>> - rename current "xdp_tx" counter in "xdp_xmit" and increment it for >>>> XDP_TX verdict and for ndo_xdp_xmit >>>> - introduce a new "xdp_tx" counter only for XDP_TX verdict. >>>> >>>> If we agree I can post a follow-up patch. >>> >>> I agree, that sounds like an improvement to this patchset. >>> >>> >>> I suspect David Ahern have some opinions about more general stats for >>> XDP, but that it is a more general discussion, that it outside this >>> patchset, but we should also have that discussion. >> >> Hi Jesper >> >> I've not been following XDP too much, but xdp_xmit seems pretty >> generic. It would be nice if all drivers used the same statistics >> names. Less user confusion that way. So why is this outside of the >> discussion? Hi Andrew: I brought this up over a year ago - the need for some consistency in XDP stats (names and meaning) across drivers: https://lore.kernel.org/netdev/1d9a6548-4d1d-6624-e808-6ab0460a8655@xxxxxxxxx/ I don't have strong preferences on which driver is right in the current naming, only that we have consistency. There has not been much progress in the past 15 months, so I am glad to see someone take this on. > > I do want to have this discussion, please. > > I had hoped this patchset sparked this that discussion... maybe we can > have it despite this patchset already got applied? > > My only request is that, if we don't revert, we fixup the "xdp_tx" > counter name. It would make it easier for us[1] if we can keep them > applied, as we are preparing (asciinema) demos for [1]. Jesper: what about the mlx5 naming scheme: rx_xdp_drop: 86468350180 rx_xdp_redirect: 18860584 rx_xdp_tx_xmit: 0 The rx prefix shows the xdp action is in the Rx path, and then the Tx path has tx_xdp_xmit. i40e seems to have something similar for the Rx path: rx-0.xdp.pass: 0 rx-0.xdp.drop: 0 rx-0.xdp.tx: 0 rx-0.xdp.unknown: 0 rx-0.xdp.redirect: 0 rx-0.xdp.redirect_fail: 0 I don't see any Tx stats for xdp, but this is an older kernel so not sure what 5.x has. Looks like sfc has a similar naming scheme: rx_xdp_drops: 0 rx_xdp_bad_drops: 0 rx_xdp_tx: 0 rx_xdp_redirect: 0 So if mvneta follows these 3, the names just need rx_ prepended.
