CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NIC #1681
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@murali-reddy This issue might be related to PR1660. Do you mind testing against latest master to see if you it is addressed already? |
Contributor
Author
|
@serg38 Though same code path, i ran into different issue. When i was adding new tier to VPC, eth3 was plugged into VR from vCenter. As commented in the function there is delay for ethernet device to appear in the guest instance. I could see code loops through eth0, eth1, eth2 only, because it read the '/proc/sys/net/ipv4/conf' too early. |
|
@murali-reddy Thanks for the explanation. LGTM for code review |
Contributor
|
The fix looks like the correct thing to do. |
Member
|
@murali-reddy Makes sense to get fresh listing of virtual NICs in next iteration as the first iteration might have missed the hot-plugged virtual NIC (for VPC tier being configured). Though one side effect is we will loop through all the virtual NICs, except the newly hot-plugged NIC, at least one more time. |
Contributor
|
@murali-reddy have you had a chance to address @sateesh-chodapuneedi comments? |
Contributor
|
@blueorangutan package |
|
@jburwell a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-25 |
Contributor
|
@blueorangutan test centos7 vmware55u3 |
|
@jburwell a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
Contributor
|
@murali-reddy is there a way to create or update a Marvin test case to verify this fix? |
Contributor
Author
|
@sateesh-chodapuneedi added optimization for now, but its better we eliminate the whole logic of finding device index from the VR. |
Contributor
Author
|
@jburwell function 'findRouterEthDeviceIndex' which is modified, is invoked when ever a new tier to VPC is added (virtual NIC is hot-plugged). Many VPC tests run againest the VMware environment will cover this scenario. |
Contributor
Author
|
@blueorangutan test centos7 vmware55u3 |
|
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
|
Trillian test result (tid-58)
|
Member
|
Code LGTM, 👍 +1 |
Member
|
LGTM, some test failures have been addressed here: #1692 |
Contributor
|
@murali-reddy could you please squash your commits? |
Member
|
@murali-reddy can you squash your commits |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-86 |
…dex of plugged NIC In VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given the mac address. This method is used, once a nic is plugged to determine ethernet interface. "/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right ethernet interface. However current logic read it once, and loops through the device list. Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged in the VM from vCenter.Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf, so that right device list is processed.
Contributor
Author
|
@blueorangutan package |
|
@murali-reddy a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-114 |
Contributor
Author
|
@blueorangutan test centos7 vmware55u3 |
|
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
Contributor
Author
|
@blueorangutan test centos7 vmware55u3 |
|
@murali-reddy a Trillian-Jenkins test job (centos7 mgmt + vmware55u3) has been kicked to run smoke tests |
Contributor
|
@rhtyd could you check into the status of the test run for this PR? |
Member
|
Invalid params were provided, the hypervisor to be passed was vmware-55u3. I'll fix this in blueorangutan as it currently does not check/validate the hypervisor name properly. Kicked a new job manually from Jenkins. |
|
Trillian test result (tid-232)
|
Contributor
|
@murali-reddy could you please investigate the following test case failures:
|
Contributor
|
@murali-reddy have you had a chance to investigate the test failures? |
Member
|
LGTM on tests, the failures wrt VPC are known intermittent issues. I'm not sure about the internal lb failures, I'll kick another test round since there have been several changes on the base branch. @blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-226 |
Member
|
@blueorangutan test centos7 vmware55u3 |
|
@rhtyd unsupported parameters provided. Supported mgmt server os are: |
Member
|
@blueorangutan test centos7 vmware-55u3 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
|
Trillian test result (tid-378)
|
Member
asfgit
pushed a commit
that referenced
this pull request
Nov 23, 2016
CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NICIn VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given the mac address. This method is used, once a nic is plugged to determine ethernet interface. "/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right ethernet interface. Howver current logic read it once, and loops through the device list. Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged in the VM from vCenter. Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf , so that right device list is processed. Manual tested VPC scenarios of adding new tiers which uses findRouterEthDeviceIndex, to find the guest/public network ethernet index. * pr/1681: CLOUDSTACK-9491: incorrect parsing of device list to find ethernet index of plugged NIC Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In VmwareResource, findRouterEthDeviceIndex() method find ethernet interface index given
the mac address. This method is used, once a nic is plugged to determine ethernet interface.
"/proc/sys/net/ipv4/conf" from the VR and looped through the devices to find the right
ethernet interface. Howver current logic read it once, and loops through the device list.
Its observerd device may not show up '/proc/sys/net/ipv4/conf' immediatly once NIC is plugged
in the VM from vCenter.
Fix ensured, while waiting for 15 sec in the loop, read the latest content from /proc/sys/net/ipv4/conf
, so that right device list is processed.
Manual tested VPC scenarios of adding new tiers which uses findRouterEthDeviceIndex, to find the guest/public network ethernet index.