On Mon, May 22, 2023 at 9:55 AM Dumitru Ceara <dce...@redhat.com> wrote: > > On 5/22/23 15:42, Ihar Hrachyshka wrote: > > On Mon, May 22, 2023 at 7:55 AM Dumitru Ceara <dce...@redhat.com> wrote: > >> > >> On 5/17/23 18:46, Ihar Hrachyshka wrote: > >>> Thank you Dumitru! See below. > >>> > >>> On Tue, May 16, 2023 at 9:41 AM Dumitru Ceara <dce...@redhat.com> wrote: > >>>> I'm not necessarily rejecting this change. I just wanted to bring up an > >>>> alternative approach (I'm not sure if it's possible to implement it > >>>> though): > >>>> > >>>> The CMS (e.g., Neutron) probably knows before hand the reduced MTU value > >>>> it should impose on the localnet port. That means it might be possible > >>>> to just implement all this through logical flows that use > >>>> 'check_pkt_larger()'. > >>>> > >>> > >>> I think I mentioned this alternative in my cover letter for the > >>> series. (At least one of the variations.) > >>> > >>> I don't understand the specifics of what you are describing here > >>> though. Yes, CMS may do calculations for the "desired path MTU". But > >>> what's the mechanism for CMS to pass this information down to the OVN > >>> controller? ACL? I don't think the space of possible actions that > >>> would allow CMS itself to define the behavior is available to CMS. > >>> > >>> Perhaps you mean (as I think we discussed before in private) the > >>> approach where OVN northd creates Logical_Flow objects that would > >>> implement the behavior. If so, yes that could be possible. But note > >>> that northd doesn't know the desired path MTU (because it's not CMS > >>> and OVN northbound database API doesn't support MTU primitive for > >>> ports.) So the best we could do in the alternative way is implement > >>> half of the solution via Logical_Flow (the one that sends ICMP replies > >>> for "too-big" frames), leaving the other half (tagging the "too-big" > >>> frames based on interface MTU for processing by the Logical_Flows) to > >>> OVN controller. I think I listed this variation of the approach in my > >>> cover letter, but please let me know if it's not sufficient. > >>> > >>> (The split could be avoided if OVN adds a MTU attribute to its ports. > >>> But I of course am not trying to tackle this in this series, for > >>> obvious reasons. But MTU for ports is - in my mind - a worthy feature > >>> to have in OVN, if you ask me.) > >> > >> I was referring to this last part: adding MTU for ports and ovn-northd > >> translating those to logical flows that use check_pkt_larger() and set > >> REGBIT_PKT_LARGER. I was also hoping to see logical flows that generate > >> the ICMP error packets. But there's indeed no obvious way of doing that > >> because we need to act only on traffic that's destined to remote > >> hypervisors and that's not really an ovn-northd concept but more of an > >> ovn-controller one. > >> > >> If we go for your current approach and later add MTU for OVN ports, do > >> we need to be careful now in order to avoid complicated upgrades later? > >> > > > > I'm trying to think what complications could arise. Isn't it just > > generating effectively the same flows, now via Logical_Flow path? The > > flows act on local frames generated by regular VIF ports, so it > > shouldn't affect the rest of cluster that e.g. some of them are > > generated by Logical_Flow path and some through the proposed solution. > > Or am I talking about something else? > > > > Assuming your current approach is accepted it means ovn-controller will > automatically compute min MTU to use for multi-chassis ports. But if we > add an explicit OVN port MTU configuration what do we do with the > automatic computation? > > Do we leave it in place or do we remove it?
We'll make OVN LSP `mtu` override interface-derived value. (If ovn-controller sees `mtu` set for LSP, it seizes its operation; otherwise it sets flows itself.) > > If we remove it it means we'll break upgrades as northd is upgraded last > and it will only set the OVN port binding MTU after it gets upgraded. > We'll have both for one LTS release and sunset the interface-derived value, if needed? > If we leave it in place I'm not sure what the benefit of the explicit > MTU is. I might be wrong though. There are benefits of OVN LSP API `mtu` regardless of Path MTU Discovery series. For one, OVN could enforce its MTU on VIF interfaces (set it in `mtu_request`). This would also potentially allow a chassisA to inject Path MTU Discovery flows when `mtu` is changed on chassisB independently of ports hosted on chassisA. Right now ovn-controller is aware of MTUs for the ports it owns, but not for the ports on other chassis. I wouldn't look at `mtu` as OVN API as just a means to simplify the task for Path MTU, there are other benefits too. > > Regards, > Dumitru > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
