Skip to contentSkip to conversation

AX25 packet routing improvement

PATCH In review f6bvp 4 participants 10 messages Dec 2, 2010 – Feb 15, 2011

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

f6bvp

author
#0 lore raw

PATCH 1/1 AX25 packet routing improvement

Hi,

Sorry for this second post.
I found that spin_lock_bh was missing in the first loop !

[PATCH] [ROSE] rose AX25 packet routing improvement

FPAC AX25 packet application is using Linux kernel ROSE routing skills in order to connect or send packets to remote stations knowing their ROSE address via a network of interconnected nodes.

Each FPAC node has a ROSE routing table that Linux ROSE module is looking at each time a ROSE frame is relayed by the node or when a connect request to a neighbor node is received.

A previous patch improved the system time response by looking at already established routes each time the system was looking for a route to relay a frame. If a neighbor node routing the destination address was already connected, then the frame would be sent through him. If not, a connection request would be issued.

The present patch extends the same routing capability to a connect request asked by a user locally connected into an FPAC node. Without this patch, a connect request was not well handled unless it was directed to an immediate connected neighbor of the local node.

Implemented at a number of ROSE FPAC node stations, the present patch improved dramatically FPAC ROSE routing time response and efficiency.

Signed-off-by Bernard Pidoux <[email protected]>
net/rose/rose_route.c +16 -12
@@ -672,29 +672,34 @@
  *	Find a neighbour or a route given a ROSE address.
  */
 struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
-	unsigned char *diagnostic, int new)
+	unsigned char *diagnostic, int route_frame)
 {
 	struct rose_neigh *res = NULL;
 	struct rose_node *node;
 	int failed = 0;
 	int i;
 
-	if (!new) spin_lock_bh(&rose_node_list_lock);
+	if (!route_frame) spin_lock_bh(&rose_node_list_lock);
 	for (node = rose_node_list; node != NULL; node = node->next) {
 		if (rosecmpm(addr, &node->address, node->mask) == 0) {
 			for (i = 0; i < node->count; i++) {
-				if (new) {
-					if (node->neighbour[i]->restarted) {
-						res = node->neighbour[i];
-						goto out;
-					}
+				if (node->neighbour[i]->restarted) {
+					res = node->neighbour[i];
+					goto out;
 				}
-				else {
+			}
+		}
+	}
+	if (!route_frame) { /* connect request */
+		for (node = rose_node_list; node != NULL; node = node->next) {
+			if (rosecmpm(addr, &node->address, node->mask) == 0) {
+				for (i = 0; i < node->count; i++) {
 					if (!rose_ftimer_running(node->neighbour[i])) {
 						res = node->neighbour[i];
+						failed = 0;
 						goto out;
-					} else
-						failed = 1;
+					}
+					failed = 1;
 				}
 			}
 		}

@@ -709,8 +714,7 @@
 	}
 
 out:
-	if (!new) spin_unlock_bh(&rose_node_list_lock);
-
+	if (!route_frame) spin_unlock_bh(&rose_node_list_lock);
 	return res;
 }
author
#1 lore raw

Hi Ralf,

Here is a new patch that completes the one I sent recently.

I hope you will find time to have a look at both patches and approve them.

----------

[PATCH] [ROSE] [AX25] finding routes simplification

With previous patch, rose_get_neigh() routine investigates the full list of neighbor nodes until it finds or not an already connected node whether it is called locally or through a level 3 transit frame. If no routes are opened through an adjacent connected node then a classical connect request is attempted.

Then there is no more reason for an extra loop such as the one removed by this patch.

Signed-off-by Bernard Pidoux <[email protected]>
Quoted 22 lines · Le 02/12/2010 13:05, f6bvp a écrit :

Hi,

Sorry for this second post.
I found that spin_lock_bh was missing in the first loop !

[PATCH] [ROSE] rose AX25 packet routing improvement

FPAC AX25 packet application is using Linux kernel ROSE routing skills in order to connect or send packets to remote stations knowing their ROSE address via a network of interconnected nodes.

Each FPAC node has a ROSE routing table that Linux ROSE module is looking at each time a ROSE frame is relayed by the node or when a connect request to a neighbor node is received.

A previous patch improved the system time response by looking at already established routes each time the system was looking for a route to relay a frame. If a neighbor node routing the destination address was already connected, then the frame would be sent through him. If not, a connection request would be issued.

Quoted 10 lines

The present patch extends the same routing capability to a connect request asked by a user locally connected into an FPAC node. Without this patch, a connect request was not well handled unless it was directed to an immediate connected neighbor of the local node.

Implemented at a number of ROSE FPAC node stations, the present patch improved dramatically FPAC ROSE routing time response and efficiency.

Signed-off-by Bernard Pidoux <[email protected]>
net/rose/af_rose.c +0 -7
@@ -803,7 +803,6 @@
 
 		rose_insert_socket(sk);		/* Finish the bind */
 	}
-rose_try_next_neigh:
 	rose->dest_addr   = addr->srose_addr;
 	rose->dest_call   = addr->srose_call;
 	rose->rand        = ((long)rose & 0xFFFF) + rose->lci;

