Re: [ovs-dev] Is there a way to get port status directly on kernel module?

2014-02-20 Thread Rafael Duarte Vencioneck
Thanks! I didn't know about the existence of megaflows feature.
I'll search more on that topic.
If you have some hints, they will be welcome.

best regards


Rafael Duarte Vencioneck


2014-02-19 17:48 GMT-03:00 Ben Pfaff :

> On Wed, Feb 19, 2014 at 05:12:22PM -0300, Rafael Duarte Vencioneck wrote:
> > I'm thinking about the possibility of fast forwarding without upcalls. If
> > the link is down, maybe the module could know someway for what port to
> > forward.
> > It's really impossible to do that?
>
> It might be possible.  With the "megaflows" feature introduced in
> 1.11, it's probably not worthwhile, because only a small number of
> datapath flows have to be updated, most of the time, to redirect all
> of the traffic to the new port.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] tests/run-ryu: Correct logfile reporting

2014-02-20 Thread Ben Pfaff
On Thu, Feb 20, 2014 at 01:48:10PM +0900, Simon Horman wrote:
> $logfile is already prefixed by "$sandbox/" and suffixed by ".log"
> so do not duplicate this prefix and suffix combination when appending
> $logfile to $logs.
> 
> Cc: YAMAMOTO Takashi 
> Signed-off-by: Simon Horman 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofp-util: Avoid gcc warning on NetBSD

2014-02-20 Thread Ben Pfaff
On Thu, Feb 20, 2014 at 11:11:00AM +0900, YAMAMOTO Takashi wrote:
> Avoid the following warning caused by the combination
> of NetBSD's htons implementation and gcc bug.
> Now --enable-Werror build succeeds on NetBSD-6.
> 
> lib/ofp-util.c: In function 'ofputil_match_from_ofp10_match':
> lib/ofp-util.c:174:15: error: integer overflow in expression
> 
> Signed-off-by: YAMAMOTO Takashi 

Applied, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] ONF Extension for OF1.3.X

2014-02-20 Thread Nipun Gupta
Hi everyone,

I am CS Dual Degree (B.Tech+M.Tech) student in IIT Delhi, currently in my
3rd year. I am looking forward to work on a 6-7 week project, along with a
batchmate of mine in a team of two, to extend OVS with OF1.3+ specs.
So we looked at some extensions required for OF1.4 and got interested in
Flow Monitoring, Role Status Events, and Group and Meter Change
Notifications.

We would like to ask if anyone is already working on any of these features?
Also, any useful information that would help us through this implementation
would be highly appreciated.

The expected amount of work from our side would be almost 10 hours per
week. Do consider that we are presently unfamiliar with OVS's code, but we
do understand OF and OVS and also have some experience with Mininet. So,
suggestions are welcome on what extensions we should pursue in the given
amount of time.

Thank You


Nipun Gupta
Computer Science and Engineering
IIT Delhi
India
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-discuss] ONF Extension for OF1.3.X

2014-02-20 Thread Ben Pfaff
On Thu, Feb 20, 2014 at 11:44:00PM +0530, Nipun Gupta wrote:
> I am CS Dual Degree (B.Tech+M.Tech) student in IIT Delhi, currently in my
> 3rd year. I am looking forward to work on a 6-7 week project, along with a
> batchmate of mine in a team of two, to extend OVS with OF1.3+ specs.
> So we looked at some extensions required for OF1.4 and got interested in
> Flow Monitoring, Role Status Events, and Group and Meter Change
> Notifications.
> 
> We would like to ask if anyone is already working on any of these features?
> Also, any useful information that would help us through this implementation
> would be highly appreciated.

I do not know of anyone working on these features.

Flow monitoring is likely to be pretty simple, because OVS already
supports this feature, but not the protocol format the OpenFlow 1.3+
uses for it.  That means that the project would just be to add new code
to translate to and from the OpenFlow 1.3+ format.

Role status events are a very small project.

Group and meter change notifications are probably a medium size project.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-discuss] ONF Extension for OF1.3.X

2014-02-20 Thread Nipun Gupta
Thanks for the suggestion.

Also, we'd like to know about Bundles extensions. It seemed pretty
interesting too.

Nipun Gupta
Computer Science and Engineering
IIT Delhi
India


On Fri, Feb 21, 2014 at 1:29 AM, Ben Pfaff  wrote:

> On Thu, Feb 20, 2014 at 11:44:00PM +0530, Nipun Gupta wrote:
> > I am CS Dual Degree (B.Tech+M.Tech) student in IIT Delhi, currently in my
> > 3rd year. I am looking forward to work on a 6-7 week project, along with
> a
> > batchmate of mine in a team of two, to extend OVS with OF1.3+ specs.
> > So we looked at some extensions required for OF1.4 and got interested in
> > Flow Monitoring, Role Status Events, and Group and Meter Change
> > Notifications.
> >
> > We would like to ask if anyone is already working on any of these
> features?
> > Also, any useful information that would help us through this
> implementation
> > would be highly appreciated.
>
> I do not know of anyone working on these features.
>
> Flow monitoring is likely to be pretty simple, because OVS already
> supports this feature, but not the protocol format the OpenFlow 1.3+
> uses for it.  That means that the project would just be to add new code
> to translate to and from the OpenFlow 1.3+ format.
>
> Role status events are a very small project.
>
> Group and meter change notifications are probably a medium size project.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [ovs-discuss] ONF Extension for OF1.3.X

