From owner-freebsd-net@FreeBSD.ORG Wed Jan 30 09:25:47 2013 Return-Path: Delivered-To: net@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id C71A1EFF for ; Wed, 30 Jan 2013 09:25:47 +0000 (UTC) (envelope-from glebius@FreeBSD.org) Received: from cell.glebius.int.ru (glebius.int.ru [81.19.69.10]) by mx1.freebsd.org (Postfix) with ESMTP id 01B6F9DA for ; Wed, 30 Jan 2013 09:25:46 +0000 (UTC) Received: from cell.glebius.int.ru (localhost [127.0.0.1]) by cell.glebius.int.ru (8.14.6/8.14.6) with ESMTP id r0U9Pic2085156 for ; Wed, 30 Jan 2013 13:25:44 +0400 (MSK) (envelope-from glebius@FreeBSD.org) Received: (from glebius@localhost) by cell.glebius.int.ru (8.14.6/8.14.6/Submit) id r0U9Pik7085155 for net@FreeBSD.org; Wed, 30 Jan 2013 13:25:44 +0400 (MSK) (envelope-from glebius@FreeBSD.org) X-Authentication-Warning: cell.glebius.int.ru: glebius set sender to glebius@FreeBSD.org using -f Date: Wed, 30 Jan 2013 13:25:44 +0400 From: Gleb Smirnoff To: net@FreeBSD.org Subject: [patch] good bye sockaddr_inarp Message-ID: <20130130092544.GA84981@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8t9RHnE3ZwKMSgU+" Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Jan 2013 09:25:47 -0000 --8t9RHnE3ZwKMSgU+ Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Hello! It looks to me that the only thing the sockaddr_inarp was ever used for is to carry the SIN_PROXY flag. The SIN_PROXY flag in its turn, meant install a "proxy only" ARP entry. Such entry behaves as any "published" entry, but doesn't modify the routing table of the host. Please correct me, if I am wrong in the above ^^. Now, once ARP and routing are somewhat divorced, the meaning of "proxy only" is lost, because any entry doesn't affect routing table. This allows us to cleanup usage of SIN_PROXY and after that it appears that we don't need sockaddr_inarp at all. Attached patch does that. I didn't notice any functionality regressions, and I'd be grateful if anyone points me at a test case, that would. P.S. More reading on the history can be found here: http://www.freebsd.org/cgi/query-pr.cgi?pr=12357 -- Totus tuus, Glebius. --8t9RHnE3ZwKMSgU+ Content-Type: text/x-diff; charset=koi8-r Content-Disposition: attachment; filename="sockaddr_inarp.diff" Index: usr.sbin/ndp/ndp.c =================================================================== --- usr.sbin/ndp/ndp.c (revision 246095) +++ usr.sbin/ndp/ndp.c (working copy) @@ -436,9 +436,6 @@ goto overwrite; } } - /* - * IPv4 arp command retries with sin_other = SIN_PROXY here. - */ fprintf(stderr, "set: cannot configure a new entry\n"); return 1; } @@ -523,9 +520,6 @@ !(rtm->rtm_flags & RTF_GATEWAY)) { goto delete; } - /* - * IPv4 arp command retries with sin_other = SIN_PROXY here. - */ fprintf(stderr, "delete: cannot delete non-NDP entry\n"); return 1; } Index: usr.sbin/ppp/arp.c =================================================================== --- usr.sbin/ppp/arp.c (revision 246095) +++ usr.sbin/ppp/arp.c (working copy) @@ -91,7 +91,7 @@ */ static struct { struct rt_msghdr hdr; - struct sockaddr_inarp dst; + struct sockaddr_in dst; struct sockaddr_dl hwa; char extra[128]; } arpmsg; @@ -124,10 +124,9 @@ arpmsg.hdr.rtm_seq = ++bundle->routing_seq; arpmsg.hdr.rtm_addrs = RTA_DST | RTA_GATEWAY; arpmsg.hdr.rtm_inits = RTV_EXPIRE; - arpmsg.dst.sin_len = sizeof(struct sockaddr_inarp); + arpmsg.dst.sin_len = sizeof(struct sockaddr_in); arpmsg.dst.sin_family = AF_INET; arpmsg.dst.sin_addr.s_addr = addr.s_addr; - arpmsg.dst.sin_other = SIN_PROXY; arpmsg.hdr.rtm_msglen = (char *) &arpmsg.hwa - (char *) &arpmsg + arpmsg.hwa.sdl_len; Index: usr.sbin/arp/arp.c =================================================================== --- usr.sbin/arp/arp.c (revision 246095) +++ usr.sbin/arp/arp.c (working copy) @@ -81,28 +81,28 @@ #include typedef void (action_fn)(struct sockaddr_dl *sdl, - struct sockaddr_inarp *s_in, struct rt_msghdr *rtm); + struct sockaddr_in *s_in, struct rt_msghdr *rtm); static int search(u_long addr, action_fn *action); static action_fn print_entry; static action_fn nuke_entry; -static int delete(char *host, int do_proxy); +static int delete(char *host); static void usage(void); static int set(int argc, char **argv); static int get(char *host); static int file(char *name); static struct rt_msghdr *rtmsg(int cmd, - struct sockaddr_inarp *dst, struct sockaddr_dl *sdl); + struct sockaddr_in *dst, struct sockaddr_dl *sdl); static int get_ether_addr(in_addr_t ipaddr, struct ether_addr *hwaddr); -static struct sockaddr_inarp *getaddr(char *host); +static struct sockaddr_in *getaddr(char *host); static int valid_type(int type); static int nflag; /* no reverse dns lookups */ static char *rifname; static time_t expire_time; -static int flags, doing_proxy, proxy_only; +static int flags, doing_proxy; /* which function we're supposed to do */ #define F_GET 1 @@ -179,7 +179,7 @@ if (argc < 2 || argc > 6) usage(); if (func == F_REPLACE) - (void)delete(argv[0], 0); + delete(argv[0]); rtn = set(argc, argv) ? 1 : 0; break; case F_DELETE: @@ -187,15 +187,8 @@ if (argc != 0) usage(); search(0, nuke_entry); - } else { - if (argc == 2 && strncmp(argv[1], "pub", 3) == 0) - ch = SIN_PROXY; - else if (argc == 1) - ch = 0; - else - usage(); - rtn = delete(argv[0], ch); - } + } else + rtn = delete(argv[0]); break; case F_FILESET: if (argc != 1) @@ -246,15 +239,15 @@ } /* - * Given a hostname, fills up a (static) struct sockaddr_inarp with + * Given a hostname, fills up a (static) struct sockaddr_in with * the address of the host and returns a pointer to the * structure. */ -static struct sockaddr_inarp * +static struct sockaddr_in * getaddr(char *host) { struct hostent *hp; - static struct sockaddr_inarp reply; + static struct sockaddr_in reply; bzero(&reply, sizeof(reply)); reply.sin_len = sizeof(reply); @@ -298,8 +291,8 @@ static int set(int argc, char **argv) { - struct sockaddr_inarp *addr; - struct sockaddr_inarp *dst; /* what are we looking for */ + struct sockaddr_in *addr; + struct sockaddr_in *dst; /* what are we looking for */ struct sockaddr_dl *sdl; struct rt_msghdr *rtm; struct ether_addr *ea; @@ -316,7 +309,7 @@ dst = getaddr(host); if (dst == NULL) return (1); - doing_proxy = flags = proxy_only = expire_time = 0; + doing_proxy = flags = expire_time = 0; while (argc-- > 0) { if (strncmp(argv[0], "temp", 4) == 0) { struct timespec tp; @@ -332,7 +325,12 @@ flags |= RTF_ANNOUNCE; doing_proxy = 1; if (argc && strncmp(argv[1], "only", 3) == 0) { - proxy_only = 1; + /* + * Compatibility: in pre FreeBSD 8 times + * the "only" keyword used to mean that + * an ARP entry should be announced, but + * not installed into routing table. + */ argc--; argv++; } } else if (strncmp(argv[0], "blackhole", 9) == 0) { @@ -385,7 +383,7 @@ warn("%s", host); return (1); } - addr = (struct sockaddr_inarp *)(rtm + 1); + addr = (struct sockaddr_in *)(rtm + 1); sdl = (struct sockaddr_dl *)(SA_SIZE(addr) + (char *)addr); if ((sdl->sdl_family != AF_LINK) || @@ -405,7 +403,7 @@ static int get(char *host) { - struct sockaddr_inarp *addr; + struct sockaddr_in *addr; addr = getaddr(host); if (addr == NULL) @@ -425,9 +423,9 @@ * Delete an arp entry */ static int -delete(char *host, int do_proxy) +delete(char *host) { - struct sockaddr_inarp *addr, *dst; + struct sockaddr_in *addr, *dst; struct rt_msghdr *rtm; struct sockaddr_dl *sdl; struct sockaddr_dl sdl_m; @@ -456,7 +454,7 @@ warn("%s", host); return (1); } - addr = (struct sockaddr_inarp *)(rtm + 1); + addr = (struct sockaddr_in *)(rtm + 1); sdl = (struct sockaddr_dl *)(SA_SIZE(addr) + (char *)addr); /* @@ -504,7 +502,7 @@ size_t needed; char *lim, *buf, *next; struct rt_msghdr *rtm; - struct sockaddr_inarp *sin2; + struct sockaddr_in *sin2; struct sockaddr_dl *sdl; char ifname[IF_NAMESIZE]; int st, found_entry = 0; @@ -538,7 +536,7 @@ lim = buf + needed; for (next = buf; next < lim; next += rtm->rtm_msglen) { rtm = (struct rt_msghdr *)next; - sin2 = (struct sockaddr_inarp *)(rtm + 1); + sin2 = (struct sockaddr_in *)(rtm + 1); sdl = (struct sockaddr_dl *)((char *)sin2 + SA_SIZE(sin2)); if (rifname && if_indextoname(sdl->sdl_index, ifname) && strcmp(ifname, rifname)) @@ -562,7 +560,7 @@ static void print_entry(struct sockaddr_dl *sdl, - struct sockaddr_inarp *addr, struct rt_msghdr *rtm) + struct sockaddr_in *addr, struct rt_msghdr *rtm) { const char *host; struct hostent *hp; @@ -612,8 +610,6 @@ else printf(" expired"); } - if (addr->sin_other & SIN_PROXY) - printf(" published (proxy only)"); if (rtm->rtm_flags & RTF_ANNOUNCE) printf(" published"); switch(sdl->sdl_type) { @@ -659,12 +655,12 @@ */ static void nuke_entry(struct sockaddr_dl *sdl __unused, - struct sockaddr_inarp *addr, struct rt_msghdr *rtm __unused) + struct sockaddr_in *addr, struct rt_msghdr *rtm __unused) { char ip[20]; snprintf(ip, sizeof(ip), "%s", inet_ntoa(addr->sin_addr)); - (void)delete(ip, 0); + delete(ip); } static void @@ -682,7 +678,7 @@ } static struct rt_msghdr * -rtmsg(int cmd, struct sockaddr_inarp *dst, struct sockaddr_dl *sdl) +rtmsg(int cmd, struct sockaddr_in *dst, struct sockaddr_dl *sdl) { static int seq; int rlen; @@ -728,14 +724,9 @@ rtm->rtm_rmx.rmx_expire = expire_time; rtm->rtm_inits = RTV_EXPIRE; rtm->rtm_flags |= (RTF_HOST | RTF_STATIC | RTF_LLDATA); - dst->sin_other = 0; if (doing_proxy) { - if (proxy_only) - dst->sin_other = SIN_PROXY; - else { - rtm->rtm_addrs |= RTA_NETMASK; - rtm->rtm_flags &= ~RTF_HOST; - } + rtm->rtm_addrs |= RTA_NETMASK; + rtm->rtm_flags &= ~RTF_HOST; } /* FALLTHROUGH */ case RTM_GET: Index: usr.sbin/rarpd/rarpd.c =================================================================== --- usr.sbin/rarpd/rarpd.c (revision 246095) +++ usr.sbin/rarpd/rarpd.c (working copy) @@ -692,11 +692,10 @@ * host (i.e. the guy running rarpd), won't try to ARP for the hardware * address of the guy being booted (he cannot answer the ARP). */ -struct sockaddr_inarp sin_inarp = { - sizeof(struct sockaddr_inarp), AF_INET, 0, +struct sockaddr_in sin_inarp = { + sizeof(struct sockaddr_in), AF_INET, 0, {0}, {0}, - 0, 0 }; struct sockaddr_dl sin_dl = { sizeof(struct sockaddr_dl), AF_LINK, 0, IFT_ETHER, 0, 6, @@ -712,7 +711,7 @@ { struct timespec tp; int cc; - struct sockaddr_inarp *ar, *ar2; + struct sockaddr_in *ar, *ar2; struct sockaddr_dl *ll, *ll2; struct rt_msghdr *rt; int xtype, xindex; @@ -740,7 +739,7 @@ rt->rtm_addrs = RTA_DST; rt->rtm_type = RTM_GET; rt->rtm_seq = ++seq; - ar2 = (struct sockaddr_inarp *)rtmsg.rtspace; + ar2 = (struct sockaddr_in *)rtmsg.rtspace; bcopy(ar, ar2, sizeof(*ar)); rt->rtm_msglen = sizeof(*rt) + sizeof(*ar); errno = 0; Index: sbin/route/route.c =================================================================== --- sbin/route/route.c (revision 246095) +++ sbin/route/route.c (working copy) @@ -86,7 +86,6 @@ #endif struct sockaddr_at sat; struct sockaddr_dl sdl; - struct sockaddr_inarp sinarp; struct sockaddr_storage ss; /* added to avoid memory overrun */ } so_dst, so_gate, so_mask, so_genmask, so_ifa, so_ifp; @@ -923,10 +922,8 @@ flags |= RTF_HOST; if ((nrflags & F_INTERFACE) == 0) flags |= RTF_GATEWAY; - if (nrflags & F_PROXY) { - so_dst.sinarp.sin_other = SIN_PROXY; + if (nrflags & F_PROXY) flags |= RTF_ANNOUNCE; - } if (dest == NULL) dest = ""; if (gateway == NULL) Index: contrib/ipfilter/ipsend/44arp.c =================================================================== --- contrib/ipfilter/ipsend/44arp.c (revision 246095) +++ contrib/ipfilter/ipsend/44arp.c (working copy) @@ -72,7 +72,7 @@ size_t needed; char *lim, *buf, *next; struct rt_msghdr *rtm; - struct sockaddr_inarp *sin; + struct sockaddr_in *sin; struct sockaddr_dl *sdl; #ifdef IPSEND @@ -113,7 +113,7 @@ for (next = buf; next < lim; next += rtm->rtm_msglen) { rtm = (struct rt_msghdr *)next; - sin = (struct sockaddr_inarp *)(rtm + 1); + sin = (struct sockaddr_in *)(rtm + 1); sdl = (struct sockaddr_dl *)(sin + 1); if (!bcmp(addr, (char *)&sin->sin_addr, sizeof(struct in_addr))) Index: libexec/bootpd/rtmsg.c =================================================================== --- libexec/bootpd/rtmsg.c (revision 246095) +++ libexec/bootpd/rtmsg.c (working copy) @@ -106,9 +106,9 @@ } static struct sockaddr_in so_mask = {8, 0, 0, { 0xffffffff}}; -static struct sockaddr_inarp blank_sin = {sizeof(blank_sin), AF_INET }, sin_m; +static struct sockaddr_in blank_sin = {sizeof(blank_sin), AF_INET }, sin_m; static struct sockaddr_dl blank_sdl = {sizeof(blank_sdl), AF_LINK }, sdl_m; -static int expire_time, flags, export_only, doing_proxy; +static int expire_time, flags, doing_proxy; static struct { struct rt_msghdr m_rtm; char m_space[512]; @@ -122,7 +122,7 @@ char *eaddr; int len; { - register struct sockaddr_inarp *sin = &sin_m; + register struct sockaddr_in *sin = &sin_m; register struct sockaddr_dl *sdl; register struct rt_msghdr *rtm = &(m_rtmsg.m_rtm); u_char *ea; @@ -137,7 +137,7 @@ ea = (u_char *)LLADDR(&sdl_m); bcopy(eaddr, ea, len); sdl_m.sdl_alen = len; - doing_proxy = flags = export_only = expire_time = 0; + doing_proxy = flags = expire_time = 0; /* make arp entry temporary */ clock_gettime(CLOCK_MONOTONIC, &tp); @@ -148,7 +148,7 @@ report(LOG_WARNING, "rtmget: %s", strerror(errno)); return (1); } - sin = (struct sockaddr_inarp *)(rtm + 1); + sin = (struct sockaddr_in *)(rtm + 1); sdl = (struct sockaddr_dl *)(sin->sin_len + (char *)sin); if (sin->sin_addr.s_addr == sin_m.sin_addr.s_addr) { if (sdl->sdl_family == AF_LINK && @@ -163,13 +163,6 @@ inet_ntoa(sin->sin_addr)); return (1); } - if (sin_m.sin_other & SIN_PROXY) { - report(LOG_WARNING, - "set: proxy entry exists for non 802 device\n"); - return(1); - } - sin_m.sin_other = SIN_PROXY; - export_only = 1; goto tryagain; } overwrite: @@ -209,14 +202,9 @@ rtm->rtm_rmx.rmx_expire = expire_time; rtm->rtm_inits = RTV_EXPIRE; rtm->rtm_flags |= (RTF_HOST | RTF_STATIC | RTF_LLDATA); - sin_m.sin_other = 0; if (doing_proxy) { - if (export_only) - sin_m.sin_other = SIN_PROXY; - else { - rtm->rtm_addrs |= RTA_NETMASK; - rtm->rtm_flags &= ~RTF_HOST; - } + rtm->rtm_addrs |= RTA_NETMASK; + rtm->rtm_flags &= ~RTF_HOST; } /* FALLTHROUGH */ case RTM_GET: Index: sys/netinet/in.c =================================================================== --- sys/netinet/in.c (revision 246095) +++ sys/netinet/in.c (working copy) @@ -1494,7 +1494,7 @@ /* XXX stack use */ struct { struct rt_msghdr rtm; - struct sockaddr_inarp sin; + struct sockaddr_in sin; struct sockaddr_dl sdl; } arpc; int error, i; @@ -1515,7 +1515,7 @@ /* * produce a msg made of: * struct rt_msghdr; - * struct sockaddr_inarp; (IPv4) + * struct sockaddr_in; (IPv4) * struct sockaddr_dl; */ bzero(&arpc, sizeof(arpc)); @@ -1529,12 +1529,8 @@ arpc.sin.sin_addr.s_addr = SIN(lle)->sin_addr.s_addr; /* publish */ - if (lle->la_flags & LLE_PUB) { + if (lle->la_flags & LLE_PUB) arpc.rtm.rtm_flags |= RTF_ANNOUNCE; - /* proxy only */ - if (lle->la_flags & LLE_PROXY) - arpc.sin.sin_other = SIN_PROXY; - } sdl = &arpc.sdl; sdl->sdl_family = AF_LINK; Index: sys/netinet/if_ether.h =================================================================== --- sys/netinet/if_ether.h (revision 246095) +++ sys/netinet/if_ether.h (working copy) @@ -89,16 +89,6 @@ #define arp_pln ea_hdr.ar_pln #define arp_op ea_hdr.ar_op -struct sockaddr_inarp { - u_char sin_len; - u_char sin_family; - u_short sin_port; - struct in_addr sin_addr; - struct in_addr sin_srcaddr; - u_short sin_tos; - u_short sin_other; -#define SIN_PROXY 1 -}; /* * IP and ethernet specific routing flags */ Index: sys/net/if_llatbl.c =================================================================== --- sys/net/if_llatbl.c (revision 246095) +++ sys/net/if_llatbl.c (working copy) @@ -285,28 +285,8 @@ switch (rtm->rtm_type) { case RTM_ADD: - if (rtm->rtm_flags & RTF_ANNOUNCE) { + if (rtm->rtm_flags & RTF_ANNOUNCE) flags |= LLE_PUB; -#ifdef INET - if (dst->sa_family == AF_INET && - ((struct sockaddr_inarp *)dst)->sin_other != 0) { - struct rtentry *rt; - ((struct sockaddr_inarp *)dst)->sin_other = 0; - rt = rtalloc1(dst, 0, 0); - if (rt == NULL || !(rt->rt_flags & RTF_HOST)) { - log(LOG_INFO, "%s: RTM_ADD publish " - "(proxy only) is invalid\n", - __func__); - if (rt) - RTFREE_LOCKED(rt); - return EINVAL; - } - RTFREE_LOCKED(rt); - - flags |= LLE_PROXY; - } -#endif - } flags |= LLE_CREATE; break; @@ -345,7 +325,7 @@ * LLE_DELETED flag, and reset the expiration timer */ bcopy(LLADDR(dl), &lle->ll_addr, ifp->if_addrlen); - lle->la_flags |= (flags & (LLE_PUB | LLE_PROXY)); + lle->la_flags |= (flags & LLE_PUB); lle->la_flags |= LLE_VALID; lle->la_flags &= ~LLE_DELETED; #ifdef INET6 @@ -367,15 +347,12 @@ laflags = lle->la_flags; LLE_WUNLOCK(lle); #ifdef INET - /* gratuitous ARP */ - if ((laflags & LLE_PUB) && dst->sa_family == AF_INET) { + /* gratuitous ARP */ + if ((laflags & LLE_PUB) && dst->sa_family == AF_INET) arprequest(ifp, &((struct sockaddr_in *)dst)->sin_addr, &((struct sockaddr_in *)dst)->sin_addr, - ((laflags & LLE_PROXY) ? - (u_char *)IF_LLADDR(ifp) : - (u_char *)LLADDR(dl))); - } + (u_char *)LLADDR(dl)); #endif } else { if (flags & LLE_EXCLUSIVE) Index: sys/net/if_llatbl.h =================================================================== --- sys/net/if_llatbl.h (revision 246095) +++ sys/net/if_llatbl.h (working copy) @@ -172,7 +172,6 @@ #define LLE_STATIC 0x0002 /* entry is static */ #define LLE_IFADDR 0x0004 /* entry is interface addr */ #define LLE_VALID 0x0008 /* ll_addr is valid */ -#define LLE_PROXY 0x0010 /* proxy entry ??? */ #define LLE_PUB 0x0020 /* publish entry ??? */ #define LLE_LINKED 0x0040 /* linked to lookup structure */ #define LLE_EXCLUSIVE 0x2000 /* return lle xlocked */ --8t9RHnE3ZwKMSgU+--