@@ -865,12 +864,6 @@
 	}
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
-	/* Try next neighbour */
-		rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic, 0);
-		if (rose->neighbour)
-			goto rose_try_next_neigh;
-
-		/* No more neighbours */
 		sock->state = SS_UNCONNECTED;
 		err = sock_error(sk);	/* Always set at this point */
 		goto out_release;
author
#2 lore raw

Hi Ralf,

One month ago I sent two patches for ROSE aimed at improving rose network routing, especially when a user sends a connect request on a local node.

They have been implemented on a dozen of ROSE FPAC nodes in the United States and other countries in the world and seem to be quite efficient and cause no problem.

As I have no feedback since, I am a little bit puzzled. Could you tell us if you have any restrictive opinion about applying these patches ?

73 de Bernard, f6bvp

Quoted 18 lines · Le 20/12/2010 19:11, f6bvp a écrit :

Hi Ralf,

Here is a new patch that completes the one I sent recently.

I hope you will find time to have a look at both patches and approve them.

----------

[PATCH] [ROSE] [AX25] finding routes simplification

With previous patch, rose_get_neigh() routine investigates the full list of neighbor nodes until it finds or not an already connected node whether it is called locally or through a level 3 transit frame. If no routes are opened through an adjacent connected node then a classical connect request is attempted.

Quoted 41 lines

Then there is no more reason for an extra loop such as the one removed by this patch.

Signed-off-by Bernard Pidoux <[email protected]>
Quoted 32 lines · Le 02/12/2010 13:05, f6bvp a écrit :

Hi,

Sorry for this second post.
I found that spin_lock_bh was missing in the first loop !

[PATCH] [ROSE] rose AX25 packet routing improvement

FPAC AX25 packet application is using Linux kernel ROSE routing skills in order to connect or send packets to remote stations knowing their ROSE address via a network of interconnected nodes.

Each FPAC node has a ROSE routing table that Linux ROSE module is looking at each time a ROSE frame is relayed by the node or when a connect request to a neighbor node is received.

A previous patch improved the system time response by looking at already established routes each time the system was looking for a route to relay a frame. If a neighbor node routing the destination address was already connected, then the frame would be sent through him. If not, a connection request would be issued.

The present patch extends the same routing capability to a connect request asked by a user locally connected into an FPAC node. Without this patch, a connect request was not well handled unless it was directed to an immediate connected neighbor of the local node.

Implemented at a number of ROSE FPAC node stations, the present patch improved dramatically FPAC ROSE routing time response and efficiency.

Signed-off-by Bernard Pidoux <[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

Can AX25 socket debug message be removed ?

In reply to f6bvp

Hi Ralf,

I would like to know your thinking about removing the following kernel debug messages that fills /var/log/kernel/info and does not seem to be actually usefull.

I build a patch project included here.
In case you approve I will submit it.
Should the SOCK_DEBUG lines be commented or removed ?

Bernard Pidoux <[email protected]>

net/ax25/af_ax25.c +5 -5
@@ -1538,7 +1538,7 @@
 	}
 
 	/* Build a packet */
-	SOCK_DEBUG(sk, "AX.25: sendto: Addresses built. Building packet.\n");
+/*	SOCK_DEBUG(sk, "AX.25: sendto: Addresses built. Building packet.\n");*/
 
 	/* Assume the worst case */
 	size = len + ax25->ax25_dev->dev->hard_header_len;

@@ -1549,7 +1549,7 @@
 
 	skb_reserve(skb, size - len);
 
