Message-ID: <CAL+tcoC_5106onp6yQh-dKnCTLtEr73EZVC31T_YeMtqbZ5KBw@mail.gmail.com>
Date: Thu, 6 Feb 2025 11:41:42 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: Martin KaFai Lau <martin.lau@...ux.dev>, Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, dsahern@...nel.org, 
	willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, horms@...nel.org, bpf@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH bpf-next v8 10/12] bpf: make TCP tx timestamp bpf
 extension work

On Thu, Feb 6, 2025 at 11:25 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> > > > I think we can split the whole idea into two parts: for now, because
> > > > of the current series implementing the same function as SO_TIMETAMPING
> > > > does, I will implement the selective sample feature in the series.
> > > > After someday we finish tracing all the skb, then we will add the
> > > > corresponding selective sample feature.
> > >
> > > Are you saying that you will include selective sampling now or want to
> > > postpone it?
> >
> > A few months ago, I planned to do it after this series. Since you all
> > ask, it's not complex to have it included in this series :)
> >
> > Selective sampling has two kinds of meaning like I mentioned above, so
> > in the next re-spin I will implement the cmsg feature for bpf
> > extension in this series.
>
> Great thanks.

I have to rephrase a bit in case Martin visits here soon: I will
compare two approaches 1) reply value, 2) bpf kfunc and then see which
way is better.

>
> > I'm doing the test right now. And leave
> > another selective sampling small feature until the feature of tracing
> > all the skbs is implemented if possible.
>
> Can you elaborate on this other feature?

Do you recall oneday I asked your opinion privately about whether we
can trace _all the skbs_ (not the last skb from each sendmsg) to have
a better insight of kernel behaviour? I can also see a couple of
latency issues in the kernel. If it is approved, then corresponding
selective sampling should be supported. It's what I was trying to
describe.

The advantage of relying on the timestamping feature is that we can
isolate normal flows and monitored flow so that normal flows wouldn't
be affected because of enabling the monitoring feature, compared to so
many open source monitoring applications I've dug into. They usually
directly hook the hot path like __tcp_transmit_skb() or
dev_queue_xmit, which will surely influence the normal flows and cause
performance degradation to some extent. I noticed that after
conducting some tests a few months ago. The principle behind the bpf
fentry is to replace some instructions at the very beginning of the
hooked function, so every time even normal flows entering the
monitored function will get affected.

Thanks,
Jason

>
> > >
> > > Jakub brought up a great point. Our continuous deployment would not be
> > > feasible without sampling. Indeed implemented using cmsg.
> >
> > Right, right. I just realized that I misunderstood what Jakub offered.
> >
> > >
> > > I think it should be included from the initial patch series.
> >
> > I agree to include this in this series. Like what I wrote in the
> > previous thread, it should be simple :) And it will be manifested in
> > the selftests as well.
> >
> > Thanks,
> > Jason
>
>

Powered by blists - more mailing lists