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

[intfmgr]: Support configuring IPs for all regular interfaces #544

Merged
merged 1 commit into from
Aug 13, 2018

Conversation

stcheng
Copy link
Contributor

@stcheng stcheng commented Jul 24, 2018

Enable configuring IPv4 and IPv6 addresses on regular ports
Add tests to test IPv4 add/remove to regular ports

Signed-off-by: Shu0T1an ChenG shuche@microsoft.com

NOTE:

  1. LAG IP configuration test will be added later once the support of LAG creation/removal feature is added.
  2. IPv6 tests will be added later once the docker vs supports IPv6 after some docker configuration changes.

@stcheng stcheng changed the title [intfmgr]: Support configuring all interfaces including regular ports IPs [intfmgr]: Support configuring IPs for all regular interfaces Jul 24, 2018
Copy link
Contributor

@jipanyang jipanyang left a comment

Choose a reason for hiding this comment

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

Unlike that for VLAN interface, interface.j2 still has address configuration for Ethernet and LAG interfaces, how to handle the possible conflict? Should only one source of configuration be available?

@stcheng
Copy link
Contributor Author

stcheng commented Jul 24, 2018

@jipanyang This commit is to address the intfmgrd so that it supports the configurations via the database. I'll then remove the /etc/network/interfaces file in the buildimage repository and add support to configure all the IPs in the configuration database at first.

@stcheng stcheng self-assigned this Jul 24, 2018
@jipanyang
Copy link
Contributor

@stcheng Thanks for the explanation, change looks good to me.

Copy link
Contributor

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

:shipit:

@lguohan
Copy link
Contributor

lguohan commented Jul 26, 2018

do not merge until to have the buildimage pr ready to merge.

}
else
{
cmd <<IP_CMD << " -6 address " << opCmd << " " << ipPrefixStr << " dev " << alias;;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Add space between << and IP_CMD
  • Remove extra trailing semicolon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " dev " << alias;;
if (ipv4)
{
cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " dev " << alias;;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra trailing semicolon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

Enable configuring IPv4 and IPv6 addresses on regular ports
Add tests to test IPv4 add/remove to regular ports

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
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.

4 participants