From owner-freebsd-hackers@freebsd.org Sun Apr 12 00:58:48 2020 Return-Path: Delivered-To: freebsd-hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id CD8E92A9100 for ; Sun, 12 Apr 2020 00:58:48 +0000 (UTC) (envelope-from neel@neelc.org) Received: from rainpuddle.neelc.org (rainpuddle.neelc.org [IPv6:2001:19f0:8001:fed:5400:2ff:fe73:c622]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 490D076JXWz4MbS; Sun, 12 Apr 2020 00:58:47 +0000 (UTC) (envelope-from neel@neelc.org) Received: from mail.neelc.org (rainpuddle.neelc.org [IPv6:2001:19f0:8001:fed:5400:2ff:fe73:c622]) by rainpuddle.neelc.org (Postfix) with ESMTPSA id 36481B288E; Sat, 11 Apr 2020 17:58:44 -0700 (PDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 11 Apr 2020 17:58:43 -0700 From: Neel Chauhan To: "Rodney W. Grimes" Cc: freebsd-hackers@freebsd.org, lev@freebsd.org, "Andrey V. Elsukov" Subject: Re: Committing one ipfw(8) userland patch In-Reply-To: <202004110447.03B4l0x0020210@gndrsh.dnsmgr.net> References: <202004110447.03B4l0x0020210@gndrsh.dnsmgr.net> User-Agent: Roundcube Webmail/1.4.1 Message-ID: <66a7224c6bab0f5f1c456707a84d39ae@neelc.org> X-Sender: neel@neelc.org X-Rspamd-Queue-Id: 490D076JXWz4MbS X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=pass (policy=none) header.from=neelc.org; spf=pass (mx1.freebsd.org: domain of neel@neelc.org designates 2001:19f0:8001:fed:5400:2ff:fe73:c622 as permitted sender) smtp.mailfrom=neel@neelc.org X-Spamd-Result: default: False [-5.99 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; RCPT_COUNT_THREE(0.00)[4]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+a]; FROM_HAS_DN(0.00)[]; MIME_GOOD(-0.10)[text/plain]; IP_SCORE(-3.29)[ip: (-9.84), ipnet: 2001:19f0:8000::/38(-4.92), asn: 20473(-1.63), country: US(-0.05)]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DMARC_POLICY_ALLOW(-0.50)[neelc.org,none]; RCVD_COUNT_ONE(0.00)[1]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:20473, ipnet:2001:19f0:8000::/38, country:US]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_TLS_ALL(0.00)[]; ONCE_RECEIVED(0.10)[] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Apr 2020 00:58:48 -0000 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.