Weird memset argument in RangeIPLookup::flush_table()

[pallas]

In elements/ip/rangeiplookup.cc, it says

void
RangeIPLookup::flush_table()
{
    _helper.flush();
    memset(_range_base, 0, sizeof(_range_base));
    memset(_range_len, 0, sizeof(_range_len));
    memset(_range_t, 0, sizeof(_range_t));
}

Since those members are pointers, memset is clearing something the size of the pointer, not an element in the underlying array. Is this the intention? or should the code be

void
RangeIPLookup::flush_table()
{
    _helper.flush();
    memset(_range_base, 0, sizeof(*_range_base));
    memset(_range_len, 0, sizeof(*_range_len));
    memset(_range_t, 0, sizeof(*_range_t));
}

or

void
RangeIPLookup::flush_table()
{
    _helper.flush();
    memset(_range_base, 0, (1 << KICKSTART_BITS) * sizeof(*_range_base));
    memset(_range_len, 0, (1 << KICKSTART_BITS) * sizeof(*_range_len));
    memset(_range_t, 0, RANGES_MAX * sizeof(*_range_t));
}

[kohler]

An interesting question. Here's what I think: The expand() function overwrites the entire contents of _range_base, _range_t, and _range_len, on EVERY route update. In the configure() path we call flush_table(), but then in initialize() call expand(). So..........hmm.

(1) I would really not use RangeIPLookup if I were you. RadixIPLookup is much better (fast but small memory usage). If you are having problems with Radix, let me know; some students of mine have made good progress on improving its performance.

(2) You are right that when flush_table() is called from flush_handler, it should memset() the ENTIRE table to 0 -- the last of your 3 variants.

(3) It is correct, but expensive, to do that memset() during configure() as well.

(4) In the longer term doing an expand() after every routing table update is silly, we should expand after a group of updates if possible.

(5) But still, better to go with Radix.

Make sense?

[pallas]

Yep. I'm not using the element anyway, clang complained at me during compilation.

[kohler]

Go clang!