Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-03-13
12:52:37 sean-k-mooney so that is why you need this code
12:53:06 justas_napa yep
12:53:28 sean-k-mooney so the problem i see is the pci_slot is always going to be set for vnic_type virtio-forwarder
12:53:54 sean-k-mooney but we only want to take the else branch if its napatech
12:54:14 sean-k-mooney so ninstead of the current if we shoudl add a config value to the agent and check if that is set
12:54:25 sean-k-mooney so just like
12:54:27 sean-k-mooney sockdir = agent['configurations'].get('vhostuser_socket_dir',
12:54:29 sean-k-mooney ovs_const.VHOST_USER_SOCKET_DIR)
12:55:20 sean-k-mooney you shoudl add something like sockdir = agent['configurations'].get('vhostuser_socket_name_scheme','port_name')
12:55:50 sean-k-mooney and for napatech you woudl set vhostuser_socket_name_scheme to VF_index
12:55:54 sean-k-mooney or something liek that
12:55:56 justas_napa OK
12:56:22 justas_napa dvo-plv - does this work for us?
12:57:23 sean-k-mooney i need to think about the nova patch a bit but i think if you do that the nova patch is fine as is
12:57:54 justas_napa ack
12:57:55 sean-k-mooney well it needs test but the core functionality is proably correct
12:58:32 dvo-plv in our solution, socket name value is depends on the pci slot
12:58:55 dvo-plv This value vhostuser_socket_name_scheme will be located at the neutron conf file
12:59:11 sean-k-mooney yes used by the neutron_l2_agent
12:59:24 sean-k-mooney it will need to get added to the agent report
12:59:36 sean-k-mooney which will make it avaiable to the ml2 driver during binding
13:00:23 sean-k-mooney like this https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L379-L380
13:00:45 sean-k-mooney its how we pass the per host vhostuser_socket_dir and datapath_type info today
13:01:59 sean-k-mooney dvo-plv: looking at teh os-vif code i think that is also ok just needs tests
13:02:28 sean-k-mooney justas_napa: dvo-plv my concern basically is if we enable this and we use vaniall ovs-dpdk will it still work
13:03:06 dvo-plv So, instead of extending ovs with virtio-forwarder, you suggest to add new parameter ( socket scheme ) and we will create port with vhost user vif type, and based on a new parameter, it will affect socket path
13:03:08 justas_napa as long as user does not try to setup qinq and advance qos, it will work
13:03:36 sean-k-mooney dvo-plv: not quite
13:03:52 sean-k-mooney dvo-plv: keep your ml2/ovs driver patch as it is using virtio-forwarder
13:03:59 sean-k-mooney i think that is the correct vnic_type
13:04:24 sean-k-mooney im suggesting not basing the format on if the pci_slot it set
13:04:44 sean-k-mooney we shoudl either have a config option for it or base it on vnic_type virtio-forwarder
13:04:49 sean-k-mooney im just checkign something
13:05:01 sean-k-mooney we might not need to modify your patches at all
13:06:45 dvo-plv I see, make this function agent_vhu_sockpath able to change the structure of socket's name if it is specified in the config file
13:06:59 sean-k-mooney yep
13:07:19 sean-k-mooney so im trying to make sure we dont break agilio_ovs in the process of enabling napatech
13:07:33 sean-k-mooney im just checkign how we determin its agilio_ovs today in nova
13:08:20 sean-k-mooney ok your current patch shoudl be ok as is
13:08:28 sean-k-mooney they use a seperate vif_type
13:09:07 sean-k-mooney justas_napa: dvo-plv: so based on the refactoring that you have already done following my intial feedback
13:09:19 sean-k-mooney i think the current solution shoudl be generic enough to work as is
13:09:40 sean-k-mooney we dont have any exsiting use of vif_type=ovs and virtio-forwarder
13:10:06 sean-k-mooney we can add a comment that if we need a diffent nameing scheme in the futrue then a config option can be added at that point
13:11:24 justas_napa I'm not sure I follow
13:11:37 justas_napa do you think we need further updates
13:11:53 justas_napa or are we going as-is?
13:11:54 sean-k-mooney justas_napa: let me rephase. your code is fine as is it just need tests and docs
13:11:58 justas_napa OK
13:12:50 sean-k-mooney ill try and review the spec this week. if i have not done so by thrusday ping me
13:13:43 dvo-plv We need to update spec file accodring this concept, I will ping you when it will be ready
13:13:53 sean-k-mooney my general feedback is we need test to go along with the functional changes you have made and based on a quick review of the surrounding code and the questions you answered here i think the current approch is correct
13:15:10 sean-k-mooney so the spec is pretty light on detail in general
13:15:25 sean-k-mooney it would be good to update it with links to https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors
13:15:34 sean-k-mooney since that is really what you are tryign to enable
13:16:07 justas_napa sure. will do
13:16:12 sean-k-mooney i was hevially invovled in enabling dpdk supprot in openstack in general so most of the other core reviews dont have the same levle of context that i have
13:21:46 plibeau bauzas: thx for the review, I have reply on your comment. https://review.opendev.org/c/openstack/nova/+/861172
13:22:07 sean-k-mooney justas_napa: dvo-plv: one general comment on the spec (this is hard to get right by the way) the spec is intended to capture all the info require so that if you could not complete the feature someone else with a familararity with nova could compelte it. given the code is already written and looks mostly correct what you shoudl really focus on is providing enouch context for
13:22:09 sean-k-mooney nova reviews who dont really have famiariaty with ovs/dpdk/vf representors. basically add som short context paragraphs explaing what the technology is and how you are reusing/extendign the existing functionaly in nova/os-vif so that its simpler for other cores to review.
13:24:47 dvo-plv yes, sure, we will process this comment
13:24:49 dvo-plv I have some concenrs about this patch https://review.opendev.org/c/openstack/os-vif/+/859574/4/vif_plug_ovs/ovs.py How properly we should pass scheme to the os-vif, it hould be part of the vif in _plug_vf method?
13:29:03 jrosser should HW_ARCH_* trait be automatically set on compute hosts?
13:31:42 sean-k-mooney dvo-plv: we should be able to just pass the vhost-user socket path
13:32:28 sean-k-mooney jrosser: ideally yes but i don tknow if the libvirt driver does that today
13:33:11 jrosser my test suggests that for Z they're not there
13:34:50 sean-k-mooney then that was likely not implemtned
13:47:30 opendevreview sean mooney proposed openstack/nova stable/xena: Nova resize don't extend disk in one specific case https://review.opendev.org/c/openstack/nova/+/877260
13:50:42 opendevreview sean mooney proposed openstack/nova stable/wallaby: Nova resize don't extend disk in one specific case https://review.opendev.org/c/openstack/nova/+/877284
13:55:18 dvo-plv sean-k-mooney: does it will be flexible for another usage, if we pass socket path and parse it, getting vf_num for representor port
13:56:12 dvo-plv I mean that we add additional parameter to the neutron conf to get abiltiy set scheme for socket name to add some flexibility
13:57:45 dvo-plv https://review.opendev.org/c/openstack/os-vif/+/859574/4/vif_plug_ovs/ovs.py#307
13:57:46 dvo-plv here
14:11:24 bauzas Uggla: sorry I said to you that we could discuss about your series at 2 pm CET, but I was doing another stuff, do you want to discuss this now ?
14:11:48 Uggla bauzas, yes it is possible
14:12:10 bauzas cool
14:13:08 bauzas Uggla: (and other people wanting to discuss at https://review.opendev.org/c/openstack/nova/+/839401/24) meet.google.com/cah-soio-ard
14:13:12 bauzas shit
14:13:39 bauzas https://meet.google.com/cah-soio-ard
15:11:56 opendevreview Dan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2 https://review.opendev.org/c/openstack/nova-specs/+/877291
15:20:19 dansmith bauzas: you ready for a 2023.2 specs directory patch?
15:23:40 bauzas dansmith: we should already have it
15:23:47 bauzas amirite ?
15:23:49 dansmith is it proposed?
15:24:07 sean-k-mooney https://github.com/openstack/nova-specs/tree/master/specs/2023.2
15:24:09 sean-k-mooney its merged
15:24:23 dansmith wtf, I just fetched and had to create it myself
15:25:16 dansmith hrm, okay
15:25:23 dansmith must'n'tve worked or something
15:25:59 sean-k-mooney i assume your working on the set service_id in compute node table spec
15:26:10 dansmith yeah ^
15:26:20 sean-k-mooney cool
15:31:16 dansmith I'm realizing maybe I should have tried harder to get this second phase into 2023.1 because of the SLURP rules, but oh well
15:37:47 tobias-urdin sean-k-mooney: if you have some time over this week can you check the mdev naming fixes proposed to stable branches, starting with zed https://review.opendev.org/c/openstack/nova/+/866152 and the parent patch and cherry-picks w/ parents, ty!
16:01:20 bauzas tobias-urdin: I think I said +2 for stable/zed, right?
16:01:39 bauzas correct, so we need one stable core ^
16:01:56 tobias-urdin bauzas: yes! just need more, then moving on the the cherry-picks to older releases
16:10:21 bauzas ++
16:12:53 opendevreview Dan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2 https://review.opendev.org/c/openstack/nova-specs/+/877291
18:04:21 opendevreview Merged openstack/nova master: Unbind port when offloading a shelved instance https://review.opendev.org/c/openstack/nova/+/853682

Earlier   Later