[bind10-dev] DNS message API: resource data classes

[JINMEI Tatuya / 神明達哉]

Thanks for the prompt, very insightful comments.  And sorry for the
delayed response, I've been busy for other tasks.

At Mon, 17 Aug 2009 09:29:22 +0000,
Francis Dupont <fdupont at isc.org> wrote:

> Some questions about resource data classes:
>  - what is DNSInvalidSubstitution? (still unused)

Oops, it's a leftover from an intermediate version.  Removed from the
repository.

>  - why there is no getClass() method in Rdata abstract class?

Good point, that's one thing I wondered...and as I revisited RFC1034 I
noticed that I overlooked the fact that RDATA can also be dependent on
the class and that type A Rdata is actually different for class IN and
for class CH.  So, yes, we need something like getClass().

The next point is related to this issue:

>  - how the fact some types exist only in a particular class
>   will be handled? BTW I like the way dnspython solves this
>   through name spaces.

We also need to address this.  We could adopt a namespace-based
solution, too:

        namespace RDATA {
            namespace IN {
                class ARdata : public Rdata {
			....
		};
	    }
	}

	// usage example:
	RDATA::IN::ARdata a("192.0.2.1");

(if we use a dedicated "RDATA" namespace, we might simply use the type
name as the C++ class name:
        namespace RDATA {
            namespace IN {
                class A : public Rdata {
			...
		};
                class AAAA : public Rdata {
			...
		};
	    }
	}
)

Another obvious candidate is to use inheritance:

        class INRdata : public Rdata {};
        class ARdata : public INRdata {};

but this is probably not the best approach because the same type could
belong to different classes (so we need to address name conflict
anyway).

>  - I can't see a virtual fromText(), I believe the idea is to
>   provide it in constructors of subclasses and with an external
>   factory function, isn't it?

I don't understand the question.  Could you be more explicit?

>  - there is no comparaison virtual function. I have no problem
>   with limiting comparaison between Rdatas inside the same
>   subclass (i.e., rdatatype and rdataclass must be the same) even
>   we need full comparaison (i.e., not only == and !=).
>   Note DNSSEC requires a more generic comparaison between rdatasets.

I thought about it, and was not sure if we'd need it or how to realize
that.  But we'll probably need it to handle a set (or list or some
container) of Rdatas in polymorphic way, even though all the RDATAs
are assumed to be of the same class and type.

>  - why _rdclass and _rdtype are not defined in Rdata class itself
>   (i.e., Rdata class has not to be fully abstract).

Because the type can be determined by the class type itself in most
cases.  We could still keep _rdtype, but it would be redundant
information in such cases and may cause a trouble in ensuring
consistency.

>  - there are a lot of cases we'd like to see a XXRdata to be
>   a WireRdata too, and the opposite. So the separate class is
>   perhaps a bad idea, I imagine more a cache in the Rdata class itself.

That (combine wire/non-wire in a single class) is certainly an
option.  I actually considered that path, but chose to separate them
in this version of proposal to keep XXRdata as small as possible.
When we parse a response and construct a set of RDATA from wire, we
probably want to handle the data as opaque information whenever
possible, and want to avoid the overhead of instantiating specific
XXRdata classes.

But I agree this is worth revisiting.

>  - obviously ARdata, AAAARdata, etc, should go into their own files.

It's not that *obvious* to me:-), but I don't stick to having all in a
single file at all.  For the prototype version, it was simply because
it's a prototype.

>  - toWire/fromWire implies string copies, IMHO we should imagine
>   a buffer class and overload these methods.

Yes, how to abstract "wire data" is an open issue.  A language
specific issue is that it won't be so helpful to just typedef a
"wiredata" class as string:
  typedef WireBuffer string;
because the compiler doesn't differentiate them in detecting conflict
definitions.

>  - IMHO RdataSet could be sorted. If it is always kept sorted some
>   operations are easier but this has a cost of all others. Perhaps
>   we simply need a flag to keep the sorted/unsorted state.

Good point.  I wondered this issue, too (btw you mean sorting RDATA of
an RdataSet, right?).  If we want to support "rrset-order fixed", I
think we should keep the "given ordering" (and support the ability to
sort them separately) anyway.  Otherwise, we might simply keep them as
an STL set (then we can automatically keep them sorted, and don't have
to worry about duplicate RDATA).

>  - I have a question about the covers of RRSIG RR. BTW can two RRSIG
>   RRs with different covers be members of the same RRset? I don't
>   believe (nor dnspython).

nor BIND9 libdns.  I've not thought about it so closely (in generally,
I've not paid much attention to DNSSEC related things in the initial
proposal; I'll have to), but my gut feeling is that we can (and
probably should) the convention of dnspython and libdns.

>  - should RR be a subclass of Rdata?

I'm not sure about that.  I actually considered that option, but
didn't adopt it because I was not so confident about the inheritance
relationship and it didn't seem to me to make the implementation
clearer.  (I may be wrong).

>  - we need the fromText() and fromWire() functions (note, not methods).
>   I don't know how C++ gives access to the class lookup but there should
>   be a way to write the equivalent of dnspython get_rdata_class()
>   (without dynamic loading :-).

Yes, I saw the need for something like that in my experiment.  My
current thinking is that we let each well-know (class, type) pair
register itself to some internal DB (whose entry maps the pair to a
concrete RDATA class) on initialization, and we lookup the DB
run-time.  powerdns recursor takes this approach.

>  - we need a GenericRdata to catch unknown class/types.

Yes.

---
JINMEI, Tatuya