EtherCard icon indicating copy to clipboard operation
EtherCard copied to clipboard

Add ARP cache

Open ArnaudD-FR opened this issue 6 years ago • 4 comments

Pros:

  • keep MAC/IP relations into an ARP 'store'.
  • fix issue when IP packet is received from local network: answer is no more broadcasted
  • define ETHERCARD_ARP_STORE_SIZE to set cache size
  • Fix issue #181, issue #269 and issue #309 (Issues #181 and #309 looks to be the same)

Cons:

  • increase size of Ethercard in my project:
    • +198 bytes in Program section (.text + .data + .bootloader)
    • +27 bytes in Data section (.data + .bss + .noinit)

About issue #269 the following code should be used:

#define PORT 5000

#include <EtherCard.h>
#include <IPAddress.h>

uint8_t macAddr[] = {0x74,0x69,0x69,0x2D,0x30,0x32};
uint8_t ipAddr[] = { 192, 168, 1, 200 };
uint8_t gwAddr[] = { 192, 168, 1, 2 };
uint8_t netMask[] = { 255, 255, 255, 0 };

uint8_t distAddr[] = { 192, 168, 1, 52 }; // I can't send packet to 172.16.255.2.

byte Ethernet::buffer[256];

void setup() {
    Serial.begin(57600);
    ether.begin(sizeof(Ethernet::buffer), macAddr);
    ether.staticSetup(ipAddr, gwAddr, 0, netMask);

    // wait for link to be up before any ARP lookup
    while (!ether.isLinkUp())
        ;

    // send ARP lookup...
    ether.clientResolveIp(distAddr);
    // ...and wait for answer
    while (ether.clientWaitIp(distAddr))
        ether.packetLoop(ether.packetReceive());
    Serial.println("setup done");
}

void loop() {
    char c[] = { 'h', 'i' };
    ether.sendUdp(c, sizeof(c), PORT, distAddr, PORT);
    ether.packetLoop(ether.packetReceive());
    delay(1000);
}

ArnaudD-FR avatar Jan 06 '19 08:01 ArnaudD-FR

@ArnaudD-FR this is a good feature! However, it would be nice if one could disable ARP cache completely to save some RAM and program memory, if needed. Useful for memory-constrained setups where broadcasting would be fine. I suggest adding a pre-processor macro like DISABLE_ARP_CACHE (commented out by default).

nuno-silva avatar Jan 06 '19 21:01 nuno-silva

I understand your concern but this implementation replace the previous mechanism to store MAC address for gateway, dnsip and hisip. So if we disable the ARP cache, there is no way to communicate.

Do you have some use case in such environment to determine the best implementation?

  • communication is done only with one device (gateway or whatever?)
  • ...

ArnaudD-FR avatar Jan 07 '19 10:01 ArnaudD-FR

Wow, that is quite a piece of work, thanks ArnaudD-FR.

It changes quite a lot of stuff in this changeset- not just adding ARP cache - such as new IP header structures.

I agree with @nuno-silva that RAM is one of the scarcest commodities, particularly on the older AVR processors. This is probably the reason why there wasn't an ARP cache in the first place.

Rather than disabling the ARP cache, what would happen if it only had a single entry (for the router)?

njh avatar Jan 07 '19 11:01 njh

The RAM usage is the following struct size multiplied by the number of ARP cache entries: (IP_LEN + ETH_LEN + 1)*ETHERCARD_ARP_STORE_SIZE, so by default it is 44 bytes.

struct ArpEntry
{
    uint8_t ip[IP_LEN];
    uint8_t mac[ETH_LEN];
    uint8_t count;
};

But my patch remove some global variables (17 bytes) from tcpip.cpp:

  • uint8_t destmacaddr[ETH_LEN]
  • boolean waiting_for_dns_mac
  • boolean has_dns_mac
  • boolean waiting_for_dest_mac
  • boolean has_dest_mac
  • uint8_t gwmacaddr[ETH_LEN]
  • uint8_t waitgwmac

So the RAM increase is about 27 bytes as defined in my first message when using the default cache size. If you set the cache to 1 entry then you save 6 bytes... But only the gateway can be contacted. If something else contact the device, the gateway MAC address will be replaced by the last one received and on next packetLoop the gateway MAC address will come back...

Yes I really don't like the idea to use array index to find ethernet/arp/ip header fields. It's so much easier to use and understand a structure...

The IP header definition is here just to get the IP address of an IP packet and update the corresponding ARP entry.

In this commit I have completely replaced the ARP indexes by a struct ArpHeader.

ArnaudD-FR avatar Jan 07 '19 13:01 ArnaudD-FR