[PATCH] [ROSE] finding a connected ROSE neighbor node

[PATCH] [ROSE] finding a connected ROSE neighbor node

[Bernard Pidoux]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

Hi,

This patch introduces a test to determine if a neighbor node with a 
given ROSE address is already connected and if so returns its address.
The previous test was only performed on timer t0 and not on parameter 
restarted that is true when a node is connected.
Also, previously the test was not performed on the whole node list.
A new function __rose_get_neigh() is introduced to cure a spin lock 
conflict related to rose_node_list_lock.

signed off by Alexey Dobriyan,adobriyan@gmail.com
signed off by Bernard Pidoux, f6bvp @ amsat.org



[-- Attachment #2: rose-2.6.24-rc5.patch2 --]
[-- Type: text/plain, Size: 2755 bytes --]

--- linux-2.6.24-rc5/include/net/rose.h	2007-12-11 04:48:43.000000000 +0100
+++ b/include/net/rose.h	2007-12-14 14:25:02.000000000 +0100
@@ -202,6 +202,7 @@
 extern struct net_device *rose_dev_get(rose_address *);
 extern struct rose_route *rose_route_free_lci(unsigned int, struct rose_neigh *);
 extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
+extern struct rose_neigh *__rose_get_neigh(rose_address *, unsigned char *, unsigned char *);
 extern int  rose_rt_ioctl(unsigned int, void __user *);
 extern void rose_link_failed(ax25_cb *, int);
 extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
--- linux-2.6.24-rc5/net/rose/rose_route.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/rose_route.c	2007-12-14 14:25:02.000000000 +0100
@@ -664,25 +664,22 @@
 /*
  *	Find a neighbour given a ROSE address.
  */
-struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+struct rose_neigh *__rose_get_neigh(rose_address *addr, unsigned char *cause,
 	unsigned char *diagnostic)
 {
-	struct rose_neigh *res = NULL;
 	struct rose_node *node;
 	int failed = 0;
 	int i;
 
-	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 (!rose_ftimer_running(node->neighbour[i])) {
-					res = node->neighbour[i];
-					goto out;
-				} else
-					failed = 1;
+				if (node->neighbour[i]->restarted)
+					return node->neighbour[i];
+				if (!rose_ftimer_running(node->neighbour[i]))			
+					return node->neighbour[i];
+				failed = 1;
 			}
-			break;
 		}
 	}
 
@@ -694,7 +691,16 @@
 		*diagnostic = 0;
 	}
 
-out:
+	return NULL;
+}
+
+struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char *cause,
+	unsigned char *diagnostic)
+{
+	struct rose_neigh *res;
+
+	spin_lock_bh(&rose_node_list_lock);
+	res = __rose_get_neigh(addr, cause, diagnostic);
 	spin_unlock_bh(&rose_node_list_lock);
 
 	return res;
@@ -1019,7 +1025,7 @@
 		rose_route = rose_route->next;
 	}
 
-	if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
+	if ((new_neigh = __rose_get_neigh(dest_addr, &cause, &diagnostic)) == NULL) {
 		rose_transmit_clear_request(rose_neigh, lci, cause, diagnostic);
 		goto out;
 	}
--- linux-2.6.24-rc5/net/rose/af_rose.c	2007-12-11 04:48:43.000000000 +0100
+++ b/net/rose/af_rose.c	2007-12-14 14:25:02.000000000 +0100
@@ -750,7 +750,7 @@
 	sk->sk_state   = TCP_CLOSE;
 	sock->state = SS_UNCONNECTED;
 
-	rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
+	rose->neighbour = __rose_get_neigh(&addr->srose_addr, &cause,
 					 &diagnostic);
 	if (!rose->neighbour)
 		return -ENETUNREACH;

Re: [PATCH] [ROSE] finding a connected ROSE neighbor node

[Bernard Pidoux]

Hi,

I propose to drop the previously submitted patch for I performed more
investigations on ROSE frame routing and found that it was not
completely satisfactory.

Please find here another commit to net-2.6.25 with explanations.


 From fd66cc115e058b2fc63a0e26aa73f1d27113105a Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Thu, 10 Jan 2008 23:10:44 +0100
Subject: [PATCH 1/4] [ROSE] new rose_get_route() function

