Re: [PATCH 1/2] Add new input plugin UNIXSOCK

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Pierre Chifflier wrote:
> On Sun, Feb 28, 2010 at 05:28:38PM +0100, Pablo Neira Ayuso wrote:
>>>>> +struct ulogd_unixsock_option_t  {
>>>>> +	uint16_t option_id;
>>>>> +	uint16_t option_length;
>>>>> +	char     option_value[0];
>>>>> +} __attribute__((packed));
>>>>> +
>>>>> +#define ALIGN_SIZE 8
>>>> Minor question: why align this to 64 bits?
>>> I originally used an alignment to 32 bits, but Jan noticed it would
>>> break if using options/values on 64 bits (and a test confirmed that). I
>>> took 64 bits as the biggest allowed value for integers.
>> I would need to look into this in more detail, not sure where the
>> problem is. I think that you can use something like `struct nlattr' (see
>> include/linux/netlink.h) and then nla_put() to add attributes in the TLV
>> format (see lib/nlattr.c). Those are align-safe. I'm using something
>> similar for conntrackd for the synchronization messages (src/build.c and
>> src/parse.c).
> 
> Yes, this is very similar though NLA_ALIGNTO is set to 4 which will
> cause problems with 64 bits integers.
> The other way to solve this would be to read integers byte per byte,
> like in [1], but I found this not very elegant (and is likely to be slow
> compared to aligned access).
> 
> Or do you have any preferred solution ? Maybe using nlattr + one special
> function for dealing with 64 bits variables ?

Aligning this to 8 bytes seems fine to me (messages would be bigger
though). I think that you can use something like:

struct ulogd_unixsock_option_t  {
	uint32_t option_id;
	uint32_t option_length;
	char     option_value[0];
} __attribute__((packed));

So you can benefit from the extra bytes that would be added as padding.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux