Page MenuHomeFreeBSD
Differential D8053

Properly preserve ip_tos bits for IPv4 packets
ClosedPublic
Actions

Authored by • lidl on Sep 28 2016, 3:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 6, 3:48 AM
Unknown Object (File)
Wed, Nov 5, 3:50 PM
Unknown Object (File)
Tue, Nov 4, 7:42 AM
Unknown Object (File)
Tue, Nov 4, 7:42 AM
Unknown Object (File)
Tue, Nov 4, 7:42 AM
Unknown Object (File)
Tue, Nov 4, 3:55 AM
Unknown Object (File)
Sun, Nov 2, 4:52 AM
Unknown Object (File)
Thu, Oct 30, 1:00 PM
View All Files
Subscribers
bz
gnn
imp

Details

Reviewers
melifaro
ae
cem
hiren
glebius
gnn
Group Reviewers
transport
Commits
rS306458: Properly preserve ip_tos bits for IPv4 packets
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

Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 5333
Build 5518: CI src buildJenkins

Event Timeline

• lidl updated this revision to Diff 20751.Sep 28 2016, 3:06 AM
• 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.
Herald added a reviewer: transport. · View Herald TranscriptSep 28 2016, 3:06 AM
Herald added a subscriber: imp. · View Herald Transcript
cem accepted this revision.Sep 28 2016, 3:09 AM
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 accepted this revision.Sep 28 2016, 12:04 PM
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
bz added a subscriber: bz.Sep 28 2016, 12:26 PM

Just a random comment...

sys/netinet/tcp_input.c
678

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...

gnn added a comment.Sep 28 2016, 12:41 PM

I'd prefer a broad, cleanup commit.

• lidl added a reviewer: hiren.Sep 28 2016, 1:15 PM
hiren accepted this revision.Sep 28 2016, 2:34 PM
hiren edited edge metadata.

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

Closed by commit rS306458: Properly preserve ip_tos bits for IPv4 packets (authored by • lidl). · Explain WhySep 29 2016, 7:45 PM
This revision was automatically updated to reflect the committed changes.
• lidl added inline comments.Sep 29 2016, 7:58 PM
sys/netinet/tcp_input.c
678

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

glebius added inline comments.Oct 5 2016, 11:57 PM
head/sys/netinet/tcp_input.c
726 ↗(On Diff #20824)

Can you please explain why this line changed?

cem added inline comments.Oct 6 2016, 12:36 AM
head/sys/netinet/tcp_input.c
726 ↗(On Diff #20824)

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

• lidl added inline comments.Oct 6 2016, 12:51 PM
head/sys/netinet/tcp_input.c
726 ↗(On Diff #20824)

I was avoiding doing the same calculation twice.

Revision Contents
Changeset List

PathSize
sys/
netinet/
23 lines

Diff 20751

View Options

sys/netinet/tcp_input.c

Loading...