rose_get_neigh() was called by two different functions.
Firstly, by rose_connect() in order to establish connections to
adjacent rose nodes. This worked correctly.
Secondly, it was called by rose_route_frame() to find a route
via an adjacent node. This was not working efficiently, for the
proper test was not performed in order to check if the node was
already connected.
A new function rose_get_route() is devoted to frame routing.
It returns a ROSE node address for sending a frame to the
specified destination via a connected node.

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
  net/rose/rose_route.c |   34 +++++++++++++++++++++++++++++++++-
  1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 540c0f2..ec79567 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -662,6 +662,38 @@ struct rose_route *rose_route_free_lci(unsigned int 
lci, struct rose_neigh *neig
  }

  /*
+ *     Find an opened route given a ROSE address.
+ */
+struct rose_neigh *rose_get_route(rose_address *addr, unsigned char *cause,
+       unsigned char *diagnostic)
+{
+       struct rose_node *node;
+       int failed = 0;
+       int i;
+
+       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 (node->neighbour[i]->restarted)
+                                       return node->neighbour[i];
+                               failed = 1;
+                       }
+               }
+       }
+
+       if (failed) {
+               *cause      = ROSE_OUT_OF_ORDER;
+               *diagnostic = 0;
+       } else {
+               *cause      = ROSE_NOT_OBTAINABLE;
+               *diagnostic = 0;
+       }
+
+       return NULL;
+}
+
+
+/*
   *     Find a neighbour given a ROSE address.
   */
  struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char 
*cause,
@@ -1019,7 +1051,7 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb 
*ax25)
                 rose_route = rose_route->next;
         }

-       if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) 
== NULL) {
+       if ((new_neigh = rose_get_route(dest_addr, &cause, &diagnostic)) 
== NULL) {
                 rose_transmit_clear_request(rose_neigh, lci, cause, 
diagnostic);
                 goto out;
         }
--
1.5.3.7

[PATCH 2/4] [ROSE] rose_get_route() template

[Bernard Pidoux]

 From 46bccce1e660a39bcc8f8cf87d4c17de33f4ba48 Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Thu, 10 Jan 2008 23:01:46 +0100
Subject: [PATCH 2/4] [ROSE] template declaration for rose_get_route()

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
  include/net/rose.h |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/net/rose.h b/include/net/rose.h
index e5bb084..d3ab453 100644
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first(void);
  extern struct net_device *rose_dev_get(rose_address *);
  extern struct rose_route *rose_route_free_lci(unsigned int, struct 
rose_neigh *);
  extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char 
*, unsigned char *);
+extern struct rose_neigh *rose_get_route(rose_address *, unsigned char 
*, unsigned char *);
  extern int  rose_rt_ioctl(unsigned int, void __user *);
  extern void rose_link_failed(ax25_cb *, int);
  extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
--
1.5.3.7

Re: [PATCH 2/4] [ROSE] rose_get_route() template

[Eric Dumazet]

Bernard Pidoux a écrit :
>  From 46bccce1e660a39bcc8f8cf87d4c17de33f4ba48 Mon Sep 17 00:00:00 2001
> From: Bernard Pidoux <f6bvp@amsat.org>
> Date: Thu, 10 Jan 2008 23:01:46 +0100
> Subject: [PATCH 2/4] [ROSE] template declaration for rose_get_route()
> 
> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
> ---
>  include/net/rose.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/rose.h b/include/net/rose.h
> index e5bb084..d3ab453 100644
> --- a/include/net/rose.h
> +++ b/include/net/rose.h
> @@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first(void);
>  extern struct net_device *rose_dev_get(rose_address *);
>  extern struct rose_route *rose_route_free_lci(unsigned int, struct 
> rose_neigh *);
>  extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned char 
> *, unsigned char *);
> +extern struct rose_neigh *rose_get_route(rose_address *, unsigned char 
> *, unsigned char *);
>  extern int  rose_rt_ioctl(unsigned int, void __user *);
>  extern void rose_link_failed(ax25_cb *, int);
>  extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
> -- 

Strange... if rose_get_route() is used only in net/rose/rose_route.c, why dont 
you define it static, and not extern in include/net/rose.h ?

Re: [PATCH 2/4] [ROSE] rose_get_route() template

[Bernard Pidoux]

Eric Dumazet wrote:
> Bernard Pidoux a écrit :
>>  From 46bccce1e660a39bcc8f8cf87d4c17de33f4ba48 Mon Sep 17 00:00:00 2001
>> From: Bernard Pidoux <f6bvp@amsat.org>
>> Date: Thu, 10 Jan 2008 23:01:46 +0100
>> Subject: [PATCH 2/4] [ROSE] template declaration for rose_get_route()
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>> ---
>>  include/net/rose.h |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/rose.h b/include/net/rose.h
>> index e5bb084..d3ab453 100644
>> --- a/include/net/rose.h
>> +++ b/include/net/rose.h
>> @@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first(void);
>>  extern struct net_device *rose_dev_get(rose_address *);
>>  extern struct rose_route *rose_route_free_lci(unsigned int, struct 
>> rose_neigh *);
>>  extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned 
>> char *, unsigned char *);
>> +extern struct rose_neigh *rose_get_route(rose_address *, unsigned 
>> char *, unsigned char *);
>>  extern int  rose_rt_ioctl(unsigned int, void __user *);
>>  extern void rose_link_failed(ax25_cb *, int);
>>  extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
>> -- 
> 
> Strange... if rose_get_route() is used only in net/rose/rose_route.c, 
> why dont you define it static, and not extern in include/net/rose.h ?
> 
> 
> 