2014-02-20 Thread Ben Pfaff
On Fri, Feb 21, 2014 at 02:12:28AM +0530, Nipun Gupta wrote:
> Also, we'd like to know about Bundles extensions. It seemed pretty
> interesting too.

That one is a very big project.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto: Send port status message for port-mods, right away.

2014-02-20 Thread Ben Pfaff
On Thu, Feb 20, 2014 at 12:45:49PM +0800, Kmindg G wrote:
> On Thu, Feb 20, 2014 at 3:20 AM, Ben Pfaff  wrote:
> > Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has waited
> > for ports to notify it that their status has changed before it sends a
> > port status update to controllers.
> >
> > Also, Open vSwitch never sent port config updates at all for port
> > modifications other than OFPPC_PORT_DOWN.  I guess that this is a relic
> > from the era when there was only one controller, since presumably the
> > controller already knew that it changed the port configuration.  But in the
> > multi-controller world, it makes sense to send such port status updates,
> > and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
> > shouldn't be sent.
> >
> > Signed-off-by: Ben Pfaff 
> > Reported-by: Kmindg G 
> > ---
> >  AUTHORS   |1 +
> >  ofproto/ofproto.c |   25 +
> >  2 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/AUTHORS b/AUTHORS
> > index c557303..2fda8d7 100644
> > --- a/AUTHORS
> > +++ b/AUTHORS
> > @@ -197,6 +197,7 @@ John Hurley john.hur...@netronome.com
> >  Kevin Mancuso   kevin.manc...@rackspace.com
> >  Kiran Shanbhog  ki...@vmware.com
> >  Kirill Kabardin
> > +Kmindg Gkmi...@gmail.com
> >  Koichi Yagishitayagishita.koi...@jrc.co.jp
> >  Konstantin Khorenko khore...@openvz.org
> >  Kris zhang  zhang.k...@gmail.com
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index 62c97a2..48e10ca 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -2986,22 +2986,23 @@ update_port_config(struct ofport *port,
> > enum ofputil_port_config config,
> > enum ofputil_port_config mask)
> >  {
> > -enum ofputil_port_config old_config = port->pp.config;
> > -enum ofputil_port_config toggle;
> > -
> > -toggle = (config ^ port->pp.config) & mask;
> > -if (toggle & OFPUTIL_PC_PORT_DOWN) {
> > -if (config & OFPUTIL_PC_PORT_DOWN) {
> > -netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL);
> > -} else {
> > -netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL);
> > -}
> > +enum ofputil_port_config toggle = (config ^ port->pp.config) & mask;
> > +
> > +if (toggle & OFPUTIL_PC_PORT_DOWN
> > +&& (config & OFPUTIL_PC_PORT_DOWN
> > +? netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL)
> > +: netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL))) {
> > +/* We tried to bring the port up or down, but it failed, so don't
> > + * update the "down" bit. */
> >  toggle &= ~OFPUTIL_PC_PORT_DOWN;
> >  }
> >
> > -port->pp.config ^= toggle;
> > -if (port->pp.config != old_config) {
> > +if (toggle) {
> > +enum ofputil_port_config old_config = port->pp.config;
> > +port->pp.config ^= toggle;
> >  port->ofproto->ofproto_class->port_reconfigured(port, old_config);
> > +connmgr_send_port_status(port->ofproto->connmgr, &port->pp,
> > + OFPPR_MODIFY);
> >  }
> >  }
> >
> > --
> > 1.7.10.4
> >
> 
> looks good to me.

Thanks.  What do you think of this version?  I think that it retains
better compatibility with OpenFlow 1.0 in particular.

--8<--cut here-->8--

From: Ben Pfaff 
Date: Thu, 20 Feb 2014 13:18:51 -0800
Subject: [PATCH] ofproto: Send port status message for port-mods, right away.

Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has waited
for ports to notify it that their status has changed before it sends a
port status update to controllers.

Also, Open vSwitch never sent port config updates at all for port
modifications other than OFPPC_PORT_DOWN.  I guess that this is a relic
from the era when there was only one controller, since presumably the
controller already knew that it changed the port configuration.  But in the
multi-controller world, it makes sense to send such port status updates,
and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
shouldn't be sent.

Signed-off-by: Ben Pfaff 
Reported-by: Kmindg G 
---
 ofproto/connmgr.c | 30 --
 ofproto/connmgr.h |  4 ++--
 ofproto/ofproto.c | 38 --
 3 files changed, 50 insertions(+), 22 deletions(-)

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index a58e785..7a25828 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1456,9 +1456,11 @@ static void schedule_packet_in(struct ofconn *, struct 
ofproto_packet_in,
enum ofp_packet_in_reason wire_reason);
 
 /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