-	SOCK_DEBUG(sk, "AX.25: Appending user data\n");
+/*	SOCK_DEBUG(sk, "AX.25: Appending user data\n");*/
 
 	/* User data follows immediately after the AX.25 data */
 	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {

@@ -1564,7 +1564,7 @@
 	if (!ax25->pidincl)
 		*skb_push(skb, 1) = sk->sk_protocol;
 
-	SOCK_DEBUG(sk, "AX.25: Transmitting buffer\n");
+/*	SOCK_DEBUG(sk, "AX.25: Transmitting buffer\n");*/
 
 	if (sk->sk_type == SOCK_SEQPACKET) {
 		/* Connected mode sockets go via the LAPB machine */

@@ -1583,7 +1583,7 @@
 
 	skb_push(skb, 1 + ax25_addr_size(dp));
 
-	SOCK_DEBUG(sk, "Building AX.25 Header (dp=%p).\n", dp);
+/*	SOCK_DEBUG(sk, "Building AX.25 Header (dp=%p).\n", dp);*/
 
 	if (dp != NULL)
 		SOCK_DEBUG(sk, "Num digipeaters=%d\n", dp->ndigi);

@@ -1592,7 +1592,7 @@
 	lv = ax25_addr_build(skb->data, &ax25->source_addr, &sax.sax25_call,
 			     dp, AX25_COMMAND, AX25_MODULUS);
 
-	SOCK_DEBUG(sk, "Built header (%d bytes)\n",lv);
+/*	SOCK_DEBUG(sk, "Built header (%d bytes)\n",lv);*/
 
 	skb_set_transport_header(skb, lv);
#4 lore raw

Re: Can AX25 socket debug message be removed ?

Quoted 7 lines · On Tue, Dec 21, 2010 at 11:38:25AM +0100, f6bvp wrote:

I would like to know your thinking about removing the following kernel debug messages that fills /var/log/kernel/info and does not seem to be actually usefull.

I build a patch project included here.
In case you approve I will submit it.
Should the SOCK_DEBUG lines be commented or removed ?

The messages are only printed for sockets that have the SOCK_DBG flag set so you may want to audit the application code for use of this flag.

Either way - the messages were there when I took maintainership over the code and I don't know of anybody using them since so I don't mind removing them. Imho they stopped making sense ages ago.

Remove rubbish, don't comment it out unless it's meant to disactivate code for a short time only. For other code that needs to be restored there is the git repository.

So how about below more radical patch that also deals with NET\ROM?

Ralf

Signed-off-by Ralf Baechle <[email protected]>
 net/ax25/af_ax25.c     |   16 +---------------
 net/netrom/af_netrom.c |   12 +-----------
 net/rose/af_rose.c     |   16 ++--------------
 3 files changed, 4 insertions(+), 40 deletions(-)
net/ax25/af_ax25.c +1 -15
index bb86d29..64d2832 100644
@@ -1538,8 +1538,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 	}
 
 	/* Build a packet */
-	SOCK_DEBUG(sk, "AX.25: sendto: Addresses built. Building packet.\n");
-
 	/* Assume the worst case */
 	size = len + ax25->ax25_dev->dev->hard_header_len;
 

@@ -1549,8 +1547,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	skb_reserve(skb, size - len);
 
-	SOCK_DEBUG(sk, "AX.25: Appending user data\n");
-
 	/* User data follows immediately after the AX.25 data */
 	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
 		err = -EFAULT;

@@ -1564,8 +1560,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 	if (!ax25->pidincl)
 		*skb_push(skb, 1) = sk->sk_protocol;
 
-	SOCK_DEBUG(sk, "AX.25: Transmitting buffer\n");
-
 	if (sk->sk_type == SOCK_SEQPACKET) {
 		/* Connected mode sockets go via the LAPB machine */
 		if (sk->sk_state != TCP_ESTABLISHED) {

@@ -1583,22 +1577,14 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	skb_push(skb, 1 + ax25_addr_size(dp));
 
-	SOCK_DEBUG(sk, "Building AX.25 Header (dp=%p).\n", dp);
-
-	if (dp != NULL)
-		SOCK_DEBUG(sk, "Num digipeaters=%d\n", dp->ndigi);
+	/* Building AX.25 Header */
 
 	/* Build an AX.25 header */
 	lv = ax25_addr_build(skb->data, &ax25->source_addr, &sax.sax25_call,
 			     dp, AX25_COMMAND, AX25_MODULUS);
 
-	SOCK_DEBUG(sk, "Built header (%d bytes)\n",lv);
-
 	skb_set_transport_header(skb, lv);
 
-	SOCK_DEBUG(sk, "base=%p pos=%p\n",
-		   skb->data, skb_transport_header(skb));
-
 	*skb_transport_header(skb) = AX25_UI;
 
 	/* Datagram frames go straight out of the door as UI */
net/netrom/af_netrom.c +1 -11
index 06cb027..732152f 100644
@@ -591,7 +591,6 @@ static int nr_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		return -EINVAL;
 	}
 	if ((dev = nr_dev_get(&addr->fsa_ax25.sax25_call)) == NULL) {
-		SOCK_DEBUG(sk, "NET/ROM: bind failed: invalid node callsign\n");
 		release_sock(sk);
 		return -EADDRNOTAVAIL;
 	}

@@ -632,7 +631,7 @@ static int nr_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	sock_reset_flag(sk, SOCK_ZAPPED);
 	dev_put(dev);
 	release_sock(sk);
-	SOCK_DEBUG(sk, "NET/ROM: socket is bound\n");
+
 	return 0;
 }
 

@@ -1082,8 +1081,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
 		sax.sax25_call   = nr->dest_addr;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: Addresses built.\n");
-
 	/* Build a packet - the conventional user limit is 236 bytes. We can
 	   do ludicrously large NetROM frames but must not overflow */
 	if (len > 65536) {

@@ -1091,7 +1088,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
 		goto out;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: building packet.\n");
 	size = len + NR_NETWORK_LEN + NR_TRANSPORT_LEN;
 
 	if ((skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT, &err)) == NULL)

@@ -1105,7 +1101,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
 	 */
 
 	asmptr = skb_push(skb, NR_TRANSPORT_LEN);
-	SOCK_DEBUG(sk, "Building NET/ROM Header.\n");
 
 	/* Build a NET/ROM Transport header */
 

@@ -1114,15 +1109,12 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
 	*asmptr++ = 0;		/* To be filled in later */
 	*asmptr++ = 0;		/*      Ditto            */
 	*asmptr++ = NR_INFO;
-	SOCK_DEBUG(sk, "Built header.\n");
 
 	/*
 	 *	Put the data on the end
 	 */
 	skb_put(skb, len);
 
-	SOCK_DEBUG(sk, "NET/ROM: Appending user data\n");
-
 	/* User data follows immediately after the NET/ROM transport header */
 	if (memcpy_fromiovec(skb_transport_header(skb), msg->msg_iov, len)) {
 		kfree_skb(skb);

@@ -1130,8 +1122,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
 		goto out;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: Transmitting buffer\n");
-
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		kfree_skb(skb);
 		err = -ENOTCONN;
net/rose/af_rose.c +2 -14
index d952e7e..29f4a38 100644
@@ -682,10 +682,8 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	if ((unsigned int) addr->srose_ndigis > ROSE_MAX_DIGIS)
 		return -EINVAL;
 
-	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL) {
-		SOCK_DEBUG(sk, "ROSE: bind failed: invalid address\n");
+	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL)
 		return -EADDRNOTAVAIL;
-	}
 
 	source = &addr->srose_call;
 

