Page MenuHomeFreeBSD

Clean up unused bandwidth entry in the TCP hostcache
ClosedPublic

Authored by j-nitrology.com on Nov 13 2015, 9:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 15, 4:02 PM
Unknown Object (File)
Fri, Mar 15, 4:01 PM
Unknown Object (File)
Fri, Mar 8, 12:40 AM
Unknown Object (File)
Feb 23 2024, 3:22 AM
Unknown Object (File)
Dec 23 2023, 8:52 PM
Unknown Object (File)
Dec 22 2023, 10:29 PM
Unknown Object (File)
Nov 13 2023, 1:40 PM
Unknown Object (File)
Nov 7 2023, 2:05 PM

Details

Summary

The bandwidth calculation that was saved into the hostcache was removed by r211315, and r212765 marked it unused in tcp_info as it was found conflicting with the new modular congestion control. Clean up the remnants of this in the hostcache.

Test Plan

This has been tested on production boxes within Limelight.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1130
Build 1134: arc lint + arc unit

Event Timeline

j-nitrology.com retitled this revision from to Clean up unused bandwidth entry in the TCP hostcache.
j-nitrology.com updated this object.
j-nitrology.com edited the test plan for this revision. (Show Details)
j-nitrology.com added a reviewer: hiren.
hiren edited edge metadata.

Looks good to me.

Only thing I am not sure about is if we want to keep struct size unchanged and also keep a spare field where we removed the bandwidth field in struct hc_metrics and hc_metrics_lite. On a second thought, I can't think of anything that I'd want to add to hostcache at this point so might as well remove them. And as I understand, MFC is not planned for this.

It'd be great if someone else can also review the change.

Thank you, Jason.

This revision is now accepted and ready to land.Nov 13 2015, 10:09 PM
hiren edited subscribers, added: network, transport; removed: imp.
rrs added a reviewer: rrs.
rrs added a subscriber: rrs.

Looks good to me :-)

This revision was automatically updated to reflect the committed changes.

dammit we have a functional bandwidth estimator that used this field..

If it could be useful to others, might you commit it?

This was committed over 2 years ago. So that sounds like a pretty local mod. I would recommend either carrying it in your local tree or otherwise upstreaming the estimator.

This comment was removed by bz.
In D4154#295111, @kevin.bowling_kev009.com wrote:

This was committed over 2 years ago. So that sounds like a pretty local mod. I would recommend either carrying it in your local tree or otherwise upstreaming the estimator.

that's what we are doing (just adding the fields back locally..
The estimator is, unfortunately, a bit crude for public consumption, or I'd commit it.

Contact me privately if you (anyone) needs a per-peer bandwidth estimation feature.