- * controllers managed by 'mgr'. */
+ * controllers managed by 'mgr'.  For messages caused by a controller
+ * OFPT_PORT_MOD, specify 'source'

Re: [ovs-dev] [PATCH] Windows implementation of stream-fd.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 12, 2014 at 01:50:47PM -0800, Linda Sun wrote:
> Use send/recv for socket stream instead of read/write.
> Use event handle for polling on socket stream.
> Check windows specific return code.
> 
> Signed-off-by: Linda Sun 

Guru, will you review this (and apply it if you are happy)?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 01/14] socket-util: Move sock_errno() to socket-util.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:12PM -0800, Gurucharan Shetty wrote:
> And add more users.
> 
> Signed-off-by: Gurucharan Shetty 

Bitwise operators don't make sense to me here:
> +if (error == EINPROGRESS
> +#ifdef _WIN32
> +| WSAEALREADY | WSAEWOULDBLOCK
> +#endif

Otherwise,
Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 02/14] socket-util: set_nonblocking for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:13PM -0800, Gurucharan Shetty wrote:
> Co-authored-by: Linda Sun 
> Signed-off-by: Linda Sun 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 03/14] socket-util: Maximum file descriptors for windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:14PM -0800, Gurucharan Shetty wrote:
> Windows does not have a getrlimit() function. As such,
> there is no limit to number of file descriptors that
> can be opened. So, set an aritificial limit of 1024.
> This may be increased in the future if the limit is too
> low.
> 
> Co-authored-by: Linda Sun 
> Signed-off-by: Linda Sun 
> Signed-off-by: Gurucharan Shetty 

get_max_fds() is only used by process_start() in process.c and is only
relevant for the Unix "fork" model of creating processes, not for
Windows.  I'd rather move the function to process.c and #ifdef it out
entirely for Windows.  How does that sound?

Thanks,

Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 04/14] socket-util: inet_aton for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:15PM -0800, Gurucharan Shetty wrote:
> Windows does not have a inet_aton(). But does have a
> inet_pton().
> 
> Signed-off-by: Gurucharan Shetty 

inet_aton() isn't in POSIX, but inet_pton() is.  I think it's better if
we just change every use of inet_aton() to use inet_pton() instead.
(There's only a few.)  I think all it takes is changing the function
name and adding AF_INET at the beginning.

What do you think?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 05/14] socket-util: getaddrinfo return values for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:16PM -0800, Gurucharan Shetty wrote:
> Couple of return values need changes.
> * EAI_NODATA is the same as EAI_NONAME. So we prevent duplicate cases.
> * Windows does not have a EAI_SYSTEM.
> 
> Signed-off-by: Gurucharan Shetty 

To make this a little less Win32 specific, can we use "#if
defined(EAI_NODATA) && EAI_NODATA != EAI_NONAME" instead of adding the
#ifndef _WIN32?

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/2] ovs-vswitchd: Don't register for SIGHUP.

2014-02-20 Thread Gurucharan Shetty
We are registering an interest in SIGHUP to reopen
log files. But there is an 'ovs-appctl vlog/reopen'
command that does the same and is used in the
logrotate config for the distributions.

So remove the redundant functionality.

Signed-off-by: Gurucharan Shetty 
---
 vswitchd/ovs-vswitchd.8.in |3 ---
 vswitchd/ovs-vswitchd.c|7 ---
 2 files changed, 10 deletions(-)

diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
index d2544f7..e9dc483 100644
--- a/vswitchd/ovs-vswitchd.8.in
+++ b/vswitchd/ovs-vswitchd.8.in
@@ -30,9 +30,6 @@ switching across each bridge described in its configuration 
files.  As
 the database changes, \fBovs\-vswitchd\fR automatically updates its
 configuration to match.
 .PP
-Upon receipt of a SIGHUP signal, \fBovs\-vswitchd\fR reopens its log
-file, if one was specified on the command line.
-.PP
 \fBovs\-vswitchd\fR switches may be configured with any of the following
 features:
 .
diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
index 9da2f49..c4e61d1 100644
--- a/vswitchd/ovs-vswitchd.c
+++ b/vswitchd/ovs-vswitchd.c
@@ -38,7 +38,6 @@
 #include "ovsdb-idl.h"
 #include "poll-loop.h"
 #include "process.h"
-#include "signals.h"
 #include "simap.h"
 #include "stream-ssl.h"
 #include "stream.h"
