[Click] New element: icmpipencap, a general icmppingencap

[Eddie Kohler]

Hi guys!

Thanks very much for this element.  I'd definitely like to add it to the 
repository.  However, I have some concerns that if you could address, it would 
be great.

1. Element documentation in the header file is missing.  Even just one 
paragraph plus the use case (e.g. "=c ICMPEncap(SRC, DST, TYPE [, CODE])".)

2. The _icmp_type and _icmp_code members should be read using IPNameInfo, so 
users can give symbolic names.  E.g.:

int32_t icmp_type;
...  "TYPE", cpkP+cpkM, cpNamedInteger, NameInfo::T_ICMP_TYPE, &icmp_type, ...

The CODE requires special handling since you need to know the TYPE before 
parsing the CODE.  Take a look at ICMPError for an example.

3. This is the big one: The current element adds a clcik_icmp_echo header. 
But this header is only correct for TYPE echo or echo-reply.  Other ICMP types 
add a different header, often with a different length.  For example tstamp and 
tcstamp-reply have longer ehaders.  See include/clicknet/icmp.h for more 
examples.  Either the element should be changed to add the appropriate header 
for _type, or the configure() method should return an error on an 
inappropriate TYPE.

Thanks very much,
Eddie



Braem Bart wrote:
> Dear list,
> 
> Attached you will find an element our students (Jens De Wit and Nico Van Looy) produced while working on their FireSim project, the Firewall Simulation in Click as presented on SyClick.
> The element allows transmitting ICMP packets with all types of codes and types, which makes this element more general than ICMPPingEncap. It is a bit the ICMP alternative to UDPIPEncap.
> The element code is largely based on ICMPPingEncap so Jens and Nico suggested just patching ICMPPingEncap with the new behaviour, although of course the name then does not cover the contents of the element. 
> Jens promised to write a unit test with the testie framework, so this can be expected as well.
> 
> best regards,
> Bart
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> click mailing list
> click at amsterdam.lcs.mit.edu
> https://amsterdam.lcs.mit.edu/mailman/listinfo/click