[PATCH] ax25.h unsigned long type for ax25 timers

[PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

In linux/include/linux/ax25.h
ax25_info_struct timers types remain unsigned

struct ax25_info_struct {
	unsigned int	n2, n2count;
	unsigned int	t1, t1timer;
	unsigned int	t2, t2timer;
	unsigned int	t3, t3timer;
	unsigned int	idle, idletimer;
        .....
};

while in linux/include/net/ax25.h timers are unsigned long according
to kernel 2.6 timers.

typedef struct ax25_cb {
        .....
        struct timer_list       t1timer, t2timer, t3timer, idletimer;
        unsigned long           t1, t2, t3, idle, rtt;
        ....

Although ax25_info_struct is not much used it is refered at least
into libax25 and xfbb BBS application program.

It seems thus reasonable to make the change.

Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
---
 include/linux/ax25.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/ax25.h b/include/linux/ax25.h
index 56c11f0..75282bb 100644
--- a/include/linux/ax25.h
+++ b/include/linux/ax25.h
@@ -97,10 +97,10 @@ struct ax25_info_struct_deprecated {
 
 struct ax25_info_struct {
 	unsigned int	n2, n2count;
-	unsigned int	t1, t1timer;
-	unsigned int	t2, t2timer;
-	unsigned int	t3, t3timer;
-	unsigned int	idle, idletimer;
+	unsigned long	t1, t1timer;
+	unsigned long	t2, t2timer;
+	unsigned long	t3, t3timer;
+	unsigned long	idle, idletimer;
 	unsigned int	state;
 	unsigned int	rcv_q, snd_q;
 	unsigned int	vs, vr, va, vs_max;
-- 
1.5.5

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux <bpidoux@free.fr>
Date: Tue, 17 Jun 2008 15:04:38 +0200

> In linux/include/linux/ax25.h
> ax25_info_struct timers types remain unsigned
> 
> struct ax25_info_struct {
> 	unsigned int	n2, n2count;
> 	unsigned int	t1, t1timer;
> 	unsigned int	t2, t2timer;
> 	unsigned int	t3, t3timer;
> 	unsigned int	idle, idletimer;
>         .....
> };
> 
> while in linux/include/net/ax25.h timers are unsigned long according
> to kernel 2.6 timers.
> 
> typedef struct ax25_cb {
>         .....
>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>         unsigned long           t1, t2, t3, idle, rtt;
>         ....
> 
> Although ax25_info_struct is not much used it is refered at least
> into libax25 and xfbb BBS application program.
> 
> It seems thus reasonable to make the change.
> 
> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>

Unfortunately this datastructure is exported to userspace,
and therefore we cannot change the structure layout without
breaking userspace.

We cannot, as a result, make this change.

The only way to fix this is to make a new fixed structure, and add new
ax25 calls that accept this new structure.

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

David,

Unfortunately I don't quite understand exactly why a structure in user 
space cannot be changed.
But this means that I have to make some effort to learn a bit more about 
the kernel structure and userspace.
However, thank you for your explanation about how to fix the discrepancy 
between both timer structures.
I will try to dig out more information on this subject.

Bernard Pidoux, f6bvp
 

David Miller a écrit :
> From: Bernard Pidoux <bpidoux@free.fr>
> Date: Tue, 17 Jun 2008 15:04:38 +0200
>
>   
>> In linux/include/linux/ax25.h
>> ax25_info_struct timers types remain unsigned
>>
>> struct ax25_info_struct {
>> 	unsigned int	n2, n2count;
>> 	unsigned int	t1, t1timer;
>> 	unsigned int	t2, t2timer;
>> 	unsigned int	t3, t3timer;
>> 	unsigned int	idle, idletimer;
>>         .....
>> };
>>
>> while in linux/include/net/ax25.h timers are unsigned long according
>> to kernel 2.6 timers.
>>
>> typedef struct ax25_cb {
>>         .....
>>         struct timer_list       t1timer, t2timer, t3timer, idletimer;
>>         unsigned long           t1, t2, t3, idle, rtt;
>>         ....
>>
>> Although ax25_info_struct is not much used it is refered at least
>> into libax25 and xfbb BBS application program.
>>
>> It seems thus reasonable to make the change.
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>     
>
> Unfortunately this datastructure is exported to userspace,
> and therefore we cannot change the structure layout without
> breaking userspace.
>
> We cannot, as a result, make this change.
>
> The only way to fix this is to make a new fixed structure, and add new
> ax25 calls that accept this new structure.
>
>
>   

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

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux <bpidoux@free.fr>
Date: Thu, 19 Jun 2008 18:28:46 +0200

> Unfortunately I don't quite understand exactly why a structure in user 
> space cannot be changed.

Because there are userland binaries already compiled out there using
the current structure layout.  If we change the kernel, those binaries
stop using a datastructure that matches what the kernel is using.
Such binaries will break.

In generic, because of this, you can never change the layout of a
data structure exported to userland as a kernel interface.

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux]

David,

Your explanation is perfectly clear and I understand the problem now.
Thank you very much for taking the time to explain me that.

Bernard Pidoux




David Miller wrote:
> From: Bernard Pidoux <bpidoux@free.fr>
> Date: Thu, 19 Jun 2008 18:28:46 +0200
> 
>> Unfortunately I don't quite understand exactly why a structure in user 
>> space cannot be changed.
> 
> Because there are userland binaries already compiled out there using
> the current structure layout.  If we change the kernel, those binaries
> stop using a datastructure that matches what the kernel is using.
> Such binaries will break.
> 
> In generic, because of this, you can never change the layout of a
> data structure exported to userland as a kernel interface.
> 
> 

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[Bernard Pidoux F6BVP]

There have been in the past already some modifications of 
ax25_info_struct and the old one was renamed deprecated.

/* this will go away. Please do not export to user land */
struct ax25_info_struct_deprecated {
         unsigned int    n2, n2count;
         unsigned int    t1, t1timer;
         unsigned int    t2, t2timer;
         unsigned int    t3, t3timer;
         unsigned int    idle, idletimer;
         unsigned int    state;
         unsigned int    rcv_q, snd_q;
};

Three new items were included :

...
         unsigned int    vs, vr, va, vs_max;
         unsigned int    paclen;
         unsigned int    window;
};

At the same time  case SIOCAX25GETINFOOLD:
was added in ax25_ioctl()
and would trigger a warning message.


  /* old structure? */
                 if (cmd == SIOCAX25GETINFOOLD) {
                         static int warned = 0;
                         if (!warned) {
                                 printk(KERN_INFO "%s uses old 
SIOCAX25GETINFO\n",
                                         current->comm);
                                 warned=1;
                         }

                         if (copy_to_user(argp, &ax25_info, 
sizeof(struct ax25_info_struct_deprecated))) {
                                 res = -EFAULT;
                                 break;
                         }
                 } else {
                         if (copy_to_user(argp, &ax25_info, 
sizeof(struct ax25_info_struct))) {
                                 res = -EINVAL;
                                 break;
                         }
                 }
                 res = 0;
                 break;

Here is my question :

Can't we remove the probably quite old ax25_info_struct_deprecated 
structure, and rename the present ax25_info_struct, 
ax25_info_struct_deprecated, then create the new ax25_info_struct with 
resized timers unsigned long ?

The second solution is to declare a new structure

* new ax25 info struct */
struct ax25_info_long_timers_struct {
         unsigned int    n2, n2count;
         unsigned long   t1, t1timer;
         unsigned long   t2, t2timer;
         unsigned long   t3, t3timer;
         unsigned long   idle, idletimer;
         unsigned int    state;
         unsigned int    rcv_q, snd_q;
         unsigned int    vs, vr, va, vs_max;
         unsigned int    paclen;
         unsigned int    window;
};

adding another case SIOCAX25GETNEWINFO
that would be defined
#define SIOCAX25GETNEWINFO         (SIOCPROTOPRIVATE+14)

and trigger warning message by calls to SIOCAX25GETINFOOLD and
SIOCAX25GETINFO.

It that the right way to do it ?


Bernard, f6bvp



Bernard Pidoux wrote:
> David,
> 
> Your explanation is perfectly clear and I understand the problem now.
> Thank you very much for taking the time to explain me that.
> 
> Bernard Pidoux
> 
> 
> 
> 
> David Miller wrote:
>> From: Bernard Pidoux <bpidoux@free.fr>
>> Date: Thu, 19 Jun 2008 18:28:46 +0200
>>
>>> Unfortunately I don't quite understand exactly why a structure in 
>>> user space cannot be changed.
>>
>> Because there are userland binaries already compiled out there using
>> the current structure layout.  If we change the kernel, those binaries
>> stop using a datastructure that matches what the kernel is using.
>> Such binaries will break.
>>
>> In generic, because of this, you can never change the layout of a
>> data structure exported to userland as a kernel interface.
>>
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] ax25.h unsigned long type for ax25 timers

[David Miller]

From: Bernard Pidoux F6BVP <f6bvp@free.fr>
Date: Fri, 20 Jun 2008 15:17:39 +0200

> The second solution is to declare a new structure
> 
> * new ax25 info struct */
> struct ax25_info_long_timers_struct {
 ...
> 
> adding another case SIOCAX25GETNEWINFO
> that would be defined
> #define SIOCAX25GETNEWINFO         (SIOCPROTOPRIVATE+14)
> 
> and trigger warning message by calls to SIOCAX25GETINFOOLD and
> SIOCAX25GETINFO.
> 
> It that the right way to do it ?

Yes.