@@ -66,7 +65,6 @@ main(int argc, char *argv[])
 {
 char *unixctl_path = NULL;
 struct unixctl_server *unixctl;
-struct signal *sighup;
 char *remote;
 bool exiting;
 int retval;
@@ -76,7 +74,6 @@ main(int argc, char *argv[])
 service_start(&argc, &argv);
 remote = parse_options(argc, argv, &unixctl_path);
 signal(SIGPIPE, SIG_IGN);
-sighup = signal_register(SIGHUP);
 process_init();
 ovsrec_init();
 
@@ -103,9 +100,6 @@ main(int argc, char *argv[])
 
 exiting = false;
 while (!exiting) {
-if (signal_poll(sighup)) {
-vlog_reopen_log_file();
-}
 memory_run();
 if (memory_should_report()) {
 struct simap usage;
@@ -119,7 +113,6 @@ main(int argc, char *argv[])
 unixctl_server_run(unixctl);
 netdev_run();
 
-signal_wait(sighup);
 memory_wait();
 bridge_wait();
 unixctl_server_wait(unixctl);
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 2/2] signals: Remove unused functions.

2014-02-20 Thread Gurucharan Shetty
Signed-off-by: Gurucharan Shetty 
---
 lib/signals.c |   69 -
 lib/signals.h |7 --
 2 files changed, 76 deletions(-)

diff --git a/lib/signals.c b/lib/signals.c
index 27da5d6..85e5c79 100644
--- a/lib/signals.c
+++ b/lib/signals.c
@@ -39,66 +39,6 @@ VLOG_DEFINE_THIS_MODULE(signals);
 #define N_SIGNALS 32
 #endif
 
-struct signal {
-int fds[2];
-};
-
-static struct signal signals[N_SIGNALS];
-
-static void signal_handler(int signr);
-
-/* Sets up a handler for 'signr' and returns a structure that represents it.
- *
- * Only one handler for a given signal may be registered. */
-struct signal *
-signal_register(int signr)
-{
-struct sigaction sa;
-struct signal *s;
-
-ovs_assert(signr >= 1 && signr < N_SIGNALS);
-
-/* Create a pipe. */
-s = &signals[signr];
-ovs_assert(!s->fds[0] && !s->fds[1]);
-xpipe_nonblocking(s->fds);
-
-/* Install signal handler. */
-memset(&sa, 0, sizeof sa);
-sa.sa_handler = signal_handler;
-sigemptyset(&sa.sa_mask);
-sa.sa_flags = SA_RESTART;
-xsigaction(signr, &sa, NULL);
-
-return s;
-}
-
-/* Returns true if signal 's' has been received since the last call to this
- * function with argument 's'. */
-bool
-signal_poll(struct signal *s)
-{
-char buf[_POSIX_PIPE_BUF];
-
-return read(s->fds[0], buf, sizeof buf) > 0;
-}
-
-/* Causes the next call to poll_block() to wake up when signal_poll(s) would
- * return true. */
-void
-signal_wait(struct signal *s)
-{
-poll_fd_wait(s->fds[0], POLLIN);
-}
-
-static void
-signal_handler(int signr)
-{
-if (signr >= 1 && signr < N_SIGNALS) {
-ignore(write(signals[signr].fds[1], "", 1));
-}
-}
-
 /* Returns the name of signal 'signum' as a string.  The return value is either
  * a statically allocated constant string or the 'bufsize'-byte buffer
  * 'namebuf'.  'bufsize' should be at least SIGNAL_NAME_BUFSIZE.
@@ -133,12 +73,3 @@ xsigaction(int signum, const struct sigaction *new, struct 
sigaction *old)
ovs_strerror(errno));
 }
 }
-
-void
-xpthread_sigmask(int how, const sigset_t *new, sigset_t *old)
-{
-int error = pthread_sigmask(how, new, old);
-if (error) {
-VLOG_FATAL("pthread_sigmask failed (%s)", ovs_strerror(error));
-}
-}
diff --git a/lib/signals.h b/lib/signals.h
index 3294293..3ef1b5b 100644
--- a/lib/signals.h
+++ b/lib/signals.h
@@ -18,18 +18,11 @@
 #define SIGNALS_H 1
 
 #include 
-#include 
-#include 
 #include "type-props.h"
 
-struct signal *signal_register(int signr);
-bool signal_poll(struct signal *);
-void signal_wait(struct signal *);
-
 enum { SIGNAL_NAME_BUFSIZE = 7 + INT_STRLEN(int) + 1 };
 const char *signal_name(int signum, char *namebuf, size_t bufsize);
 
 void xsigaction(int signum, const struct sigaction *, struct sigaction *old);
-void xpthread_sigmask(int how, const sigset_t *, sigset_t *old);
 
 #endif /* signals.h */
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V3] ofproto-dpif-upcall: Remove the dispatcher thread.

2014-02-20 Thread Pravin Shelar
On Tue, Feb 18, 2014 at 10:52 AM, Alex Wang  wrote:
> This commit removes the 'dispatcher' thread by allowing 'handler'
> threads to read upcalls directly from dpif.  vport in dpif will
> open netlink sockets for each handler and will use the 5-tuple
> hash from the missed packet to choose which socket (handler) to
> send the upcall.
>
> This patch also significantly simplifies the flow miss handling
> code and brings slight improvement to flow setup rate.
>
> Signed-off-by: Alex Wang 
>
> ---
> v2->v3:
> - removes the rcu read-side critical section in ovs_vport_set_upcall_pids().
> - fix unused variables, acquire rdlock instead of rwlock for read-only
>   critical section.
> - issues to be discussed:
>   1. Should we pass in the "struct flow *" and allow dpif to decide
>  what to hash ?
>   2. Should we add a new API for setting the n_handlers ? currently,
>  recv_set() do both upcall enable/disable and set n_handlers.
>
> PATCH->v2:
> - change attribute name back to OVS_VPORT_ATTR_UPCALL_PID for consistency.
>
> RFC->PATCH
> - use XOR to calculate the 5-tuple hash.  this fixes the performance
>   variation issue.
> - replace the malloc of 'struct upcall *'  in udpif_upcall_handler()
>   by local 'struct upcall' array.
> ---
>  datapath/datapath.c   |   22 +-
>  datapath/vport.c  |  127 +++-
>  datapath/vport.h  |   25 ++-
>  include/linux/openvswitch.h   |9 +-
>  lib/dpif-linux.c  |  454 
> +++--
>  lib/dpif-linux.h  |3 +-
>  lib/dpif-netdev.c |9 +-
>  lib/dpif-provider.h   |   26 ++-
>  lib/dpif.c|   38 ++--
>  lib/dpif.h|   10 +-
>  lib/flow.c|   18 ++
>  lib/flow.h|3 +-
>  ofproto/ofproto-dpif-upcall.c |  265 ++--
>  ofproto/ofproto-dpif-xlate.c  |5 +-
>  ofproto/ofproto-dpif.c|8 +-
>  15 files changed, 556 insertions(+), 466 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index f7c3391..e972c5e 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -242,7 +242,7 @@ void ovs_dp_process_received_packet(struct vport *p, 
> struct sk_buff *skb)
> upcall.cmd = OVS_PACKET_CMD_MISS;
> upcall.key = &key;
> upcall.userdata = NULL;
> -   upcall.portid = p->upcall_portid;
> +   upcall.portid = ovs_vport_find_pid(p, &key);
> ovs_dp_upcall(dp, skb, &upcall);

Can you use different name than pid? pid is generally used for process id.

> consume_skb(skb);
> stats_counter = &stats->n_missed;
> @@ -1241,7 +1241,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
> genl_info *info)
> parms.options = NULL;
> parms.dp = dp;
> parms.port_no = OVSP_LOCAL;
> -   parms.upcall_portid = nla_get_u32(a[OVS_DP_ATTR_UPCALL_PID]);
> +   parms.upcall_pids = a[OVS_DP_ATTR_UPCALL_PID];
>
> ovs_dp_change(dp, a);
>
> @@ -1459,7 +1459,7 @@ static const struct nla_policy 
> vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
> [OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
> [OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
> [OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
> -   [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_U32 },
> +   [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_UNSPEC },
> [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
>  };
>

Have you tried this patch with old userspace to check compatibility?

> @@ -1494,8 +1494,7 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, 
> struct sk_buff *skb,
>
> if (nla_put_u32(skb, OVS_VPORT_ATTR_PORT_NO, vport->port_no) ||
> nla_put_u32(skb, OVS_VPORT_ATTR_TYPE, vport->ops->type) ||
> -   nla_put_string(skb, OVS_VPORT_ATTR_NAME, 
> vport->ops->get_name(vport)) ||
> -   nla_put_u32(skb, OVS_VPORT_ATTR_UPCALL_PID, vport->upcall_portid))
> +   nla_put_string(skb, OVS_VPORT_ATTR_NAME, 
> vport->ops->get_name(vport)))
> goto nla_put_failure;
>
> ovs_vport_get_stats(vport, &vport_stats);
> @@ -1503,6 +1502,9 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, 
> struct sk_buff *skb,
> &vport_stats))
> goto nla_put_failure;
>
> +   if (ovs_vport_get_upcall_pids(vport, skb))
> +   goto nla_put_failure;
> +
> err = ovs_vport_get_options(vport, skb);
> if (err == -EMSGSIZE)
> goto error;
> @@ -1579,8 +1581,7 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, 
> struct genl_info *info)
> int err;
>
> err = -EINVAL;
> -   if (!a[OVS_VPORT_ATTR_NAME] || !a[OVS_VPORT_ATTR_TYPE] ||
> -   !a[OVS_VPORT_ATTR_UPCALL_PID])
> +   if (!a[OVS_VPORT_ATTR_NAME] || !a[OVS_VPORT_ATTR_TYPE])
> g

