There is regression in ipfw table adding, in compare with, for example, 9.0-RELEASE. Adding some hostname in a ipfw table leading in appearance of record '::/0 0' instead of correct IPv4 address. See below ("how to repeat the problem"). After this record appeared in ipfw table it passing ALL traffic, even all IPv4 addresses. Fix: Don't know How-To-Repeat: # uname -a FreeBSD nata1 10.0-STABLE FreeBSD 10.0-STABLE #1 r261884M: Mon Mar 31 16:01:36 MSK 2014 root@nata1:/usr/obj/usr/src/sys/ROUTER_HN amd64 # ipfw table 100 flush # host 3ds.ubrr.ru 3ds.ubrr.ru has address 91.208.121.209 # ipfw table 100 add 3ds.ubrr.ru # ipfw table 100 list ::/0 0 --- # uname -a FreeBSD utm.leskolovo.ru 9.2-RELEASE FreeBSD 9.2-RELEASE #0 r255898: Thu Sep 26 22:50:31 UTC 2013 root@bake.isc.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64 # ipfw table 92 flush # host 3ds.ubrr.ru 3ds.ubrr.ru has address 91.208.121.209 # ipfw table 92 add 3ds.ubrr.ru # ipfw table 92 list ::/0 0 ---- # uname -a FreeBSD stek 9.0-RELEASE FreeBSD 9.0-RELEASE #1: Fri Mar 16 05:08:15 MSK 2012 root@bsd9:/usr/obj/usr/src/sys/ROUTER_HOME_NETS amd64 # ipfw table 90 flush # host 3ds.ubrr.ru 3ds.ubrr.ru has address 91.208.121.209 # ipfw table 90 add 3ds.ubrr.ru # ipfw table 90 list 91.208.121.209/32 0
developers are still working on it, so 1. you can get the latest source which already fixed this issue. 2. or just comment out the whole else trunk as below } else { /* Port or any other key */ key = strtol(arg, &p, 10); /* Skip non-base 10 entries like 'fa1' */ if (p != arg) { pkey = (uint32_t *)paddr; *pkey = htonl(key); type = IPFW_TABLE_CIDR; addrlen = sizeof(uint32_t); } } 3. or only comment out this line in the else trunk type = IPFW_TABLE_CIDR; Can the developer help to explain what kind of data you want to support in the table?
OK, I will try. I think it's a time to add "helper" argument to *ipfw table add* command so that you could set data type explicity, is it IPv6 or IPv4. Something like that: ipfw table XX add beef.de ipv6 ipfw table XX add beef.de ipv4 AFAIK, it greatly reduce complexity of programming attempt to guess what kind of input data user meant. -- Best regards, Dennis Yusupoff, network engineer of Smart-Telecom ISP Russia, Saint-Petersburg
Responsible Changed From-To: freebsd-bugs->freebsd-ipfw Over to maintainer(s).
The problem continues. Try: # ipfw table 99 add 0.0.0.0/8 # ipfw table 99 list ::/8 0 # uname -a FreeBSD mail.xxxxxx.com.br 10.0-STABLE FreeBSD 10.0-STABLE #14 r265947: Tue May 13 08:18:03 BRT 2014 root@mail.xxxxxx.com.br:/usr/obj/usr/src/sys/GONDIM amd64 Cheers,
Responsible Changed From-To: freebsd-ipfw->melifaro Take.
Author: melifaro Date: Sat May 17 13:45:03 2014 New Revision: 266310 URL: http://svnweb.freebsd.org/changeset/base/266310 Log: Fix wrong formatting of 0.0.0.0/X table records in ipfw(8). Add `flags` u16 field to the hole in ipfw_table_xentry structure. Kernel has been guessing address family for supplied record based on xent length size. Userland, however, has been getting fixed-size ipfw_table_xentry structures guessing address family by checking address by IN6_IS_ADDR_V4COMPAT(). Fix this behavior by providing specific IPFW_TCF_INET flag for IPv4 records. PR: bin/189471 Submitted by: Dennis Yusupoff <dyr@smartspb.net> MFC after: 2 weeks Modified: head/sbin/ipfw/ipfw2.c head/sys/netinet/ip_fw.h head/sys/netpfil/ipfw/ip_fw_table.c Modified: head/sbin/ipfw/ipfw2.c ============================================================================== --- head/sbin/ipfw/ipfw2.c Sat May 17 12:47:11 2014 (r266309) +++ head/sbin/ipfw/ipfw2.c Sat May 17 13:45:03 2014 (r266310) @@ -4389,7 +4389,7 @@ table_list(uint16_t num, int need_header addr6 = &xent->k.addr6; - if (IN6_IS_ADDR_V4COMPAT(addr6)) { + if ((xent->flags & IPFW_TCF_INET) != 0) { /* IPv4 address */ inet_ntop(AF_INET, &addr6->s6_addr32[3], tbuf, sizeof(tbuf)); } else { Modified: head/sys/netinet/ip_fw.h ============================================================================== --- head/sys/netinet/ip_fw.h Sat May 17 12:47:11 2014 (r266309) +++ head/sys/netinet/ip_fw.h Sat May 17 13:45:03 2014 (r266310) @@ -614,6 +614,7 @@ typedef struct _ipfw_table_xentry { uint8_t type; /* entry type */ uint8_t masklen; /* mask length */ uint16_t tbl; /* table number */ + uint16_t flags; /* record flags */ uint32_t value; /* value */ union { /* Longest field needs to be aligned by 4-byte boundary */ @@ -621,6 +622,7 @@ typedef struct _ipfw_table_xentry { char iface[IF_NAMESIZE]; /* interface name */ } k; } ipfw_table_xentry; +#define IPFW_TCF_INET 0x01 /* CIDR flags: IPv4 record */ typedef struct _ipfw_table { u_int32_t size; /* size of entries in bytes */ Modified: head/sys/netpfil/ipfw/ip_fw_table.c ============================================================================== --- head/sys/netpfil/ipfw/ip_fw_table.c Sat May 17 12:47:11 2014 (r266309) +++ head/sys/netpfil/ipfw/ip_fw_table.c Sat May 17 13:45:03 2014 (r266310) @@ -697,6 +697,7 @@ dump_table_xentry_base(struct radix_node xent->masklen = 33 - ffs(ntohl(n->mask.sin_addr.s_addr)); /* Save IPv4 address as deprecated IPv6 compatible */ xent->k.addr6.s6_addr32[3] = n->addr.sin_addr.s_addr; + xent->flags = IPFW_TCF_INET; xent->value = n->value; tbl->cnt++; return (0); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
On 5/13/14 17:58, Dennis Yusupoff wrote: > OK, I will try. > > I think it's a time to add "helper" argument to *ipfw table add* > command so that you could set data type explicity, is it IPv6 or IPv4. > Something like that: > > ipfw table XX add beef.de ipv6 > ipfw table XX add beef.de ipv4 > > AFAIK, it greatly reduce complexity of programming attempt to guess > what kind of input data user meant. > > 13.05.2014 10:45, bycn82 ?????: >> developers are still working on it, so >> >> 1. you can get the latest source which already fixed this issue. >> >> 2. or just comment out the whole else trunk as below >> } else { >> /* Port or any other key */ >> key = strtol(arg, &p, 10); >> /* Skip non-base 10 entries like 'fa1' */ >> if (p != arg) { >> pkey = (uint32_t *)paddr; >> *pkey = htonl(key); >> type = IPFW_TABLE_CIDR; >> addrlen = sizeof(uint32_t); >> } >> } >> >> 3. or only comment out this line in the else trunk >> type = IPFW_TABLE_CIDR; >> >> >> Can the developer help to explain what kind of data you want to >> support in the table? > > -- > Best regards, > Dennis Yusupoff, > network engineer of > Smart-Telecom ISP > Russia, Saint-Petersburg 1.Totally agree with you,And maybe the better design will be "define the type of the table in the beginning." 2.Currently it supports only ipv4 &ipv6, actually it can support more types. 3.Currently it supports hostname/domain in the command line,But the the mapping between IP and domain is not always accurate because it is depends on the dns service.
A commit references this bug: Author: melifaro Date: Fri Sep 18 17:29:25 UTC 2015 New revision: 287963 URL: https://svnweb.freebsd.org/changeset/base/287963 Log: MFC r266310 Fix wrong formatting of 0.0.0.0/X table records in ipfw(8). Add `flags` u16 field to the hole in ipfw_table_xentry structure. Kernel has been guessing address family for supplied record based on xent length size. Userland, however, has been getting fixed-size ipfw_table_xentry structures guessing address family by checking address by IN6_IS_ADDR_V4COMPAT(). Fix this behavior by providing specific IPFW_TCF_INET flag for IPv4 records. PR: bin/189471,kern/200169 Changes: _U stable/10/ stable/10/sbin/ipfw/ipfw2.c stable/10/sys/netinet/ip_fw.h stable/10/sys/netpfil/ipfw/ip_fw_table.c
Thanks melifaro! Your commit in revision [1] fixed the problem. [1] https://svnweb.freebsd.org/changeset/base/287963