> On 4/15/21 9:03 AM, Lorenzo Bianconi wrote: > >> On 4/15/21 8:05 AM, Daniel Borkmann wrote: > > > > [...] > >>>> &stats); > >>> > >>> Given we stop counting drops with the netif_receive_skb_list(), we > >>> should then > >>> also remove drops from trace_xdp_cpumap_kthread(), imho, as otherwise it > >>> is rather > >>> misleading (as in: drops actually happening, but 0 are shown from the > >>> tracepoint). > >>> Given they are not considered stable API, I would just remove those to > >>> make it clear > >>> to users that they cannot rely on this counter anymore anyway. > >>> > >> > >> What's the visibility into drops then? Seems like it would be fairly > >> easy to have netif_receive_skb_list return number of drops. > >> > > > > In order to return drops from netif_receive_skb_list() I guess we need to introduce > > some extra checks in the hot path. Moreover packet drops are already accounted > > in the networking stack and this is currently the only consumer for this info. > > Does it worth to do so? > > right - softnet_stat shows the drop. So the loss here is that the packet > is from a cpumap XDP redirect. > > Better insights into drops is needed, but I guess in this case coming > from the cpumap does not really aid into why it is dropped - that is > more core to __netif_receive_skb_list_core. I guess this is ok to drop > the counter from the tracepoint. > Applying the current patch, drops just counts the number of kmem_cache_alloc_bulk() failures. Looking at kmem_cache_alloc_bulk() code, it does not seem to me there any failure counters. So I am wondering, is this an important info for the user? Is so I guess we can just rename the counter in something more meaningful (e.g. skb_alloc_failures). Regards, Lorenzo
Attachment:
signature.asc
Description: PGP signature
