On Mon, Jul 02, 2007 at 07:33:45PM -0700, [EMAIL PROTECTED] wrote:
[...]
> >>I agree with this.
> >>But having seen the sdt:::ip-xmit-v4 probe, wouldn't it be
> >>nice if you could present that as fbt::ip-xmit-v4:entry but
> >>pass in ire->ire_nce as arg2 instead of ire?
> >>   
> >>
> >
> >I would have thought that the existing (ire_t *)ire as args[2] was already
> >ideal, as users could refer to args[2]->ire_nce as needed. It might be
> >a different story if the kernel wasn't CTF'd and these wern't already
> >casted.
> >
> 
> Indeed and I suspect that this hook is simply "ease of use"
> in debugging and could be a candidate for removing..

Yes; and an advantage of private sdt probes is that they could be added
to support a particular performance project and then removed later
when no longer needed...

> >>DTRACE_VIEW2(ip__hook__abc, ip_send, mblk_t *, void_ip_t *);
> >>DTRACE_VIEW3(ip__hook__abc, ip4__send, mblk_t *, ipha_t *, ill_t *);
[...]
> >It would take some moderate work to get this done; and when complete, there
> >would still be more non-enabled probe overhead in other places - like the
> >mib probes, that wouldn't benifit from this approach.
> >
> 
> Understood.  Should I RFE this or is there some other virtual
> whiteboard on opensolaris.org for new things to add to dtrace?

AFAIK, the DTrace todo list is the RFE list from bugster.

[...]
> >There might be an easier way to drop the duplicate probe overhead, which
> >currently looks like,
> >
> >  14223                 DTRACE_PROBE4(ip4__physical__out__start,
> >  14224                     ill_t *, NULL, ill_t *, stq_ill,
> >  14225                     ipha_t *, ipha, mblk_t *, mp);
> >  14226                 FW_HOOKS(ipst->ips_ip4_physical_out_event,
> >  14227                     ipst->ips_ipv4firewall_physical_out,
> >  14228                     NULL, stq_ill, ipha, mp, mpip, ipst);
> >  14229                 DTRACE_PROBE1(ip4__physical__out__end, mblk_t *,
> >  14230                     mp);
> >  14231                 if (mp == NULL)
> >  14232                         goto drop;
> >  14233 
> >  14234                 DTRACE_IP5(send, mblk_t *, mp, void_ip_t *, ipha,
> >  14235                     ill_t *, stq_ill, ipha_t *, ipha, ip6_t *, 
> >  NULL);
> >
> >Why have sdt:::ip4-physical-out-start and sdt:::ip4-physical-out-end 
> >anyway?
> > 
> >
> 
> For -start, the point is to have a dtrace probe that is particular to the
> location of the hook in the stack relative to packet processing.

That sounds doable from fbt,

   # dtrace -n 'fbt::hook_run:entry { 
trace(stringof(args[0]->hei_event->he_name)); stack(); }'
   dtrace: description 'fbt::hook_run:entry ' matched 1 probe
   CPU     ID                    FUNCTION:NAME
     0  73492                   hook_run:entry   PHYSICAL_OUT                   
  
                 ip`tcp_send_data+0x78e
                 ip`tcp_output+0x78f
                 ip`squeue_enter+0x41a
                 ip`tcp_wput+0xf8
                 unix`putnext+0x2f1
                 rpcmod`mir_wput+0x1aa
                 rpcmod`rmm_wput+0x1e
                 unix`put+0x270
                 rpcmod`clnt_dispatch_send+0x11a
                 rpcmod`clnt_cots_kcallit+0x596
                 nfs`nfs4_rfscall+0x4e2
                 nfs`rfs4call+0x102
                 nfs`nfs4lookupvalidate_otw+0x2d5
                 nfs`nfs4lookup+0x212
                 nfs`nfs4_lookup+0xe5
                 genunix`fop_lookup+0x53
                 genunix`lookuppnvp+0x2e5
                 genunix`lookuppnat+0x125
                 genunix`lookupnameat+0x82
                 genunix`cstatat_getvp+0x160
   [...]

> For -end, it is useful to see the mp after the FW_HOOKS() is complete.
> Is it NULL?  Has it been changed?  Both of these are possible here.
> Or in other words, the complexity of using dtrace to achieve something
> meaningful is why there are two hooks.

A minute reading FW_HOOKS() shows that it will set mp to NULL and complete
if hook_run() returns zero, which fbt can trace.

Yes, there are some really complex associations and mental leaps that must
be made with fbt to solve some tracing issues, where it would be useful
to use sdt instead. It doesn't look like this is one of them.

> Plus, none of the standard fbt things will show us what happens to mp on
> the successful return of the call to hook_run.

This is also doable from fbt, although a few lines of code,

        fbt::hook_run:entry
        {
                self->info = (hook_pkt_event_t *)arg1;
        }

        fbt::hook_run:return
        /self->info/
        {
                this->mp = arg1 != 0 ? NULL : (uint64_t)*self->info->hpe_mp;
                printf("mp: %x", this->mp);
                self->info = 0;
        }

The above shows the value of mp after FW_HOOKS(), and accounts for both
hook_run() and FW_HOOKS() itself changing mp.

> In the scripts that you added, yes, you've achieved the same thing,
> but it's not a trivial exercise.  You need to (a) know that you can work
> this way with dtrace and (b) know what's inside the args.  Recommending
> that people take that approach isn't something we should be doing a lot
> of if they end up using interfaces that aren't stable.

Those probes export mblk_t's and ill_t's around hook calls, so whoever is
interested in using them must have some familiarity with kernel code. 

> Or to put it another way, it it was possible to use a probes that
> presented just as simple an interface but implemented it using
> other means then sure, the probes could be eliminated.

Who is this for? Who would be working on this code and not have read the
contents of FW_HOOKS()?

> Or to use the syntax from above, I would be just as happy if I could
> write a probe like this:
> 
> dtrace -I provider/pfh -n 'pfh:::ip4-physical-out-end{...}'
> 
> where "-I" is an "include" function, and dtrace somehow built the
> ip4-physical-out-end using fbt probes like you did, why do I care
> how the probe is implemented?  A consideration that needs to be
> taken into account is the performance cost of this approach.

DTrace already hase some functionality that could be used; People can
build custom /usr/lib/dtrace include files containing translators,
inlines and #defines to make life easier for tracing certain targets.

It might make sense to add a little functionality, such as provider
aliasing as you suggested, to tie this together as a user customised
view of fbt. Especially for fbt probes that are complex to use.

Of course, anyone writing these include files will need to maintain them
as the kernel changes.

> A danger in relying on fbt is that it only works when the function
> call works.  So, for example, if ip4-physical-out-start/end were made
> to depend on fbt::hook_run, if that function doesn't get called then
> there's no physical-out-start/end.  While the -end probe is only
> really useful in the presence of hook run being called, the other is
> not.

Yes, that is a difference between fbt and those sdt probes. You can at
least detect this situation using dtrace (the lack of hook_run() calls
from certain funcitons).

cheers,

Brendan

-- 
Brendan
[CA, USA]
_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to