Bernard Pidoux wrote, On 12/04/2007 11:26 PM:

> Jarek Poplawski wrote:
>> Bernard Pidoux wrote, On 12/02/2007 06:37 PM:
>>
>>> Hi,
>>>
>>> Many thanks for your patch for ~/net/ax25/ax25_subr.c
>>>
>>> Introduction of local_bh_disable() ... local_bh_enable()
>>>
>>> cured the inconsistent lock state related to AX25 connect timeout.
>>>
>>> I have now a stable monoprocessor system running AX25 and ROSE network 
>>> packet switching application FPAC, whether kernel is compiled with or 
>>> without hack option.
>>>
>>> There is no more problem during normal operations.
>>>
>>> This was achieved, thanks to your AX25 patch and the patch from Alexey 
>>> Dobriyan for rose module.
>>>
>>> I also patched rose module in order to get packet routing more 
>>> efficient, taking into account the "restarted" flag that is raised when 
>>> a neighbour node is already connected.
>>>
>>> To summarize the present situation on my Linux machine, I built a patch 
>>> against kernel 2.6.23.9.
>>>
>>> I would appreciate if you could make it included into a next kernel release.
>> ... 
>>
>> Bernard, I'm very glad I could be a little helpful, but I'm not sure of
>> your intentions: my patch proposal is rather trivial interpretation of
>> lockdep's report; I haven't studied AX25 enough even to be sure there is
>> a real lockup possible in this place. Since this change looks not very
>> costly and quite safe, I can 'take a risk' to sign this off after your
>> testing. But anything more is beyond my 'range'.
>>
>> So, since you've spent quite a lot of time on this all, maybe it would
>> be simpler if you've tried the same with the current kernel, and resent
>> "proper" (not gzipped and with changelog) patch or patches. Then, I hope,
>> Ralf, as the maintainer, will make the rest.
>>
>> Regards,
>> Jarek P.
>>
>>
> 
> As required I send again in clear text the summary of ax25 and rose 
> patch against 2.6.23.9.
> As I said, the kernel stability, after applying these patch, is behind us.
> I did not observe any warning nor lockup after a few days of AX25 
> intensive use.
> Also rose module is handling routing of frames much more efficiently.
> 
> This will considerably help us to focus on application programs now.
> I am now concentrating my efforts on ROSE/FPAC and Linux FBB code 
> adjustement.
> 
> Thanks to you all for your help.
> 
> 73 de Bernard, f6bvp
> 
> 
> 


Bernard, I think you've forgotten at least about some distinct
changelog and signed-off-by?

Jarek P.
---

ax25_subr.c part:
Acked-by: Jarek Poplawski <[EMAIL PROTECTED]>

> ------------------------------------------------------------------------
> 
> diff -pruN a/include/net/rose.h b/include/net/rose.h
> --- a/include/net/rose.h      2007-10-12 18:43:44.000000000 +0200
> +++ b/include/net/rose.h      2007-12-01 23:56:57.000000000 +0100
> @@ -202,6 +202,7 @@ extern struct net_device *rose_dev_first
>  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 *);
> diff -pruN a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
> --- a/net/ax25/ax25_subr.c    2007-10-12 18:43:44.000000000 +0200
> +++ b/net/ax25/ax25_subr.c    2007-12-01 23:32:01.000000000 +0100
> @@ -279,6 +279,7 @@ void ax25_disconnect(ax25_cb *ax25, int 
>       ax25_link_failed(ax25, reason);
>  
>       if (ax25->sk != NULL) {
> +             local_bh_disable();
>               bh_lock_sock(ax25->sk);
>               ax25->sk->sk_state     = TCP_CLOSE;
>               ax25->sk->sk_err       = reason;
> @@ -288,5 +289,6 @@ void ax25_disconnect(ax25_cb *ax25, int 
>                       sock_set_flag(ax25->sk, SOCK_DEAD);
>               }
>               bh_unlock_sock(ax25->sk);
> +             local_bh_enable(); 
>       }
>  }
> diff -pruN a/net/rose/af_rose.c b/net/rose/af_rose.c
> --- a/net/rose/af_rose.c      2007-10-12 18:43:44.000000000 +0200
> +++ b/net/rose/af_rose.c      2007-12-02 10:06:31.000000000 +0100
> @@ -62,7 +62,7 @@ int sysctl_rose_window_size             
>  static HLIST_HEAD(rose_list);
>  static DEFINE_SPINLOCK(rose_list_lock);
>  
> -static struct proto_ops rose_proto_ops;
> +static const struct proto_ops rose_proto_ops;
>  
>  ax25_address rose_callsign;
>  
> @@ -741,7 +741,7 @@ static int rose_connect(struct socket *s
>       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;
> @@ -773,7 +773,6 @@ static int rose_connect(struct socket *s
>  
>               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;
> @@ -835,12 +834,6 @@ rose_try_next_neigh:
>       }
>  
>       if (sk->sk_state != TCP_ESTABLISHED) {
> -     /* Try next neighbour */
> -             rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, 
> &diagnostic);
> -             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;
> @@ -1481,7 +1474,7 @@ static struct net_proto_family rose_fami
>       .owner          =       THIS_MODULE,
>  };
>  
> -static struct proto_ops rose_proto_ops = {
> +static const struct proto_ops rose_proto_ops = {
>       .family         =       PF_ROSE,
>       .owner          =       THIS_MODULE,
>       .release        =       rose_release,
> Les fichiers linux-2.6.23.9-orig/net/rose/rose.ko et 
> linux-2.6.23.9/net/rose/rose.ko sont différents.
> diff -pruN a/net/rose/rose_route.c b/net/rose/rose_route.c
> --- a/net/rose/rose_route.c   2007-10-12 18:43:44.000000000 +0200
> +++ b/net/rose/rose_route.c   2007-12-02 00:15:24.000000000 +0100
> @@ -664,25 +664,22 @@ struct rose_route *rose_route_free_lci(u
>  /*
>   *   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 @@ struct rose_neigh *rose_get_neigh(rose_a
>               *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 @@ int rose_route_frame(struct sk_buff *skb
>               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;
>       }
> 


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

Reply via email to