@@ -716,7 +714,7 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	rose_insert_socket(sk);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
-	SOCK_DEBUG(sk, "ROSE: socket is bound\n");
+
 	return 0;
 }
 

@@ -1116,10 +1114,7 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
 			srose.srose_digis[n] = rose->dest_digis[n];
 	}
 
-	SOCK_DEBUG(sk, "ROSE: sendto: Addresses built.\n");
-
 	/* Build a packet */
-	SOCK_DEBUG(sk, "ROSE: sendto: building packet.\n");
 	/* Sanity check the packet size */
 	if (len > 65535)
 		return -EMSGSIZE;

@@ -1134,7 +1129,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
 	/*
 	 *	Put the data on the end
 	 */
-	SOCK_DEBUG(sk, "ROSE: Appending user data\n");
 
 	skb_reset_transport_header(skb);
 	skb_put(skb, len);

@@ -1159,8 +1153,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
 	 */
 	asmptr = skb_push(skb, ROSE_MIN_LEN);
 
-	SOCK_DEBUG(sk, "ROSE: Building Network Header.\n");
-
 	/* Build a ROSE Network header */
 	asmptr[0] = ((rose->lci >> 8) & 0x0F) | ROSE_GFI;
 	asmptr[1] = (rose->lci >> 0) & 0xFF;

@@ -1169,10 +1161,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
 	if (qbit)
 		asmptr[0] |= ROSE_Q_BIT;
 
-	SOCK_DEBUG(sk, "ROSE: Built header.\n");
-
-	SOCK_DEBUG(sk, "ROSE: Transmitting buffer\n");
-
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		kfree_skb(skb);
 		return -ENOTCONN;
author
#5 lore raw

Re: Can AX25 socket debug message be removed ?

Hi Ralf,

I agree that your patch including NetRom socket debug messages suppression is fine as AX25, NetRom and ROSE modules are nowadays much more stable in kernel 2.6.

This will simplify the output of kernel log messages allowing further debuging work.

I am actually concerned by two remaining bugs in ROSE module :
First, is the occurence of negative values for neighbour->use parameter from time to time.
Second, is the impossibility to remove rose module after an application such as FPAC has initialized rose nodes and routes.

Then I approve and sustain you proposed patch.

Bernard

Selon Ralf Baechle DL5RB <[email protected]>:

Quoted 241 lines · On Tue, Dec 21, 2010 at 11:38:25AM +0100, f6bvp wrote:
Quoted 8 lines

I would like to know your thinking about removing the following kernel debug messages that fills /var/log/kernel/info and does not seem to be actually usefull.

I build a patch project included here.
In case you approve I will submit it.
Should the SOCK_DEBUG lines be commented or removed ?

The messages are only printed for sockets that have the SOCK_DBG flag set so you may want to audit the application code for use of this flag.

Either way - the messages were there when I took maintainership over the code and I don't know of anybody using them since so I don't mind removing them. Imho they stopped making sense ages ago.

Remove rubbish, don't comment it out unless it's meant to disactivate code for a short time only. For other code that needs to be restored there is the git repository.

So how about below more radical patch that also deals with NET\ROM?

Ralf

Signed-off-by Ralf Baechle <[email protected]>
 net/ax25/af_ax25.c     |   16 +---------------
 net/netrom/af_netrom.c |   12 +-----------
 net/rose/af_rose.c     |   16 ++--------------
 3 files changed, 4 insertions(+), 40 deletions(-)
net/ax25/af_ax25.c +0 -4
index bb86d29..64d2832 100644
@@ -1538,8 +1538,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	}
 
 	/* Build a packet */
-	SOCK_DEBUG(sk, "AX.25: sendto: Addresses built. Building packet.\n");
-
 	/* Assume the worst case */
 	size = len + ax25->ax25_dev->dev->hard_header_len;

@@ -1549,8 +1547,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 
 	skb_reserve(skb, size - len);
 
-	SOCK_DEBUG(sk, "AX.25: Appending user data\n");
-
 	/* User data follows immediately after the AX.25 data */
 	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
net/ax25/af_ax25.c +0 -2
 		err = -EFAULT;

@@ -1564,8 +1560,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	if (!ax25->pidincl)
 		*skb_push(skb, 1) = sk->sk_protocol;
 
