Jesper Dangaard Brouer <brouer@xxxxxxxxxx> writes: > On Wed, 15 Jan 2020 23:11:21 +0100 > Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > >> Jesper Dangaard Brouer <brouer@xxxxxxxxxx> writes: >> >> > On Mon, 13 Jan 2020 19:10:55 +0100 >> > Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >> > >> >> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c >> >> index da9c832fc5c8..030d125c3839 100644 >> >> --- a/kernel/bpf/devmap.c >> >> +++ b/kernel/bpf/devmap.c >> > [...] >> >> @@ -346,8 +340,7 @@ static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags) >> >> out: >> >> bq->count = 0; >> >> >> >> - trace_xdp_devmap_xmit(&obj->dtab->map, obj->idx, >> >> - sent, drops, bq->dev_rx, dev, err); >> >> + trace_xdp_devmap_xmit(NULL, 0, sent, drops, bq->dev_rx, dev, err); >> > >> > Hmm ... I don't like that we lose the map_id and map_index identifier. >> > This is part of our troubleshooting interface. >> >> Hmm, I guess I can take another look at whether there's a way to avoid >> that. Any ideas? > > Looking at the code and the other tracepoints... > > I will actually suggest to remove these two arguments, because the > trace_xdp_redirect_map tracepoint also contains the ifindex'es, and to > troubleshoot people can record both tracepoints and do the correlation > themselves. > > When changing the tracepoint I would like to keep member 'drops' and > 'sent' at the same struct offsets. As our xdp_monitor example reads > these and I hope we can kept it working this way. > > I've coded it up, and tested it. The new xdp_monitor will work on > older kernels, but the old xdp_monitor will fail attaching on newer > kernels. I think this is fair enough, as we are backwards compatible. SGTM - thanks! I'll respin and include this :) -Toke
