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

[interfaces]: Combine vlan_interfaces and lag_interfaces file and add allow-hotplug #381

Merged
merged 2 commits into from
Mar 16, 2017

Conversation

stcheng
Copy link
Contributor

@stcheng stcheng commented Mar 7, 2017

  • start interface <interface_name> when the kernel detects
    a hotplug event from the interface

ref: https://www.debian.org/doc/manuals/debian-reference/ch05.en.html

Signed-off-by: Shuotian Cheng shuche@microsoft.com

@stcheng
Copy link
Contributor Author

stcheng commented Mar 7, 2017

To address the issue: sonic-net/sonic-swss#165

@lguohan
Copy link
Collaborator

lguohan commented Mar 7, 2017

what about vlan interface>

@stcheng
Copy link
Contributor Author

stcheng commented Mar 7, 2017

vlan interfaces are different here.

for regular front panel ports, the host interfaces are created by calling the sai function calls.

for teamd instances, the host interfaces are created by the teamd daemon.

but for vlan interfaces, manual commands are needed to create them AFTER all the regular front panel ports' host interfaces are created.

we have two ways of creating vlan interfaces:

  1. use the command ifup --all --force --interfaces /etc/network/interfaces.d/vlan_interfaces to bring up all the vlan interfaces in a one-shot command. it will automatically create the bridge and add interfaces into the bridge and bring up the interface. in order to use ifup, the only requirement is the auto key word in the interfaces file.

  2. use the command brctl addbr and brctl addif commands. then we will also need to wait until all the regular host interfaces are created. this could be down by putting the commands into the start.sh file and checking the db if all the host interfaces are created. otherwise, if we add some interfaces which are not created, we will get an error indicating that the interfaces are not existing. for this case, allow-hotplug will be needed because we create the interfaces via the brctl and they are by default not admin up.

that's why allow-hotplug is not needed for vlan interfaces in this case as we are using ifup. (it doesn't hurt to add this stanza to vlan interfaces, it just doesn't take effect.

[DEPRECATED]

@lguohan
Copy link
Collaborator

lguohan commented Mar 15, 2017

let's try to add the interface to the bridge when we try to bring up the interface that belongs to vlan.

@stcheng
Copy link
Contributor Author

stcheng commented Mar 15, 2017

this pull request is updated:

[interfaces]: Combine vlan_interfaces and lag_interfaces file and add allow-hotplug

1. Remove vlan_interfaces and lag_interfaces file
2. Add all interfaces to /etc/network/interfaces file
3. Add allow-hotplug stanza
4. Add up <command> to automatically add interfaces to VLAN and LAG
5. Add unique_name filter to minigraph.py to remove duplicate interface names

Signed-off-by: Shuotian Cheng <shuche@microsoft.com>

@stcheng stcheng changed the title [files]: Add allow-hotplug stanza to interfaces files [interfaces]: Combine vlan_interfaces and lag_interfaces file and add allow-hotplug Mar 15, 2017
auto {{ interface }}
allow-hotplug {{ interface }}
iface {{ interface }} inet manual
pre-up /sbin/ifconfig {{ interface }} up
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need this here but not for team member port?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because teamdctl can only add the member when the member is down. if the interface is up, it cannot be added to the teamd instance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

then for team member, it should be pre-up?

Copy link
Collaborator

Choose a reason for hiding this comment

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

and for vlan, it can be

post-up brctl addif {{ vlan_interface['name'] }} {{ interface }}

