[MPTCP] Re: [MPTCP][RFC mptcp-next 0/6] add trace events

[MPTCP] Re: [MPTCP][RFC mptcp-next 0/6] add trace events

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

On Fri, 29 Jan 2021, Paolo Abeni wrote:

> Hello,
>
> On Tue, 2021-01-26 at 21:21 +0800, Geliang Tang wrote:
>> This patchset addressed issues 131, replace some/most pr_debug with trace
>> events.
>>
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/131
>>
>> Geliang Tang (6):
>>   mptcp: add tracepoints for data operations
>>   mptcp: add echo field in mptcp_out_options
>>   mptcp: add tracepoints for options operations
>>   mptcp: add mptcpi_local_addr_* in mptcp_info
>>   mptcp: add tracepoints for PM operations
>>   mptcp: add tracepoints for subflow operations
>
> I'm sorry for the late feedback.
>
> I think we should first agree on which functions we want to
> instrument/where we want to add the tracepoint.
>
> I suggest avoiding functions invoked not frequently (pr_debug would
> probably suffice there) and avoiding exposing data accessible elsewhere
> e.g. packet capture.
>
> I think it would be nice to instrument the packet scheduler and the
> mapping status. I would not add much more tracepoints than that.
>

Yes, these seem like good places to focus first as the main high-frequency 
tracepoints that will be helpful.

> Additionally, I suggest dropping the pr_debug() from places where
> tracepoints are added.
>
> I'd love to ear others opinions, too!
>

Agree with Paolo on all the above!

Thanks for the patches Geliang, and thanks for the review Paolo.

--
Mat Martineau
Intel

[MPTCP] Re: [MPTCP][RFC mptcp-next 0/6] add trace events

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Hello,

On Tue, 2021-01-26 at 21:21 +0800, Geliang Tang wrote:
> This patchset addressed issues 131, replace some/most pr_debug with trace
> events.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/131
> 
> Geliang Tang (6):
>   mptcp: add tracepoints for data operations
>   mptcp: add echo field in mptcp_out_options
>   mptcp: add tracepoints for options operations
>   mptcp: add mptcpi_local_addr_* in mptcp_info
>   mptcp: add tracepoints for PM operations
>   mptcp: add tracepoints for subflow operations

I'm sorry for the late feedback.

I think we should first agree on which functions we want to
instrument/where we want to add the tracepoint.

I suggest avoiding functions invoked not frequently (pr_debug would
probably suffice there) and avoiding exposing data accessible elsewhere
e.g. packet capture.

I think it would be nice to instrument the packet scheduler and the
mapping status. I would not add much more tracepoints than that.

Additionally, I suggest dropping the pr_debug() from places where
tracepoints are added.

I'd love to ear others opinions, too!

Cheers,

Paolo