[GitHub] [incubator-nuttx] patacongo edited a comment on issue #188: Fixed USB RNDIS not to send over MTU packet

[GitBox <gi...@apache.org>]

patacongo edited a comment on issue #188: Fixed USB RNDIS not to send over MTU packet
URL: https://github.com/apache/incubator-nuttx/pull/188#issuecomment-580740973
 
 
   There is a lot more that we would need to discuss to decide what to do with CONFIG_NET_GUARDSIZE.  Currently it is a very small value, defaulting to only two bytes.  So it is not a memory hog and the simplest response is to keep it present in all drivers for commonality.
   
   It should (eventually) be larger and common for all drivers.  We need to look at how it is used today and how it might be used tomorrow.  There is a probably a lot more involved that you are considering.  It is not just for hardware packet receipt:
   
   For packet receipt, it is necessary for some hardware, but not others.  Often the hardware will DMA  2 byte FCS at the end of the package, or other hardware-specific info.  That is the driver-specific part that you are considering but that is only part of the whole story.
   
   There are several issues for packet transmission.  These are less well defined and need further study, but we need to keep all of the driver packet definitions in place until we understand how we are going to handle these things:
   
   1. Memory overrun bug.  There was in the past, a bug that caused write past the end of the buffer by a couple of bytes during TX message formatting.  I don't know if that bug still exists, but the minimum CONFIG_NET_GUARDSIZE was sufficient to eliminate the bug.  That is why it has the name GUARD:  Its primary purpose is to protect from overrunning the packet buffer and corrupting the following memory.
   
   I do no know if we have any such bugs today.  Perhaps they do?  Perhaps they do not?  Having such a guard is a good thing for reliability in any case.
   
   2. Variable size IP/TCP headers.  There is a limitation in the way IP packets are formatted now.  Basically they are formatted like this:
   
   - When the packet is received a pointer to the location of the payload is set (d_appdata).  This is an offset into the packet buffer  For TCP, that accounts for the MAC/Ethernet header, the minimum IPv4/IPv6 header size, and the minimum TCP header size.
   - The TCP payload data is written at that location using a constant MSS,
   - The minimum sized IPv4/IPv6 headers and the minimum sizde TCP header are added below the payload, and finally
   - The MAC/Ethernet header as add at the very beginning of the buffer.
   
   The problem is, of course, is that IPv4/IPv6 and TCP headers are not constant.  Using the minimum size for these headers precludes using IPv4/IPv6 or TCP header options.  There are currenly only a few places where options are required and these haveto be handled as special cases.  This needs to be formalized into the common packet formatting logic.  Otherwise, the network will never be able to handle advanced features like tunneling or NAT.
   
   I am thinking that this should be like:
   
   - When the packet is received a pointer to the location of the payload is set (d_appdata).  This is an offset into the packet buffer  For TCP, that accounts for the MAC/Ethernet header, the **maximum** IPv4/IPv6 header size, and the **maximum** TCP header size.
   - The TCP payload is written at that location,
   - The **correctly** sized IPv4/IPv6 headers and the **correctly** sized TCP header are added below the payload, and finally
   - The MAC/Ethernet header as added.
   - The start offset of the packet in the packet is no longer zero, but some variable offset into the packet buffer.
   
   The key to making this all work is:
   
   - Keep CONFIG_NET_GUARDSIZE in all driver buffers, and
   - Set the CONFIG_NET_GUARDSIZE to the maximum size of IPv4/IPv6 and TCP options (depending on which IP enabled and if TCP is enabled)
   
   3. Variable MSS.  Closely related to this is the MSS which is the maximum size of the payload.  Currently that is a constant because it assumes the minimum header lengths.  It should be variable.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services