neutron

[RFE] Create a modular L2 agent framework for linuxbridge, sriov and macvtap agents

Bug #1468803 reported by Sean M. Collins on 2015-06-25

This bug affects 3 people

Affects		Status	Importance	Assigned to	Milestone
	neutron	Fix Released	Wishlist	Andreas Scheuring	neutron mitaka-3 "m3"

Bug Description

Problem Statement
=================

Currently, the Open vSwitch, Linux Bridge, and sriov mechanism drivers for the ML2 plugin have their own agents. This means that when improvements are made to one agent implementation, they have to be ported over to the other agents to gain the improvement. This has already happened, with patches like [1]. Much of the agent functionality is common enough that much of the code could be shared.

Discussion on the mailing list [2]

Analysis of current agents
==========================

Currently the following agents are in the neutron tree
- openvswitch [4]
- linuxbridge [3]
- sriov [5]
- mlnx agent [6]

For Mitaka the following agent is proposed
- Macvtap agent [7]

The following agent use a classical agent loop, monitoring or new devices to show up: linuxbridge, sriov, mlnx, macvtap

OVS uses ovsdb events (or similar things) to get notified about new events.

Proposal
========

High level architecture
-----------------------

Today the linuxbridge agent exists of 4 classes
- NetworkSegment
- LinuxBridgeManager --> encpsulating most of the bridge specifics
- LinuxBridgeRpcCallbacks --> Class containing all the rpc callback methods
- LinuxBridgeNeutronAgentRPC --> The agent loop itself

#1 Get a clear separation between agent loop and bridge impl specifics
Move all bridge specific code from LinuxBridgeNeutronAgentRPC to LinuxBridgeManager, like config options, rpc registrations,...

#2 Modify the LinuxBridgeNeutronAgentRPC to take the manager class as arg instead of creating it within the constructor. Manager class will be instantiated in lb main method

#3 Merge LinuxBridgeRpcCallbacks into LinuxBridgeNeutronAgentRPC

#4 Establish a clear interface for a manager class and enforce this in the common agent. Other manager must satisfy this interface in order to work properly with the common agent

#5 Move common agent into a new location

Benefit
-------

Sharing agent code, getting improvements/fixes for all agents. No needs for porting anymore.

Scope
-----
The proposal will restructure the lb agent in such a way and establish the lb agent as a first user.

NOT part of this proposal is to move over the sriov agent. However the common agent is designed in a way to make that easily possible. I'm just saying this is a separate effort.

This proposal will have no impact on the OVS agent.

Possible follow-up stages
-------------------------

- Implement macvtap agent as exploiter
- Move over sriov agent as exploiter
- Get shared code between the common agent and ovs agent?
- mlnx agent?

Sources
=======

[1] https://review.openstack.org/#/c/138512/
[2] http://lists.openstack.org/pipermail/openstack-dev/2015-June/067605.html
[3] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
[4] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
[5] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py
[6] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mlnx/agent/eswitch_neutron_agent.py
[7] https://bugs.launchpad.net/neutron/+bug/1480979

See original description

Tags:

Revision history for this message

Kyle Mestery (mestery) wrote on 2015-07-01:

This seems like a reasonable thing to do. I'd argue that once we have the reference implementation split out we could quite easily do this in that repository. This does seem quite invasive, so I'd really like Kevin and Yamamoto to comment here. I've added them to this bug.

Changed in neutron:
status:	New → Confirmed

Revision history for this message

Kevin Benton (kevinbenton) wrote on 2015-07-06:

I agree that this is a good idea. I think we might want to wait a couple of weeks until a lot of the feature churn slows down (e.g. the outstanding feature branch for QoS).

Revision history for this message

Akihiro Motoki (amotoki) wrote on 2015-07-08:

I agree too that this is a reasonable bug.

I think this work can be split into multiple works. odular L2 agent is a generic word.

- Modular for features (Miguel proposed event callback mechanism for RPC messages. This is one example)
- Modular for backend (OVS, linux bridge, ...)

any other aspects?

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-07-29: Related fix proposed to neutron (master)

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-07-29: Change abandoned on neutron (master)

Change abandoned by Sean M. Collins (<email address hidden>) on branch: master
Review: https://review.openstack.org/207071

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-08-05: Related fix proposed to neutron (master)

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-08-07:

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-08-11:

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-08-14:

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

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-09-22: Change abandoned on neutron (master)

#10

