[net-next 0/5] clean up SOCK_DEBUG()

[net-next 0/5] clean up SOCK_DEBUG()

[Yafang Shao]

Per discussion with Daniel[1] and Eric[2], the SOCK_DEBUG() is a very
ancient interface, which is not very useful for debugging.
So we'd better clean up it.

After this patchset, the SO_DEBUG interface will not take any effect,
but I still keep it as-is in sock_{s,g}etsockopt() to avoid breaking
applications.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533

Yafang Shao (5):
  tcp: clean up SOCK_DEBUG()
  x25: clean up SOCK_DEBUG()
  appletalk: clean up SOCK_DEBUG()
  dccp: clean up SOCK_DEBUG()
  net: sock: remove the definition of SOCK_DEBUG()

 include/net/sock.h       | 19 -------------------
 net/appletalk/ddp.c      | 14 --------------
 net/core/sock.c          |  3 +++
 net/dccp/ipv6.c          |  2 --
 net/ipv4/tcp_input.c     | 19 +------------------
 net/ipv6/tcp_ipv6.c      |  2 --
 net/x25/af_x25.c         | 12 ------------
 net/x25/x25_facilities.c | 32 ++++++++++----------------------
 net/x25/x25_out.c        |  4 +---
 9 files changed, 15 insertions(+), 92 deletions(-)

-- 
1.8.3.1

[net-next 1/5] tcp: clean up SOCK_DEBUG()

[Yafang Shao]

Per discussion with Daniel[1] and Eric[2], the SOCK_DEBUG() is a very
ancient interface, which is not very useful for debugging.
So we'd better clean up it.

This patch cleans up it for TCP.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv4/tcp_input.c | 19 +------------------
 net/ipv6/tcp_ipv6.c  |  2 --
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7a027dec..6d2750e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3595,7 +3595,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 	 * this segment (RFC793 Section 3.9).
 	 */
 	if (after(ack, tp->snd_nxt))
-		goto invalid_ack;
+		return -1;
 
 	if (after(ack, prior_snd_una)) {
 		flag |= FLAG_SND_UNA_ADVANCED;
@@ -3714,10 +3714,6 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		tcp_process_tlp_ack(sk, ack, flag);
 	return 1;
 
-invalid_ack:
-	SOCK_DEBUG(sk, "Ack %u after %u:%u\n", ack, tp->snd_una, tp->snd_nxt);
-	return -1;
-
 old_ack:
 	/* If data was SACKed, tag it and see if we should send more data.
 	 * If data was DSACKed, see if we can undo a cwnd reduction.
@@ -3731,7 +3727,6 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
 		tcp_xmit_recovery(sk, rexmit);
 	}
 
-	SOCK_DEBUG(sk, "Ack %u before %u:%u\n", ack, tp->snd_una, tp->snd_nxt);
 	return 0;
 }
 
@@ -4432,13 +4427,9 @@ static void tcp_ofo_queue(struct sock *sk)
 		rb_erase(&skb->rbnode, &tp->out_of_order_queue);
 
 		if (unlikely(!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt))) {
-			SOCK_DEBUG(sk, "ofo packet was already received\n");
 			tcp_drop(sk, skb);
 			continue;
 		}
-		SOCK_DEBUG(sk, "ofo requeuing : rcv_next %X seq %X - %X\n",
-			   tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
-			   TCP_SKB_CB(skb)->end_seq);
 
 		tail = skb_peek_tail(&sk->sk_receive_queue);
 		eaten = tail && tcp_try_coalesce(sk, tail, skb, &fragstolen);
@@ -4502,8 +4493,6 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
 	seq = TCP_SKB_CB(skb)->seq;
 	end_seq = TCP_SKB_CB(skb)->end_seq;
-	SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
-		   tp->rcv_nxt, seq, end_seq);
 
 	p = &tp->out_of_order_queue.rb_node;
 	if (RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
@@ -4779,10 +4768,6 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 
 	if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
 		/* Partial packet, seq < rcv_next < end_seq */
-		SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",
-			   tp->rcv_nxt, TCP_SKB_CB(skb)->seq,
-			   TCP_SKB_CB(skb)->end_seq);
-
 		tcp_dsack_set(sk, TCP_SKB_CB(skb)->seq, tp->rcv_nxt);
 
 		/* If window is closed, drop tail of packet. But after
@@ -5061,8 +5046,6 @@ static int tcp_prune_queue(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	SOCK_DEBUG(sk, "prune_queue: c=%x\n", tp->copied_seq);
-
 	NET_INC_STATS(sock_net(sk), LINUX_MIB_PRUNECALLED);
 
 	if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index e51cda7..57ef69a1 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -220,8 +220,6 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 		u32 exthdrlen = icsk->icsk_ext_hdr_len;
 		struct sockaddr_in sin;
 
-		SOCK_DEBUG(sk, "connect: ipv4 mapped\n");
-
 		if (__ipv6_only_sock(sk))
 			return -ENETUNREACH;
 
-- 
1.8.3.1

[net-next 2/5] x25: clean up SOCK_DEBUG()

[Yafang Shao]

Per discussion with Daniel[1] and Eric[2], the SOCK_DEBUG() is a very
ancient interface, which is not very useful for debugging.
So we'd better clean up it.

This patch cleans up it for x25.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/x25/af_x25.c         | 12 ------------
 net/x25/x25_facilities.c | 32 ++++++++++----------------------
 net/x25/x25_out.c        |  4 +---
 3 files changed, 11 insertions(+), 37 deletions(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 5121729..5892d05 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -705,7 +705,6 @@ static int x25_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	x25_insert_socket(sk);
 	sock_reset_flag(sk, SOCK_ZAPPED);
 	release_sock(sk);
-	SOCK_DEBUG(sk, "x25_bind: socket is bound\n");
 out:
 	return rc;
 }
@@ -1148,11 +1147,7 @@ static int x25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 		goto out;
 	}
 
-	SOCK_DEBUG(sk, "x25_sendmsg: sendto: Addresses built.\n");
-
 	/* Build a packet */
