Why are <click/packet.hh> & <click/packet_anno.hh> included in ip6address.hh if CLICK_TOOL is not set?

[glemin]

Hello all,

I have a question. Why are <click/packet.hh> & <click/packet_anno.hh> included in ip6address.hh if CLICK_TOOL is not set?

a) What is CLICK_TOOL and when is this macro set? (just out of curiosity and being sure that I have done nothing wrong when makig changes)

b) I suggest reversing the include order and remove the following 3 macros,

#if !CLICK_TOOL
inline const IP6Address &
DST_IP6_ANNO(Packet *p)
{
    return *reinterpret_cast<IP6Address *>(p->anno_u8() + DST_IP6_ANNO_OFFSET);
}

inline void
SET_DST_IP6_ANNO(Packet *p, const IP6Address &a)
{
    memcpy(p->anno_u8() + DST_IP6_ANNO_OFFSET, a.data(), DST_IP6_ANNO_SIZE);
}

inline void
SET_DST_IP6_ANNO(Packet *p, const struct in6_addr &a)
{
    memcpy(p->anno_u8() + DST_IP6_ANNO_OFFSET, &a, DST_IP6_ANNO_SIZE);
}
#endif

and replace them by the following two Packet class functions in stead:

inline IP6Address
Packet::dst_ip6_anno() const
{
    return IP6Address(anno_u8());
}

inline void
Packet::set_dst_ip6_anno(IP6Address a)
{
    memcpy(anno_u8(), a.data(), 16); // The size of an IPv6 address is 16 bytes.
}

I have already done this in my private project and it seems to work (see code here) . Then we have to use a macro less (which is always good according to Stroustrup in his famous C++ book) and it feels more natural I think that our packet class just includes the ip6address information, just as it includes the ipaddress information. I think it is a bit weird to see included somewhere actually.

Best Regards, Glenn

[tbarbette]

CLICK_TOOL is used when including click files while building tools from the /tools folder (and maybe other "external" software?). I don't know why it is needed in this specific case and never touched tools so I can't help much.

[glemin]

I am not sure but what happens if CLICK_TOOL is set? Then maybe you can't compile click with the --ip6 configuration option?

When I look at setip6address for instance I see the following,

/*
 * setip6address.{cc,hh} -- element sets destination address annotation
 * to a particular IP6 address
 * Eddie Kohler, Peilei Fan
 *
 * Copyright (c) 2000 Massachusetts Institute of Technology
 *
 * Permission is hereby granted, free of charge, to any person obtaining a
 * copy of this software and associated documentation files (the "Software"),
 * to deal in the Software without restriction, subject to the conditions
 * listed in the Click LICENSE file. These conditions include: you must
 * preserve this copyright notice, and you cannot mention the copyright
 * holders in advertising related to the Software without their permission.
 * The Software is provided WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED. This
 * notice is a summary of the Click LICENSE file; the license in that file is
 * legally binding.
 */

#include <click/config.h>
#include "setip6address.hh"
#include <click/args.hh>
CLICK_DECLS

SetIP6Address::SetIP6Address()
{
}

SetIP6Address::~SetIP6Address()
{
}

int
SetIP6Address::configure(Vector<String> &conf, ErrorHandler *errh)
{
    return Args(conf, this, errh).read_mp("IPADDR", _ip6).complete();
}

Packet *
SetIP6Address::simple_action(Packet *p)
{
    SET_DST_IP6_ANNO(p, _ip6);
    return p;
}

CLICK_ENDDECLS
EXPORT_ELEMENT(SetIP6Address)

Am I right that click is maybe not compileable then when if CLICK_TOOL is set and the ip6 directory is compiled at the same time? Because maybe he can't find the macro then, because it says #if !CLICK_TOOL . Actually I have also no idea what the compiler will do if you use a MACRO that wasn't set.

I have never compiled any Click Tools before to my knowledge (or maybe I do without knowing it). The configuration I always use now is ./configure --disable-linuxmodule --enable-ip6, and I compile with sudo make install.

[kohler]

You have compiled Click tools. click-install, etc., are called the Click tools. A Click tool is something that manipulates Click configurations but doesn't actually run packets. Anything in the tools directory.

Your proposed change—adding Packet functions dst_ip6_anno() and set_dst_ip6_anno—would be a good one. Those functions would have to be #ifdef CLICK_IP6. But I would accept that change. You would change the elements to match.

Such changes are always a little unfortunate, because other elements out in the world might depend on the existing macros. So even better than removing the macros would be replacing them with deprecated functions.

[glemin]

Sounds great. I will do that.