api

api copied to clipboard

Ref: https://github.com/istio/istio/issues/38982

😊 Welcome @luksa! This is either your first contribution to the Istio api repo, or it's been awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

Hi @luksa. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

The reason I took the per-pod approach is because I am not sure how change affects performance. The early demux feature was supposed to be an optimization, but in some cases leads to a reduction of throughput (see https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/), which is why the sysctl option was then introduced.

I feel like users will want to use the disableIPEarlyDemux annotation only on pods that expose more than one port. I don't feel I can get a definitive answer on whether making this change globally would be okay, hence the per-pod option, just to be safe.

IMHO, this option should only be a (temporary) workaround, as I still think this is a Kernel bug that should be fixed (optimizations shouldn't break stuff).

One other question I had - there is also tcp_ and udp_ variants. Does setting only the TCP work for us?

FWIW I sent a message to the google kernel networking team to get advice, haven't heard back yet

Any movement on this? It does sound per pod is reasonable given the reasonings @luksa outlined - this is only needed for pods with 1+ ports

/ok-to-test

@luksa: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
release-notes_api	7a6042a0a8b28cc60b42bc5875dbf39e57210a4a	link	false	/test release-notes
gencheck_api	7a6042a0a8b28cc60b42bc5875dbf39e57210a4a	link	true	/test gencheck

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.