[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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
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... 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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
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... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... 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... From: "Xu, Chenjie" <chenjie.xu@intel.com<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 10:11 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
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... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... 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... From: "Xu, Chenjie" <chenjie.xu@intel.com<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 10:11 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
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<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 9:22 PM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... 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<mailto:chenjie.xu@intel.com>> Cc: starlingx-discuss@lists.starlingx.io<mailto: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... From: "Xu, Chenjie" <chenjie.xu@intel.com<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 10:11 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
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<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 9:22 PM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... 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<mailto:chenjie.xu@intel.com>> Cc: starlingx-discuss@lists.starlingx.io<mailto: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... From: "Xu, Chenjie" <chenjie.xu@intel.com<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 10:11 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
Hi Matt, Thank you so much for your revisions! The RFE has been submitted to the Launchpad. The link is below: https://bugs.launchpad.net/neutron/+bug/1806316 I will try to discuss this RFE on OpenStack Neutron Driver Meeting. Best Regards, Xu, Chenjie From: Peters, Matt [mailto:Matt.Peters@windriver.com] Sent: Monday, December 10, 2018 10:37 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 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<mailto:chenjie.xu@intel.com>> Date: Thursday, November 29, 2018 at 10:29 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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<mailto:chenjie.xu@intel.com>> Cc: starlingx-discuss@lists.starlingx.io<mailto: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<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 9:22 PM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... https://github.com/starlingx-staging/stx-neutron/blob/master/neutron/plugins... 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<mailto:chenjie.xu@intel.com>> Cc: starlingx-discuss@lists.starlingx.io<mailto: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... From: "Xu, Chenjie" <chenjie.xu@intel.com<mailto:chenjie.xu@intel.com>> Date: Wednesday, November 28, 2018 at 10:11 AM To: "Peters, Matt" <Matt.Peters@windriver.com<mailto:Matt.Peters@windriver.com>> Cc: "starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io>" <starlingx-discuss@lists.starlingx.io<mailto: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... 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... I think this should be a bug. Could you please help review and comment? Best Regards, Xu, Chenjie
participants (2)
-
Peters, Matt
-
Xu, Chenjie