Skip to contentSkip to conversation

New: X25 fails to negotiate connection with Cisco router due to bad value for X25_DEFAULT_THROUGHPUT

Bugme-new Andrew Morton 1 participant 1 message Apr 5, 2010 1 message not in the archive

Subsystems (3)
Networking Network drivers X.25 stack
lore
Share & tools
lore

Andrew Morton

author
#1 lore raw
In reply to a message not in the archive

(switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface).

Quoted 21 lines · On Fri, 2 Apr 2010 14:48:17 GMT [email protected] wrote:

https://bugzilla.kernel.org/show_bug.cgi?id=15681

Summary: X25 fails to negotiate connection with Cisco router
due to bad value for X25_DEFAULT_THROUGHPUT
Product: Networking
Version: 2.5
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
AssignedTo: [email protected]
ReportedBy: [email protected]
Regression: No

X.25 connection with a Cisco router fails because Linux X.25 tries to negotiate an illegal value "0" for the inbound throughput.

Quoted 8 lines

This bug was first reported by Daniel Ferenci to the linux-x25 mailing list on 6/8/2004, but is still present.

X25_DEFAULT_THROUGHPUT is defined as 0x0A, i.e. when the Linux system calls it wants a throughput of at least 0xA (9600bps) outbound and at least 0x0 (Reserved value) inbound.

See X.25 (1988) section 7.2.2.2

Thanks, but... please don't send patches via bugzilla - it bypasses all our review and merge processes and tools.

I merged your patch as below and shall send it on to Ralf and David for review.

From: John Hughes <[email protected]>

The current (2.6.34) x.25 code doesn't seem to know that the X.25 throughput facility includes two values, one for the required throughput outbound, one for inbound.

This causes it to attempt to negotiate throughput 0x0A, which is throughput 9600 inbound and the illegal value "0" for inbound throughput.

Because of this some X.25 devices (e.g. Cisco 1600) refuse to connect to Linux X.25.

The patch fixes this behaviour. Unless the user specifies a required throughput it does not attempt to negotiate. If the user does not specify a throughput it accepts the suggestion of the remote X.25 system. If the user requests a throughput then it validates both the input and output throughputs and correctly negotiates them with the remote end.

Signed-off-by John Hughes <[email protected]>
Cc Ralf Baechle <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

---

 net/x25/af_x25.c         |   20 ++++++++++++++++----
 net/x25/x25_facilities.c |   15 ++++++++++++---
 2 files changed, 28 insertions(+), 7 deletions(-)
net/x25/af_x25.c renamed from net/x25/af_x25.c~a +16 -4
diff -puN net/x25/af_x25.c~a net/x25/af_x25.c
@@ -554,7 +554,8 @@ static int x25_create(struct net *net, s
 	x25->facilities.winsize_out = X25_DEFAULT_WINDOW_SIZE;
 	x25->facilities.pacsize_in  = X25_DEFAULT_PACKET_SIZE;
 	x25->facilities.pacsize_out = X25_DEFAULT_PACKET_SIZE;
-	x25->facilities.throughput  = X25_DEFAULT_THROUGHPUT;
+	x25->facilities.throughput  = 0;	/* by default don't negotiate
+						   throughput */
 	x25->facilities.reverse     = X25_DEFAULT_REVERSE;
 	x25->dte_facilities.calling_len = 0;
 	x25->dte_facilities.called_len = 0;

@@ -1415,9 +1416,20 @@ static int x25_ioctl(struct socket *sock
 			if (facilities.winsize_in < 1 ||
 			    facilities.winsize_in > 127)
 				break;
-			if (facilities.throughput < 0x03 ||
-			    facilities.throughput > 0xDD)
-				break;
+			if (facilities.throughput) {
+				int out = facilities.throughput & 0xf0;
+				int in  = facilities.throughput & 0x0f;
+				if (!out)
+					facilities.throughput |=
+						X25_DEFAULT_THROUGHPUT << 4;
+				else if (out < 0x30 || out > 0xD0)
+					break;
+				if (!in)
+					facilities.throughput |=
+						X25_DEFAULT_THROUGHPUT;
+				else if (in < 0x03 || in > 0x0D)
+					break;
+			}
 			if (facilities.reverse &&
 				(facilities.reverse & 0x81) != 0x81)
 				break;
net/x25/x25_facilities.c renamed from net/x25/x25_facilities.c~a +12 -3
diff -puN net/x25/x25_facilities.c~a net/x25/x25_facilities.c
@@ -259,9 +259,18 @@ int x25_negotiate_facilities(struct sk_b
 	new->reverse = theirs.reverse;
 
 	if (theirs.throughput) {
-		if (theirs.throughput < ours->throughput) {
-			SOCK_DEBUG(sk, "X.25: throughput negotiated down\n");
-			new->throughput = theirs.throughput;
+		int theirs_in =  theirs.throughput & 0x0f;
+		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");
+			new->throughput = (new->throughput & 0xf0) | theirs_in;
+		}
+		if (!ours_out || theirs_out < ours_out) {
+			SOCK_DEBUG(sk,
+				"X.25: outbound throughput negotiated\n");
+			new->throughput = (new->throughput & 0x0f) | theirs_out;
 		}
 	}

_