[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [lwip-users] ARP ETHARP_TRY_HARD
From: |
Paul Clarke |
Subject: |
RE: [lwip-users] ARP ETHARP_TRY_HARD |
Date: |
Mon, 29 Nov 2004 10:04:38 +1030 |
Hi Leon
>
> Hello Paul,
>
> thanks for looking in the code, I enjoy having extra pairs of eyes
> checking my code!
> >
> >I like the patches you have put in etharp.c but I still think that the
> >exit if !ETHARP_TRY_HARD is missing.
> >
> Could you explain why please?
>
Although the code is correct with its TRY_HARD from a code maintenance
perspective it is a lot easier to see if there is an exit test
for no empty slot and !flags & ETHARP_TRY_HARD.
The following code can then remove the TRY_HARD case also making
it easier to understand.
example
for (i = 0; i<ARP_TABLE_SIZE; i++){}
// at exit i is either an empty slot or ARP_TABLE_SIZE
if ( i== ARP_TABLE_SIZE && !flags & ETHARP_TRY_HARD)
{
return ERR_ARG; // could not find an empty slot
}
// NOW we add this entry into our table
/* b) choose the least destructive entry to recycle:
* 1) empty entry
* 2) oldest stable entry
* 3) oldest pending entry without queued packets
* 4) oldest pending entry with queued packets
*/
> I think the current code is correct in the find_entry(..., flags
> = 0) case.
>
> find_entry will:
> a) find candidate ARP entries for the given IP address
> b) select the least-cache-destructive entry
> c) see if it may alter that entry
>
> Steps a and b are non-modifying, and are marked a and b in the source
> code comments of etharp.c
> Step c ONLY recycles existing entries when an empty ARP entry is found:
> Step c ALWAYS fills empty ARP entries. (I assume you do not want that
> to happen, am I correct??)
Now that we add ARP Entries with the right TRY_HARD setting it is not
an issue. When I was testing this code it was easier to see what was
happening if the ARP table only got filled with entries we care about
ie our connections.
>
> See etharp.c line 308
>
> /* allowed to recycle a entry? */
> if (flags & ETHARP_TRY_HARD) {
> /* recycle (no-op for an already empty entry) */
> arp_table[i].state = ETHARP_STATE_EMPTY;
> }
>
> Yes, it will fill up the cache with any broadcast traffic, BUT only
> until the cache
> is full. Once the cache is full, it will return ERR_MEM, see line 327:
>
> /* no entry available */
> } else {
> /* return failure */
> i = (s8_t)ERR_MEM;
> }
>
> Only if ETHARP_TRY_HARD is specified, it will recycle cache entries.
>
> At least this is exactly what I think should happen.
>
> Please let me know where my thoughts go wrong, or what you expected
> the algorithm to do?
>
> Regards,
>
> Leon Woestenberg.
>
>
Paul
- [lwip-users] ARP ETHARP_TRY_HARD, Paul C, 2004/11/25
- Re: [lwip-users] ARP ETHARP_TRY_HARD, Leon Woestenberg, 2004/11/28
- RE: [lwip-users] ARP ETHARP_TRY_HARD,
Paul Clarke <=
- RE: [lwip-users] ARP ETHARP_TRY_HARD, Leon Woestenberg, 2004/11/29
- RE: [lwip-users] ARP ETHARP_TRY_HARD, Paul C, 2004/11/29
- RE: [lwip-users] ARP ETHARP_TRY_HARD, Paul Clarke, 2004/11/29
- RE: [lwip-users] ARP ETHARP_TRY_HARD, Paul Clarke, 2004/11/29
- Re: [lwip-users] ARP ETHARP_TRY_HARD, Leon Woestenberg, 2004/11/30
- Re: [lwip-users] ARP ETHARP_TRY_HARD, Leon Woestenberg, 2004/11/30
RE: [lwip-users] ARP ETHARP_TRY_HARD, Paul Clarke, 2004/11/28