iface {{ interface['alias'] }} {{ 'inet' if interface['addr'] | ipv4 else 'inet6' }} static
address {{ interface['addr'] }}
netmask {{ interface['mask'] }}
#
{% endfor %}
{% endblock front_panel_interface %}
{% for vlan_interface in minigraph_vlan_interfaces|unique_name %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we having duplicated vlan_interface names? we should prevent that from happening in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as below. also currently we don't have ipv6 vlan interfaces.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

#
{% endfor %}
{% endfor %}
{% for pc_interface in minigraph_portchannel_interfaces|unique_name %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we having duplicated vlan_interface names? we should prevent that from happening in the first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is to remove the duplicated interfaces entries when the same interface is having both ipv4 and ipv6 addresses. otherwise we will end up having two same member entries.

Copy link
Collaborator

@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.

as comments

@stcheng
Copy link
Contributor Author

stcheng commented Mar 15, 2017

comments answered.

@stcheng stcheng self-assigned this Mar 15, 2017
@stcheng
Copy link
Contributor Author

stcheng commented Mar 15, 2017

updated.
also add brctl to base image.

auto {{ interface }}
allow-hotplug {{ interface }}
iface {{ interface }} inet manual
pre-up teamdctl {{ pc_interface['name'] }} port add {{ interface }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if the teamd interface does not exist?

Copy link
Collaborator

Choose a reason for hiding this comment

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

what if teamd starts after swss, in that case you have ports up but no teamd ports, then teamdctl add will fail.

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 under the teamd interface section, we could add same commands so that once the teamd interface goes up, it will also try to add all its members. there is no harm adding one member twice.

Copy link
Collaborator

@marian-pritsak marian-pritsak Mar 15, 2017

Choose a reason for hiding this comment

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

teamd always tries to add members on start (in case of our configuration).

From what I see:

  1. If swss starts first, then interfaces are added to LAGs by teamd itself on startup;
  2. if teamd starts first, then pre-up command do its job;

@stcheng Do I understand it correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marian-pritsak I see what you mean. currently I just remove the members from the teamd.j2 configurations. I think i still need to add them back.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, I think we should add it back.


In reply to: 106252590 [](ancestors = 106252590)

@lguohan
Copy link
Collaborator

lguohan commented Mar 15, 2017

build failure, can you check?

Copy link
Contributor

@jleveque jleveque left a comment

Choose a reason for hiding this comment

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

Just curious about the submodule updates. They all claim 0 files have changed but they all seem to be moving back to commits that are about a month old.

ETA: This might be the source of your build failures.

- start interface <interface_name> when the kernel detects
a hotplug event from the interface

ref: https://www.debian.org/doc/manuals/debian-reference/ch05.en.html

Signed-off-by: Shuotian Cheng <shuche@microsoft.com>
@stcheng
Copy link
Contributor Author

stcheng commented Mar 15, 2017

@jleveque thanks! yes. i updated the commit so that the submodules are untouched.

@stcheng
Copy link
Contributor Author

stcheng commented Mar 15, 2017

@lguohan added sonic-swss submodule update

if intfname in pc_map:
pc_map[intfname].append(intf)
else:
pc_map[intfname] = [intf]
Copy link
Collaborator

Choose a reason for hiding this comment

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

convert into spaces

new_list = []
for item in l:
if item['name'] not in name_list:
name_list.append(item['name'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

convert into spaces

auto {{ interface }}
allow-hotplug {{ interface }}
iface {{ interface }} inet manual
pre-up teamdctl {{ pc_interface['name'] }} port add {{ interface }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

pre-up teamdctl {{ pc_interface['name'] }} port add {{ interface }} [](start = 4, length = 67)

since this could fail, we should add "|| true" in the end.

… allow-hotplug

1. Remove vlan_interfaces and lag_interfaces file and members in teamd.j2
2. Add all interfaces to /etc/network/interfaces file
3. Add allow-hotplug stanza
4. Add up <command> to automatically add interfaces to VLAN and LAG
5. Add unique_name filter to minigraph.py to remove duplicate interface names
6. Add brctl to base image
7. Update sonic-swss submodule

Signed-off-by: Shuotian Cheng <shuche@microsoft.com>
@stcheng
Copy link
Contributor Author

stcheng commented Mar 16, 2017

Updated 🆙

@stcheng stcheng merged commit 05e6b36 into sonic-net:master Mar 16, 2017
@stcheng stcheng deleted the stanza branch March 16, 2017 18:22
lguohan added a commit to lguohan/sonic-buildimage that referenced this pull request Nov 18, 2018
sonic-sairedis

* 398d24a 2018-11-16 |  Add comments aspell check (sonic-net#382) (master) [Kamil Cudnik]
* 08fccb0 2018-11-15 | Add buffer pool logic and unittests (sonic-net#381) [Kamil Cudnik]
* 4d5a7bb 2018-11-15 | Rebuild VS local metadata on warm boot (sonic-net#380) [Kamil Cudnik]
* b011f3d 2018-11-15 | Add cold discovered oids (sonic-net#379) [Kamil Cudnik]
* 11c386e 2018-11-14 | [syncd] always log ASIC operations performed after warm restart (sonic-net#378) [Ying Xie]
* ebb9b7a 2018-11-14 | [syncd] provide docker name when checking warm restart (sonic-net#377) [Ying Xie]
* 72d8cc3 2018-11-12 | Add warm boot support for virtual switch (sonic-net#374) [Kamil Cudnik]
* 333c5ff 2018-11-11 | Refactor sairedis (sonic-net#375) [Kamil Cudnik]

sonic-utilities

* c6b4fe7 2018-11-15 | Adding bgp's warmrestart timer and on-off knob (sonic-net#378) (HEAD, origin/master, origin/HEAD, master) [Rodny Molina]
* 45d85c9 2018-11-15 | [warm boot] move warm reboot/fast reboot knowledge to syncd service script (sonic-net#372) [Ying Xie]

sonic-swss
* 067928d 2018-11-17 | [aclorch]: Use the Observer class to listen to port changes (sonic-net#689) (HEAD, origin/master, origin/HEAD) [Shuotian Cheng]
* d7d887a 2018-11-15 | [test]: Remove deprecated TODO comment (sonic-net#688) (master) [Shuotian Cheng]
* b42710a 2018-11-15 | [teammgrd]: Check if port is enslaved on all conditions (sonic-net#687) [Shuotian Cheng]
* 75972e7 2018-11-13 | Restore port oper status form appDB port table during restore phase o… (sonic-net#683) [Jipan Yang]
* 48c3bfb 2018-11-13 | [WarmStart]: Use right docker name for the teammgrd checkWarmStart (sonic-net#685) [pavel-shirshov]
* 6292786 2018-11-13 | [teammgrd]: Add kernel master check for port before enslaving (sonic-net#681) [Shuotian Cheng]
* a42ef4c 2018-11-12 | Adjust schema to match bgp warm-restart timer interval (sonic-net#682) [Rodny Molina]
* dc2ff77 2018-11-12 | [portsorch]: Move status comparison outside updatePortOperStatus function (sonic-net#679) [Shuotian Cheng]

Signed-off-by: Guohan Lu <gulv@microsoft.com>
lguohan added a commit that referenced this pull request Nov 18, 2018
sonic-sairedis

* 398d24a 2018-11-16 |  Add comments aspell check (#382) (master) [Kamil Cudnik]
* 08fccb0 2018-11-15 | Add buffer pool logic and unittests (#381) [Kamil Cudnik]
* 4d5a7bb 2018-11-15 | Rebuild VS local metadata on warm boot (#380) [Kamil Cudnik]
* b011f3d 2018-11-15 | Add cold discovered oids (#379) [Kamil Cudnik]
* 11c386e 2018-11-14 | [syncd] always log ASIC operations performed after warm restart (#378) [Ying Xie]
* ebb9b7a 2018-11-14 | [syncd] provide docker name when checking warm restart (#377) [Ying Xie]
* 72d8cc3 2018-11-12 | Add warm boot support for virtual switch (#374) [Kamil Cudnik]
* 333c5ff 2018-11-11 | Refactor sairedis (#375) [Kamil Cudnik]

sonic-utilities

* c6b4fe7 2018-11-15 | Adding bgp's warmrestart timer and on-off knob (#378) (HEAD, origin/master, origin/HEAD, master) [Rodny Molina]
* 45d85c9 2018-11-15 | [warm boot] move warm reboot/fast reboot knowledge to syncd service script (#372) [Ying Xie]

sonic-swss
* 067928d 2018-11-17 | [aclorch]: Use the Observer class to listen to port changes (#689) (HEAD, origin/master, origin/HEAD) [Shuotian Cheng]
* d7d887a 2018-11-15 | [test]: Remove deprecated TODO comment (#688) (master) [Shuotian Cheng]
* b42710a 2018-11-15 | [teammgrd]: Check if port is enslaved on all conditions (#687) [Shuotian Cheng]
* 75972e7 2018-11-13 | Restore port oper status form appDB port table during restore phase o… (#683) [Jipan Yang]
* 48c3bfb 2018-11-13 | [WarmStart]: Use right docker name for the teammgrd checkWarmStart (#685) [pavel-shirshov]
* 6292786 2018-11-13 | [teammgrd]: Add kernel master check for port before enslaving (#681) [Shuotian Cheng]
* a42ef4c 2018-11-12 | Adjust schema to match bgp warm-restart timer interval (#682) [Rodny Molina]
* dc2ff77 2018-11-12 | [portsorch]: Move status comparison outside updatePortOperStatus function (#679) [Shuotian Cheng]

Signed-off-by: Guohan Lu <gulv@microsoft.com>
madhanmellanox pushed a commit to madhanmellanox/sonic-buildimage that referenced this pull request Mar 23, 2020
dmytroxshevchuk pushed a commit to dmytroxshevchuk/sonic-buildimage that referenced this pull request Aug 31, 2020
* Add cold discovered oids

* Add cold discovered oids

* Add buffer pool logic and unittests

* Bring back logging
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jun 20, 2023
Update sonic-platform-common submodule pointer to include the following:
* 4d270f0 [actions] Support Semgrep by Github Actions ([sonic-net#383](sonic-net/sonic-platform-common#383))
* 74ddd4e Fix issue: should use 'Value' column to calculate the health percentage ([sonic-net#381](sonic-net/sonic-platform-common#381))

Signed-off-by: dprital <drorp@nvidia.com>
mssonicbld added a commit that referenced this pull request Jun 22, 2023
… automatically (#15577)

#### Why I did it
src/sonic-platform-common
```
* 459ffaa - (HEAD -> 202211, origin/202211) Fix issue: should use 'Value' column to calculate the health percentage (#381) (3 hours ago) [Junchao-Mellanox]
```
#### How I did it
#### How to verify it
#### Description for the changelog
mssonicbld added a commit that referenced this pull request Jul 13, 2023
…D automatically (#15810)

#### Why I did it
src/sonic-platform-daemons
```
* d73808c - (HEAD -> master, origin/master, origin/HEAD) Added PCIe transaction check for all peripherals on the bus (#331) (9 hours ago) [Ashwin Srinivasan]
* 432602a - Update active application selected code in transceiver_info table aft… (#381) (13 hours ago) [Michael Wang - TW]
```
#### How I did it
#### How to verify it
#### Description for the changelog
mssonicbld added a commit that referenced this pull request Aug 7, 2023
…D automatically (#16062)

#### Why I did it
src/sonic-platform-daemons
```
* 6c47906 - (HEAD -> 202305, origin/202305) Update active application selected code in transceiver_info table aft… (#381) (13 hours ago) [Michael Wang - TW]
```
#### How I did it
#### How to verify it
#### Description for the changelog
yxieca pushed a commit that referenced this pull request Aug 11, 2023
…D automatically (#16108)

src/sonic-platform-daemons

* f5a0ffc - (HEAD -> 202205, origin/202205) Update active application selected code in transceiver_info table aft… (#381) (4 hours ago) [Michael Wang - TW]
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
…D automatically (sonic-net#15810)

#### Why I did it
src/sonic-platform-daemons
```
* d73808c - (HEAD -> master, origin/master, origin/HEAD) Added PCIe transaction check for all peripherals on the bus (sonic-net#331) (9 hours ago) [Ashwin Srinivasan]
* 432602a - Update active application selected code in transceiver_info table aft… (sonic-net#381) (13 hours ago) [Michael Wang - TW]
```
#### How I did it
#### How to verify it
#### Description for the changelog
mssonicbld added a commit that referenced this pull request Mar 7, 2024
…tomatically (#18290)

#### Why I did it
src/sonic-linux-kernel
```
* 45295bf - (HEAD -> master, origin/master, origin/HEAD) Fix the issue with signed kernel compilation for ARM64 architecture (#381) (54 minutes ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
sonic-otn pushed a commit to Weitang-Zheng/sonic-buildimage that referenced this pull request Mar 11, 2024
…tomatically (sonic-net#18290)

#### Why I did it
src/sonic-linux-kernel
```
* 45295bf - (HEAD -> master, origin/master, origin/HEAD) Fix the issue with signed kernel compilation for ARM64 architecture (sonic-net#381) (54 minutes ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
saksarav-nokia pushed a commit to saksarav-nokia/sonic-buildimage that referenced this pull request Mar 12, 2024
…tomatically (sonic-net#18290)

#### Why I did it
src/sonic-linux-kernel
```
* 45295bf - (HEAD -> master, origin/master, origin/HEAD) Fix the issue with signed kernel compilation for ARM64 architecture (sonic-net#381) (54 minutes ago) [Oleksandr Ivantsiv]
```
#### How I did it
#### How to verify it
#### Description for the changelog
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