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 <[email protected]> > > ------------------------------------------- > *Van:* Eddie Kohler[SMTP:[email protected] <smtp%[email protected]>] > *Verzonden:* zondag 28 februari 2010 21:52:07 > *Aan:* Braem Bart > *CC:* [email protected]; 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 > > [email protected] > > https://amsterdam.lcs.mit.edu/mailman/listinfo/click >
#include <click/config.h>
#include "icmpipencap.hh"
#include <click/confparse.hh>
#include <click/error.hh>
#include <clicknet/ip.h>
#include <clicknet/icmp.h>
#include <click/packet_anno.hh>
#include <click/nameinfo.hh>
#include <click/standard/alignmentinfo.hh>
CLICK_DECLS
ICMPIPEncap::ICMPIPEncap()
: _icmp_id(0), _ip_id(1), _icmp_type(0), _icmp_code(0)
{
}
ICMPIPEncap::~ICMPIPEncap()
{
}
int
ICMPIPEncap::configure(Vector<String> &conf, ErrorHandler *errh)
{
String code_str = "0";
int code;
if (cp_va_kparse(conf, this, errh,
"SRC", cpkP+cpkM, cpIPAddress, &_src,
"DST", cpkP+cpkM, cpIPAddress, &_dst,
"TYPE", cpkP+cpkM, cpNamedInteger, NameInfo::T_ICMP_TYPE, &_icmp_type,
"CODE", cpkP, cpWord, &code_str,
"IDENTIFIER", 0, cpUnsignedShort, &_icmp_id,
cpEnd) < 0)
return -1;
if (!NameInfo::query_int(NameInfo::T_ICMP_CODE + _icmp_type, this, code_str, &code) || code < 0 || code > 255)
return errh->error("invalid code");
_icmp_code = code;
#if HAVE_FAST_CHECKSUM && FAST_CHECKSUM_ALIGNED
{ // check alignment
int ans, c, o;
ans = AlignmentInfo::query(this, 0, c, o);
_aligned = (ans && c == 4 && o == 0);
if (!_aligned)
errh->warning("IP header unaligned, cannot use fast IP checksum");
if (!ans)
errh->message("(Try passing the configuration through `click-align'.)");
}
#endif
return 0;
}
Packet *
ICMPIPEncap::simple_action(Packet *p)
{
WritablePacket *q;
bool need_seq_number = false;
switch(_icmp_type) {
case ICMP_UNREACH:
if (_icmp_code == ICMP_UNREACH_NEEDFRAG)
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp_needfrag));
else
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp));
break;
case ICMP_TSTAMP:
case ICMP_TSTAMPREPLY:
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp_tstamp));
need_seq_number = true;
break;
case ICMP_REDIRECT:
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp_redirect));
break;
case ICMP_PARAMPROB:
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp_paramprob));
break;
case ICMP_ECHO:
case ICMP_ECHOREPLY:
case ICMP_IREQ:
case ICMP_IREQREPLY:
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp_sequenced));
need_seq_number = true;
break;
default:
q = p->push(sizeof(click_ip) + sizeof(struct click_icmp));
}
if (q) {
click_ip *ip = reinterpret_cast<click_ip *>(q->data());
ip->ip_v = 4;
ip->ip_hl = sizeof(click_ip) >> 2;
ip->ip_tos = 0;
ip->ip_len = htons(q->length());
ip->ip_id = htons(_ip_id);
ip->ip_off = 0;
ip->ip_ttl = 255;
ip->ip_p = IP_PROTO_ICMP; /* icmp */
ip->ip_sum = 0;
ip->ip_src = _src;
ip->ip_dst = _dst;
click_icmp_echo *icmp = (struct click_icmp_echo *) (ip + 1);
icmp->icmp_type = _icmp_type;
icmp->icmp_code = _icmp_code;
icmp->icmp_cksum = 0;
#ifdef __linux__
icmp->icmp_identifier = _icmp_id;
if(need_seq_number)
icmp->icmp_sequence = _ip_id;
#else
icmp->icmp_identifier = htons(_icmp_id);
if(need_seq_number)
icmp->icmp_sequence = htons(_ip_id);
#endif
#if HAVE_FAST_CHECKSUM && FAST_CHECKSUM_ALIGNED
if (_aligned)
ip->ip_sum = ip_fast_csum((unsigned char *)ip, sizeof(click_ip) >> 2);
else
ip->ip_sum = click_in_cksum((unsigned char *)ip, sizeof(click_ip));
#elif HAVE_FAST_CHECKSUM
ip->ip_sum = ip_fast_csum((unsigned char *)ip, sizeof(click_ip) >> 2);
#else
ip->ip_sum = click_in_cksum((unsigned char *)ip, sizeof(click_ip));
#endif
icmp->icmp_cksum = click_in_cksum((const unsigned char *)icmp, q->length() - sizeof(click_ip));
q->set_dst_ip_anno(IPAddress(_dst));
q->set_ip_header(ip, sizeof(click_ip));
_ip_id += (_ip_id == 0xFFFF ? 2 : 1);
return q;
} else
return 0;
}
String ICMPIPEncap::read_handler(Element *e, void *thunk)
{
ICMPIPEncap *i = static_cast<ICMPIPEncap *>(e);
if (thunk)
return IPAddress(i->_dst).unparse();
else
return IPAddress(i->_src).unparse();
}
int ICMPIPEncap::write_handler(const String &str, Element *e, void *thunk, ErrorHandler *errh)
{
ICMPIPEncap *i = static_cast<ICMPIPEncap *>(e);
IPAddress a;
if (!cp_ip_address(str, &a))
return errh->error("expected IP address");
if (thunk)
i->_dst = a;
else
i->_src = a;
return 0;
}
void ICMPIPEncap::add_handlers()
{
add_read_handler("src", read_handler, (void *) 0, Handler::CALM);
add_write_handler("src", write_handler, (void *) 0);
add_read_handler("dst", read_handler, (void *) 1, Handler::CALM);
add_write_handler("dst", write_handler, (void *) 1);
}
CLICK_ENDDECLS
EXPORT_ELEMENT(ICMPIPEncap)
#ifndef CLICK_ICMPIPENCAP_HH
#define CLICK_ICMPIPENCAP_HH
#include <click/element.hh>
#include <click/timer.hh>
CLICK_DECLS
/*
=c
ICMPIPEncap(SRC, DST, TYPE [, CODE, I<keywords> IDENTIFIER])
=s icmp
encapsulates packets in ICMP/IP headers
=d
Encapsulates input packets in an ICMP/IP header with source IP address SRC,
destination IP address DST, ICMP type TYPE and code CODE. TYPE and CODE
may be integers between 0 and 255 or mnemonic names; CODE defaults to 0.
Advances the "sequence" field by one for each packet. (The sequence field is
stored in network byte order in the packet.)
Keyword arguments are:
=over 8
=item CODE
Integer. Determines the ICMP code field in emitted ICMP packets. Default is 0.
=item IDENTIFIER
Integer. Determines the ICMP identifier field in emitted ICMP packets. Default is
0.
=back
=h src read/write
Returns or sets the SRC argument.
=h dst read/write
Returns or sets the DST argument.
=a
ICMPPingEncap, ICMPError */
class ICMPIPEncap : public Element { public:
ICMPIPEncap();
~ICMPIPEncap();
const char *class_name() const { return "ICMPIPEncap"; }
const char *port_count() const { return PORTS_1_1; }
const char *processing() const { return AGNOSTIC; }
const char *flags() const { return "A"; }
int configure(Vector<String> &, ErrorHandler *);
void add_handlers();
Packet *simple_action(Packet *);
private:
struct in_addr _src;
struct in_addr _dst;
uint16_t _icmp_id;
uint16_t _ip_id;
uint8_t _icmp_type;
uint8_t _icmp_code;
#if HAVE_FAST_CHECKSUM && FAST_CHECKSUM_ALIGNED
bool _aligned;
#endif
static String read_handler(Element *, void *);
static int write_handler(const String &, Element *, void *, ErrorHandler *);
};
CLICK_ENDDECLS
#endif
ICMPIPEncap-01.testie
Description: Binary data
_______________________________________________ click mailing list [email protected] https://amsterdam.lcs.mit.edu/mailman/listinfo/click
