On 2018-11-19 10:27 a.m., Geneviève Bastien wrote: > On 2018-11-18 1:19 a.m., David Miller wrote: >> From: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> >> Date: Sat, 17 Nov 2018 13:27:29 -0500 (EST) >> >>> I see two possible solutions: >>> >>> 1) Remove the "skb" argument from the sbk_exit tracepoints completely. >>> Anyway, I think it's not really needed for analysis purposes because >>> we can link the "entry" with the associated "exit" using the thread ID >>> executing those tracepoints. (Genevi�ve, would that work for your >>> analyses ?) >>> >>> 2) Move the skb_exit tracepoints before freeing the skb pointer. My >>> concern here is that the instrumentation may become much uglier than >>> the currently proposed patch. (I have not looked at the specifics >>> though, so I may be wrong.) >>> >>> Do you have a preference between those two approaches, or perhaps you >>> have an alternative solution in mind ? >> I wonder how other situations handle this. >> >> About #2, if you put the tracepoint beforehand you can't log the >> 'ret' value. So at least in that regard I prefer #1. >> >> If tracepoints generally handle this by matching up the thread >> ID, then definitely that's how we should do it here too instead >> of trying to use the SKB pointer for this purpose. > I would go for #1 too, the "skb" is not used to match entry/exit, it is more > the context in which they appear (thread, softirq, etc). And I did indeed get > seg faults on my first attempt when I tried to use the existing templates. > > There's just the list tracepoint that would now log nothing, so there's no > point looping through the list. > After further investigation, the 'netif_receive_skb_list_exit' tracepoint poses a problem: There is no return value in that function and the TRACE_EVENT infrastructure does not support tracepoints without payload, afaik.
Here's a few possible solutions: 1) Add a unique trace_netif_receive_skb_list_exit tracepoint with a 'fake' return value of 0, so it can use the same template as the others. Easiest, but looks hackish. 2) Add tracepoints in the callees in the various locations where individual skbs get removed from the list. But that's quite a few code paths to cover, and we may miss a few. 3) Simply ignore the exit of the list code path. Dependency analyses will miss this case though. 4) Replace the proposed exit tracepoints by others after the delivery of packets, ie in the 'netif_receive_skb_internal' and 'netif_receive_skb_list_internal' functions, again at the risk of missing code paths. 5) None of the above (and fallback to using kretprobes with the tracers) Any one of these approaches sound more appealing? Thanks, Geneviève
