Paths

Table of Contentst

-
sys/netinet/
-
netinet/
-
tcp_stacks/
2/3
bbr.c
2/3
rack.c
2/3
tcp_subr.c
2/3
tcp_var.h

tcp: mark TCP stacks which can serve as a default stack
ClosedPublic
Actions

Authored by tuexen on May 31 2024, 9:50 AM.

Details

Reviewers

gallatin
rrs
rscheff
peter.lei_ieee.org
lstewart

Group Reviewers

transport

Commits

rG9ea13f8547a9: tcp: mark TCP stacks which can serve as a default stack
rGe0b080f850cc: tcp: mark TCP stacks which can serve as a default stack

Summary

Allow a TCP function block (tfb) to become the default stack only if tfb->tfb_flags has the TCP_FUNC_DEFAULT_OK flags set. This allows a TCP function block, that is not suitable as a default function block to ensure that it is not set as the default via sysctl. In this case sysctl would return EINVAL.

One could do the same thing using a flag TCP_FUNC_DEFAULT_NOT_OK. Then this change would be smaller, but doing it the way described above seems safer. But if you prefer this approach, we can use it. I'm open to that.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

tuexen created this revision.May 31 2024, 9:50 AM

Herald added a reviewer: transport. · View Herald TranscriptMay 31 2024, 9:50 AM

Herald added subscribers: glebius, melifaro, imp. · View Herald Transcript

tuexen requested review of this revision.May 31 2024, 9:50 AM

tuexen retitled this revision from tcp: allow a TCP stack to to tcp: allow a TCP stack to not become a default stack.May 31 2024, 11:13 AM

tuexen retitled this revision from tcp: allow a TCP stack to not become a default stack to tcp: mark TCP stacks which can serve as a default stack.May 31 2024, 11:29 AM

Can you give a brief explanation of the problem this is supposed to solve?

As far as the "sense" of the option, can you explain why the default presumption is that stacks can not be used at the default stack?

gallatin accepted this revision.May 31 2024, 4:02 PM

In D45419#1036179, @jtl wrote:

Can you give a brief explanation of the problem this is supposed to solve?

Sure. Assume there is a TCP function block, which only deals with TCP endpoints in the ESTABLISHED state.
If you use such a TCP function block as the default stack, you can not create an TCP endpoint anymore,
since the tfb_tcp_fb_init function call will fail. All TCP endpoints are created in the CLOSED state.
Such a stack would not have the TCP_FUNC_DEFAULT_OK flag be set and this patch would protect
the system to get in such a bad state, where no TCP endpoints could be created anymore.

As far as the "sense" of the option, can you explain why the default presumption is that stacks can not be used at the default stack?

We can mark the stacks, which can be used as a default stack or mark the stacks which cannot be used as a default stack.
I thought it is safer that the developer has to say it is OK to use this stack as a default. If you forget it, nothing bad will happen, you just have limited functionality. If you forget to say that it is not OK, you run into the bad state.

I don't have a strong opinion on the default flag state (DEFAULT_OK or DEFAULT_NOT_OK), so this seems fine to me.

tuexen accepted this revision as: transport.Jul 21 2024, 11:50 PM

This revision is now accepted and ready to land.Jul 21 2024, 11:50 PM

This has been committed as e0b080f850cc.

tuexen closed this revision.Jul 22 2024, 5:21 PM

tuexen added a commit: rGe0b080f850cc: tcp: mark TCP stacks which can serve as a default stack.Jul 24 2024, 10:57 PM

tuexen added a commit: rG9ea13f8547a9: tcp: mark TCP stacks which can serve as a default stack.Aug 8 2024, 6:23 PM

Revision Contents
Changeset List

Path

Size

sys/

netinet/

tcp_stacks/

2 lines

2 lines

5 lines

1 line

Diff 139253

View Options

sys/netinet/tcp_stacks/bbr.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/netinet/tcp_stacks/rack.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/netinet/tcp_subr.c

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

View Options

sys/netinet/tcp_var.h

	.\" $FreeBSD$			.\" $FreeBSD$
	Set to not build and install the enhanced			Install the traditional FreeBSD
	.Xr bc 1			.Xr bc 1
				jhbUnsubmitted Not Done Inline Actions I would maybe avoid making other changes for this commit than just the "Set to" part. Losing the Xr sections for bc and dc seems rather odd. Granted, I would rewrite this description to be something like "Use the traditional FreeBSD versions of bc(1) and dc(1) rather than the <foo> versions." where <foo> is more descriptive than just "enhanced". jhb: I would maybe avoid making other changes for this commit than just the "Set to" part. Losing…
				emasteAuthorUnsubmitted Done Inline Actions Huh, I think this must have been an accident during a rebase, I did not intend to drop these. When I first did this it was a bit of sed with manual review/touch-ups. In can incorporate feedback before this gets committed and/or we can make further improvements like this one after commit. I am also not sure what <foo> would be off hand. emaste: Huh, I think this must have been an accident during a rebase, I did not intend to drop these.
	and			and
	.Xr dc 1			.Xr dc 1
	programs instead of the traditional FreeBSD versions.			programs instead of the enahnced versions.
				emasteAuthorUnsubmitted Done Inline Actions and of course I have a typo here. corrected locally. emaste: and of course I have a typo here. corrected locally.

tcp: mark TCP stacks which can serve as a default stackClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 139253

sys/netinet/tcp_stacks/bbr.c

sys/netinet/tcp_stacks/rack.c

sys/netinet/tcp_subr.c

sys/netinet/tcp_var.h

tcp: mark TCP stacks which can serve as a default stack
ClosedPublic
Actions

Revision Contents
Changeset List