-	SOCK_DEBUG(sk, "AX.25: Transmitting buffer\n");
-
 	if (sk->sk_type == SOCK_SEQPACKET) {
 		/* Connected mode sockets go via the LAPB machine */
net/ax25/af_ax25.c +1 -9
 		if (sk->sk_state != TCP_ESTABLISHED) {

@@ -1583,22 +1577,14 @@ static int ax25_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 
 	skb_push(skb, 1 + ax25_addr_size(dp));
 
-	SOCK_DEBUG(sk, "Building AX.25 Header (dp=%p).\n", dp);
-
-	if (dp != NULL)
-		SOCK_DEBUG(sk, "Num digipeaters=%d\n", dp->ndigi);
+	/* Building AX.25 Header */
 
 	/* Build an AX.25 header */
 	lv = ax25_addr_build(skb->data, &ax25->source_addr, &sax.sax25_call,
 			     dp, AX25_COMMAND, AX25_MODULUS);
 
-	SOCK_DEBUG(sk, "Built header (%d bytes)\n",lv);
-
 	skb_set_transport_header(skb, lv);
 
-	SOCK_DEBUG(sk, "base=%p pos=%p\n",
-		   skb->data, skb_transport_header(skb));
-
 	*skb_transport_header(skb) = AX25_UI;
net/ax25/af_ax25.c +0 -0
 	/* Datagram frames go straight out of the door as UI */
net/netrom/af_netrom.c +0 -1
index 06cb027..732152f 100644
@@ -591,7 +591,6 @@ static int nr_bind(struct socket *sock, struct sockaddr
 *uaddr, int addr_len)
 		return -EINVAL;
 	}
 	if ((dev = nr_dev_get(&addr->fsa_ax25.sax25_call)) == NULL) {
-		SOCK_DEBUG(sk, "NET/ROM: bind failed: invalid node callsign\n");
 		release_sock(sk);
 		return -EADDRNOTAVAIL;
net/netrom/af_netrom.c +1 -3
 	}

@@ -632,7 +631,7 @@ static int nr_bind(struct socket *sock, struct sockaddr
 *uaddr, int addr_len)
 	sock_reset_flag(sk, SOCK_ZAPPED);
 	dev_put(dev);
 	release_sock(sk);
-	SOCK_DEBUG(sk, "NET/ROM: socket is bound\n");
+
 	return 0;
 }

@@ -1082,8 +1081,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket
 *sock,
 		sax.sax25_call   = nr->dest_addr;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: Addresses built.\n");
-
 	/* Build a packet - the conventional user limit is 236 bytes. We can
 	   do ludicrously large NetROM frames but must not overflow */
net/netrom/af_netrom.c +0 -1
 	if (len > 65536) {

@@ -1091,7 +1088,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket
 *sock,
 		goto out;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: building packet.\n");
 	size = len + NR_NETWORK_LEN + NR_TRANSPORT_LEN;

if ((skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT,
&err)) == NULL)

net/netrom/af_netrom.c +0 -4
@@ -1105,7 +1101,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket
 *sock,
 	 */
 
 	asmptr = skb_push(skb, NR_TRANSPORT_LEN);
-	SOCK_DEBUG(sk, "Building NET/ROM Header.\n");
 
 	/* Build a NET/ROM Transport header */

@@ -1114,15 +1109,12 @@ static int nr_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	*asmptr++ = 0;		/* To be filled in later */
 	*asmptr++ = 0;		/*      Ditto            */
 	*asmptr++ = NR_INFO;
-	SOCK_DEBUG(sk, "Built header.\n");
 
 	/*
 	 *	Put the data on the end
 	 */
 	skb_put(skb, len);
 
-	SOCK_DEBUG(sk, "NET/ROM: Appending user data\n");
-
 	/* User data follows immediately after the NET/ROM transport header */
 	if (memcpy_fromiovec(skb_transport_header(skb), msg->msg_iov, len)) {
net/netrom/af_netrom.c +0 -2
 		kfree_skb(skb);

@@ -1130,8 +1122,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket
 *sock,
 		goto out;
 	}
 
-	SOCK_DEBUG(sk, "NET/ROM: Transmitting buffer\n");
-
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		kfree_skb(skb);
net/netrom/af_netrom.c +0 -0
 		err = -ENOTCONN;
net/rose/af_rose.c +2 -7
index d952e7e..29f4a38 100644
@@ -682,10 +682,8 @@ static int rose_bind(struct socket *sock, struct
 sockaddr *uaddr, int addr_len)
 	if ((unsigned int) addr->srose_ndigis > ROSE_MAX_DIGIS)
 		return -EINVAL;
 
-	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL) {
-		SOCK_DEBUG(sk, "ROSE: bind failed: invalid address\n");
+	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL)
 		return -EADDRNOTAVAIL;
-	}
 
 	source = &addr->srose_call;

@@ -716,7 +714,7 @@ static int rose_bind(struct socket *sock, struct sockaddr
 *uaddr, int addr_len)
 	rose_insert_socket(sk);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
-	SOCK_DEBUG(sk, "ROSE: socket is bound\n");
+
 	return 0;
 }

