Re: Comments about IPT_ALIGN

[Thomas Heinz <creatix@hipac.org>]

Hi Laszlo

You wrote:
>>     - limit: struct ipt_rateinfo {
>>                      u_int32_t avg;    /* Average secs between packets 
>>* scale */
>>                      u_int32_t burst;  /* Period multiplier for upper 
>>limit. */
>>                     /* Used internally by the kernel */
>>                     unsigned long prev;
>>                     u_int32_t credit;
>>                     u_int32_t credit_cap, cost;
>>
>>                     /* Ugly, ugly fucker. */
>>                     struct ipt_rateinfo *master;
>>              };
>>
>>       The unsigned long prev in the middle does not hurt because it
>>       is automatically 8 byte aligned by avg and burst. Because
>>       the pointer (master) is at the end of the struct it does not
>>       cause any problems except that the whole struct must be
>>       aligned correctly.
> 
> The structure is aligned correctly, however the offsets of the fields,
> and sizeof() is different, so it does not work.

Ah yes of course, as the size of unsigned long differs in user and
kernel space every member after prev has a different offset.
So the limit match is a valid example :)

> Well, I wrote a hack that makes the layout the same, but that's
> rather ugly (I call it workaround :).

I just flew over it. Sorry, I must have overlooked it before.

>>BTW, pointers and arrays of pointers within target or match
>>structs can be forced to have the 8 byte alignment requirement
>>(on sparc64 and parisc64) by simply putting them into a
>>union: union {void *p; __u64 pad;}
> 
> Yes, that's true, except for the fact that you will have to deal
> with endianness as well, if you are interested in actually exchanging
> data... 

The only match/target I'm aware of that has a pointer in its info
struct is the limit match. Generally this is necessary for each
match/target that does not treat the info data as read only on a
smp system (at least with the current infrastructure as each cpu has
its own match/target info).

Anyway, I don't see a reason why one wants to exchange a valid pointer
between user and kernel space.

> And don't forget the size change that brings in on every 32-bit
> platform effectively killing compatibility...

One could use #ifdef's for compatibility purpose but as I read you
don't like these :-)


Thomas