<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"Segoe UI";
panose-1:2 11 5 2 4 2 4 2 2 3;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:#0563C1;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0in;
margin-right:0in;
margin-bottom:0in;
margin-left:.5in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle19
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle21
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle23
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle24
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle25
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle26
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle27
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle28
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle29
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle30
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle31
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle32
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle33
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:windowtext;}
span.EmailStyle34
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle35
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle36
{mso-style-type:personal;
font-family:"Calibri",sans-serif;
color:#1F497D;}
span.EmailStyle37
{mso-style-type:personal-compose;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="#0563C1" vlink="#954F72">
<div class="WordSection1">
<p class="MsoNormal"><span style="color:#1F497D">Matt,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks much for the comments! Some comments for upstream in line, please help to review, thanks!<o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D"><o:p> </o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D">Best Regards,<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D">Le, Huifeng</span></i></b><span style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="color:#1F497D"><o:p> </o:p></span></a></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><a name="_____replyseparator"></a><b>From:</b> Peters, Matt [mailto:Matt.Peters@windriver.com]
<br>
<b>Sent:</b> Friday, September 7, 2018 11:01 PM<br>
<b>To:</b> Le, Huifeng <huifeng.le@intel.com>; Jolliffe, Ian <Ian.Jolliffe@windriver.com>; Jones, Bruce E <bruce.e.jones@intel.com>; Rowsell, Brent <Brent.Rowsell@windriver.com><br>
<b>Cc:</b> Zhao, Forrest <forrest.zhao@intel.com>; Troyer, Dean <dean.troyer@intel.com>; starlingx-discuss@lists.starlingx.io<br>
<b>Subject:</b> RE: Analysis report about Network Trunk feature for StartlingX upstreaming<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:#1F497D">See inline for specific responses to the review information</span><i><span lang="EN-CA" style="color:#1F497D">.<o:p></o:p></span></i></p>
<p class="MsoNormal"><i><span lang="EN-CA" style="color:#1F497D">Responses marked with [MP>]</span></i><span style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma",sans-serif">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma",sans-serif"> Le, Huifeng [<a href="mailto:huifeng.le@intel.com">mailto:huifeng.le@intel.com</a>]
<br>
<b>Sent:</b> Sunday, August 19, 2018 10:32 PM<br>
<b>To:</b> Jolliffe, Ian; Jones, Bruce E; Rowsell, Brent; Peters, Matt<br>
<b>Cc:</b> Zhao, Forrest; Troyer, Dean; <a href="mailto:starlingx-discuss@lists.starlingx.io">
starlingx-discuss@lists.starlingx.io</a><br>
<b>Subject:</b> RE: Analysis report about Network Trunk feature for StartlingX upstreaming<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="color:#1F497D">Ian,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D">Thanks very much for the comments. some comments below for you reference, and please help to review, thanks much!<o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D"><o:p> </o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D">Best Regards,<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span style="color:#1F497D">Le, Huifeng</span></i></b><span style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Jolliffe, Ian [<a href="mailto:Ian.Jolliffe@windriver.com">mailto:Ian.Jolliffe@windriver.com</a>]
<br>
<b>Sent:</b> Saturday, August 18, 2018 4:16 AM<br>
<b>To:</b> Le, Huifeng <<a href="mailto:huifeng.le@intel.com">huifeng.le@intel.com</a>>; Jones, Bruce E <<a href="mailto:bruce.e.jones@intel.com">bruce.e.jones@intel.com</a>>; Rowsell, Brent <<a href="mailto:Brent.Rowsell@windriver.com">Brent.Rowsell@windriver.com</a>>;
Peters, Matt <<a href="mailto:Matt.Peters@windriver.com">Matt.Peters@windriver.com</a>><br>
<b>Cc:</b> Zhao, Forrest <<a href="mailto:forrest.zhao@intel.com">forrest.zhao@intel.com</a>>; Troyer, Dean <<a href="mailto:dean.troyer@intel.com">dean.troyer@intel.com</a>>;
<a href="mailto:starlingx-discuss@lists.starlingx.io">starlingx-discuss@lists.starlingx.io</a><br>
<b>Subject:</b> Re: Analysis report about Network Trunk feature for StartlingX upstreaming<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><a name="_MailOriginalBody"><span lang="EN-CA" style="font-size:12.0pt">Hi Huifeng;</span></a><span lang="EN-CA" style="font-size:12.0pt"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="font-size:12.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="font-size:12.0pt">Thanks for the updates/analysis, comments below.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="font-size:12.0pt"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="font-size:12.0pt">Ian<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">Ian/Brent/Matt,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">We did analysis about the Network trunk related patches for StartingX upstream, below are the suggestions for upstreaming, could you please help to review and comment? Thanks much!<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">1. ba9d9f60a7a2665194cacb92a05e0acd2dc3de41: Add rpc notification for trunk updates<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Function</b>: sent notification to the agent when a trunk is updated<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Analysis</b>:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> (1)Trunk’s AFTER_UPDATE event is generated for API call: PUT /v2.0/trunks/{trunk-id}<o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:.25in"><i><span lang="EN" style="font-size:10.0pt;font-family:"Segoe UI",sans-serif;color:#333333">The update request is only for changing fields like name, description or admin_state_up. Setting the admin_state_up to
False locks the trunk in that it prevents operations such as adding/removing subports.</span></i><span lang="EN-CA"><o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:.25in"><span lang="EN-CA">In Neutron upstream, admin_state_up is used in server side, e.g. add_subports, remove subports, delete_trunk and not used in agent side<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> (2)OVS trunk agent driver uses OVSDB event to handle trunk event, no need to manually trigger trunk update event<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> (3)Linux trunk agent driver will handle trunk update event triggered by server, while it will need apply the patch only in case admin_state_up update need to be handled<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Suggestion: </b>Not a bug for Neutron upstream, suggest not to upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">If this is not upstreamed, are the dependencies or changes required in the StarlingX code base? What are the implications of not upstreaming?<o:p></o:p></span></p>
<p class="MsoNormal"><b><span lang="EN-CA" style="color:#0070C0">[hle2]</span></b><span lang="EN-CA" style="color:#0070C0"> for STX, trunk_updated event will force the trunk’s parent-port to refresh (e.g. handle_trunks->mark_port_for_refresh(trunk['port_id'])
etc.) to get the new “admin_state_up” value from server and this value will be used in handle_updated_port() to determine whether it is allowed to update port/device status in server side.
<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0">“admin_state_up” is mainly used to control operation at neutron server side like add_subports, remove subports, delete_trunk etc. and all these 3 operations will force port to refresh (handle_trunks/handle_subports->mark_port_for_refresh),
so suppose, the general flow will not be impacted whether to handle trunk_updated event or not.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0">But in some wired cases, add “admin_state_up” check in agent side may cause issues (<span style="background:yellow;mso-highlight:yellow">please help to review whether it is make sense</span>), e.g
for below calling flow (suppose trunk’s ‘admin_state_up’ is ‘up’): (1) add_subports (2) set “admin_state_up” to ‘down’, step(1) may fail to set device’s state at agent side in case aws agent’s handle_updated_port() (in daemon loop) executed after step (2)
<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0">So to my understanding:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0">(1) if using OVS agent in STX, no impact for not upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#0070C0">(2) if using AVS agent + STX, suggest removing “admin_state_up” check in AVS agent (in function handle_updated_port () of avs/agent.py) like below.<o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:.5in"><i><span lang="EN-CA" style="font-size:10.0pt">if trunk_details
<s>and trunk_details['admin_state_up']</s>:<o:p></o:p></span></i></p>
<p class="MsoNormal" style="text-indent:.5in"><i><span lang="EN-CA" style="font-size:10.0pt"> …<o:p></o:p></span></i></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA" style="color:#1F497D">[MP>] Thanks for the detailed analysis. The suggested change will need to be tested to see if it full resolves the original issue. However, since the driver could be used by other agents
(beyond just the AVS agent), I think it would not hurt to have the additional notification sent to complete the driver definition for the set of RPC notifications.
<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span lang="EN-CA" style="color:#00B050">[hle2] yes, it is not hurt to have additional notification sent to complete the driver definition for PRC notification.
<o:p></o:p></span></b></p>
<p class="MsoNormal"><b><span lang="EN-CA" style="color:#00B050">And do you think it is need for upstream in case it is not a bug for OVS and linux bridge driver? if yes, do you have any strong reason which can convince community to buy in the change? Thanks!<o:p></o:p></span></b></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">2. 6955351c5eca6e37061fb0140d11ea53693fe0e1: Add support to delete bound network<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Function</b>: enable delete trunk if it is can_be_trunked (not bounded or driver’s can_trunk_bound_port=true)<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Analysis</b>: Applied for LinuxBridge Driver and AVS bridge Driver (can_trunk_bound_port=True), no impact for OVSTrunkDriver (can_trunk_bound_port=False). workaround also available for linux bridge (e.g. unbind
the port first then delete the trunk)<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Suggestion</b>: it is a low priority bug for Neutron upstream (only applied for linux bridge and workround available), suggest not to upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">I think you need to propose a fix. Or this will need to be carried long term.<o:p></o:p></span></p>
<p class="MsoNormal"><b><span lang="EN-CA" style="color:#0070C0">[hle2]</span></b><span lang="EN-CA" style="color:#1F497D">
</span><span lang="EN-CA" style="color:#0070C0">yes, let’s try to propose a fix for upstream.</span><span lang="EN-CA" style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA" style="color:#1F497D">[MP>] Agree.<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><b><span lang="EN-CA" style="color:#00B050">[hle2] Since this issue is not applied for OVS/OVS DPDK, suggest to put it as low priority and try to solve it after other high issues. make sense?
<o:p></o:p></span></b></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#00B050"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">3. 43a684946e781a25d21a4f50b8dc67d61be42809: Enable trunk service by default<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Function</b>: add “trunk” in DEFAULT_SERVICE_PLUGINS<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Analysis</b>: It is a deploy configuration for downstream product<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Suggestion</b>: Not a bug for Neutron upstream, suggest not to upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">Agree<o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA" style="color:#1F497D">[MP>] Agree.</span></i></b><span lang="EN-CA" style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">4. c54d804792f10b7f505de6794274c4df4768f6f0: Include trunk presence in port details<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Function</b>: add trunk_port (bool) flag in port_details to identify whether this port is a parent port for a trunk<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Analysis</b>: It is a performance improvement for AVS agent by reducing RPC call from agent to server. OVS agent has different implementation with no improvement by introducing this field<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Suggestion</b>: Not a bug for Neutron upstream, suggest not to upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">Agree<o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA" style="color:#1F497D">[MP>] Agree.</span></i></b><span lang="EN-CA" style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">5. 3eed837ebd236e6b1959ea88d9ab5322c9eef6b9: Ignore trunk subports on same vlan as vlan-subnet ports<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Function</b>: Ignore trunk subports on same vlan as vlan-subnet ports<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Analysis</b>: It is a bug fix for AVS agent<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"> <b>Suggestion</b>: Not a bug for Neutron upstream, suggest not to upstream<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA">Agree<o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA" style="color:#1F497D">[MP>] Agree.</span></i></b><span lang="EN-CA" style="color:#1F497D"><o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA"><o:p> </o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span lang="EN-CA"><o:p> </o:p></span></i></b></p>
<p class="MsoNormal"><b><i><span lang="EN-CA"> </span></i></b><span lang="EN-CA"><o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA">Best Regards,</span></i></b><span lang="EN-CA"><o:p></o:p></span></p>
<p class="MsoNormal"><b><i><span lang="EN-CA">Le, Huifeng</span></i></b><span lang="EN-CA"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#1F497D"> </span><span lang="EN-CA"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-CA" style="color:#1F497D"> </span><span lang="EN-CA"><o:p></o:p></span></p>
</div>
</div>
</body>
</html>