That looked like a good suggestion and will make the code cleaner but
with that change ran into this unexpected build error. Ideas?
CC [M] /home/smfrench/cifs-2.6/fs/cifs/file.o
In file included from ./include/trace/define_trace.h:102,
from /home/smfrench/cifs-2.6/fs/cifs/trace.h:977,
from /home/smfrench/cifs-2.6/fs/cifs/trace.c:8:
./include/linux/socket.h:42:26: error: conversion to non-scalar type requested
42 | #define sockaddr_storage __kernel_sockaddr_storage
| ^~~~~~~~~~~~~~~~~~~~~~~~~
./include/trace/trace_events.h:477:9: note: in definition of macro
‘DECLARE_EVENT_CLASS’
477 | tstruct
\
| ^~~~~~~
/home/smfrench/cifs-2.6/fs/cifs/./trace.h:864:9: note: in expansion of
macro ‘TP_STRUCT__entry’
864 | TP_STRUCT__entry(
| ^~~~~~~~~~~~~~~~
./include/trace/trace_events.h:439:22: note: in expansion of macro
‘is_signed_type’
439 | .is_signed = is_signed_type(_type), .filter_type =
_filter_type },
| ^~~~~~~~~~~~~~
./include/trace/trace_events.h:448:33: note: in expansion of macro ‘__field_ext’
448 | #define __field(type, item) __field_ext(type, item, FILTER_OTHER)
| ^~~~~~~~~~~
/home/smfrench/cifs-2.6/fs/cifs/./trace.h:867:17: note: in expansion
of macro ‘__field’
867 | __field(struct sockaddr_storage, dst_addr)
| ^~~~~~~
/home/smfrench/cifs-2.6/fs/cifs/./trace.h:867:32: note: in expansion
of macro ‘sockaddr_storage’
867 | __field(struct sockaddr_storage, dst_addr)
On Thu, Nov 4, 2021 at 2:14 AM Stefan Metzmacher <metze@xxxxxxxxx> wrote:
>
> Hi Steve,
>
> you should made this generic (not ipv4/ipv6 specific) and use something like this:
>
> TP_PROTO(const char *hostname, __u64 conn_id, const struct sockaddr_storage *dst_addr)
>
> TP_STRUCT__entry(
> __string(hostname, hostname)
> __field(__u64, conn_id)
> __field(struct sockaddr_storage, dst_addr)
>
> TP_fast_assign(
> __entry->conn_id = conn_id;
> __entry->dst_addr = *dst_addr;
> __assign_str(hostname, hostname);
> ),
>
> With that you should be able to use this:
>
> TP_printk("conn_id=0x%llx server=%s addr=%pISpsfc",
> __entry->conn_id,
> __get_str(hostname),
> &__entry->dst_addr)
>
> I hope that helps.
>
> metze
>
> Am 04.11.21 um 07:09 schrieb Steve French:
> > It wasn't obvious to me the best way to pass in a pointer to the ipv4
> > (and ipv6 address) to a dynamic trace point (unless I create a temp
> > string first in generic_ip_connect and do the conversion (via "%pI4"
> > and "%pI6" with sprintf) e.g.
> >
> > sprintf(ses->ip_addr, "%pI4", &addr->sin_addr);
> >
> > The approach I tried passing in the pointer to sin_addr (the
> > ipv4_address) caused an oops on loading it the first time and the
> > warning:
> >
> > [14928.818532] event smb3_ipv4_connect has unsafe dereference of argument 3
> > [14928.818534] print_fmt: "conn_id=0x%llx server=%s addr=%pI4:%d",
> > REC->conn_id, __get_str(hostname), REC->ipaddr, REC->port
> >
> >
> > What I tried was the following (also see attached diff) to print the
> > ipv4 address that we were trying to connect to
> >
> > DECLARE_EVENT_CLASS(smb3_connect_class,
> > TP_PROTO(char *hostname,
> > __u64 conn_id,
> > __u16 port,
> > struct in_addr *ipaddr),
> > TP_ARGS(hostname, conn_id, port, ipaddr),
> > TP_STRUCT__entry(
> > __string(hostname, hostname)
> > __field(__u64, conn_id)
> > __field(__u16, port)
> > __field(const void *, ipaddr)
> > ),
> > TP_fast_assign(
> > __entry->port = port;
> > __entry->conn_id = conn_id;
> > __entry->ipaddr = ipaddr;
> > __assign_str(hostname, hostname);
> > ),
> > TP_printk("conn_id=0x%llx server=%s addr=%pI4:%d",
> > __entry->conn_id,
> > __get_str(hostname),
> > __entry->ipaddr,
> > __entry->port)
> > )
> >
> > #define DEFINE_SMB3_CONNECT_EVENT(name) \
> > DEFINE_EVENT(smb3_connect_class, smb3_##name, \
> > TP_PROTO(char *hostname, \
> > __u64 conn_id, \
> > __u16 port, \
> > struct in_addr *ipaddr), \
> > TP_ARGS(hostname, conn_id, port, ipaddr))
> >
> > DEFINE_SMB3_CONNECT_EVENT(ipv4_connect);
> >
> > Any ideas how to pass in the ipv4 address - or is it better to convert
> > it to a string before we call the trace point (which seems wasteful to
> > me but there must be examples of how to pass in structs to printks in
> > trace in Linux)
> >
>
--
Thanks,
Steve
Attachment:
build-fail-out
Description: Binary data
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index e6e261dfd107..fd96de59b968 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2592,6 +2592,8 @@ generic_ip_connect(struct TCP_Server_Info *server)
ntohs(sport));
}
+ trace_smb3_connect(server->hostname, server->conn_id, &server->dstaddr);
+
if (socket == NULL) {
rc = __sock_create(cifs_net_ns(server), sfamily, SOCK_STREAM,
IPPROTO_TCP, &socket, 1);
diff --git a/fs/cifs/trace.h b/fs/cifs/trace.h
index dafcb6ab050d..a20ece00733f 100644
--- a/fs/cifs/trace.h
+++ b/fs/cifs/trace.h
@@ -11,6 +11,7 @@
#define _CIFS_TRACE_H
#include <linux/tracepoint.h>
+#include <linux/inet.h>
/*
* Please use this 3-part article as a reference for writing new tracepoints:
@@ -854,6 +855,36 @@ DEFINE_EVENT(smb3_lease_err_class, smb3_##name, \
DEFINE_SMB3_LEASE_ERR_EVENT(lease_err);
+DECLARE_EVENT_CLASS(smb3_connect_class,
+ TP_PROTO(char *hostname,
+ __u64 conn_id,
+ const struct sockaddr_storage *dst_addr),
+ TP_ARGS(hostname, conn_id, dst_addr),
+ TP_STRUCT__entry(
+ __string(hostname, hostname)
+ __field(__u64, conn_id)
+ __field(struct sockaddr_storage, dst_addr)
+ ),
+ TP_fast_assign(
+ __entry->conn_id = conn_id;
+ __entry->dst_addr = *dst_addr;
+ __assign_str(hostname, hostname);
+ ),
+ TP_printk("conn_id=0x%llx server=%s addr=%pISpsfc",
+ __entry->conn_id,
+ __get_str(hostname),
+ &__entry->dst_addr)
+)
+
+#define DEFINE_SMB3_CONNECT_EVENT(name) \
+DEFINE_EVENT(smb3_connect_class, smb3_##name, \
+ TP_PROTO(char *hostname, \
+ __u64 conn_id, \
+ const struct sockaddr_storage *addr), \
+ TP_ARGS(hostname, conn_id, addr))
+
+DEFINE_SMB3_CONNECT_EVENT(connect);
+
DECLARE_EVENT_CLASS(smb3_reconnect_class,
TP_PROTO(__u64 currmid,
__u64 conn_id,
