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 > [email protected] > https://amsterdam.lcs.mit.edu/mailman/listinfo/click _______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
