Page MenuHomeFreeBSD
Differential D38644

netinet6: allow disabling excess log messages
ClosedPublic
Actions

Authored by kaktus on Feb 16 2023, 6:39 PM.
Tags
None
Referenced Files
F146591368: D38644.id117469.diff
Tue, Mar 3, 10:20 PM
F146581402: D38644.id117810.diff
Tue, Mar 3, 8:16 PM
Unknown Object (File)
Mon, Mar 2, 2:28 PM
Unknown Object (File)
Sun, Mar 1, 9:13 PM
Unknown Object (File)
Sat, Feb 28, 6:28 PM
Unknown Object (File)
Sat, Feb 28, 6:02 PM
Unknown Object (File)
Mon, Feb 23, 9:20 PM
Unknown Object (File)
Sun, Feb 22, 10:30 PM
View All Files
Subscribers
ae
cy
glebius
imp
melifaro
pauamma_gundo.com
zarychtam_plan-b.pwste.edu.pl
zlei

Details

Reviewers
zlei
cy
pauamma_gundo.com
Group Reviewers
network
manpages
Commits
rG3eaffc626589: netinet6: allow disabling excess log messages
Summary

RFC 4443 specifies cases where certain packets, like those originating from local-scope addresses destined outside of the scope shouldn't be forwarded. The current practice is to drop them, send ICMPv6 message where appropriate, and log the message:

cannot forward src fe80:10::426:82ff:fe36:1d8, dst 2001:db8:db8::10, nxt 58, rcvif vlan5, outif vlan2

At times the volume of such messages cat get very high. Let's allow local admins to disable such messages on per vnet basis, keeping the current default (log).

Reported by: zarychtam@plan-b.pwste.edu.pl

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kaktus created this revision.Feb 16 2023, 6:39 PM
Herald added subscribers: glebius, melifaro, ae, imp. · View Herald TranscriptFeb 16 2023, 6:39 PM
kaktus requested review of this revision.Feb 16 2023, 6:39 PM
kaktus updated this revision to Diff 117469.Feb 16 2023, 8:18 PM
zlei accepted this revision.Feb 17 2023, 2:17 AM
zlei added a subscriber: zlei.

Looks good to me

sys/netinet6/in6_proto.c
409

I think for the reasons outside of firewall rules can be omitted.

People utilize firewalls should be aware that firewalls have their own logging facilities for the dropped packets (those can't be forwarded).

sys/netinet6/ip6_forward.c
117–118

As V_ip6_log_discarded is mostly constant then I'd prefer to check it first.

This revision is now accepted and ready to land.Feb 17 2023, 2:17 AM
zarychtam_plan-b.pwste.edu.pl added a comment.Feb 17 2023, 6:12 AM

Thanks for taking care of it, looks to me like another positive change.

sys/netinet6/in6_proto.c
408

Perhaps a sentence like "Log packets originating from link-local source addresses that weren't forwarded" will be more accurate (if a precise description is really required), but I am not the best at phrasing it.

kaktus updated this revision to Diff 117767.Feb 22 2023, 3:54 PM
kaktus edited the summary of this revision. (Show Details)
kaktus added a reviewer: manpages.
This revision now requires review to proceed.Feb 22 2023, 3:54 PM
kaktus added inline comments.Feb 22 2023, 3:56 PM
sys/netinet6/in6_proto.c
409

I believe they do but the extra 41 bytes of description could save some time for those who may be in a hurry to find a solution for a local firewall issue :-)

kaktus updated this revision to Diff 117768.Feb 22 2023, 4:02 PM
kaktus updated this revision to Diff 117774.Feb 22 2023, 5:23 PM

Rename to match log statement, stats variable and counter description presented by netstat -s.

Discussed with: zarychtam@plan-b.pwste.edu.pl

cy requested changes to this revision.Feb 22 2023, 6:01 PM
cy added a subscriber: cy.
cy added inline comments.
sys/netinet6/in6_proto.c
409

There are other examples (MIBs) which enable dropping packets. They do not mention firewalls. I agree with zlei. The extra verbiage should be removed.

This revision now requires changes to proceed.Feb 22 2023, 6:01 PM
pauamma_gundo.com requested changes to this revision.Feb 22 2023, 7:36 PM
pauamma_gundo.com added a subscriber: pauamma_gundo.com.
pauamma_gundo.com added inline comments.
share/man/man4/inet6.4
32

Remember to bump.

348
459

Add RFC 4443?

460–468

Out of scope for this change, but noting while here that a 23 year old draft isn't the best reference.

kaktus updated this revision to Diff 117785.Feb 22 2023, 9:19 PM

Address comments:

  • by popular demand don't mention firewall
  • update man page wording
share/man/man4/inet6.4
459

Other protocols use the SEE ALSO section for that, for example RFC4443 is mentioned like that in icmp6(4) and I don't know which way is preferred by the docs team. This manual page as a whole could use some love as there are many other RFCs to be mentioned in the STANDARDS / SEE ALSO sections.

460–468

It seems that this extension hasn't been ratified to official standard[1] but our implementation supports it so maybe as a future rework of this whole manpage at least the "work in progress material" should be removed.

  1. https://datatracker.ietf.org/doc/draft-ietf-ipngwg-scopedaddr-format/
pauamma_gundo.com added inline comments.Feb 22 2023, 11:17 PM
share/man/man4/inet6.4
459

"SEE ALSO" works for me. I have no marked preference, but it would be puzzling, when the ICMP6 RFC is there in icmp6(4), to have it as a standard in this page.

kaktus updated this revision to Diff 117810.Feb 23 2023, 12:40 PM

as requested mention RFC4443 in man page

tidy up

mjg added a subscriber: mjg.Feb 23 2023, 12:43 PM

the standard approach is to rate limit with ppsratecheck

i would say this should be 2 patches: 1. do the rate check 2. add the tunable

cy added inline comments.Feb 23 2023, 4:15 PM
sys/netinet6/in6_proto.c
349

Just a nit but cannot is a little more formal than can't. Seems to flow a little better too.

pauamma_gundo.com accepted this revision.Feb 23 2023, 10:23 PM
kaktus mentioned this in D38758: net.inet6.ip6.log_interval: use ppsratecheck(9) inernally.Feb 28 2023, 1:07 PM
In D38644#881703, @mjg wrote:

Please refrain from making further requests if you're unwilling to review them later.

kaktus removed a subscriber: mjg.Feb 28 2023, 1:08 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 13 2023, 4:50 PM
Closed by commit rG3eaffc626589: netinet6: allow disabling excess log messages (authored by kaktus). · Explain Why
This revision was automatically updated to reflect the committed changes.
kaktus added a commit: rG3eaffc626589: netinet6: allow disabling excess log messages.

Revision Contents
Changeset List

PathSize
share/
man/
man4/
20 lines
sys/
netinet6/
5 lines
6 lines
3 lines
3 lines

Diff 118748

View Options

share/man/man4/inet6.4

Loading...
View Options

sys/netinet6/in6_proto.c

Loading...
View Options

sys/netinet6/ip6_forward.c

Loading...
View Options

sys/netinet6/ip6_mroute.c

Loading...
View Options

sys/netinet6/ip6_var.h

Loading...