Change abandoned by Kyle Mestery (<email address hidden>) on branch: master
Review: https://review.openstack.org/213163
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-09-22:

#11

Change abandoned by Kyle Mestery (<email address hidden>) on branch: master
Review: https://review.openstack.org/211637
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2015-09-29: Re: Modular L2 Agent

#12

Can we change the title of this bug to be more meaningful?

Sean M. Collins (scollins) on 2015-10-11

summary:

- Modular L2 Agent
+ Create a modular L2 agent framework for OVS and Linux Bridge agents to
+ inherit from

Andreas Scheuring (andreas-scheuring) on 2015-10-30

summary:

- Create a modular L2 agent framework for OVS and Linux Bridge agents to
- inherit from
+ Create a modular L2 agent framework for linuxbridge, sriov and macvtap
+ agents

Andreas Scheuring (andreas-scheuring) on 2015-10-30

description:

updated

Andreas Scheuring (andreas-scheuring) on 2015-10-30

description:	updated
tags:	removed: ovs

Armando Migliaccio (armando-migliaccio) on 2015-11-11

Changed in neutron:
importance:	Undecided → Wishlist

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-11-17: Fix proposed to neutron (master)

#13

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

Changed in neutron:
assignee:	nobody → Andreas Scheuring (andreas-scheuring)
status:	Confirmed → In Progress

Revision history for this message

Miguel Angel Ajo (mangelajo) wrote on 2015-11-17: Re: Create a modular L2 agent framework for linuxbridge, sriov and macvtap agents

#14

nice to see advance on this front.

Armando Migliaccio (armando-migliaccio) on 2015-11-24

tags:

added: rfe-approved
removed: rfe

Henry Gessau (gessau) on 2015-11-24

summary:

- Create a modular L2 agent framework for linuxbridge, sriov and macvtap
- agents
+ [RFE] Create a modular L2 agent framework for linuxbridge, sriov and
+ macvtap agents

Armando Migliaccio (armando-migliaccio) on 2015-11-24

Changed in neutron:
milestone:	none → mitaka-1

OpenStack Infra (hudson-openstack) on 2015-11-27

Changed in neutron:
assignee:	Andreas Scheuring (andreas-scheuring) → Slawek Kaplonski (slaweq)

Armando Migliaccio (armando-migliaccio) on 2015-12-03

Changed in neutron:
milestone:	mitaka-1 → mitaka-2

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-12-04: Change abandoned on neutron (master)

#15

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/208666
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-12-04:

#16

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/209385
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2015-12-04:

#17

Change abandoned by Armando Migliaccio (<email address hidden>) on branch: master
Review: https://review.openstack.org/210250
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

OpenStack Infra (hudson-openstack) on 2015-12-04

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Andreas Scheuring (andreas-scheuring)

OpenStack Infra (hudson-openstack) on 2015-12-04

Changed in neutron:
assignee:	Andreas Scheuring (andreas-scheuring) → Slawek Kaplonski (slaweq)

OpenStack Infra (hudson-openstack) on 2015-12-07

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Andreas Scheuring (andreas-scheuring)

OpenStack Infra (hudson-openstack) on 2015-12-10

Changed in neutron:
assignee:	Andreas Scheuring (andreas-scheuring) → Slawek Kaplonski (slaweq)

OpenStack Infra (hudson-openstack) on 2015-12-11

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Andreas Scheuring (andreas-scheuring)

OpenStack Infra (hudson-openstack) on 2015-12-11

Changed in neutron:
assignee:	Andreas Scheuring (andreas-scheuring) → Slawek Kaplonski (slaweq)

OpenStack Infra (hudson-openstack) on 2015-12-18

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Andreas Scheuring (andreas-scheuring)

OpenStack Infra (hudson-openstack) on 2015-12-18

Changed in neutron:
assignee:	Andreas Scheuring (andreas-scheuring) → Slawek Kaplonski (slaweq)

OpenStack Infra (hudson-openstack) on 2015-12-21

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Ihar Hrachyshka (ihar-hrachyshka)

OpenStack Infra (hudson-openstack) on 2015-12-21

Changed in neutron:
assignee:	Ihar Hrachyshka (ihar-hrachyshka) → Slawek Kaplonski (slaweq)

OpenStack Infra (hudson-openstack) on 2016-01-05

Changed in neutron:
assignee:	Slawek Kaplonski (slaweq) → Ihar Hrachyshka (ihar-hrachyshka)

