Page MenuHomeFreeBSD

tcp: use enum for all congestion control signals
ClosedPublic

Authored by rscheff on Feb 11 2024, 6:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 8:07 AM
Unknown Object (File)
Wed, Nov 20, 1:58 AM
Unknown Object (File)
Mon, Nov 18, 9:26 PM
Unknown Object (File)
Sun, Nov 17, 3:51 AM
Unknown Object (File)
Sat, Nov 16, 8:09 PM
Unknown Object (File)
Sat, Nov 16, 8:36 AM
Unknown Object (File)
Sat, Nov 16, 8:14 AM
Unknown Object (File)
Fri, Nov 15, 4:55 PM

Details

Summary

Facilitate easier troubleshooting by enumerating
all congestion control signals. Typecast the
enum to int, when a cc module uses private signals.
While there, ensure that the currently used
private signals are unique too. While not strictly
necessary to use individual bits, retain the
possibility of combined signals for the future.

No external change.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

glebius added inline comments.
sys/netinet/cc/cc_cdg.c
457

Did compiler ask you to add the cast? enums are ints by standard, so if compiler did, that means something is off here. If it did not, there is no reason for this cast.

rscheff added inline comments.
sys/netinet/cc/cc_cdg.c
457

Yes. The "private" signals are not part of the enum, and if a cc module wants to refer to them in a switch{}, the cast is necessary to silence that warning; cc_cdg, cc_chd and cc_vegas make use of these private signals. The alternative would be to add CC_CDG_DELAY, CC_CHD_DELAY, and CC_VEGAS_DELAY into cc.h as global enums.

I thought to leave these only in the scope of the respective cc module, and do the cast to int instead.

sys/netinet/cc/cc_cdg.c
457

I see. Unfortunately enums are not expandable. One hack that can be done is this:

#define CC_GENERIC_ENUM  CC_ACK = 1, CC_DUPACK = .......................

enum cdgsignal_t {CC_GENERIC_ENUM, CC_CDG_DELAY = ...};

Probably using int cast is simpler.

rscheff added inline comments.
sys/netinet/cc/cc_cdg.c
457

Presumably, if you create a new type, for gdb to consistenly decode these private signals, you would have to use that type in the function definition - which would collide with the cc module function block definition; so to compile, you'd typecast within the switch, and loose gdb visibility again. So probably not much gain after all...

rscheff marked an inline comment as done.
  • rebase main
This revision is now accepted and ready to land.Feb 22 2024, 3:54 PM