| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-22 | |||
| 12:47:03 | sean-k-mooney | to ensure that mac is not coming form the profile but the db | |
| 12:47:23 | ralonsoh | ahhhh sorry | |
| 12:47:32 | ralonsoh | yes, this is after deleting the port binding | |
| 12:47:52 | sean-k-mooney | after deleteing the source one yes | |
| 12:47:58 | ralonsoh | so we are reading the port randomly generated mac | |
| 12:48:03 | ralonsoh | port = self._create_unbound_port() | |
| 12:48:03 | ralonsoh | # neutron generates a MAC for each port created | |
| 12:48:03 | ralonsoh | generated_mac = port[port_def.PORT_MAC_ADDRESS] | |
| 12:48:36 | sean-k-mooney | with the current code ya but we expect it to keep the host1 pf mac | |
| 12:48:42 | sean-k-mooney | it wont now because of the bug | |
| 12:49:12 | sean-k-mooney | so i woudl like the test to be writen so that it caught the bug so it cant be regressed going forward | |
| 12:50:11 | ralonsoh | sean-k-mooney, when the port is first assigned, the mac is changed, right? | |
| 12:50:20 | ralonsoh | before the live migration | |
| 12:50:34 | ralonsoh | the Neutron DB port register MAC is changed | |
| 12:50:35 | sean-k-mooney | yes | |
| 12:50:43 | ralonsoh | ok | |
| 12:50:55 | sean-k-mooney | nova does a port update before we bind the port | |
| 12:51:06 | sean-k-mooney | the issue is we cant do a port update while it is bound | |
| 12:51:23 | ralonsoh | so the case here is to add this exception: to allow to change the MAC of the port even if it is bound | |
| 12:51:24 | ralonsoh | but | |
| 12:51:31 | ralonsoh | you also need to keep the original MAC | |
| 12:51:33 | ralonsoh | right? | |
| 12:51:34 | gibi | no | |
| 12:51:44 | gibi | we need to be able to define MAC per binding | |
| 12:51:49 | gibi | for mutliple binding | |
| 12:52:10 | gibi | at least that is how I model this | |
| 12:52:13 | ralonsoh | so this is a different feature I think | |
| 12:52:15 | gibi | one MAC per binding | |
| 12:52:16 | sean-k-mooney | we dont generally care about the orginal mac since we can always just read that form the PF if we need it | |
| 12:52:21 | ralonsoh | yes, like multiple port binding | |
| 12:52:55 | ralonsoh | ok, do you mind if I spend some time this afternoon or tomorrow thinking about this? | |
| 12:53:00 | ralonsoh | I need to finish some patches now | |
| 12:53:00 | gibi | ralonsoh: sure | |
| 12:53:03 | ralonsoh | thanks!! | |
| 12:53:08 | ralonsoh | I'll ping you | |
| 12:53:11 | gibi | thanks! | |
| 12:53:24 | gibi | I anyhow need to work on the nova side of this too | |
| 12:54:07 | gibi | sean-k-mooney: I like the idea to define the MAC when nova creates the binding instead of defining it when nova activates an inactive binding | |
| 12:54:20 | gibi | so I'd like to store the MAC in the binding profile | |
| 12:54:35 | gibi | it feels clearer | |
| 12:54:54 | gibi | as we define the rest of the infor in the profile at the same time like slot | |
| 12:55:14 | sean-k-mooney | ack | |
| 12:55:26 | sean-k-mooney | but in that case we need to permently keep the mac in the profile yes | |
| 12:55:47 | sean-k-mooney | so we shoudl be doing this for first spawn and sriov attach too | |
| 12:55:50 | sean-k-mooney | for consitency | |
| 12:55:58 | gibi | yes we do it | |
| 12:56:07 | gibi | any time we bind a SRIOV PF | |
| 12:56:28 | gibi | or create an inactive binding | |
| 12:56:45 | sean-k-mooney | ok and the key we use in teh profile for the PF mac is also different form teh key we use for the PF mac in the case of remote managed prots right | |
| 12:57:01 | sean-k-mooney | just making sure we dont have a collision there | |
| 12:57:17 | gibi | it is differen | |
| 12:57:18 | gibi | t | |
| 12:57:24 | gibi | the remote managed codepath stores pf_mac_address | |
| 12:57:29 | gibi | in the VF profiles | |
| 12:57:42 | gibi | my bugfix stores mac_address in the PF profile | |
| 12:57:57 | sean-k-mooney | ack | |
| 13:21:44 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Fix migration with remote-managed ports & add FT https://review.opendev.org/c/openstack/nova/+/829974 | |
| 13:24:11 | dmitriis | sean-k-mooney, gibi: ^ found a missing piece related to migration when adding more func tests. Since binding:profile is updated differently during live migration there is a slight change needed to also update new fields. | |
| 13:25:08 | dmitriis | I added a test case for live migration, looking into adding resize/evacuate/cold_migrate now as well. | |
| 13:29:55 | sean-k-mooney | dmitriis: you will need to update the inactive port bindign with the dest host info | |
| 13:30:29 | sean-k-mooney | dmitriis: i assume you are goign to supprot live migration by hot unplugging and hot plugging the VF as we do for vnic_type=direct yes | |
| 13:30:56 | sean-k-mooney | in the future we might eb able to read if the vf supprot live migration from sysfs and skip that if it does | |
| 13:31:59 | dmitriis | sean-k-mooney: the fix is quite simple https://review.opendev.org/c/openstack/nova/+/829974/2/nova/compute/manager.py#10805 - as you say, it's updating the inactive port binding | |
| 13:32:41 | sean-k-mooney | ya although that is adding more coupling to sysfs | |
| 13:32:59 | sean-k-mooney | gibi how do you feel about ^ | |
| 13:33:17 | dmitriis | yes, I was thinking of storing more in extra_info but this kind of information is more volatile (pf mac, vf num) | |
| 13:33:38 | dmitriis | compared to the serial number that we currently store in extra_info | |
| 13:33:44 | sean-k-mooney | dmitriis: gibi was going to store it in extra info | |
| 13:34:00 | sean-k-mooney | well the pf mac is basically hardcoded in the flash | |
| 13:34:03 | gibi | I'm on the side of moving this info the PciDevice object populated by the virt driver | |
| 13:34:10 | sean-k-mooney | ya | |
| 13:34:24 | sean-k-mooney | we have that object here so we can read if form there | |
| 13:34:38 | chateaulav | to set image metadata during a ci tempest, i would most likely have to use a script as there are no variables, correct? | |
| 13:34:46 | sean-k-mooney | so really we shoudl put this info in the extra_info dict with properties to access it form teh object | |
| 13:35:20 | sean-k-mooney | chateaulav: am it depends if you are writing new test then you woudl do it in the test | |
| 13:35:24 | dmitriis | sean-k-mooney: well, the PF itself may be a vport of a NIC switch. I've seen HW with variable number of PFs configurable (per a given flash configuration though) | |
| 13:35:53 | sean-k-mooney | but if you are trying to reuse exsitng tempest then you woudl have a pre playbook that generates a local.sh that runs at the end of devstack before tempest is run to update the default image with the properties | |
| 13:36:26 | dmitriis | it's *probably* safe to assume that the PF mac won't change across reboots is what I am going for. But we may encounter odd cases too. | |
| 13:36:36 | sean-k-mooney | chateaulav: you can also config tempst to use a different image and have devstack download addtional images via the local.conf | |
| 13:36:40 | chateaulav | sean-k-mooney: k, thanks | |
| 13:37:32 | sean-k-mooney | dmitriis: changing the number of PF would change ther PCI adress which would not be supported if you had existing vms | |
| 13:37:55 | sean-k-mooney | we can also update the mac in the db on agent start | |
| 13:38:11 | sean-k-mooney | althoguh that wont update the pf mac in the neutron port so maybe we shoudl detect that and cause an error | |
| 13:38:22 | sean-k-mooney | or at least a warning | |
| 13:39:04 | dmitriis | VF numbers have an offset but they should remain stable unless the number of PFs changes | |
| 13:39:05 | sean-k-mooney | in the unlikely case a nic dies we likely want to be able to replace it with another nic | |
| 13:39:22 | sean-k-mooney | dmitriis: right which we dont want to supprot | |
| 13:39:52 | dmitriis | sean-k-mooney: ok, then I agree in principle that we can store vf_num and pf_mac in extra_info | |
| 13:40:01 | dmitriis | not hard to retrieve it from there either | |
| 13:40:09 | sean-k-mooney | we are already storing them in the neutron db in the port bidnign | |
| 13:40:16 | sean-k-mooney | so we already have the requirement that they be stable | |
| 13:40:23 | dmitriis | right | |
| 13:41:53 | sean-k-mooney | if we detach a change in the mac we can warn on that in agent start if we want to and tell the operator what the old mac was and the new one so that they know they have to manually update the ports. we could also automaticaly do that i guess but i woudl proably defer that to a followup patch | |
| 13:43:16 | dmitriis | ack | |
| 13:46:50 | dmitriis | So I think that's the change gibi is working on https://review.opendev.org/c/openstack/nova/+/829248 | |
| 13:48:39 | gibi | dmitriis: yes, but note the self -1 :) I realized that I need to change direction | |
| 13:48:46 | gibi | to use the PciDevice object | |
| 13:50:25 | dmitriis | gibi: ack. I'll focus on adding the remaining func test cases in the short term but I'll be around to help with moving things to PciDevice object | |
| 13:51:02 | gibi | dmitriis: thanks | |
| 13:57:22 | opendevreview | Felix Huettner proposed openstack/nova stable/queens: Gracefull recovery when attaching volume fails https://review.opendev.org/c/openstack/nova/+/829861 | |