@@ -1116,10 +1114,7 @@ static int rose_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 			srose.srose_digis[n] = rose->dest_digis[n];
 	}
 
-	SOCK_DEBUG(sk, "ROSE: sendto: Addresses built.\n");
-
 	/* Build a packet */
-	SOCK_DEBUG(sk, "ROSE: sendto: building packet.\n");
 	/* Sanity check the packet size */
 	if (len > 65535)
net/rose/af_rose.c +0 -1
 		return -EMSGSIZE;

@@ -1134,7 +1129,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	/*
 	 *	Put the data on the end
 	 */
-	SOCK_DEBUG(sk, "ROSE: Appending user data\n");
 
 	skb_reset_transport_header(skb);
net/rose/af_rose.c +0 -2
 	skb_put(skb, len);

@@ -1159,8 +1153,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	 */
 	asmptr = skb_push(skb, ROSE_MIN_LEN);
 
-	SOCK_DEBUG(sk, "ROSE: Building Network Header.\n");
-
 	/* Build a ROSE Network header */
 	asmptr[0] = ((rose->lci >> 8) & 0x0F) | ROSE_GFI;
net/rose/af_rose.c +0 -4
 	asmptr[1] = (rose->lci >> 0) & 0xFF;

@@ -1169,10 +1161,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct
 socket *sock,
 	if (qbit)
 		asmptr[0] |= ROSE_Q_BIT;
 
-	SOCK_DEBUG(sk, "ROSE: Built header.\n");
-
-	SOCK_DEBUG(sk, "ROSE: Transmitting buffer\n");
-
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		kfree_skb(skb);

return -ENOTCONN;

Signature · 1 line
73 de Bernard, F6BVP
author
#6 lore raw

Re: Can AX25 socket debug message be removed ?

In reply to Ralf Baechle DL5RB

Hello Ralf,

I approve your more radical patch and I am sorry for the late response. af_rose patch could display a small offset as I re-sent a patch to Linux-netdev that has been just applied by David Miller to net-next-2.6. I had put you on Cc for the patch I sent.

73 de Bernard, f6bvp

Quoted 214 lines · Le 21/12/2010 18:33, Ralf Baechle DL5RB a écrit :
Quoted 8 lines · On Tue, Dec 21, 2010 at 11:38:25AM +0100, f6bvp wrote:

I would like to know your thinking about removing the following kernel debug messages that fills /var/log/kernel/info and does not seem to be actually usefull.

I build a patch project included here.
In case you approve I will submit it.
Should the SOCK_DEBUG lines be commented or removed ?

The messages are only printed for sockets that have the SOCK_DBG flag set so you may want to audit the application code for use of this flag.

Either way - the messages were there when I took maintainership over the code and I don't know of anybody using them since so I don't mind removing them. Imho they stopped making sense ages ago.

Remove rubbish, don't comment it out unless it's meant to disactivate code for a short time only. For other code that needs to be restored there is the git repository.

So how about below more radical patch that also deals with NET\ROM?

Ralf

Signed-off-by Ralf Baechle<[email protected]>
  net/ax25/af_ax25.c     |   16 +---------------
  net/netrom/af_netrom.c |   12 +-----------
  net/rose/af_rose.c     |   16 ++--------------
  3 files changed, 4 insertions(+), 40 deletions(-)
net/ax25/af_ax25.c +1 -15
index bb86d29..64d2832 100644
@@ -1538,8 +1538,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
  	}
 
  	/* Build a packet */
-	SOCK_DEBUG(sk, "AX.25: sendto: Addresses built. Building packet.\n");
-
  	/* Assume the worst case */
  	size = len + ax25->ax25_dev->dev->hard_header_len;
 

@@ -1549,8 +1547,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 
  	skb_reserve(skb, size - len);
 
-	SOCK_DEBUG(sk, "AX.25: Appending user data\n");
-
  	/* User data follows immediately after the AX.25 data */
  	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
  		err = -EFAULT;

@@ -1564,8 +1560,6 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
  	if (!ax25->pidincl)
  		*skb_push(skb, 1) = sk->sk_protocol;
 
