l3_db updates port db without calling l2 plugin

Bug #1475093 reported by Isaku Yamahata on 2015-07-16
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
neutron
Undecided
Isaku Yamahata

Bug Description

l3 db updates port::owner directly without calling l2 plugin when adding an interface to the router.
So ML2 mechanism driver gets confused resulting an error.

Fix proposed to branch: master
Review: https://review.openstack.org/202357

Changed in neutron:
assignee: nobody → Isaku Yamahata (yamahata)
status: New → In Progress

Related fix proposed to branch: master
Review: https://review.openstack.org/202359

Reviewed: https://review.openstack.org/202358
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=9ba23658a34eac696a7eed9a8aaca6a4625d1391
Submitter: Jenkins
Branch: master

commit 9ba23658a34eac696a7eed9a8aaca6a4625d1391
Author: Isaku Yamahata <email address hidden>
Date: Wed Jul 15 12:34:12 2015 -0700

    l3_db: not use L2 plugin _get_port unnecessarily

    This patch is clean up to prevent future breakage by eliminating
    potentially dangerous code.

    l3_db uses L2 plugin _get_port method unnecessarily instead of get_port.
    It's dangerous because _get_port returns ORM db object which allows
    the caller to update db rows directly. So the caller of _get_port may
    update port db without notifying L2 plugin unintentionally.
    In that case, L2 plugin or ML2 mechanism driver will be confused.
    This patch replace _get_port with get_port method where possible.

    Change-Id: I5a23f6cac5ea359645e6947fd69978f060c4ba97
    Related-Bug: #1475093

Reviewed: https://review.openstack.org/202359
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=ff709d5b83abdc954311be9e7d52dfa9e9c3d7ba
Submitter: Jenkins
Branch: master

commit ff709d5b83abdc954311be9e7d52dfa9e9c3d7ba
Author: Isaku Yamahata <email address hidden>
Date: Wed Jul 15 19:27:16 2015 -0700

    l3: not use L2 plugin _get_subnet unnecessarily

    This patch is clean up to prevent future breakage by eliminating
    potentially dangerous code.

    l3_db and related code use L2 plugin _get_subnet and related method
    unnecessarily instead of get_subnet.
    It's dangerous because _get_subnet returns ORM db object which allows
    the caller to update db rows directly. So the caller of _get_subnet
    may update subnet db without notifying L2 plugin unintentionally.
    In that case, L2 plugin or ML2 mechanism driver will be confused.
    This patch replaces _get_subnet and _get_subnets_by_network with
    get_subnet, get_subnets_by_network where possible.

    Change-Id: I85769e639a408a292b5bd70a9d9a1ac292e2b51c
    Related-Bug: #1475093

Download full text (155.6 KiB)

Reviewed: https://review.openstack.org/218710
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2c5f44e1b3bd4ed8a0b7232fd293b576cc8c1c87
Submitter: Jenkins
Branch: feature/pecan

commit f35d1c5c50dccbef1a2e079f967b82f0df0e22e9
Author: Adelina Tuvenie <email address hidden>
Date: Thu Aug 27 02:27:28 2015 -0700

    Fixes wrong neutron Hyper-V Agent name in constants

    Change Id03fb147e11541be309c1cd22ce27e70fadc28b5 moved the
    AGENT_TYPE_HYPERV constant from common.constants to
    plugins.ml2.drivers.hyperv.constants but change the value of the
    constant from 'HyperV agent' to 'hyperv'. This patch changes
    the name back to 'HyperV agent'

    Change-Id: If74b4b2a84811e266c8b12e70bf6bfe74ed4ea21
    Partial-Bug: #1487598

commit de604de334854e2eb6b4312ff57920564cbd4459
Author: OpenStack Proposal Bot <email address hidden>
Date: Sun Aug 30 01:39:06 2015 +0000

    Updated from global requirements

    Change-Id: Ie52aa3b59784722806726e4046bd07f4a4d97328

