This is an old revision of the document!


TCP cleanup

Cleanup TCP close actions

  • Reduce duplicate code by separating tcp_terminate() from tcp_close().
  • Use tcp_terminate to immediately free all tcp resources.
  • Use tcp_close() to bing TCP connection to closing state.
  • Make tcp_close() function simpler, not doing different job in different state.

Trace gPXE boot initialize steps about memory environment setup

  • After recent Michael Brown's [prefix] patch-set:
    • [prefix] Add A20-enabling code in libflat
    • [prefix] Move flatten_real_mode to libflat.S
    • [prefix] Move flatten_real_mode to .text16.early
    • [prefix] Add .text16.early section
    • [prefix] Use flat real mode for access to high memory
    • [prefix] Use flat real mode instead of real mode
  • gPXE now works fully on Flat real mode.
  • Here is another nice document explains it.
  • Which solved the problem that big image relocate failure.
  • We can now safely modify the heap_size as we want. :)

Michael's reply of my recent patchset

Thank you for your TCP patches, some of which I have applied directly to iPXE,
some of which have inspired me to make my own improvements.  I have not been
commenting on your patches, but I have carefully reviewed every version of
your patch set, picked out those that I want to apply or rework, and
maintained a list of those that are not yet integrated into iPXE.  At this
point, I'm down to a list of three patches:

[tcp] Receive and Close flow adjustment

This is difficult to apply following the changes to support out-of-order
packets, and I'm not sure how valuable it is.  Since commit 9ff8229 ("[tcp]
Update received sequence number before delivering received data"), which fixes
a problem that you identified (thank you!), I think that the TCP state is
consistent at the time the data is delivered to the upper layer, so any
actions it might take are safe.

[tcp] Fix possible misjudged SYN/FIN ACKed status

I believe this is unnecessary; I'm pretty sure that iPXE will never send SYN
or FIN in a packet that also contains data.  Therefore, if new data is ACKed
while we are sending one of these flags, it must be ACKing these flags.  Have
you found a way in which it is possible for us to send both SYN/FIN and data
at the same time?

[tcp] Distinguish passive and active close with proper actions

This one I have not yet reviewed thoroughly, though I am expecting that it
will be applied in some form.

I am sorry that you have been disheartened by the lack of comments and
feedback on the gPXE mailing list.  I tend to avoid commenting on the gPXE
lists, because I do not find them to be a welcoming environment any more.

Michael
On Thursday 22 Jul 2010 11:09:23 Michael Brown wrote:
> [tcp] Receive and Close flow adjustment
> 
> This is difficult to apply following the changes to support out-of-order
> packets, and I'm not sure how valuable it is.  Since commit 9ff8229 ("[tcp]
> Update received sequence number before delivering received data"), which
> fixes a problem that you identified (thank you!), I think that the TCP
> state is consistent at the time the data is delivered to the upper layer,
> so any actions it might take are safe.

Ignore that; I've just worked out that this patch makes perfect sense *if* we
also distinguish between passive and active close, and I see how it can be
implemented cleanly on top of the recent out-of-order changes.

tcp_rx_data() should add the I/O buffer to a list rather than delivering it via
xfer_deliver_iob().  tcp_rx_fin() should not call tcp_close().
tcp_process_rx_queue() should be adjusted to do something like:

  struct list_head received = LIST_HEAD_INIT ( received );
  ...
  while ( ! list_empty ( &tcp->rx_queue ) ) {
     ...
     tcp_rx_data ( tcp, seq, iob_disown ( iobuf ), &received );
     ...
  }

  list_for_each_entry_safe ( iobuf, tmp, &received, list ) {
    // deliver iobuf via xfer_deliver_iob()
  }

  if ( tcp->state & TCP_STATE_RCVD ( TCP_FIN ) )
    tcp_close ( tcp, 0 );

I think that would handle everything sensibly, and would mean that we could
properly handle passive close since, in the case of a received data+FIN packet
that will also cause our higher layer protocol (e.g. http.c) to close the
connection, tcp_rx_fin() would see the FIN before http.c called xfer_close().

Does this seem correct to you?  If so, would you like to put together a patch?

Michael

QR Code
QR Code soc:2010:cooldavid:journal:week9 (generated for current page)