Page MenuHomeFreeBSD

Properly preserve ip_tos bits for IPv4 packets
ClosedPublic

Authored by lidl on Sep 28 2016, 3:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 9, 2:12 AM
Unknown Object (File)
Tue, Apr 30, 9:40 AM
Unknown Object (File)
Tue, Apr 30, 9:40 AM
Unknown Object (File)
Tue, Apr 30, 9:40 AM
Unknown Object (File)
Tue, Apr 30, 6:17 AM
Unknown Object (File)
Mon, Apr 29, 12:13 PM
Unknown Object (File)
Mon, Apr 29, 8:55 AM
Unknown Object (File)
Fri, Apr 26, 4:40 AM
Subscribers

Details

Summary

Restructure code slightly to save ip_tos bits earlier.
Fix the bug where the ip_tos field is zeroed out before
assigning to the iptos variable. Restore the ip_tos
and ip_ver fields only if they have been zeroed during
the pseudo-header checksum calculation.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lidl retitled this revision from to Properly preserve ip_tos bits for IPv4 packets.
lidl updated this object.
lidl edited the test plan for this revision. (Show Details)
lidl added reviewers: ae, melifaro, glebius, cem.
lidl set the repository for this revision to rS FreeBSD src repository - subversion.
cem edited edge metadata.

But take my approval with a huge grain of salt. I don't have any experience in our TCP stack.

gnn added a reviewer: gnn.
gnn added a subscriber: gnn.

This looks correct to me.

This revision is now accepted and ready to land.Sep 28 2016, 12:04 PM

Just a random comment...

sys/netinet/tcp_input.c
678 ↗(On Diff #20751)

I know we have these splattered all over the tree about 15 times; I keep wondering if I want a macro with a sensible name such as IPV6_GET_TC() or something so that people looking at the code will understand that we get the traffic class there... Maybe in a separate commit to fix them all...

I'd prefer a broad, cleanup commit.

hiren edited edge metadata.

And I agree that it'd be nice if cleanup comes as a separate commit.

This revision was automatically updated to reflect the committed changes.
sys/netinet/tcp_input.c
678 ↗(On Diff #20751)

For what it's worth, I count a total of 8 instances of this
construct in the kernel sources.

head/sys/netinet/tcp_input.c
726

Can you please explain why this line changed?

head/sys/netinet/tcp_input.c
726

Look up 5 lines. len == tlen + off0.

head/sys/netinet/tcp_input.c
726

I was avoiding doing the same calculation twice.