commit f0415ac20eaf5ab4abb9bd4839bf6d04ceee85d0
Author: armando-migliaccio <email address hidden>
Date: Fri Aug 28 13:53:04 2015 -0700

    Revert "Add support for unaddressed port"

    This implementation may expose a vulnerability where a malicious
    user can sieze the opportunity of a time window where a port
    may land unaddressed on a shared network, thus allowing him/her
    to suck up all the tenant traffic he/she wants....oh the shivers.

    This reverts commit d4c52b7f5a36a103a92bf9dcda7f371959112292.

    Change-Id: I7ebdaa8d3defa80eab90e460fde541a5bdd8864c

commit 013fdcd2a6d45dbe4de5d6e7077e5e9b60985ef9
Author: Assaf Muller <email address hidden>
Date: Fri Aug 28 16:41:07 2015 -0400

    Improve logging upon failure in iptables functional tests

    This will help us nail down a more accurate and efficient logstash
    query.

    Change-Id: Iee4238e358f7b056e373c7be8d6aa3202117a680
    Related-Bug: #1478847

commit 622dea818d851224a43d5276a81d5ce8a6eebb76
Author: Ivar Lazzaro <email address hidden>
Date: Mon Aug 17 17:17:42 2015 -0700

    handle gw_info outside of the db transaction on router creation

    Move the gateway interface creation outside the DB transaction
    to avoid lock timeout.

    Change-Id: I5a78d7f32e8ca912016978105221d5f34618af19
    Closes-bug: 1485809

commit 5b27d290a0a95f6247fc5a0fe6da1e7d905e6b2d
Author: Assaf Muller <email address hidden>
Date: Wed Aug 26 10:07:03 2015 -0400

    Remove ml2 resource extension success logging

    This is the cause of a tremendous amount of logs, for no
    perceivable gain. A normal dvr run in the gate shows this debug
    message around 120K times, which is way too much.

    Closes-Bug: #1489952

    Change-Id: I26fca8515d866a7cc1638d07fa33bc04479ae221

commit 8d3faf549cba2f58c872ef4121b2481e73464010
Author: huangpengtao <email address hidden>
Date: Fri Aug 28 23:20:46 2015 +0800

    Replace "prt" variable by "port"

    the local variable prt is meaningless,
    and port is used popular.

    Change-Id: I20849102cf5b4d84433c46791b4b1e2a22dc4739

commit ee374e7a5f4dea538fcd942f5...

tags: added: in-feature-pecan
Wim De Clercq (wim-de-clercq) wrote :

bgpvpn is also having this issue for https://blueprints.launchpad.net/bgpvpn/+spec/router-bgpvpn-association

Inside _add_interface_by_port the port update is happening without using ml2, so there is no way to get notified by this type of router-interface-add.

Changed in bgpvpn:
status: New → Confirmed
importance: Undecided → Wishlist

Reviewed: https://review.openstack.org/202357
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=0947458018725b241603139f4ec6f92e84b2f29b
Submitter: Jenkins
Branch: master

commit 0947458018725b241603139f4ec6f92e84b2f29b
Author: Isaku Yamahata <email address hidden>
Date: Wed Jul 15 12:17:48 2015 -0700

    l3_db: it updates port attribute without L2 plugin

    L3_NAT_dbonly_mixin._add_interface_by_port update Port.owner
    db entry directly without notifying l2 plugin.
    Thus L2 plugin/ML2 mechanism driver will be confused when
    interface is attached to router by port because port owner is different.
    Use L2 plugin update_port method to update port:owner.

    Change-Id: If0178887282456842b6078a851a9233cb58a391a
    Closes-Bug: #1475093

Changed in neutron:
status: In Progress → Fix Committed
Changed in neutron:
status: Fix Committed → Fix Released

This issue was fixed in the openstack/neutron 8.0.0.0b2 development milestone.

Mathieu Rohon (mathieu-rohon) wrote :

We can remove the bgpvpn project as an affected project. The related bug in bgpvpn will be :

https://bugs.launchpad.net/bgpvpn/+bug/1537067

no longer affects: bgpvpn

Change abandoned by Oded Shvartz (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/277169

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

Other bug subscribers