Attached is a copy with my revisions.  There were no major changes, just some minor corrections and additions.

Overall looks good.

 

Thanks, Matt

 

From: "Xu, Chenjie" <chenjie.xu@intel.com>
Date: Thursday, November 29, 2018 at 10:29 AM
To: "Peters, Matt" <Matt.Peters@windriver.com>
Cc: "starlingx-discuss@lists.starlingx.io" <starlingx-discuss@lists.starlingx.io>
Subject: RE: [Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

 

Hi Matt,

Thanks for your reply! Yes, we can fix this bug as part of the upstreaming. The code can be viewed by following link:

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

Please help comment and fix the bug. The RFE for patch 88b7bc7 is attached. Could you please help review and comment?

 

Best Regards,

Xu, Chenjie

From: Peters, Matt [mailto:Matt.Peters@windriver.com]
Sent: Thursday, November 29, 2018 8:59 PM
To: Xu, Chenjie <chenjie.xu@intel.com>
Cc: starlingx-discuss@lists.starlingx.io
Subject: Re: [Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

 

Hi Chenjie,

Since this isn’t causing a runtime issue at the moment, I think this can be fixed as part of the upstreaming where it can be reviewed in Gerrit.  If you are looking for confirmation of the changes prior to that I would create a pull request so it can be reviewed/merged prior.

 

-Matt

 

From: "Xu, Chenjie" <chenjie.xu@intel.com>
Date: Wednesday, November 28, 2018 at 9:22 PM
To: "Peters, Matt" <Matt.Peters@windriver.com>
Cc: "starlingx-discuss@lists.starlingx.io" <starlingx-discuss@lists.starlingx.io>
Subject: RE: [Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

 

Hi Matt,

Thanks for your comments! An RFE has been written to upstream patch 88b7bc7. Before I submit the code, I think we should fix this bug. I think we can change the parameter “host” to “agent” in the following lines:

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/rpc_manager/l2population_rpc.py#L44

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/rpc_manager/l2population_rpc.py#L57

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/mech_driver.py#L444

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/mech_driver.py#L434

Please let me know what you think of the proposal.

 

Best Regards,

Xu, Chenjie

 

From: Peters, Matt [mailto:Matt.Peters@windriver.com]
Sent: Thursday, November 29, 2018 1:43 AM
To: Xu, Chenjie <chenjie.xu@intel.com>
Cc: starlingx-discuss@lists.starlingx.io
Subject: Re: [Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

 

Hello Chenjie,

I agree, this is a bug.  I think the only reason it hasn’t been an issue is because the only user of the get_fdb_entries RPC is the BGP agent and it is not passing a host in the request (it is None), therefore it excludes the attempt to try and access the parameter as an agent based on the condition in _create_agent_fdb.

 

Here is the BGP caller:

https://github.com/starlingx-staging/stx-neutron-dynamic-routing/blob/master/neutron_dynamic_routing/services/bgp/agent/bgp_dragent.py#L1498

 

 

From: "Xu, Chenjie" <chenjie.xu@intel.com>
Date: Wednesday, November 28, 2018 at 10:11 AM
To: "Peters, Matt" <Matt.Peters@windriver.com>
Cc: "starlingx-discuss@lists.starlingx.io" <starlingx-discuss@lists.starlingx.io>
Subject: [Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

 

 

Hi Matt,

I think that there exists a bug in patch 88b7bc7. Function _get_fdb_entries will call function _create­_agent_fdb by following line:

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/mech_driver.py#L441

The variable “host” will be used to call _create_agent_fdb. However function _create_agent_fdb expects variable “agent”:

https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins/ml2/drivers/l2pop/mech_driver.py#L264

I think this should be a bug. Could you please help review and comment?

 

Best Regards,

Xu, Chenjie