| Message ID | 1550334537-380-3-git-send-email-laoar.shao@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | clean up SOCK_DEBUG() | expand |
On Sun, 2019-02-17 at 00:28 +0800, Yafang Shao wrote: > SOCK_DEBUG() is a old facility for debugging. > If the user want to use it for debugging, the user must modify the > application first, that doesn't seem like a good way. > Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind > of debugging purpose. > So we'd better disable it by default. > The reason why I don't remove it comepletely is that someone may still > would like to use it for debugging. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Suggested-by: Joe Perches <joe@perches.com> > --- > include/net/sock.h | 13 ++++++++----- > net/core/sock.c | 3 +++ > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/include/net/sock.h b/include/net/sock.h > index 6679f3c..d41e8f4 100644 > --- a/include/net/sock.h > +++ b/include/net/sock.h > @@ -81,14 +81,17 @@ > */ > > /* Define this to get the SOCK_DBG debugging facility. */ > -#define SOCK_DEBUGGING > +/* #define SOCK_DEBUGGING */ > #ifdef SOCK_DEBUGGING > -#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \ > - printk(KERN_DEBUG msg); } while (0) > +#define SOCK_DEBUG(sk, fmt, ...) \ > +do { \ > + if ((sk) && sock_flag((sk), SOCK_DBG)) \ > + pr_debug(fmt, ##__VA_ARGS__); \ trivia: I would not suggest pr_debug here as it also requires either DEBUG to be defined or CONFIG_DYNAMIC_DEBUG to be set. If you really set SOCK_DEBUGGING, then printk(KERN_DEBUG is probably right.
On Sun, Feb 17, 2019 at 7:58 PM Joe Perches <joe@perches.com> wrote: > > On Sun, 2019-02-17 at 00:28 +0800, Yafang Shao wrote: > > SOCK_DEBUG() is a old facility for debugging. > > If the user want to use it for debugging, the user must modify the > > application first, that doesn't seem like a good way. > > Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind > > of debugging purpose. > > So we'd better disable it by default. > > The reason why I don't remove it comepletely is that someone may still > > would like to use it for debugging. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > Suggested-by: Joe Perches <joe@perches.com> > > --- > > include/net/sock.h | 13 ++++++++----- > > net/core/sock.c | 3 +++ > > 2 files changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/include/net/sock.h b/include/net/sock.h > > index 6679f3c..d41e8f4 100644 > > --- a/include/net/sock.h > > +++ b/include/net/sock.h > > @@ -81,14 +81,17 @@ > > */ > > > > /* Define this to get the SOCK_DBG debugging facility. */ > > -#define SOCK_DEBUGGING > > +/* #define SOCK_DEBUGGING */ > > #ifdef SOCK_DEBUGGING > > -#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \ > > - printk(KERN_DEBUG msg); } while (0) > > +#define SOCK_DEBUG(sk, fmt, ...) \ > > +do { \ > > + if ((sk) && sock_flag((sk), SOCK_DBG)) \ > > + pr_debug(fmt, ##__VA_ARGS__); \ > > trivia: > > I would not suggest pr_debug here as it also requires > either DEBUG to be defined or CONFIG_DYNAMIC_DEBUG > to be set. > > If you really set SOCK_DEBUGGING, then printk(KERN_DEBUG > is probably right. > Sure. Will change it. Thanks Yafang
diff --git a/include/net/sock.h b/include/net/sock.h index 6679f3c..d41e8f4 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -81,14 +81,17 @@ */ /* Define this to get the SOCK_DBG debugging facility. */ -#define SOCK_DEBUGGING +/* #define SOCK_DEBUGGING */ #ifdef SOCK_DEBUGGING -#define SOCK_DEBUG(sk, msg...) do { if ((sk) && sock_flag((sk), SOCK_DBG)) \ - printk(KERN_DEBUG msg); } while (0) +#define SOCK_DEBUG(sk, fmt, ...) \ +do { \ + if ((sk) && sock_flag((sk), SOCK_DBG)) \ + pr_debug(fmt, ##__VA_ARGS__); \ +} while (0) #else /* Validate arguments and do nothing */ -static inline __printf(2, 3) -void SOCK_DEBUG(const struct sock *sk, const char *msg, ...) +__printf(2, 3) +static inline void SOCK_DEBUG(const struct sock *sk, const char *fmt, ...) { } #endif diff --git a/net/core/sock.c b/net/core/sock.c index 71ded4d..7c15835 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -753,6 +753,9 @@ int sock_setsockopt(struct socket *sock, int level, int optname, switch (optname) { case SO_DEBUG: + /* This option takes effect only when SOCK_DEBUGGING + * is defined. + */ if (val && !capable(CAP_NET_ADMIN)) ret = -EACCES; else
SOCK_DEBUG() is a old facility for debugging. If the user want to use it for debugging, the user must modify the application first, that doesn't seem like a good way. Now we have more powerful facilities, i.e. bpf or tracepoint, for this kind of debugging purpose. So we'd better disable it by default. The reason why I don't remove it comepletely is that someone may still would like to use it for debugging. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Suggested-by: Joe Perches <joe@perches.com> --- include/net/sock.h | 13 ++++++++----- net/core/sock.c | 3 +++ 2 files changed, 11 insertions(+), 5 deletions(-)