[Starlingx-discuss] Analysis of patch 88b7bc7 for StartlingX upstreaming

[Peters, Matt]

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 at intel.com>
Date: Thursday, November 29, 2018 at 10:29 AM
To: "Peters, Matt" <Matt.Peters at windriver.com>
Cc: "starlingx-discuss at lists.starlingx.io" <starlingx-discuss at 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 at windriver.com]
Sent: Thursday, November 29, 2018 8:59 PM
To: Xu, Chenjie <chenjie.xu at intel.com>
Cc: starlingx-discuss at 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 at intel.com<mailto:chenjie.xu at intel.com>>
Date: Wednesday, November 28, 2018 at 9:22 PM
To: "Peters, Matt" <Matt.Peters at windriver.com<mailto:Matt.Peters at windriver.com>>
Cc: "starlingx-discuss at lists.starlingx.io<mailto:starlingx-discuss at lists.starlingx.io>" <starlingx-discuss at lists.starlingx.io<mailto:starlingx-discuss at 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 at windriver.com]
Sent: Thursday, November 29, 2018 1:43 AM
To: Xu, Chenjie <chenjie.xu at intel.com<mailto:chenjie.xu at intel.com>>
Cc: starlingx-discuss at lists.starlingx.io<mailto:starlingx-discuss at 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 at intel.com<mailto:chenjie.xu at intel.com>>
Date: Wednesday, November 28, 2018 at 10:11 AM
To: "Peters, Matt" <Matt.Peters at windriver.com<mailto:Matt.Peters at windriver.com>>
Cc: "starlingx-discuss at lists.starlingx.io<mailto:starlingx-discuss at lists.starlingx.io>" <starlingx-discuss at lists.starlingx.io<mailto:starlingx-discuss at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.starlingx.io/pipermail/starlingx-discuss/attachments/20181210/b8123e13/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RFE_ADD_RPC_QUERY_API_TO_L2POP_FOR_FDB_RESYNC-MP.docx
Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document
Size: 19178 bytes
Desc: RFE_ADD_RPC_QUERY_API_TO_L2POP_FOR_FDB_RESYNC-MP.docx
URL: <http://lists.starlingx.io/pipermail/starlingx-discuss/attachments/20181210/b8123e13/attachment-0001.docx>