[Click] DS: New element: icmpipencap, a general icmppingencap

[Jens De Wit]

Dear list,

I send this mail in reply to Eddie's remarks on the new ICMPIPEncap element.
Attached you will find the improved version of the element, as well as a
testie file containing a basic unit test for the new element.

Kind regards,
Jens De Wit



2010/2/28 De Wit Jens <Jens.DeWit at student.ua.ac.be>

>
> -------------------------------------------
> *Van:* Eddie Kohler[SMTP:KOHLER at CS.UCLA.EDU <SMTP%3AKOHLER at CS.UCLA.EDU>]
> *Verzonden:* zondag 28 februari 2010 21:52:07
> *Aan:* Braem Bart
> *CC:* click at amsterdam.lcs.mit.edu; De Wit Jens
> *Onderwerp:* Re: [Click] New element: icmpipencap, a general icmppingencap
>
> *Automatisch doorgezonden volgens een regel*
>
> 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icmpipencap.cc
Type: text/x-c++src
Size: 4576 bytes
Desc: not available
Url : http://amsterdam.lcs.mit.edu/pipermail/click/attachments/20100301/73048774/attachment.cc 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: icmpipencap.hh
Type: text/x-c++hdr
Size: 1784 bytes
Desc: not available
Url : http://amsterdam.lcs.mit.edu/pipermail/click/attachments/20100301/73048774/attachment.hh 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ICMPIPEncap-01.testie
Type: application/octet-stream
Size: 473 bytes
Desc: not available
Url : http://amsterdam.lcs.mit.edu/pipermail/click/attachments/20100301/73048774/attachment.obj