Re: [Chromium-dev] Fixing the pack SPU's mtu usage

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Here is Samual's reasoning for the change, from a previous discussion:

When udptcpip.c was integrated in the main stream, the buffer size got to
5/4 of the current MTU (from conf, or detected by udp), see in
tilesortspu_config.c:252, so that with the packing strategy drawn above,
the current MTU will always be filled (there is always at least 4 bytes of
data for a 1 byte opcode), even if there is only one opcode and a big
bunch of data taking 4/5 of the buffer size, ie the whole MTU.

But it indeed seems not apply to myrinet/gm, where the buffer size should
be chooseable and no MTU would then be needed, only the buffer size
shouldn't be overcome (which is ensured in crPackCanHold*()).

<after having a quick look at the code>

Hum, indeed, everything is messed up a bit everywhere because lack of
clear documentation, I guess...

To my mind, what could be fine would be that network layers could choose
their own buffer size and MTU, with the possibility of using the
configured ones, and asking the packing code for the magic 5/4 ratio to
get sure the MTU will be filled in the udp case. myrinet/gm will then use
the configured buffer size and set an infinite MTU, and udp/tcp will set
the discovered mtu and ask the packing code for computing the
corresponding optimal buffer size. The result would be stored in the
CRConnection structure. Then everything using buffers for a particular
connection should use that connection's parameters.

Btw, the code at tilesortspu_context.c:105 is for udp/tcp: if the
connection mtu is less than the main one, the main one should be reduced
to it. Of course, this would be unnecessary if we knew exactly what should
go to which connection and adapt the MTU accordingly. But for instance,
the geometry buffer shouldn't get filled more than the main MTU (ie the
minimum of all MTUs of every connection from the tilesortSPU) minus
(24+END_FLUFF+4+4) (extra room for the packing magics, see
tilesortspu_init.c:85). So that the geometry buffer uses buffers of the
common size, but the mtu is a bit less than the minimum of all MTUs. If
this is not ensured, UDP can't work.

Well, it is a bit hairy, indeed. I don't have time to track & correct
everything right now, but maybe in the following week or two.

Regards,
Samuel

---------------------------------------------------------------------------------------------

I'm going to yank the code out of the tilesort and the pack spu for now 
so that we don't fubar the other network layers.  It seems that Samuel 
can figure out a way to work around the problem in the UDP layer 
itself.  It seems to me that the problems of MTU sizes would have to be 
special cased everywhere if not purely handled in the network layer 
itself.  I'm guessing that the UDP code is going to need to get smarter 
and fragment packets itself.  But, this is a slippery slope in that 
handling fragmentation in the UDP layer starts to make it look alot like 
the TCP layer.

-Mike

Brian Paul wrote:

> Greg Humphreys wrote:
>
>> This code is more complex than the original one that I wrote back in the
>> day, and I'm not sure where it came from.
>>
>> Mike -- can you track down by a binary-search of 'cvs diff' when this 
>> was
>> added, and by whom, and for what reason?
>
> >
>
>> Typically code gets checked in for a reason; it would be nice to know 
>> what
>> that was.
>
>
>
> Looks like it was added on 2002/04/30 in version 1.13.2.1 when I 
> checked in a patch from Samuel Thibault:
>
> "Applied Samuel Thibault's patch for IPv6."
>
> http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/chromium/cr/spu/pack/packspu_config.c.diff?r1=1.13&r2=1.13.2.1 
>
>
>
> -Brian
>
>
>
>> -- 
>> Greg Humphreys, Assistant Professor
>> Department of Computer Science, University of Virginia
>> hu...@cs...
>>
>>
>>> -----Original Message-----
>>> From: chr...@li... 
>>> [mailto:chr...@li...] On Behalf Of 
>>> Michael Houston
>>> Sent: Monday, September 15, 2003 2:58 PM
>>> To: chromium-dev
>>> Subject: [Chromium-dev] Fixing the pack SPU's mtu usage
>>>
>>>
>>> Here is the code that I have a problem with from packspu_config:
>>>
>>> /* get a buffer which can hold one big big opcode (counter computing
>>>     * against packer/pack_buffer.c) */
>>>    pack_spu.buffer_size = ((((pack_spu.buffer_size - 
>>> sizeof(CRMessageOpcodes)) * 5 + 3) / 4 + 0x03) & ~0x03) + 
>>> sizeof(CRMessageOpcodes);
>>>
>>> WTF?!  Why are we trying to counter the computing in 
>>> crPackcanHoldOpcode?  This seems completely bogus to me.   Here is 
>>> the code in crPackCanHoldOpcode:
>>>
>>>    fitsInMTU = (((pc->buffer.data_current - 
>>> pc->buffer.opcode_current - 1
>>>               + num_opcode + num_data
>>>               + 0x3 ) & ~0x3) + sizeof(CRMessageOpcodes)
>>>             <= pc->buffer.mtu);
>>>
>>> Regardless of what we set the packspu's buffer size to, the code 
>>> above does the write thing.  As I read it, the fucntions figures out 
>>> if it can fit another opcode and data in the current buffer.
>>>
>>> I have taken out the line in packspu_config and things still seem to 
>>> work.  This causes havoc on the network layers and doesn't seem to 
>>> have a real purpose.  I would like to remove it unless someone can 
>>> give a good clear explanation of why we must essentially do 5/4 mtu. 
>>> My main objection is that if the user sets an MTU they probably want 
>>> that EXACT MTU used!
>>>
>>> The tilesort SPU does the same thing!  We need to fix the packer to 
>>> do the right thing and not force the SPU's to do funky math to get 
>>> the "right"
>>> sized buffer.
>>>
>>> -Mike
>>>
>>>
>>>
>>> -------------------------------------------------------
>>> This sf.net email is sponsored by:ThinkGeek
>>> Welcome to geek heaven.
>>> http://thinkgeek.com/sf
>>> _______________________________________________
>>> Chromium-dev mailing list
>>> Chr...@li...
>>> https://lists.sourceforge.net/lists/listinfo/chromium-dev
>>>
>>
>>
>>
>>
>> -------------------------------------------------------
>> This sf.net email is sponsored by:ThinkGeek
>> Welcome to geek heaven.
>> http://thinkgeek.com/sf
>> _______________________________________________
>> Chromium-dev mailing list
>> Chr...@li...
>> https://lists.sourceforge.net/lists/listinfo/chromium-dev
>>
>>
>
>
>
>
> -------------------------------------------------------
> This sf.net email is sponsored by:ThinkGeek
> Welcome to geek heaven.
> http://thinkgeek.com/sf
> _______________________________________________
> Chromium-dev mailing list
> Chr...@li...
> https://lists.sourceforge.net/lists/listinfo/chromium-dev