Re: [ovs-dev] [groups v3] Implement OpenFlow 1.1+ "groups" protocol.

2013-08-30 Thread Simon Horman
On Thu, Aug 29, 2013 at 02:00:51PM -0700, Ben Pfaff wrote:
> From: Neil Zhu 
> 
> This doesn't include a dpif implementation of groups functionality.  In its
> current form, it is untested.  Before this is committed, it needs some
> more comments and an ovs-ofctl manpage update.
> 
> Signed-off-by: Neil Zhu 
> Co-authored-by: Ben Pfaff 
> Signed-off-by: Ben Pfaff 

...

> +ofputil_decode_group_stats_reply(struct ofpbuf *msg,
> + struct ofputil_group_stats *gs)
> +{
> +struct ofp11_bucket_counter *obc;
> +struct ofp11_group_stats *ogs11;
> +enum ofpraw raw;
> +enum ofperr error;
> +size_t base_len;
> +size_t length;
> +size_t i;
> +
> +if (!msg->size) {
> +return EOF;
> +}
> +
> +error = (msg->l2
> + ? ofpraw_decode(&raw, msg->l2)
> + : ofpraw_pull(&raw, msg));
> +if (error) {
> +return error;
> +}

It seems to me that the msg->size check should appear after
the ofpraw_decode()/ofpraw_pull() call in order to allow for
a stats reply with an empty array of stats.

This would also be consistent with the code present in
ofputil_decode_queue_stats();


With the code above I see:
# ovs-ofctl -O OpenFlow11 dump-group-stats br0
2013-08-30T06:58:51Z|3|ofp_util|WARN|OFPST_GROUP reply reply has 0 leftover 
bytes at end
OFPST_GROUP reply (OF1.1) (xid=0x2): ***parse error***

With my suggested change I see:
# ovs-ofctl -O OpenFlow11 dump-group-stats br0
OFPST_GROUP reply (OF1.1) (xid=0x2):
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ovs-ofctl: Document group commands

2013-08-30 Thread Simon Horman
Signed-off-by: Simon Horman 

---

Ben, feel free to squash this into 'Implement OpenFlow 1.1+ "groups" protocol'.
---
 utilities/ovs-ofctl.8.in | 108 +++
 1 file changed, 108 insertions(+)

diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 85b2e44..5923fc9 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -178,6 +178,21 @@ the statistics are aggregated across all flows in the 
switch's flow
 tables.  See \fBFlow Syntax\fR, below, for the syntax of \fIflows\fR.
 The output format is described in \fBTable Entry Output\fR.
 .
+.TP
+\fBdump\-groups \fIswitch
+Prints to the console all group entries in \fIswitch\fR's tables. Each line
+of output is a group entry as described in \fBGroup Syntax\fR below.
+.
+.TP
+\fBdump\-group\-features \fIswitch
+Prints to the console the group features of the \fIswitch\fR.
+.TP
+\fBdump\-group-stats \fIswitch \fR[\fIgroups\fR]
+Prints to the console statistics for the specified \fIgroups in the
+\fIswitch\fR's tables.  If \fIgroups\fR is omitted then statistics for all
+groups are printed.  See \fBGroup Syntax\fR, below, for the syntax of
+\fIgroups\fR.
+.
 .IP "\fBqueue\-stats \fIswitch \fR[\fIport \fR[\fIqueue\fR]]"
 Prints to the console statistics for the specified \fIqueue\fR on
 \fIport\fR within \fIswitch\fR.  \fIport\fR can be an OpenFlow port
@@ -258,6 +273,30 @@ keyword \fBLOCAL\fR (the preferred way to refer to the 
OpenFlow
 ``local'' port), or the keyword \fBNONE\fR to indicate that the packet
 was generated by the switch itself.
 .
+.SS "OpenFlow Switch Group Table Commands"
+.
+These commands manage the group table in an OpenFlow switch.  In each
+case, \fIgroup\fR specifies a group entry in the format described in
+\fBGroup Syntax\fR, below, and \fIfile\fR is a text file that contains
+zero or more groups in the same syntax, one per line.
+
+.IP "\fBadd\-group \fIswitch group\fR"
+.IQ "\fBadd\-group \fIswitch \fB\- < \fIfile\fR"
+.IQ "\fBadd\-groups \fIswitch file\fR"
+Add each group entry to \fIswitch\fR's tables.
+.
+.IP "\fBmod\-group \fIswitch group\fR"
+.IQ "\fBmod\-group \fIswitch \fB\- < \fIfile\fR"
+Modify the action buckets in entries from \fIswitch\fR's tables for
+each group entry.
+.
+.IP "\fBdel\-groups \fIswitch\fR"
+.IQ "\fBdel\-groups \fIswitch \fR[\fIgroup\fR]"
+.IQ "\fBdel\-groups \fIswitch \fB\- < \fIfile\fR"
+Deletes entries from \fIswitch\fR's group table.  With only a
+\fIswitch\fR argument, deletes all groups.  Otherwise, deletes the group
+for each group entry.
+.
 .SS "OpenFlow Switch Monitoring Commands"
 .
 .IP "\fBsnoop \fIswitch\fR"
@@ -1439,6 +1478,75 @@ of \fBduration\fR.  (This is separate from 
\fBduration\fR because
 The integer number of seconds that have passed without any packets
 passing through the flow.
 .
+.SS "Group Syntax"
+.PP
+Some \fBovs\-ofctl\fR commands accept an argument that describes a group or
+groups.  Such flow descriptions comprise a series
+\fIfield\fB=\fIvalue\fR assignments, separated by commas or white
+space.  (Embedding spaces into a group description normally requires
+quoting to prevent the shell from breaking the description into
+multiple arguments.). Unless noted otherwise only the last instance
+of each field is honoured.
+.PP
+.IP \fBgroup_id=\fIid\fR
+The integer group id of group.
+When this field is specified in \fBdel-groups\fR or \fBdump-groups\fR,
+the keyword "all" may be used to designate all groups.
+.
+This field is required.
+
+
+.IP \fBtype=\fItype\fR
+The type of the group.  This \fBadd-group\fR, \fBadd-groups\fR and
+\fBdel-groups\fR command require this field.  The following keywords
+designated the allowed types:
+.RS
+.IP \fBall\fR
+Execute all buckets in the group.
+.IP \fBselect\fR
+Execute one bucket in the group.
+The switch should select the bucket in such a way that should implement
+equal load sharing is achieved.  The switch may optionally select the
+bucket based on bucket weights.
+.IP \fBindirect\fR
+Executes the one bucket in the group.
+.IP \fBff\fR
+.IP \fBfast_failover\fR
+Executes the first live bucket in the group which is associated with
+a live port or group.
+.RE
+
+.IP \fBbucket\fR=\fIbucket_parameters\fR
+The \fBadd-group\fR, \fBadd-groups\fR and \fBmod-group\fR commands
+require at least one bucket field. Bucket fields must appear after
+all other fields.
+.
+Multiple bucket fields to specify multiple buckets.
+The order in which buckets are specified corresponds to their order in
+the group. If the type of the group is "indirect" then only one group may
+be specified.
+.
+\fIbucket_parameters\fR consists of a list of \fIfield\fB=\fIvalue\fR
+assignments, separated by commas or white space followed by a
+comma-separated list of actions.
+The syntax of actions are same
+to \fBactions=\fR field described in \fBFlow Syntax\fR above.
+The fields for \fIbucket_parameters\fR are:
+.
+.RS
+.IP \fBweight=\fIvalue\fR
+The relative weight of the bucket as an integer. This may 

[ovs-dev] [PATCH] util.c: Include pthread_np.h on FreeBSD

2013-08-30 Thread Ed Maste
Signed-off-by: Ed Maste 
---
On FreeBSD pthread_set_name_np's prototype is provided by pthread_np.h.
As I believe it is the only platform to provide the "set_name" (with an
underscore) variant I hope it's fine to use the existing autoconf macro
HAVE_PTHREAD_SET_NAME_NP.

 lib/util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/util.c b/lib/util.c
index 76c33cd..3cada4a 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -30,6 +30,9 @@
 #include "coverage.h"
 #include "ovs-thread.h"
 #include "vlog.h"
+#ifdef HAVE_PTHREAD_SET_NAME_NP
+#include 
+#endif
 
 VLOG_DEFINE_THIS_MODULE(util);
 
-- 
1.8.3.3

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


Re: [ovs-dev] [ovs-announce] Task List for Open vSwitch Hackathon Sep. 6 and 7

2013-08-30 Thread Ben Pfaff
[redirecting to ovs-dev]

On Fri, Aug 30, 2013 at 06:17:52AM -0700, Gurucharan Shetty wrote:
> On Aug 29, 2013, at 9:57 PM, Ben Pfaff  wrote:
> 
> >Unclaimed Tasks (Small, Required)
> >-
> >
> >The following tasks could use volunteers.  These tasks are all
> >required for OpenFlow compliance.  My guess is that an average
> >developer could build any of these within the two days of the
> >hackathon or less.  Roughly in order of increasing amount of work:
> >
> >   * OFPT_FLOW_MOD ability to delete flows in all tables.
> I would like to do this.

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


Re: [ovs-dev] 1.9.1, 1.9.2, 1.10.1 "released"?

2013-08-30 Thread Ben Pfaff
On Thu, Aug 29, 2013 at 03:05:05PM -0700, Ben Pfaff wrote:
> James pointed out that branch-1.9 has version 1.9.2 and branch-1.10
> has version 1.10.1, but that none of those has any tarball or tag or
> announcement.  I can do the tags, but you usually do the rest.  Also,
> each of those branches has a lot of commits beyond those versions, so
> we might really want 1.9.3 and 1.10.2.

Talking to James on #openvswitch, it sounds like he could really use
both 1.9.3 and 1.10.2.  1.9.3 makes sense regardless but James says
that he'd like to push an official 1.10.x release into Ubuntu 13.10.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-ofctl: Document group commands

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 05:42:54PM +0900, Simon Horman wrote:
> Signed-off-by: Simon Horman 
> 
> ---
> 
> Ben, feel free to squash this into 'Implement OpenFlow 1.1+ "groups"
> protocol'.

Thanks, done.  I added your sign-off and gave you co-authorship also
(I really like to encourage documentation contributions!).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] 1.9.1, 1.9.2, 1.10.1 "released"?

2013-08-30 Thread James Page
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 30/08/13 17:02, Ben Pfaff wrote:
> On Thu, Aug 29, 2013 at 03:05:05PM -0700, Ben Pfaff wrote:
>>> James pointed out that branch-1.9 has version 1.9.2 and
>>> branch-1.10 has version 1.10.1, but that none of those has any
>>> tarball or tag or announcement.  I can do the tags, but you
>>> usually do the rest.  Also, each of those branches has a lot of
>>> commits beyond those versions, so we might really want 1.9.3
>>> and 1.10.2.
> Talking to James on #openvswitch, it sounds like he could really
> use both 1.9.3 and 1.10.2.  1.9.3 makes sense regardless but James
> says that he'd like to push an official 1.10.x release into Ubuntu
> 13.10.
> 

Yes please:

1.9.3 -> SRU for Ubuntu 13.04
1.10.2 -> Point release for Ubuntu Saucy prior to release in October.

Cheers

James

- -- 
James Page
Ubuntu and Debian Developer
james.p...@ubuntu.com
jamesp...@debian.org
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQIcBAEBCAAGBQJSIMUAAAoJEL/srsug59jDUncP+gOniyaeMyM+owozHHXiLhLE
qEHT03kf664ASaDUJtsJfMPqCjikmbMZbWowPxrJ4+/wXIZs389tfaKV2Cc+dxKt
nYBETptAv49ZlED9uPVUAUCQDrb1CQ7nypXpFBqhI7/OUTKHdaBnFBgieTnPSfp3
g8lB8zwcpaCq4kgKXoCOZzDbz1BfYN4K3g+Emk5nJY5T0Sl3qqfJMJYG55eJC29l
7ea6R5GCzbTxhA8RJBTnNIgqvV2laGoADmJjQlgW2UTAGLnmuPpZ89r/2vZys3QX
zGstUpkxCg34/NpYYhrCMf8Sug8OWGFBwdMAMWb6L27zAYrYPnrMGuSyMyFSBK1T
BxlvWQAuCwnvVGjDOEeSfmmzT1DaaksM+47xHraeBy3DkyJ92rdUonblXxjG4PHX
B1MOUYPabG2XgjGlKtFLMcwPQl3uApgwH7o40cC5iQiNJ/U6ZyKVIhc6Nsj35BBW
ICHiZFkyJ/srl42m43owczfN6epWzgEG/yfQvwGY5mTQZlREWynXQS7mDZkD5t+M
hmm/Emc19QhHQ1HxGpcW9lKPnnekGthyn+RR7w5CDg7GgUSqU3SzN5gLztPPnLgg
PnxhgQE6xUYGcrzIEFzu6hxjXrlDTGc8tbj0PDfGIiJoDdQuIfbKdjyIkOSUl0Ta
qfiL7e5VBctaklM8zF9k
=44c+
-END PGP SIGNATURE-
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Guolin Yang, VMware
I will send a v2 soon based on Andy's feedback.

Thanks
Guolin


On Thu, Aug 29, 2013 at 8:43 PM, Ben Pfaff  wrote:

> Andy, thanks for the review.
>
> Guolin, are you willing to submit a v2?
>
> Thanks,
>
> Ben.
>
> On Thu, Aug 29, 2013 at 05:21:03PM -0700, Andy Zhou wrote:
> > Good catch. The logic looks good to me.
> >
> > When compiling, I got the following warnings:
> >
> > b/odp-util.c: In function ?odp_flow_key_from_flow__?:
> > lib/odp-util.c:2559:17: error: comparison is always false due to limited
> > range of data type [-Werror=type-limits]
> > lib/odp-util.c:2560:31: error: comparison is always false due to limited
> > range of data type [-Werror=type-limits]
> >
> > It would be nice to describe the bug explicitly in the commit message.
> >
> >
> >
> >
> > On Thu, Aug 29, 2013 at 4:18 PM, Guolin Yang, VMware  >wrote:
> >
> > > I found the isue happen for icmpv6  packet, that is why I read the code
> > > and found the bug.
> > >
> > > Thanks
> > > Guolin
> > >
> > > -- Forwarded message --
> > > From: 
> > > Date: Thu, Aug 29, 2013 at 10:35 AM
> > > Subject: [PATCH] Fix a bug in conversion between flow/mask and flow key
> > > To: d...@openvswitch.com
> > > Cc: Guolin Yang 
> > >
> > >
> > > From: Guolin Yang 
> > >
> > > For ICMPv6, when convert flow/mask to flow/mask key, the check
> > > for whether there is ND information is incorrect. Similar error
> > > in reverse conversion.
> > >
> > > Signed-off-by: Guolin Yang 
> > > ---
> > >  lib/odp-util.c |   16 +++-
> > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > > index 15ad2be..6ddde9d 100644
> > > --- a/lib/odp-util.c
> > > +++ b/lib/odp-util.c
> > > @@ -2553,8 +2553,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf,
> const
> > > struct flow *data,
> > >  icmpv6_key->icmpv6_type = ntohs(data->tp_src);
> > >  icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
> > >
> > > -if (icmpv6_key->icmpv6_type == ND_NEIGHBOR_SOLICIT
> > > -|| icmpv6_key->icmpv6_type == ND_NEIGHBOR_ADVERT)
> {
> > > +if (flow->tp_dst == htons(0) &&
> > > +(flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > > + flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) &&
> > > +(!is_mask || (icmpv6_key->icmpv6_type == 0x &&
> > > +  icmpv6_key->icmpv6_code == 0x))) {
> > > +
> > >  struct ovs_key_nd *nd_key;
> > >
> > >  nd_key = nl_msg_put_unspec_uninit(buf,
> OVS_KEY_ATTR_ND,
> > > @@ -2965,8 +2969,9 @@ parse_l2_5_onward(const struct nlattr
> > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > >  flow->tp_src = htons(icmpv6_key->icmpv6_type);
> > >  flow->tp_dst = htons(icmpv6_key->icmpv6_code);
> > >  expected_bit = OVS_KEY_ATTR_ICMPV6;
> > > -if (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > > -src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) {
> > > +if (src_flow->tp_dst == 0 &&
> > > +(src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > > + src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
> > >  if (!is_mask) {
> > >  expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
> > >  }
> > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > >  if (is_mask) {
> > >  if (!is_all_zeros((const uint8_t *) nd_key,
> > >sizeof *nd_key) &&
> > > -flow->tp_src != htons(0x)) {
> > > +(flow->tp_src != htons(0x) &&
> > > + flow->tp_dst != htons(0x))) {
> > >  return ODP_FIT_ERROR;
> > >  } else {
> > >  expected_attrs |= UINT64_C(1) <<
> > > OVS_KEY_ATTR_ND;
> > > --
> > > 1.7.9.5
> > >
> > >
> > >
>
> > ___
> > 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] [groups v3] Implement OpenFlow 1.1+ "groups" protocol.

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 04:30:39PM +0900, Simon Horman wrote:
> On Thu, Aug 29, 2013 at 02:00:51PM -0700, Ben Pfaff wrote:
> > From: Neil Zhu 
> > 
> > This doesn't include a dpif implementation of groups functionality.  In its
> > current form, it is untested.  Before this is committed, it needs some
> > more comments and an ovs-ofctl manpage update.
> > 
> > Signed-off-by: Neil Zhu 
> > Co-authored-by: Ben Pfaff 
> > Signed-off-by: Ben Pfaff 
> 
> ...
> 
> > +ofputil_decode_group_stats_reply(struct ofpbuf *msg,
> > + struct ofputil_group_stats *gs)
> > +{
> > +struct ofp11_bucket_counter *obc;
> > +struct ofp11_group_stats *ogs11;
> > +enum ofpraw raw;
> > +enum ofperr error;
> > +size_t base_len;
> > +size_t length;
> > +size_t i;
> > +
> > +if (!msg->size) {
> > +return EOF;
> > +}
> > +
> > +error = (msg->l2
> > + ? ofpraw_decode(&raw, msg->l2)
> > + : ofpraw_pull(&raw, msg));
> > +if (error) {
> > +return error;
> > +}
> 
> It seems to me that the msg->size check should appear after
> the ofpraw_decode()/ofpraw_pull() call in order to allow for
> a stats reply with an empty array of stats.

You're right.  Thanks a lot, I've now moved it.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] FAQ: Explain the two kinds of "promiscuous mode" and how to configure them.

2013-08-30 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
 FAQ |   34 ++
 1 file changed, 34 insertions(+)

diff --git a/FAQ b/FAQ
index 75d9007..5406e84 100644
--- a/FAQ
+++ b/FAQ
@@ -250,6 +250,40 @@ A: The following commands configure br0 with eth0 and tap0 
as trunk
 
ovs-vsctl clear bridge br0 mirrors
 
+Q: Does Open vSwitch support configuring a port in promiscuous mode?
+
+A: Yes.  How you configure it depends on what you mean by "promiscuous
+   mode":
+
+  - Conventionally, "promiscuous mode" is a feature of a network
+interface card.  Ordinarily, a NIC passes to the CPU only the
+packets actually destined to its host machine.  It discards
+the rest to avoid wasting memory and CPU cycles.  When
+promiscuous mode is enable, however, it passes every packet to
+the CPU.  On an old-style shared-media or hub-based network,
+this allows the host to spy on all packets on the network.
+But in the switched networks you'll find pretty much
+everywhere these days, promiscuous mode doesn't have much
+effect, because few packets not destined to a host are
+delivered to the host's NIC.
+
+This form of promiscuous mode is configured in the guest OS of
+the VMs on your bridge, e.g. with "ifconfig".
+
+  - The VMware vSwitch uses a different definition of "promiscuous
+mode".  When you configure promiscuous mode on a VMware vNIC,
+the vSwitch sends a copy of every packet received by the
+vSwitch to that vNIC.  That has a much bigger effect than just
+enabling promiscuous mode in a guest OS.  Rather than getting
+a few stray packets for which the switch does not yet know the
+correct destination, the vNIC gets every packet.  The effect
+is similar to replacing the vSwitch by a virtual hub.
+
+This "promiscuous mode" is what switches normally call "port
+mirroring" or "SPAN".  For information on how to configure
+SPAN, see "How do I configure a port as a SPAN port, that is,
+enable mirroring of all traffic to that port?"
+
 Q: How do I configure a VLAN as an RSPAN VLAN, that is, enable
mirroring of all traffic to that VLAN?
 
-- 
1.7.10.4

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


[ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread gyang
From: Guolin Yang 

In odp_flow_key_from_flow__(), when converting ICMPv6 flow/mask
to flow/mask key, we should always use flow to check for whether
ND informaition is present or not. In mask case, both type and code
field should be masked, otherwise ND fields can be masked.

Similarly in reverse conversion (parse_l2_5_onward()), we should
have same check.

Signed-off-by: Guolin Yang 
---
v1--> v2
This change fix the compiling warning and revised the description.
---
 lib/odp-util.c |   16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 15ad2be..d19446e 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2553,8 +2553,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const 
struct flow *data,
 icmpv6_key->icmpv6_type = ntohs(data->tp_src);
 icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
 
-if (icmpv6_key->icmpv6_type == ND_NEIGHBOR_SOLICIT
-|| icmpv6_key->icmpv6_type == ND_NEIGHBOR_ADVERT) {
+if (flow->tp_dst == htons(0) &&
+(flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
+ flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) &&
+(!is_mask || (data->tp_src == htons(0x) &&
+  data->tp_dst == htons(0x {
+
 struct ovs_key_nd *nd_key;
 
 nd_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_ND,
@@ -2965,8 +2969,9 @@ parse_l2_5_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
 flow->tp_src = htons(icmpv6_key->icmpv6_type);
 flow->tp_dst = htons(icmpv6_key->icmpv6_code);
 expected_bit = OVS_KEY_ATTR_ICMPV6;
-if (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
-src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) {
+if (src_flow->tp_dst == htons(0) &&
+(src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
+ src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
 if (!is_mask) {
 expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
 }
@@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr 
*attrs[OVS_KEY_ATTR_MAX + 1],
 if (is_mask) {
 if (!is_all_zeros((const uint8_t *) nd_key,
   sizeof *nd_key) &&
-flow->tp_src != htons(0x)) {
+(flow->tp_src != htons(0x) &&
+ flow->tp_dst != htons(0x))) {
 return ODP_FIT_ERROR;
 } else {
 expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH] util.c: Include pthread_np.h on FreeBSD

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 10:56:08AM -0400, Ed Maste wrote:
> Signed-off-by: Ed Maste 

Thanks, applied to master and branch-2.0.

> ---
> On FreeBSD pthread_set_name_np's prototype is provided by pthread_np.h.
> As I believe it is the only platform to provide the "set_name" (with an
> underscore) variant I hope it's fine to use the existing autoconf macro
> HAVE_PTHREAD_SET_NAME_NP.

This looked like useful information so I moved it into the commit
message, hope that's OK.

Thanks,

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


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
Looks good to me. Thanks for the revision.

Acked-by: Andy Zhou 


On Fri, Aug 30, 2013 at 9:57 AM,  wrote:

> From: Guolin Yang 
>
> In odp_flow_key_from_flow__(), when converting ICMPv6 flow/mask
> to flow/mask key, we should always use flow to check for whether
> ND informaition is present or not. In mask case, both type and code
> field should be masked, otherwise ND fields can be masked.
>
> Similarly in reverse conversion (parse_l2_5_onward()), we should
> have same check.
>
> Signed-off-by: Guolin Yang 
> ---
> v1--> v2
> This change fix the compiling warning and revised the description.
> ---
>  lib/odp-util.c |   16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 15ad2be..d19446e 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -2553,8 +2553,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const
> struct flow *data,
>  icmpv6_key->icmpv6_type = ntohs(data->tp_src);
>  icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
>
> -if (icmpv6_key->icmpv6_type == ND_NEIGHBOR_SOLICIT
> -|| icmpv6_key->icmpv6_type == ND_NEIGHBOR_ADVERT) {
> +if (flow->tp_dst == htons(0) &&
> +(flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> + flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) &&
> +(!is_mask || (data->tp_src == htons(0x) &&
> +  data->tp_dst == htons(0x {
> +
>  struct ovs_key_nd *nd_key;
>
>  nd_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_ND,
> @@ -2965,8 +2969,9 @@ parse_l2_5_onward(const struct nlattr
> *attrs[OVS_KEY_ATTR_MAX + 1],
>  flow->tp_src = htons(icmpv6_key->icmpv6_type);
>  flow->tp_dst = htons(icmpv6_key->icmpv6_code);
>  expected_bit = OVS_KEY_ATTR_ICMPV6;
> -if (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> -src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) {
> +if (src_flow->tp_dst == htons(0) &&
> +(src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> + src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
>  if (!is_mask) {
>  expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
>  }
> @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> *attrs[OVS_KEY_ATTR_MAX + 1],
>  if (is_mask) {
>  if (!is_all_zeros((const uint8_t *) nd_key,
>sizeof *nd_key) &&
> -flow->tp_src != htons(0x)) {
> +(flow->tp_src != htons(0x) &&
> + flow->tp_dst != htons(0x))) {
>  return ODP_FIT_ERROR;
>  } else {
>  expected_attrs |= UINT64_C(1) <<
> OVS_KEY_ATTR_ND;
> --
> 1.7.9.5
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ofproto: Convert units correctly in ofport_open().

2013-08-30 Thread Ben Pfaff
netdev_features_to_bps() returns a speed in bps, but struct
ofputil_phy_port's curr_speed and max_speed are in kbps, so a conversion
is necessary.  This commit fixes the problem.

Reported-by: Benjamin Lunsky 
Signed-off-by: Ben Pfaff 
---
 ofproto/ofproto.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0b054e7..0caaad9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1838,8 +1838,8 @@ ofport_open(struct ofproto *ofproto,
 pp->state = netdev_get_carrier(netdev) ? 0 : OFPUTIL_PS_LINK_DOWN;
 netdev_get_features(netdev, &pp->curr, &pp->advertised,
 &pp->supported, &pp->peer);
-pp->curr_speed = netdev_features_to_bps(pp->curr, 0);
-pp->max_speed = netdev_features_to_bps(pp->supported, 0);
+pp->curr_speed = netdev_features_to_bps(pp->curr, 0) / 1000;
+pp->max_speed = netdev_features_to_bps(pp->supported, 0) / 1000;
 
 return netdev;
 }
-- 
1.7.10.4

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


Re: [ovs-dev] [PATCH] FAQ: Explain the two kinds of "promiscuous mode" and how to configure them.

2013-08-30 Thread Pritesh Kothari (pritkoth)
ack, looks good to me.
-pritesh

On Aug 30, 2013, at 10:02 AM, Ben Pfaff wrote:

> Signed-off-by: Ben Pfaff 
> ---
> FAQ |   34 ++
> 1 file changed, 34 insertions(+)
> 
> diff --git a/FAQ b/FAQ
> index 75d9007..5406e84 100644
> --- a/FAQ
> +++ b/FAQ
> @@ -250,6 +250,40 @@ A: The following commands configure br0 with eth0 and 
> tap0 as trunk
> 
>ovs-vsctl clear bridge br0 mirrors
> 
> +Q: Does Open vSwitch support configuring a port in promiscuous mode?
> +
> +A: Yes.  How you configure it depends on what you mean by "promiscuous
> +   mode":
> +
> +  - Conventionally, "promiscuous mode" is a feature of a network
> +interface card.  Ordinarily, a NIC passes to the CPU only the
> +packets actually destined to its host machine.  It discards
> +the rest to avoid wasting memory and CPU cycles.  When
> +promiscuous mode is enable, however, it passes every packet to
> +the CPU.  On an old-style shared-media or hub-based network,
> +this allows the host to spy on all packets on the network.
> +But in the switched networks you'll find pretty much
> +everywhere these days, promiscuous mode doesn't have much
> +effect, because few packets not destined to a host are
> +delivered to the host's NIC.
> +
> +This form of promiscuous mode is configured in the guest OS of
> +the VMs on your bridge, e.g. with "ifconfig".
> +
> +  - The VMware vSwitch uses a different definition of "promiscuous
> +mode".  When you configure promiscuous mode on a VMware vNIC,
> +the vSwitch sends a copy of every packet received by the
> +vSwitch to that vNIC.  That has a much bigger effect than just
> +enabling promiscuous mode in a guest OS.  Rather than getting
> +a few stray packets for which the switch does not yet know the
> +correct destination, the vNIC gets every packet.  The effect
> +is similar to replacing the vSwitch by a virtual hub.
> +
> +This "promiscuous mode" is what switches normally call "port
> +mirroring" or "SPAN".  For information on how to configure
> +SPAN, see "How do I configure a port as a SPAN port, that is,
> +enable mirroring of all traffic to that port?"
> +
> Q: How do I configure a VLAN as an RSPAN VLAN, that is, enable
>mirroring of all traffic to that VLAN?
> 
> -- 
> 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


[ovs-dev] [PATCH] openvswitch: fix sw_flow_key alignment

2013-08-30 Thread Andy Zhou
sw_flow_key alignment was declared as " __aligned(__alignof__(long))"
However, this breaks on m68k architecture where long is 32 bit in size
but 16 bit aligned by default. Use __aligned(sizeof(long) instead.

Reported by: Fengguang Wu 

Signed-off-by: Andy Zhou 
---
 net/openvswitch/flow.c |4 +---
 net/openvswitch/flow.h |2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index ad1aeeb..fe7524c4 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -1009,9 +1009,6 @@ static u32 ovs_flow_hash(const struct sw_flow_key *key, 
int key_start,
u32 *hash_key = (u32 *)((u8 *)key + key_start);
int hash_u32s = (key_end - key_start) >> 2;
 
-   /* Make sure number of hash bytes are multiple of u32. */
-   BUILD_BUG_ON(sizeof(long) % sizeof(u32));
-
return jhash2(hash_key, hash_u32s, 0);
 }
 
@@ -1982,6 +1979,7 @@ nla_put_failure:
 int ovs_flow_init(void)
 {
BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long));
+   BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));
 
flow_cache = kmem_cache_create("sw_flow", sizeof(struct sw_flow), 0,
0, NULL);
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index b65f885..202c4c4 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -125,7 +125,7 @@ struct sw_flow_key {
} nd;
} ipv6;
};
-} __aligned(__alignof__(long));
+} __aligned(sizeof(long));
 
 struct sw_flow {
struct rcu_head rcu;
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Ben Pfaff
It doesn't apply directly to branch-1.11.  Can you take a look and see
whether there's some other commit we're missing there?

Thanks,

Ben.

On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> Yes, this patch fixes a bug.
> 
> 
> On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:
> 
> > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:
> > >
> > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > >  if (is_mask) {
> > > > >  if (!is_all_zeros((const uint8_t *) nd_key,
> > > > >sizeof *nd_key) &&
> > > > > -flow->tp_src != htons(0x)) {
> > > >
> > > > I suspect that && here should be ||.  Is that correct?  If so, then
> > > > I'll update the patch and commit this:
> > > >
> > > I agree. I was planing on send out a separate patch for this.
> >
> > Thanks, I folded that in and applied this to master and branch-2.0.
> > Do we want it on branch-1.11 also?
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:
> 
> > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > *attrs[OVS_KEY_ATTR_MAX + 1],
> > >  if (is_mask) {
> > >  if (!is_all_zeros((const uint8_t *) nd_key,
> > >sizeof *nd_key) &&
> > > -flow->tp_src != htons(0x)) {
> >
> > I suspect that && here should be ||.  Is that correct?  If so, then
> > I'll update the patch and commit this:
> >
> I agree. I was planing on send out a separate patch for this.

Thanks, I folded that in and applied this to master and branch-2.0.
Do we want it on branch-1.11 also?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr 
> *attrs[OVS_KEY_ATTR_MAX + 1],
>  if (is_mask) {
>  if (!is_all_zeros((const uint8_t *) nd_key,
>sizeof *nd_key) &&
> -flow->tp_src != htons(0x)) {

I suspect that && here should be ||.  Is that correct?  If so, then
I'll update the patch and commit this:

> +(flow->tp_src != htons(0x) &&
> + flow->tp_dst != htons(0x))) {
>  return ODP_FIT_ERROR;
>  } else {
>  expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
Thanks,

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


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:

> On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> *attrs[OVS_KEY_ATTR_MAX + 1],
> >  if (is_mask) {
> >  if (!is_all_zeros((const uint8_t *) nd_key,
> >sizeof *nd_key) &&
> > -flow->tp_src != htons(0x)) {
>
> I suspect that && here should be ||.  Is that correct?  If so, then
> I'll update the patch and commit this:
>
> I agree. I was planing on send out a separate patch for this.

> +(flow->tp_src != htons(0x) &&
> > + flow->tp_dst != htons(0x))) {
> >  return ODP_FIT_ERROR;
> >  } else {
> >  expected_attrs |= UINT64_C(1) <<
> OVS_KEY_ATTR_ND;
> Thanks,
>
> Ben.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
Yes, this patch fixes a bug.


On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:

> On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:
> >
> > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > >  if (is_mask) {
> > > >  if (!is_all_zeros((const uint8_t *) nd_key,
> > > >sizeof *nd_key) &&
> > > > -flow->tp_src != htons(0x)) {
> > >
> > > I suspect that && here should be ||.  Is that correct?  If so, then
> > > I'll update the patch and commit this:
> > >
> > I agree. I was planing on send out a separate patch for this.
>
> Thanks, I folded that in and applied this to master and branch-2.0.
> Do we want it on branch-1.11 also?
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V2 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command

2013-08-30 Thread Alex Wang
> After this patch, coverage_read() seems halfway between two designs.
> My goal was to avoid holding coverage_mutex while constructing the log
> message, to keep the hold time low, by copying out the totals and then
> building the message with the copies.  Your version retains the
> copy-out for totals but extends the mutex hold time across the log
> messages anyway since it needs access to new data.  I'd suggest doing
> the summations holding the lock but then dropping it before
> constructing the log mesage.
>

Yes, I'd very much like to do this. During my poking-around these two days,
I know lock contention better.



> I think that the units might be a little confusing.  All of the units
> are really per second, right?  It's just that they are per second over
> the last second, per second averaged over the last hour, per second
> averaged over the last day.  (Right?)  We might want to clarify that.
> I am not sure how to make it really clear in the coverage/show output,
> but writing the results in some way other than "/min", "/hr" might
> help, since people naturally read that as per-minute or per-hour.
>

No. We sample every 5 seconds.  Put the sample into the minute moving
averager.  So when calculating the per-minute rate, we sum all the 5-second
counts in the minute moving averager (12 5-sec slots = 1 minute count).
 The per-hour rate use the similar idea.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: fix sw_flow_key alignment

2013-08-30 Thread Jesse Gross
On Fri, Aug 30, 2013 at 10:32 AM, Andy Zhou  wrote:
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index ad1aeeb..fe7524c4 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
>  int ovs_flow_init(void)
>  {
> BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long));
> +   BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));

Should this be checking alignof struct sw_flow_key instead of the size?

> flow_cache = kmem_cache_create("sw_flow", sizeof(struct sw_flow), 0,
> 0, NULL);
> diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
> index b65f885..202c4c4 100644
> --- a/net/openvswitch/flow.h
> +++ b/net/openvswitch/flow.h
> @@ -125,7 +125,7 @@ struct sw_flow_key {
> } nd;
> } ipv6;
> };
> -} __aligned(__alignof__(long));
> +} __aligned(sizeof(long));

This is going to result in the alignment issue discussed yesterday
where on 32-bit machines the 64 bit members potentially become
misaligned. The suggestion that Geert made was to just drop this
entirely and rely on the natural alignment from these values.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V2 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command

2013-08-30 Thread Ben Pfaff
On Tue, Aug 27, 2013 at 09:25:10PM -0700, Alex Wang wrote:
> This commit changes the "ovs-appctl coverage/show" command to show the
> per-second, per-minute and per-hour rates of function invocation.  More
> importantly, this makes using coverage counter an easy way to monitor
> the execution of specific functions.
> 
> Signed-off-by: Alex Wang 

Thanks, Alex.

After this patch, coverage_read() seems halfway between two designs.
My goal was to avoid holding coverage_mutex while constructing the log
message, to keep the hold time low, by copying out the totals and then
building the message with the copies.  Your version retains the
copy-out for totals but extends the mutex hold time across the log
messages anyway since it needs access to new data.  I'd suggest doing
the summations holding the lock but then dropping it before
constructing the log mesage.

I think that the units might be a little confusing.  All of the units
are really per second, right?  It's just that they are per second over
the last second, per second averaged over the last hour, per second
averaged over the last day.  (Right?)  We might want to clarify that.
I am not sure how to make it really clear in the coverage/show output,
but writing the results in some way other than "/min", "/hr" might
help, since people naturally read that as per-minute or per-hour.

The comma is out of place here:
+static unsigned int min_idx ,hr_idx;

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


Re: [ovs-dev] [PATCH V2 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command

2013-08-30 Thread Ben Pfaff
On Fri, Aug 30, 2013 at 11:20:54AM -0700, Alex Wang wrote:
> > After this patch, coverage_read() seems halfway between two designs.
> > My goal was to avoid holding coverage_mutex while constructing the log
> > message, to keep the hold time low, by copying out the totals and then
> > building the message with the copies.  Your version retains the
> > copy-out for totals but extends the mutex hold time across the log
> > messages anyway since it needs access to new data.  I'd suggest doing
> > the summations holding the lock but then dropping it before
> > constructing the log mesage.
> >
> 
> Yes, I'd very much like to do this. During my poking-around these two days,
> I know lock contention better.

OK, great.

> > I think that the units might be a little confusing.  All of the units
> > are really per second, right?  It's just that they are per second over
> > the last second, per second averaged over the last hour, per second
> > averaged over the last day.  (Right?)  We might want to clarify that.
> > I am not sure how to make it really clear in the coverage/show output,
> > but writing the results in some way other than "/min", "/hr" might
> > help, since people naturally read that as per-minute or per-hour.
> >
> 
> No. We sample every 5 seconds.  Put the sample into the minute moving
> averager.  So when calculating the per-minute rate, we sum all the 5-second
> counts in the minute moving averager (12 5-sec slots = 1 minute count).
>  The per-hour rate use the similar idea.

Ah.  I understand now.  But I'm not sure that this is actually a good
way to do it, because it makes the output harder to interpret.  If I
see that the rate over the last 5 seconds is 123/s, for the last
minute is 4920/min, and for the last hour is 19680/h, then I have a
hard time figuring out what that means.  But if I see that the rate
over the last 5 seconds is 123/s, over the last minute is 82.0/s, and
over the last hour is 5.4/s (which are the same rates but per-second
rather than per-minute or per-hour) then I can immediately see that
the rates have recently spiked.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH V2 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command

2013-08-30 Thread Alex Wang
>
> Ah.  I understand now.  But I'm not sure that this is actually a good
> way to do it, because it makes the output harder to interpret.  If I
> see that the rate over the last 5 seconds is 123/s, for the last
> minute is 4920/min, and for the last hour is 19680/h, then I have a
> hard time figuring out what that means.  But if I see that the rate
> over the last 5 seconds is 123/s, over the last minute is 82.0/s, and
> over the last hour is 5.4/s (which are the same rates but per-second
> rather than per-minute or per-hour) then I can immediately see that
> the rates have recently spiked.
>

Yes, this makes more sense.  I'd like to adjust accordingly.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Recotizá tu seguro y ahorrá un 35%, congelamos tu cuota! todas las aseguradoras

2013-08-30 Thread abSeguro.com

+ Conseguí los mejores descuentos
+ Elegí entre las mejores empresas
+ Congelamos tu cuota semestralmente !!!

Porque... más atención y más beneficios es mucho más que un seguro!.

abSeguro.com/cotizar___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] openvswitch: fix sw_flow_key alignment

2013-08-30 Thread David Miller
From: Jesse Gross 
Date: Fri, 30 Aug 2013 11:22:11 -0700

> The suggestion that Geert made was to just drop this entirely and
> rely on the natural alignment from these values.

Indeed, Geert's patch was 1,000 times superior to this one.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] groups: Add groups API to ofproto-provider.h

2013-08-30 Thread Casey Barker
This declares a new API extension between ofproto and the ofproto-provider
to
support Openflow forwarding groups as introduced in the 1.1 spec. This patch
depends on an earlier submission that defines 'struct ofpbucket.' There is
no
executable code in this patch.

The API is modeled after that for flows, and supports asynchronous
completion
for construct, destruct, and modify.

Signed-off-by: Casey Barker 
---
Ben: As before, please feel free to accept or reject this.  (However, I'd be
surprised if your implementation were substantially different, as the API
mimics flows.)  Sorry I've been lax in getting my changes to you, but I'll
adapt to whatever you end up with.  Thanks!

Patch attached to prevent gmail from eating it.
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 820ec34..73aa082 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6356,6 +6356,12 @@ const struct ofproto_class ofproto_dpif_class = {
 rule_get_stats,
 rule_execute,
 rule_modify_actions,
+NULL,   /* group_alloc */
+NULL,   /* group_construct */
+NULL,   /* group_destruct */
+NULL,   /* group_dealloc */
+NULL,   /* group_get_stats */
+NULL,   /* group_modify */
 set_frag_handling,
 packet_out,
 set_netflow,
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index d6a8a0b..fc46d25 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -286,6 +286,23 @@ void ofproto_rule_reduce_timeouts(struct rule *rule, 
uint16_t idle_timeout,
 
 bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t out_port);
 
+/* An OpenFlow forwarding group within a "struct ofproto".
+ *
+ * As a rule, ofproto implementations may look at these fields but should
+ * not modify them. */
+struct group {
+struct ofproto *ofproto; /* The ofproto that contains this group. */
+struct hmap_node hmap_node;  /* In struct ofproto's 'groups' hmap. */
+struct list ofproto_node;/* Owned by ofproto base code. */
+struct ofoperation *pending; /* Operation now in progress, if nonnull. */
+long long int created;   /* Creation time. */
+
+uint32_t group_id;   /* Which group this is. */
+uint8_t type;/* What kind of group.  One of OFPGT_*. */
+struct ofpbucket *ofpbuckets;/* Array of "struct ofpbucket". */
+size_t n_ofpbuckets; /* Number of 'ofpbucket' in the array. */
+};
+
 void ofoperation_complete(struct ofoperation *, enum ofperr);
 
 bool ofoperation_has_out_port(const struct ofoperation *, ofp_port_t out_port);
@@ -1107,6 +1124,108 @@ struct ofproto_class {
  * rule. */
 void (*rule_modify_actions)(struct rule *rule, bool reset_counters);
 
+
+/* ##  ## */
+/* ## OpenFlow Group Functions ## */
+/* ##  ## */
+
+/* Life-cycle functions for a "struct group" (see "Life Cycle" above).
+ *
+ * Asynchronous Operation Support
+ * ==
+ *
+ * The life-cycle operations on groups can operate asynchronously,
+ * just as for rules.  See the rules description for more information.
+ *
+ * Construction
+ * 
+ *
+ * When ->group_construct() is called, the caller has already inserted
+ * 'group' into 'group->ofproto''s group table.  Unlike rules, there
+ * is no "victim" handling for groups.
+ *
+ * ->group_construct() should set the following in motion:
+ *
+ *   - Validate that 'group->buckets' and 'group->buckets_len' are
+ * well-formed OpenFlow action buckets that the datapath can
+ * correctly implement.
+ *
+ *   - If the group is valid, update the datapath group table, adding
+ * the new group or replacing the existing one.
+ *
+ * (On failure, the ofproto code will roll back the insertion from the
+ * group table, either removing 'group' or replacing it by the group
+ * that was originally in its place.)
+ *
+ * ->group_construct() must act in one of the following ways:
+ *
+ *   - If it succeeds, it must call ofoperation_complete() and return 0.
+ *
+ *   - If it fails, it must act in one of the following ways:
+ *
+ *   * Call ofoperation_complete() and return 0.
+ *
+ *   * Return an OpenFlow error code (as returned by ofp_mkerr()).
+ *(Do not call ofoperation_complete() in this case.)
+ *
+ * In either failure case, ->group_destruct() will not be called and
+ * ->group_dealloc() will be called.
+ *
+ *   - If the operation is only partially complete, then it must return
+ * 0.  Later, when the operation is complete, the ->run() or
+ * ->destruct() function must call ofoperation_complete() to
+ * report success or failure.
+ 

Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
May be it caused by missing commit 4a2216156e6e168 ?


On Fri, Aug 30, 2013 at 11:11 AM, Ben Pfaff  wrote:

> It doesn't apply directly to branch-1.11.  Can you take a look and see
> whether there's some other commit we're missing there?
>
> Thanks,
>
> Ben.
>
> On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> > Yes, this patch fixes a bug.
> >
> >
> > On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:
> >
> > > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:
> > > >
> > > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > > >  if (is_mask) {
> > > > > >  if (!is_all_zeros((const uint8_t *)
> nd_key,
> > > > > >sizeof *nd_key) &&
> > > > > > -flow->tp_src != htons(0x)) {
> > > > >
> > > > > I suspect that && here should be ||.  Is that correct?  If so, then
> > > > > I'll update the patch and commit this:
> > > > >
> > > > I agree. I was planing on send out a separate patch for this.
> > >
> > > Thanks, I folded that in and applied this to master and branch-2.0.
> > > Do we want it on branch-1.11 also?
> > >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Ben Pfaff
I still get conflicts:

blp@sigsegv:~/ovs$ git checkout openvswitch/branch-1.11
HEAD is now at 37f9d3a... Set release date for 1.11.0.
blp@sigsegv:~/ovs$ git cherry-pick 4a2216156e6e168
error: could not apply 4a22161... odp-util: New function 
odp_flow_key_to_mask().
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'
blp@sigsegv:~/ovs$ 

On Fri, Aug 30, 2013 at 01:15:49PM -0700, Andy Zhou wrote:
> May be it caused by missing commit 4a2216156e6e168 ?
> 
> 
> On Fri, Aug 30, 2013 at 11:11 AM, Ben Pfaff  wrote:
> 
> > It doesn't apply directly to branch-1.11.  Can you take a look and see
> > whether there's some other commit we're missing there?
> >
> > Thanks,
> >
> > Ben.
> >
> > On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> > > Yes, this patch fixes a bug.
> > >
> > >
> > > On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:
> > >
> > > > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff  wrote:
> > > > >
> > > > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gy...@nicira.com wrote:
> > > > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > > > >  if (is_mask) {
> > > > > > >  if (!is_all_zeros((const uint8_t *)
> > nd_key,
> > > > > > >sizeof *nd_key) &&
> > > > > > > -flow->tp_src != htons(0x)) {
> > > > > >
> > > > > > I suspect that && here should be ||.  Is that correct?  If so, then
> > > > > > I'll update the patch and commit this:
> > > > > >
> > > > > I agree. I was planing on send out a separate patch for this.
> > > >
> > > > Thanks, I folded that in and applied this to master and branch-2.0.
> > > > Do we want it on branch-1.11 also?
> > > >
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 00/11] datapath: Remove old compatibility code.

2013-08-30 Thread Pravin B Shelar
datapath support for older kernel is been broken for kernel
older than 2.6.32 for some time now. There have not been any
complains about this. So it makes sense to get rid of the code.
This make datapath module much close to upstream datapath and
make code easy to understand.
To make review easy I have broken down patches.

Pravin B Shelar (11):
  datapath: Move kernel version check to configure.
  datapath: vport: Remove compat support
  datapath: Remove namespace compat support.
  datapath: Remove skb->mask compat code.
  datapath: Remove checksum compat support
  datapath: Remove vlan compat support
  datapath: Cleanup netlink compat code.
  datapath: Cleanup compat support.
  datapath: Remove compat support for NLA_NUL_STRING
  datapath: Remove reciprocal_div compat code.
  datapath: Remove compat header files.

 acinclude.m4   |   16 +-
 datapath/Modules.mk|3 -
 datapath/actions.c |   17 +-
 datapath/checksum.c|  271 
 datapath/checksum.h|  173 
 datapath/compat.h  |   73 
 datapath/datapath.c|   95 +
 datapath/datapath.h|   15 -
 datapath/dp_notify.c   |2 +-
 datapath/flow.c|9 +-
 datapath/linux/Modules.mk  |   20 +-
 datapath/linux/compat/addrconf_core-openvswitch.c  |   82 
 datapath/linux/compat/genetlink-openvswitch.c  |  132 --
 datapath/linux/compat/include/linux/dmi.h  |  114 -
 datapath/linux/compat/include/linux/if.h   |7 -
 datapath/linux/compat/include/linux/if_ether.h |   13 -
 datapath/linux/compat/include/linux/inetdevice.h   |   14 -
 datapath/linux/compat/include/linux/kernel.h   |   31 --
 datapath/linux/compat/include/linux/kobject.h  |   30 --
 datapath/linux/compat/include/linux/lockdep.h  |  449 
 datapath/linux/compat/include/linux/mutex.h|   59 ---
 datapath/linux/compat/include/linux/netdevice.h|   88 
 .../linux/compat/include/linux/netfilter_bridge.h  |   24 -
 .../linux/compat/include/linux/netfilter_ipv4.h|   19 -
 datapath/linux/compat/include/linux/netlink.h  |   16 -
 datapath/linux/compat/include/linux/rculist.h  |   18 -
 .../linux/compat/include/linux/reciprocal_div.h|   40 --
 datapath/linux/compat/include/linux/rtnetlink.h|   43 --
 datapath/linux/compat/include/linux/skbuff.h   |   45 --
 datapath/linux/compat/include/linux/slab.h |   31 --
 datapath/linux/compat/include/linux/timer.h|   96 -
 datapath/linux/compat/include/net/checksum.h   |   12 +-
 datapath/linux/compat/include/net/genetlink.h  |  167 +---
 datapath/linux/compat/include/net/ip.h |6 -
 datapath/linux/compat/include/net/net_namespace.h  |   80 +
 datapath/linux/compat/include/net/netlink.h|  113 -
 datapath/linux/compat/include/net/netns/generic.h  |   12 -
 datapath/linux/compat/include/net/protocol.h   |   12 -
 datapath/linux/compat/include/net/route.h  |   21 -
 datapath/linux/compat/include/net/sctp/checksum.h  |5 -
 datapath/linux/compat/include/net/sock.h   |   15 -
 datapath/linux/compat/ip_output-openvswitch.c  |   33 --
 datapath/linux/compat/ip_tunnels_core.c|4 -
 datapath/linux/compat/kmemdup.c|   22 -
 datapath/linux/compat/net_namespace.c  |   35 --
 datapath/linux/compat/netdevice.c  |4 -
 datapath/linux/compat/reciprocal_div.c |1 +
 datapath/linux/compat/skbuff-openvswitch.c |   22 -
 datapath/linux/compat/time.c   |   39 --
 datapath/linux/compat/utils.c  |   39 ++
 datapath/linux/compat/vxlan.c  |   11 +-
 datapath/vlan.c|   58 ---
 datapath/vlan.h|   39 --
 datapath/vport-gre.c   |   15 +-
 datapath/vport-internal_dev.c  |   32 +-
 datapath/vport-lisp.c  |   28 +-
 datapath/vport-netdev.c|   23 +-
 datapath/vport-vxlan.c |8 +-
 datapath/vport.c   |2 -
 59 files changed, 122 insertions(+), 2781 deletions(-)
 delete mode 100644 datapath/checksum.c
 delete mode 100644 datapath/checksum.h
 delete mode 100644 datapath/linux/compat/addrconf_core-openvswitch.c
 delete mode 100644 datapath/linux/compat/include/linux/dmi.h
 delete mode 100644 datapath/linux/compat/include/linux/inetdevice.h
 delete mode 100644 datapath/linux/compat/include/linux/kobject.h
 delete mode 100644 datapath/linux/compat/include/linux/lockdep.

[ovs-dev] [PATCH 08/11] datapath: Cleanup compat support.

2013-08-30 Thread Pravin B Shelar
cleanup various header file.

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/include/linux/if_ether.h  |   13 
 datapath/linux/compat/include/linux/kernel.h|   31 -
 datapath/linux/compat/include/linux/netdevice.h |   81 ---
 datapath/linux/compat/include/linux/rculist.h   |   18 -
 datapath/linux/compat/include/linux/skbuff.h|   45 -
 5 files changed, 0 insertions(+), 188 deletions(-)

diff --git a/datapath/linux/compat/include/linux/if_ether.h 
b/datapath/linux/compat/include/linux/if_ether.h
index e22ea96..25f63ca 100644
--- a/datapath/linux/compat/include/linux/if_ether.h
+++ b/datapath/linux/compat/include/linux/if_ether.h
@@ -3,19 +3,6 @@
 
 #include_next 
 
-#include 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
-
-#define ETH_P_TEB  0x6558  /* Trans Ether Bridging */
-
-#endif /* linux kernel < 2.6.28 */
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,30)
-
-#define ETH_P_FCOE 0x8906  /* Fibre Channel over Ethernet  */
-
-#endif /* linux kernel < 2.6.30 */
-
 #ifndef ETH_P_802_3_MIN
 #define ETH_P_802_3_MIN0x0600
 #endif
diff --git a/datapath/linux/compat/include/linux/kernel.h 
b/datapath/linux/compat/include/linux/kernel.h
index 6e248c5..5dfe08e 100644
--- a/datapath/linux/compat/include/linux/kernel.h
+++ b/datapath/linux/compat/include/linux/kernel.h
@@ -8,33 +8,6 @@
 
 #include 
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
-#undef pr_emerg
-#define pr_emerg(fmt, ...) \
-   printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_alert
-#define pr_alert(fmt, ...) \
-   printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_crit
-#define pr_crit(fmt, ...) \
-   printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_err
-#define pr_err(fmt, ...) \
-   printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_warning
-#define pr_warning(fmt, ...) \
-   printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_notice
-#define pr_notice(fmt, ...) \
-   printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_info
-#define pr_info(fmt, ...) \
-   printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
-#undef pr_cont
-#define pr_cont(fmt, ...) \
-   printk(KERN_CONT fmt, ##__VA_ARGS__)
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,35)
 #define pr_warn pr_warning
 #endif
@@ -73,10 +46,6 @@
 
 #endif
 
-#if defined(CONFIG_PREEMPT) && LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
-#error "CONFIG_PREEMPT is broken before 2.6.21--see commit 4498121ca3, 
\"[NET]: Handle disabled preemption in gfp_any()\""
-#endif
-
 #ifndef USHRT_MAX
 #define USHRT_MAX  ((u16)(~0U))
 #define SHRT_MAX   ((s16)(USHRT_MAX>>1))
diff --git a/datapath/linux/compat/include/linux/netdevice.h 
b/datapath/linux/compat/include/linux/netdevice.h
index bd7a4b0..4e2b7f5 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -6,15 +6,6 @@
 struct net;
 
 #include 
-/* Before 2.6.21, struct net_device has a "struct class_device" member named
- * class_dev.  Beginning with 2.6.21, struct net_device instead has a "struct
- * device" member named dev.  Otherwise the usage of these members is pretty
- * much the same. */
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
-#define NETDEV_DEV_MEMBER class_dev
-#else
-#define NETDEV_DEV_MEMBER dev
-#endif
 
 #ifndef to_net_dev
 #define to_net_dev(class) container_of(class, struct net_device, 
NETDEV_DEV_MEMBER)
@@ -25,53 +16,6 @@ extern struct sk_buff 
*(*openvswitch_handle_frame_hook)(struct sk_buff *skb);
 extern int nr_bridges;
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
-static inline
-struct net *dev_net(const struct net_device *dev)
-{
-#ifdef CONFIG_NET_NS
-   return dev->nd_net;
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,25)
-   return &init_net;
-#else
-   return NULL;
-#endif
-}
-
-static inline
-void dev_net_set(struct net_device *dev, const struct net *net)
-{
-#ifdef CONFIG_NET_NS
-   dev->nd_dev = net;
-#endif
-}
-#endif /* linux kernel < 2.6.26 */
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,24)
-#define NETIF_F_NETNS_LOCAL 0
-#endif
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,24)
-#define proc_net init_net.proc_net
-#endif
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-typedef int netdev_tx_t;
-#endif
-
-#ifndef for_each_netdev
-/* Linux before 2.6.22 didn't have for_each_netdev at all. */
-#define for_each_netdev(net, d) for (d = dev_base; d; d = d->next)
-#elif LINUX_VERSION_CODE < KERNEL_VERSION(2,6,24)
-/* Linux 2.6.24 added a network namespace pointer to the macro. */
-#undef for_each_netdev
-#define for_each_netdev(net, d) list_for_each_entry(d, &dev_base_head, 
dev_list)
-#endif
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
-#define net_xmit_eval(e)   ((e) == NET_XMIT_CN ? 0 : (e))
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
 extern void unregister_netdevice_queue(struct net_device *dev,
   

[ovs-dev] [PATCH 06/11] datapath: Remove vlan compat support

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/Modules.mk   |1 -
 datapath/datapath.c   |   10 --
 datapath/datapath.h   |5 ---
 datapath/linux/compat/netdevice.c |4 --
 datapath/linux/compat/vxlan.c |   10 +-
 datapath/vlan.c   |   58 -
 datapath/vlan.h   |   34 -
 datapath/vport-gre.c  |   11 +--
 datapath/vport-internal_dev.c |   12 +--
 datapath/vport-lisp.c |   10 +-
 datapath/vport-netdev.c   |2 -
 11 files changed, 38 insertions(+), 119 deletions(-)
 delete mode 100644 datapath/vlan.c

diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index e2a4dad..7ddf79c 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -11,7 +11,6 @@ openvswitch_sources = \
datapath.c \
dp_notify.c \
flow.c \
-   vlan.c \
vport.c \
vport-gre.c \
vport-internal_dev.c \
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 9ed213e..bbec48f 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -411,10 +411,12 @@ static int queue_userspace_packet(struct net *net, int 
dp_ifindex,
nskb = skb_clone(skb, GFP_ATOMIC);
if (!nskb)
return -ENOMEM;
-   
-   err = vlan_deaccel_tag(nskb);
-   if (err)
-   return err;
+   
+   nskb = __vlan_put_tag(nskb, nskb->vlan_proto, 
vlan_tx_tag_get(nskb));
+   if (!nskb)
+   return -ENOMEM;
+
+   vlan_set_tci(nskb, 0);
 
skb = nskb;
}
diff --git a/datapath/datapath.h b/datapath/datapath.h
index e3cd2f7..5d50dd4 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -93,16 +93,11 @@ struct datapath {
  * @pkt_key: The flow information extracted from the packet.  Must be nonnull.
  * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
  * packet is not being tunneled.
- * @vlan_tci: Provides a substitute for the skb->vlan_tci field on kernels
- * before 2.6.27.
  */
 struct ovs_skb_cb {
struct sw_flow  *flow;
struct sw_flow_key  *pkt_key;
struct ovs_key_ipv4_tunnel  *tun_key;
-#ifdef NEED_VLAN_FIELD
-   u16 vlan_tci;
-#endif
 };
 #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
 
diff --git a/datapath/linux/compat/netdevice.c 
b/datapath/linux/compat/netdevice.c
index f03efde..248066d 100644
--- a/datapath/linux/compat/netdevice.c
+++ b/datapath/linux/compat/netdevice.c
@@ -49,11 +49,7 @@ static u32 harmonize_features(struct sk_buff *skb, __be16 
protocol, u32 features
 
 u32 rpl_netif_skb_features(struct sk_buff *skb)
 {
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
-   unsigned long vlan_features = 0;
-#else
unsigned long vlan_features = skb->dev->vlan_features;
-#endif /* kernel version < 2.6.26 */
 
__be16 protocol = skb->protocol;
u32 features = skb->dev->features;
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 780344e..d774b6c 100644
--- a/datapath/linux/compat/vxlan.c
+++ b/datapath/linux/compat/vxlan.c
@@ -230,8 +230,14 @@ int vxlan_xmit_skb(struct net *net, struct vxlan_sock *vs,
if (unlikely(err))
return err;
 
-   if (unlikely(vlan_deaccel_tag(skb)))
-   return -ENOMEM;
+   if (vlan_tx_tag_present(skb)) {
+   if (unlikely(!__vlan_put_tag(skb,
+   skb->vlan_proto,
+   vlan_tx_tag_get(skb
+   return -ENOMEM;
+
+   vlan_set_tci(skb, 0);
+   }
 
vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
vxh->vx_flags = htonl(VXLAN_FLAGS);
diff --git a/datapath/vlan.c b/datapath/vlan.c
deleted file mode 100644
index 104ed55..000
--- a/datapath/vlan.c
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * Copyright (c) 2007-2011 Nicira, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include 
-#include 
-
-#include "datapath.h"
-#include "vlan.h"
-
-#ifdef NEED_VLAN_FIELD
-void vlan_copy_skb_tci(struct sk_buff *skb)
-{
- 

[ovs-dev] [PATCH 11/11] datapath: Remove compat files.

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 acinclude.m4   |3 -
 datapath/linux/Modules.mk  |   16 -
 datapath/linux/compat/addrconf_core-openvswitch.c  |   82 
 datapath/linux/compat/include/linux/dmi.h  |  114 -
 datapath/linux/compat/include/linux/inetdevice.h   |   14 -
 datapath/linux/compat/include/linux/kobject.h  |   30 --
 datapath/linux/compat/include/linux/lockdep.h  |  449 
 datapath/linux/compat/include/linux/mutex.h|   59 ---
 .../linux/compat/include/linux/netfilter_bridge.h  |   24 -
 .../linux/compat/include/linux/netfilter_ipv4.h|   19 -
 datapath/linux/compat/include/linux/slab.h |   31 --
 datapath/linux/compat/include/linux/timer.h|   96 -
 datapath/linux/compat/include/net/protocol.h   |   12 -
 datapath/linux/compat/include/net/route.h  |   21 -
 datapath/linux/compat/include/net/sock.h   |   15 -
 datapath/linux/compat/ip_output-openvswitch.c  |   33 --
 datapath/linux/compat/kmemdup.c|   22 -
 datapath/linux/compat/time.c   |   39 --
 18 files changed, 0 insertions(+), 1079 deletions(-)
 delete mode 100644 datapath/linux/compat/addrconf_core-openvswitch.c
 delete mode 100644 datapath/linux/compat/include/linux/dmi.h
 delete mode 100644 datapath/linux/compat/include/linux/inetdevice.h
 delete mode 100644 datapath/linux/compat/include/linux/kobject.h
 delete mode 100644 datapath/linux/compat/include/linux/lockdep.h
 delete mode 100644 datapath/linux/compat/include/linux/mutex.h
 delete mode 100644 datapath/linux/compat/include/linux/netfilter_bridge.h
 delete mode 100644 datapath/linux/compat/include/linux/netfilter_ipv4.h
 delete mode 100644 datapath/linux/compat/include/linux/slab.h
 delete mode 100644 datapath/linux/compat/include/linux/timer.h
 delete mode 100644 datapath/linux/compat/include/net/protocol.h
 delete mode 100644 datapath/linux/compat/include/net/route.h
 delete mode 100644 datapath/linux/compat/include/net/sock.h
 delete mode 100644 datapath/linux/compat/ip_output-openvswitch.c
 delete mode 100644 datapath/linux/compat/kmemdup.c
 delete mode 100644 datapath/linux/compat/time.c

diff --git a/acinclude.m4 b/acinclude.m4
index c5c2d62..c4ea012 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -266,9 +266,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_reset_mac_len])
   OVS_GREP_IFELSE([$KSRC/include/linux/skbuff.h], [skb_unclone])
 
-  OVS_GREP_IFELSE([$KSRC/include/linux/string.h], [kmemdup], [],
-  [OVS_GREP_IFELSE([$KSRC/include/linux/slab.h], [kmemdup])])
-
   OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [bool],
   [OVS_DEFINE([HAVE_BOOL_TYPE])])
   OVS_GREP_IFELSE([$KSRC/include/linux/types.h], [__wsum],
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 085e8fd..0a780dc 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -1,5 +1,4 @@
 openvswitch_sources += \
-   linux/compat/addrconf_core-openvswitch.c \
linux/compat/dev-openvswitch.c \
linux/compat/exthdrs_core.c \
linux/compat/flex_array.c \
@@ -7,14 +6,11 @@ openvswitch_sources += \
linux/compat/gre.c \
linux/compat/gso.c \
linux/compat/genetlink-openvswitch.c \
-   linux/compat/ip_output-openvswitch.c \
linux/compat/ip_tunnels_core.c \
-   linux/compat/kmemdup.c \
linux/compat/netdevice.c \
linux/compat/net_namespace.c \
linux/compat/reciprocal_div.c \
linux/compat/skbuff-openvswitch.c \
-   linux/compat/time.c \
linux/compat/vxlan.c\
linux/compat/workqueue.c \
linux/compat/utils.c
@@ -24,7 +20,6 @@ openvswitch_headers += \
linux/compat/include/linux/compiler.h \
linux/compat/include/linux/compiler-gcc.h \
linux/compat/include/linux/cpumask.h \
-   linux/compat/include/linux/dmi.h \
linux/compat/include/linux/err.h \
linux/compat/include/linux/etherdevice.h \
linux/compat/include/linux/flex_array.h \
@@ -36,22 +31,16 @@ openvswitch_headers += \
linux/compat/include/linux/if_tunnel.h \
linux/compat/include/linux/if_vlan.h \
linux/compat/include/linux/in.h \
-   linux/compat/include/linux/inetdevice.h \
linux/compat/include/linux/ip.h \
linux/compat/include/linux/ipv6.h \
linux/compat/include/linux/jiffies.h \
linux/compat/include/linux/kconfig.h \
linux/compat/include/linux/kernel.h \
-   linux/compat/include/linux/kobject.h \
linux/compat/include/linux/list.h \
-   linux/compat/include/linux/lockdep.h \
linux/compat/include/linux/log2.h \
-   linux/compat/include/linux/mutex.h \
linux/compat/include/linux/net.h \
linux/compat/include/linux/netdevice.h \
linux/compat/include/linux/netdev_

Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
It will be simpler if we just fix the bug instead of pulling in more
patches. I will derive a patch and send it out.


On Fri, Aug 30, 2013 at 1:19 PM, Ben Pfaff  wrote:

> I still get conflicts:
>
> blp@sigsegv:~/ovs$ git checkout openvswitch/branch-1.11
> HEAD is now at 37f9d3a... Set release date for 1.11.0.
> blp@sigsegv:~/ovs$ git cherry-pick 4a2216156e6e168
> error: could not apply 4a22161... odp-util: New function
> odp_flow_key_to_mask().
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add ' or 'git rm '
> hint: and commit the result with 'git commit'
> blp@sigsegv:~/ovs$
>
> On Fri, Aug 30, 2013 at 01:15:49PM -0700, Andy Zhou wrote:
> > May be it caused by missing commit 4a2216156e6e168 ?
> >
> >
> > On Fri, Aug 30, 2013 at 11:11 AM, Ben Pfaff  wrote:
> >
> > > It doesn't apply directly to branch-1.11.  Can you take a look and see
> > > whether there's some other commit we're missing there?
> > >
> > > Thanks,
> > >
> > > Ben.
> > >
> > > On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> > > > Yes, this patch fixes a bug.
> > > >
> > > >
> > > > On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:
> > > >
> > > > > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > > > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff 
> wrote:
> > > > > >
> > > > > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gyang@nicira.comwrote:
> > > > > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > > > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > > > > >  if (is_mask) {
> > > > > > > >  if (!is_all_zeros((const uint8_t *)
> > > nd_key,
> > > > > > > >sizeof *nd_key) &&
> > > > > > > > -flow->tp_src != htons(0x)) {
> > > > > > >
> > > > > > > I suspect that && here should be ||.  Is that correct?  If so,
> then
> > > > > > > I'll update the patch and commit this:
> > > > > > >
> > > > > > I agree. I was planing on send out a separate patch for this.
> > > > >
> > > > > Thanks, I folded that in and applied this to master and branch-2.0.
> > > > > Do we want it on branch-1.11 also?
> > > > >
> > >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Ben Pfaff
That's perfect, thanks a lot!

On Fri, Aug 30, 2013 at 01:26:44PM -0700, Andy Zhou wrote:
> It will be simpler if we just fix the bug instead of pulling in more
> patches. I will derive a patch and send it out.
> 
> 
> On Fri, Aug 30, 2013 at 1:19 PM, Ben Pfaff  wrote:
> 
> > I still get conflicts:
> >
> > blp@sigsegv:~/ovs$ git checkout openvswitch/branch-1.11
> > HEAD is now at 37f9d3a... Set release date for 1.11.0.
> > blp@sigsegv:~/ovs$ git cherry-pick 4a2216156e6e168
> > error: could not apply 4a22161... odp-util: New function
> > odp_flow_key_to_mask().
> > hint: after resolving the conflicts, mark the corrected paths
> > hint: with 'git add ' or 'git rm '
> > hint: and commit the result with 'git commit'
> > blp@sigsegv:~/ovs$
> >
> > On Fri, Aug 30, 2013 at 01:15:49PM -0700, Andy Zhou wrote:
> > > May be it caused by missing commit 4a2216156e6e168 ?
> > >
> > >
> > > On Fri, Aug 30, 2013 at 11:11 AM, Ben Pfaff  wrote:
> > >
> > > > It doesn't apply directly to branch-1.11.  Can you take a look and see
> > > > whether there's some other commit we're missing there?
> > > >
> > > > Thanks,
> > > >
> > > > Ben.
> > > >
> > > > On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> > > > > Yes, this patch fixes a bug.
> > > > >
> > > > >
> > > > > On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff  wrote:
> > > > >
> > > > > > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > > > > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff 
> > wrote:
> > > > > > >
> > > > > > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700, gyang@nicira.comwrote:
> > > > > > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > > > > > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > > > > > >  if (is_mask) {
> > > > > > > > >  if (!is_all_zeros((const uint8_t *)
> > > > nd_key,
> > > > > > > > >sizeof *nd_key) &&
> > > > > > > > > -flow->tp_src != htons(0x)) {
> > > > > > > >
> > > > > > > > I suspect that && here should be ||.  Is that correct?  If so,
> > then
> > > > > > > > I'll update the patch and commit this:
> > > > > > > >
> > > > > > > I agree. I was planing on send out a separate patch for this.
> > > > > >
> > > > > > Thanks, I folded that in and applied this to master and branch-2.0.
> > > > > > Do we want it on branch-1.11 also?
> > > > > >
> > > >
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 04/11] datapath: Remove skb->mask compat code.

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/actions.c |2 +-
 datapath/compat.h  |   39 ---
 datapath/datapath.c|   10 ++
 datapath/flow.c|9 ++---
 datapath/vport-gre.c   |2 +-
 datapath/vport-lisp.c  |2 +-
 datapath/vport-vxlan.c |2 +-
 7 files changed, 8 insertions(+), 58 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index fa4b904..33633df 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -478,7 +478,7 @@ static int execute_set_action(struct sk_buff *skb,
break;
 
case OVS_KEY_ATTR_SKB_MARK:
-   skb_set_mark(skb, nla_get_u32(nested_attr));
+   skb->mark = nla_get_u32(nested_attr);
break;
 
case OVS_KEY_ATTR_IPV4_TUNNEL:
diff --git a/datapath/compat.h b/datapath/compat.h
index 8457dbf..c786b97 100644
--- a/datapath/compat.h
+++ b/datapath/compat.h
@@ -72,40 +72,6 @@ static inline void skb_clear_rxhash(struct sk_buff *skb)
 #define SET_PARALLEL_OPS
 #endif
 
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
-#ifdef CONFIG_NETFILTER
-static inline u32 skb_get_mark(struct sk_buff *skb)
-{
-   return skb->nfmark;
-}
-
-static inline void skb_set_mark(struct sk_buff *skb, u32 mark)
-{
-   skb->nfmark = mark;
-}
-#else /* CONFIG_NETFILTER */
-static inline u32 skb_get_mark(struct sk_buff *skb)
-{
-   return 0;
-}
-
-static inline void skb_set_mark(struct sk_buff *skb, u32 mark)
-{
-}
-#endif
-#else /* before 2.6.20 */
-static inline u32 skb_get_mark(struct sk_buff *skb)
-{
-   return skb->mark;
-}
-
-static inline void skb_set_mark(struct sk_buff *skb, u32 mark)
-{
-   skb->mark = mark;
-}
-#endif /* after 2.6.20 */
-
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,36)
 #define rt_dst(rt) (rt->dst)
 #else
@@ -130,13 +96,8 @@ static inline struct rtable *find_route(struct net *net,
struct flowi fl = { .nl_u = { .ip4_u = {
.daddr = daddr,
.saddr = *saddr,
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
-   .fwmark = skb_mark,
-#endif
.tos   = RT_TOS(tos) } },
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20)
.mark = skb_mark,
-#endif
.proto = ipproto };
 
if (unlikely(ip_route_output_key(net, &rt, &fl)))
diff --git a/datapath/datapath.c b/datapath/datapath.c
index b6410c4..1f7560a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -657,14 +657,8 @@ static int validate_set(const struct nlattr *a,
int err;
 
case OVS_KEY_ATTR_PRIORITY:
-   case OVS_KEY_ATTR_ETHERNET:
-   break;
-
case OVS_KEY_ATTR_SKB_MARK:
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) && !defined(CONFIG_NETFILTER)
-   if (nla_get_u32(ovs_key) != 0)
-   return -EINVAL;
-#endif
+   case OVS_KEY_ATTR_ETHERNET:
break;
 
case OVS_KEY_ATTR_TUNNEL:
@@ -927,7 +921,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, 
struct genl_info *info)
OVS_CB(packet)->flow = flow;
OVS_CB(packet)->pkt_key = &flow->key;
packet->priority = flow->key.phy.priority;
-   skb_set_mark(packet, flow->key.phy.skb_mark);
+   packet->mark = flow->key.phy.skb_mark;
 
rcu_read_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
diff --git a/datapath/flow.c b/datapath/flow.c
index 7a697a4..d0b0dcd 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -853,7 +853,7 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, 
struct sw_flow_key *key)
if (OVS_CB(skb)->tun_key)
memcpy(&key->tun_key, OVS_CB(skb)->tun_key, 
sizeof(key->tun_key));
key->phy.in_port = in_port;
-   key->phy.skb_mark = skb_get_mark(skb);
+   key->phy.skb_mark = skb->mark;
 
skb_reset_mac_header(skb);
 
@@ -1377,12 +1377,7 @@ static int metadata_from_nlattrs(struct sw_flow_match 
*match,  u64 *attrs,
 
if (*attrs & (1ULL << OVS_KEY_ATTR_SKB_MARK)) {
uint32_t mark = nla_get_u32(a[OVS_KEY_ATTR_SKB_MARK]);
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) && !defined(CONFIG_NETFILTER)
-   if (!is_mask && mark != 0) {
-   OVS_NLERR("skb->mark must be zero on this kernel 
(mark=%d).\n", mark);
-   return -EINVAL;
-   }
-#endif
+
SW_FLOW_KEY_PUT(match, phy.skb_mark, mark, is_mask);
*attrs &= ~(1ULL << OVS_KEY_ATTR_SKB_MARK);
}
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index 5af6dbe..b9fff8a 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -138,7 +138,7 @@ static int __send(struct vport *vport, struct sk_buff *skb,
OVS_CB(skb)->tun_key->ipv4_dst,

[ovs-dev] [PATCH 09/11] datapath: Remove compat support for NLA_NUL_STRING

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 acinclude.m4|1 -
 datapath/compat.h   |   26 
 datapath/datapath.c |   55 ---
 3 files changed, 0 insertions(+), 82 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index e754cfa..c5c2d62 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -280,7 +280,6 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   OVS_GREP_IFELSE([$KSRC/include/net/checksum.h], [csum_unfold])
 
   OVS_GREP_IFELSE([$KSRC/include/net/genetlink.h], [parallel_ops])
-  OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [NLA_NUL_STRING])
   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_get_be16])
   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_put_be16])
   OVS_GREP_IFELSE([$KSRC/include/net/netlink.h], [nla_put_be32])
diff --git a/datapath/compat.h b/datapath/compat.h
index 2873f0a..bc7e880 100644
--- a/datapath/compat.h
+++ b/datapath/compat.h
@@ -25,32 +25,6 @@
 #include 
 #include 
 
-
-#ifndef HAVE_NLA_NUL_STRING
-static inline int CHECK_NUL_STRING(struct nlattr *attr, int maxlen)
-{
-   char *s;
-   int len;
-   if (!attr)
-   return 0;
-
-   len = nla_len(attr);
-   if (len >= maxlen)
-   return -EINVAL;
-
-   s = nla_data(attr);
-   if (s[len - 1] != '\0')
-   return -EINVAL;
-
-   return 0;
-}
-#else
-static inline int CHECK_NUL_STRING(struct nlattr *attr, int maxlen)
-{
-   return 0;
-}
-#endif  /* !HAVE_NLA_NUL_STRING */
-
 static inline void skb_clear_rxhash(struct sk_buff *skb)
 {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index dbdb6e4..fd4af42 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -947,11 +947,7 @@ err:
 }
 
 static const struct nla_policy packet_policy[OVS_PACKET_ATTR_MAX + 1] = {
-#if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,18)
[OVS_PACKET_ATTR_PACKET] = { .len = ETH_HLEN },
-#else
-   [OVS_PACKET_ATTR_PACKET] = { .minlen = ETH_HLEN },
-#endif
[OVS_PACKET_ATTR_KEY] = { .type = NLA_NESTED },
[OVS_PACKET_ATTR_ACTIONS] = { .type = NLA_NESTED },
 };
@@ -1571,9 +1567,7 @@ static struct genl_ops dp_flow_genl_ops[] = {
 };
 
 static const struct nla_policy datapath_policy[OVS_DP_ATTR_MAX + 1] = {
-#ifdef HAVE_NLA_NUL_STRING
[OVS_DP_ATTR_NAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ - 1 },
-#endif
[OVS_DP_ATTR_UPCALL_PID] = { .type = NLA_U32 },
 };
 
@@ -1651,11 +1645,6 @@ static struct sk_buff *ovs_dp_cmd_build_info(struct 
datapath *dp, u32 portid,
return skb;
 }
 
-static int ovs_dp_cmd_validate(struct nlattr *a[OVS_DP_ATTR_MAX + 1])
-{
-   return CHECK_NUL_STRING(a[OVS_DP_ATTR_NAME], IFNAMSIZ - 1);
-}
-
 /* Called with ovs_mutex. */
 static struct datapath *lookup_datapath(struct net *net,
struct ovs_header *ovs_header,
@@ -1690,10 +1679,6 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])
goto err;
 
-   err = ovs_dp_cmd_validate(a);
-   if (err)
-   goto err;
-
ovs_lock();
 
err = -ENOMEM;
@@ -1803,10 +1788,6 @@ static int ovs_dp_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
struct datapath *dp;
int err;
 
-   err = ovs_dp_cmd_validate(info->attrs);
-   if (err)
-   return err;
-
ovs_lock();
dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
err = PTR_ERR(dp);
@@ -1836,10 +1817,6 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
struct datapath *dp;
int err;
 
-   err = ovs_dp_cmd_validate(info->attrs);
-   if (err)
-   return err;
-
ovs_lock();
dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
err = PTR_ERR(dp);
@@ -1871,10 +1848,6 @@ static int ovs_dp_cmd_get(struct sk_buff *skb, struct 
genl_info *info)
struct datapath *dp;
int err;
 
-   err = ovs_dp_cmd_validate(info->attrs);
-   if (err)
-   return err;
-
ovs_lock();
dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
if (IS_ERR(dp)) {
@@ -1945,12 +1918,8 @@ static struct genl_ops dp_datapath_genl_ops[] = {
 };
 
 static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
-#ifdef HAVE_NLA_NUL_STRING
[OVS_VPORT_ATTR_NAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ - 1 },
[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
-#else
-   [OVS_VPORT_ATTR_STATS] = { .minlen = sizeof(struct ovs_vport_stats) },
-#endif
[OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
[OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
[OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_U32 },
@@ -2027,11 +1996,6 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vp

[ovs-dev] [PATCH 10/11] datapath: Remove reciprocal_div compat code.

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 .../linux/compat/include/linux/reciprocal_div.h|   40 
 datapath/linux/compat/reciprocal_div.c |1 +
 2 files changed, 1 insertions(+), 40 deletions(-)
 delete mode 100644 datapath/linux/compat/include/linux/reciprocal_div.h

diff --git a/datapath/linux/compat/include/linux/reciprocal_div.h 
b/datapath/linux/compat/include/linux/reciprocal_div.h
deleted file mode 100644
index f618bdd..000
--- a/datapath/linux/compat/include/linux/reciprocal_div.h
+++ /dev/null
@@ -1,40 +0,0 @@
-#ifndef __LINUX_RECIPROCAL_DIV_WRAPPER_H
-#define __LINUX_RECIPROCAL_DIV_WRAPPER_H
-
-#include 
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,20)
-#include_next 
-#else
-
-#include 
-
-/*
- * This file describes reciprocical division.
- *
- * This optimizes the (A/B) problem, when A and B are two u32
- * and B is a known value (but not known at compile time)
- *
- * The math principle used is :
- *   Let RECIPROCAL_VALUE(B) be (((1LL << 32) + (B - 1))/ B)
- *   Then A / B = (u32)(((u64)(A) * (R)) >> 32)
- *
- * This replaces a divide by a multiply (and a shift), and
- * is generally less expensive in CPU cycles.
- */
-
-/*
- * Computes the reciprocal value (R) for the value B of the divisor.
- * Should not be called before each reciprocal_divide(),
- * or else the performance is slower than a normal divide.
- */
-extern u32 reciprocal_value(u32 B);
-
-
-static inline u32 reciprocal_divide(u32 A, u32 R)
-{
-   return (u32)(((u64)A * R) >> 32);
-}
-
-#endif /* Linux kernel < 2.6.20 */
-#endif /* __LINUX_RECIPROCAL_DIV_WRAPPER_H */
diff --git a/datapath/linux/compat/reciprocal_div.c 
b/datapath/linux/compat/reciprocal_div.c
index 6a3bd48..d1495bf 100644
--- a/datapath/linux/compat/reciprocal_div.c
+++ b/datapath/linux/compat/reciprocal_div.c
@@ -1,6 +1,7 @@
 #include 
 #include 
 
+/* definition is required since reciprocal_value() is not exported */
 u32 reciprocal_value(u32 k)
 {
u64 val = (1LL << 32) + (k - 1);
-- 
1.7.1

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


[ovs-dev] [PATCH 05/11] datapath: Remove checksum compat support

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/Modules.mk   |2 -
 datapath/actions.c|   15 +-
 datapath/checksum.c   |  271 -
 datapath/checksum.h   |  173 -
 datapath/datapath.c   |3 -
 datapath/datapath.h   |   10 -
 datapath/linux/Modules.mk |3 +-
 datapath/linux/compat/include/linux/netdevice.h   |7 -
 datapath/linux/compat/include/net/checksum.h  |   12 +-
 datapath/linux/compat/include/net/sctp/checksum.h |5 -
 datapath/linux/compat/ip_tunnels_core.c   |4 -
 datapath/linux/compat/skbuff-openvswitch.c|   22 --
 datapath/linux/compat/utils.c |   39 +++
 datapath/linux/compat/vxlan.c |1 -
 datapath/vlan.h   |5 -
 datapath/vport-gre.c  |2 -
 datapath/vport-internal_dev.c |7 -
 datapath/vport-lisp.c |   12 +-
 datapath/vport-netdev.c   |5 -
 datapath/vport-vxlan.c|2 -
 20 files changed, 55 insertions(+), 545 deletions(-)
 delete mode 100644 datapath/checksum.c
 delete mode 100644 datapath/checksum.h
 create mode 100644 datapath/linux/compat/utils.c

diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index ccf4dfa..e2a4dad 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -8,7 +8,6 @@ dist_modules = $(both_modules)  # Modules to distribute
 
 openvswitch_sources = \
actions.c \
-   checksum.c \
datapath.c \
dp_notify.c \
flow.c \
@@ -21,7 +20,6 @@ openvswitch_sources = \
vport-vxlan.c
 
 openvswitch_headers = \
-   checksum.h \
compat.h \
datapath.h \
flow.h \
diff --git a/datapath/actions.c b/datapath/actions.c
index 33633df..2a9d6e5 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 
-#include "checksum.h"
 #include "datapath.h"
 #include "vlan.h"
 #include "vport.h"
@@ -60,7 +59,7 @@ static int __pop_vlan_tci(struct sk_buff *skb, __be16 
*current_tci)
if (unlikely(err))
return err;
 
-   if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
+   if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = csum_sub(skb->csum, csum_partial(skb->data
+ (2 * ETH_ALEN), VLAN_HLEN, 0));
 
@@ -117,7 +116,7 @@ static int push_vlan(struct sk_buff *skb, const struct 
ovs_action_push_vlan *vla
if (!__vlan_put_tag(skb, skb->vlan_proto, current_tag))
return -ENOMEM;
 
-   if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
+   if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = csum_add(skb->csum, csum_partial(skb->data
+ (2 * ETH_ALEN), VLAN_HLEN, 0));
 
@@ -134,14 +133,14 @@ static int set_eth_addr(struct sk_buff *skb,
if (unlikely(err))
return err;
 
-   if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
+   if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = csum_sub(skb->csum, csum_partial(eth_hdr(skb),
 ETH_ALEN * 2, 0));
 
memcpy(eth_hdr(skb)->h_source, eth_key->eth_src, ETH_ALEN);
memcpy(eth_hdr(skb)->h_dest, eth_key->eth_dst, ETH_ALEN);
 
-   if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
+   if (skb->ip_summed == CHECKSUM_COMPLETE)
skb->csum = csum_add(skb->csum, csum_partial(eth_hdr(skb),
 ETH_ALEN * 2, 0));
 
@@ -162,7 +161,7 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr 
*nh,
struct udphdr *uh = udp_hdr(skb);
 
if (uh->check ||
-   get_ip_summed(skb) == OVS_CSUM_PARTIAL) {
+   skb->ip_summed == CHECKSUM_PARTIAL) {
inet_proto_csum_replace4(&uh->check, skb,
 *addr, new_addr, 1);
if (!uh->check)
@@ -190,7 +189,7 @@ static void update_ipv6_checksum(struct sk_buff *skb, u8 
l4_proto,
struct udphdr *uh = udp_hdr(skb);
 
if (uh->check ||
-   get_ip_summed(skb) == OVS_CSUM_PARTIAL) {
+   skb->ip_summed == CHECKSUM_PARTIAL) {
inet_proto_csum_replace16(&uh->check, skb,
  addr, new_addr, 1);
if (!uh->check)
@@ -311,7 +310,7 @@ static void set_udp_port(struct sk_buff *skb, 

[ovs-dev] [PATCH 07/11] datapath: Cleanup netlink compat code.

2013-08-30 Thread Pravin B Shelar
Patch removes genl, netlink, rtnl compat code.

Signed-off-by: Pravin B Shelar 
---
 datapath/compat.h   |8 --
 datapath/datapath.c |   12 +-
 datapath/dp_notify.c|2 +-
 datapath/linux/compat/genetlink-openvswitch.c   |  132 
 datapath/linux/compat/include/linux/netlink.h   |   16 ---
 datapath/linux/compat/include/linux/rtnetlink.h |   43 ---
 datapath/linux/compat/include/net/genetlink.h   |  149 +--
 datapath/linux/compat/include/net/netlink.h |  113 -
 8 files changed, 8 insertions(+), 467 deletions(-)

diff --git a/datapath/compat.h b/datapath/compat.h
index c786b97..2873f0a 100644
--- a/datapath/compat.h
+++ b/datapath/compat.h
@@ -58,14 +58,6 @@ static inline void skb_clear_rxhash(struct sk_buff *skb)
 #endif
 }
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-#define GENL_SOCK(net) (genl_sock)
-#define SET_NETNSOK
-#else
-#define GENL_SOCK(net) ((net)->genl_sock)
-#define SET_NETNSOK.netnsok = true,
-#endif
-
 #ifdef HAVE_PARALLEL_OPS
 #define SET_PARALLEL_OPS   .parallel_ops = true,
 #else
diff --git a/datapath/datapath.c b/datapath/datapath.c
index bbec48f..dbdb6e4 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -272,7 +272,7 @@ static struct genl_family dp_packet_genl_family = {
.name = OVS_PACKET_FAMILY,
.version = OVS_PACKET_VERSION,
.maxattr = OVS_PACKET_ATTR_MAX,
-SET_NETNSOK
+   .netnsok = true,
 SET_PARALLEL_OPS
 };
 
@@ -1003,7 +1003,7 @@ static struct genl_family dp_flow_genl_family = {
.name = OVS_FLOW_FAMILY,
.version = OVS_FLOW_VERSION,
.maxattr = OVS_FLOW_ATTR_MAX,
-SET_NETNSOK
+   .netnsok = true,
 SET_PARALLEL_OPS
 };
 
@@ -1387,7 +1387,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, 
struct genl_info *info)
if (!IS_ERR(reply))
ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
else
-   netlink_set_err(GENL_SOCK(sock_net(skb->sk)), 0,
+   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
ovs_dp_flow_multicast_group.id, PTR_ERR(reply));
return 0;
 
@@ -1583,7 +1583,7 @@ static struct genl_family dp_datapath_genl_family = {
.name = OVS_DATAPATH_FAMILY,
.version = OVS_DATAPATH_VERSION,
.maxattr = OVS_DP_ATTR_MAX,
-SET_NETNSOK
+   .netnsok = true,
 SET_PARALLEL_OPS
 };
 
@@ -1850,7 +1850,7 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
  info->snd_seq, OVS_DP_CMD_NEW);
if (IS_ERR(reply)) {
err = PTR_ERR(reply);
-   netlink_set_err(GENL_SOCK(sock_net(skb->sk)), 0,
+   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
ovs_dp_datapath_multicast_group.id, err);
err = 0;
goto unlock;
@@ -1963,7 +1963,7 @@ static struct genl_family dp_vport_genl_family = {
.name = OVS_VPORT_FAMILY,
.version = OVS_VPORT_VERSION,
.maxattr = OVS_VPORT_ATTR_MAX,
-SET_NETNSOK
+   .netnsok = true,
 SET_PARALLEL_OPS
 };
 
diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c
index d530893..847f611 100644
--- a/datapath/dp_notify.c
+++ b/datapath/dp_notify.c
@@ -35,7 +35,7 @@ static void dp_detach_port_notify(struct vport *vport)
  OVS_VPORT_CMD_DEL);
ovs_dp_detach_port(vport);
if (IS_ERR(notify)) {
-   netlink_set_err(GENL_SOCK(ovs_dp_get_net(dp)), 0,
+   netlink_set_err(ovs_dp_get_net(dp)->genl_sock, 0,
ovs_dp_vport_multicast_group.id,
PTR_ERR(notify));
return;
diff --git a/datapath/linux/compat/genetlink-openvswitch.c 
b/datapath/linux/compat/genetlink-openvswitch.c
index 810223b..359f916 100644
--- a/datapath/linux/compat/genetlink-openvswitch.c
+++ b/datapath/linux/compat/genetlink-openvswitch.c
@@ -1,138 +1,13 @@
 #include 
 #include 
 
-#define GENL_FIRST_MCGROUP 16
-#define GENL_LAST_MCGROUP  31
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
-#include 
-#include 
-
-#include "openvswitch/datapath-compat.h"
-
-static DEFINE_MUTEX(mc_group_mutex);
-
-int genl_register_mc_group(struct genl_family *family,
-  struct genl_multicast_group *grp)
-{
-   static int next_group = GENL_FIRST_MCGROUP;
-
-   grp->family = family;
-
-   if (!strcmp(grp->name, OVS_VPORT_MCGROUP)) {
-   grp->id = OVS_VPORT_MCGROUP_FALLBACK_ID;
-   return 0;
-   }
-
-   mutex_lock(&mc_group_mutex);
-   grp->id = next_group;
-
-   if (++next_group > GENL_LAST_MCGROUP)
-   next_group = GENL_FIRST_MCGROUP;
-   mutex_unlock(&mc_group_mutex);
-
-   return 0;

[ovs-dev] [PATCH 01/11] datapath: Move kernel version check to configure.

2013-08-30 Thread Pravin B Shelar
Rather than having compile time check in datapath.c, its better
to check kernel version at configuration step.
This patch also removes support for older kernel.

Signed-off-by: Pravin B Shelar 
---
 acinclude.m4|   12 ++--
 datapath/datapath.c |5 -
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 73ee5ce..e754cfa 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -134,9 +134,17 @@ AC_DEFUN([OVS_CHECK_LINUX], [
 AC_MSG_RESULT([$kversion])
 
 if test "$version" -ge 3; then
-   : # Linux 3.x
+   if test "$patchlevel" -le 10; then
+  : # Linux 3.x
+   else
+ AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 3.10.x is not supported])
+   fi
 elif test "$version" = 2 && test "$patchlevel" -ge 6; then
-   : # Linux 2.6.x
+   if test "$sublevel" -le 31; then
+ AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
2.6.32 or later is required])
+   else
+ : # Linux 2.6.x
+   fi
 else
if test "$KBUILD" = "$KSRC"; then
  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
2.6 or later is required])
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 27deec8..b6410c4 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -61,11 +61,6 @@
 #include "vport-internal_dev.h"
 #include "vport-netdev.h"
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,18) || \
-LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
-#error Kernels before 2.6.18 or after 3.9 are not supported by this version of 
Open vSwitch.
-#endif
-
 #define REHASH_FLOW_INTERVAL (10 * 60 * HZ)
 static void rehash_flow_table(struct work_struct *work);
 static DECLARE_DELAYED_WORK(rehash_flow_wq, rehash_flow_table);
-- 
1.7.1

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


[ovs-dev] [PATCH 02/11] datapath: vport: Remove compat support

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/include/linux/if.h |7 ---
 datapath/linux/compat/include/net/ip.h   |6 --
 datapath/vport-internal_dev.c|   13 -
 datapath/vport-lisp.c|4 
 datapath/vport-netdev.c  |   16 +---
 datapath/vport-vxlan.c   |4 
 datapath/vport.c |2 --
 7 files changed, 1 insertions(+), 51 deletions(-)

diff --git a/datapath/linux/compat/include/linux/if.h 
b/datapath/linux/compat/include/linux/if.h
index f53cf97..c4c656c 100644
--- a/datapath/linux/compat/include/linux/if.h
+++ b/datapath/linux/compat/include/linux/if.h
@@ -3,13 +3,6 @@
 
 #include_next 
 
-#include 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,31)
-
-#define IFF_XMIT_DST_RELEASE 0
-
-#endif /* linux kernel < 2.6.31 */
-
 #ifndef IFF_TX_SKB_SHARING
 #define IFF_TX_SKB_SHARING 0
 #endif
diff --git a/datapath/linux/compat/include/net/ip.h 
b/datapath/linux/compat/include/net/ip.h
index 1dccdea..4193d32 100644
--- a/datapath/linux/compat/include/net/ip.h
+++ b/datapath/linux/compat/include/net/ip.h
@@ -4,12 +4,6 @@
 #include_next 
 
 #include 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,25)
-
-extern int __ip_local_out(struct sk_buff *skb);
-extern int ip_local_out(struct sk_buff *skb);
-
-#endif /* linux kernel < 2.6.25 */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3,1,0)
 static inline bool ip_is_fragment(const struct iphdr *iph)
diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
index db55ee0..904c0b3 100644
--- a/datapath/vport-internal_dev.c
+++ b/datapath/vport-internal_dev.c
@@ -41,9 +41,6 @@
 
 struct internal_dev {
struct vport *vport;
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,22)
-   struct net_device_stats stats;
-#endif
 };
 
 static struct internal_dev *internal_dev_priv(struct net_device *netdev)
@@ -59,12 +56,8 @@ static struct rtnl_link_stats64 
*internal_dev_get_stats(struct net_device *netde
 #else
 static struct net_device_stats *internal_dev_sys_stats(struct net_device 
*netdev)
 {
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,22)
-   struct net_device_stats *stats = &internal_dev_priv(netdev)->stats;
-#else
struct net_device_stats *stats = &netdev->stats;
 #endif
-#endif
struct vport *vport = ovs_internal_dev_get_vport(netdev);
struct ovs_vport_stats vport_stats;
 
@@ -187,10 +180,8 @@ static void do_setup(struct net_device *netdev)
netdev->features = NETIF_F_LLTX | NETIF_F_SG | NETIF_F_FRAGLIST |
   NETIF_F_HIGHDMA | NETIF_F_HW_CSUM | NETIF_F_TSO;
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27)
netdev->vlan_features = netdev->features;
netdev->features |= NETIF_F_HW_VLAN_CTAG_TX;
-#endif
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,39)
netdev->hw_features = netdev->features & ~NETIF_F_LLTX;
@@ -287,10 +278,6 @@ static int internal_dev_recv(struct vport *vport, struct 
sk_buff *skb)
 
netif_rx(skb);
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,29)
-   netdev->last_rx = jiffies;
-#endif
-
return len;
 }
 
diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
index 80e980a..3c6e784 100644
--- a/datapath/vport-lisp.c
+++ b/datapath/vport-lisp.c
@@ -20,7 +20,6 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,26)
 
 #include 
 #include 
@@ -646,6 +645,3 @@ const struct vport_ops ovs_lisp_vport_ops = {
.get_options= lisp_get_options,
.send   = lisp_tnl_send,
 };
-#else
-#warning LISP tunneling will not be available on kernels before 2.6.26
-#endif /* Linux kernel < 2.6.26 */
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index 50373b1..1c2d7c5 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -80,7 +80,7 @@ static struct sk_buff *netdev_frame_hook(struct sk_buff *skb)
 
return NULL;
 }
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
+#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,32)
 /*
  * Used as br_handle_frame_hook.  (Cannot run bridge at the same time, even on
  * different set of devices!)
@@ -92,17 +92,6 @@ static struct sk_buff *netdev_frame_hook(struct 
net_bridge_port *p,
netdev_port_receive((struct vport *)p, skb);
return NULL;
 }
-#elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
-/*
- * Used as br_handle_frame_hook.  (Cannot run bridge at the same time, even on
- * different set of devices!)
- */
-/* Called with rcu_read_lock and bottom-halves disabled. */
-static int netdev_frame_hook(struct net_bridge_port *p, struct sk_buff **pskb)
-{
-   netdev_port_receive((struct vport *)p, *pskb);
-   return 1;
-}
 #else
 #error
 #endif
@@ -186,9 +175,6 @@ static struct vport *netdev_create(const struct vport_parms 
*parms)
goto error_master_upper_dev_unlink;
 
dev_set_promiscuity(netd

[ovs-dev] [PATCH 03/11] datapath: Remove namespace compat support.

2013-08-30 Thread Pravin B Shelar
Signed-off-by: Pravin B Shelar 
---
 datapath/linux/Modules.mk |1 -
 datapath/linux/compat/include/net/genetlink.h |   18 -
 datapath/linux/compat/include/net/net_namespace.h |   80 +
 datapath/linux/compat/include/net/netns/generic.h |   12 ---
 datapath/linux/compat/net_namespace.c |   35 -
 5 files changed, 1 insertions(+), 145 deletions(-)
 delete mode 100644 datapath/linux/compat/include/net/netns/generic.h

diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 178cd5e..15c518a 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -81,6 +81,5 @@ openvswitch_headers += \
linux/compat/include/net/protocol.h \
linux/compat/include/net/route.h \
linux/compat/include/net/sock.h \
-   linux/compat/include/net/netns/generic.h \
linux/compat/include/net/vxlan.h \
linux/compat/include/net/sctp/checksum.h
diff --git a/datapath/linux/compat/include/net/genetlink.h 
b/datapath/linux/compat/include/net/genetlink.h
index ac199ec..c17459b 100644
--- a/datapath/linux/compat/include/net/genetlink.h
+++ b/datapath/linux/compat/include/net/genetlink.h
@@ -49,8 +49,6 @@ extern int busted_nlmsg_multicast(struct sock *sk, struct 
sk_buff *skb,
 #define nlmsg_multicast rpl_nlmsg_multicast
 #endif /* linux kernel < v2.6.19 */
 
-#include 
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
 
 #include 
@@ -112,11 +110,6 @@ static inline int genlmsg_multicast_flags(struct sk_buff 
*skb, u32 portid,
 }
 #endif /* linux kernel < 2.6.19 */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-#define genlmsg_multicast_netns(net, skb, portid, grp, flags) \
-   genlmsg_multicast(skb, portid, grp, flags)
-#endif
-
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20)
 
 #define genlmsg_put(skb, p, seq, fam, flg, c) \
@@ -174,15 +167,4 @@ int genl_register_family_with_ops(struct genl_family 
*family,
 extern void genl_notify(struct sk_buff *skb, struct net *net, u32 portid,
u32 group, struct nlmsghdr *nlh, gfp_t flags);
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,24) && \
-LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-static inline struct net *genl_info_net(struct genl_info *info)
-{
-   return &init_net;
-}
-#endif
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-#define genlmsg_unicast(ignore_net, skb, portid)   genlmsg_unicast(skb, portid)
-#endif
 #endif /* genetlink.h */
diff --git a/datapath/linux/compat/include/net/net_namespace.h 
b/datapath/linux/compat/include/net/net_namespace.h
index 440c601..198ab22 100644
--- a/datapath/linux/compat/include/net/net_namespace.h
+++ b/datapath/linux/compat/include/net/net_namespace.h
@@ -1,69 +1,10 @@
 #ifndef __NET_NET_NAMESPACE_WRAPPER_H
 #define __NET_NET_NAMESPACE_WRAPPER_H 1
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,24)
-/*  exists, go ahead and include it. */
 #include_next 
-#else
-/* No network namespace support. */
-struct net;
-
-static inline struct net *hold_net(struct net *net)
-{
-   return net;
-}
-
-static inline void release_net(struct net *net)
-{
-}
-
-#define __net_init  __init
-#define __net_exit  __exit
-#endif /* 2.6.24 */
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
-#ifdef CONFIG_NET_NS
-static inline
-int net_eq(const struct net *net1, const struct net *net2)
-{
-   return net1 == net2;
-}
-#else
-static inline
-int net_eq(const struct net *net1, const struct net *net2)
-{
-   return 1;
-}
-#endif /* CONFIG_NET_NS */
-#endif /* 2.6.26 */
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,29)
-#ifdef CONFIG_NET_NS
-
-static inline void write_pnet(struct net **pnet, struct net *net)
-{
-   *pnet = net;
-}
-
-static inline struct net *read_pnet(struct net * const *pnet)
-{
-   return *pnet;
-}
-
-#else
-
-#define write_pnet(pnet, net)   do { (void)(net); } while (0)
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,24)
-#define read_pnet(pnet) (&init_net)
-#else
-#define read_pnet(pnet) (NULL)
-#endif /* 2.6.24 */
-
-#endif /* CONFIG_NET_NS */
-#endif /* 2.6.29 */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,33)
+/* for 2.6.32* */
 struct rpl_pernet_operations {
int (*init)(struct net *net);
void (*exit)(struct net *net);
@@ -76,12 +17,6 @@ struct rpl_pernet_operations {
 #define register_pernet_device rpl_register_pernet_gen_device
 #define unregister_pernet_device rpl_unregister_pernet_gen_device
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,32)
-extern int rpl_register_pernet_gen_device(struct rpl_pernet_operations *ops);
-extern void rpl_unregister_pernet_gen_device(struct rpl_pernet_operations 
*ops);
-
-#else /* for 2.6.32* */
-
 int compat_init_net(struct net *net, struct rpl_pernet_operations *pnet);
 void compat_exit_net(struct net *net, struct rpl_pernet_operations *pnet);
 
@@ -110,18 +45,5 @@ static void rpl_unregister_pernet_gen_##TYPE(struct 
rpl_pernet_operations *rpl_p
 { 

[ovs-dev] [PATCH 3/3] Locking for groups.

2013-08-30 Thread Jarno Rajahalme
The "groups" container is protected with a rwlock, and each group is
protected with their own rwlock.  Whenever groups are looked up, the
container's lock is taken.  If a group is found, the group is locked
before the container is released, as otherwise the group might be
deleted while the lock is being acquired.  The group's hmap_node is
considered part of the container, and is governed by the container's
lock.  This means that while a reader is still accessing the group,
the hmap itself may be rebalanced, or the group may be removed from
the container for deletion, but the group itself is not deleted as
long as there are any readers left.

A groups lock is only locked for writing when the container's write
lock is already held.  This prevents deadlocks where one thread is
holding a write lock on the group, and is waiting for a write lock
on the container, while another (reading) thread is holding a lock
on the container and is waiting for a lock for the group.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto-provider.h |   23 -
 ofproto/ofproto.c  |  206 +++-
 2 files changed, 164 insertions(+), 65 deletions(-)

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index d37014f..ab640c2 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -113,8 +113,9 @@ struct ofproto {
 int min_mtu;/* Current MTU of non-internal ports. */
 
 /* Groups. */
-struct hmap groups; /* Contains "struct ofgroup"s. */
-uint32_t n_groups[4];   /* Number of existing groups of each type. */
+struct ovs_rwlock groups_rwlock;
+struct hmap groups OVS_GUARDED;   /* Contains "struct ofgroup"s. */
+uint32_t n_groups[4] OVS_GUARDED; /* # of existing groups of each type. */
 struct ofputil_group_features ogf;
 };
 
@@ -303,7 +304,16 @@ bool ofproto_rule_is_hidden(const struct rule *);
  * With few exceptions, ofproto implementations may look at these fields but
  * should not modify them. */
 struct ofgroup {
-struct hmap_node hmap_node; /* In struct ofproto's "group" hmap. */
+/* The rwlock is used to prevent groups from being deleted while child
+ * threads are using them to xlate flows.  A read lock means the
+ * group is currently being used.  A write lock means the group is
+ * in the process of being deleted or updated.  Note that since
+ * a read lock on the groups container is held while searching, and
+ * a group is ever write locked only while holding a write lock
+ * on the container, the user's of groups will never face a group
+ * in the write locked state. */
+struct ovs_rwlock rwlock;
+struct hmap_node hmap_node; /* In struct ofproto's "groups" hmap. */
 struct ofproto *ofproto;/* The ofproto that contains this group. */
 uint32_t group_id;
 uint8_t type;   /* One of OFPGT_*. */
@@ -315,6 +325,13 @@ struct ofgroup {
 uint32_t n_buckets;
 };
 
+bool ofproto_group_lookup(const struct ofproto *ofproto, uint32_t group_id,
+  struct ofgroup **group)
+OVS_TRY_RDLOCK(true, (*group)->rwlock);
+
+void ofproto_group_release(struct ofgroup *group)
+OVS_RELEASES(group->rwlock);
+
 /* ofproto class structure, to be defined by each ofproto implementation.
  *
  *
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0eb1ee7..61d74f4 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -457,6 +457,7 @@ ofproto_create(const char *datapath_name, const char 
*datapath_type,
 ofproto->vlan_bitmap = NULL;
 ofproto->vlans_changed = false;
 ofproto->min_mtu = INT_MAX;
+ovs_rwlock_init(&ofproto->groups_rwlock);
 hmap_init(&ofproto->groups);
 
 error = ofproto->ofproto_class->construct(ofproto);
@@ -1142,6 +1143,7 @@ ofproto_destroy__(struct ofproto *ofproto)
 }
 
 delete_group(ofproto, OFPG_ALL);
+ovs_rwlock_destroy(&ofproto->groups_rwlock);
 hmap_destroy(&ofproto->groups);
 
 connmgr_destroy(ofproto->connmgr);
@@ -4694,24 +4696,67 @@ handle_meter_request(struct ofconn *ofconn, const 
struct ofp_header *request,
 return 0;
 }
 
-static struct ofgroup *
-ofproto_group_lookup(const struct ofproto *ofproto,
-   uint32_t group_id)
+bool
+ofproto_group_lookup(const struct ofproto *ofproto, uint32_t group_id,
+ struct ofgroup **group)
+OVS_TRY_RDLOCK(true, (*group)->rwlock)
 {
-struct ofgroup *group;
+ovs_rwlock_rdlock(&ofproto->groups_rwlock);
+HMAP_FOR_EACH_IN_BUCKET (*group, hmap_node,
+ hash_int(group_id, 0), &ofproto->groups) {
+if ((*group)->group_id == group_id) {
+ovs_rwlock_rdlock(&(*group)->rwlock);
+ovs_rwlock_unlock(&ofproto->groups_rwlock);
+return true;
+}
+}
+ovs_rwlock_unlock(&ofproto->groups_rwlock);
+return false;
+}
+
+void
+ofproto_group_release(struct ofgroup *gro

[ovs-dev] [PATCH 2/3] Delete groups in ofproto_destruct.

2013-08-30 Thread Jarno Rajahalme
Also, since all kinds of groups can refer to other groups, there is no
reason to delete indirect groups first.

Signed-off-by: Jarno Rajahalme 
---
 ofproto/ofproto.c |   20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9a4eda9..0eb1ee7 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1126,6 +1126,8 @@ ofproto_flush__(struct ofproto *ofproto)
 }
 }
 
+static void delete_group(struct ofproto *ofproto, uint32_t group_id);
+
 static void
 ofproto_destroy__(struct ofproto *ofproto)
 {
@@ -1139,6 +1141,9 @@ ofproto_destroy__(struct ofproto *ofproto)
 free(ofproto->meters);
 }
 
+delete_group(ofproto, OFPG_ALL);
+hmap_destroy(&ofproto->groups);
+
 connmgr_destroy(ofproto->connmgr);
 
 hmap_remove(&all_ofprotos, &ofproto->hmap_node);
@@ -4952,23 +4957,16 @@ delete_group__(struct ofgroup *ofgroup)
 
 /* Implements OFPGC_DELETE. */
 static void
-delete_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm)
+delete_group(struct ofproto *ofproto, uint32_t group_id)
 {
-if (gm->group_id == OFPG_ALL) {
+if (group_id == OFPG_ALL) {
 struct ofgroup *ofgroup, *next;
 
-/* Remove indirect groups first, to allow other groups to be removed
- * successfully. */
-HMAP_FOR_EACH_SAFE (ofgroup, next, hmap_node, &ofproto->groups) {
-if (ofgroup->type == OFPGT11_INDIRECT) {
-delete_group__(ofgroup);
-}
-}
 HMAP_FOR_EACH_SAFE (ofgroup, next, hmap_node, &ofproto->groups) {
 delete_group__(ofgroup);
 }
 } else {
-struct ofgroup *ofgroup = ofproto_group_lookup(ofproto, gm->group_id);
+struct ofgroup *ofgroup = ofproto_group_lookup(ofproto, group_id);
 if (ofgroup) {
 delete_group__(ofgroup);
 }
@@ -5000,7 +4998,7 @@ handle_group_mod(struct ofconn *ofconn, const struct 
ofp_header *oh)
 return modify_group(ofproto, &gm);
 
 case OFPGC11_DELETE:
-delete_group(ofproto, &gm);
+delete_group(ofproto, gm->group_id);
 return 0;
 
 default:
-- 
1.7.10.4

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


[ovs-dev] [PATCH 1/3] Check the number of buckets for indirect groups.

2013-08-30 Thread Jarno Rajahalme
Indirect groups can have at most one bucket.

Signed-off-by: Jarno Rajahalme 
---
 lib/ofp-parse.c |   14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 9098467..f17364f 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -2075,9 +2075,17 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod *gm, 
uint16_t command,
 goto out;
 }
 /* Validate buckets. */
-LIST_FOR_EACH(bucket, list_node, &gm->buckets) {
-if (bucket->weight != 1 && gm->type != OFPGT11_SELECT) {
-error = xstrdup("Only select groups can have bucket weights.");
+{
+size_t n_buckets = 0;
+LIST_FOR_EACH(bucket, list_node, &gm->buckets) {
+++n_buckets;
+if (bucket->weight != 1 && gm->type != OFPGT11_SELECT) {
+error = xstrdup("Only select groups can have bucket weights.");
+goto out;
+}
+}
+if (gm->type == OFPGT11_INDIRECT && n_buckets > 1) {
+error = xstrdup("Indirect groups can have at most one bucket.");
 goto out;
 }
 }
-- 
1.7.10.4

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


[ovs-dev] [PATCH 0/3] Incrementals on groups v3

2013-08-30 Thread Jarno Rajahalme
These should apply on the groups 2/2 v3.

Jarno Rajahalme (3):
  Check the number of buckets for indirect groups.
  Delete groups in ofproto_destruct.
  Locking for groups.

 lib/ofp-parse.c|   14 ++-
 ofproto/ofproto-provider.h |   23 -
 ofproto/ofproto.c  |  220 ++--
 3 files changed, 181 insertions(+), 76 deletions(-)

-- 
1.7.10.4

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


Re: [ovs-dev] [PATCH v2] Fix a bug in conversion between flow/mask and flow key

2013-08-30 Thread Andy Zhou
Patch for branch 1.11 is attached.


On Fri, Aug 30, 2013 at 1:27 PM, Ben Pfaff  wrote:

> That's perfect, thanks a lot!
>
> On Fri, Aug 30, 2013 at 01:26:44PM -0700, Andy Zhou wrote:
> > It will be simpler if we just fix the bug instead of pulling in more
> > patches. I will derive a patch and send it out.
> >
> >
> > On Fri, Aug 30, 2013 at 1:19 PM, Ben Pfaff  wrote:
> >
> > > I still get conflicts:
> > >
> > > blp@sigsegv:~/ovs$ git checkout openvswitch/branch-1.11
> > > HEAD is now at 37f9d3a... Set release date for 1.11.0.
> > > blp@sigsegv:~/ovs$ git cherry-pick 4a2216156e6e168
> > > error: could not apply 4a22161... odp-util: New function
> > > odp_flow_key_to_mask().
> > > hint: after resolving the conflicts, mark the corrected paths
> > > hint: with 'git add ' or 'git rm '
> > > hint: and commit the result with 'git commit'
> > > blp@sigsegv:~/ovs$
> > >
> > > On Fri, Aug 30, 2013 at 01:15:49PM -0700, Andy Zhou wrote:
> > > > May be it caused by missing commit 4a2216156e6e168 ?
> > > >
> > > >
> > > > On Fri, Aug 30, 2013 at 11:11 AM, Ben Pfaff  wrote:
> > > >
> > > > > It doesn't apply directly to branch-1.11.  Can you take a look and
> see
> > > > > whether there's some other commit we're missing there?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Ben.
> > > > >
> > > > > On Fri, Aug 30, 2013 at 11:09:21AM -0700, Andy Zhou wrote:
> > > > > > Yes, this patch fixes a bug.
> > > > > >
> > > > > >
> > > > > > On Fri, Aug 30, 2013 at 11:06 AM, Ben Pfaff 
> wrote:
> > > > > >
> > > > > > > On Fri, Aug 30, 2013 at 11:03:24AM -0700, Andy Zhou wrote:
> > > > > > > > On Fri, Aug 30, 2013 at 11:01 AM, Ben Pfaff 
> > > wrote:
> > > > > > > >
> > > > > > > > > On Fri, Aug 30, 2013 at 09:57:13AM -0700,
> gyang@nicira.comwrote:
> > > > > > > > > > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct
> nlattr
> > > > > > > > > *attrs[OVS_KEY_ATTR_MAX + 1],
> > > > > > > > > >  if (is_mask) {
> > > > > > > > > >  if (!is_all_zeros((const
> uint8_t *)
> > > > > nd_key,
> > > > > > > > > >sizeof
> *nd_key) &&
> > > > > > > > > > -flow->tp_src !=
> htons(0x)) {
> > > > > > > > >
> > > > > > > > > I suspect that && here should be ||.  Is that correct?  If
> so,
> > > then
> > > > > > > > > I'll update the patch and commit this:
> > > > > > > > >
> > > > > > > > I agree. I was planing on send out a separate patch for this.
> > > > > > >
> > > > > > > Thanks, I folded that in and applied this to master and
> branch-2.0.
> > > > > > > Do we want it on branch-1.11 also?
> > > > > > >
> > > > >
> > >
>


0001-odp_util-Fix-a-bug-in-conversion-between-flow-mask-a.patch
Description: Binary data
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH V3 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command

2013-08-30 Thread Alex Wang
This commit changes the "ovs-appctl coverage/show" command to show the
the averaged per-second rates for the last few seconds, the last minute
and the last hour, and the total counts of all of the coverage counters.

Signed-off-by: Alex Wang 

---

v2 -> v3:
- fix typo.
- provide the averaged per-second rates rather than the
  total counts for the last minute and the last hour.
- shorten the lock holding time.

v1 -> v2:
- coverage_run() can be called by multiple threads.
- move min_idx and hr_idx inside coverage_run().
- fix the indentation.
- use BUILD_ASSERT_DECL to check the COVERAGE_RUN_INTERVAL.

---
 lib/coverage-unixctl.man |4 +-
 lib/coverage.c   |  113 --
 lib/coverage.h   |   17 ++-
 lib/timeval.c|1 +
 tests/ofproto-dpif.at|   34 ++
 5 files changed, 162 insertions(+), 7 deletions(-)

diff --git a/lib/coverage-unixctl.man b/lib/coverage-unixctl.man
index 9718894..8e5df81 100644
--- a/lib/coverage-unixctl.man
+++ b/lib/coverage-unixctl.man
@@ -8,4 +8,6 @@ main loop takes unusually long to run.
 Coverage counters are useful mainly for performance analysis and
 debugging.
 .IP "\fBcoverage/show\fR"
-Displays the values of all of the coverage counters.
+Displays the averaged per-second rates for the last few seconds, the
+last minute and the last hour, and the total counts of all of the
+coverage counters.
diff --git a/lib/coverage.c b/lib/coverage.c
index 23e2997..4364734 100644
--- a/lib/coverage.c
+++ b/lib/coverage.c
@@ -63,7 +63,14 @@ struct coverage_counter *coverage_counters[] = {
 
 static struct ovs_mutex coverage_mutex = OVS_MUTEX_INITIALIZER;
 
+static long long int coverage_run_time = LLONG_MIN;
+
+/* Index counter used to compute the moving average array's index. */
+static unsigned int idx_count = 0;
+
 static void coverage_read(struct svec *);
+static unsigned int coverage_array_sum(const unsigned int *arr,
+   const unsigned int len);
 
 static void
 coverage_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
@@ -206,6 +213,7 @@ coverage_log(void)
 static void
 coverage_read(struct svec *lines)
 {
+struct coverage_counter **c = coverage_counters;
 unsigned long long int *totals;
 size_t n_never_hit;
 uint32_t hash;
@@ -215,24 +223,37 @@ coverage_read(struct svec *lines)
 
 n_never_hit = 0;
 svec_add_nocopy(lines,
-xasprintf("Event coverage, hash=%08"PRIx32":", hash));
+xasprintf("Event coverage, avg rate over last: %d "
+  "seconds, last minute, last hour,  "
+  "hash=%08"PRIx32":",
+  COVERAGE_RUN_INTERVAL/1000, hash));
 
 totals = xmalloc(n_coverage_counters * sizeof *totals);
 ovs_mutex_lock(&coverage_mutex);
 for (i = 0; i < n_coverage_counters; i++) {
-totals[i] = coverage_counters[i]->total;
+totals[i] = c[i]->total;
 }
 ovs_mutex_unlock(&coverage_mutex);
 
 for (i = 0; i < n_coverage_counters; i++) {
 if (totals[i]) {
-svec_add_nocopy(lines, xasprintf("%-24s %9llu",
- coverage_counters[i]->name,
- totals[i]));
+/* Shows the averaged per-second rates for the last
+ * COVERAGE_RUN_INTERVAL interval, the last minute and
+ * the last hour. */
+svec_add_nocopy(lines,
+xasprintf("%-24s %5.1f/sec %9.3f/sec "
+  "%13.4f/sec   total: %llu",
+  c[i]->name,
+  (c[i]->min[(idx_count - 1) % MIN_AVG_LEN]
+   * 1000.0 / COVERAGE_RUN_INTERVAL),
+  coverage_array_sum(c[i]->min, MIN_AVG_LEN) / 60.0,
+  coverage_array_sum(c[i]->hr,  HR_AVG_LEN) / 3600.0,
+  totals[i]));
 } else {
 n_never_hit++;
 }
 }
+
 svec_add_nocopy(lines, xasprintf("%zu events never hit", n_never_hit));
 free(totals);
 }
@@ -249,3 +270,85 @@ coverage_clear(void)
 }
 ovs_mutex_unlock(&coverage_mutex);
 }
+
+/* Runs approximately every COVERAGE_RUN_INTERVAL amount of time to update the
+ * coverage counters' 'min' and 'hr' array.  'min' array is for cumulating
+ * per second counts into per minute count.  'hr' array is for cumulating per
+ * minute counts into per hour count.  Every thread may call this function. */
+void
+coverage_run(void)
+{
+/* Defines the moving average array index variables. */
+static unsigned int min_idx, hr_idx;
+struct coverage_counter **c = coverage_counters;
+long long int now;
+
+ovs_mutex_lock(&coverage_mutex);
+now = time_msec();
+/* Initialize the coverage_run_time. */
+if (coverage_run_time == LLONG_MIN) {
+coverage_run_time = now + COVERAGE_RUN

[ovs-dev] [PATCH] timeval: Remove CACHE_TIME scheme.

2013-08-30 Thread Alex Wang
This commit removes the CACHE_TIME scheme from timeval module.  This
is for eliminating the lock contention over the read/write lock of
the cached time.  To get the time, the thread now will directly do
the system call 'clock_gettime()'.

As a side effect, time can only be warpped after timer is stopped
by 'ovs-appctl time/stop' command.

Signed-off-by: Alex Wang 
---
 lib/timeval.c  |  122 +++-
 lib/timeval.h  |   18 --
 m4/openvswitch.m4  |   16 --
 ofproto/ofproto-dpif.c |2 +-
 tests/automake.mk  |6 --
 tests/learn.at |1 +
 tests/ofproto-dpif.at  |8 +++
 tests/test-timeval.c   |  145 
 tests/testsuite.at |1 -
 tests/timeval.at   |   22 
 vswitchd/bridge.c  |3 -
 11 files changed, 19 insertions(+), 325 deletions(-)
 delete mode 100644 tests/test-timeval.c
 delete mode 100644 tests/timeval.at

diff --git a/lib/timeval.c b/lib/timeval.c
index faf8e7b..2c8d491 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -41,14 +41,12 @@ VLOG_DEFINE_THIS_MODULE(timeval);
 
 struct clock {
 clockid_t id;   /* CLOCK_MONOTONIC or CLOCK_REALTIME. */
-struct ovs_rwlock rwlock;   /* Mutual exclusion for 'cache'. */
 
 /* Features for use by unit tests.  Protected by 'rwlock'. */
+struct ovs_rwlock rwlock;
 struct timespec warp;   /* Offset added for unit tests. */
 bool stopped;   /* Disables real-time updates if true.  */
 
-/* Relevant only if CACHE_TIME is true. */
-volatile sig_atomic_t tick; /* Has the timer ticked?  Set by signal. */
 struct timespec cache;  /* Last time read from kernel. */
 };
 
@@ -67,11 +65,6 @@ static long long int deadline = LLONG_MAX;
  * up. */
 DEFINE_STATIC_PER_THREAD_DATA(long long int, last_wakeup, 0);
 
-static void set_up_timer(void);
-static void set_up_signal(int flags);
-static void sigalrm_handler(int);
-static void block_sigalrm(sigset_t *);
-static void unblock_sigalrm(const sigset_t *);
 static void log_poll_interval(long long int last_wakeup);
 static struct rusage *get_recent_rusage(void);
 static void refresh_rusage(void);
@@ -83,7 +76,6 @@ init_clock(struct clock *c, clockid_t id)
 {
 memset(c, 0, sizeof *c);
 c->id = id;
-ovs_rwlock_init(&c->rwlock);
 xclock_gettime(c->id, &c->cache);
 }
 
@@ -99,9 +91,6 @@ do_init_time(void)
   : CLOCK_REALTIME));
 init_clock(&wall_clock, CLOCK_REALTIME);
 boot_time = timespec_to_msec(&monotonic_clock.cache);
-
-set_up_signal(SA_RESTART);
-set_up_timer();
 }
 
 /* Initializes the timetracking module, if not already initialized. */
@@ -112,43 +101,7 @@ time_init(void)
 pthread_once(&once, do_init_time);
 }
 
-static void
-set_up_signal(int flags)
-{
-struct sigaction sa;
-
-memset(&sa, 0, sizeof sa);
-sa.sa_handler = sigalrm_handler;
-sigemptyset(&sa.sa_mask);
-sa.sa_flags = flags;
-xsigaction(SIGALRM, &sa, NULL);
-}
-
-static void
-set_up_timer(void)
-{
-static timer_t timer_id;/* "static" to avoid apparent memory leak. */
-struct itimerspec itimer;
-
-if (!CACHE_TIME) {
-return;
-}
-
-if (timer_create(monotonic_clock.id, NULL, &timer_id)) {
-VLOG_FATAL("timer_create failed (%s)", ovs_strerror(errno));
-}
-
-itimer.it_interval.tv_sec = 0;
-itimer.it_interval.tv_nsec = TIME_UPDATE_INTERVAL * 1000 * 1000;
-itimer.it_value = itimer.it_interval;
-
-if (timer_settime(timer_id, 0, &itimer, NULL)) {
-VLOG_FATAL("timer_settime failed (%s)", ovs_strerror(errno));
-}
-}
-
-/* Set up the interval timer, to ensure that time advances even without calling
- * time_refresh().
+/* Set up the interval timer, to ensure that time advances.
  *
  * A child created with fork() does not inherit the parent's interval timer, so
  * this function needs to be called from the child after fork(). */
@@ -157,41 +110,18 @@ time_postfork(void)
 {
 assert_single_threaded();
 time_init();
-set_up_timer();
-}
-
-/* Forces a refresh of the current time from the kernel.  It is not usually
- * necessary to call this function, since the time will be refreshed
- * automatically at least every TIME_UPDATE_INTERVAL milliseconds.  If
- * CACHE_TIME is false, we will always refresh the current time so this
- * function has no effect. */
-void
-time_refresh(void)
-{
-monotonic_clock.tick = wall_clock.tick = true;
 }
 
 static void
 time_timespec__(struct clock *c, struct timespec *ts)
 {
 time_init();
-for (;;) {
-/* Use the cached time by preference, but fall through if there's been
- * a clock tick.  */
-ovs_rwlock_rdlock(&c->rwlock);
-if (c->stopped || !c->tick) {
-timespec_add(ts, &c->cache, &c->warp);
-ovs_rwlock_unlock(&c->rwlock);
-return;
-}
-ovs_rwlock_unlock(&c->rwlock);
 
-   

Re: [ovs-dev] [PATCH 00/11] datapath: Remove old compatibility code.

2013-08-30 Thread Justin Pettit
It's probably worth mentioning in NEWS and updating FAQ, SubmittingPatches, and 
README to remove references to 2.6.18.

--Justin


On Aug 30, 2013, at 1:22 PM, Pravin B Shelar  wrote:

> datapath support for older kernel is been broken for kernel
> older than 2.6.32 for some time now. There have not been any
> complains about this. So it makes sense to get rid of the code.
> This make datapath module much close to upstream datapath and
> make code easy to understand.
> To make review easy I have broken down patches.
> 
> Pravin B Shelar (11):
>  datapath: Move kernel version check to configure.
>  datapath: vport: Remove compat support
>  datapath: Remove namespace compat support.
>  datapath: Remove skb->mask compat code.
>  datapath: Remove checksum compat support
>  datapath: Remove vlan compat support
>  datapath: Cleanup netlink compat code.
>  datapath: Cleanup compat support.
>  datapath: Remove compat support for NLA_NUL_STRING
>  datapath: Remove reciprocal_div compat code.
>  datapath: Remove compat header files.
> 
> acinclude.m4   |   16 +-
> datapath/Modules.mk|3 -
> datapath/actions.c |   17 +-
> datapath/checksum.c|  271 
> datapath/checksum.h|  173 
> datapath/compat.h  |   73 
> datapath/datapath.c|   95 +
> datapath/datapath.h|   15 -
> datapath/dp_notify.c   |2 +-
> datapath/flow.c|9 +-
> datapath/linux/Modules.mk  |   20 +-
> datapath/linux/compat/addrconf_core-openvswitch.c  |   82 
> datapath/linux/compat/genetlink-openvswitch.c  |  132 --
> datapath/linux/compat/include/linux/dmi.h  |  114 -
> datapath/linux/compat/include/linux/if.h   |7 -
> datapath/linux/compat/include/linux/if_ether.h |   13 -
> datapath/linux/compat/include/linux/inetdevice.h   |   14 -
> datapath/linux/compat/include/linux/kernel.h   |   31 --
> datapath/linux/compat/include/linux/kobject.h  |   30 --
> datapath/linux/compat/include/linux/lockdep.h  |  449 
> datapath/linux/compat/include/linux/mutex.h|   59 ---
> datapath/linux/compat/include/linux/netdevice.h|   88 
> .../linux/compat/include/linux/netfilter_bridge.h  |   24 -
> .../linux/compat/include/linux/netfilter_ipv4.h|   19 -
> datapath/linux/compat/include/linux/netlink.h  |   16 -
> datapath/linux/compat/include/linux/rculist.h  |   18 -
> .../linux/compat/include/linux/reciprocal_div.h|   40 --
> datapath/linux/compat/include/linux/rtnetlink.h|   43 --
> datapath/linux/compat/include/linux/skbuff.h   |   45 --
> datapath/linux/compat/include/linux/slab.h |   31 --
> datapath/linux/compat/include/linux/timer.h|   96 -
> datapath/linux/compat/include/net/checksum.h   |   12 +-
> datapath/linux/compat/include/net/genetlink.h  |  167 +---
> datapath/linux/compat/include/net/ip.h |6 -
> datapath/linux/compat/include/net/net_namespace.h  |   80 +
> datapath/linux/compat/include/net/netlink.h|  113 -
> datapath/linux/compat/include/net/netns/generic.h  |   12 -
> datapath/linux/compat/include/net/protocol.h   |   12 -
> datapath/linux/compat/include/net/route.h  |   21 -
> datapath/linux/compat/include/net/sctp/checksum.h  |5 -
> datapath/linux/compat/include/net/sock.h   |   15 -
> datapath/linux/compat/ip_output-openvswitch.c  |   33 --
> datapath/linux/compat/ip_tunnels_core.c|4 -
> datapath/linux/compat/kmemdup.c|   22 -
> datapath/linux/compat/net_namespace.c  |   35 --
> datapath/linux/compat/netdevice.c  |4 -
> datapath/linux/compat/reciprocal_div.c |1 +
> datapath/linux/compat/skbuff-openvswitch.c |   22 -
> datapath/linux/compat/time.c   |   39 --
> datapath/linux/compat/utils.c  |   39 ++
> datapath/linux/compat/vxlan.c  |   11 +-
> datapath/vlan.c|   58 ---
> datapath/vlan.h|   39 --
> datapath/vport-gre.c   |   15 +-
> datapath/vport-internal_dev.c  |   32 +-
> datapath/vport-lisp.c  |   28 +-
> datapath/vport-netdev.c|   23 +-
> datapath/vport-vxlan.c |8 +-
> datapath/vport.c   |2 -
> 59 files changed, 122 insertions(+), 2781 deletions(-)
> delete mode 100644 datapath/checksum.c
> delete mode 100644 datapath/checksum.h
> delete mode 100644 datapath/linux/compat/addrconf_co

Re: [ovs-dev] [PATCH] openvswitch: fix sw_flow_key alignment

2013-08-30 Thread Jesse Gross
On Fri, Aug 30, 2013 at 11:42 AM, David Miller  wrote:
> From: Jesse Gross 
> Date: Fri, 30 Aug 2013 11:22:11 -0700
>
>> The suggestion that Geert made was to just drop this entirely and
>> rely on the natural alignment from these values.
>
> Indeed, Geert's patch was 1,000 times superior to this one.

I looked through the struct definition and I think that the idea of
manually padding as Geert did in his patch will be difficult to
maintain over time (and actually there are a few that he missed) since
there are a number of different structs/unions contained in there.
Dropping the alignment specifier completely still has the same
potential problem on architectures where the size and alignment of
long are not the same.

Maybe the easiest thing to do at this point is just to always align it
to 8 bytes.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [LNG] Re: datapath: remove HAVE_MAC_RAW

2013-08-30 Thread Jesse Gross
On Wed, Aug 28, 2013 at 1:57 AM, Viresh Kumar  wrote:
> On 28 August 2013 05:37, Jesse Gross  wrote:
>> On Tue, Aug 27, 2013 at 3:51 AM, Viresh Kumar  
>> wrote:
>>> On 26 August 2013 21:59, Jesse Gross  wrote:
>>>
 This is compatibility code for older kernels so by definition it
 doesn't exist in current kernels. You can't just remove it.
>>>
>>> My mistake :(
>>>
>>> Will something like this makes sense? This was changed in 2.6.22 by
>>> following patch:
>>> b0e380b [SK_BUFF]: unions of just one member don't get anything done, kill 
>>> them
>>
>> I suspect that this is just some unrelated key word that was added to
>> skbuff.h for the RT kernels. I think the best solution would be to
>> just tighten up the grep expression since that is simpler and clearer
>> than adding version checks as well.
>
> What about this?

Out of curiosity, what is the actual symbol that we're finding in the RT kernel?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [LNG] Re: [ovs-discuss] OVS Support for RT Kernel

2013-08-30 Thread Jesse Gross
On Wed, Aug 28, 2013 at 2:57 AM, Viresh Kumar  wrote:
> On 28 August 2013 05:48, Jesse Gross  wrote:
>
>> The implementation is actually pretty much exactly the same as before.
>> The only reason why there are no longer separate process/interrupt
>> counters is because we started disabling bottom halves when processing
>> packets for userspace. However, with your change that is longer the
>> case and it's possible to be interrupted/rescheduled while using a
>> per-CPU counter. This will produce non-deterministic results since the
>> processing for multiple packets are sharing the same loop counter.
>
> Ahh, I didn't wanted to change how the system works today and assumed
> incorrectly that get_cpu_var() will also disable bh but it only disables
> preemption, not bh/irqs..
>
> So my original patch should have been like this:
>
> diff --git a/datapath/actions.c b/datapath/actions.c
> index fa4b904..37b29d6 100644
> --- a/datapath/actions.c
> +++ b/datapath/actions.c
> @@ -607,6 +607,9 @@ int ovs_execute_actions(struct datapath *dp,
> struct sk_buff *skb)
> int error;
>
> /* Check whether we've looped too much. */
> spin_lock_bh(some-lock);
> loop = &__get_cpu_var(loop_counters);
> if (unlikely(++loop->count > MAX_LOOPS))
> loop->looping = true;
> @@ -629,5 +632,8 @@ out_loop:
> if (!--loop->count)
> loop->looping = false;
>
>spin_unlock_bh(some-lock);
>return error;
>  }
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 27deec8..a0fa3de 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -940,9 +940,7 @@ static int ovs_packet_cmd_execute(struct sk_buff
> *skb, struct genl_info *info)
> if (!dp)
> goto err_unlock;
>
> -   local_bh_disable();
> err = ovs_execute_actions(dp, packet);
> -   local_bh_enable();
> rcu_read_unlock();
>
> ovs_flow_free(flow, false);
> (END)
>
> Which should have worked for both RT and non-RT kernels..

I think that will work for the loop checker but executing the actions
could be a very long code path - lots of actions, trips through the IP
stack, etc. It doesn't seem like a great idea to hold a spin lock for
this amount of time, particularly if the goal is preemptibility.

 The current
 method will yield non-deterministic results in the presence of
 preemption.
>>>
>>> I see one problem: We may access these per-cpu variables
>>> concurrently from process context and bh.. And so we need
>>> something to guarantee the serialization here.. And then probably
>>> we don't need to do: get_cpu_var() for non-RT case..
>>>
>>> Do we have some existing lock that can be reused here? Otherwise
>>> I will create a new one.. spinlock would be better then a mutex as we
>>> would be accessing this code from BH too..
>>
>> There's something similar in dev.c so you could check to see if they
>> did anything to that in the RT patchset.
>
> net/core/dev.c??

Yes, that's right.

> There is only one place where they are using local_bh_disable() for
> RT kernel with following patch:

I doubt that the solution would be to turn off bottom halves here. I
would look for the counter code and see what they do with it
specifically.

 Have you also audited the other use of per-CPU variables?
>>>
>>> Yes, I did. But couldn't find anything that would be broken with RT..
>>> Per-cpu doesn't have a problem with RT, only the access to those
>>> can be a issue.. And if things are serialized enough, then there
>>> shouldn't be any issue, I hope.
>>
>> Can't you be interrupted/preempted while modifying the counters?
>
> Yes, you can be. But that will happen today (non-RT) as well without
> proper locking or something like local_bh_disable()..
>
> And I thought if we aren't doing local_bh_disable() at multiple places
> then proper locking is there with spin_lock_bh() or similar.. And I see
> such lockings in place at most of the places..

Most of the network stack operates with bottom halves disabled so
packets ingressing OVS from anywhere except userspace will already
have them turned off. Since we also turn off bottom halves with
packets coming from userspace, this is used as a synchronization
mechanism.

By the way, I don't have a lot of knowledge about the RT patchset,
particularly recent changes. You might want to ask someone with more
direct knowledge.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-ofctl: Document group commands

2013-08-30 Thread Simon Horman
On Fri, Aug 30, 2013 at 09:15:19AM -0700, Ben Pfaff wrote:
> On Fri, Aug 30, 2013 at 05:42:54PM +0900, Simon Horman wrote:
> > Signed-off-by: Simon Horman 
> > 
> > ---
> > 
> > Ben, feel free to squash this into 'Implement OpenFlow 1.1+ "groups"
> > protocol'.
> 
> Thanks, done.  I added your sign-off and gave you co-authorship also
> (I really like to encourage documentation contributions!).

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


Re: [ovs-dev] [PATCH] openvswitch: fix sw_flow_key alignment

2013-08-30 Thread David Miller
From: Jesse Gross 
Date: Fri, 30 Aug 2013 16:20:25 -0700

> I looked through the struct definition and I think that the idea of
> manually padding as Geert did in his patch will be difficult to
> maintain over time (and actually there are a few that he missed) since
> there are a number of different structs/unions contained in there.

You have to be mindful of the gaps and wasted space for performance
reasons anyways.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] OPENFLOW-1.1+: Remove completed tasks.

2013-08-30 Thread Simon Horman
On Thu, Aug 29, 2013 at 09:52:07PM -0700, Ben Pfaff wrote:
> It appears to me that OFPST_TABLE is correct for OF1.1+, so I'm removing
> that.
> 
> DESIGN describes OF1.2 flow_mod, so I'm removing that.
> 
> Port and queue stats include duration these days.  The proposed patch to
> implement groups includes duration also.
> 
> Signed-off-by: Ben Pfaff 


Reviewed-by: Simon Horman 

> ---
>  OPENFLOW-1.1+ |   18 --
>  1 file changed, 18 deletions(-)
> 
> diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> index 753dbba..aacbca9 100644
> --- a/OPENFLOW-1.1+
> +++ b/OPENFLOW-1.1+
> @@ -69,18 +69,6 @@ probably incomplete.
>behavior does not change.
>[required for OF1.1 and OF1.2]
>  
> -* Flow table stats (OFPST_TABLE).
> -
> -* Reference count (active entries)
> -  [implemented]
> -  [required for OF1.1 and OF1.2]
> -
> -* Packet Lookups
> -  [required for OF1.1; optional for OF1.2]
> -
> -* Packet Matches
> -  [required for OF1.1; optional for OF1.2]
> -
>  * MPLS.  Simon Horman maintains a patch series that adds this
>feature.  This is partially merged.
>[optional for OF1.1+]
> @@ -122,9 +110,6 @@ end of the OF1.2 spec.  I didn't compare the specs 
> carefully yet.)
>  * Add ability to delete flow in all tables.
>[required for OF1.2+]
>  
> -* Update DESIGN to describe OF1.2 behavior also.
> -  [required for OF1.2+]
> -
>  OpenFlow 1.3
>  
>  
> @@ -187,9 +172,6 @@ didn't compare the specs carefully yet.)
>optimization in some cases for the software switch.
>[optional for OF1.3+]
>  
> -* Duration Statistics
> -  - New for Per Port, Per Queue, Per Group
> -  [required for v1.3+]
>  
>  How to contribute
>  -
> -- 
> 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 1/3] Check the number of buckets for indirect groups.

2013-08-30 Thread Simon Horman
On Fri, Aug 30, 2013 at 01:40:16PM -0700, Jarno Rajahalme wrote:
> Indirect groups can have at most one bucket.
> 
> Signed-off-by: Jarno Rajahalme 

Reviewed-by: Simon Horman 

> ---
>  lib/ofp-parse.c |   14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 9098467..f17364f 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -2075,9 +2075,17 @@ parse_ofp_group_mod_str__(struct ofputil_group_mod 
> *gm, uint16_t command,
>  goto out;
>  }
>  /* Validate buckets. */
> -LIST_FOR_EACH(bucket, list_node, &gm->buckets) {
> -if (bucket->weight != 1 && gm->type != OFPGT11_SELECT) {
> -error = xstrdup("Only select groups can have bucket weights.");
> +{
> +size_t n_buckets = 0;
> +LIST_FOR_EACH(bucket, list_node, &gm->buckets) {
> +++n_buckets;

Any reason you favour a pre-increment over a post-increment.
I realise it makes no difference to the logic here but I am curious.

> +if (bucket->weight != 1 && gm->type != OFPGT11_SELECT) {
> +error = xstrdup("Only select groups can have bucket 
> weights.");
> +goto out;
> +}
> +}
> +if (gm->type == OFPGT11_INDIRECT && n_buckets > 1) {
> +error = xstrdup("Indirect groups can have at most one bucket.");
>  goto out;
>  }
>  }
> -- 
> 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 2/3] Delete groups in ofproto_destruct.

2013-08-30 Thread Simon Horman
On Fri, Aug 30, 2013 at 01:40:17PM -0700, Jarno Rajahalme wrote:
> Also, since all kinds of groups can refer to other groups, there is no
> reason to delete indirect groups first.

This seems reasonable to me, but its not clear to me
how the code prevents the deletion of groups that
are still referenced either by other groups or by rules.

> Signed-off-by: Jarno Rajahalme 
> ---
>  ofproto/ofproto.c |   20 +---
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 9a4eda9..0eb1ee7 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1126,6 +1126,8 @@ ofproto_flush__(struct ofproto *ofproto)
>  }
>  }
>  
> +static void delete_group(struct ofproto *ofproto, uint32_t group_id);
> +
>  static void
>  ofproto_destroy__(struct ofproto *ofproto)
>  {
> @@ -1139,6 +1141,9 @@ ofproto_destroy__(struct ofproto *ofproto)
>  free(ofproto->meters);
>  }
>  
> +delete_group(ofproto, OFPG_ALL);
> +hmap_destroy(&ofproto->groups);
> +
>  connmgr_destroy(ofproto->connmgr);
>  
>  hmap_remove(&all_ofprotos, &ofproto->hmap_node);
> @@ -4952,23 +4957,16 @@ delete_group__(struct ofgroup *ofgroup)
>  
>  /* Implements OFPGC_DELETE. */
>  static void
> -delete_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm)
> +delete_group(struct ofproto *ofproto, uint32_t group_id)
>  {
> -if (gm->group_id == OFPG_ALL) {
> +if (group_id == OFPG_ALL) {
>  struct ofgroup *ofgroup, *next;
>  
> -/* Remove indirect groups first, to allow other groups to be removed
> - * successfully. */
> -HMAP_FOR_EACH_SAFE (ofgroup, next, hmap_node, &ofproto->groups) {
> -if (ofgroup->type == OFPGT11_INDIRECT) {
> -delete_group__(ofgroup);
> -}
> -}
>  HMAP_FOR_EACH_SAFE (ofgroup, next, hmap_node, &ofproto->groups) {
>  delete_group__(ofgroup);
>  }
>  } else {
> -struct ofgroup *ofgroup = ofproto_group_lookup(ofproto, 
> gm->group_id);
> +struct ofgroup *ofgroup = ofproto_group_lookup(ofproto, group_id);
>  if (ofgroup) {
>  delete_group__(ofgroup);
>  }
> @@ -5000,7 +4998,7 @@ handle_group_mod(struct ofconn *ofconn, const struct 
> ofp_header *oh)
>  return modify_group(ofproto, &gm);
>  
>  case OFPGC11_DELETE:
> -delete_group(ofproto, &gm);
> +delete_group(ofproto, gm->group_id);
>  return 0;
>  
>  default:
> -- 
> 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] Recirculation with milti-threaded miss handling

2013-08-30 Thread Simon Horman
On Wed, Aug 28, 2013 at 05:45:28PM +0900, Simon Horman wrote:
> Hi Ethan,
> 
> On Wed, Aug 28, 2013 at 12:25:31AM -0700, Ethan Jackson wrote:
> > > In short, it would make my life much easier if facets could be looked up
> > > execute_flow_miss(). And I am interested to know if you have any plans in
> > > that regards.
> > 
> > This actually is in the exact opposite of the direction I'm planning
> > to move.  Facets don't scale to tens of thousands of datapath flows,
> > are confusing, and are difficult to maintain, so I'm in the process of
> > cleaning up patches I've written which do away with them entirely.  I
> > don't have a lot of context on this recirculation stuff, but it sounds
> > like you'll need some thread-safe global state, completely independent
> > of facets, which can be used for your purposes.  A giant global hash
> > table of some sort perhaps.
> 
> I had assumed that it was the opposite of your planned direction which
> is why I didn't jump in and start accessing the facets.
> 
> > 
> > Comments inline.
> > 
> > >However, my understanding is that with your recent changes facets 
> > > aren't
> > >available in the thread where misses are executed which seems to be 
> > > when
> > >action translation occurs and the rule is required.
> > 
> > Correct, and indeed facets are going away.
> > 
> > >I had planned to get around this by creating a new recirculator
> > >structure which would hold a recirculation id and flow. The flow
> > >holds the parent recirculation id and it should be possible to
> > >use this to find the top-most recirculator. This would provide
> > >the thus the top-most flow which could be use for rule lookup.
> > 
> > Sounds like the right approach, but again I don't have much context.
> > 
> > >However I see two problems with this scheme:
> > >
> > >i.  If multiple misses are in-flight but handled in different
> > >flow_miss_batches then multiple recirculators would be created for
> > >what is in fact the same flow. This doesn't map comfortably to the
> > >idea associate a recirculator with a facet. Though in a pinch it
> > >ought to be possible to associate multiple recirculators with a
> > >facet.
> > 
> > When facets go away flow miss batches will go away to.  A miss handler
> > will forward the packet and then be done with it, a new thread called
> > a "revalidator" will later clean up unused flows in the datapath
> > created by the miss handlers.
> 
> Ok, so the "revalidator" will know which flows to expire from the datapath
> without the need for facets in ovs-vswitchd?
> 
> > 
> > >Another solution would be to allow recirculators to be looked up be
> > >classifier. But it seems this would require re-working or 
> > > augmenting
> > >the thread-safety of classifiers as in the scheme I propose
> > >recirculators are created when translation occurs and this is not 
> > > in
> > >the main thread where write-access to classifiers is thread-safe.
> > 
> > Perhaps they could be given they're own classifier?  The classifier
> > itself is thread safe if used within the context of a rwlock.
> 
> Thanks, that that sounds like it could work.
> 
> > >ii.  A more acute problem seems to that it seems that revalidation
> > > should occur when rule lookup for a recirculated packet occurs.
> > > This is because the scheme described above involves using offsets,
> > > calculated during previous action translations, to determine which
> > > part of a rules ofpacts should be used when translating actions
> > > for a recirculated packet.
> > 
> > Revalidation is also going away.  There will only be the equivalent of
> > the "expire" function.  If the datapath actions happen to be wrong, of
> > course we'll fix them, but this won't have any special machinery.
> > 
> > If you'd like I could send you a prototype version of what I have in
> > the works so you could start planning.  It's a couple of weeks away
> > from being ready for an official posting on the dev list, but you can
> > get a sense of the jist.
> 
> Thanks, a prototype would be very helpful in conjunction with your
> comments to come up with a plan for recirculation.

Hi Ethan,

I'm wondering if you have a prototype that you could post at this time.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [RFC PATCH net-next 1/4] iptunnels: remove net arg from iptunnel_xmit()

2013-08-30 Thread Pravin Shelar
On Wed, Aug 28, 2013 at 8:20 AM, Nicolas Dichtel
 wrote:
> This argument is not used, let's remove it.
>
> Signed-off-by: Nicolas Dichtel 
> ---
>  drivers/net/vxlan.c | 3 +--
>  include/net/ip_tunnels.h| 3 +--
>  net/ipv4/ip_tunnel.c| 3 +--
>  net/ipv4/ip_tunnel_core.c   | 3 +--
>  net/ipv6/sit.c  | 4 ++--
>  net/openvswitch/vport-gre.c | 2 +-
>  6 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 3b21aca0c0c2..3bc27c2ca569 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1171,8 +1171,7 @@ int vxlan_xmit_skb(struct net *net, struct vxlan_sock 
> *vs,
> if (err)
> return err;
>
> -   return iptunnel_xmit(net, rt, skb, src, dst,
> -   IPPROTO_UDP, tos, ttl, df);
> +   return iptunnel_xmit(rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df);

Can you remove argument `net` from vxlan_xmit_skb() also?

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