| Message ID | 1524015646-12855-1-git-send-email-ashishvarma.ovs@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] ovs-ofctl: Fixed the "snoop" command of ovs-ofctl | expand |
On Tue, Apr 17, 2018 at 06:40:46PM -0700, Ashish Varma wrote: > In normal ovs-ofctl commands (e.g. add-flow), ovs-ofctl connects to > ovs-vswitchd process on “<ovs_rundir()>/<bridge_name>.mgmt” unix socket. > In an output that contains a port or table, port name or table name can be > displayed, instead of their numbers, if the user turns on this feature. > (by -—names option) Also, this feature is enabled when interacting with a user > on console. To fetch the names, ovs-ofctl sends TABLE FEATURE / > PORT DESCRIPTION request message to ovs-vswitchd process and expects a reply > on the connection. > When ovs-ofctl runs the snoop command, it connects to > “<ovs_rundir()>/<bridge_name>.snoop” unix socket. ovs-vswitchd process would > not reply to the TABLE FEATURE / PORT DESCRIPTION request message on this > connection. It would only send any open flow message it receives from the > controller. > When using port/table name feature with snoop command, the print of open flow > message would call ‘tables_to_show()’/‘ports_to_show()’ which in turn would > send the request message on the snoop connection. ovs-vswitchd would not reply > back on this connection, but instead would keep sending the open flow messages > received from controller. ‘table_iterator_next()/port_iterator_next()’ function > would loop for ever waiting for response. > The fix for this is to turn off display of table/port names when running > snoop command. > > Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> > --- > v1-v2 > > Added more description to the cause of the issue and reason to add the fix. Thanks for the update. I think I didn't describe my request on v1 very well. I thought that the commit message was fine; it described the problem and the solution well. I was asking for the comment in the code to describe the problem that it solved. As is, the following comment: > + use_names = 0; /* don't show port and table names */ explains the immediate effect, but not why. The "why" is more important than the immediate effect, because the effect is pretty easy to understand if the reader looks at the comments on the definition of "use_names", but the reason is nonobvious. Thanks, Ben.
Sure, I will update the comment in the code to explain the effect of using: "use_names = 0". Also, would change the comment in the commit back to the original message. Thanks, On Wed, Apr 18, 2018 at 4:29 PM, Ben Pfaff <blp@ovn.org> wrote: > On Tue, Apr 17, 2018 at 06:40:46PM -0700, Ashish Varma wrote: > > In normal ovs-ofctl commands (e.g. add-flow), ovs-ofctl connects to > > ovs-vswitchd process on “<ovs_rundir()>/<bridge_name>.mgmt” unix socket. > > In an output that contains a port or table, port name or table name can > be > > displayed, instead of their numbers, if the user turns on this feature. > > (by -—names option) Also, this feature is enabled when interacting with > a user > > on console. To fetch the names, ovs-ofctl sends TABLE FEATURE / > > PORT DESCRIPTION request message to ovs-vswitchd process and expects a > reply > > on the connection. > > When ovs-ofctl runs the snoop command, it connects to > > “<ovs_rundir()>/<bridge_name>.snoop” unix socket. ovs-vswitchd process > would > > not reply to the TABLE FEATURE / PORT DESCRIPTION request message on this > > connection. It would only send any open flow message it receives from the > > controller. > > When using port/table name feature with snoop command, the print of open > flow > > message would call ‘tables_to_show()’/‘ports_to_show()’ which in turn > would > > send the request message on the snoop connection. ovs-vswitchd would not > reply > > back on this connection, but instead would keep sending the open flow > messages > > received from controller. ‘table_iterator_next()/port_iterator_next()’ > function > > would loop for ever waiting for response. > > The fix for this is to turn off display of table/port names when running > > snoop command. > > > > Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> > > --- > > v1-v2 > > > > Added more description to the cause of the issue and reason to add the > fix. > > Thanks for the update. > > I think I didn't describe my request on v1 very well. I thought that > the commit message was fine; it described the problem and the solution > well. I was asking for the comment in the code to describe the problem > that it solved. As is, the following comment: > > + use_names = 0; /* don't show port and table names */ > explains the immediate effect, but not why. The "why" is more important > than the immediate effect, because the effect is pretty easy to > understand if the reader looks at the comments on the definition of > "use_names", but the reason is nonobvious. > > Thanks, > > Ben. >
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 6708b07..3023787 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2326,6 +2326,7 @@ ofctl_snoop(struct ovs_cmdl_context *ctx) { struct vconn *vconn; + use_names = 0; /* don't show port and table names */ open_vconn__(ctx->argv[1], SNOOP, &vconn); monitor_vconn(vconn, false, false); }
In normal ovs-ofctl commands (e.g. add-flow), ovs-ofctl connects to ovs-vswitchd process on “<ovs_rundir()>/<bridge_name>.mgmt” unix socket. In an output that contains a port or table, port name or table name can be displayed, instead of their numbers, if the user turns on this feature. (by -—names option) Also, this feature is enabled when interacting with a user on console. To fetch the names, ovs-ofctl sends TABLE FEATURE / PORT DESCRIPTION request message to ovs-vswitchd process and expects a reply on the connection. When ovs-ofctl runs the snoop command, it connects to “<ovs_rundir()>/<bridge_name>.snoop” unix socket. ovs-vswitchd process would not reply to the TABLE FEATURE / PORT DESCRIPTION request message on this connection. It would only send any open flow message it receives from the controller. When using port/table name feature with snoop command, the print of open flow message would call ‘tables_to_show()’/‘ports_to_show()’ which in turn would send the request message on the snoop connection. ovs-vswitchd would not reply back on this connection, but instead would keep sending the open flow messages received from controller. ‘table_iterator_next()/port_iterator_next()’ function would loop for ever waiting for response. The fix for this is to turn off display of table/port names when running snoop command. Signed-off-by: Ashish Varma <ashishvarma.ovs@gmail.com> --- v1-v2 Added more description to the cause of the issue and reason to add the fix. --- utilities/ovs-ofctl.c | 1 + 1 file changed, 1 insertion(+)