Re: [ovs-dev] [PATCH] FAQ: Describe how to add new OpenFlow messages.

2014-02-20 Thread Jarno Rajahalme

> On Jan 21, 2014, at 9:34 AM, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 
> ---
> FAQ |   17 +
> 1 file changed, 17 insertions(+)
> 
> diff --git a/FAQ b/FAQ
> index 75d9e6b..dac8fe1 100644
> --- a/FAQ
> +++ b/FAQ
> @@ -1454,6 +1454,23 @@ A: These flows drop the ARP packets that IP hosts use 
> to establish IP
>   
> priority=5,in_port=1,dl_dst=01:00:00:00:00:00/01:00:00:00:00:00,actions=2
>   
> priority=5,in_port=2,dl_dst=01:00:00:00:00:00/01:00:00:00:00:00,actions=1
> 
> +
> +Development
> +---
> +
> +Q: How do I implement a new OpenFlow message?
> +
> +A: Add your new message to "enum ofpraw" and "enum ofptype" in
> +   lib/ofp-msgs.h, following the existing pattern, then recompile and
> +   fix all of the new warnings.

I would add here something like this:

", implementing new functionality for the new message as needed."

Acked-by: Jarno Rajahalme 

>  (If you configure with
> +   --enable-Werror, as described in INSTALL, then it is impossible to
> +   miss any warnings.)
> +
> +   If you need to add an OpenFlow vendor extension message for a
> +   vendor that doesn't yet have any extension messages, then you will
> +   also need to edit build-aux/extract-ofp-msgs.
> +
> +
> Contact 
> ---
> 
> -- 
> 1.7.10.4
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 06/14] socket-util: poll() for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:17PM -0800, Gurucharan Shetty wrote:
> Windows send() does not have a MSG_DONTWAIT.
> So, use the get_socket_error() function.
> 
> Co-authored-by: Linda Sun 
> Signed-off-by: Linda Sun 
> Signed-off-by: Gurucharan Shetty 

