Обсуждение: Re: hackers-digest V1 #1013
i would very much like inet_net_pton to not be changed in this way, even though it's an internal server function the way postgres 6.4 will be packaged. there is an RFC specifying what this function does.
Thus spake Paul A Vixie
> i would very much like inet_net_pton to not be changed in this way,
You mean inet_net_ntop, right?
> even though it's an internal server function the way postgres 6.4
> will be packaged. there is an RFC specifying what this function does.
Are you talking about RFC2133? That one doesn't even specify bits as
an argument so this is already different. Is there another one I
should be looking at?
If a function is based on a standards document like this, shouldn't
we include that as a comment in the file?
Anyway, I seem to be mistaken about the whole cidr or inet type.
Based on the discussions we had earlier I am surprised by the
following.
darcy=> select '198.1.2.3/8'::inet;
?column?
--------
198/8
(1 row)
I would have expected it to print what I entered. If the above is
correct then perhaps we still need a cidr type that behaves differently
or rename this to cidr and write a new inet type.
Here is what I thought we were talking about taken from postings in
this list back in July.
From Bruce Momjian:
> My guess is that it is going to output x.x.x.x/32, but we should supply
> a function so they can get just the IP or the mask from the type. That
> way, people who don't want the cidr format can pull out the part they
> want.
This suggests that the whole address is stored and by default would be
output. Are we outputting just the network part now and expecting my
functions to get the host part?
I said:
> Perhaps there is an underlying difference of assumptions about what
> the actual type is. Let me take a stab at defining it (without
> naming it) and see if we're all on the same bus.
>
> I see the underlying data type storing two things, a host address
> (which can hold an IPv4 or IPv6 IP) and a netmask which can be
> stored as a small int, 8 bits is plenty. The input function would
> read IP numbers as follows (I'm making some of this up as I go.)
>
> x.x.x.x/y IP x.x.x.x with masklen y
> x.x.x.x:y.y.y.y IP x.x.x.x with masklen determined by examining
> y.y.y.y raising an exception if it is an invalid
> mask (defined as all ones followed by all zeroes)
> x.x.x.x IP x.x.x.x masklen of 32
>
> The output functions would print in a standard way, possibly allowing
> alternate representations like we do for money. Also, there would
> be functions to extract the host, the network or the netmask.
>
> Is this close to what everyone thinks or are we talking about completely
> different things?
No one contradicted me so I assumed that there was agreement.
From Bruce Momjian:
> That way, if they specify cidr bits, we store it. If they don't we make
> the bits field equal -1, and print/sort appropriately. The addr len is
> usually 3, but ip6 is also easy to add by making the addr len equal 6.
Supporting the idea of setting bits to -1 to mean an unspecified netmask.
I also checked doc/README.inet. It seems to support what I expect
although it doesn't mention setting bits to -1.
So what do I do? Should I redo the inet functions without using the
inet_net_* functions as described above or is the current behaviour the
one we wanted?
--
D'Arcy J.M. Cain <darcy@{druid|vex}.net> | Democracy is three wolves
http://www.druid.net/darcy/ | and a sheep voting on
+1 416 424 2871 (DoD#0082) (eNTP) | what's for dinner.
darcy@druid.net (D'Arcy J.M. Cain) writes:
> Based on the discussions we had earlier I am surprised by the
> following.
> darcy=> select '198.1.2.3/8'::inet;
> ?column?
> --------
> 198/8
> (1 row)
> I would have expected it to print what I entered.
Why? You told it to truncate the data to 8 bits, so it did. (At least,
that's my understanding of what the /n notation means, but maybe I'm
mistaken.)
regards, tom lane
Thus spake Tom Lane
> darcy@druid.net (D'Arcy J.M. Cain) writes:
> > Based on the discussions we had earlier I am surprised by the
> > following.
>
> > darcy=> select '198.1.2.3/8'::inet;
> > ?column?
> > --------
> > 198/8
> > (1 row)
>
> > I would have expected it to print what I entered.
>
> Why? You told it to truncate the data to 8 bits, so it did. (At least,
> that's my understanding of what the /n notation means, but maybe I'm
> mistaken.)
As I explained, I was surprised based on my understanding of the type
based on previous postings.
BTW, for a real world example of the usage I was expecting, look at an
Ascend router. In a connection profile you can specify an IP for the
remote side as, e.g., 198.96.119.225/28. The Ascend pulls out all
the information it needs to set up that connection. It assigns
198.96.119.225 to the remote host, it routes the 16 addresses in that
subnet to that interface and, if RIP is enabled (a bad idea but allowed)
then it knows to announce on 198.96.119.239.
--
D'Arcy J.M. Cain <darcy@{druid|vex}.net> | Democracy is three wolves
http://www.druid.net/darcy/ | and a sheep voting on
+1 416 424 2871 (DoD#0082) (eNTP) | what's for dinner.
my take on this is that (1) inet_net_pton() is definitely broken in that
it is willing to write one more byte of output than its arguments allow,
and (2) inet_net_{pton,ntop}() is not suitable for postgresql's use here.
i can fix (1) in bind 8.1.3, and postgresql already has it fixed in its
version of bind's library.
there is no way to fix (2) without an api change. we need, in order to
meet the stated needs of folks here who have uses for an inet-like type,
a way to have the prefix length be less than the size of the bitstring
being introduced. "16.1.0.2/28" was given as an example, that being a
way to give both a host address and its netmask on some wire. i've
wanted this functionality from time to time in the past and i can see
why for postgresql's purposes that's what should be provided for the
"inet" (or, given this change, more properly the "cidr") type.
but inet_net_ntop() only returns one thing (the prefix length) and the
caller is expected to know how many octets of mantissa were generated
based on this returned prefix length. i propose a new interface, which
would have a different name, a different argument list, and a different
use:
int
inet_cidr_pton(af, src, dst, size, int *used)
this would work very much the same as inet_net_pton() except for three
things: (1) nonzero trailing mantissas (host parts) would be allowed; (2)
the number of consumed octets of dst would be set into *used; and (3) there
would be absolutely no classful assumptions made if the pfxlen is not
specified in the input.
int
inet_cidr_ntop(ag, src, len, bits, dst, size)
this would work very much the same as inet_net_ntop() except that the
size (in octets) of src's mantissa would be given in the new "len" argument
and not imputed from "bits" as occurs now. "bits" would just be output
as the "/NN" at the end of the string, and would never be optional.
if this is agreeable, i'll code it up and submit it here for testing before
i push it out in bind 8.1.3. i really do agree with the functionality we've
drifted toward in this type -- if folks want to express the netmask of a
host address without getting in trouble for a nonzero host part that's
lost during parsing and storage, then they jolly well ought to be able to
do that.
Paul A Vixie <paul@vix.com> writes: > if this is agreeable, i'll code it up and submit it here for testing > before i push it out in bind 8.1.3. i really do agree with the > functionality we've drifted toward in this type -- if folks want to > express the netmask of a host address without getting in trouble for > a nonzero host part that's lost during parsing and storage, then > they jolly well ought to be able to do that. I certainly agree. We should then be able to switch very easily and cleanly to the use of these new input and output functions, and add the various utility functions that D'Arcy has outlined. Should be quick work. We should leave the type name as INET, I think. Bruce/Marc: we're OK for 6.4 with this still, right? Even if it takes a couple more days to get everything to fall into place? -tih -- Popularity is the hallmark of mediocrity. --Niles Crane, "Frasier"
> Paul A Vixie <paul@vix.com> writes: > > > if this is agreeable, i'll code it up and submit it here for testing > > before i push it out in bind 8.1.3. i really do agree with the > > functionality we've drifted toward in this type -- if folks want to > > express the netmask of a host address without getting in trouble for > > a nonzero host part that's lost during parsing and storage, then > > they jolly well ought to be able to do that. > > I certainly agree. We should then be able to switch very easily and > cleanly to the use of these new input and output functions, and add > the various utility functions that D'Arcy has outlined. Should be > quick work. > > We should leave the type name as INET, I think. > > Bruce/Marc: we're OK for 6.4 with this still, right? Even if it takes > a couple more days to get everything to fall into place? I think so. It does not affect people testing other things. -- Bruce Momjian | http://www.op.net/~candle maillist@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
Thus spake Tom Ivar Helbekkmo
> > if this is agreeable, i'll code it up and submit it here for testing
> > before i push it out in bind 8.1.3. i really do agree with the
> > functionality we've drifted toward in this type -- if folks want to
> > express the netmask of a host address without getting in trouble for
> > a nonzero host part that's lost during parsing and storage, then
> > they jolly well ought to be able to do that.
OK, I have cobbled up some functions and modified others in inet.c
based on the suggested cidr utility functions. I have made a few
assumptions which I am sure we will have to look at so that everyone
is comfortable with the code. I have added the following new functions.
char *inet_netmask(inet * addr);
int4 inet_masklen(inet * addr);
char *inet_host(inet * addr);
char *inet_network_no_bits(inet * addr);
char *inet_network_and_bits(inet * addr);
char *inet_broadcast(inet * addr);
The difference between inet_network_no_bits and inet_network_and_bits is
that for a.b.c.d/24, the former will return a.b.c and the latter will
return a.b.c/24. I couldn't use inet_network as a name anyway since
it conflicts with something in arpa/inet.h (On NetBSD -current).
If someone will add the necessary entries to the catalogues, I'll
start testing them. I'll post the changes to inet.c and builtins.h
to the patches list. I can't guarantee that they are bug-free yet
but it does compile and shouldn't interfere with anything anyone
else is doing.
--
D'Arcy J.M. Cain <darcy@{druid|vex}.net> | Democracy is three wolves
http://www.druid.net/darcy/ | and a sheep voting on
+1 416 424 2871 (DoD#0082) (eNTP) | what's for dinner.