Thread: Ipv6 network cleanup patch #2.
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
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
Attachment
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