| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-08 | |||
| 12:33:41 | jangutter | sean-k-mooney: the current review might be "option 3" then :-p | |
| 12:34:19 | jangutter | sean-k-mooney: it contains the PCI address of the repr? | |
| 12:34:23 | sean-k-mooney | jangutter: it contins info for how to attach the datapane to ovs but no info about offloads | |
| 12:34:37 | sean-k-mooney | jangutter: that is not offload metadata as far as im concserned | |
| 12:34:43 | jangutter | sean-k-mooney: yeah, "plugging" more than "hey set these TC flows". | |
| 12:35:03 | sean-k-mooney | jangutter: right so is your intent to enable the later or the former | |
| 12:35:28 | jangutter | sean-k-mooney: the former, this is to make plugging more generic. | |
| 12:35:35 | sean-k-mooney | well tc flows is also wrong | |
| 12:36:02 | jangutter | sean-k-mooney: been staring at the words "offload" too long so it kinda lost its meaning in my mind. | |
| 12:36:03 | sean-k-mooney | enable TSO or LSO is a offload | |
| 12:36:32 | sean-k-mooney | * offload metadata | |
| 12:36:53 | jangutter | sean-k-mooney: You can also enable or disable TC offloads (and if you want to be really specific, choosing the TC dataplane for OVS, with the offloaded option and then plugging into the resulting SR-IOV VF). | |
| 12:37:32 | sean-k-mooney | jangutter: yes but TC offloads are not flows e.g. they do not change where a packet is sent | |
| 12:37:43 | jangutter | sean-k-mooney: na-ah, they can. | |
| 12:38:22 | sean-k-mooney | tc flows can but its not an offload if send the packet to new york instad of sydney | |
| 12:39:34 | sean-k-mooney | jangutter: that is what i ment by choosing where its send not is this done in software or hadware | |
| 12:39:43 | jangutter | sean-k-mooney: it is an offload if the packet is encapped in a vxlan tunnel before it goes to NY :-p | |
| 12:39:51 | mdbooth | Folks, when running gertty for the first time, how many years should you wait for the UI to become responsive? | |
| 12:40:10 | openstackgerrit | Yikun Jiang proposed openstack/nova master: Remove the allocation ratios adjusting logic https://review.openstack.org/602805 | |
| 12:40:10 | openstackgerrit | Yikun Jiang proposed openstack/nova master: Use new ``initial_xxx_allocation_ratio`` CONF https://review.openstack.org/602804 | |
| 12:40:20 | sean-k-mooney | jangutter: no it would have been encapulated in software without the offload and set there in ether case | |
| 12:40:21 | jangutter | sean-k-mooney: but yeah, I take your point, my semantics is all wrong. | |
| 12:41:50 | jangutter | sean-k-mooney: so, I guess there's at least one action point for me: at least s/offloads/plugging modes/ in many respects on the spec. | |
| 12:42:03 | sean-k-mooney | jangutter: so back to what you acatully want to achive. im fine with haveing a generic port forifle that carries the data plane info for represntors | |
| 12:42:28 | jangutter | sean-k-mooney: and keep the OVS representor profile as is? | |
| 12:42:29 | sean-k-mooney | if we want to allow offload config the it has to be at the port level only. | |
| 12:42:51 | sean-k-mooney | jangutter: well that depend on if you want to do offload config | |
| 12:42:57 | sean-k-mooney | or how | |
| 12:43:20 | sean-k-mooney | the thing is the generic represtor port profile should not know anything about ovs | |
| 12:43:47 | sean-k-mooney | so if we have to enable an offload in a ovs specific way then that should not be in the generic profile | |
| 12:44:12 | jangutter | sean-k-mooney: yep.... you do realise it's making more of Jay's case for him :-p | |
| 12:44:33 | sean-k-mooney | if we have some traits like thing we can use as an indirection and have the driver interperate that then thats fine | |
| 12:45:12 | jangutter | sean-k-mooney: but I understand what you mean, the distinction between offload metadata and plugging modes should be separate. | |
| 12:45:19 | sean-k-mooney | jangutter: not really i just said storing backend sepcific metadata in the generic represtor profile would not be ok | |
| 12:45:27 | sean-k-mooney | that include via compostion | |
| 12:45:39 | sean-k-mooney | jangutter: ya | |
| 12:46:30 | jangutter | sean-k-mooney: cool, so you had in mind "add one more class" not "convert one more class to a random bag of dicts"... | |
| 12:46:48 | sean-k-mooney | yes | |
| 12:47:21 | sean-k-mooney | random bag of dicts that will some day be sent over api is less then ideal | |
| 12:47:37 | jangutter | sean-k-mooney: verily. | |
| 12:48:35 | jangutter | sean-k-mooney: thanks very much, will archive this review as a warning to future developers. | |
| 12:48:56 | sean-k-mooney | im fine with VIFPortProfileOVSRepresentor having no addtional field and inheriting from VIFPortProfileRepresentor by the way | |
| 12:49:28 | jangutter | sean-k-mooney: that won't work, unfortunately, will have to be multiple inheritance. | |
| 12:49:37 | sean-k-mooney | the thing is right now we dont have any offload metadata that we are sending so the pluggins modes info is all that class contains | |
| 12:49:44 | fried_rice | mdbooth: I set up gertty during the PTG and it took like three days before it finished loading everything down. | |
| 12:49:54 | sean-k-mooney | jangutter: oh why? | |
| 12:50:16 | mdbooth | efried: Ah, ok. That's when I started, and it took so long I assumed I'd done it wrong and deleted it. | |
| 12:50:31 | efried | mdbooth: That had been my experience the first time I tried it. | |
| 12:50:35 | jangutter | sean-k-mooney: with VIFHostDevice or VIFVhostUser we need the bridge name and other thingies too. | |
| 12:50:54 | efried | mdbooth: Perhaps broadband you could get it to load up faster, though actually I suspect not. I wasn't getting close to maxing out my bw when I was doing it. | |
| 12:51:01 | jangutter | sean-k-mooney: so OVSRepresentor would have to have multiple inheritance (OVS and Representor). | |
| 12:51:09 | sean-k-mooney | efried: ya there are some knonw issue. if its worth anything the pip version works better the package manager | |
| 12:51:22 | mdbooth | Well I'm currently on 4G mobile data, so I guess I'd better kill it | |
| 12:51:25 | efried | I don't remember how I installed it. | |
| 12:51:27 | sean-k-mooney | jangutter: no it wouldn't | |
| 12:51:33 | jangutter | efried: yeah, apparently ONLY USE THE PIP VERSION. | |
| 12:51:35 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Add native implementation OVSDB API https://review.openstack.org/482226 | |
| 12:51:41 | sean-k-mooney | VIFPortProfileOVSRepresentor is a port profile not a vif | |
| 12:52:08 | mdbooth | No, seriously | |
| 12:52:55 | jangutter | sean-k-mooney: ah you're right: interface_id is what's currently used. | |
| 12:52:59 | sean-k-mooney | mdbooth: it takes a while to sync because its cloneing all the git repos you subsribe to | |
| 12:53:23 | mdbooth | My gate is finally open. | |
| 12:53:28 | sean-k-mooney | jangutter: interface_id is used for what exactly | |
| 12:54:17 | jangutter | sean-k-mooney: when the port gets plugged into OVS, there's an interface_id UUID added to the OVSDB config. | |
| 12:54:45 | sean-k-mooney | yes that is pulled form the ID field in the base os-vif VIF object | |
| 12:54:47 | jangutter | sean-k-mooney: last time I checked, Neutron listens to OVSDB for that interface_id UUID to pop up in order to confirm the plugging. | |
| 12:55:22 | sean-k-mooney | jangutter: for the ml2 agent yes it check the interface_id in the external_ids colume of the port table | |
| 12:55:56 | jangutter | sean-k-mooney: yep, but is that guaranteed to be the same as VIFBase.id ? | |
| 12:56:12 | sean-k-mooney | jangutter: yes that is where we read if from | |
| 12:56:23 | jangutter | sean-k-mooney: why is interface_id then a field in the OVS port profile? | |
| 12:57:09 | sean-k-mooney | legacy reasons | |
| 12:57:15 | sean-k-mooney | basically it was used by libvirt | |
| 12:57:38 | sean-k-mooney | it got copied when we did the import from nova but its the same id | |
| 12:57:51 | jangutter | sean-k-mooney: o.m.w. here I was thinking that it's the whole reason behind port profiles! | |
| 12:58:02 | sean-k-mooney | jangutter: nope | |
| 12:58:46 | jangutter | sean-k-mooney: so, one VIF can only ever ever have one interface ID, and that's the same as its UUID? | |
| 12:58:59 | sean-k-mooney | looks like we are using it https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/ovs.py#L124 | |
| 12:59:15 | sean-k-mooney | jangutter: yep it should be | |
| 12:59:46 | sean-k-mooney | let me check nova to confirm but they should never be different as far as i know | |
| 12:59:53 | jangutter | sean-k-mooney: yep, that was the line of code that made me thought it could differ from vifbase.id. | |
| 13:00:27 | sean-k-mooney | i dont think it can but im checking | |
| 13:00:47 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Remove IPTools deprecated implementation https://review.openstack.org/605422 | |
| 13:02:04 | sean-k-mooney | jangutter: so ya the neutron port uuid is stored in the id field https://github.com/openstack/nova/blob/master/nova/network/os_vif_util.py#L248-L264 | |
| 13:03:08 | jangutter | sean-k-mooney: interface_id=vif.get('ovs_interfaceid') or vif['id'] | |
| 13:03:14 | sean-k-mooney | jangutter: and here we even default it to the vif[id] if ovs_interfaceid is not set https://github.com/openstack/nova/blob/master/nova/network/os_vif_util.py#L288 | |
| 13:03:23 | sean-k-mooney | yes | |
| 13:03:47 | jangutter | sean-k-mooney: yep.... Guess what, I've also wrote code like that. | |
| 13:04:18 | jangutter | sean-k-mooney: but my interpretation was that "ovs_interfaceid" is the new hotness and vif['id'] is old-and-busted. | |
| 13:05:26 | sean-k-mooney | jangutter: nope othere way around ovs_interfaceid i think was a nova networks thing | |
| 13:05:57 | sean-k-mooney | using neutron i dont think they can ever be different at least not currently | |
| 13:06:09 | jangutter | https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_vif.py#L84 | |
| 13:06:46 | jangutter | sean-k-mooney: yeah, in the tests, there's a definite difference between uuids.ovs and uuids.vif | |
| 13:07:05 | sean-k-mooney | jangutter: that does not mean the tests are correct :) | |
| 13:07:30 | jangutter | sean-k-mooney: yep! thanks for showing me the error of my ways again! Important safety tip. | |
| 13:07:54 | sean-k-mooney | jangutter: https://github.com/openstack/nova/blob/a330c9a143dea8095a3d1c3eabd56193ad6f38b1/nova/network/neutronv2/api.py#L2651 so this is where its set :) | |
| 13:08:01 | jangutter | sean-k-mooney: makes me think of basic particle physics where time symmetry is a thing. | |
| 13:09:51 | sean-k-mooney | jangutter: by the way want to open a bug for the incorrect tests? | |
| 13:10:02 | jangutter | sean-k-mooney: this is pretty hilarious now that I think of it, I was almost actively undoing the direction that Nova, Neutron and OS-VIF is going. | |
| 13:10:41 | jangutter | sean-k-mooney: I'll try to get on it today: that looks like something nice to fix for future Jan's not to step into. | |
| 13:11:10 | sean-k-mooney | jangutter: ya i was thinking it would make a nice low hanging fruit style bug. | |