Ipv6 network cleanup patch #2.

Why did you change the test for HAVE_UNIX_SOCKETS?

Kurt Roeckx writes:

Here is an updated patch for my ipv6 related changes.

Most important change is to get my cidr support in line with the
patch Andrew Dunstan made.

Could you explain what your patch changes to the rest of us?

- configure now checks inet_ntop() to check if there is ipv6
support, where we don't even use that function anymore now.
Maybe just checking for sockaddr_in6 will be enough?

Seems reasonable.

- pg_hba.conf.sample has ipv6 addresses in them. If you don't
have support for ipv6 it will give an error on that. Not sure
what to do with that.

That depends on what we decide(d?) to do with the IPv6 support in the
first place. Some people have suggested to silently disable IPv6 if the
kernel isn't set up for it, others wanted an explicit switch.
pg_hba.conf should then behave similarly.

--
Peter Eisentraut peter_e@gmx.net

On Sun, Jun 01, 2003 at 11:53:58PM +0200, Peter Eisentraut wrote:

Could you explain what your patch changes to the rest of us?

The main idea behind the patch is to work protocol indepedent, so
you don't have to write code for each protocol. This also means
reducing the use of things like HAVE_IPV6 and HAVE_UNIX_SOCKETS
to a minimum.

The right way to do that is using an API that supports that. The
use of getaddrinfo and getnameinfo is the right way to do that,
while inet_ntop and inet_pton are not. It was even changed
to SockAddr_ntop and pton to basicly do the same thing.

There already were some cleanups done that reduced the use of
HAVE_IPV6, because getaddrinfo2() was different in case you had
it or not.

You'll notice that the only place HAVE_IPV6 is used now is in
code that actually is related directly to network code. It's all
in ip.c except for 2 small ones that check the family in hba.c
and getaddrinfo.c.

Why did you change the test for HAVE_UNIX_SOCKETS?

Basicly, I've renamed HAVE_STRUCT_SOCKADDR_UN to
HAVE_UNIX_SOCKETS. HAVE_STRUCT_SOCKADDR_UN wasn't used anywhere
anymore. We don't need to have a struct sockaddr_un when the
system doesn't define it. It's basicly the same as what we do
for HAVE_IPV6.

I also thing it's a better way to find out what system do have
unix domain sockets and which don't than by saying all except
those 3 have it. Now you have to add them manually, while
configure can do it for you. This was actually already changed
before the test changed for the win32 port.

If configure can do something for you, why don't you do it?

Any other comments?

Kurt

On Sun, Jun 01, 2003 at 11:53:58PM +0200, Peter Eisentraut wrote:

- pg_hba.conf.sample has ipv6 addresses in them. If you don't
have support for ipv6 it will give an error on that. Not sure
what to do with that.

That depends on what we decide(d?) to do with the IPv6 support in the
first place. Some people have suggested to silently disable IPv6 if the
kernel isn't set up for it, others wanted an explicit switch.
pg_hba.conf should then behave similarly.

This has nothing to do with the kernel supporting it but with the
libc (or other libs) supporting it.

If we use our getaddrinfo() because the system doesn't provide
any, we give an error trying to read what is in the config file.

Kurt

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Kurt Roeckx wrote:

Here is an updated patch for my ipv6 related changes.

Most important change is to get my cidr support in line with the
patch Andrew Dunstan made.

I have 3 things that might need changing:
- configure now checks inet_ntop() to check if there is ipv6
support, where we don't even use that function anymore now.
Maybe just checking for sockaddr_in6 will be enough?

- pg_hba.conf.sample has ipv6 addresses in them. If you don't
have support for ipv6 it will give an error on that. Not sure
what to do with that.

- Maybe have a getnameinfo_unix(), like getaddrinfo_unix()?

Note that I only tested this on Linux so far.

I also attached a file with the changes in it.

Kurt

[ Attachment, skipping... ]

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Fri, Jun 06, 2003 at 01:41:30PM -0400, Bruce Momjian wrote:

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

You might want to change some of the close() calls into
closesocket(). Seems I forgot to update those.

Kurt

OK, thanks.

---------------------------------------------------------------------------

Kurt Roeckx wrote:

On Fri, Jun 06, 2003 at 01:41:30PM -0400, Bruce Momjian wrote:

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

You might want to change some of the close() calls into
closesocket(). Seems I forgot to update those.

Kurt

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Fri, Jun 06, 2003 at 01:41:30PM -0400, Bruce Momjian wrote:

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

I've made an updated patch since Tom Lane seem to have changed some
code that affects my patch.

Changes since last time:
- Use closesocket() everywhere now, instead of only a few places.
- Removed part of the getaddrinfo_unix fix that Tom Lane already
changed. He didn't fix the possible mem leak of the case
strlen(path) >= sizeof(unp->sun_path).
- He also rewrote connectDBStart() for asynch connections, so I
had to move my changes a little bit.

Kurt

Added updated version.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Kurt Roeckx wrote:

On Fri, Jun 06, 2003 at 01:41:30PM -0400, Bruce Momjian wrote:

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

I've made an updated patch since Tom Lane seem to have changed some
code that affects my patch.

Changes since last time:
- Use closesocket() everywhere now, instead of only a few places.
- Removed part of the getaddrinfo_unix fix that Tom Lane already
changed. He didn't fix the possible mem leak of the case
strlen(path) >= sizeof(unp->sun_path).
- He also rewrote connectDBStart() for asynch connections, so I
had to move my changes a little bit.

Kurt

[ Attachment, skipping... ]

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Patch applied.

I had to adjust a few places. I had to change:

if (port->raddr.sa.sa_family == AF_UNIX)

to:

if (port->raddr.addr.ss_family == AF_UNIX)

I saw other such changes in the patch, so I assume it is correct.

---------------------------------------------------------------------------

Kurt Roeckx wrote:

On Fri, Jun 06, 2003 at 01:41:30PM -0400, Bruce Momjian wrote:

I will apply this patch with the adjustments mentioned in later emails.
I will post the final version when I apply it.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

I've made an updated patch since Tom Lane seem to have changed some
code that affects my patch.

Changes since last time:
- Use closesocket() everywhere now, instead of only a few places.
- Removed part of the getaddrinfo_unix fix that Tom Lane already
changed. He didn't fix the possible mem leak of the case
strlen(path) >= sizeof(unp->sun_path).
- He also rewrote connectDBStart() for asynch connections, so I
had to move my changes a little bit.

Kurt

[ Attachment, skipping... ]

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Kurt Roeckx wrote:

I also thing it's a better way to find out what system do have
unix domain sockets and which don't than by saying all except
those 3 have it. Now you have to add them manually, while
configure can do it for you. This was actually already changed
before the test changed for the win32 port.

If configure can do something for you, why don't you do it?

I did have to modify your patch because the AC_DEFINE macro didn't have
a text string associated with it, and autoheader wouldn't work without
it. I ran autoconf too, of course.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Kurt Roeckx writes:

I also thing it's a better way to find out what system do have
unix domain sockets and which don't than by saying all except
those 3 have it. Now you have to add them manually, while
configure can do it for you. This was actually already changed
before the test changed for the win32 port.

The problem is that some systems have all the interfaces required for
Unix-domain sockets, but don't support them when you try to create or use
them. Therefore, the change is incorrect.

--
Peter Eisentraut peter_e@gmx.net