I agree. You are perfectly right.
There is no need to declare rose_get_route() external.
I stupidly copied rose_get_neigh()definition from which I derived 
rose_get_route();

Also I am not sure that setting cause and diagnostic is necessary, as 
they are not used by the calling function.

By the way. I made a typo in [PATCH 4/4].

Instead of "Initial connection to rose neighbour nodes was unusually,
t0 timer was blocked and application program could not
use socket."

One should read
"Initial connection to rose neighbour nodes was unusually,
long, t0 timer was blocked and application program could not
use socket."

Bernard P.

Re: [PATCH 2/4] [ROSE] rose_get_route() template

[Bernard Pidoux]

Hi,

I wrote a "simplified" get_route() function.
It is declared as static following judicious Eric's remark.

Then, the following patch of include/net/rose.h is no more
necessary.

A new commit for rose_get_route() will be presented in next
message.

Thank you Eric for pushing me to reexamine my code.

Regards,

Bernard P.



Bernard Pidoux wrote:
> 
> 
> Eric Dumazet wrote:
>> Bernard Pidoux a écrit :
>>>  From 46bccce1e660a39bcc8f8cf87d4c17de33f4ba48 Mon Sep 17 00:00:00 2001
>>> From: Bernard Pidoux <f6bvp@amsat.org>
>>> Date: Thu, 10 Jan 2008 23:01:46 +0100
>>> Subject: [PATCH 2/4] [ROSE] template declaration for rose_get_route()
>>>
>>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>> ---
>>>  include/net/rose.h |    1 +
>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/net/rose.h b/include/net/rose.h
>>> index e5bb084..d3ab453 100644
>>> --- a/include/net/rose.h
>>> +++ b/include/net/rose.h
>>> @@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first(void);
>>>  extern struct net_device *rose_dev_get(rose_address *);
>>>  extern struct rose_route *rose_route_free_lci(unsigned int, struct 
>>> rose_neigh *);
>>>  extern struct rose_neigh *rose_get_neigh(rose_address *, unsigned 
>>> char *, unsigned char *);
>>> +extern struct rose_neigh *rose_get_route(rose_address *, unsigned 
>>> char *, unsigned char *);
>>>  extern int  rose_rt_ioctl(unsigned int, void __user *);
>>>  extern void rose_link_failed(ax25_cb *, int);
>>>  extern int  rose_route_frame(struct sk_buff *, ax25_cb *);
>>> -- 
>>
>> Strange... if rose_get_route() is used only in net/rose/rose_route.c, 
>> why dont you define it static, and not extern in include/net/rose.h ?
>>
>>
>>
> 
> I agree. You are perfectly right.
> There is no need to declare rose_get_route() external.
> I stupidly copied rose_get_neigh()definition from which I derived 
> rose_get_route();
> 
> Also I am not sure that setting cause and diagnostic is necessary, as 
> they are not used by the calling function.
> 
> By the way. I made a typo in [PATCH 4/4].
> 
> Instead of "Initial connection to rose neighbour nodes was unusually,
> t0 timer was blocked and application program could not
> use socket."
> 
> One should read
> "Initial connection to rose neighbour nodes was unusually,
> long, t0 timer was blocked and application program could not
> use socket."
> 
> Bernard P.
> 
> 
> 

[PATCH 3/4] [ROSE] return with lock held

[Bernard Pidoux]

 From bc108e5ee0b0353c3707df25e12e40038da0160a Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Fri, 11 Jan 2008 10:23:55 +0100
Subject: [PATCH 3/4] [ROSE] return with lock held

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>

================================================
[ BUG: lock held when returning to user space! ]

------------------------------------------------
fpacwpd/3057 is leaving the kernel with locks still held!
1 lock held by fpacwpd/3057:
#0:  (sk_lock-AF_ROSE){--..}, at: [<d8bfda7f>] rose_connect+0x6c/0x357 
[rose]
---
  net/rose/af_rose.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 8f70ad8..9419946 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -752,7 +752,8 @@ static int rose_connect(struct socket *sock, struct 
sockaddr *uaddr, int addr_le
         rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
                                          &diagnostic);
         if (!rose->neighbour)
