Differential D19317

in.h inplace of handcrafted code
ClosedPublic
Actions

Authored by rgrimes on Feb 24 2019, 5:36 AM.

Details

Reviewers

cem
karels
kp

Group Reviewers

network
transport

Commits

rS346710: MFC: r345888: Use IN_foo() macros from sys/netinet/in.h inplace of
rS345888: Use IN_foo() macros from sys/netinet/in.h inplace of handcrafted code

Summary

There are a few places that use hand crafted versions of the macros from sys/netinet/in.h making it difficult to actually alter the values in use by these macros.

Correct that by replacing handcrafted code with proper macro usage.

Test Plan

An external document shall be referenced in the future describing the testing being done.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rgrimes created this revision.Feb 24 2019, 5:36 AM

Herald added subscribers: ae, imp. · View Herald TranscriptFeb 24 2019, 5:36 AM

rgrimes added a parent revision: D19316: IPV4 Experimental address space usage and cleanup of code not using IN_foo.Feb 24 2019, 5:37 AM

logan_cyberstorm.mu added a subscriber: logan_cyberstorm.mu.Feb 24 2019, 6:19 AM

Add some more places that hand crafted macros are used, ip_input, ip_output, netdump code.

Herald added a subscriber: farrokhi. · View Herald TranscriptFeb 24 2019, 7:38 AM

rgrimes edited reviewers, added: karels; removed: bz.Feb 24 2019, 5:30 PM

rgrimes added a reviewer: kp.Feb 24 2019, 5:51 PM

karels added inline comments.Feb 24 2019, 5:52 PM

sys/netpfil/ipfw/nat64/nat64_translate.h
117 ↗	(On Diff #54282)	Do you intend to commit this comment, or to follow up on it? Sure looks like everything is in network byte order, might as well just fix it.

The pf bit is good.
Everything else looks good to me as well, modulo the remark karels had about the XXX comment.

rgrimes mentioned this in D19316: IPV4 Experimental address space usage and cleanup of code not using IN_foo.Feb 27 2019, 2:27 PM

Convert nat64_check_ip4 in sys/netpfil/ipfw/nat64/nat64_translate.h to use IN_foo macros rather than hand rolled code.

rgrimes marked an inline comment as done.Feb 28 2019, 2:51 PM

rgrimes added inline comments.

sys/netinet/in.c
196 ↗	(On Diff #54282)	Since we already block IN_LOOPBACK in ip_input and ip_output it seems to me that we could remove IN_LOOPBACK from here and save a few cycles in the forwarding fast path.
sys/netpfil/ipfw/nat64/nat64_translate.h
117 ↗	(On Diff #54282)	I intend to fix it, this is WIP. But I also need some feedback before I go make a 1000 line diff, I have just done the critical stuff in the kernel that makes it even possible to transport packets when you alter the macro's values.

Can I get some final accept on this, I updated the open issues

Although I have a couple of inline comments, I wouldn't object to committing this as-is. Touching up would be fine too.

sys/netinet/in.c
196 ↗	(On Diff #54282)	I could go either way on that. I don't mind having the test here, so it doesn't depend on the details elsewhere.
sys/netinet/ip_output.c
597 ↗	(On Diff #54538)	This comment is probably not accurate with this change. RFC1122 at least implies 127/8, as Class A is implied. It wouldn't be bad to return the previous comment, but also say that we relax the requirement to include just the standard loopback address itself. An aside, I just checked a recent Ubuntu; I can ping any address on 127/8 successfully except 127.255.255.255. At least it doesn't forward those to itself.