[Top][All Lists]

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

[lwip-devel] [patch #6537] wnd_scale TCP option addition

From: Jonathan Larmour
Subject: [lwip-devel] [patch #6537] wnd_scale TCP option addition
Date: Fri, 20 Jun 2008 14:46:35 +0000
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20070530 Fedora/ Firefox/

Update of patch #6537 (project lwip):

                  Status:                    None => Need Info              
             Assigned to:                    None => jifl                   


Follow-up Comment #1:

I think this patch looks good, although would suggest a few tweaks, and I
have 2 questions.

First the questions: 
1) why are the various window sizes in tcp.h changed to u32_t - I thought
they were still limited to 65536?

2) why is snd_queuelen increased to u32_t - I can't foresee having more than
65536 segs for a single PCB!

As for some suggested tweaks, rather than trying to bolt-on optdata2, and do
a memcpy, I think we should just rationalise the option addition into a single
new function in tcp.c (which tcp_in.c could call), which allows for some
future additions.

Something like
typedef u32_t tcp_option_data[2];
typedef u32_t tcp_option_data;

void tcp_build_options( struct tcp_pcb *pcb, tcp_option_data *optdata, u8_t
*optlen );

And then the tcp_enqueue stuff becomes something like:
tcp_option_data optdata;
u16_t optlen;
tcp_build_options(npcb, &optdata, &optdatasize)'
tcp_enqueue(npcb, NULL, 0, TCP_SYN | TCP_ACK, 0, (u8_t *)&optdata, optlen);

To make the code more readable, it may be nice to have and use the following
macros in tcp.h:
#define RCV_WND_SCALE(pcb, wnd) ((wnd) >> (pcb)->rcv_scale)
#define SND_WND_SCALE(pcb, wnd) ((wnd) << (pcb)->snd_scale)
#define RCV_WND_SCALE(pcb, wnd) (wnd)
#define SND_WND_SCALE(pcb, wnd) (wnd)

So that elsewhere we could have just something like e.g.
seg->tcphdr->wnd = RCV_WND_SCALE(pcb, htons(pcb->rcv_ann_wnd));

Would you be able to look at doing it that way? I hope it should be an easy
change and I think makes the patch a bit nicer.

Thanks for contributing back!



Reply to this item at:


  Message sent via/by Savannah

reply via email to

[Prev in Thread] Current Thread [Next in Thread]