| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-03-13 | |||
| 12:26:44 | dvo-plv | when you are talking about original approach. You mean separate driver or implementation support with openvswitch | |
| 12:27:26 | sean-k-mooney | when i was suggesting using a singel ml2/driver for both i was suggesting ensuring your nic works with the dpdk_user supprot in vaniall ovs | |
| 12:27:40 | sean-k-mooney | and i was asking if that was what you were trying to enable | |
| 12:27:55 | sean-k-mooney | the answer to the second quetion is no | |
| 12:28:04 | sean-k-mooney | you are not trying to enabel to upstream ovs feature | |
| 12:28:15 | sean-k-mooney | your are trying to enable your vendor specific version | |
| 12:28:26 | sean-k-mooney | in which case usign an out of tree ml2 dirver is appropreiate | |
| 12:28:56 | sean-k-mooney | that way you do not need to worry about compatiablity with netonomes implamation of virtio forward | |
| 12:29:38 | sean-k-mooney | nova just uses the vhost-user path provided by neutron so as long as you set it to the correct value that should be transparent to nova | |
| 12:32:35 | sean-k-mooney | dvo-plv: have dpdk removed or raised the limit on dpdk type ports above 32 yet | |
| 12:37:19 | sean-k-mooney | dvo-plv: what i would suggest is adding a VIF_DETAILS_VHOSTUSER_NAPATECH_PLUG | |
| 12:37:49 | sean-k-mooney | so in the port binding details set a flag to indicate taht its napatech | |
| 12:38:34 | sean-k-mooney | continue to use vnic_type=vritio-forwarder | |
| 12:39:29 | justas_napa | Hi. To answer the question regarding ovs support - it really depends on the end user requirements | |
| 12:39:38 | sean-k-mooney | in nova check for both and in a new if in os_vif_util implement the logic you require | |
| 12:40:13 | justas_napa | we maintain our own OvS to support features like QinQ and some special QoS schemes | |
| 12:40:18 | sean-k-mooney | justas_napa: the in tree ml2/ovs and ml2/ovn dirver are only ment to supprot ovs form openvsiwtch.org | |
| 12:41:04 | sean-k-mooney | so if the basic virtio-forward fucntionality is not upstream to vanilla ovs it should be in a seperatem ml2 driver and os_vif driver | |
| 12:41:12 | justas_napa | I think we are OK to limit ourselves to vanilla ovs | |
| 12:41:53 | justas_napa | unless adding support for our own OVS is trivial | |
| 12:42:52 | sean-k-mooney | well vaniall ovs has two ways to enabel vhost-user but the proposal is not using either of them | |
| 12:42:59 | sean-k-mooney | https://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/ | |
| 12:43:17 | sean-k-mooney | instead you are created dpdk vdevs https://docs.openvswitch.org/en/latest/topics/dpdk/vdev/ | |
| 12:44:58 | sean-k-mooney | there is supprot for dpdk representor prots | |
| 12:45:27 | justas_napa | so then it's custom ml2 and os_vif driver | |
| 12:45:57 | sean-k-mooney | https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors | |
| 12:46:02 | sean-k-mooney | am i think so. | |
| 12:46:26 | justas_napa | yes, we are heavy users of representors | |
| 12:46:26 | sean-k-mooney | if you go with the out of tree ml2/os-vif drivers then all that is requried in nova is a minor change to call them | |
| 12:46:46 | sean-k-mooney | so ok maybe we need to level set | |
| 12:47:07 | sean-k-mooney | justas_napa: the dpdk type port that you are creating for use with virio-forward | |
| 12:47:14 | sean-k-mooney | is that a vf representor | |
| 12:47:21 | sean-k-mooney | as descirbed here https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors | |
| 12:47:32 | justas_napa | yes | |
| 12:47:50 | justas_napa | becuase all dataplane is offloaded to FPGA | |
| 12:48:01 | sean-k-mooney | ok so if that is what we are enableing and the supprot in upstream ovs also works with your nics we can enabel that in the intree support | |
| 12:48:54 | sean-k-mooney | so next question you require the vhost-user socket path to have a specific format to corralate teh vhost-user path to the dpdk representor correct | |
| 12:49:42 | justas_napa | I don't think we require specific path, just the socket name | |
| 12:49:53 | sean-k-mooney | the orginal requriements when vhsot-user was first added to ovs was the socket path's final segment and port name must be the same | |
| 12:50:20 | sean-k-mooney | right so for normal vhost-user when it was first added the socket-name and port name had to be the same | |
| 12:50:47 | sean-k-mooney | later we added a socket path to ovs to remove that limiation as part of supproting vhost-user-client mode where qemu is the server | |
| 12:51:22 | justas_napa | -chardev socket,id=char0,path=/usr/local/var/run/stdvio5,server | |
| 12:51:50 | justas_napa | so we are flexible on thje path, but the last part is stdvio<#VF> | |
| 12:52:00 | sean-k-mooney | ya so your dirver breaks the convention that the final segment of the name matches the name of the ovs port which is fine | |
| 12:52:07 | sean-k-mooney | ack | |
| 12:52:32 | sean-k-mooney | https://review.opendev.org/c/openstack/neutron/+/869510/2/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#217 | |
| 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 | |