Re: [lwip-devel] MSG_MORE flag for send

[Simon Goldschmidt]

Am 27.10.2007 um 16:45 schrieb Jonathan Larmour:

> Simon Goldschmidt wrote:
> > Interesting patch (this functionality is required by the TCP RFCs  
> > by  the way),
>
> Are you sure about that? We already only set PSH on the last  
> segment of enqueued data from a single 'write', so isn't that all  
> that is required? It seems more to me that this is an extension to  
> give the user more control over when to send a PSH, rather than it  
> being at the end of every 'write'.

Sorry, seems I remembered it wrong, it's a MAY in RFC 1122 (page  
107), not a MUST.

> > but I'd rather solve this without adding an extra parameter  to a  
> > load of functions throughout the whole stack (I can already hear   
> > people saying' why add an extra parameter (which might be put on  
> > the  stack when calling the funciton) for something I don't need?').
>
> I can see you glancing at me ;-). But yes, I think this patch would  
> need more discussion and development. I am a bit concerned with the  
> consequences of running out of memory (segs, pbufs, netbufs, ...) -  
> much more likely for us than other stacks of course. My concern,  
> which may be unjustified, is that PSH can affect the remote end's  
> behaviour for ACKs. And if so, could result in unnecessary delays  
> before remote ACKs are transmitted, leading to a stalled local  
> unacked queue. I'd have to look more closely at the RFCs and  
> existing practice to find out for sure admittedly.

Agree with you there.

> Certainly, adding an extra param is not a requirement; this could  
> be done with a separate API message to set a flag in the pcb, e.g.  
> in tcp.h:

But that would set the push flag for every segment. I'd rather let  
netconn_write put the flag into the message (flags are supported in  
socket API, why not in netconn API?). Then netconn layer can set the  
flag, call TCP and set it back again.

> #define TF_DELAY_PSH (u8_t)0x80U
>
> And then in tcp_enqueue:
>    if (seg != NULL && seglen > 0 && seg->tcphdr != NULL &&
>        0 == (pcb->flags & TF_DELAY_PSH) ) {
>      TCPH_SET_FLAG(seg->tcphdr, TCP_PSH);
>    }
>
> New equivalent functions can be provided in the higher APIs. The  
> advantage there being that they are left out if unused (if using a  
> linker with dead function garbage collection).
>
> Rather than there being a specific API message to set this flag  
> (which would take space that couldn't be removed without an  
> lwipopts.h define), it may be better to have one to get/set any PCB  
> flag, which has more generic purpose.

That might be a good idea, much like get/setsockopt.

Simon