Committing one ipfw(8) userland patch
Neel Chauhan
neel at neelc.org
Sun Apr 12 00:58:48 UTC 2020
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.
More information about the freebsd-hackers
mailing list