-	SOCK_DEBUG(sk, "AX.25: Transmitting buffer\n");
-
  	if (sk->sk_type == SOCK_SEQPACKET) {
  		/* Connected mode sockets go via the LAPB machine */
  		if (sk->sk_state != TCP_ESTABLISHED) {

@@ -1583,22 +1577,14 @@ static int ax25_sendmsg(struct kiocb *iocb, struct socket *sock,
 
  	skb_push(skb, 1 + ax25_addr_size(dp));
 
-	SOCK_DEBUG(sk, "Building AX.25 Header (dp=%p).\n", dp);
-
-	if (dp != NULL)
-		SOCK_DEBUG(sk, "Num digipeaters=%d\n", dp->ndigi);
+	/* Building AX.25 Header */
 
  	/* Build an AX.25 header */
  	lv = ax25_addr_build(skb->data,&ax25->source_addr,&sax.sax25_call,
  			     dp, AX25_COMMAND, AX25_MODULUS);
 
-	SOCK_DEBUG(sk, "Built header (%d bytes)\n",lv);
-
  	skb_set_transport_header(skb, lv);
 
-	SOCK_DEBUG(sk, "base=%p pos=%p\n",
-		   skb->data, skb_transport_header(skb));
-
  	*skb_transport_header(skb) = AX25_UI;
 
  	/* Datagram frames go straight out of the door as UI */
net/netrom/af_netrom.c +1 -11
index 06cb027..732152f 100644
@@ -591,7 +591,6 @@ static int nr_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
  		return -EINVAL;
  	}
  	if ((dev = nr_dev_get(&addr->fsa_ax25.sax25_call)) == NULL) {
-		SOCK_DEBUG(sk, "NET/ROM: bind failed: invalid node callsign\n");
  		release_sock(sk);
  		return -EADDRNOTAVAIL;
  	}

@@ -632,7 +631,7 @@ static int nr_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
  	sock_reset_flag(sk, SOCK_ZAPPED);
  	dev_put(dev);
  	release_sock(sk);
-	SOCK_DEBUG(sk, "NET/ROM: socket is bound\n");
+
  	return 0;
  }
 

@@ -1082,8 +1081,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
  		sax.sax25_call   = nr->dest_addr;
  	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: Addresses built.\n");
-
  	/* Build a packet - the conventional user limit is 236 bytes. We can
  	   do ludicrously large NetROM frames but must not overflow */
  	if (len>  65536) {

@@ -1091,7 +1088,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
  		goto out;
  	}
 
-	SOCK_DEBUG(sk, "NET/ROM: sendto: building packet.\n");
  	size = len + NR_NETWORK_LEN + NR_TRANSPORT_LEN;
 
  	if ((skb = sock_alloc_send_skb(sk, size, msg->msg_flags&  MSG_DONTWAIT,&err)) == NULL)

@@ -1105,7 +1101,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
  	 */
 
  	asmptr = skb_push(skb, NR_TRANSPORT_LEN);
-	SOCK_DEBUG(sk, "Building NET/ROM Header.\n");
 
  	/* Build a NET/ROM Transport header */
 

@@ -1114,15 +1109,12 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
  	*asmptr++ = 0;		/* To be filled in later */
  	*asmptr++ = 0;		/*      Ditto            */
  	*asmptr++ = NR_INFO;
-	SOCK_DEBUG(sk, "Built header.\n");
 
  	/*
  	 *	Put the data on the end
  	 */
  	skb_put(skb, len);
 
-	SOCK_DEBUG(sk, "NET/ROM: Appending user data\n");
-
  	/* User data follows immediately after the NET/ROM transport header */
  	if (memcpy_fromiovec(skb_transport_header(skb), msg->msg_iov, len)) {
  		kfree_skb(skb);

@@ -1130,8 +1122,6 @@ static int nr_sendmsg(struct kiocb *iocb, struct socket *sock,
  		goto out;
  	}
 
-	SOCK_DEBUG(sk, "NET/ROM: Transmitting buffer\n");
-
  	if (sk->sk_state != TCP_ESTABLISHED) {
  		kfree_skb(skb);
  		err = -ENOTCONN;
net/rose/af_rose.c +2 -10
index d952e7e..29f4a38 100644
@@ -682,10 +682,8 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
  	if ((unsigned int) addr->srose_ndigis>  ROSE_MAX_DIGIS)
  		return -EINVAL;
 
-	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL) {
-		SOCK_DEBUG(sk, "ROSE: bind failed: invalid address\n");
+	if ((dev = rose_dev_get(&addr->srose_addr)) == NULL)
  		return -EADDRNOTAVAIL;
-	}
 
  	source =&addr->srose_call;
 

@@ -716,7 +714,7 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
  	rose_insert_socket(sk);
 
  	sock_reset_flag(sk, SOCK_ZAPPED);
-	SOCK_DEBUG(sk, "ROSE: socket is bound\n");
+
  	return 0;
  }
 

@@ -1116,10 +1114,7 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
  			srose.srose_digis[n] = rose->dest_digis[n];
  	}
 
-	SOCK_DEBUG(sk, "ROSE: sendto: Addresses built.\n");
-
  	/* Build a packet */
-	SOCK_DEBUG(sk, "ROSE: sendto: building packet.\n");
  	/* Sanity check the packet size */
  	if (len>  65535)
  		return -EMSGSIZE;

@@ -1134,7 +1129,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
  	/*
  	 *	Put the data on the end
  	 */
-	SOCK_DEBUG(sk, "ROSE: Appending user data\n");
 
  	skb_reset_transport_header(skb);
  	skb_put(skb, len);

@@ -1159,8 +1153,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
  	 */
  	asmptr = skb_push(skb, ROSE_MIN_LEN);
 
-	SOCK_DEBUG(sk, "ROSE: Building Network Header.\n");
-
  	/* Build a ROSE Network header */
  	asmptr[0] = ((rose->lci>>  8)&  0x0F) | ROSE_GFI;
  	asmptr[1] = (rose->lci>>  0)&  0xFF;

