[portsyncd]: Removing manual commands to bring up interfaces #174
Conversation
|
there is one more in portsyncd for vlan interface, can we remove that ifup as well? string cmd = "/sbin/ifup --all --force --interfaces " + file; |
|
|
||
| return; | ||
| m_portTableProducer.set(key, fvVector); |
There was a problem hiding this comment.
can you use {} for the else region?
| /* Bring up the front panel port as the first place*/ | ||
| /* TODO: handle system retur code, if-block is just a warning suppressor */ | ||
| if (system(("/sbin/ifup --force " + key).c_str())) | ||
| ; | ||
| g_portSet.erase(key); |
There was a problem hiding this comment.
g_portSet [](start = 12, length = 9)
the description in line 190 is not complete. Check if the port is in the PORT_TABLE. if the port is in the PORT_TABLE, then what? If not, then what?
I do not understand line 206, if a new link is detected and is not in g_portSet, then you set m_portTableProducer, why? what's the use case here?
There was a problem hiding this comment.
the g_portSet is used to track all the ports that are needed for creating the host interfaces. If the host interfaces are created, in other words the nlmsg is recieved, the port is removed from the g_portSet. After all ports are removed from the g_portSet, all host interfaces are created and thus is the time to create the VLAN interfaces via kernel. Otherwise, the VLAN members cannot be added into the VLAN interface.
I remove the 'else' condition, so that after the host interface is created, the nlmsg will be sent to the db via the set function. This is the place that the port admin status is set.
| @@ -40,7 +40,7 @@ LinkSync::LinkSync(DBConnector *db) : | |||
| m_vlanTableConsumer(db, APP_VLAN_TABLE_NAME), | |||
| m_lagTableConsumer(db, APP_LAG_TABLE_NAME) | |||
| { | |||
| /* See the comments for g_portSet in linksync.h */ | |||
| /* See the comments for g_portSet in portsyncd.cpp */ | |||
There was a problem hiding this comment.
in the portsyncd.cpp, it is m_portSet, is it g_portSet?
There was a problem hiding this comment.
thanks, that's a typo.
| @@ -200,17 +197,12 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) | |||
| return; | |||
| } | |||
|
|
|||
| /* Host interface is created */ | |||
| if (!g_init && g_portSet.find(key) != g_portSet.end()) | |||
There was a problem hiding this comment.
g_init is always false, where did you set it to true?
There was a problem hiding this comment.
portsyncd.cpp:122: g_init = true;
| /* Bring up the front panel port as the first place*/ | ||
| /* TODO: handle system retur code, if-block is just a warning suppressor */ | ||
| if (system(("/sbin/ifup --force " + key).c_str())) | ||
| ; | ||
| g_portSet.erase(key); |
There was a problem hiding this comment.
in line 53, you also do g_portSet.erase(), why two places?
There was a problem hiding this comment.
line 53 indicates that the admin status is already in the db, which indicates that the host interface is already created. the g_portSet is only used to track the ports that don't have host interfaces created yet.
this part of the code is to handle portsyncd restart issues so that after portsyncd restarts, it could still finish all the uninitialized ports/VLANs.
|
🕐 |
|
updated some comments |
- allow-hotplug stanza is added to interfaces file - vlan_interfaces and lag_interfaces files are removed Signed-off-by: Shuotian Cheng <[email protected]>
|
sonic-net/sonic-buildimage#381 is needed for this change. |
* Add portconfig utility Signed-off-by: Andriy Moroz <[email protected]> * Fix units conversion in scripts/intfutil Signed-off-by: Andriy Moroz <[email protected]> * Remove old script Signed-off-by: Andriy Moroz <[email protected]>
… API's read size for read_eeprom (sonic-net#174) This PR adds stub functions definition with details as to how firmware upgrade should be implemented by a Y cable vendor. It also fixes the read_eeprom read size for vendor part number and vendor name retreival API's Description Added a stub functions definition with a doc string describing how it should be implemented Added these API's stub functions for vendors to implement def download_firmware(physical_port, fwfile) def activate_firmware(physical_port) def rollback_firmware(physical_port) Motivation and Context Firmware upgrade is a functionality that vendors need to implement, this PR adds the definition's and a description of how the implementation of this firmware upgrade procedure API's should be implemented by breaking down the firmware upgrade into above specified sub routines. Signed-off-by: vaibhav-dahiya <[email protected]>
Signed-off-by: Shuotian Cheng [email protected]
#165