[DTrace-devel] [PATCH 2/2] Implement the pcap action

[Kris Van Hees]

On Tue, Oct 10, 2023 at 01:39:04PM -0400, Eugene Loh via DTrace-devel wrote:
> On 10/10/23 00:35, Kris Van Hees wrote:
> > On Mon, Oct 09, 2023 at 11:54:32PM -0400, Eugene Loh via DTrace-devel wrote:
> > > I am not familiar with pcap...
> 
> In legacy DTrace, in dtrace/dtrace_probe.c, dtrace_probe_pcap() deals with
> some complications around fragments.  Why are there no such complications in
> this patch?

Like your io provider patch, there are still some (uncommon) cases that are not
fully covered.  In discussing this with Alan Maguire, we decided that the case
of fragment reconstruction is sufficiently rare that this version is definitely
the most important support that is needed.  Support for fragmented packets is
a follow-up development.

> And, another comment on the patch.  In dt_cg_act_pcap(), pcapsz is defined
> as dtp->dt_options[DTRACEOPT_PCAPSIZE].  Later, data_off is defined in terms
> of dtp->dt_options[DTRACEOPT_PCAPSIZE].  This is an opportunity to use
> pcapsz.

Sure - thanks.

> > > That's "a lot" of instructions simply to read a word from an unverified
> > > address.  Not too many, but such a read is relatively common.  So, we should
> > > have a simple way of generating these instructions.  How about
> > > dt_cg_load_scalar()?  Bottom line is that we should look at the various
> > > sites that read words in this manner and put the associated code generation
> > > under a function that can be called from code like this.  Code reuse.
> > Yes, we do need to look into better (more) utility functions that help with
> > common code like this.  I am not sure whether we ought to do that right now,
> > but I'll look at it because it is becoming more common (here and also in
> > some provider trampolines).
> 
> Yeah.  One issue with trampolines is that the context for reg management is
> different.  Anyhow, yes, and my patch for the io provider, fwiw, has a
> utility function for this stuff.  There, it's especially useful, because
> there are a lot of such reads, including pointer chasing and stuff.  So
> there is a big incentive to simplify the cg.

Yes, definitely.  And we should spend some work on consolidating this type of
work across the entire code base, i.e. the code generator in general and also
across all the provider trampoline generation code.

We can assess and discuss whether that should be done now or whether we should
do it as a follow-up patch (since there are other areas that can benefit from
this).