openstack
/
neutron
Code Issues Proposed changes

get_subnet_for_dvr returns SNAT mac instead of distributed gateway in subnet_info 

On hosts with dvr_snat agent mode, after restarting OVS agent,
sometimes the SNAT port is processed first instead of the distributed port.
The subnet_info is cached locally via get_subnet_for_dvr when either of these ports
are processed. However, it returns the MAC address of the port used to query
as the gateway for the subnet. Using the SNAT port, this puts the wrong
MAC as the gateway, causing some flows such as the DVR flows on br-int
for local src VMs to have the wrong MAC.

This patch fixes the get_subnet_for_dvr with fixed_ips as None for the csnat port,
as that causes the server side handler to fill in the subnet's actual gateway
rather than using the port's MAC.

Change-Id: If045851819fd53c3b9a1506cc52bc1757e6d6851
Closes-Bug: #1783470
This commit is contained in:
Arjun Baindur 2018-07-30 14:22:30 -07:00 committed by Swaminathan Vasudevan
parent 0dd28ee0fd
commit c6de172e58
2 changed files with 27 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py
View File

@ -538,7 +538,7 @@ class OVSDVRNeutronAgent(object):
# no csnat ports seen on this subnet - create csnat state # no csnat ports seen on this subnet - create csnat state
# for this subnet # for this subnet
subnet_info = self.plugin_rpc.get_subnet_for_dvr( subnet_info = self.plugin_rpc.get_subnet_for_dvr(
self.context, subnet_uuid, fixed_ips=fixed_ips) self.context, subnet_uuid, fixed_ips=None)
if not subnet_info: if not subnet_info:
LOG.warning("DVR: Unable to retrieve subnet information " LOG.warning("DVR: Unable to retrieve subnet information "
"for subnet_id %s. The subnet or the gateway " "for subnet_id %s. The subnet or the gateway "

26
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
View File

@ -3049,6 +3049,32 @@ class TestOvsDvrNeutronAgent(object):
] ]
self.assertEqual(expected_on_tun_br, tun_br.mock_calls) self.assertEqual(expected_on_tun_br, tun_br.mock_calls)
def test_port_bound_for_dvr_with_csnat_port_without_passing_fixed_ip(self):
self._setup_for_dvr_test()
int_br = mock.create_autospec(self.agent.int_br)
tun_br = mock.create_autospec(self.agent.tun_br)
int_br.set_db_attribute.return_value = True
int_br.db_get_val.return_value = {}
with mock.patch.object(self.agent.dvr_agent.plugin_rpc,
'get_subnet_for_dvr') as mock_getsub,\
mock.patch.object(self.agent.dvr_agent.plugin_rpc,
'get_ports_on_host_by_subnet',
return_value=[]),\
mock.patch.object(self.agent.dvr_agent.int_br,
'get_vif_port_by_id',
return_value=self._port),\
mock.patch.object(self.agent, 'int_br', new=int_br),\
mock.patch.object(self.agent, 'tun_br', new=tun_br),\
mock.patch.object(self.agent.dvr_agent, 'int_br', new=int_br),\
mock.patch.object(self.agent.dvr_agent, 'tun_br', new=tun_br):
self.agent.port_bound(
self._port, self._net_uuid, 'vxlan',
None, None, self._fixed_ips,
n_const.DEVICE_OWNER_ROUTER_SNAT,
False)
mock_getsub.assert_called_with(
self.agent.context, mock.ANY, fixed_ips=None)
def test_port_bound_for_dvr_with_csnat_ports_ofport_change(self): def test_port_bound_for_dvr_with_csnat_ports_ofport_change(self):
self._setup_for_dvr_test() self._setup_for_dvr_test()
self._port_bound_for_dvr_with_csnat_ports() self._port_bound_for_dvr_with_csnat_ports()