Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supplyment multicast document with a limitation #4850

Merged

Conversation

ceclinux
Copy link
Contributor

Document the limitation of a large number of receiver groups in one node for multicast.

@ceclinux ceclinux added the kind/documentation Categorizes issue or PR as related to a documentation. label Apr 12, 2023
### Large number of receiver groups for one node

A Linux host limits the maximum number of multicast groups it can subscribe to
by the number of [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Append "The default value is 20." after this sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated


A Linux host limits the maximum number of multicast groups it can subscribe to
by the number of [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Users should change this number according to their needs, and Antrea is not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove "and Antrea is not responsible for changing it."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated


A Linux host limits the maximum number of multicast groups it can subscribe to
by the number of [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Users should change this number according to their needs, and Antrea is not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Users should change this number according to their needs, and Antrea is not
Users are responsible to change the value if more than 20 multicast groups are expected per Node.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. I think the correct wording should be "responsible for" https://www.britannica.com/dictionary/eb/qa/What-preposition-should-be-used-after-responsible-

@ceclinux ceclinux force-pushed the document-igmp_max_memberships-for-multicast branch 3 times, most recently from cb0fecf to 49395bd Compare April 13, 2023 13:16
@wenyingd wenyingd requested a review from jianjuns April 14, 2023 03:18
wenyingd
wenyingd previously approved these changes Apr 14, 2023
Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -137,6 +137,13 @@ Support for Windows and IPv6 will be added in the future.
Configuration option `multicastInterfaces` is not supported with encap mode.
Multicast packets in encap mode are SNATed and forwarded to the transport interface only.

### Large number of receiver groups for one node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Large number of receiver groups for one node
### Large number of receiver groups for one Node

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Comment on lines 142 to 143
A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to say:

Suggested change
A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
A Linux host limits the maximum number of multicast groups it can subscribe to,
the default number is 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your suggestion is grammatically incorrect. A Linux host limits the maximum number of multicast groups it can subscribe to, the default number is 20 contains two independent clauses, and they are improperly joined with a comma.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe: "A Linux host limits the maximum number of multicast groups it can subscribe to; the default number is 20. The limit can be changed by setting /proc/sys/net/ipv4/igmp_max_memberships."?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ceclinux you can follow @jianjuns 's suggestion to refine the sentence, the purpose is to make the sentence more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated. Thanks


A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Users are responsible for changing the value if Pods on this Node are expected to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Users are responsible for changing the value if Pods on this Node are expected to
Users are responsible to change the value if Pods on the Node are expected to

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for changing" is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change "the value" to "the limit".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@@ -137,6 +137,13 @@ Support for Windows and IPv6 will be added in the future.
Configuration option `multicastInterfaces` is not supported with encap mode.
Multicast packets in encap mode are SNATed and forwarded to the transport interface only.

### Large number of receiver groups for one node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Maximum number of receiver groups on one Node"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Comment on lines 142 to 143
A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe: "A Linux host limits the maximum number of multicast groups it can subscribe to; the default number is 20. The limit can be changed by setting /proc/sys/net/ipv4/igmp_max_memberships."?


A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Users are responsible for changing the value if Pods on this Node are expected to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for changing" is correct.


A Linux host limits the maximum number of multicast groups it can subscribe to
by the default number of 20 [/proc/sys/net/ipv4/igmp_max_memberships](https://sysctl-explorer.net/net/ipv4/igmp_max_memberships/).
Users are responsible for changing the value if Pods on this Node are expected to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change "the value" to "the limit".

@luolanzone luolanzone added the action/backport Indicates a PR that requires backports. label Apr 17, 2023
@ceclinux ceclinux force-pushed the document-igmp_max_memberships-for-multicast branch from 49395bd to f01566a Compare April 17, 2023 06:29
Document the limitation of a large number of receiver groups
in one node for multicast.

Signed-off-by: ceclinux <src655@gmail.com>
@ceclinux ceclinux force-pushed the document-igmp_max_memberships-for-multicast branch from f01566a to 9b825b5 Compare April 17, 2023 12:55
@wenyingd
Copy link
Contributor

/skip-all

Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. kind/documentation Categorizes issue or PR as related to a documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants