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

[sonic-buildimage] Changes to make network specific sysctl common for both host and docker namespace #4838

Merged
merged 3 commits into from
Jul 1, 2020

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Jun 23, 2020

- Why I did it
Changes to make network specific sysctl common for both host and docker namespace (in multi-asic).
This change is triggered with issue found in multi-npu platforms
where in docker namespace
net.ipv6.conf.all.forwarding was 0 (should be 1) because of
which IPv6 RS/RA message were triggered and routes on link-local address were learnt.

Beside this there were some other sysctl.net.ipv6* params whose value
in docker namespace is not same as host namespace.

- How I did it
So to make we are always in sync in host and docker namespace
created common file that list all sysctl.net.* params and used
both by host and docker namespace. Any change will get applied
to both namespace.

The new files is used only in docker namespace using command "sysctl --system -e"
For Host namespace current mechanism of set -f will be used via reading this file.

- How to verify it
Verified both Single and Multi-Asic sysctl.net.* params are applied correctly.
Also IPv6 RS/RA message are not triggered and routes were learnt on global address.

common for both host and docker namespace (in multi-npu).

This change is triggered with issue found in multi-npu platforms
where in docker namespace
net.ipv6.conf.all.forwarding was 0 (should be 1) because of
which RS/RA message were triggered and link-local router were learnt.

Beside this there were some other sysctl.net.ipv6* params whose value
in docker namespace is not same as host namespace.

So to make we are always in sync in host and docker namespace
created common file that list all sysctl.net.* params and used
both by host and docker namespace. Any change will get applied
to both namespace.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@rlhui
Copy link
Contributor

rlhui commented Jun 23, 2020

Thanks. Could we please use multi-asic term consistently and not use multi-npu term any more (so that code is also consistent)?

@rlhui this PR does not reference this term in code. Updated in PR description from multi-npu to multi-asic

@abdosi
Copy link
Contributor Author

abdosi commented Jun 25, 2020

retest vsimage please

@abdosi
Copy link
Contributor Author

abdosi commented Jun 26, 2020

@prsunny @arlakshm @lguohan
Can you please review this.

@prsunny prsunny requested a review from yxieca June 26, 2020 18:23
build_debian.sh Outdated Show resolved Hide resolved
only one and do string concatenation of all set commands
@abdosi
Copy link
Contributor Author

abdosi commented Jun 29, 2020

@yxieca @lguohan
can you please help to review this

@abdosi
Copy link
Contributor Author

abdosi commented Jun 30, 2020

retest vsimage please

prsunny
prsunny previously approved these changes Jun 30, 2020
Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

lgtm, wait for other reviewers.

yxieca
yxieca previously approved these changes Jun 30, 2020
@abdosi
Copy link
Contributor Author

abdosi commented Jul 1, 2020

@jleveque

can you please review/approve this.

@abdosi abdosi dismissed stale reviews from yxieca and prsunny via 329606d July 1, 2020 17:29
@abdosi abdosi merged commit e66cb47 into sonic-net:master Jul 1, 2020
@abdosi abdosi deleted the sysctl_net branch July 1, 2020 22:58
abdosi added a commit that referenced this pull request Jul 5, 2020
… both host and docker namespace (#4838)

* [sonic-buildimage] Changes to make network specific sysctl
common for both host and docker namespace (in multi-npu).

This change is triggered with issue found in multi-npu platforms
where in docker namespace
net.ipv6.conf.all.forwarding was 0 (should be 1) because of
which RS/RA message were triggered and link-local router were learnt.

Beside this there were some other sysctl.net.ipv6* params whose value
in docker namespace is not same as host namespace.

So to make we are always in sync in host and docker namespace
created common file that list all sysctl.net.* params and used
both by host and docker namespace. Any change will get applied
to both namespace.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

* Address Review Comments and made sure to invoke augtool
only one and do string concatenation of all set commands

* Address Review Comments.
qiluo-msft pushed a commit to qiluo-msft/sonic-buildimage that referenced this pull request Jul 12, 2020
… both host and docker namespace (sonic-net#4838)

* [sonic-buildimage] Changes to make network specific sysctl
common for both host and docker namespace (in multi-npu).

This change is triggered with issue found in multi-npu platforms
where in docker namespace
net.ipv6.conf.all.forwarding was 0 (should be 1) because of
which RS/RA message were triggered and link-local router were learnt.

Beside this there were some other sysctl.net.ipv6* params whose value
in docker namespace is not same as host namespace.

So to make we are always in sync in host and docker namespace
created common file that list all sysctl.net.* params and used
both by host and docker namespace. Any change will get applied
to both namespace.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

* Address Review Comments and made sure to invoke augtool
only one and do string concatenation of all set commands

* Address Review Comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants