Date: Sat, 11 Apr 2020 17:58:43 -0700 From: Neel Chauhan <neel@neelc.org> To: "Rodney W. Grimes" <freebsd-rwg@gndrsh.dnsmgr.net> Cc: freebsd-hackers@freebsd.org, lev@freebsd.org, "Andrey V. Elsukov" <bu7cher@yandex.ru> Subject: Re: Committing one ipfw(8) userland patch Message-ID: <66a7224c6bab0f5f1c456707a84d39ae@neelc.org> In-Reply-To: <202004110447.03B4l0x0020210@gndrsh.dnsmgr.net> References: <202004110447.03B4l0x0020210@gndrsh.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
I have an updated diff which should work with almost all use cases (All I tested, including 1.2.3.4:255.255.255.0 which was broken in an earlier diff). I believe this is backwards-compatible with today's src-ip, but others should test as well. However, if the dual-stack src-ip is used, IPv4 and IPv6 addresses cannot be mixed if a comma is used for multiple addresses. Commas are for multiple IPv4 only or multiple IPv6 only. Being able to mix IPv4 and IPv6 addresses probably requires a separate opcode, or at least a new fill_ip() type function. If a dual-stack src-ip is not required/desired, and src-ip should remain IPv4-only (like it is today), let me know and I can revert to the older patch (which just adds src-ip4/dst-ip4 and src-ipv4/dst-ipv4 aliases). Separating the IP address types (v4 or v6) is now done with inet_pton(), and temporarily removing the comma/slash while doing this and putting it back after to be able to parse the IPv6/prefixlen format. Please review and give your opinions. -Neel On 2020-04-10 21:47, Rodney W. Grimes wrote: >> Thank you all for your feedback. >> >> Using the same Phabricator revision here: >> https://reviews.freebsd.org/D24234 >> >> I have added the src-ip4/dst-ip4 and src-ipv4/dst-ipv4 specifiers and >> made src-ip/dst-ip dual-stack, to be consistent with me/me4/me6 >> described in this thread. >> >> Could you all please give your opinions on it? > > I took a look at this and D24021 and am a bit confused as no > changes are actually being made to the kernel ipfw code, so > how does it know which are now dual vs single stack. As > far as I can see no actual change would be experienced > by the me/me4/me6 changes as they are all simply encoded > as O_IP_{SRC,DST}_ME when it gets to the kernel. > > It could be I am missing something, it has been a very > long time since I looked at the inards of ipfw. > > Also I am not sure if you want to attempt to flag no-op cases like > ipfw add ip4 from me6 to any > which I believe would be allowed and create a rule that never > matched anything. (Actually with the current code I think it > would still match local ipv4 address, which arguable is wrong.) > >> -Neel >> >> On 2020-04-10 04:10, Lev Serebryakov wrote: >> > On 10.04.2020 13:46, Andrey V. Elsukov wrote: >> > >> >> On 07.04.2020 20:35, Rodney W. Grimes wrote: >> >>> But that is not what this review does. I would be in support of >> >>> changing the "official" names to src-ip4/dst-ip4/src-ip6/dst-ip6 >> >>> and making src-ip/dst-ip a backwards compatible alias. >> >> >> >> I also think this idea sounds better. >> > >> > +1 > > I am glad people liked this solution, lets make sure it is > implemented cleanly and in a 100% backwards compatible way, > breaking ipfw rule sets is frowned upon by users.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?66a7224c6bab0f5f1c456707a84d39ae>