-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
To address the issue: sonic-net/sonic-swss#165 |
what about vlan interface> |
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:
that's why allow-hotplug is not needed for vlan interfaces in this case as we are using [DEPRECATED] |
let's try to add the interface to the bridge when we try to bring up the interface that belongs to vlan. |
this pull request is updated:
|
auto {{ interface }} | ||
allow-hotplug {{ interface }} | ||
iface {{ interface }} inet manual | ||
pre-up /sbin/ifconfig {{ interface }} up |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 %} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 %} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as comments
comments answered. |
updated. |
auto {{ interface }} | ||
allow-hotplug {{ interface }} | ||
iface {{ interface }} inet manual | ||
pre-up teamdctl {{ pc_interface['name'] }} port add {{ interface }} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- If swss starts first, then interfaces are added to LAGs by teamd itself on startup;
- if teamd starts first, then pre-up command do its job;
@stcheng Do I understand it correctly?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build failure, can you check? |
There was a problem hiding this 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 <[email protected]>
@jleveque thanks! yes. i updated the commit so that the submodules are untouched. |
@lguohan added sonic-swss submodule update |
src/sonic-config-engine/minigraph.py
Outdated
if intfname in pc_map: | ||
pc_map[intfname].append(intf) | ||
else: | ||
pc_map[intfname] = [intf] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convert into spaces
src/sonic-config-engine/sonic-cfggen
Outdated
new_list = [] | ||
for item in l: | ||
if item['name'] not in name_list: | ||
name_list.append(item['name']) |
There was a problem hiding this comment.
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 }} |
There was a problem hiding this comment.
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 <[email protected]>
Updated 🆙 |
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 <[email protected]>
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 <[email protected]>
* Add cold discovered oids * Add cold discovered oids * Add buffer pool logic and unittests * Bring back logging
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 <[email protected]>
… 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
…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
…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
…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
…sonic-buildimage into internal Fix conflict for rsyslog. Skip partial DNS unit test in internal branch after confirmed with Gang. Related work items: sonic-net#113, sonic-net#131, sonic-net#132, sonic-net#134, sonic-net#321, sonic-net#331, sonic-net#381, sonic-net#382, sonic-net#2525, sonic-net#2676, sonic-net#2698, sonic-net#2737, sonic-net#2789, sonic-net#2839, sonic-net#2845, sonic-net#2850, sonic-net#2882, sonic-net#2885, sonic-net#2887, sonic-net#2890, sonic-net#2895, sonic-net#13338, sonic-net#14105, sonic-net#15142, sonic-net#15223, sonic-net#15456, sonic-net#15487, sonic-net#15520, sonic-net#15726, sonic-net#15727, sonic-net#15758, sonic-net#15764, sonic-net#15765, sonic-net#15772, sonic-net#15779, sonic-net#15782, sonic-net#15785, sonic-net#15797, sonic-net#15798, sonic-net#15810, sonic-net#15811, sonic-net#15821
…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
…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
…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
…onic-net#381) CONFIG_KEXEC_SIG_FORCE is not available for ARM64 architecture. CONFIG_KEXEC_SIG should be used instead. Signed-off-by: Oleksandr Ivantsiv <[email protected]> Co-authored-by: Yakiv Huryk <[email protected]>
a hotplug event from the interface
ref: https://www.debian.org/doc/manuals/debian-reference/ch05.en.html
Signed-off-by: Shuotian Cheng [email protected]