-	SOCK_DEBUG(sk, "x25_sendmsg: sendto: building packet.\n");
-
 	if ((msg->msg_flags & MSG_OOB) && len > 32)
 		len = 32;
 
@@ -1170,8 +1165,6 @@ static int x25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	/*
 	 *	Put the data on the end
 	 */
-	SOCK_DEBUG(sk, "x25_sendmsg: Copying user data\n");
-
 	skb_reset_transport_header(skb);
 	skb_put(skb, len);
 
@@ -1194,8 +1187,6 @@ static int x25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	/*
 	 *	Push down the X.25 header
 	 */
-	SOCK_DEBUG(sk, "x25_sendmsg: Building X.25 Header.\n");
-
 	if (msg->msg_flags & MSG_OOB) {
 		if (x25->neighbour->extended) {
 			asmptr    = skb_push(skb, X25_STD_MIN_LEN);
@@ -1228,9 +1219,6 @@ static int x25_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 			skb->data[0] |= X25_Q_BIT;
 	}
 
-	SOCK_DEBUG(sk, "x25_sendmsg: Built header.\n");
-	SOCK_DEBUG(sk, "x25_sendmsg: Transmitting buffer\n");
-
 	rc = -ENOTCONN;
 	if (sk->sk_state != TCP_ESTABLISHED)
 		goto out_kfree_skb;
diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index ad1734d..74a5284 100644
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -286,10 +286,8 @@ int x25_negotiate_facilities(struct sk_buff *skb, struct sock *sk,
 	/*
 	 *	They want reverse charging, we won't accept it.
 	 */
-	if ((theirs.reverse & 0x01 ) && (ours->reverse & 0x01)) {
-		SOCK_DEBUG(sk, "X.25: rejecting reverse charging request\n");
+	if ((theirs.reverse & 0x01) && (ours->reverse & 0x01))
 		return -1;
-	}
 
 	new->reverse = theirs.reverse;
 
@@ -298,37 +296,27 @@ int x25_negotiate_facilities(struct sk_buff *skb, struct sock *sk,
 		int theirs_out = theirs.throughput & 0xf0;
 		int ours_in  = ours->throughput & 0x0f;
 		int ours_out = ours->throughput & 0xf0;
-		if (!ours_in || theirs_in < ours_in) {
-			SOCK_DEBUG(sk, "X.25: inbound throughput negotiated\n");
+		if (!ours_in || theirs_in < ours_in)
 			new->throughput = (new->throughput & 0xf0) | theirs_in;
-		}
-		if (!ours_out || theirs_out < ours_out) {
-			SOCK_DEBUG(sk,
-				"X.25: outbound throughput negotiated\n");
+
+		if (!ours_out || theirs_out < ours_out)
 			new->throughput = (new->throughput & 0x0f) | theirs_out;
-		}
 	}
 
 	if (theirs.pacsize_in && theirs.pacsize_out) {
-		if (theirs.pacsize_in < ours->pacsize_in) {
-			SOCK_DEBUG(sk, "X.25: packet size inwards negotiated down\n");
+		if (theirs.pacsize_in < ours->pacsize_in)
 			new->pacsize_in = theirs.pacsize_in;
-		}
-		if (theirs.pacsize_out < ours->pacsize_out) {
-			SOCK_DEBUG(sk, "X.25: packet size outwards negotiated down\n");
+
+		if (theirs.pacsize_out < ours->pacsize_out)
 			new->pacsize_out = theirs.pacsize_out;
-		}
 	}
 
 	if (theirs.winsize_in && theirs.winsize_out) {
-		if (theirs.winsize_in < ours->winsize_in) {
-			SOCK_DEBUG(sk, "X.25: window size inwards negotiated down\n");
+		if (theirs.winsize_in < ours->winsize_in)
 			new->winsize_in = theirs.winsize_in;
-		}
-		if (theirs.winsize_out < ours->winsize_out) {
-			SOCK_DEBUG(sk, "X.25: window size outwards negotiated down\n");
+
+		if (theirs.winsize_out < ours->winsize_out)
 			new->winsize_out = theirs.winsize_out;
-		}
 	}
 
 	return len;
diff --git a/net/x25/x25_out.c b/net/x25/x25_out.c
index 0144271..2a74f26 100644
--- a/net/x25/x25_out.c
+++ b/net/x25/x25_out.c
@@ -77,9 +77,7 @@ int x25_output(struct sock *sk, struct sk_buff *skb)
 					kfree_skb(skb);
 					return sent;
 				}
-				SOCK_DEBUG(sk, "x25_output: fragment alloc"
-					       " failed, err=%d, %d bytes "
-					       "sent\n", err, sent);
+
 				return err;
 			}
 