Ihar Hrachyshka (ihar-hrachyshka) on 2016-01-05

Changed in neutron:
assignee:	Ihar Hrachyshka (ihar-hrachyshka) → Andreas Scheuring (andreas-scheuring)

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2016-01-06: Fix merged to neutron (master)

#18

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

commit 23e0e29a2bad1d410db3f4b043fb4f232b40e87c
Author: Sławek Kapłoński <email address hidden>
Date: Thu Nov 26 23:31:11 2015 +0100

Add extension_manager and support for extensions in linuxbridge agent

    There is extensions mechanism for l2 agents already but it was
    implemented only for openvswitch l2 agent. This patch adds support for
    such extensions also for linuxbridge agent.

    This patch also adds support for network_update events received by the
    agent via RPC. It is required because sometimes when a network is
    updated (for example with a QoS policy is attached to it) all ports that
    belong to the network should also be updated.

Change-Id: Ie81c818d0eb817b044a6df1cbddc5864f118fe3f
Partial-bug: 1468803

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2016-01-13: Change abandoned on neutron (master)

#19

Change abandoned by Doug Wiegley (<email address hidden>) on branch: master
Review: https://review.openstack.org/248138
Reason: This review is > 4 weeks without comment, and failed Jenkins 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.

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2016-01-13:

#20

This is at danger of having the same fate of bug 1480979.

@Andreas: any heartbeat?

Changed in neutron:
status:	In Progress → Incomplete
assignee:	Andreas Scheuring (andreas-scheuring) → nobody
milestone:	mitaka-2 → none

Revision history for this message

Andreas Scheuring (andreas-scheuring) wrote on 2016-01-13:

#21

Yes, now as the minor patchsets got merged, I updated the main patchset that is doing the split off between linuxbridge specific and common part:

https://review.openstack.org/#/c/246318

After this patchset, there will be another one just moving the common parts into a new file. That's what I've planned for the modular agent in Mitaka! Everything else (sriov support) can happen in N

OpenStack Infra (hudson-openstack) on 2016-01-13

Changed in neutron:
assignee:	nobody → Andreas Scheuring (andreas-scheuring)
status:	Incomplete → In Progress

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2016-01-28: Fix proposed to neutron (master)

#22

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

Armando Migliaccio (armando-migliaccio) on 2016-01-28

Changed in neutron:
milestone:	none → mitaka-3

Revision history for this message

Armando Migliaccio (armando-migliaccio) wrote on 2016-01-28:

#23

status update: http://eavesdrop.openstack.org/meetings/neutron_drivers/2016/neutron_drivers.2016-01-28-22.20.log.txt

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2016-02-01: Fix merged to neutron (master)

#24

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

commit 6e29cdd6b654874e4003e31891228d3abc107700
Author: Andreas Scheuring <email address hidden>
Date: Tue Oct 13 13:21:32 2015 +0200

lb: ml2-agt: Separate AgentLoop from LinuxBridge specific impl

    The goal is to extract the common agent code from the linuxbridge agent
    to share this code with other agents (e.g. sriov and new macvtap [1]).
    This is a first step into the direction of a so called modular l2
    agent.

    Therefore all linuxbridge implementation specifics are moved into the
    LinuxBridgeManager class. The manager class will be passed as argument
    into the common agent loop instead of instantiating it in its
    constructor. In addition the network_maps and the updated_devices map
    has been moved into the rpc class.

A clear manager interface has been defined for the communication
between the common agent loop and the impl specific manager class.

    In a follow up patchset, the common agent loop will be moved into a
    new file. This has not yet happened to simplify tracking the code
    changes during review.

[1] https://bugs.launchpad.net/neutron/+bug/1480979

    Change-Id: Ia71f5a403b7029f8cc591f83df91ab2d3916f3f8
    Partial-Bug: #1468803
    Partial-Bug: #1480979

Revision history for this message

OpenStack Infra (hudson-openstack) wrote on 2016-02-10:

#25

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

commit 7d153a671b5fcc77437bc1e9b41015da1acc57f8
Author: Andreas Scheuring <email address hidden>
Date: Thu Jan 28 10:28:43 2016 +0100

Moving Common Agent into separate module

Moving the CommonAgent and all it's unittests into a speparate module.

Closes-Bug: #1468803

Change-Id: Ifccc6ee1a77eef3928ad326cd5857092aeef4a17