- User Since
- Dec 19 2016, 4:11 AM (362 w, 5 d)
Sun, Nov 5
This looks appropriate to me.
May 13 2023
You could add the hier(7) man page update to this review since this change is what effects that man page too.
Thank you Mike
Sep 14 2022
Jul 28 2022
With the constraints that are here I am in favor of allowing these.
Jul 17 2022
Adding some more eyes to this, IIRC this is ofed code and actually maintained and imported from some place else. Though I agree with Glebius that this is the right way to fix it, it may need or be desired to do this upstream. Hselesky is the infiniband expert.
Jul 12 2022
Jul 11 2022
Agree that this should be committed at the same time as the functional code changes.
Jul 10 2022
I like the numberical ip_allow_netX version as well. I also agree that changing rc.firewall at this time is premature.
Jul 7 2022
Adding a reference to https://reviews.freebsd.org/D19316 for some historical discussion.
Jul 5 2022
Dec 6 2021
Pi, thank you for spotting this, and bz thanks for the quick fix. This explains some issues I had when I tried to migrate to 13 on a router, but had no time to investigate what was going wrong.
Sep 24 2021
I strongly disagree with adding "site and user specific" tweaks to the dot.cshrc files. Further I note that a huge down side of systems that do this xterm tittle setting create very miss leading situations when the terminal disconnects for any reason (ie your xterm is now local, not remote). Some systems do infact try to clean that up in the logout scripts, but that doesnt work if your disconnected for any other reason. There is already a very clear default prompt setting that shows you the hostname of the system you are logged into.
Sep 18 2021
Sep 12 2021
Sep 6 2021
Add some reviewers, and pull in most of the subscribers fro D19316
Looks ok to me, just a comment on keeping code speed the same in almost all cases.
Jul 27 2021
Jul 9 2021
Jul 8 2021
Jun 19 2021
Jun 11 2021
May 21 2021
Though I am not a big fan of these, as the src tree would need a massive quantity of these added to make much of a dent at all in documenting what part of what rfc is being implemented, I wont stand in the way of one more near useless comment.
May 13 2021
May 3 2021
The adding of blank lines between all the forward declarations seems wrong to me.
May 2 2021
May 1 2021
Apr 29 2021
Apr 27 2021
I am accepting this without the IPSTAT_INC(ips_forward) issue being fixed, as it looks to me as if that is an existing and separate problem in the code. Probably a walk through should be done to see that ips_forward and ips_cantforward are all done correctly.
Apr 26 2021
I would still like to see the "169.254.0.0/16" changed to IN_LINKLOCAL, purpose of macro is to locate this value one place and one place only, scattering this string in the code undoes that.
Apr 23 2021
Apr 20 2021
Thanks for chasing these down
This is already a good improvement and acceptable as is with Greg's corrections.
Apr 10 2021
Apr 9 2021
Apr 7 2021
Though this is not exactly a normal service, to me listening on any by default is the correct thing to do here. One thing I would like to note, this code appears to only listens on one of IPv4 or IPv6, which is probably a short coming.
Apr 6 2021
Hum, not a core system functionality? May I remind you that ftp://ftp.freebsd.org exists? Though it probably does not run the base system ftpd, it DOES run that functionality. Also so much for the project being a democracy, seems that core@ and security@ now simply dictate to developers@ how things are to be done... nothing personal Gordon, but that is exactly how this is coming across.
Rather than spend cycles on deprecating USED features, please spend cycles on getting pkg base done so that those that feel they have no need for things like ftp/ftpd/telnet/telnetd/foo/bar/etc can choose to not have these and those of us who live and die by such tools can choose to have them.
Mar 5 2021
I am ok with doing this, though it still leaves all the other issues open.
Given the issues I would rather NOT add dscp support at all, until these issues can be addressed from down/upstream? The current situation, both in pfsense and freebsd is poor at best and may actually be one of the sources of issues we are seeing in our ECN data collection work. We know for a fact people are being told to use these ancient and obsolete TOS values by looking for them in blog posts and how-to's. Using the pry bar that is "reduce diffs between" as a reason to add a bad situation is an ever worse situation!
I'd be happy to extend the man page to make that clearer. Do you have any suggested phrasing?
I have a concern here, some of which goes beyond just this change, about aliasing DSCP to TOS. We (a small congestion control research group) have found that due to ambigious manual pages, headers and other factors, some present in PF it appears are causing mismarked traffic on the internet. DSCP is shifted left by the 2 ECN bits, the manual pages and any other documentation MUST be made explicity clear about this fact as to if your supplying the values as specified in the RFC"s on DSCP (unshifted) or are you applying TOS bits that may or may not have the ECN bits masked out.
This is a move in the right directions, at least this is state created by the installer.
Mar 4 2021
Maybe a seperate pass over this code to try and clean up the fact that we have used the string "/efi" many places such that it should probably become some type of #define or derived from the system make file values so that any changes to this value can be done in one place and one place only and all things using it are fixed.
Mar 3 2021
I personally prefer /efi, as I consider /boot to belong to FreeBSD and contain the FreeBSD specific boot code. EFI is a platform specific set of boot code and belongs to the system board, and though it may contain freeBSD specific code it may also contain OTHER code, especially in a multiboot environment.
Feb 14 2021
/24 is not an assumption of home lab, it is the longest of the 3 prefixes that would of been picked before, and infact is probably most often the correct mask to be used.
Feb 13 2021
I would like to see a survey of what some other OS's due in this situation. I have participated in discussions elsewhere on this issue and just blindly making these host only, IMHO, is probably going to silently cause a lot of failures on finger memory. Infact it is probalby safer to assume a /24 than a /32. BUTT I think the preferable solution here may be to return an error if the netmask is not specified, thus doing nothing rather than guessing and doing something wrong.
Feb 11 2021
No statement about do_prr being on by default?
Feb 9 2021
Feb 2 2021
Feb 1 2021
Jan 29 2021
Jan 25 2021
Jan 21 2021
How is the default 2 minute "linger" timer being implemented now if this is dead code? iirc this value use to be used when a socket was closed on the server side to control how long one waited in close_wait for the client to finish up the connection (server initiated close.)
Jan 17 2021
This fixes at least one dependance on /usr, I see another later, mktemp which is /usr/bin/mktemp. I would of marked it above, but a full file context was not uploaded to this review.
Dec 4 2020
Nov 24 2020
Oct 26 2020
Oct 25 2020
Though this is good intent, it is not a good approach. I'll assert again at one time we had erradicated all /usr/local from the base system, these above have crept in over time and again should be erradicated. There should never ever be a /usr/local #define in paths.h, having to recompile code to change this, again, is the wrong approach. This should _possibly_/_probably_ be moved to a kernel variable that can be changed at boot time.
Oct 1 2020
Sep 25 2020
Aug 25 2020
Aug 22 2020
Aug 19 2020
The summary of this review is lacking detail
Aug 17 2020
Aug 16 2020
Aug 15 2020
Aug 14 2020
Aug 13 2020
Is there any concern or additional work needed for ipV6?
Aug 12 2020
To all those harping about it not being foo or bar or zed, this *is* an underlying frame work that is very flexible and allows one to implement what ever foo, bar or zed configuration you want on top of it. This leaves json, xml, or any other implementation out of the base layer code and those can be implemented in a layer above this. This has been discussed for a some time extensively by those that attend the semi-monthly bhyve call.
Aug 7 2020
Aug 6 2020
Approving as mentor only.
Aug 1 2020
Jul 31 2020
Reflowing sentences and removal of .Tn seems like busy work to me. Much easier to just make mandoc ignore .Tn and igor silent on its usage. These macros DO make the groff man pages output look different, and IMHO, nicer.
Jul 30 2020
I still see request changes markers from emaste and jhb/bhyve, what are these changes?