-- 
1.8.3.1

[net-next 3/5] appletalk: clean up SOCK_DEBUG()

[Yafang Shao]

Per discussion with Daniel[1] and Eric[2], the SOCK_DEBUG() is a very
ancient interface, which is not very useful for debugging.
So we'd better clean up it.

This patch cleans up it for appletalk.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/appletalk/ddp.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 9b6bc5a..326c4fd 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1608,8 +1608,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	}
 
 	/* Build a packet */
-	SOCK_DEBUG(sk, "SK %p: Got address.\n", sk);
-
 	/* For headers */
 	size = sizeof(struct ddpehdr) + len + ddp_dl->header_length;
 
@@ -1628,10 +1626,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 		goto out;
 
 	dev = rt->dev;
-
-	SOCK_DEBUG(sk, "SK %p: Size needed %d, device %s\n",
-			sk, size, dev->name);
-
 	size += dev->hard_header_len;
 	release_sock(sk);
 	skb = sock_alloc_send_skb(sk, size, (flags & MSG_DONTWAIT), &err);
@@ -1643,8 +1637,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	skb_reserve(skb, dev->hard_header_len);
 	skb->dev = dev;
 
-	SOCK_DEBUG(sk, "SK %p: Begin build.\n", sk);
-
 	ddp = skb_put(skb, sizeof(struct ddpehdr));
 	ddp->deh_len_hops  = htons(len + sizeof(*ddp));
 	ddp->deh_dnet  = usat->sat_addr.s_net;
@@ -1654,8 +1646,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	ddp->deh_dport = usat->sat_port;
 	ddp->deh_sport = at->src_port;
 
