Page MenuHomeFreeBSD

Use IN_foo() macros from sys/netinet/in.h inplace of handcrafted code
ClosedPublic

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

rgrimes created this revision.Feb 24 2019, 5:36 AM
rgrimes updated this revision to Diff 54282.Feb 24 2019, 7:38 AM

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

rgrimes edited reviewers, added: karels; removed: bz.Feb 24 2019, 5:30 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 updated this revision to Diff 54538.Feb 28 2019, 2:50 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.

rgrimes marked an inline comment as done.Mar 19 2019, 3:01 AM

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

karels accepted this revision.Mar 19 2019, 6:34 AM

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.

This revision is now accepted and ready to land.Mar 19 2019, 6:34 AM
kristof accepted this revision as: kristof.Mar 19 2019, 7:34 AM
This revision was automatically updated to reflect the committed changes.