The use of MSG_DONTWAIT here is just caution, since it only makes sense
to use check_connection_completion() on a nonblocking socket anyhow.  So
I would prefer to just change MSG_DONTWAIT to 0 here.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 07/14] socket-util: drain_rcvbuf() for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:18PM -0800, Gurucharan Shetty wrote:
> drain_rcvbuf() is currenlty called from netlink-socket.c and
> netdev-linux.c. As of now, I don't see it being used for Windows.
> 
> Signed-off-by: Gurucharan Shetty 

Is this because of the MSG_DONTWAIT here too?  If so, then I think we
can just delete that, because we only use this with nonblocking sockets
(I don't think OVS uses a blocking socket anywhere--blocking is bad).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 08/14] socket-util: Unix socket related calls for non-windows platform.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:19PM -0800, Gurucharan Shetty wrote:
> Don't try to compile Unix socket related functions for Windows.
> 
> Signed-off-by: Gurucharan Shetty 

Can we check for AF_UNIX instead of for _WIN32?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 09/14] socket-util: closesocket() for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:20PM -0800, Gurucharan Shetty wrote:
> For Windows sockets, one has to call closesocket() to
> close the sockets.
> 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 10/14] socket-util: Windows does not have /dev/null.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:21PM -0800, Gurucharan Shetty wrote:
> Signed-off-by: Gurucharan Shetty 

This is only used in daemon.c, which isn't compiled on Windows.  Can we
just move it into there (as a static function)?

Alternatively, I'm pretty sure that Windows does have /dev/null under
another name:
http://en.wikipedia.org/wiki/%5CDevice%5CNull
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 11/14] socket-util: fsync directory for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:22PM -0800, Gurucharan Shetty wrote:
> There is no corresponding function for Windows.
> open() does not work on directories.
> There is a function _commit(fd), but that is only meant
> for files.
> 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 12/14] socket-util: getsockopt for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:23PM -0800, Gurucharan Shetty wrote:
> Windows defines the 'optval' argument as char * instead of void *.
> 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 13/14] socket-util: Describe fd for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:24PM -0800, Gurucharan Shetty wrote:
> In windows there is no clear way to distinguish between a
> socket fd and a file fd.
> 
> We use the function, describe_fd() mostly for debugging.
> For now, return a generic statement.
> 
> Co-authored-by: Linda Sun 
> Signed-off-by: Linda Sun 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 14/14] socket-util: af_inet_ioctl() for Windows.

2014-02-20 Thread Ben Pfaff
On Wed, Feb 19, 2014 at 03:36:25PM -0800, Gurucharan Shetty wrote:
> There is no direct mapping for the ioctl function in
> Windows.  As of now, af_inet_ioctl() is being used for Linux
> and BSD. So, don't try to compile it for Windows.
> 
> Signed-off-by: Gurucharan Shetty 

Acked-by: Ben Pfaff 

Maybe we need a separate socket-unix.c.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ofproto: Send port status message for port-mods, right away.