net/rose/af_rose.c static int rose_sendmsg(struct kiocb *iocb, struct socket *sock, · line 1166

Quoted 10 lines
net/rose/af_rose.c +0 -4
@@ -1169,10 +1161,6 @@ static int rose_sendmsg(struct kiocb *iocb, struct socket *sock,
  	if (qbit)
  		asmptr[0] |= ROSE_Q_BIT;
 
-	SOCK_DEBUG(sk, "ROSE: Built header.\n");
-
-	SOCK_DEBUG(sk, "ROSE: Transmitting buffer\n");
-
  	if (sk->sk_state != TCP_ESTABLISHED) {
  		kfree_skb(skb);
  		return -ENOTCONN;

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

Can ROSE socket debug message be removed ?

In reply to f6bvp

Hi Ralf,

The same question about removing the following ROSE kernel debug messages that fills /var/log/kernel/info and does not seem to be actually usefull.

Should the SOCK_DEBUG lines be commented or removed ?

Bernard Pidoux <[email protected]>

net/rose/af_rose.c +7 -7
@@ -716,7 +739,7 @@
 	rose_insert_socket(sk);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
-	SOCK_DEBUG(sk, "ROSE: socket is bound\n");
+/*	SOCK_DEBUG(sk, "ROSE: socket is bound\n");*/
 	return 0;
 }
 

@@ -1113,10 +1138,10 @@
 			srose.srose_digis[n] = rose->dest_digis[n];
 	}
 
-	SOCK_DEBUG(sk, "ROSE: sendto: Addresses built.\n");
+/*	SOCK_DEBUG(sk, "ROSE: sendto: Addresses built.\n");*/
 
 	/* Build a packet */
-	SOCK_DEBUG(sk, "ROSE: sendto: building packet.\n");
+/*	SOCK_DEBUG(sk, "ROSE: sendto: building packet.\n");*/
 	/* Sanity check the packet size */
 	if (len > 65535)
 		return -EMSGSIZE;

@@ -1131,7 +1156,7 @@
 	/*
 	 *	Put the data on the end
 	 */
-	SOCK_DEBUG(sk, "ROSE: Appending user data\n");
+/*	SOCK_DEBUG(sk, "ROSE: Appending user data\n");*/
 
 	skb_reset_transport_header(skb);
 	skb_put(skb, len);

@@ -1156,7 +1181,7 @@
 	 */
 	asmptr = skb_push(skb, ROSE_MIN_LEN);
 
-	SOCK_DEBUG(sk, "ROSE: Building Network Header.\n");
+/*	SOCK_DEBUG(sk, "ROSE: Building Network Header.\n");*/
 
 	/* Build a ROSE Network header */
 	asmptr[0] = ((rose->lci >> 8) & 0x0F) | ROSE_GFI;

@@ -1166,9 +1191,9 @@
 	if (qbit)
 		asmptr[0] |= ROSE_Q_BIT;
 
-	SOCK_DEBUG(sk, "ROSE: Built header.\n");
+/*	SOCK_DEBUG(sk, "ROSE: Built header.\n");*/
 
-	SOCK_DEBUG(sk, "ROSE: Transmitting buffer\n");
+/*	SOCK_DEBUG(sk, "ROSE: Transmitting buffer\n");*/
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
 		kfree_skb(skb);
#8 lore raw
In reply to f6bvp

Hi,

Here is a second try for this patch that completes the previous one.

[PATCH] [ROSE] [AX25] finding routes simplification

With previous patch, rose_get_neigh() routine investigates the full list of neighbor nodes until it finds or not an already connected node whether it is called locally or through a level 3 transit frame. If no routes are opened through an adjacent connected node then a classical connect request is attempted.

Then there is no more reason for an extra loop such as the one removed by this patch.

Signed-off-by Bernard Pidoux <[email protected]>
net/rose/af_rose.c +0 -7
@@ -803,7 +803,6 @@
 
 		rose_insert_socket(sk);		/* Finish the bind */
 	}
-rose_try_next_neigh:
 	rose->dest_addr   = addr->srose_addr;
 	rose->dest_call   = addr->srose_call;
 	rose->rand        = ((long)rose & 0xFFFF) + rose->lci;

@@ -865,12 +864,6 @@
 	}
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
-	/* Try next neighbour */
-		rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic, 0);
-		if (rose->neighbour)
-			goto rose_try_next_neigh;
-
-		/* No more neighbours */
 		sock->state = SS_UNCONNECTED;
 		err = sock_error(sk);	/* Always set at this point */
 		goto out_release;
maintainer
#9 lore raw

From: Bernard Pidoux <[email protected]>
Date: Mon, 14 Feb 2011 18:52:09 +0100

Quoted 9 lines

[PATCH] [ROSE] [AX25] finding routes simplification

With previous patch, rose_get_neigh() routine investigates the full list of neighbor nodes until it finds or not an already connected node whether it is called locally or through a level 3 transit frame. If no routes are opened through an adjacent connected node then a classical connect request is attempted.

Quoted 4 lines

Then there is no more reason for an extra loop such as the one removed by this patch.

Signed-off-by Bernard Pidoux <[email protected]>

Also applied to net-next-2.6, thanks!