* [PATCH RFC] net: qrtr: correct types of trace event parameters
@ 2023-04-02 11:15 Simon Horman
2023-04-03 5:14 ` Manivannan Sadhasivam
2023-04-04 8:20 ` Mukesh Ojha
0 siblings, 2 replies; 4+ messages in thread
From: Simon Horman @ 2023-04-02 11:15 UTC (permalink / raw)
To: Manivannan Sadhasivam, Steven Rostedt, Masami Hiramatsu
Cc: linux-arm-msm, linux-kernel, linux-trace-kernel, netdev
The arguments passed to the trace events are of type unsigned int,
however the signature of the events used __le32 parameters.
I may be missing the point here, but sparse flagged this and it
does seem incorrect to me.
net/qrtr/ns.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/qrtr.h):
./include/trace/events/qrtr.h:11:1: warning: cast to restricted __le32
./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
... (a lot more similar warnings)
net/qrtr/ns.c:115:47: expected restricted __le32 [usertype] service
net/qrtr/ns.c:115:47: got unsigned int service
net/qrtr/ns.c:115:61: warning: incorrect type in argument 2 (different base types)
... (a lot more similar warnings)
Signed-off-by: Simon Horman <horms@kernel.org>
---
include/trace/events/qrtr.h | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/include/trace/events/qrtr.h b/include/trace/events/qrtr.h
index b1de14c3bb93..441132c67133 100644
--- a/include/trace/events/qrtr.h
+++ b/include/trace/events/qrtr.h
@@ -10,15 +10,16 @@
TRACE_EVENT(qrtr_ns_service_announce_new,
- TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
+ TP_PROTO(unsigned int service, unsigned int instance,
+ unsigned int node, unsigned int port),
TP_ARGS(service, instance, node, port),
TP_STRUCT__entry(
- __field(__le32, service)
- __field(__le32, instance)
- __field(__le32, node)
- __field(__le32, port)
+ __field(unsigned int, service)
+ __field(unsigned int, instance)
+ __field(unsigned int, node)
+ __field(unsigned int, port)
),
TP_fast_assign(
@@ -36,15 +37,16 @@ TRACE_EVENT(qrtr_ns_service_announce_new,
TRACE_EVENT(qrtr_ns_service_announce_del,
- TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
+ TP_PROTO(unsigned int service, unsigned int instance,
+ unsigned int node, unsigned int port),
TP_ARGS(service, instance, node, port),
TP_STRUCT__entry(
- __field(__le32, service)
- __field(__le32, instance)
- __field(__le32, node)
- __field(__le32, port)
+ __field(unsigned int, service)
+ __field(unsigned int, instance)
+ __field(unsigned int, node)
+ __field(unsigned int, port)
),
TP_fast_assign(
@@ -62,15 +64,16 @@ TRACE_EVENT(qrtr_ns_service_announce_del,
TRACE_EVENT(qrtr_ns_server_add,
- TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
+ TP_PROTO(unsigned int service, unsigned int instance,
+ unsigned int node, unsigned int port),
TP_ARGS(service, instance, node, port),
TP_STRUCT__entry(
- __field(__le32, service)
- __field(__le32, instance)
- __field(__le32, node)
- __field(__le32, port)
+ __field(unsigned int, service)
+ __field(unsigned int, instance)
+ __field(unsigned int, node)
+ __field(unsigned int, port)
),
TP_fast_assign(
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] net: qrtr: correct types of trace event parameters
2023-04-02 11:15 [PATCH RFC] net: qrtr: correct types of trace event parameters Simon Horman
@ 2023-04-03 5:14 ` Manivannan Sadhasivam
2023-04-03 13:10 ` Simon Horman
2023-04-04 8:20 ` Mukesh Ojha
1 sibling, 1 reply; 4+ messages in thread
From: Manivannan Sadhasivam @ 2023-04-03 5:14 UTC (permalink / raw)
To: Simon Horman
Cc: Steven Rostedt, Masami Hiramatsu, linux-arm-msm, linux-kernel,
linux-trace-kernel, netdev
On Sun, Apr 02, 2023 at 01:15:33PM +0200, Simon Horman wrote:
> The arguments passed to the trace events are of type unsigned int,
> however the signature of the events used __le32 parameters.
>
> I may be missing the point here, but sparse flagged this and it
> does seem incorrect to me.
>
> net/qrtr/ns.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/qrtr.h):
> ./include/trace/events/qrtr.h:11:1: warning: cast to restricted __le32
> ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> ... (a lot more similar warnings)
> net/qrtr/ns.c:115:47: expected restricted __le32 [usertype] service
> net/qrtr/ns.c:115:47: got unsigned int service
> net/qrtr/ns.c:115:61: warning: incorrect type in argument 2 (different base types)
> ... (a lot more similar warnings)
>
You are right. The actual arguments (service, instance, node, port) transferred/
received over QRTR are in le32 as per the protocol. But in the NS driver, the
arguments passed to the trace events are in the native endian (i.e) before
getting typecased to le32 for transmission.
And my intention was to trace the arguments in native endian format only. So
this patch indeed fixes the issue.
> Signed-off-by: Simon Horman <horms@kernel.org>
Please add the fixes tag once you remove RFC,
Fixes: dfddb54043f0 ("net: qrtr: Add tracepoint support")
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> include/trace/events/qrtr.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/trace/events/qrtr.h b/include/trace/events/qrtr.h
> index b1de14c3bb93..441132c67133 100644
> --- a/include/trace/events/qrtr.h
> +++ b/include/trace/events/qrtr.h
> @@ -10,15 +10,16 @@
>
> TRACE_EVENT(qrtr_ns_service_announce_new,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
> @@ -36,15 +37,16 @@ TRACE_EVENT(qrtr_ns_service_announce_new,
>
> TRACE_EVENT(qrtr_ns_service_announce_del,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
> @@ -62,15 +64,16 @@ TRACE_EVENT(qrtr_ns_service_announce_del,
>
> TRACE_EVENT(qrtr_ns_server_add,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] net: qrtr: correct types of trace event parameters
2023-04-03 5:14 ` Manivannan Sadhasivam
@ 2023-04-03 13:10 ` Simon Horman
0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2023-04-03 13:10 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Steven Rostedt, Masami Hiramatsu, linux-arm-msm, linux-kernel,
linux-trace-kernel, netdev
On Mon, Apr 03, 2023 at 10:44:56AM +0530, Manivannan Sadhasivam wrote:
> On Sun, Apr 02, 2023 at 01:15:33PM +0200, Simon Horman wrote:
> > The arguments passed to the trace events are of type unsigned int,
> > however the signature of the events used __le32 parameters.
> >
> > I may be missing the point here, but sparse flagged this and it
> > does seem incorrect to me.
> >
> > net/qrtr/ns.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/qrtr.h):
> > ./include/trace/events/qrtr.h:11:1: warning: cast to restricted __le32
> > ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> > ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> > ... (a lot more similar warnings)
> > net/qrtr/ns.c:115:47: expected restricted __le32 [usertype] service
> > net/qrtr/ns.c:115:47: got unsigned int service
> > net/qrtr/ns.c:115:61: warning: incorrect type in argument 2 (different base types)
> > ... (a lot more similar warnings)
> >
>
> You are right. The actual arguments (service, instance, node, port) transferred/
> received over QRTR are in le32 as per the protocol. But in the NS driver, the
> arguments passed to the trace events are in the native endian (i.e) before
> getting typecased to le32 for transmission.
>
> And my intention was to trace the arguments in native endian format only. So
> this patch indeed fixes the issue.
>
> > Signed-off-by: Simon Horman <horms@kernel.org>
>
> Please add the fixes tag once you remove RFC,
>
> Fixes: dfddb54043f0 ("net: qrtr: Add tracepoint support")
>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Hi Manivannan,
thanks for your review.
I'll add the tags and drop the RFC designation.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] net: qrtr: correct types of trace event parameters
2023-04-02 11:15 [PATCH RFC] net: qrtr: correct types of trace event parameters Simon Horman
2023-04-03 5:14 ` Manivannan Sadhasivam
@ 2023-04-04 8:20 ` Mukesh Ojha
1 sibling, 0 replies; 4+ messages in thread
From: Mukesh Ojha @ 2023-04-04 8:20 UTC (permalink / raw)
To: Simon Horman, Manivannan Sadhasivam, Steven Rostedt, Masami Hiramatsu
Cc: linux-arm-msm, linux-kernel, linux-trace-kernel, netdev
On 4/2/2023 4:45 PM, Simon Horman wrote:
> The arguments passed to the trace events are of type unsigned int,
> however the signature of the events used __le32 parameters.
>
> I may be missing the point here, but sparse flagged this and it
> does seem incorrect to me.
>
> net/qrtr/ns.c: note: in included file (through include/trace/trace_events.h, include/trace/define_trace.h, include/trace/events/qrtr.h):
> ./include/trace/events/qrtr.h:11:1: warning: cast to restricted __le32
> ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> ./include/trace/events/qrtr.h:11:1: warning: restricted __le32 degrades to integer
> ... (a lot more similar warnings)
> net/qrtr/ns.c:115:47: expected restricted __le32 [usertype] service
> net/qrtr/ns.c:115:47: got unsigned int service
> net/qrtr/ns.c:115:61: warning: incorrect type in argument 2 (different base types)
> ... (a lot more similar warnings)
>
> Signed-off-by: Simon Horman <horms@kernel.org>
> ---
> include/trace/events/qrtr.h | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/include/trace/events/qrtr.h b/include/trace/events/qrtr.h
> index b1de14c3bb93..441132c67133 100644
> --- a/include/trace/events/qrtr.h
> +++ b/include/trace/events/qrtr.h
> @@ -10,15 +10,16 @@
>
> TRACE_EVENT(qrtr_ns_service_announce_new,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
> @@ -36,15 +37,16 @@ TRACE_EVENT(qrtr_ns_service_announce_new,
>
> TRACE_EVENT(qrtr_ns_service_announce_del,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
> @@ -62,15 +64,16 @@ TRACE_EVENT(qrtr_ns_service_announce_del,
>
> TRACE_EVENT(qrtr_ns_server_add,
>
> - TP_PROTO(__le32 service, __le32 instance, __le32 node, __le32 port),
> + TP_PROTO(unsigned int service, unsigned int instance,
> + unsigned int node, unsigned int port),
>
> TP_ARGS(service, instance, node, port),
>
> TP_STRUCT__entry(
> - __field(__le32, service)
> - __field(__le32, instance)
> - __field(__le32, node)
> - __field(__le32, port)
> + __field(unsigned int, service)
> + __field(unsigned int, instance)
> + __field(unsigned int, node)
> + __field(unsigned int, port)
> ),
>
> TP_fast_assign(
LGTM.
Acked-by: Mukesh Ojha <quic_mojha@quicinc.com>
--Mukesh
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-04 8:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-02 11:15 [PATCH RFC] net: qrtr: correct types of trace event parameters Simon Horman
2023-04-03 5:14 ` Manivannan Sadhasivam
2023-04-03 13:10 ` Simon Horman
2023-04-04 8:20 ` Mukesh Ojha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).