Re: [mcip-list] Patch: let MCIP packets get a size up to 65535 bytes

[Christian T. <chr...@go...>]

Am 14.07.2014 09:31, schrieb Markus Lauterbach:
> On Sun, 13 Jul 2014 10:48:08 +0200
> Michael Kress <m-...@m-...> wrote:
>
>> Hello list,
>>
>> there is an artificial limit of 1500 bytes for a MCIP packet. The
>> length byte of the protocol is defined to be 2 bytes long, so this
>> limit does not make sense.
>>
>> The attached patch sets the maximum size of a packet to 65535 bytes.
>>
>> When MCIP reads from a device it needs a temporary buffer. This buffer
>> stays at a length of 4 kByte for the first read attempt - the size of
>> a RAM page. Most of the MCIP telegrams are typically only a few bytes
>> long, so the read will most likely get the whole telegram in one
>> chunk. In case of short telegrams the behaviour and thus the
>> performance should stay identical as without the patch.
>>
>> When MCIP once again reads from a device, because the telegram was too
>> long to be read at the first time, the temporary buffer will up to
>> 65535 bytes. This increases the chance, that a telegram can be read
>> completely at the second read() instead of further read chunks each
>> of 4 kByte. The disadvantage is, that on relatively slow connections
>> (e.g. TCP devices) read() still will only get a few bytes and the huge
>> temporary buffer is useless. I still prefer the huge temporary buffer,
>> because
>> - 64 kByte is not too huge on modern hardware
>> - the temporary buffer is released quite fast again, so it is mostly a
>>    performance issue
>> - long telegrams are seldom
>> - on RAM critical machines the limit can be reduced with #define
>> MCIP_MAX_READ_BUFFER
>>
>>
> Hi Michael,
>
> this patch does really make sense. Did not have tested it yet, but
> accepted from my side anyway.
>
> ciao
>   Markus
>

Hi,

the patch as is makes sense and looks (without testing) correct.

But there is an issue:
In libmcip there is the function mcip_read_data (libmcip/libmcip.c, line 
249).
This function is most likely used by the driver/application to read the 
data from the mcip demon. This function does only accept data until a 
maximum of 4096 bytes. If the amount of data exceeds 4096 bytes, this 
function will cause the application to discard the whole mcip packet 
(line 272).

So IMHO this patch only makes sense if the libmcip is patched 
accordingly (within the same patch) because without a patched library 
current applications (using this library) won't benefit from the change.

Regards,
Christian