Summary: | [patch] changing interface ipaddress doesn't seem to work | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Guido Laubner <Guido.Laubner> | ||||||
Component: | kern | Assignee: | freebsd-net (Nobody) <net> | ||||||
Status: | Open --- | ||||||||
Severity: | Affects Only Me | Keywords: | patch | ||||||
Priority: | Normal | ||||||||
Version: | 4.0-RELEASE | ||||||||
Hardware: | Any | ||||||||
OS: | Any | ||||||||
Attachments: |
|
Description
Guido Laubner
2002-05-25 20:30:02 UTC
On Sat, May 25, 2002 at 12:23:40PM -0700, Guido Laubner wrote:
> >Description:
> When I change the IP address of an interface the system continues
> to use the old address. Ifconfig does display the new address though.
Have you deleted the old address? You should be able to do it with
a command like:
ifconfig inet 192.168.1.1 delete
David.
I've verified that this bug still exists in 4.7-RELEASE. I made sure to delete the old address (replacing it) rather than just adding the new one. What happens is that if you have a connected socket, then change the interface IP address, packets transmitted as a result of writing to the socket still use the old original IP address. This is "logical" because the socket is once and for all time bound to its original source IP address. Of course, any reply packets coming back from the remote host are dropped as they have an unrecognized destination IP address. My test used a TCP socket (ssh connection) but I'm sure that UDP would do the same thing as well. Interestingly, when you change the IP address back to its original value the socket comes back alive. Obviously it would be wrong to force outgoing packets use the new address, so probably the right thing to do here is return ENETDOWN to the write(2) (or whaveter) system call that prompted the outgoing data. -Archie __________________________________________________________________________ Archie Cobbs * Packet Design * http://www.packetdesign.com Here's a patch to -current that seems to fix the problem. I chose EADDRNOTAVAIL as the error code because: (a) The phrase "Can't assign requested address" seems most appropriate (b) It's consistent with what happens when you try to use an IP address that never existed in the first place, e.g., with IP_SENDSRCADDR. (c) You don't normally get this error from a send/write, which is when you'll get it -Archie __________________________________________________________________________ Archie Cobbs * Packet Design * http://www.packetdesign.com Index: sys/netinet/in.c =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/in.c,v retrieving revision 1.67 diff -u -r1.67 in.c --- sys/netinet/in.c 22 Oct 2002 22:50:38 -0000 1.67 +++ sys/netinet/in.c 22 Nov 2002 01:27:58 -0000 @@ -422,6 +422,11 @@ */ in_ifadown(&ia->ia_ifa, 1); /* + * Mark the interface address as no longer valid. + * Sockets that are bound to it should notice. + */ + ia->ia_ifa.ifa_flags |= RTF_REJECT; + /* * XXX horrible hack to detect that we are being called * from if_detach() */ Index: sys/netinet/in_pcb.c =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/in_pcb.c,v retrieving revision 1.114 diff -u -r1.114 in_pcb.c --- sys/netinet/in_pcb.c 8 Nov 2002 23:50:32 -0000 1.114 +++ sys/netinet/in_pcb.c 22 Nov 2002 01:27:58 -0000 @@ -57,6 +57,7 @@ #include <net/if.h> #include <net/if_types.h> #include <net/route.h> +#include <net/net_osdep.h> #include <netinet/in.h> #include <netinet/in_pcb.h> @@ -199,14 +200,17 @@ anonport = inp->inp_lport == 0 && (nam == NULL || ((struct sockaddr_in *)nam)->sin_port == 0); error = in_pcbbind_setup(inp, nam, &inp->inp_laddr.s_addr, - &inp->inp_lport, td); + &inp->inp_lport, &inp->inp_locia, td); if (error) return (error); if (in_pcbinshash(inp) != 0) { inp->inp_laddr.s_addr = INADDR_ANY; inp->inp_lport = 0; + inp->inp_locia = NULL; return (EAGAIN); } + if (inp->inp_locia != NULL) + IFAREF(&inp->inp_locia->ia_ifa); if (anonport) inp->inp_flags |= INP_ANONPORT; return (0); @@ -215,24 +219,29 @@ /* * Set up a bind operation on a PCB, performing port allocation * as required, but do not actually modify the PCB. Callers can - * either complete the bind by setting inp_laddr/inp_lport and - * calling in_pcbinshash(), or they can just use the resulting + * either complete the bind by setting inp_laddr/inp_lport/inp_locia + * and calling in_pcbinshash(), or they can just use the resulting * port and address to authorise the sending of a once-off packet. * - * On error, the values of *laddrp and *lportp are not changed. + * If iap is not NULL, *iap is set to the interface address corresponding + * to *laddrp, if any, but no new reference to it has been added. + * + * On error, the values of *laddrp, *lportp, and *iap are not changed. */ int -in_pcbbind_setup(inp, nam, laddrp, lportp, td) +in_pcbbind_setup(inp, nam, laddrp, lportp, iap, td) struct inpcb *inp; struct sockaddr *nam; in_addr_t *laddrp; u_short *lportp; + struct in_ifaddr **iap; struct thread *td; { struct socket *so = inp->inp_socket; unsigned short *lastport; struct sockaddr_in *sin; struct inpcbinfo *pcbinfo = inp->inp_pcbinfo; + struct in_ifaddr *ia = NULL; struct in_addr laddr; u_short lport = 0; int wild = 0, reuseport = (so->so_options & SO_REUSEPORT); @@ -280,7 +289,8 @@ } else if (sin->sin_addr.s_addr != INADDR_ANY) { sin->sin_port = 0; /* yech... */ bzero(&sin->sin_zero, sizeof(sin->sin_zero)); - if (ifa_ifwithaddr((struct sockaddr *)sin) == 0) + if ((ia = (struct in_ifaddr *)ifa_ifwithaddr( + (struct sockaddr *)sin)) == 0) return (EADDRNOTAVAIL); } laddr = sin->sin_addr; @@ -403,6 +413,8 @@ return (EINVAL); *laddrp = laddr.s_addr; *lportp = lport; + if (iap != NULL) + *iap = ia; return (0); } @@ -420,13 +432,14 @@ { u_short lport, fport; in_addr_t laddr, faddr; + struct in_ifaddr *locia; int anonport, error; lport = inp->inp_lport; laddr = inp->inp_laddr.s_addr; anonport = (lport == 0); error = in_pcbconnect_setup(inp, nam, &laddr, &lport, &faddr, &fport, - NULL, td); + NULL, &locia, td); if (error) return (error); @@ -444,6 +457,9 @@ /* Commit the remaining changes. */ inp->inp_lport = lport; inp->inp_laddr.s_addr = laddr; + inp->inp_locia = locia; + if (inp->inp_locia != NULL) + IFAREF(&inp->inp_locia->ia_ifa); inp->inp_faddr.s_addr = faddr; inp->inp_fport = fport; in_pcbrehash(inp); @@ -457,7 +473,9 @@ * On entry, *laddrp and *lportp should contain the current local * address and port for the PCB; these are updated to the values * that should be placed in inp_laddr and inp_lport to complete - * the connect. + * the connect. If iap is not NULL, *iap is set to the interface + * address corresponding to *laddrp, if any, but no new reference + * to it has been added. * * On success, *faddrp and *fportp will be set to the remote address * and port. These are not updated in the error case. @@ -468,7 +486,7 @@ * is set to NULL. */ int -in_pcbconnect_setup(inp, nam, laddrp, lportp, faddrp, fportp, oinpp, td) +in_pcbconnect_setup(inp, nam, laddrp, lportp, faddrp, fportp, oinpp, iap, td) register struct inpcb *inp; struct sockaddr *nam; in_addr_t *laddrp; @@ -476,10 +494,11 @@ in_addr_t *faddrp; u_short *fportp; struct inpcb **oinpp; + struct in_ifaddr **iap; struct thread *td; { struct sockaddr_in *sin = (struct sockaddr_in *)nam; - struct in_ifaddr *ia; + struct in_ifaddr *ia = NULL; struct sockaddr_in sa; struct ucred *cred; struct inpcb *oinp; @@ -506,7 +525,7 @@ sa.sin_len = sizeof(sa); sa.sin_family = AF_INET; error = in_pcbbind_setup(inp, (struct sockaddr *)&sa, - &laddr.s_addr, &lport, td); + &laddr.s_addr, &lport, &ia, td); if (error) return (error); } @@ -608,7 +627,8 @@ return (EADDRINUSE); } if (lport == 0) { - error = in_pcbbind_setup(inp, NULL, &laddr.s_addr, &lport, td); + error = in_pcbbind_setup(inp, NULL, &laddr.s_addr, &lport, + &ia, td); if (error) return (error); } @@ -616,6 +636,8 @@ *lportp = lport; *faddrp = faddr.s_addr; *fportp = fport; + if (iap != NULL) + *iap = ia; return (0); } @@ -649,6 +671,8 @@ (void)m_free(inp->inp_options); if (inp->inp_route.ro_rt) rtfree(inp->inp_route.ro_rt); + if (inp->inp_locia != NULL) + IFAFREE(&inp->inp_locia->ia_ifa); ip_freemoptions(inp->inp_moptions); inp->inp_vflag = 0; INP_LOCK_DESTROY(inp); Index: sys/netinet/in_pcb.h =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/in_pcb.h,v retrieving revision 1.57 diff -u -r1.57 in_pcb.h --- sys/netinet/in_pcb.h 12 Nov 2002 20:44:38 -0000 1.57 +++ sys/netinet/in_pcb.h 22 Nov 2002 01:27:58 -0000 @@ -75,6 +75,7 @@ struct in_endpoints { u_int16_t ie_fport; /* foreign port */ u_int16_t ie_lport; /* local port */ + struct in_ifaddr *ie_locia; /* locally bound address */ /* protocol dependent part, local and foreign addr */ union { /* foreign host table entry */ @@ -113,6 +114,7 @@ #define inc_isipv6 inc_flags /* temp compatability */ #define inc_fport inc_ie.ie_fport #define inc_lport inc_ie.ie_lport +#define inc_locia inc_ie.ie_locia #define inc_faddr inc_ie.ie_faddr #define inc_laddr inc_ie.ie_laddr #define inc_route inc_dependroute.inc4_route @@ -151,6 +153,7 @@ } inp_depend4; #define inp_fport inp_inc.inc_fport #define inp_lport inp_inc.inc_lport +#define inp_locia inp_inc.inc_locia #define inp_faddr inp_inc.inc_faddr #define inp_laddr inp_inc.inc_laddr #define inp_route inp_inc.inc_route @@ -315,6 +318,8 @@ #define INP_CHECK_SOCKAF(so, af) (INP_SOCKAF(so) == af) #ifdef _KERNEL +struct in_ifaddr; + extern int ipport_lowfirstauto; extern int ipport_lowlastauto; extern int ipport_firstauto; @@ -329,11 +334,11 @@ int in_pcballoc(struct socket *, struct inpcbinfo *, struct thread *); int in_pcbbind(struct inpcb *, struct sockaddr *, struct thread *); int in_pcbbind_setup(struct inpcb *, struct sockaddr *, in_addr_t *, - u_short *, struct thread *); + u_short *, struct in_ifaddr **, struct thread *); int in_pcbconnect(struct inpcb *, struct sockaddr *, struct thread *); int in_pcbconnect_setup(struct inpcb *, struct sockaddr *, in_addr_t *, u_short *, in_addr_t *, u_short *, struct inpcb **, - struct thread *); + struct in_ifaddr **, struct thread *); void in_pcbdetach(struct inpcb *); void in_pcbdisconnect(struct inpcb *); int in_pcbinshash(struct inpcb *); Index: sys/netinet/tcp_output.c =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/tcp_output.c,v retrieving revision 1.73 diff -u -r1.73 tcp_output.c --- sys/netinet/tcp_output.c 16 Oct 2002 19:16:33 -0000 1.73 +++ sys/netinet/tcp_output.c 22 Nov 2002 01:27:58 -0000 @@ -53,12 +53,15 @@ #include <sys/sysctl.h> #include <net/route.h> +#include <net/if.h> +#include <net/if_var.h> #include <netinet/in.h> #include <netinet/in_systm.h> #include <netinet/ip.h> #include <netinet/in_pcb.h> #include <netinet/ip_var.h> +#include <netinet/in_var.h> #ifdef INET6 #include <netinet6/in6_pcb.h> #include <netinet/ip6.h> @@ -686,6 +689,16 @@ /* this picks up the pseudo header (w/o the length) */ tcp_fillheaders(tp, ip, th); } + + /* + * Check that our local (source) IP address is still valid. + */ + if (tp->t_inpcb->inp_locia != NULL + && (tp->t_inpcb->inp_locia->ia_ifa.ifa_flags & RTF_REJECT) != 0) { + error = EADDRNOTAVAIL; + m_freem(m); + goto out; + } /* * Fill in fields, remembering maximum advertised Index: sys/netinet/tcp_usrreq.c =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/tcp_usrreq.c,v retrieving revision 1.84 diff -u -r1.84 tcp_usrreq.c --- sys/netinet/tcp_usrreq.c 24 Oct 2002 02:02:34 -0000 1.84 +++ sys/netinet/tcp_usrreq.c 22 Nov 2002 01:27:58 -0000 @@ -54,6 +54,7 @@ #include <sys/jail.h> #include <net/if.h> +#include <net/net_osdep.h> #include <net/route.h> #include <netinet/in.h> @@ -849,6 +850,7 @@ struct socket *so = inp->inp_socket; struct tcpcb *otp; struct rmxp_tao *taop; + struct in_ifaddr *locia; struct rmxp_tao tao_noncached; struct in_addr laddr; u_short lport; @@ -867,8 +869,9 @@ */ laddr = inp->inp_laddr; lport = inp->inp_lport; + locia = inp->inp_locia; error = in_pcbconnect_setup(inp, nam, &laddr.s_addr, &lport, - &inp->inp_faddr.s_addr, &inp->inp_fport, &oinp, td); + &inp->inp_faddr.s_addr, &inp->inp_fport, &oinp, &locia, td); if (error && oinp == NULL) return error; if (oinp) { @@ -883,6 +886,11 @@ return EADDRINUSE; } inp->inp_laddr = laddr; + if (inp->inp_locia != NULL) + IFAFREE(&inp->inp_locia->ia_ifa); + inp->inp_locia = locia; + if (inp->inp_locia != NULL) + IFAREF(&inp->inp_locia->ia_ifa); in_pcbrehash(inp); /* Compute window scaling to request. */ Index: sys/netinet/udp_usrreq.c =================================================================== RCS file: /home/cvs/freebsd/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.130 diff -u -r1.130 udp_usrreq.c --- sys/netinet/udp_usrreq.c 20 Nov 2002 19:00:54 -0000 1.130 +++ sys/netinet/udp_usrreq.c 22 Nov 2002 01:27:58 -0000 @@ -688,6 +688,7 @@ { register struct udpiphdr *ui; register int len = m->m_pkthdr.len; + struct in_ifaddr *locia; struct in_addr faddr, laddr; struct cmsghdr *cm; struct sockaddr_in *sin, src; @@ -754,13 +755,14 @@ goto release; laddr = inp->inp_laddr; lport = inp->inp_lport; + locia = inp->inp_locia; if (src.sin_addr.s_addr != INADDR_ANY) { if (lport == 0) { error = EINVAL; goto release; } error = in_pcbbind_setup(inp, (struct sockaddr *)&src, - &laddr.s_addr, &lport, td); + &laddr.s_addr, &lport, &locia, td); if (error) goto release; } @@ -774,7 +776,7 @@ goto release; } error = in_pcbconnect_setup(inp, addr, &laddr.s_addr, &lport, - &faddr.s_addr, &fport, NULL, td); + &faddr.s_addr, &fport, NULL, &locia, td); if (error) goto release; @@ -797,6 +799,15 @@ goto release; } } + + /* + * Check that the local (source) IP address is valid. + */ + if (locia != NULL && (locia->ia_ifa.ifa_flags & RTF_REJECT) != 0) { + error = EADDRNOTAVAIL; + goto release; + } + /* * Calculate data length and get a mbuf * for UDP and IP headers. @@ -1009,6 +1020,10 @@ s = splnet(); in_pcbdisconnect(inp); inp->inp_laddr.s_addr = INADDR_ANY; + if (inp->inp_locia != NULL) { + IFAFREE(&inp->inp_locia->ia_ifa); + inp->inp_locia = NULL; + } INP_UNLOCK(inp); INP_INFO_WUNLOCK(&udbinfo); splx(s); Responsible Changed From-To: freebsd-bugs->bms I'll try to look at this State Changed From-To: open->analyzed I've rejigged Archie's patch for -CURRENT and applied it locally. It seems to work well (tested with ntp, syslogd, and Quagga bgpd sessions). I'm inclined to commit but wish to follow-up on -net first. Please review the attached patch (which is a reworking of Archie's patch for -CURRENT). When the underlying IP address is changed, wildcard-bound UDP sockets which are temporarily bound locally for a sendto() (by userland apps such as ntp, syslogd etc) will begin using the new IP address, whilst apps using TCP (ssh, Quagga bgpd) will error out with EADDRINUSE. I would appreciate any feedback on our adopting this behaviour (which strikes me as similar to that of Solaris and a few other OSes). BMS Bruce M Simpson wrote: > > Please review the attached patch (which is a reworking of Archie's > patch for -CURRENT). When the underlying IP address is changed, > wildcard-bound UDP sockets which are temporarily bound locally for UDP socket which are not bound to a particular IP address would do the right thing already? I thought so. > a sendto() (by userland apps such as ntp, syslogd etc) will begin > using the new IP address, whilst apps using TCP (ssh, Quagga bgpd) will > error out with EADDRINUSE. This error is misleading. The address is gone, not in use. Isn't there a better fit? > I would appreciate any feedback on our adopting this behaviour (which > strikes me as similar to that of Solaris and a few other OSes). I have only quickly glanced over it, but it sounds the right thing to do. -- Andre On Sat, 3 Jul 2004, Andre Oppermann wrote: > Bruce M Simpson wrote: > > > > Please review the attached patch (which is a reworking of Archie's > > patch for -CURRENT). When the underlying IP address is changed, > > wildcard-bound UDP sockets which are temporarily bound locally for > > UDP socket which are not bound to a particular IP address would do the > right thing already? I thought so. Temporarily connecting sockets/pcbs during a datagram send with an explicit address (i.e., sendto()) occurs in both the UDP code and UNIX domain socket code. Since this is expensive, and also increases the potential for races of various sorts, it would be nice to do this in a more efficient form where possible. Robert N M Watson FreeBSD Core Team, TrustedBSD Projects robert@fledge.watson.org Principal Research Scientist, McAfee Research Before I suspend my work on this PR, here's a diff I pulled from trying to port the changes to today's CURRENT. The patch doesn't work but haven't tested exhaustively. Need to focus on other things. Responsible Changed From-To: bms->freebsd-net Back to the world for you, but not after actually doing some work on it... batch change: For bugs that match the following - Status Is In progress AND - Untouched since 2018-01-01. AND - Affects Base System OR Documentation DO: Reset to open status. Note: I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed. Keyword: patch or patch-ready – in lieu of summary line prefix: [patch] * bulk change for the keyword * summary lines may be edited manually (not in bulk). Keyword descriptions and search interface: <https://bugs.freebsd.org/bugzilla/describekeywords.cgi> |