-               return -ENETUNREACH;
+               err = -ENETUNREACH;
+               goto out_release;

         rose->lci = rose_new_lci(rose->neighbour);
         if (!rose->lci) {
--
1.5.3.7

[PATCH 4/4] [ROSE] ENETUNREACH held rose_connect()

[Bernard Pidoux]

 From 5c50971c6088f380eafdb1a6a7de5a5d3686c8c7 Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Sat, 12 Jan 2008 20:19:34 +0100
Subject: [PATCH 4/4] [ROSE] ENETUNREACH held rose_connect()

Initial connection to rose neighbour nodes was unusually,
t0 timer was blocked and application program could not
use socket.
I performed a number of trials in order to find what was
slowing rose_connect() and blocking ROSE t0 timer.
Replacing the type of error returned did not cure the problem.
Finally removing the error returned when rose->neighbour is NULL
is the solution.
Without err = -ENETUNREACH rose_connect() returns default err = 0.
Doing that, t0 timer is running, rose_connect() performs adjacent
rose node connections as needed and rose frames are well routed.
Also, application programs can access rose connect socket.

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
  net/rose/af_rose.c |    1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 9419946..5a8c886 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -752,7 +752,6 @@ static int rose_connect(struct socket *sock, struct 
sockaddr *uaddr, int addr_le
         rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause,
                                          &diagnostic);
         if (!rose->neighbour)
-               err = -ENETUNREACH;
                 goto out_release;

         rose->lci = rose_new_lci(rose->neighbour);
--
1.5.3.7

[PATCH 1/4] [ROSE] simplified rose_get_route()

[Bernard Pidoux]

Hi,

This patch is send to replace preceding commit :

 From fd66cc115e058b2fc63a0e26aa73f1d27113105a Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Thu, 10 Jan 2008 23:10:44 +0100
Subject: [PATCH 1/4] [ROSE] new rose_get_route() function


I removed unnecessary lines of code copied from rose_get_neigh().
The function is now called with fewer arguments and if
a NULL is returned then the next call to rose_transmit_clear_request()
will manage the error code. I verified that the frame routing was
actually handled as required on a FPAC/ROSE node with heavy BBS traffic 
through 8 adjacent nodes connected through radio ports and Internet 
AXUDP / AXIP connections.


 From 6708b3853a5ecf5c185942b3757223e451af1960 Mon Sep 17 00:00:00 2001
From: Bernard Pidoux <f6bvp@amsat.org>
Date: Sun, 13 Jan 2008 20:25:40 +0100
Subject: [PATCH 1/4] [ROSE] simplified rose_get_route()

rose_get_neigh() was called by two different functions.
Firstly, by rose_connect() in order to establish connections to
adjacent rose nodes. This worked correctly.
Secondly, it was called by rose_route_frame() to find a route
via an adjacent node. This was not working efficiently, for the
whole node neighbour list was not checked and the proper test
was not performed in order to check if a node was already connected.
We create new function rose_get_route() to achieve this.
It returns a ROSE node address for sending a frame via a connected node
to the specified destination address.If a NULL is returned, the
previous program behavior is performed. ROSE tries to initiate a connect 
to the appropriate adjacent node.

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
  net/rose/rose_route.c |   24 +++++++++++++++++++++---
  1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
index 540c0f2..5d6fac4 100644
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -662,6 +662,25 @@ struct rose_route *rose_route_free_lci(unsigned int 
lci, struct rose_neigh *neig
  }

  /*
+ *     Find an opened route given a ROSE address.
+ */
+static struct rose_neigh *rose_get_route(rose_address *addr)
+{
+       struct rose_node *node;
+       int i;
+
+       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 (node->neighbour[i]->restarted)
+                                       return node->neighbour[i];
+                       }
+               }
+       }
+       return NULL;
+}
+
+/*
   *     Find a neighbour given a ROSE address.
   */
  struct rose_neigh *rose_get_neigh(rose_address *addr, unsigned char 
*cause,
@@ -842,7 +861,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
         struct sock *sk;
         unsigned short frametype;
         unsigned int lci, new_lci;
-       unsigned char cause, diagnostic;
         struct net_device *dev;
         int len, res = 0;
         char buf[11];
@@ -1019,8 +1037,8 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb 
*ax25)
                 rose_route = rose_route->next;
         }

-       if ((new_neigh = rose_get_neigh(dest_addr, &cause, &diagnostic)) 
== NULL) {
-               rose_transmit_clear_request(rose_neigh, lci, cause, 
diagnostic);
+       if ((new_neigh = rose_get_route(dest_addr)) == NULL) {
+               rose_transmit_clear_request(rose_neigh, lci, 
ROSE_NOT_OBTAINABLE, 0);
                 goto out;
         }

--
1.5.3.7