default geneve overhead is not set high enough for ovn

Bug #1868137 reported by Radosław Piliszek
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Fix Released
Medium
Elvira García Ruiz

Bug Description

Comparing:
https://opendev.org/openstack/neutron/src/commit/5e84289f68402b401d9a35d93473d9517a43e300/devstack/lib/ovn_agent#L78
with:
https://opendev.org/openstack/neutron-lib/src/commit/37deb266394a5c633309b3ad66241bb02695bdb9/neutron_lib/constants.py#L325

And we get MTU errors when leaving default.
[ml2_type_geneve]/max_header_size has to be set to 38 to make Geneve encapsulation work in OVN.

Devstack does the override above so the real default is not tested.

Release: Ussuri

This does not affect bare OVS with OVS agent.

Changed in neutron:
status: New → Confirmed
Revision history for this message
Slawek Kaplonski (slaweq) wrote :

So it seems for me that it's simply neutron-lib issue where GENEVE_ENCAP_MIN_OVERHEAD should be changed to 38, am I right?

tags: added: ovn
Changed in neutron:
importance: Undecided → Medium
Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

Agreed. I would also remove the shadowing from OVN_GENEVE_OVERHEAD.
I think it might be useful for some devs (although currently not overridden anywhere in opendev).
So maybe not set the default there but set the ini param only if an override is in effect?

PS: I found it also here: https://opendev.org/openstack/networking-ovn/src/commit/eda5d7f80d877601170631c5f5485370ea701f42/devstack/lib/networking-ovn#L75

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (master)

Related fix proposed to branch: master
Review: https://review.opendev.org/714050

Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

I updated the bug to reflect the real problem. OVN docs kinda mention that but maybe the emphasis should be a bit stronger?

summary: - default geneve overhead is set not enough
+ default geneve overhead is not set high enough for ovn
description: updated
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (master)

Change abandoned by Radosław Piliszek (<email address hidden>) on branch: master
Review: https://review.opendev.org/714050
Reason: let's not do it this way

Revision history for this message
Bence Romsics (bence-romsics) wrote :

Geneve has a variable length header as explained in the config option help:

45 cfg.IntOpt('max_header_size',
46 default=p_const.GENEVE_ENCAP_MIN_OVERHEAD,
47 help=_("Geneve encapsulation header size is dynamic, this "
48 "value is used to calculate the maximum MTU "
49 "for the driver. "
50 "The default size for this field is 30, which is the "
51 "size of the Geneve header without any additional "
52 "option headers.")),

I see one error here, that the ovn mechanism driver does not throw an error at startup time when this config option is too low for it to work.

Plus we could be a bit more user friendly if we set the config default to 38 which works for ovn. But if we do that let's not call that GENEVE_ENCAP_MIN_OVERHEAD, because for geneve the minimum is 30. ovn happens to use it in a way that requires 38.

Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

I'd go with preciser docs and stricter error checking (so that I'm not getting MTU issues).

Revision history for this message
Bence Romsics (bence-romsics) wrote :

Radoslaw, do you want to take this fix? If not, I can take it.

Revision history for this message
Radosław Piliszek (yoctozepto) wrote :

Hi Bence, I can propose something for review, sure, thanks for asking.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

Fix proposed to branch: master
Review: https://review.opendev.org/714716

Changed in neutron:
assignee: nobody → Radosław Piliszek (yoctozepto)
status: Confirmed → In Progress
Revision history for this message
Slawek Kaplonski (slaweq) wrote : auto-abandon-script

This bug has had a related patch abandoned and has been automatically un-assigned due to inactivity. Please re-assign yourself if you are continuing work or adjust the state as appropriate if it is no longer valid.

Changed in neutron:
assignee: Radosław Piliszek (yoctozepto) → nobody
status: In Progress → New
tags: added: timeout-abandon
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (master)

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.opendev.org/714716
Reason: This review is > 4 weeks without comment, and failed Zuul jobs the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

Changed in neutron:
assignee: nobody → Elvira García Ruiz (elviragr)
Changed in neutron:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.opendev.org/c/openstack/neutron/+/714716
Committed: https://opendev.org/openstack/neutron/commit/dc4a57d96661fcb56ff916cafbbbdc683653e9c4
Submitter: "Zuul (22348)"
Branch: master

commit dc4a57d96661fcb56ff916cafbbbdc683653e9c4
Author: Radosław Piliszek <email address hidden>
Date: Tue Mar 24 17:55:23 2020 +0100

    Make OVN driver validate Geneve max_header_size

    Also updates the docs to be clearer on OVN-Geneve relation topics.

    Co-Authored-By: Elvira García Ruiz <email address hidden>
    Change-Id: Ia253cc4d85261ce1535f4d27b3da91275d879903
    Closes-bug: #1868137

Changed in neutron:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/neutron 19.0.0.0rc1

This issue was fixed in the openstack/neutron 19.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.