2014-02-20 Thread Kmindg G
On Fri, Feb 21, 2014 at 5:19 AM, Ben Pfaff  wrote:
> On Thu, Feb 20, 2014 at 12:45:49PM +0800, Kmindg G wrote:
>> On Thu, Feb 20, 2014 at 3:20 AM, Ben Pfaff  wrote:
>> > Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has waited
>> > for ports to notify it that their status has changed before it sends a
>> > port status update to controllers.
>> >
>> > Also, Open vSwitch never sent port config updates at all for port
>> > modifications other than OFPPC_PORT_DOWN.  I guess that this is a relic
>> > from the era when there was only one controller, since presumably the
>> > controller already knew that it changed the port configuration.  But in the
>> > multi-controller world, it makes sense to send such port status updates,
>> > and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
>> > shouldn't be sent.
>> >
>> > Signed-off-by: Ben Pfaff 
>> > Reported-by: Kmindg G 
>> > ---
>> >  AUTHORS   |1 +
>> >  ofproto/ofproto.c |   25 +
>> >  2 files changed, 14 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/AUTHORS b/AUTHORS
>> > index c557303..2fda8d7 100644
>> > --- a/AUTHORS
>> > +++ b/AUTHORS
>> > @@ -197,6 +197,7 @@ John Hurley john.hur...@netronome.com
>> >  Kevin Mancuso   kevin.manc...@rackspace.com
>> >  Kiran Shanbhog  ki...@vmware.com
>> >  Kirill Kabardin
>> > +Kmindg Gkmi...@gmail.com
>> >  Koichi Yagishitayagishita.koi...@jrc.co.jp
>> >  Konstantin Khorenko khore...@openvz.org
>> >  Kris zhang  zhang.k...@gmail.com
>> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> > index 62c97a2..48e10ca 100644
>> > --- a/ofproto/ofproto.c
>> > +++ b/ofproto/ofproto.c
>> > @@ -2986,22 +2986,23 @@ update_port_config(struct ofport *port,
>> > enum ofputil_port_config config,
>> > enum ofputil_port_config mask)
>> >  {
>> > -enum ofputil_port_config old_config = port->pp.config;
>> > -enum ofputil_port_config toggle;
>> > -
>> > -toggle = (config ^ port->pp.config) & mask;
>> > -if (toggle & OFPUTIL_PC_PORT_DOWN) {
>> > -if (config & OFPUTIL_PC_PORT_DOWN) {
>> > -netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL);
>> > -} else {
>> > -netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL);
>> > -}
>> > +enum ofputil_port_config toggle = (config ^ port->pp.config) & mask;
>> > +
>> > +if (toggle & OFPUTIL_PC_PORT_DOWN
>> > +&& (config & OFPUTIL_PC_PORT_DOWN
>> > +? netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL)
>> > +: netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL))) {
>> > +/* We tried to bring the port up or down, but it failed, so don't
>> > + * update the "down" bit. */
>> >  toggle &= ~OFPUTIL_PC_PORT_DOWN;
>> >  }
>> >
>> > -port->pp.config ^= toggle;
>> > -if (port->pp.config != old_config) {
>> > +if (toggle) {
>> > +enum ofputil_port_config old_config = port->pp.config;
>> > +port->pp.config ^= toggle;
>> >  port->ofproto->ofproto_class->port_reconfigured(port, old_config);
>> > +connmgr_send_port_status(port->ofproto->connmgr, &port->pp,
>> > + OFPPR_MODIFY);
>> >  }
>> >  }
>> >
>> > --
>> > 1.7.10.4
>> >
>>
>> looks good to me.
>
> Thanks.  What do you think of this version?  I think that it retains
> better compatibility with OpenFlow 1.0 in particular.
>
> --8<--cut here-->8--
>
> From: Ben Pfaff 
> Date: Thu, 20 Feb 2014 13:18:51 -0800
> Subject: [PATCH] ofproto: Send port status message for port-mods, right away.
>
> Until now, when it processes OFPT_PORT_MOD message, Open vSwitch has waited
> for ports to notify it that their status has changed before it sends a
> port status update to controllers.
>
> Also, Open vSwitch never sent port config updates at all for port
> modifications other than OFPPC_PORT_DOWN.  I guess that this is a relic
> from the era when there was only one controller, since presumably the
> controller already knew that it changed the port configuration.  But in the
> multi-controller world, it makes sense to send such port status updates,
> and I couldn't quickly find anything in OF1.3 or OF1.4 that said they
> shouldn't be sent.
>
> Signed-off-by: Ben Pfaff 
> Reported-by: Kmindg G 
> ---
>  ofproto/connmgr.c | 30 --
>  ofproto/connmgr.h |  4 ++--
>  ofproto/ofproto.c | 38 --
>  3 files changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index a58e785..7a25828 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -1456,9 +1456,11 @@ static void schedule_packet_in(struct ofconn *, struct 
> ofproto_packet_in,
> enum ofp_packet_in_reason wire_reason);
>
>  /* Sends an OFPT_PORT

[ovs-dev] [PATCH 2/2] dpif-netdev: Fix memory leak.

2014-02-20 Thread Alex Wang
In dpif_netdev_flow_del() and dp_netdev_port_input(), the
referenced 'netdev_flow' is not un-referenced.  This causes
the leak of the struct's memory.

This commit fixes the above issue by calling dp_netdev_flow_unref()
after using the reference.

Signed-off-by: Alex Wang 
---
 lib/dpif-netdev.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9ba93dd..c4ba646 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1295,6 +1295,7 @@ dpif_netdev_flow_del(struct dpif *dpif, const struct 
dpif_flow_del *del)
 ovs_mutex_unlock(&netdev_flow->mutex);
 }
 dp_netdev_remove_flow(dp, netdev_flow);
+dp_netdev_flow_unref(netdev_flow);
 } else {
 error = ENOENT;
 }
@@ -1706,6 +1707,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct ofpbuf 
*packet,
 dp_netdev_execute_actions(dp, &key, packet, md,
   actions->actions, actions->size);
 dp_netdev_actions_unref(actions);
+dp_netdev_flow_unref(netdev_flow);
 ovsthread_counter_inc(dp->n_hit, 1);
 } else {
 ovsthread_counter_inc(dp->n_missed, 1);
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/2] dpif-netdev: Call ovs_refcount_destroy() before free().

2014-02-20 Thread Alex Wang
This commit makes dp_netdev_flow_unref() and dp_netdev_actions_unref()
invoke the ovs_refcount_destroy() before freeing the corresponding
pointer.

Signed-off-by: Alex Wang 
---
 lib/dpif-netdev.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 73eb99d..9ba93dd 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -871,6 +871,7 @@ dp_netdev_flow_unref(struct dp_netdev_flow *flow)
 cls_rule_destroy(CONST_CAST(struct cls_rule *, &flow->cr));
 ovs_mutex_lock(&flow->mutex);
 dp_netdev_actions_unref(flow->actions);
+ovs_refcount_destroy(&flow->ref_cnt);
 ovs_mutex_unlock(&flow->mutex);
 ovs_mutex_destroy(&flow->mutex);
 free(flow);
@@ -1543,6 +1544,7 @@ void
 dp_netdev_actions_unref(struct dp_netdev_actions *actions)
 {
 if (actions && ovs_refcount_unref(&actions->ref_cnt) == 1) {
+ovs_refcount_destroy(&actions->ref_cnt);
 free(actions->actions);
 free(actions);
 }
-- 
1.7.9.5

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] openvswitch-2.0.0 : ovs-vswitchd segfault in netdev_get_etheraddr

2014-02-20 Thread Sridhar Samudrala
I am seeing a segfault with ovs-vswitchd when a VM attached to a OVS 
bridge is shutdown.

I am running openvswitch-2.0.0 on linux kernel:3.10.30

Is this a known issue with any fix in the git repository?

Core was generated by `ovs-vswitchd --pidfile --detach'.
Program terminated with signal 11, Segmentation fault.
#0  0x00458cf4 in netdev_get_etheraddr (netdev=0xe19af0,
mac=0xe198d6 "") at lib/netdev.c:627
627return netdev->netdev_class->get_etheraddr(netdev, mac);
Missing separate debuginfos, use: debuginfo-install 
glibc-2.12-1.132.el6.x86_64 keyutils-libs-1.4-4.el6.x86_64 
krb5-libs-1.10.3-10.el6_4.6.x86_64 libcom_err-1.41.12-14.el6_4.2.x86_64 
libcom_err-1.41.12-18.el6.x86_64 libselinux-2.0.94-5.3.el6_4.1.x86_64 
openssl-1.0.1e-16.el6_5.4.x86_64 zlib-1.2.3-29.el6.x86_64

(gdb) bt
#0  0x00458cf4 in netdev_get_etheraddr (netdev=0xe19af0,
mac=0xe198d6 "") at lib/netdev.c:627
#1  0x004213df in send_bpdu_cb (pkt=0xe1c360, port_num=1,
ofproto_=0xdfdd30) at ofproto/ofproto-dpif.c:2021
#2  0x00494b09 in stp_send_bpdu (p=0xe1f498, bpdu=0x7fffecef88d0,
bpdu_size=35) at lib/stp.c:1548
#3  0x0049547a in stp_transmit_config (p=0xe1f498) at lib/stp.c:1046
#4  0x00495526 in stp_config_bpdu_generation (stp=0xe1f3b0)
at lib/stp.c:1106
#5  0x0049589c in stp_hello_timer_expiry (stp=0xe1f3b0,
ms=) at lib/stp.c:1352
#6  stp_tick (stp=0xe1f3b0, ms=) at lib/stp.c:366
#7  0x004257f0 in stp_run (ofproto_=0xdfdd40)
at ofproto/ofproto-dpif.c:2203
#8  run (ofproto_=0xdfdd40) at ofproto/ofproto-dpif.c:1529
#9  0x00416a1b in ofproto_run (p=0xdfdd40) at ofproto/ofproto.c:1355
#10 0x0040cfc4 in bridge_run () at vswitchd/bridge.c:2381
#11 0x0040e57a in main (argc=,
argv=) at vswitchd/ovs-vswitchd.c:118

looks like netdev ptr passed to netdev_get_etheraddr is invalid.

(gdb) p *netdev
$1 = {name = 0xe016c0 "", netdev_class = 0x0, ref_cnt = 14692480,
  node = 0xe15930, saved_flags_list = {prev = 0x130, next = 0x20}}

Disabling STP seems to avoid this segfault
  ovs-vsctl set bridge ovs-br0 stp_enable=false

Thanks
Sridhar
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev