[Bro-Dev] new IPv6 code
Siwek, Jonathan Luke
jsiwek at illinois.edu
Wed Feb 15 14:33:42 PST 2012
> - in BroValUnion, 'addr_val' is of type pointer type IPAddr*. I
> suppose that's to keep the union size as small as possible, which
> makes sense. I'd still be curious though how allocating the
> addresses dynamically compares against storing the instance directly
> inside the union. Have you tried that? I realize it's not an easy
> change to do that, so fine if not.
You mean having a IPAddr versus IPAddr* member in the union? I had started out that way, until the compiler told me union members cannot have non-trivial constructors. And I figured going with a pointer is better for size anyway as you mentioned, although the largest member in the old --enable-brov6 code was the subnets with five uint32's and that did not seem to increase memory as much in the tests as I expected.
> - The DNS binpac analyzer generates both dns_a6_reply and
> dns_aaaa_reply, but the standard analyzer only the latter (for both
> resource record types). What is correct?
My thought was to generate both depending on the type, but I guess I forgot to do it for the non-binpac one. I'll fix it.
> - In the RPC code, there's this:
> is_mapped_dce_rpc_endpoint(const ConnID* id, TransportProto proto)
> ········if ( id->dst_addr.family() == IPAddr::IPv6 )
> ················return false;
> Does the protocol not support IPv6 at all or is that a "todo"?
I'm not sure -- the old --enable-brov6 switch also bailed out for IPv6. So yeah, I suppose the todo is to determine how much more of a todo there is.
> - in DPM.cc, the ExpectedConn class: should it now store IPAddr
> instead of uint32 for orig and resp?
Yes, seems like it could do that.
> - Expr.cc, BinaryExpr::AddrFold: why still the uint32 here? If it's
> just to make the macro work, I'd just remove that and use
> comparision operators instead.
Ok, will do that.
> - IP.h, IP_Hdr: Do we need the new src_addr/dst_addr attributes? Why
> not construct on the fly out of ip4/ip6?
I guess not. Hadn't thought of that as I was just replicating the face that the old --enable-brov6 code also added those. Will fix.
More information about the bro-dev