-	SOCK_DEBUG(sk, "SK %p: Copy user data (%zd bytes).\n", sk, len);
-
 	err = memcpy_from_msg(skb_put(skb, len), msg, len);
 	if (err) {
 		kfree_skb(skb);
@@ -1678,7 +1668,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 
 		if (skb2) {
 			loopback = 1;
-			SOCK_DEBUG(sk, "SK %p: send out(copy).\n", sk);
 			/*
 			 * If it fails it is queued/sent above in the aarp queue
 			 */
@@ -1687,7 +1676,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	}
 
 	if (dev->flags & IFF_LOOPBACK || loopback) {
-		SOCK_DEBUG(sk, "SK %p: Loop back.\n", sk);
 		/* loop back */
 		skb_orphan(skb);
 		if (ddp->deh_dnode == ATADDR_BCAST) {
@@ -1707,7 +1695,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 		}
 		ddp_dl->request(ddp_dl, skb, dev->dev_addr);
 	} else {
-		SOCK_DEBUG(sk, "SK %p: send out.\n", sk);
 		if (rt->flags & RTF_GATEWAY) {
 		    gsat.sat_addr = rt->gateway;
 		    usat = &gsat;
@@ -1718,7 +1705,6 @@ static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 		 */
 		aarp_send_ddp(dev, skb, &usat->sat_addr, NULL);
 	}
-	SOCK_DEBUG(sk, "SK %p: Done write (%zd).\n", sk, len);
 
 out:
 	release_sock(sk);
-- 
1.8.3.1

[net-next 4/5] dccp: clean up SOCK_DEBUG()

[Yafang Shao]

Per discussion with Daniel[1] and Eric[2], the SOCK_DEBUG() is a very
ancient interface, which is not very useful for debugging.
So we'd better clean up it.

This patch cleans up it for dccp.

[1] https://patchwork.ozlabs.org/patch/1035573/
[2] https://patchwork.ozlabs.org/patch/1040533/

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/dccp/ipv6.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index d5740ba..8e72e50 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -878,8 +878,6 @@ static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
 		u32 exthdrlen = icsk->icsk_ext_hdr_len;
 		struct sockaddr_in sin;
 
-		SOCK_DEBUG(sk, "connect: ipv4 mapped\n");
-
 		if (__ipv6_only_sock(sk))
 			return -ENETUNREACH;
 
-- 
1.8.3.1

[net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Yafang Shao]

As SOCK_DEBUG() isn't used any more, we can get ride of it now.

Regarding the SO_DEBUG in sock_{s,g}etsockopt, I think it is better to
keep as-is, because if we return an errno to tell the application that
this optname isn't supported, it may break the application.
The application still can use this option but don't take any effect from
now on.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/net/sock.h | 19 -------------------
 net/core/sock.c    |  3 +++
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 6679f3c..4c6b599 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -74,25 +74,6 @@
 #include <net/smc.h>
 #include <net/l3mdev.h>
 
-/*
- * This structure really needs to be cleaned up.
- * Most of it is for TCP, and not used by any of
- * the other protocols.
- */
-
-/* Define this to get the SOCK_DBG debugging facility. */
-#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)
-#else
-/* Validate arguments and do nothing */
-static inline __printf(2, 3)
-void SOCK_DEBUG(const struct sock *sk, const char *msg, ...)
-{
-}
-#endif
-
 /* This is the per-socket lock.  The spinlock provides a synchronization
  * between user contexts and software interrupt processing, whereas the
  * mini-semaphore synchronizes multiple users amongst themselves.
diff --git a/net/core/sock.c b/net/core/sock.c
index 71ded4d..a980ff3 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 doesn't take effect now,
+		 * but we need to keep it for backward compatibility.
+		 */
 		if (val && !capable(CAP_NET_ADMIN))
 			ret = -EACCES;
 		else
-- 
1.8.3.1

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Eric Dumazet]

On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> As SOCK_DEBUG() isn't used any more, we can get ride of it now.
>

No, we are still using this infrastructure from time to time.

I told you I agreed to remove the current (obsolete) TCP call sites,
 I never suggested to remove SOCK_DEBUG() completely.

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Yafang Shao]

On Fri, Feb 15, 2019 at 10:58 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> >
>
> No, we are still using this infrastructure from time to time.
>
> I told you I agreed to remove the current (obsolete) TCP call sites,
>  I never suggested to remove SOCK_DEBUG() completely.

All right, however I'm not persuaded by you.
Because SOCK_DEBUG() is not a smart way for debugging, which requires
the user code to be modified.
We have other more flexible methords.


Thanks
Yafang

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Eric Dumazet]

On Fri, Feb 15, 2019 at 7:42 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Fri, Feb 15, 2019 at 10:58 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> > >
> >
> > No, we are still using this infrastructure from time to time.
> >
> > I told you I agreed to remove the current (obsolete) TCP call sites,
> >  I never suggested to remove SOCK_DEBUG() completely.
>
> All right, however I'm not persuaded by you.
> Because SOCK_DEBUG() is not a smart way for debugging, which requires
> the user code to be modified.
> We have other more flexible methords.

That might be the case, but only if you get enough privileges and skills.
I fail to see why we should enforce anything here.
It costs nothing having this interface kept.

