Re: [PATCH] bpf: Add bpf_skb_get_sock_comm() helper

[Martin KaFai Lau]

On Wed, Aug 19, 2020 at 02:10:56PM +0800, 江禹 wrote:
> Dear Martin,
> 
> 
> > One possibility is to use the "sk_bpf_storage" member immediately above
> > instead of adding "sk_task_com[]".
> > 
> > It is an extensible sk storage for bpf.  There are examples in selftests,
> > e.g tools/testing/selftests/bpf/progs/udp_limits.c which creates sk storage
> > at socket creation time.  Another hook point option could be "connect()"
> > for tcp, i.e. "cgroup/connect[46]".
> > 
> > Search "BPF_MAP_TYPE_SK_STORAGE" under the selftests/bpf for other examples.
> > 
> > It seems there is already a "bpf_get_current_comm()" helper which
> > could be used to initialize the task comm string in the bpf sk storage.
> > 
> > btw, bpf-next is still closed.
> 
> 
> I have rewrite my code according to your suggestion.  In general,it works as designed.
> 
> 
> But the task comm string got from "bpf_get_current_comm()" helper is belong to specific thread.
> It is not a obvious label for skb tracing. More reasonable tracing key is the task comm of process
> which this skb belongs.
>  
> It seems a new bpf helper is still needed.
May be.  It is not clear to me whether it is better to account this skb
to its process or to its thread.  I think this depends on the
use-case/assumption.

If bpf_get_current_comm() does not get what you need,
then another helper may be added to get the process comm.
This new helper may be useful for other use cases also.

btw, Have your thought about tracing at the sendmsg time?
e.g. tcp_sendmsg()?