patchlwIP - A Lightweight TCP/IP stack - Patches: patch #7135, Significant etharp optimization...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #7135: Significant etharp optimization for TX speed improvement

Submitter:  Bill Auerbach <billauerbach>
Submitted:  Fri 26 Mar 2010 04:59:33 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Mon 29 Mar 2010 02:17:32 PM UTC, comment #7: 

That might not be a common case, but it's still valid. Suppose an administrator exchanged network interfaces for a PC. If our ARP table wouldn't be updated when we see a corresponding packet, we would never be able to reach that PC unless the open connections are dropped (which can be a long time for TCP).

The current implementation has the downside that (if ETHARP_TRUS_IP==0) the timer is not reset when receiving packets (which, at the ARP layer, would be the only detection of having an open connection), so an ARP entry will always time out after the max-time (which is quite long though).

Simon Goldschmidt <goldsimon>
Group administrator
Mon 29 Mar 2010 12:48:44 PM UTC, comment #6: 

This is good - sorry for a half-baked patch.  What does bother me is why should the ARP table be involved at all if there are opened connections to the IP address?  Because in this case the ARP entry cannot change.  At this time it seems unnecessary to me.  I'll brush up on the RFC.

Bill Auerbach <billauerbach>
Mon 29 Mar 2010 09:00:41 AM UTC, comment #5: 

I much prefer Simon's version of this: having a copy of ARP data that doesn't time out or change when the ARP data changes would cause some users problems.


Kieran Mansley <kieranm>
Group Member
Sat 27 Mar 2010 04:55:51 PM UTC, comment #4: 

I just moved the code that handles "etharp_cached_entry" (or netif->addr_hint) from find_entry() to etharp_output() and etharp_query(). For LWIP_NETIF_HWADDRHINT==0 and with one netif only, this is exactly the same as the original patch only it is in sync with the arp_table. With LWIP_NETIF_HWADDRHINT==1, this is slightly slower for one connection, but faster for multiple concurrent connections.

BTW: the original patch would send to a wrong mac address once the original entry got overwritten (e.g. due to a timeout or when being overwritten because the table got full).

Simon Goldschmidt <goldsimon>
Group administrator
Sat 27 Mar 2010 09:38:56 AM UTC, comment #3: 

The current LWIP_NETIF_HWADDRHINT==1 code also avoids searching the cache, it merely test if the hwaddr is really OK (it's still marked as stable in the cache). This test would be omitted with your code and an entry would never time out with only one active connection and no other packets being sent.

However, I totally agree the current code flow can be optimized and I'd suggest moving the netif->addr_hint check from find_entry() to ehtarp_output():

#if LWIP_NETIF_HWADDRHINT
if (netif->addr_hint != NULL) {
  /* per-pcb cached entry was given */
  u8_t per_pcb_cache = *(netif->addr_hint);
  if ((per_pcb_cache < ARP_TABLE_SIZE) &&
      (arp_table[per_pcb_cache].state == ETHARP_STATE_STABLE) &&
      (ip_addr_cmp(ipaddr, &arp_table[per_pcb_cache].ipaddr))) {
    /* the per-pcb-cached entry is stable and the right one! */
    ETHARP_STATS_INC(etharp.cachehit);
    return etharp_send_ip(netif, q, (struct eth_addr*)(netif->hwaddr),
      &arp_table[per_pcb_cache].ethaddr);
  }
}
#endif /* LWIP_NETIF_HWADDRHINT */
/* queue on destination Ethernet address belonging to ipaddr */
return etharp_query(netif, ipaddr, q);


This is a little more code that your suggestion but it's more in line with the rest of the etharp module since it doesn't avoid the cache totally. Plus it also works with more than one concurrent connection on a netif.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 26 Mar 2010 09:56:27 PM UTC, comment #2: 

This avoids searching that cache (if you mean the ARP table).  If you mean something else, then I think it's not as good as this.  If it's done at the pbuf level, didn't already pass the ARP tests?

I believe I tried with and without LWIP_NETIF_HWADDRHINT.  Where I added that test to avoid the call to etharp_query, send 100,000 packets to a netif and time how much is spent in all of the calls to etharp_query.  When I did this, this was a big part of the total time to send a bunch of packets.

I didn't do the timing like this - I studied the total flow of sending a packet and timed sending 1,000,000 packets while optimizing or removing calls in the call tree where I could.

This is like the other patch - it's easy to test and if anyone else sees a respectable gain then it's worth adding (as an option or otherwise).  The code size increase here is tiny.

If it's not added, at least if it's asked for someone has something to try.  Where are all of the people who asked over the year or 2 about speeding up lwIP transmit speeds? :-)

Bill Auerbach <billauerbach>
Fri 26 Mar 2010 09:35:40 PM UTC, comment #1: 

We already have such a cache, but per pcb (which is better than per netif when having more than one simultaneous connection). Maybe we can minimize the code flow when LWIP_NETIF_HWADDRHINT==1 so that we can prevent find_entry getting called instead of adding yet another option/cache?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 26 Mar 2010 04:59:33 PM UTC, original submission:  

This is a significant optimization as it avoids ARP lookups on every packet sent through a netif to the same IP address:

In netif.h add members to struct netif:

  /* IP address of last sent packet */
  struct ip_addr last_ip_addr;
  /* MAC address of last sent packet */
  struct eth_addr *last_hw_addr;

In Etharp.c in etharp_query add the 2 lines in the if:

    if (arp_table[i].state == ETHARP_STATE_STABLE) {
      /* we have a valid IP->Ethernet address mapping */
      /* send the packet */
      netif->last_ip_addr = *ipaddr;
      netif->last_hw_addr = &arp_table[i].ethaddr;
      result = etharp_send_ip(netif, q, srcaddr, &(arp_table[i].ethaddr));
    /* pending entry? (either just created or already pending */
    } else if (arp_table[i].state == ETHARP_STATE_PENDING) {

And in etharp_output add:

    /* if the IP address is the last used, use it's MAC address directly */
    if (ip_addr_cmp (ipaddr, &netif->last_ip_addr))
      return etharp_send_ip(netif, q, (struct eth_addr*)(netif->hwaddr), netif->last_hw_addr);
    /* queue on destination Ethernet address belonging to ipaddr */
    return etharp_query(netif, ipaddr, q);

This avoids the etharp_query on successive sends to the same Ip address on an interface.  The was the #1 improvement of my changes made to speed up lwIP TX.

Bill Auerbach <billauerbach>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by billauerbach (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-04-14 goldsimon StatusReady For Test Done
        Open/ClosedOpen Closed
    2010-03-27 goldsimon StatusNone Ready For Test
        Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code