In any case, we do not deprecate features without a
Documentation/ABI/obsolete/*  file
and quarantine.

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Cong Wang]

On Fri, Feb 15, 2019 at 8:26 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> >
>
> No, we are still using this infrastructure from time to time.
>
> I told you I agreed to remove the current (obsolete) TCP call sites,
>  I never suggested to remove SOCK_DEBUG() completely.

Since when do we upstream care about any out-of-tree users?

You can always carry a patch to keep it downstream if you want,
no one can stop you doing it.

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Eric Dumazet]

On Fri, Feb 15, 2019 at 10:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Fri, Feb 15, 2019 at 8:26 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > >
> > > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> > >
> >
> > No, we are still using this infrastructure from time to time.
> >
> > I told you I agreed to remove the current (obsolete) TCP call sites,
> >  I never suggested to remove SOCK_DEBUG() completely.
>
> Since when do we upstream care about any out-of-tree users?
>

> You can always carry a patch to keep it downstream if you want,
> no one can stop you doing it.

Somehow the patch series seems to present things in this way :

Eric Dumazet suggested to remove completely the SOCK_DEBUG() interface.

I did not.

I sometimes am lazy and use SOCK_DEBUG() myself, so I would like we keep it.

I know that others are doing the same thing, so I do not feel any shame.

Feel free to ignore my feedback.

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Joe Perches]

On Fri, 2019-02-15 at 10:22 -0800, Eric Dumazet wrote:
> On Fri, Feb 15, 2019 at 10:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > On Fri, Feb 15, 2019 at 8:26 AM Eric Dumazet <edumazet@google.com> wrote:
> > > On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> > > > 
> > > 
> > > No, we are still using this infrastructure from time to time.
> > > 
> > > I told you I agreed to remove the current (obsolete) TCP call sites,
> > >  I never suggested to remove SOCK_DEBUG() completely.
> > 
> > Since when do we upstream care about any out-of-tree users?
> > 
> > You can always carry a patch to keep it downstream if you want,
> > no one can stop you doing it.
> 
> Somehow the patch series seems to present things in this way :
> 
> Eric Dumazet suggested to remove completely the SOCK_DEBUG() interface.

Well, you kinda did.
It's certainly reasonable to interpret what you wrote as such.

On Tue, 2019-02-12 at 18:15 -0800, Eric Dumazet wrote:
> Just remove all SOCK_DEBUG() calls, there are leftovers of very ancient times.

My suggestion would be to undefine SOCK_DEBUGGING.

Something like:
---
 include/net/sock.h | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 328cb7cb7b0b..7e39bdfa342a 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))			\
+		printk(KERN_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

Re: [net-next 5/5] net: sock: remove the definition of SOCK_DEBUG()

[Yafang Shao]

On Sat, Feb 16, 2019 at 2:51 AM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2019-02-15 at 10:22 -0800, Eric Dumazet wrote:
> > On Fri, Feb 15, 2019 at 10:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > On Fri, Feb 15, 2019 at 8:26 AM Eric Dumazet <edumazet@google.com> wrote:
> > > > On Fri, Feb 15, 2019 at 6:50 AM Yafang Shao <laoar.shao@gmail.com> wrote:
> > > > > As SOCK_DEBUG() isn't used any more, we can get ride of it now.
> > > > >
> > > >
> > > > No, we are still using this infrastructure from time to time.
> > > >
> > > > I told you I agreed to remove the current (obsolete) TCP call sites,
> > > >  I never suggested to remove SOCK_DEBUG() completely.
> > >
> > > Since when do we upstream care about any out-of-tree users?
> > >
> > > You can always carry a patch to keep it downstream if you want,
> > > no one can stop you doing it.
> >
> > Somehow the patch series seems to present things in this way :
> >
> > Eric Dumazet suggested to remove completely the SOCK_DEBUG() interface.
>
> Well, you kinda did.
> It's certainly reasonable to interpret what you wrote as such.
>
> On Tue, 2019-02-12 at 18:15 -0800, Eric Dumazet wrote:
> > Just remove all SOCK_DEBUG() calls, there are leftovers of very ancient times.
>
> My suggestion would be to undefine SOCK_DEBUGGING.
>

This seems like a reasonable trade-off decision.
I will change it like this.

> Something like:
> ---
>  include/net/sock.h | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 328cb7cb7b0b..7e39bdfa342a 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))                  \
> +               printk(KERN_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
>
>

Thanks
Yafang