[ovs-dev] provide mould and finished parts

[sili]

Dear Sirs,
 
We specialize in Plastic injection Mold, Die Casting Mold, Metal Stamping Mold. 
We can produce the top quality molds according to your requirements with our 
independent development team, optimized operating procedures, updated 
facilities,strictly quality control system…  
 
If you have any cooperative intentions，Welcome to: www.ebmouldings.com
 
We keen on build high quality molds and excellent products to to all customers 
with low cost . 
 
If you have any projects or any request, pls feel free to let us know, We look 
forward to working with you on your current and future projects. 
 
Looking forward to your kind reply, thanks!

reanda

Eb Mouldings Co.,Ltd
Tel:+86 755 29664151
Fax:+86 755 28942507
Skype ID:reanda-ebmould
MSN ID:reanda-ebmould
www.ebmouldings.com
rea...@ebmouldings.com
Address: 12F,1,6block,zhao shang guo ling area ,xi xiang town ,bao'an 
district,shen zhen ,guang dong province,China Company name: Eb Mouldings Co;Ltd
Postcode: 518102
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 2/2] vswitchd: Limit fake bridge MAC selection to ports in the same VLAN

[Helmut Schaa]

Limit fake bridge MAC address selection to only consider ports
that use the same VLAN as the fake bridge itself.

This prevents OVS from selecting a MAC address that was not really
present in the VLAN of the fake bridge before.

Signed-off-by: Helmut Schaa 
---

I'm running this patchset on top of the 2.0 branch right now so
I'd appreciate some review before applying.

Thanks,
Helmut

 vswitchd/bridge.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 8223b9e..f86a883 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -252,7 +252,7 @@ static struct iface *iface_lookup(const struct bridge *, 
const char *name);
 static struct iface *iface_find(const char *name);
 static struct iface *iface_from_ofp_port(const struct bridge *,
  ofp_port_t ofp_port);
-static void iface_set_mac(struct iface *, const uint8_t *);
+static void iface_set_mac(const struct bridge *, const struct port *, struct 
iface *);
 static void iface_set_ofport(const struct ovsrec_interface *, ofp_port_t 
ofport);
 static void iface_clear_db_record(const struct ovsrec_interface *if_cfg);
 static void iface_configure_qos(struct iface *, const struct ovsrec_qos *);
@@ -578,7 +578,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
*ovs_cfg)
 iface_set_ofport(iface->cfg, iface->ofp_port);
 iface_configure_cfm(iface);
 iface_configure_qos(iface, port->cfg->qos);
-iface_set_mac(iface, port->cfg->fake_bridge ? br->ea : NULL);
+iface_set_mac(br, port, iface);
 ofproto_port_set_bfd(br->ofproto, iface->ofp_port,
  &iface->cfg->bfd);
 }
@@ -1568,8 +1568,8 @@ bridge_configure_mac_table(struct bridge *br)
 }
 
 static void
-find_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
-   struct iface **hw_addr_iface)
+find_local_hw_addr(const struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
+   const struct port *fake_br, struct iface **hw_addr_iface)
 {
 struct hmapx mirror_output_ports;
 struct port *port;
@@ -1632,6 +1632,14 @@ find_local_hw_addr(struct bridge *br, uint8_t 
ea[ETH_ADDR_LEN],
 continue;
 }
 
+/* For fake bridges we only choose from ports with the same tag */
+if (fake_br && fake_br->cfg && fake_br->cfg->tag) {
+if (!port->cfg->tag)
+continue;
+if (*port->cfg->tag != *fake_br->cfg->tag)
+continue;
+}
+
 /* Grab MAC. */
 error = netdev_get_etheraddr(iface->netdev, iface_ea);
 if (error) {
@@ -1681,7 +1689,7 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t 
ea[ETH_ADDR_LEN],
 }
 
 /* Find a local hw address */
-find_local_hw_addr(br, ea, hw_addr_iface);
+find_local_hw_addr(br, ea, NULL, hw_addr_iface);
 }
 
 /* Choose and returns the datapath ID for bridge 'br' given that the bridge
@@ -3496,9 +3504,10 @@ iface_from_ofp_port(const struct bridge *br, ofp_port_t 
ofp_port)
 /* Set Ethernet address of 'iface', if one is specified in the configuration
  * file. */
 static void
-iface_set_mac(struct iface *iface, const uint8_t *mac)
+iface_set_mac(const struct bridge *br, const struct port *port, struct iface 
*iface)
 {
-uint8_t ea[ETH_ADDR_LEN];
+uint8_t ea[ETH_ADDR_LEN], *mac = NULL;
+struct iface *hw_addr_iface;
 
 if (strcmp(iface->type, "internal")) {
 return;
@@ -3506,6 +3515,10 @@ iface_set_mac(struct iface *iface, const uint8_t *mac)
 
 if (iface->cfg->mac && eth_addr_from_string(iface->cfg->mac, ea)) {
 mac = ea;
+} else if (port->cfg->fake_bridge) {
+/* Fake bridge and no MAC set in the configuration. Pick a local one. 
*/
+find_local_hw_addr(br, ea, port, &hw_addr_iface);
+mac = ea;
 }
 
 if (mac) {
-- 
1.8.1.4

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

[ovs-dev] [PATCH 1/2] vswitchd: Split pick_local_hw_addr

[Helmut Schaa]

This allows to reuse find_local_hw_addr for fake bridges.
Preparation for upcoming MAC address inheritance for fake bridges.

Signed-off-by: Helmut Schaa 
---
 vswitchd/bridge.c | 44 ++--
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index cde4bd0..8223b9e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1568,31 +1568,15 @@ bridge_configure_mac_table(struct bridge *br)
 }
 
 static void
-bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
-  struct iface **hw_addr_iface)
+find_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
+   struct iface **hw_addr_iface)
 {
 struct hmapx mirror_output_ports;
-const char *hwaddr;
 struct port *port;
 bool found_addr = false;
 int error;
 int i;
 
-*hw_addr_iface = NULL;
-
-/* Did the user request a particular MAC? */
-hwaddr = smap_get(&br->cfg->other_config, "hwaddr");
-if (hwaddr && eth_addr_from_string(hwaddr, ea)) {
-if (eth_addr_is_multicast(ea)) {
-VLOG_ERR("bridge %s: cannot set MAC address to multicast "
- "address "ETH_ADDR_FMT, br->name, ETH_ADDR_ARGS(ea));
-} else if (eth_addr_is_zero(ea)) {
-VLOG_ERR("bridge %s: cannot set MAC address to zero", br->name);
-} else {
-return;
-}
-}
-
 /* Mirror output ports don't participate in picking the local hardware
  * address.  ofproto can't help us find out whether a given port is a
  * mirror output because we haven't configured mirrors yet, so we need to
@@ -1676,6 +1660,30 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t 
ea[ETH_ADDR_LEN],
 hmapx_destroy(&mirror_output_ports);
 }
 
+static void
+bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
+  struct iface **hw_addr_iface)
+{
+const char *hwaddr;
+*hw_addr_iface = NULL;
+
+/* Did the user request a particular MAC? */
+hwaddr = smap_get(&br->cfg->other_config, "hwaddr");
+if (hwaddr && eth_addr_from_string(hwaddr, ea)) {
+if (eth_addr_is_multicast(ea)) {
+VLOG_ERR("bridge %s: cannot set MAC address to multicast "
+ "address "ETH_ADDR_FMT, br->name, ETH_ADDR_ARGS(ea));
+} else if (eth_addr_is_zero(ea)) {
+VLOG_ERR("bridge %s: cannot set MAC address to zero", br->name);
+} else {
+return;
+}
+}
+
+/* Find a local hw address */
+find_local_hw_addr(br, ea, hw_addr_iface);
+}
+
 /* Choose and returns the datapath ID for bridge 'br' given that the bridge
  * Ethernet address is 'bridge_ea'.  If 'bridge_ea' is the Ethernet address of
  * an interface on 'br', then that interface must be passed in as
-- 
1.8.1.4

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

Re: [ovs-dev] [PATCH] ofp-actions: Consider pop MPLS to non-MPLS ethtype as consistent

[Simon Horman]

On Tue, Feb 18, 2014 at 10:43:27AM -0800, Jarno Rajahalme wrote:
> 
> On Feb 16, 2014, at 5:11 PM, Simon Horman  wrote:
> 
> > On Mon, Feb 17, 2014 at 10:06:55AM +0900, Simon Horman wrote:
> >> Remove the restriction that pop MPLS to an ethtype is
> >> considered inconsistent.
> >> 
> >> As it happens this is allowed for OpenFlow 1.0, and thus
> >> the tests in the Open vSwtich test-suite, by virtue of
> >> inconsistent actions being allowed in the case of OpenFlow 1.0.
> >> 
> >> Its not clear to me why they should be considered inconsistent
> >> or disallowed for other OpenFlow versions.
> >> 
> >> This was found using Ryu tests via the new make check-ryu target.
> >> It allows all of the "POP_MPLS"[1] tests to pass where they previously
> >> failed.
> >> 
> >> [1] http://osrg.github.io/ryu-certification/switch/ovs
> >> 
> >> Signed-off-by: Simon Horman 
> >> ---
> >> lib/ofp-actions.c | 3 ---
> >> 1 file changed, 3 deletions(-)
> >> 
> >> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> >> index 781c3a1..623c726 100644
> >> --- a/lib/ofp-actions.c
> >> +++ b/lib/ofp-actions.c
> >> @@ -2068,9 +2068,6 @@ ofpact_check__(enum ofputil_protocol 
> >> *usable_protocols, struct ofpact *a,
> >> 
> >> case OFPACT_POP_MPLS:
> >> flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype;
> >> -if (!eth_type_mpls(flow->dl_type)) {
> >> -inconsistent_match(usable_protocols);
> >> -}
> >> return 0;
> >> 
> >> case OFPACT_SAMPLE:
> > 
> > Looking over this another time, I wonder if the eth_type_mpls()
> > check should be moved to above the flow->dl_type = 
> > ofpact_get_POP_MPLS(a)->ethertype
> > line. pop MPLS on a non MPLS packet would indeed be inconsistent.
> 
> 
> That sounds right to me,

Thanks, I will send an updated patch.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] Working on ONF Extensions for OF1.3.X

[Shivanker Goel]

Hi guys,

I am a CS Major, Undergraduate student at IIT Delhi. I was looking for a
roughly 6-7 week project on the lines of extending OVS with OF 1.3+ specs.
So I had a look at the OPENFLOW-1.1+ document in OVS and think that I could
work on some topics in ONF 1.3.X Extensions required for OF1.4. I got
particularly interested in Vacancy Events, Bundles and Table
Synchronization.

So my question is that is someone already working on these features? Any
pointers to relevant information would also be highly appreciated.
Also, keeping in mind that I am not familiar with OVS's code, should I
pursue all 3 of these extensions, or less or more? The expected amount of
effort I'll be putting in is about 7-8 hours a week. As for my experience,
I am familiar with using Mininet with controllers like POX and FloodLight,
and understand OpenFlow.

Thank you very much.

Best regards,
Shivanker Goel
Junior Year Undergraduate
Computer Science and Engg.
Indian Institute of Technology, Delhi
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ofp-actions: Correct pop MPLS ethtype as consistency test

[Simon Horman]

Correct pop MPLS ethtype consistency check to verify that
the packet has an MPLS ethtype before the pop action rather than after:
an MPLS ethtype is a pre-condition but not a post-condition of pop MPLS.

With this change the consistency check in ofpact_check__()
becomes consistent with that in ofpact_from_nxast().

This was found using Ryu tests via the new make check-ryu target.
It allows all of the "POP_MPLS"[1] tests to pass where they previously
failed.

[1] http://osrg.github.io/ryu-certification/switch/ovs

Cc: Jarno Rajahalme 
Signed-off-by: Simon Horman 
---
 lib/ofp-actions.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 781c3a1..3cd1e8b 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -2067,10 +2067,10 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, 
struct ofpact *a,
 return 0;
 
 case OFPACT_POP_MPLS:
-flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype;
 if (!eth_type_mpls(flow->dl_type)) {
 inconsistent_match(usable_protocols);
 }
+flow->dl_type = ofpact_get_POP_MPLS(a)->ethertype;
 return 0;
 
 case OFPACT_SAMPLE:
-- 
1.8.5.2

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

[ovs-dev] [PATCH] ofp-util: Fix a typo in ofputil_decode_ofp11_port

[kmindg]

Signed-off-by: kmindg 
---
 lib/ofp-util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7de82c6..aa25e67 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3640,7 +3640,7 @@ ofputil_decode_ofp11_port(struct ofputil_phy_port *pp,
 ovs_strlcpy(pp->name, op->name, OFP_MAX_PORT_NAME_LEN);
 
 pp->config = ntohl(op->config) & OFPPC11_ALL;
-pp->state = ntohl(op->state) & OFPPC11_ALL;
+pp->state = ntohl(op->state) & OFPPS11_ALL;
 
 pp->curr = netdev_port_features_from_ofp11(op->curr);
 pp->advertised = netdev_port_features_from_ofp11(op->advertised);
-- 
1.9.0

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

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

[Rafael Duarte Vencioneck]

Digging a little in ovs kernel module code, I can't find a way to verify if
a given port is up or down before execute an output. Is there a way to do
that?
OVS version 1.9.3

Thanks,
Rafael Duarte Vencioneck
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] OVS Netlink zerocopy vs Xen netback zerocopy

[Zoltan Kiss]

Hi,

Currently I'm working on a patchset which reintroduces grant mapping 
into netback. We used it before Linux Xen bits were upstreamed, but we 
had to change to grant copy as the original solution were fundamentally 
not upstreamable. But the advantage would be huge, as we could replace 
copy guest pages by Xen to mapping guest pages to Dom0.
Parallel to this I'm working on a grant mapping optimization, which 
makes it possible to avoid m2p_override for grant mapped pages. It 
causes lock contention and we don't need it if the pages doesn't go to 
userspace. This could be a safe assumption, as those pages would stay in 
kernel space while switched by OVS, and if they end up on the local 
port, delivered to Dom0 IP stack, deliver_skb will call skb_orphan_frags 
which swaps out those foreign (=grant mapped from guest) pages by local 
copies and notify netback through a callback that it can give back the 
pages to the guest.


And after that bit long introduction here comes the main question: OVS 
recently introduced Netlink zerocopy, which by my understanding means 
that Netlink messages from kernel are not copied but mapped to 
userspace. And such message can contain a whole packet if it haven't 
matched any flows in the kernel, or the flow action said so. As far as I 
saw skb_zerocopy will clone the frags from the real packet skb to the 
Netlink skb. Note, the linear buffer is local memory in netback case as 
well, we copy the beginning of the packet (max 128 bytes) there, only 
the pages on frags are foreign ones.
I don't know the internals of Netlink that much, how a packet is 
forwarded up in this case, but that concerns me, as if the pages on the 
skb_shinfo(skb)->frags array are still the foreign ones, and userspace 
wants to touch that data, we are in trouble.
If this is the scenario, I think the best would be to call 
skb_orphan_frags before skb_zerocopy in queue_userspace_packet, so the 
frags will become local. Fortunately this is a corner case, as it 
shouldn't happen very often that the kernel sends up packets bigger than 
128 bytes.


What do you think about the solution in the last paragraph? Or do we 
need it at all?


Regards,

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

Re: [ovs-dev] [ovs-discuss] Working on ONF Extensions for OF1.3.X

[Ben Pfaff]

On Wed, Feb 19, 2014 at 05:32:19PM +0530, Shivanker Goel wrote:
> I am a CS Major, Undergraduate student at IIT Delhi. I was looking for a
> roughly 6-7 week project on the lines of extending OVS with OF 1.3+ specs.
> So I had a look at the OPENFLOW-1.1+ document in OVS and think that I could
> work on some topics in ONF 1.3.X Extensions required for OF1.4. I got
> particularly interested in Vacancy Events, Bundles and Table
> Synchronization.
> 
> So my question is that is someone already working on these features? Any
> pointers to relevant information would also be highly appreciated.
> Also, keeping in mind that I am not familiar with OVS's code, should I
> pursue all 3 of these extensions, or less or more? The expected amount of
> effort I'll be putting in is about 7-8 hours a week. As for my experience,
> I am familiar with using Mininet with controllers like POX and FloodLight,
> and understand OpenFlow.

I don't know of anyone working on these features.

I would pick one small project to start and then later take on more if
you have time.  Bundles are a huge project, table synchronization is
pretty pointless for a software switch, and so I suggest vacancy events.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [ovs-discuss] Working on ONF Extensions for OF1.3.X

[Shivanker Goel]

On Wed, Feb 19, 2014 at 9:40 PM, Ben Pfaff  wrote:

> I don't know of anyone working on these features.
>
> I would pick one small project to start and then later take on more if
> you have time.  Bundles are a huge project, table synchronization is
> pretty pointless for a software switch, and so I suggest vacancy events.
>

Thanks a lot Ben.
Since they are very related, I'll start working on flow entry eviction and
vacancy events.


Best regards,
Shivanker Goel
Junior Year Undergraduate
Computer Science and Engg.
Indian Institute of Technology, Delhi
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] OPENFLOW-1.1+: Remove "rework tag order" from OF1.3.

[Ben Pfaff]

OpenFlow 1.3.4 is going to revert this change that OpenFlow 1.3 made.

Signed-off-by: Ben Pfaff 
---
 OPENFLOW-1.1+ | 4 
 1 file changed, 4 deletions(-)

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index eaf2ee9..8c5811b 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -129,10 +129,6 @@ didn't compare the specs carefully yet.)
   this (but we'd accept an implementation).
   [optional for OF1.3+]
 
-* Rework tag order.
-  Part of MPLS patchset by Simon Horman.
-  [required for v1.3+]
-
 * On-demand flow counters.  I think this might be a real
   optimization in some cases for the software switch.
   [optional for OF1.3+]
-- 
1.8.5.3

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

[ovs-dev] [PATCH] OPENFLOW-1.1+: Update status of a few items.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 OPENFLOW-1.1+ | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index 8c5811b..388f5fd 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -56,7 +56,7 @@ probably incomplete.
 
 * OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
   implement it.  It should be implemented so that the default OVS
-  behavior does not change.
+  behavior does not change.  Simon Horman has posted a patch.
   [required for OF1.1 and OF1.2]
 
 * MPLS.  Simon Horman maintains a patch series that adds this
@@ -91,8 +91,8 @@ didn't compare the specs carefully yet.)
   Currently we always report OFPBRC_MULTIPART_BUFFER_OVERFLOW.
   [optional for OF1.3+]
 
-* Add OFPMP_TABLE_FEATURES statistics.
-  [optional for OF1.3+]
+* Add OFPMP_TABLE_FEATURES statistics.  Alexander Wu has posted a
+  patch series.  [optional for OF1.3+]
 
 * More flexible table miss support.
   This requires the following.
-- 
1.8.5.3

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

Re: [ovs-dev] [PATCH] sFlow LAG and Tunnel export

[Neil McKee]

Finally getting back to this.  Can we do it one piece at a time?  For
example,  attached is a patch that just adds counters to the LACP
module and a call to retrieve them.

Neil
--
Neil McKee
InMon Corp.
http://www.inmon.com


On Mon, Jan 13, 2014 at 8:29 AM, Neil McKee  wrote:
> Thanks for the detailed explanation.  You should teach a class :)I'll
> rework the patch.
>
> Neil
>
>
>
> --
> Neil McKee
> InMon Corp.
> http://www.inmon.com
>
>
> On Thu, Jan 9, 2014 at 11:43 AM, Ben Pfaff  wrote:
>>
>> On Fri, Jan 03, 2014 at 09:39:00PM -0800, Neil McKee wrote:
>> > On Mon, Dec 30, 2013 at 4:08 PM, Ben Pfaff  wrote:
>> > > I'm not a fan of callbacks because of the surprising inversion of
>> > > control that they cause, and especially not of layered callbacks like
>> > > the one that this introduces.  Instead of calling back into
>> > > dpif_sflow_port_lookup_callback() from sflow_agent_get_counters(), I
>> > > think that it would be better to provide the necessary information in
>> > > the call to dpif_sflow_add_port().
>> > >
>> > >
>> > I wasn't expecting you to say that.  The purpose of the callback is
>> > really just to access the state held within ofproto-dpif,  but in an
>> > encapsulated way so that the exposure is limited and controlled.
>> > The alternatives were (1) to make those private structure fields
>> > visible to the ofproto-dpif-sflow module so it could just read them
>> > directly, or (2) to copy all the relevant state into a separate
>> > hash-table just for ofproto-dpif-sflow to use when processing packet
>> > samples. It sounds like you prefer (2)?   To understand why I think
>> > I just need to know what you mean by "surprising inversion of control".
>> > Are you referring to the possibility of mutex contention/deadlock
>> > if the data-structures within ofproto-dpif are interrogated from
>> > within the packet-processing path (i.e. when processing an
>> > sFlow packet sample)?
>>
>> Let me step back and explain, since my phrasing didn't help.  I'm
>> going to explain one of my own views of program structure.  Most of it
>> is opinion, not fact, so bear with me a little.
>>
>> The call stack in a well-structured large program usually follows a
>> tree structure from one logical module (often, one source file) to
>> another.  Often, you can consider one module to be "higher level" or
>> "lower level" than another based on the rule that higher level modules
>> call into lower level modules, but not vice versa.  Sometimes calls go
>> both ways, and in my experience that is undesirable because it makes
>> the structure of the program harder to understand.  Direct calls are
>> one thing, because it is easy enough to trace through them with "grep"
>> and other tools, but indirect calls through callbacks are harder to
>> understand.  Actually, there are at least two classes of callbacks:
>> callbacks that you pass into a function for that function to call, and
>> callbacks that you store somewhere in data owned by a module that in
>> theory any function in that module *could* call (even if it doesn't
>> currently).  The latter are harder to understand.
>>
>> My personal idea of nightmarish tangles of callbacks is GObject and in
>> particular GTK+.  Everything is a callback and it's very difficult to
>> figure out exactly when one of them could get called.  You end up
>> writing bad code because you can't figure it out.  (GTK+ has other
>> problems too, that's just one of them.)
>>
>> Looking at the callback in question again, it's going to be difficult
>> to maintain for a reason that you basically called out in the comment,
>> that is, locking.  In most places in OVS we are able to use Clang
>> thread safety annotations to statically check (to some basic level of
>> safety, anyhow) that locks are held or not held at various places.
>> But those checks can't flow through callback functions:
>>
>> /* XXX do we need to acquire ofproto_mutex here?  Or should it be
>> acquired in handle_sflow_upcall() below,
>>  * so that it will not be released until after the sFlow-module has
>> followed the pointers we are giving
>>  * it here and the sflow sample has been fully processed? */
>>
>> So I'd prefer, if we can avoid it, to use some interface more direct
>> than a callback.
>
>


0001-Add-lacp-couters-and-a-call-to-retrieve-them.patch
Description: Binary data
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ovs-vsctl: reconnect to the database if connection was dropped

[Ben Pfaff]

On Tue, Feb 18, 2014 at 02:34:58PM -0800, Ansis Atteka wrote:
> If ovs-vsctl has to wait for ovs-vswitchd to reconfigure itself
> according to the new database, then sometimes ovs-vsctl could
> end up stuck in the event loop if OVSDB connection was dropped
> while ovs-vsctl was still running.
> 
> This patch fixes this problem by letting ovs-vsctl to reconnect
> to the OVSDB, if it has to wait cur_cfg field to be updated.
> 
> Issue: 1191997
> Reported-by: Spiro Kourtessis 
> Signed-Off-By: Ansis Atteka 

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

[ovs-dev] [PATCH] ofproto: Remove 'force-miss-model' configuration.

[Joe Stringer]

This configuration item was introduced to assist testing of upcall
handling behaviour with and without facets. Facets were removed in
commit e79a6c833e0d7237, so this patch removes the configuration item.

Signed-off-by: Joe Stringer 
---
 ofproto/ofproto-provider.h |4 
 ofproto/ofproto.c  |8 
 ofproto/ofproto.h  |8 
 vswitchd/bridge.c  |   20 
 vswitchd/vswitch.xml   |   22 --
 5 files changed, 62 deletions(-)

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 08a8ac1..d05adbc 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -468,10 +468,6 @@ extern unsigned ofproto_flow_limit;
  * ofproto-dpif implementation. */
 extern size_t n_handlers, n_revalidators;
 
-/* Determines which model to use for handling misses in the ofproto-dpif
- * implementation */
-extern enum ofproto_flow_miss_model flow_miss_model;
-
 static inline struct rule *
 rule_from_cls_rule(const struct cls_rule *cls_rule)
 {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 62c97a2..f69736c 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -307,7 +307,6 @@ static size_t allocated_ofproto_classes;
 struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
 
 unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT;
-enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
 
 size_t n_handlers, n_revalidators;
 
@@ -698,13 +697,6 @@ ofproto_set_flow_limit(unsigned limit)
 ofproto_flow_limit = limit;
 }
 
-/* Sets the path for handling flow misses. */
-void
-ofproto_set_flow_miss_model(unsigned model)
-{
-flow_miss_model = model;
-}
-
 /* If forward_bpdu is true, the NORMAL action will forward frames with
  * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is 
false,
  * the NORMAL action will drop these frames. */
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 0ac4454..1f9cb15 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -215,13 +215,6 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
 
 #define OFPROTO_FLOW_LIMIT_DEFAULT 20
 
-/* How flow misses should be handled in ofproto-dpif */
-enum ofproto_flow_miss_model {
-OFPROTO_HANDLE_MISS_AUTO,   /* Based on flow eviction threshold. */
-OFPROTO_HANDLE_MISS_WITH_FACETS,/* Always create facets. */
-OFPROTO_HANDLE_MISS_WITHOUT_FACETS  /* Always handle without facets.*/
-};
-
 const char *ofproto_port_open_type(const char *datapath_type,
const char *port_type);
 int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t *ofp_portp);
@@ -243,7 +236,6 @@ void ofproto_set_extra_in_band_remotes(struct ofproto *,
const struct sockaddr_in *, size_t n);
 void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
 void ofproto_set_flow_limit(unsigned limit);
-void ofproto_set_flow_miss_model(unsigned model);
 void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
 void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
   size_t max_entries);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index cde4bd0..e7dd597 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -194,7 +194,6 @@ static void bridge_del_ports(struct bridge *,
 static void bridge_add_ports(struct bridge *,
  const struct shash *wanted_ports);
 
-static void bridge_configure_flow_miss_model(const char *opt);
 static void bridge_configure_datapath_id(struct bridge *);
 static void bridge_configure_netflow(struct bridge *);
 static void bridge_configure_forward_bpdu(struct bridge *);
@@ -499,9 +498,6 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
*ovs_cfg)
 smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0),
 smap_get_int(&ovs_cfg->other_config, "n-revalidator-threads", 0));
 
-bridge_configure_flow_miss_model(smap_get(&ovs_cfg->other_config,
-  "force-miss-model"));
-
 /* Destroy "struct bridge"s, "struct port"s, and "struct iface"s according
  * to 'ovs_cfg', with only very minimal configuration otherwise.
  *
@@ -878,22 +874,6 @@ port_configure(struct port *port)
 free(s.lacp_slaves);
 }
 
-static void
-bridge_configure_flow_miss_model(const char *opt)
-{
-enum ofproto_flow_miss_model model = OFPROTO_HANDLE_MISS_AUTO;
-
-if (opt) {
-if (!strcmp(opt, "with-facets")) {
-model = OFPROTO_HANDLE_MISS_WITH_FACETS;
-} else if (!strcmp(opt, "without-facets")) {
-model = OFPROTO_HANDLE_MISS_WITHOUT_FACETS;
-}
-}
-
-ofproto_set_flow_miss_model(model);
-}
-
 /* Pick local port hardware address and datapath ID for 'br'. */
 static void
 bridge_configure_datapath_id(struct bridge *br)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index e9

[ovs-dev] [PATCH v2] OPENFLOW-1.1+: Remove "rework tag order" and MPLS BoS matching from OF1.3.

[Ben Pfaff]

OpenFlow 1.3.4 is going to revert the tag order change made by OpenFlow
1.3.

MPLS BoS matching is implemented.

Signed-off-by: Ben Pfaff 
---
v1->v2: Also remove MPLS BoS matching since that's been implemented.

 OPENFLOW-1.1+ |8 
 1 file changed, 8 deletions(-)

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index eaf2ee9..b58fec9 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -121,18 +121,10 @@ didn't compare the specs carefully yet.)
   some kind of "hardware" support, if we judged it useful enough.)
   [optional for OF1.3+]
 
-* MPLS BoS matching.
-  Part of MPLS patchset by Simon Horman.
-  [optional for OF1.3+]
-
 * Provider Backbone Bridge tagging.  I don't plan to implement
   this (but we'd accept an implementation).
   [optional for OF1.3+]
 
-* Rework tag order.
-  Part of MPLS patchset by Simon Horman.
-  [required for v1.3+]
-
 * On-demand flow counters.  I think this might be a real
   optimization in some cases for the software switch.
   [optional for OF1.3+]
-- 
1.7.10.4

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

Re: [ovs-dev] [PATCH v2] OPENFLOW-1.1+: Remove "rework tag order" and MPLS BoS matching from OF1.3.

[Joe Stringer]

Acked-by: Joe Stringer 


On 19 February 2014 10:30, Ben Pfaff  wrote:

> OpenFlow 1.3.4 is going to revert the tag order change made by OpenFlow
> 1.3.
>
> MPLS BoS matching is implemented.
>
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: Also remove MPLS BoS matching since that's been implemented.
>
>  OPENFLOW-1.1+ |8 
>  1 file changed, 8 deletions(-)
>
> diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> index eaf2ee9..b58fec9 100644
> --- a/OPENFLOW-1.1+
> +++ b/OPENFLOW-1.1+
> @@ -121,18 +121,10 @@ didn't compare the specs carefully yet.)
>some kind of "hardware" support, if we judged it useful enough.)
>[optional for OF1.3+]
>
> -* MPLS BoS matching.
> -  Part of MPLS patchset by Simon Horman.
> -  [optional for OF1.3+]
> -
>  * Provider Backbone Bridge tagging.  I don't plan to implement
>this (but we'd accept an implementation).
>[optional for OF1.3+]
>
> -* Rework tag order.
> -  Part of MPLS patchset by Simon Horman.
> -  [required for v1.3+]
> -
>  * On-demand flow counters.  I think this might be a real
>optimization in some cases for the software switch.
>[optional for OF1.3+]
> --
> 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] OPENFLOW-1.1+: Update status of a few items.

[Joe Stringer]

Acked-by: Joe Stringer 


On 19 February 2014 08:29, Ben Pfaff  wrote:

> Signed-off-by: Ben Pfaff 
> ---
>  OPENFLOW-1.1+ | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> index 8c5811b..388f5fd 100644
> --- a/OPENFLOW-1.1+
> +++ b/OPENFLOW-1.1+
> @@ -56,7 +56,7 @@ probably incomplete.
>
>  * OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
>implement it.  It should be implemented so that the default OVS
> -  behavior does not change.
> +  behavior does not change.  Simon Horman has posted a patch.
>[required for OF1.1 and OF1.2]
>
>  * MPLS.  Simon Horman maintains a patch series that adds this
> @@ -91,8 +91,8 @@ didn't compare the specs carefully yet.)
>Currently we always report OFPBRC_MULTIPART_BUFFER_OVERFLOW.
>[optional for OF1.3+]
>
> -* Add OFPMP_TABLE_FEATURES statistics.
> -  [optional for OF1.3+]
> +* Add OFPMP_TABLE_FEATURES statistics.  Alexander Wu has posted a
> +  patch series.  [optional for OF1.3+]
>
>  * More flexible table miss support.
>This requires the following.
> --
> 1.8.5.3
>
> ___
> 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] OPENFLOW-1.1+: Update status of a few items.

[Ben Pfaff]

Thanks, I'll apply this soon.

On Wed, Feb 19, 2014 at 10:33:42AM -0800, Joe Stringer wrote:
> Acked-by: Joe Stringer 
> 
> 
> On 19 February 2014 08:29, Ben Pfaff  wrote:
> 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  OPENFLOW-1.1+ | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> > index 8c5811b..388f5fd 100644
> > --- a/OPENFLOW-1.1+
> > +++ b/OPENFLOW-1.1+
> > @@ -56,7 +56,7 @@ probably incomplete.
> >
> >  * OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
> >implement it.  It should be implemented so that the default OVS
> > -  behavior does not change.
> > +  behavior does not change.  Simon Horman has posted a patch.
> >[required for OF1.1 and OF1.2]
> >
> >  * MPLS.  Simon Horman maintains a patch series that adds this
> > @@ -91,8 +91,8 @@ didn't compare the specs carefully yet.)
> >Currently we always report OFPBRC_MULTIPART_BUFFER_OVERFLOW.
> >[optional for OF1.3+]
> >
> > -* Add OFPMP_TABLE_FEATURES statistics.
> > -  [optional for OF1.3+]
> > +* Add OFPMP_TABLE_FEATURES statistics.  Alexander Wu has posted a
> > +  patch series.  [optional for OF1.3+]
> >
> >  * More flexible table miss support.
> >This requires the following.
> > --
> > 1.8.5.3
> >
> > ___
> > 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 v2] OPENFLOW-1.1+: Remove "rework tag order" and MPLS BoS matching from OF1.3.

[Ben Pfaff]

Thanks, I'll apply this soon.

On Wed, Feb 19, 2014 at 10:33:01AM -0800, Joe Stringer wrote:
> Acked-by: Joe Stringer 
> 
> 
> On 19 February 2014 10:30, Ben Pfaff  wrote:
> 
> > OpenFlow 1.3.4 is going to revert the tag order change made by OpenFlow
> > 1.3.
> >
> > MPLS BoS matching is implemented.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> > v1->v2: Also remove MPLS BoS matching since that's been implemented.
> >
> >  OPENFLOW-1.1+ |8 
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> > index eaf2ee9..b58fec9 100644
> > --- a/OPENFLOW-1.1+
> > +++ b/OPENFLOW-1.1+
> > @@ -121,18 +121,10 @@ didn't compare the specs carefully yet.)
> >some kind of "hardware" support, if we judged it useful enough.)
> >[optional for OF1.3+]
> >
> > -* MPLS BoS matching.
> > -  Part of MPLS patchset by Simon Horman.
> > -  [optional for OF1.3+]
> > -
> >  * Provider Backbone Bridge tagging.  I don't plan to implement
> >this (but we'd accept an implementation).
> >[optional for OF1.3+]
> >
> > -* Rework tag order.
> > -  Part of MPLS patchset by Simon Horman.
> > -  [required for v1.3+]
> > -
> >  * On-demand flow counters.  I think this might be a real
> >optimization in some cases for the software switch.
> >[optional for OF1.3+]
> > --
> > 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] configure: Fix bug report email address.

[Joe Stringer]

Acked-by: Joe Stringer 


On 17 January 2014 17:50, Ben Pfaff  wrote:

> Reported-by: Arun Sharma 
> Signed-off-by: Ben Pfaff 
> ---
>  configure.ac |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 9b6c69e..2c04729 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@
>  # limitations under the License.
>
>  AC_PREREQ(2.64)
> -AC_INIT(openvswitch, 2.1.90, ovs-b...@openvswitch.org)
> +AC_INIT(openvswitch, 2.1.90, b...@openvswitch.org)
>  AC_CONFIG_SRCDIR([datapath/datapath.c])
>  AC_CONFIG_MACRO_DIR([m4])
>  AC_CONFIG_AUX_DIR([build-aux])
> --
> 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] ofproto: Send port status message for port-mods, right away.

[Ben Pfaff]

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

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

Signed-off-by: Ben Pfaff 
Reported-by: Kmindg G 
---
 AUTHORS   |1 +
 ofproto/ofproto.c |   25 +
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index c557303..2fda8d7 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -197,6 +197,7 @@ John Hurley john.hur...@netronome.com
 Kevin Mancuso   kevin.manc...@rackspace.com
 Kiran Shanbhog  ki...@vmware.com
 Kirill Kabardin
+Kmindg Gkmi...@gmail.com
 Koichi Yagishitayagishita.koi...@jrc.co.jp
 Konstantin Khorenko khore...@openvz.org
 Kris zhang  zhang.k...@gmail.com
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 62c97a2..48e10ca 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2986,22 +2986,23 @@ update_port_config(struct ofport *port,
enum ofputil_port_config config,
enum ofputil_port_config mask)
 {
-enum ofputil_port_config old_config = port->pp.config;
-enum ofputil_port_config toggle;
-
-toggle = (config ^ port->pp.config) & mask;
-if (toggle & OFPUTIL_PC_PORT_DOWN) {
-if (config & OFPUTIL_PC_PORT_DOWN) {
-netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL);
-} else {
-netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL);
-}
+enum ofputil_port_config toggle = (config ^ port->pp.config) & mask;
+
+if (toggle & OFPUTIL_PC_PORT_DOWN
+&& (config & OFPUTIL_PC_PORT_DOWN
+? netdev_turn_flags_off(port->netdev, NETDEV_UP, NULL)
+: netdev_turn_flags_on(port->netdev, NETDEV_UP, NULL))) {
+/* We tried to bring the port up or down, but it failed, so don't
+ * update the "down" bit. */
 toggle &= ~OFPUTIL_PC_PORT_DOWN;
 }
 
-port->pp.config ^= toggle;
-if (port->pp.config != old_config) {
+if (toggle) {
+enum ofputil_port_config old_config = port->pp.config;
+port->pp.config ^= toggle;
 port->ofproto->ofproto_class->port_reconfigured(port, old_config);
+connmgr_send_port_status(port->ofproto->connmgr, &port->pp,
+ OFPPR_MODIFY);
 }
 }
 
-- 
1.7.10.4

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

Re: [ovs-dev] [PATCH] ipfix: add SCTP, ICMP, and ICMPv6 flow reporting

[Ben Pfaff]

On Wed, Feb 12, 2014 at 07:06:12PM -0800, Romain Lenglet wrote:
> Signed-off-by: Romain Lenglet 

Seems fine, can I have a NEWS entry?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next] openvswitch: Use ether_addr_copy

[Pravin Shelar]

Jesse is on vacation. So I applied it to ovs master.

Thanks.

On Tue, Feb 18, 2014 at 3:15 PM, David Miller  wrote:
> From: Joe Perches 
> Date: Tue, 18 Feb 2014 11:15:45 -0800
>
>> It's slightly smaller/faster for some architectures.
>>
>> Signed-off-by: Joe Perches 
>
> I'll let Jesse take this via his tree, thanks Joe.
> ___
> 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] ofp-util: Fix a typo in ofputil_decode_ofp11_port

[Ben Pfaff]

On Wed, Feb 19, 2014 at 09:45:46PM +0800, kmindg wrote:
> Signed-off-by: kmindg 

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

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

[Ben Pfaff]

On Wed, Feb 19, 2014 at 11:34:25AM -0300, Rafael Duarte Vencioneck wrote:
> Digging a little in ovs kernel module code, I can't find a way to verify if
> a given port is up or down before execute an output. Is there a way to do
> that?

The kernel is pretty far down in the stack for OVS to be making
decisions.  What larger problem are you trying to solve?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Rafael Duarte Vencioneck]

I'm thinking about the possibility of fast forwarding without upcalls. If
the link is down, maybe the module could know someway for what port to
forward.
It's really impossible to do that?


Rafael Duarte Vencioneck


2014-02-19 16:39 GMT-03:00 Ben Pfaff :

> On Wed, Feb 19, 2014 at 11:34:25AM -0300, Rafael Duarte Vencioneck wrote:
> > Digging a little in ovs kernel module code, I can't find a way to verify
> if
> > a given port is up or down before execute an output. Is there a way to do
> > that?
>
> The kernel is pretty far down in the stack for OVS to be making
> decisions.  What larger problem are you trying to solve?
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Ben Pfaff]

On Wed, Feb 19, 2014 at 05:12:22PM -0300, Rafael Duarte Vencioneck wrote:
> I'm thinking about the possibility of fast forwarding without upcalls. If
> the link is down, maybe the module could know someway for what port to
> forward.
> It's really impossible to do that?

It might be possible.  With the "megaflows" feature introduced in
1.11, it's probably not worthwhile, because only a small number of
datapath flows have to be updated, most of the time, to redirect all
of the traffic to the new port.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 1/2] ovs-vswitchd: Get rid of call to process_init().

[Ben Pfaff]

On Fri, Feb 14, 2014 at 03:15:47PM -0800, Gurucharan Shetty wrote:
> It is not needed as we don't use any other process_* calls.
> 
> Signed-off-by: Gurucharan Shetty 

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

Re: [ovs-dev] [PATCH v2 2/2] process: Make changes for Windows.

[Ben Pfaff]

On Fri, Feb 14, 2014 at 03:15:48PM -0800, Gurucharan Shetty wrote:
> As of now, we are using the process subsystem in
> ovsdb-server to handle the "--run" command line
> option. That particular option is not used often
> and till deemed necessary, make it unsupported on
> Windows platform.
> 
> Signed-off-by: Gurucharan Shetty 

This makes process_status_msg() return a null pointer instead of a
string.  That could cause problems for the caller (e.g. segfault).  I
think it would be better to return a malloc()'d string for
consistency.  (Probably doesn't matter but just in case.)

process_wait() could be implemented as OVS_NOT_REACHED() since after
all it shouldn't be possible to get a valid "struct process *" on
Windows.

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

Re: [ovs-dev] [PATCH 2/2] tests/run-ryu: Fix setup for Ryu tests

[Ben Pfaff]

On Mon, Feb 17, 2014 at 05:14:17PM +0900, YAMAMOTO Takashi wrote:
> Tweak our configuration to match with Ryu tests' single-bridge assumption.
> 
> Signed-off-by: YAMAMOTO Takashi 

Both patches applied, thanks Yamamoto-san.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] ofp-actions: Correct pop MPLS ethtype as consistency test

[Ben Pfaff]

On Wed, Feb 19, 2014 at 09:27:16PM +0900, Simon Horman wrote:
> Correct pop MPLS ethtype consistency check to verify that
> the packet has an MPLS ethtype before the pop action rather than after:
> an MPLS ethtype is a pre-condition but not a post-condition of pop MPLS.
> 
> With this change the consistency check in ofpact_check__()
> becomes consistent with that in ofpact_from_nxast().
> 
> This was found using Ryu tests via the new make check-ryu target.
> It allows all of the "POP_MPLS"[1] tests to pass where they previously
> failed.
> 
> [1] http://osrg.github.io/ryu-certification/switch/ovs
> 
> Cc: Jarno Rajahalme 
> Signed-off-by: Simon Horman 

The check-ryu target has already paid off, then.

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

Re: [ovs-dev] [PATCH] ofproto: Remove 'force-miss-model' configuration.

[Ben Pfaff]

On Wed, Feb 19, 2014 at 10:26:31AM -0800, Joe Stringer wrote:
> This configuration item was introduced to assist testing of upcall
> handling behaviour with and without facets. Facets were removed in
> commit e79a6c833e0d7237, so this patch removes the configuration item.
> 
> Signed-off-by: Joe Stringer 

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

Re: [ovs-dev] [PATCHv3] upcall: Allow max_idle be configured through ovs-vsctl.

[Ben Pfaff]

On Tue, Feb 18, 2014 at 11:01:39AM -0800, Joe Stringer wrote:
> Signed-off-by: Joe Stringer 

This patch adds a new "max_idle" column to the Flow_Table table, and
documents it, but doesn't appear to use it anywhere in actual code.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] packet: Make set_ethertype() static

[Ben Pfaff]

On Wed, Feb 12, 2014 at 04:31:03PM +0900, Simon Horman wrote:
> Make set_ethertype() static as it is not used outside of packet.c
> 
> Signed-off-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] configure: Fix bug report email address.

[Ben Pfaff]

Applied, thanks!

On Wed, Feb 19, 2014 at 10:54:31AM -0800, Joe Stringer wrote:
> Acked-by: Joe Stringer 
> 
> 
> On 17 January 2014 17:50, Ben Pfaff  wrote:
> 
> > Reported-by: Arun Sharma 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  configure.ac |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 9b6c69e..2c04729 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -13,7 +13,7 @@
> >  # limitations under the License.
> >
> >  AC_PREREQ(2.64)
> > -AC_INIT(openvswitch, 2.1.90, ovs-b...@openvswitch.org)
> > +AC_INIT(openvswitch, 2.1.90, b...@openvswitch.org)
> >  AC_CONFIG_SRCDIR([datapath/datapath.c])
> >  AC_CONFIG_MACRO_DIR([m4])
> >  AC_CONFIG_AUX_DIR([build-aux])
> > --
> > 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] [PATCHv3] upcall: Allow max_idle be configured through ovs-vsctl.

[Joe Stringer]

Hmm. The way I follow it is that bridge_reconfigure() will read it from the
table, and call ofproto_set_max_idle() to set the ofproto_flow_idle
variable. Later, in revalidate_udumps(), we take a local copy of
ofproto_flow_idle, and this is used later in the function (by code that
already exists).

I've just tested it, and it works as expected - e.g., setting a value of 1
makes us delete flows from the datapath incredibly quickly, as observed via
ovs-appctl upcall/show. Is the "minInteger"/"maxInteger" meant to prevent
the user from setting values outside that range, or are they merely
suggestions for the manual?


On 19 February 2014 13:45, Ben Pfaff  wrote:

> On Tue, Feb 18, 2014 at 11:01:39AM -0800, Joe Stringer wrote:
> > Signed-off-by: Joe Stringer 
>
> This patch adds a new "max_idle" column to the Flow_Table table, and
> documents it, but doesn't appear to use it anywhere in actual code.
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Ben Pfaff]

On Tue, Jan 21, 2014 at 09:34:45AM -0800, Ben Pfaff wrote:
> Signed-off-by: Ben Pfaff 

Anyone want to review this?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 2/2] process: Make changes for Windows.

[Gurucharan Shetty]

On Wed, Feb 19, 2014 at 1:08 PM, Ben Pfaff  wrote:
> On Fri, Feb 14, 2014 at 03:15:48PM -0800, Gurucharan Shetty wrote:
>> As of now, we are using the process subsystem in
>> ovsdb-server to handle the "--run" command line
>> option. That particular option is not used often
>> and till deemed necessary, make it unsupported on
>> Windows platform.
>>
>> Signed-off-by: Gurucharan Shetty 
>
> This makes process_status_msg() return a null pointer instead of a
> string.  That could cause problems for the caller (e.g. segfault).  I
> think it would be better to return a malloc()'d string for
> consistency.  (Probably doesn't matter but just in case.)
Okay.
>
> process_wait() could be implemented as OVS_NOT_REACHED() since after
> all it shouldn't be possible to get a valid "struct process *" on
> Windows.
Okay.
>
> Acked-by: Ben Pfaff 

Thanks. I will add the following incremental and push it in a couple of hours.
diff --git a/lib/process.c b/lib/process.c
index fe93cd0..d0e1882 100644
--- a/lib/process.c
+++ b/lib/process.c
@@ -285,8 +285,8 @@ process_status(const struct process *p)
 char *
 process_status_msg(int status)
 {
-#ifndef _WIN32
 struct ds ds = DS_EMPTY_INITIALIZER;
+#ifndef _WIN32
 if (WIFEXITED(status)) {
 ds_put_format(&ds, "exit status %d", WEXITSTATUS(status));
 } else if (WIFSIGNALED(status)) {
@@ -305,10 +305,10 @@ process_status_msg(int status)
 if (WCOREDUMP(status)) {
 ds_put_cstr(&ds, ", core dumped");
 }
-return ds_cstr(&ds);
 #else
-return NULL;
+ds_put_cstr(&ds, "function not supported.");
 #endif
+return ds_cstr(&ds);
 }

 /* Executes periodic maintenance activities required by the process module. */
@@ -353,6 +353,8 @@ process_wait(struct process *p)
 } else {
 poll_fd_wait(fds[0], POLLIN);
 }
+#else
+OVS_NOT_REACHED();
 #endif
 }
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 2/2] packets: Always set ethertype in push_mpls()

[Ben Pfaff]

On Wed, Feb 12, 2014 at 10:13:02AM +0900, Simon Horman wrote:
> There are two different MPLS ethertypes, 0x8847 and 0x8848 and a push MPLS
> action applied to an MPLS packet may cause the ethertype to change from one
> to the other. To ensure that this happens update the ethertype in
> push_mpls() regardless of if the packet is already MPLS or not.
> 
> Test based on a similar test by Joe Stringer.
> 
> Cc: Joe Stringer 
> Signed-off-by: Simon Horman 

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

Re: [ovs-dev] [PATCH] ipfix: fix upcall cookie size checks to support 8 byte cookies

[Ben Pfaff]

On Tue, Feb 11, 2014 at 03:21:08PM -0800, Romain Lenglet wrote:
> Commit 96ed775f resizes all userspace metadata to be 8 bytes minimum.
> Fix the upcall size checks accordingly.
> 
> Signed-off-by: Romain Lenglet 

Thanks, applied to master, branch-2.1, branch-2.0, and I'm working on
branch-1.11.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v2 2/2] process: Make changes for Windows.

[Ben Pfaff]

On Wed, Feb 19, 2014 at 02:02:33PM -0800, Gurucharan Shetty wrote:
> On Wed, Feb 19, 2014 at 1:08 PM, Ben Pfaff  wrote:
> > On Fri, Feb 14, 2014 at 03:15:48PM -0800, Gurucharan Shetty wrote:
> >> As of now, we are using the process subsystem in
> >> ovsdb-server to handle the "--run" command line
> >> option. That particular option is not used often
> >> and till deemed necessary, make it unsupported on
> >> Windows platform.
> >>
> >> Signed-off-by: Gurucharan Shetty 
> >
> > This makes process_status_msg() return a null pointer instead of a
> > string.  That could cause problems for the caller (e.g. segfault).  I
> > think it would be better to return a malloc()'d string for
> > consistency.  (Probably doesn't matter but just in case.)
> Okay.
> >
> > process_wait() could be implemented as OVS_NOT_REACHED() since after
> > all it shouldn't be possible to get a valid "struct process *" on
> > Windows.
> Okay.
> >
> > Acked-by: Ben Pfaff 
> 
> Thanks. I will add the following incremental and push it in a couple
> of hours.

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

Re: [ovs-dev] [PATCHv3] upcall: Allow max_idle be configured through ovs-vsctl.

[Ben Pfaff]

bridge_reconfigure() reads other-config:max-idle from the Open_vSwitch
table.  That part is fine.  I'm complaining about max_idle in the
Flow_Table table.  Why do both exist?

On Wed, Feb 19, 2014 at 02:00:44PM -0800, Joe Stringer wrote:
> Hmm. The way I follow it is that bridge_reconfigure() will read it from the
> table, and call ofproto_set_max_idle() to set the ofproto_flow_idle
> variable. Later, in revalidate_udumps(), we take a local copy of
> ofproto_flow_idle, and this is used later in the function (by code that
> already exists).
> 
> I've just tested it, and it works as expected - e.g., setting a value of 1
> makes us delete flows from the datapath incredibly quickly, as observed via
> ovs-appctl upcall/show. Is the "minInteger"/"maxInteger" meant to prevent
> the user from setting values outside that range, or are they merely
> suggestions for the manual?
> 
> 
> On 19 February 2014 13:45, Ben Pfaff  wrote:
> 
> > On Tue, Feb 18, 2014 at 11:01:39AM -0800, Joe Stringer wrote:
> > > Signed-off-by: Joe Stringer 
> >
> > This patch adds a new "max_idle" column to the Flow_Table table, and
> > documents it, but doesn't appear to use it anywhere in actual code.
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] ipfix: add SCTP, ICMP, and ICMPv6 flow reporting

[Romain Lenglet]

Signed-off-by: Romain Lenglet 
---
 NEWS |1 +
 ofproto/ofproto-dpif-ipfix.c |  120 ++
 2 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/NEWS b/NEWS
index 34dde6b..7c6ae70 100644
--- a/NEWS
+++ b/NEWS
@@ -67,6 +67,7 @@ v2.1.0 - xx xxx 
  flows.  OVS automatically adjusts this number depending on network
  conditions.
- Added IPv6 support for active and passive socket communications.
+   - Added IPFIX support for SCTP flows and templates for ICMPv4/v6 flows.
 
 
 v2.0.0 - 15 Oct 2013
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 2500efd..a529884 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -117,7 +117,8 @@ enum ipfix_proto_l3 {
 };
 enum ipfix_proto_l4 {
 IPFIX_PROTO_L4_UNKNOWN = 0,
-IPFIX_PROTO_L4_TCP_UDP,
+IPFIX_PROTO_L4_TCP_UDP_SCTP,
+IPFIX_PROTO_L4_ICMP,
 NUM_IPFIX_PROTO_L4
 };
 
@@ -200,13 +201,21 @@ struct ipfix_data_record_flow_key_ipv6 {
 });
 BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_ipv6) == 36);
 
-/* Part of data record flow key for TCP/UDP entities. */
+/* Part of data record flow key for TCP/UDP/SCTP entities. */
 OVS_PACKED(
-struct ipfix_data_record_flow_key_tcpudp {
+struct ipfix_data_record_flow_key_transport {
 ovs_be16 source_transport_port;  /* SOURCE_TRANSPORT_PORT */
 ovs_be16 destination_transport_port;  /* DESTINATION_TRANSPORT_PORT */
 });
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_tcpudp) == 4);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_transport) == 4);
+
+/* Part of data record flow key for ICMP entities. */
+OVS_PACKED(
+struct ipfix_data_record_flow_key_icmp {
+uint8_t icmp_type;  /* ICMP_TYPE_IPV4 / ICMP_TYPE_IPV6 */
+uint8_t icmp_code;  /* ICMP_CODE_IPV4 / ICMP_CODE_IPV6 */
+});
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_icmp) == 2);
 
 /* Cf. IETF RFC 5102 Section 5.11.3. */
 enum ipfix_flow_end_reason {
@@ -231,18 +240,21 @@ BUILD_ASSERT_DECL(sizeof(struct 
ipfix_data_record_aggregated_common) == 25);
 /* Part of data record for IP aggregated elements. */
 OVS_PACKED(
 struct ipfix_data_record_aggregated_ip {
+ovs_be64 octet_delta_count;  /* OCTET_DELTA_COUNT */
 ovs_be64 octet_delta_sum_of_squares;  /* OCTET_DELTA_SUM_OF_SQUARES */
 ovs_be64 minimum_ip_total_length;  /* MINIMUM_IP_TOTAL_LENGTH */
 ovs_be64 maximum_ip_total_length;  /* MAXIMUM_IP_TOTAL_LENGTH */
 });
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 24);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_aggregated_ip) == 32);
 
-#define MAX_FLOW_KEY_LEN \
-(sizeof(struct ipfix_data_record_flow_key_common)\
- + sizeof(struct ipfix_data_record_flow_key_vlan)\
- + sizeof(struct ipfix_data_record_flow_key_ip)  \
- + sizeof(struct ipfix_data_record_flow_key_ipv6)\
- + sizeof(struct ipfix_data_record_flow_key_tcpudp))
+#define MAX_FLOW_KEY_LEN\
+(sizeof(struct ipfix_data_record_flow_key_common)   \
+ + sizeof(struct ipfix_data_record_flow_key_vlan)   \
+ + sizeof(struct ipfix_data_record_flow_key_ip) \
+ + MAX(sizeof(struct ipfix_data_record_flow_key_ipv4),  \
+   sizeof(struct ipfix_data_record_flow_key_ipv6))  \
+ + MAX(sizeof(struct ipfix_data_record_flow_key_icmp),  \
+   sizeof(struct ipfix_data_record_flow_key_transport)))
 
 #define MAX_DATA_RECORD_LEN \
 (MAX_FLOW_KEY_LEN   \
@@ -280,6 +292,7 @@ struct ipfix_flow_cache_entry {
 uint64_t flow_end_timestamp_usec;
 uint64_t packet_delta_count;
 uint64_t layer2_octet_delta_count;
+uint64_t octet_delta_count;
 uint64_t octet_delta_sum_of_squares;  /* 0 if not IP. */
 uint16_t minimum_ip_total_length;  /* 0 if not IP. */
 uint16_t maximum_ip_total_length;  /* 0 if not IP. */
@@ -781,18 +794,27 @@ ipfix_define_template_fields(enum ipfix_proto_l2 l2, enum 
ipfix_proto_l3 l3,
 if (l3 == IPFIX_PROTO_L3_IPV4) {
 DEF(SOURCE_IPV4_ADDRESS);
 DEF(DESTINATION_IPV4_ADDRESS);
+if (l4 == IPFIX_PROTO_L4_TCP_UDP_SCTP) {
+DEF(SOURCE_TRANSPORT_PORT);
+DEF(DESTINATION_TRANSPORT_PORT);
+} else if (l4 == IPFIX_PROTO_L4_ICMP) {
+DEF(ICMP_TYPE_IPV4);
+DEF(ICMP_CODE_IPV4);
+}
 } else {  /* l3 == IPFIX_PROTO_L3_IPV6 */
 DEF(SOURCE_IPV6_ADDRESS);
 DEF(DESTINATION_IPV6_ADDRESS);
 DEF(FLOW_LABEL_IPV6);
+if (l4 == IPFIX_PROTO_L4_TCP_UDP_SCTP) {
+DEF(SOURCE_TRANSPORT_PORT);
+DEF(DESTINATION_TRANSPORT_PORT);
+} else if (l4 == IPFIX_PROTO_L4_ICMP) {
+DEF(ICMP_TYPE_IP

Re: [ovs-dev] [PATCH] ipfix: add SCTP, ICMP, and ICMPv6 flow reporting

[Ben Pfaff]

On Wed, Feb 19, 2014 at 02:23:39PM -0800, Romain Lenglet wrote:
> Signed-off-by: Romain Lenglet 

Thanks, applied.

I moved the NEWS item from v2.1.0 to post-v2.1.0, because it is too
late (we branched for 2.1 a long time ago).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH] datapath: Fix build failure.

[Pravin]

Defines RCU_INIT_POINTER() for compatibility.

Signed-off-by: Pravin B Shelar 
---
 datapath/linux/compat/include/linux/rcupdate.h |   12 
 1 file changed, 12 insertions(+)

diff --git a/datapath/linux/compat/include/linux/rcupdate.h 
b/datapath/linux/compat/include/linux/rcupdate.h
index 20cdedf..a497f7b 100644
--- a/datapath/linux/compat/include/linux/rcupdate.h
+++ b/datapath/linux/compat/include/linux/rcupdate.h
@@ -22,4 +22,16 @@ static inline int rcu_read_lock_held(void)
 }
 #endif
 
+#ifndef RCU_INITIALIZER
+#define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
+#endif
+
+#ifndef RCU_INIT_POINTER
+#define RCU_INIT_POINTER(p, v) \
+do { \
+p = RCU_INITIALIZER(v); \
+} while (0)
+
+#endif
+
 #endif /* linux/rcupdate.h wrapper */
-- 
1.7.9.5

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

Re: [ovs-dev] [PATCHv3] upcall: Allow max_idle be configured through ovs-vsctl.

[Joe Stringer]

Ah, I see now. That would be my misunderstanding, from following the
"flow-limit" and "flow_limit" configuration options and equating them with
each other. I plan to send a fresh version.


On 19 February 2014 14:20, Ben Pfaff  wrote:

> bridge_reconfigure() reads other-config:max-idle from the Open_vSwitch
> table.  That part is fine.  I'm complaining about max_idle in the
> Flow_Table table.  Why do both exist?
>
> On Wed, Feb 19, 2014 at 02:00:44PM -0800, Joe Stringer wrote:
> > Hmm. The way I follow it is that bridge_reconfigure() will read it from
> the
> > table, and call ofproto_set_max_idle() to set the ofproto_flow_idle
> > variable. Later, in revalidate_udumps(), we take a local copy of
> > ofproto_flow_idle, and this is used later in the function (by code that
> > already exists).
> >
> > I've just tested it, and it works as expected - e.g., setting a value of
> 1
> > makes us delete flows from the datapath incredibly quickly, as observed
> via
> > ovs-appctl upcall/show. Is the "minInteger"/"maxInteger" meant to prevent
> > the user from setting values outside that range, or are they merely
> > suggestions for the manual?
> >
> >
> > On 19 February 2014 13:45, Ben Pfaff  wrote:
> >
> > > On Tue, Feb 18, 2014 at 11:01:39AM -0800, Joe Stringer wrote:
> > > > Signed-off-by: Joe Stringer 
> > >
> > > This patch adds a new "max_idle" column to the Flow_Table table, and
> > > documents it, but doesn't appear to use it anywhere in actual code.
> > >
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [rwlock 2/5] netdev: Change netdev_class_rwlock to recursive mutex, for POSIX safety.

[Joe Stringer]

Acked-by: Joe Stringer 


On 16 January 2014 10:07, Ben Pfaff  wrote:

> With glibc, rwlocks by default allow recursive read-locking even if a
> thread is blocked waiting for the write-lock.  POSIX allows such attempts
> to deadlock, and it appears that the libc used in NetBSD, at least, does
> deadlock.  The netdev_class_rwlock is in fact acquired recursively in this
> way, which is a bug.  This commit fixes the problem by switching to a
> recursive mutex.  This allows for less parallelism, but according to an
> existing comment that doesn't matter here anyway.
>
> Signed-off-by: Ben Pfaff 
> ---
>  lib/netdev.c |   63
> +-
>  1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 575227c..8fd63de 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -67,15 +67,14 @@ static struct shash netdev_shash
> OVS_GUARDED_BY(netdev_mutex)
>
>  /* Protects 'netdev_classes' against insertions or deletions.
>   *
> - * This is not an rwlock for performance reasons but to allow recursive
> - * acquisition when calling into providers.  For example, netdev_run()
> calls
> - * into provider 'run' functions, which might reasonably want to call one
> of
> - * the netdev functions that takes netdev_class_rwlock read-only. */
> -static struct ovs_rwlock netdev_class_rwlock OVS_ACQ_BEFORE(netdev_mutex)
> -= OVS_RWLOCK_INITIALIZER;
> + * This is a recursive mutex to allow recursive acquisition when calling
> into
> + * providers.  For example, netdev_run() calls into provider 'run'
> functions,
> + * which might reasonably want to call one of the netdev functions that
> takes
> + * netdev_class_mutex. */
> +static struct ovs_mutex netdev_class_mutex OVS_ACQ_BEFORE(netdev_mutex);
>
>  /* Contains 'struct netdev_registered_class'es. */
> -static struct hmap netdev_classes OVS_GUARDED_BY(netdev_class_rwlock)
> +static struct hmap netdev_classes OVS_GUARDED_BY(netdev_class_mutex)
>  = HMAP_INITIALIZER(&netdev_classes);
>
>  struct netdev_registered_class {
> @@ -93,11 +92,13 @@ void update_device_args(struct netdev *, const struct
> shash *args);
>
>  static void
>  netdev_initialize(void)
> -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
>  {
>  static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>
>  if (ovsthread_once_start(&once)) {
> +ovs_mutex_init_recursive(&netdev_class_mutex);
> +
>  fatal_signal_add_hook(restore_all_flags, NULL, NULL, true);
>  netdev_vport_patch_register();
>
> @@ -122,17 +123,17 @@ netdev_initialize(void)
>   * main poll loop. */
>  void
>  netdev_run(void)
> -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
>  {
>  struct netdev_registered_class *rc;
>
> -ovs_rwlock_rdlock(&netdev_class_rwlock);
> +ovs_mutex_lock(&netdev_class_mutex);
>  HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
>  if (rc->class->run) {
>  rc->class->run();
>  }
>  }
> -ovs_rwlock_unlock(&netdev_class_rwlock);
> +ovs_mutex_unlock(&netdev_class_mutex);
>  }
>
>  /* Arranges for poll_block() to wake up when netdev_run() needs to be
> called.
> @@ -141,22 +142,22 @@ netdev_run(void)
>   * main poll loop. */
>  void
>  netdev_wait(void)
> -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
>  {
>  struct netdev_registered_class *rc;
>
> -ovs_rwlock_rdlock(&netdev_class_rwlock);
> +ovs_mutex_lock(&netdev_class_mutex);
>  HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
>  if (rc->class->wait) {
>  rc->class->wait();
>  }
>  }
> -ovs_rwlock_unlock(&netdev_class_rwlock);
> +ovs_mutex_unlock(&netdev_class_mutex);
>  }
>
>  static struct netdev_registered_class *
>  netdev_lookup_class(const char *type)
> -OVS_REQ_RDLOCK(netdev_class_rwlock)
> +OVS_REQ_RDLOCK(netdev_class_mutex)
>  {
>  struct netdev_registered_class *rc;
>
> @@ -173,11 +174,11 @@ netdev_lookup_class(const char *type)
>   * registration, new netdevs of that type can be opened using
> netdev_open(). */
>  int
>  netdev_register_provider(const struct netdev_class *new_class)
> -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
>  {
>  int error;
>
> -ovs_rwlock_wrlock(&netdev_class_rwlock);
> +ovs_mutex_lock(&netdev_class_mutex);
>  if (netdev_lookup_class(new_class->type)) {
>  VLOG_WARN("attempted to register duplicate netdev provider: %s",
>

Re: [ovs-dev] [PATCH] datapath: Fix build failure.

[Jarno Rajahalme]

Thanks Pravin,

Acked-by: Jarno Rajahalme 

On Feb 19, 2014, at 2:29 PM, Pravin  wrote:

> Defines RCU_INIT_POINTER() for compatibility.
> 
> Signed-off-by: Pravin B Shelar 
> ---
> datapath/linux/compat/include/linux/rcupdate.h |   12 
> 1 file changed, 12 insertions(+)
> 
> diff --git a/datapath/linux/compat/include/linux/rcupdate.h 
> b/datapath/linux/compat/include/linux/rcupdate.h
> index 20cdedf..a497f7b 100644
> --- a/datapath/linux/compat/include/linux/rcupdate.h
> +++ b/datapath/linux/compat/include/linux/rcupdate.h
> @@ -22,4 +22,16 @@ static inline int rcu_read_lock_held(void)
> }
> #endif
> 
> +#ifndef RCU_INITIALIZER
> +#define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
> +#endif
> +
> +#ifndef RCU_INIT_POINTER
> +#define RCU_INIT_POINTER(p, v) \
> +do { \
> +p = RCU_INITIALIZER(v); \
> +} while (0)
> +
> +#endif
> +
> #endif /* linux/rcupdate.h wrapper */
> -- 
> 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] [PATCH] datapath: Fix build failure.

[Pravin Shelar]

Thanks.
Pushed to master.

On Wed, Feb 19, 2014 at 2:44 PM, Jarno Rajahalme  wrote:
> Thanks Pravin,
>
> Acked-by: Jarno Rajahalme 
>
> On Feb 19, 2014, at 2:29 PM, Pravin  wrote:
>
>> Defines RCU_INIT_POINTER() for compatibility.
>>
>> Signed-off-by: Pravin B Shelar 
>> ---
>> datapath/linux/compat/include/linux/rcupdate.h |   12 
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/datapath/linux/compat/include/linux/rcupdate.h 
>> b/datapath/linux/compat/include/linux/rcupdate.h
>> index 20cdedf..a497f7b 100644
>> --- a/datapath/linux/compat/include/linux/rcupdate.h
>> +++ b/datapath/linux/compat/include/linux/rcupdate.h
>> @@ -22,4 +22,16 @@ static inline int rcu_read_lock_held(void)
>> }
>> #endif
>>
>> +#ifndef RCU_INITIALIZER
>> +#define RCU_INITIALIZER(v) (typeof(*(v)) __force __rcu *)(v)
>> +#endif
>> +
>> +#ifndef RCU_INIT_POINTER
>> +#define RCU_INIT_POINTER(p, v) \
>> +do { \
>> +p = RCU_INITIALIZER(v); \
>> +} while (0)
>> +
>> +#endif
>> +
>> #endif /* linux/rcupdate.h wrapper */
>> --
>> 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] [PATCHv3] upcall: Allow max_idle be configured through ovs-vsctl.

[Ben Pfaff]

Thanks.

On Wed, Feb 19, 2014 at 02:34:40PM -0800, Joe Stringer wrote:
> Ah, I see now. That would be my misunderstanding, from following the
> "flow-limit" and "flow_limit" configuration options and equating them with
> each other. I plan to send a fresh version.
> 
> 
> On 19 February 2014 14:20, Ben Pfaff  wrote:
> 
> > bridge_reconfigure() reads other-config:max-idle from the Open_vSwitch
> > table.  That part is fine.  I'm complaining about max_idle in the
> > Flow_Table table.  Why do both exist?
> >
> > On Wed, Feb 19, 2014 at 02:00:44PM -0800, Joe Stringer wrote:
> > > Hmm. The way I follow it is that bridge_reconfigure() will read it from
> > the
> > > table, and call ofproto_set_max_idle() to set the ofproto_flow_idle
> > > variable. Later, in revalidate_udumps(), we take a local copy of
> > > ofproto_flow_idle, and this is used later in the function (by code that
> > > already exists).
> > >
> > > I've just tested it, and it works as expected - e.g., setting a value of
> > 1
> > > makes us delete flows from the datapath incredibly quickly, as observed
> > via
> > > ovs-appctl upcall/show. Is the "minInteger"/"maxInteger" meant to prevent
> > > the user from setting values outside that range, or are they merely
> > > suggestions for the manual?
> > >
> > >
> > > On 19 February 2014 13:45, Ben Pfaff  wrote:
> > >
> > > > On Tue, Feb 18, 2014 at 11:01:39AM -0800, Joe Stringer wrote:
> > > > > Signed-off-by: Joe Stringer 
> > > >
> > > > This patch adds a new "max_idle" column to the Flow_Table table, and
> > > > documents it, but doesn't appear to use it anywhere in actual code.
> > > >
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH] FAQ: Mention hairpinning under actions=in_port.

[Ben Pfaff]

On Wed, Feb 12, 2014 at 11:20:29AM -0800, Joe Stringer wrote:
> Signed-off-by: Joe Stringer 
> ---
> I've heard term in the context of using a switch to "bless" traffic.
> Adding this wording makes it easier to search for the relevant FAQ
> entry.

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

Re: [ovs-dev] [PATCH] ofproto-provider: Update documentation.

[Ben Pfaff]

On Tue, Feb 18, 2014 at 09:39:12AM -0800, Joe Stringer wrote:
> This wording was in ofproto.c, but missing from ofproto-provider.h.
> 
> Signed-off-by: Joe Stringer 

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

Re: [ovs-dev] [rwlock 3/5] ofproto-dpif-xlate: Avoid recursively taking read side of ofgroup rwlock.

[Joe Stringer]

This looks good to me, although aren't we meant to report something at the
OpenFlow layer?

Is this equivalent to the OpenFlow "chaining" description? So if we don't
support chaining groups together, we should return an error message with
reason OFPGMFC_CHAINING_UNSUPPORTED, rather than accepting the rules then
misbehaving?


On 16 January 2014 10:07, Ben Pfaff  wrote:

> With glibc, rwlocks by default allow recursive read-locking even if a
> thread is blocked waiting for the write-lock.  POSIX allows such attempts
> to deadlock, and it appears that the libc used in NetBSD, at least, does
> deadlock.  ofproto-dpif-xlate could take the ofgroup rwlock recursively if
> an ofgroup's actions caused the ofgroup to be executed again.  This commit
> avoids that issue by preventing recursive translation of groups (the same
> group or another group).  This is not the most user friendly solution,
> but OpenFlow allows this restriction, and we can always remove the
> restriction later (probably requiring more complicated code) if it
> proves to be a real problem to real users.
>
> Found by inspection.
>
> Signed-off-by: Ben Pfaff 
> ---
>  ofproto/ofproto-dpif-xlate.c |   32 +++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 4747ea7..e0b1bc6 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -180,6 +180,7 @@ struct xlate_ctx {
>  /* Resubmit statistics, via xlate_table_action(). */
>  int recurse;/* Current resubmit nesting depth. */
>  int resubmits;  /* Total number of resubmits. */
> +bool in_group;  /* Currently translating ofgroup, if
> true. */
>
>  uint32_t orig_skb_priority; /* Priority when packet arrived. */
>  uint8_t table_id;   /* OpenFlow table ID where flow was
> found. */
> @@ -1992,6 +1993,8 @@ xlate_select_group(struct xlate_ctx *ctx, struct
> group_dpif *group)
>  static void
>  xlate_group_action__(struct xlate_ctx *ctx, struct group_dpif *group)
>  {
> +ctx->in_group = true;
> +
>  switch (group_dpif_get_type(group)) {
>  case OFPGT11_ALL:
>  case OFPGT11_INDIRECT:
> @@ -2007,12 +2010,38 @@ xlate_group_action__(struct xlate_ctx *ctx, struct
> group_dpif *group)
>  OVS_NOT_REACHED();
>  }
>  group_dpif_release(group);
> +
> +ctx->in_group = false;
> +}
> +
> +static bool
> +xlate_group_resource_check(struct xlate_ctx *ctx)
> +{
> +if (!xlate_resubmit_resource_check(ctx)) {
> +return false;
> +} else if (ctx->in_group) {
> +/* Prevent nested translation of OpenFlow groups.
> + *
> + * OpenFlow allows this restriction.  We enforce this restriction
> only
> + * because, with the current architecture, we would otherwise
> have to
> + * take a possibly recursive read lock on the ofgroup rwlock,
> which is
> + * unsafe given that POSIX allows taking a read lock to block if
> there
> + * is a thread blocked on taking the write lock.  Other solutions
> + * without this restriction are also possible, but seem
> unwarranted
> + * given the current limited use of groups. */
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
> +
> +VLOG_ERR_RL(&rl, "cannot recursively translate OpenFlow group");
> +return false;
> +} else {
> +return true;
> +}
>  }
>
>  static bool
>  xlate_group_action(struct xlate_ctx *ctx, uint32_t group_id)
>  {
> -if (xlate_resubmit_resource_check(ctx)) {
> +if (xlate_group_resource_check(ctx)) {
>  struct group_dpif *group;
>  bool got_group;
>
> @@ -3088,6 +3117,7 @@ xlate_actions__(struct xlate_in *xin, struct
> xlate_out *xout)
>
>  ctx.recurse = 0;
>  ctx.resubmits = 0;
> +ctx.in_group = false;
>  ctx.orig_skb_priority = flow->skb_priority;
>  ctx.table_id = 0;
>  ctx.exit = false;
> --
> 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] ovs-vsctl: reconnect to the database if connection was dropped

[Ansis Atteka]

Thanks for review. I pushed this to branches where issue was observed
- master and 2.1.

On Wed, Feb 19, 2014 at 10:22 AM, Ben Pfaff  wrote:
> On Tue, Feb 18, 2014 at 02:34:58PM -0800, Ansis Atteka wrote:
>> If ovs-vsctl has to wait for ovs-vswitchd to reconfigure itself
>> according to the new database, then sometimes ovs-vsctl could
>> end up stuck in the event loop if OVSDB connection was dropped
>> while ovs-vsctl was still running.
>>
>> This patch fixes this problem by letting ovs-vsctl to reconnect
>> to the OVSDB, if it has to wait cur_cfg field to be updated.
>>
>> Issue: 1191997
>> Reported-by: Spiro Kourtessis 
>> Signed-Off-By: Ansis Atteka 
>
> Acked-by: Ben Pfaff 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [rwlock 2/5] netdev: Change netdev_class_rwlock to recursive mutex, for POSIX safety.

[Ben Pfaff]

Applied, thanks.

On Wed, Feb 19, 2014 at 02:39:06PM -0800, Joe Stringer wrote:
> Acked-by: Joe Stringer 
> 
> 
> On 16 January 2014 10:07, Ben Pfaff  wrote:
> 
> > With glibc, rwlocks by default allow recursive read-locking even if a
> > thread is blocked waiting for the write-lock.  POSIX allows such attempts
> > to deadlock, and it appears that the libc used in NetBSD, at least, does
> > deadlock.  The netdev_class_rwlock is in fact acquired recursively in this
> > way, which is a bug.  This commit fixes the problem by switching to a
> > recursive mutex.  This allows for less parallelism, but according to an
> > existing comment that doesn't matter here anyway.
> >
> > Signed-off-by: Ben Pfaff 
> > ---
> >  lib/netdev.c |   63
> > +-
> >  1 file changed, 32 insertions(+), 31 deletions(-)
> >
> > diff --git a/lib/netdev.c b/lib/netdev.c
> > index 575227c..8fd63de 100644
> > --- a/lib/netdev.c
> > +++ b/lib/netdev.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -67,15 +67,14 @@ static struct shash netdev_shash
> > OVS_GUARDED_BY(netdev_mutex)
> >
> >  /* Protects 'netdev_classes' against insertions or deletions.
> >   *
> > - * This is not an rwlock for performance reasons but to allow recursive
> > - * acquisition when calling into providers.  For example, netdev_run()
> > calls
> > - * into provider 'run' functions, which might reasonably want to call one
> > of
> > - * the netdev functions that takes netdev_class_rwlock read-only. */
> > -static struct ovs_rwlock netdev_class_rwlock OVS_ACQ_BEFORE(netdev_mutex)
> > -= OVS_RWLOCK_INITIALIZER;
> > + * This is a recursive mutex to allow recursive acquisition when calling
> > into
> > + * providers.  For example, netdev_run() calls into provider 'run'
> > functions,
> > + * which might reasonably want to call one of the netdev functions that
> > takes
> > + * netdev_class_mutex. */
> > +static struct ovs_mutex netdev_class_mutex OVS_ACQ_BEFORE(netdev_mutex);
> >
> >  /* Contains 'struct netdev_registered_class'es. */
> > -static struct hmap netdev_classes OVS_GUARDED_BY(netdev_class_rwlock)
> > +static struct hmap netdev_classes OVS_GUARDED_BY(netdev_class_mutex)
> >  = HMAP_INITIALIZER(&netdev_classes);
> >
> >  struct netdev_registered_class {
> > @@ -93,11 +92,13 @@ void update_device_args(struct netdev *, const struct
> > shash *args);
> >
> >  static void
> >  netdev_initialize(void)
> > -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> > +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
> >  {
> >  static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> >
> >  if (ovsthread_once_start(&once)) {
> > +ovs_mutex_init_recursive(&netdev_class_mutex);
> > +
> >  fatal_signal_add_hook(restore_all_flags, NULL, NULL, true);
> >  netdev_vport_patch_register();
> >
> > @@ -122,17 +123,17 @@ netdev_initialize(void)
> >   * main poll loop. */
> >  void
> >  netdev_run(void)
> > -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> > +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
> >  {
> >  struct netdev_registered_class *rc;
> >
> > -ovs_rwlock_rdlock(&netdev_class_rwlock);
> > +ovs_mutex_lock(&netdev_class_mutex);
> >  HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
> >  if (rc->class->run) {
> >  rc->class->run();
> >  }
> >  }
> > -ovs_rwlock_unlock(&netdev_class_rwlock);
> > +ovs_mutex_unlock(&netdev_class_mutex);
> >  }
> >
> >  /* Arranges for poll_block() to wake up when netdev_run() needs to be
> > called.
> > @@ -141,22 +142,22 @@ netdev_run(void)
> >   * main poll loop. */
> >  void
> >  netdev_wait(void)
> > -OVS_EXCLUDED(netdev_class_rwlock, netdev_mutex)
> > +OVS_EXCLUDED(netdev_class_mutex, netdev_mutex)
> >  {
> >  struct netdev_registered_class *rc;
> >
> > -ovs_rwlock_rdlock(&netdev_class_rwlock);
> > +ovs_mutex_lock(&netdev_class_mutex);
> >  HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
> >  if (rc->class->wait) {
> >  rc->class->wait();
> >  }
> >  }
> > -ovs_rwlock_unlock(&netdev_class_rwlock);
> > +ovs_mutex_unlock(&netdev_class_mutex);
> >  }
> >
> >  static struct netdev_registered_class *
> >  netdev_lookup_class(const char *type)
> > -OVS_REQ_RDLOCK(netdev_class_rwlock)
> > +OVS_REQ_RDLOCK(netdev_class_mutex)
> >  {
> >  struct netdev_registered_class *rc;
> >
> > @@ -173,11 +174,11 @@ netdev_lookup_class(const char *type)
> >   * registration, new netdevs of that type can be opened using
> > netdev_open(). */
> >  int
> >  netdev_register_provider(const struct netdev_class *new_class)
> > -OVS_EXCLU

Re: [ovs-dev] [rwlock 4/5] ovs-thread: Get rid of obsolete sparse wrappers.

[Joe Stringer]

Acked-by: Joe Stringer 


On 16 January 2014 10:07, Ben Pfaff  wrote:

> These were useful back when we were trying to use the sparse lock balance
> annotations, but we removed those in commit 47b52c71232c0 (sparse: Remove
> support for thread-safety annotations.) and so they serve no purpose any
> longer.
>
> Signed-off-by: Ben Pfaff 
> ---
>  lib/ovs-thread.h |   11 ---
>  1 file changed, 11 deletions(-)
>
> diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
> index 8cf2ecc..f27553e 100644
> --- a/lib/ovs-thread.h
> +++ b/lib/ovs-thread.h
> @@ -115,17 +115,6 @@ void xpthread_cond_destroy(pthread_cond_t *);
>  void xpthread_cond_signal(pthread_cond_t *);
>  void xpthread_cond_broadcast(pthread_cond_t *);
>
> -#ifdef __CHECKER__
> -/* Replace these functions by the macros already defined in the
> 
> - * annotations, because the macro definitions have correct semantics for
> the
> - * conditional acquisition that can't be captured in a function
> annotation.
> - * The difference in semantics from pthread_*() to xpthread_*() does not
> matter
> - * because sparse is not a compiler. */
> -#define xpthread_mutex_trylock pthread_mutex_trylock
> -#define xpthread_rwlock_tryrdlock pthread_rwlock_tryrdlock
> -#define xpthread_rwlock_trywrlock pthread_rwlock_trywrlock
> -#endif
> -
>  void xpthread_key_create(pthread_key_t *, void (*destructor)(void *));
>  void xpthread_key_delete(pthread_key_t);
>  void xpthread_setspecific(pthread_key_t, const void *);
> --
> 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] [rwlock 5/5] ovs-thread: Use fair (but nonrecursive) rwlocks on glibc.

[Joe Stringer]

On 16 January 2014 10:07, Ben Pfaff  wrote:
>
> + * An ovs_rwlock does not support recursive readers, because POSIX allows
> + * taking the reader lock recursively to deadlock when a thread is
> waiting on
> + * the write-lock.  (NetBSD does deadlock.)  glibc does not deadlock in
> this
> + * situation by default, but ovs_rwlock_init() initializes rwlocks to do
> so
> + * anyway for two reasons:


The last sentence seems a bit odd. Perhaps something closer to "glibc does
not deadlock in this situation by default, but ovs_rwlock_init()
initializes rwlocks as non-recursive anyway for two reasons:"?

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

[ovs-dev] [PATCHv4] upcall: Configure datapath max-idle through ovs-vsctl.

[Joe Stringer]

This patch adds a new configuration option, "max-idle" to the
Open_vSwitch "other-config" column. This sets how long datapath flows
are cached in the datapath before revalidators expire them.

Signed-off-by: Joe Stringer 

---
v4: Remove extra "max_idle" from Flow_Table table.
v3: Use 'unsigned' instead of 'long long int' for ofproto_max_idle.
v2: Don't cache the max_idle in 'struct udpif'.
Extend range of valid values to 100-3.
---
 ofproto/ofproto-dpif-upcall.c |5 ++---
 ofproto/ofproto-provider.h|4 
 ofproto/ofproto.c |9 +
 ofproto/ofproto.h |2 ++
 vswitchd/bridge.c |2 ++
 vswitchd/vswitch.xml  |   15 +++
 6 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index e0a5aed..14afb0c 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -41,7 +41,6 @@
 #define MAX_QUEUE_LENGTH 512
 #define FLOW_MISS_MAX_BATCH 50
 #define REVALIDATE_MAX_BATCH 50
-#define MAX_IDLE 1500
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
 
@@ -622,7 +621,7 @@ udpif_flow_dumper(void *arg)
   duration);
 }
 
-poll_timer_wait_until(start_time + MIN(MAX_IDLE, 500));
+poll_timer_wait_until(start_time + MIN(ofproto_max_idle, 500));
 seq_wait(udpif->reval_seq, udpif->last_reval_seq);
 latch_wait(&udpif->exit_latch);
 poll_block();
@@ -1383,7 +1382,7 @@ revalidate_udumps(struct revalidator *revalidator, struct 
list *udumps)
 n_flows = udpif_get_n_flows(udpif);
 
 must_del = false;
-max_idle = MAX_IDLE;
+max_idle = ofproto_max_idle;
 if (n_flows > flow_limit) {
 must_del = n_flows > 2 * flow_limit;
 max_idle = 100;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 08a8ac1..b1c78c1 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -464,6 +464,10 @@ void rule_collection_destroy(struct rule_collection *);
  * ofproto-dpif implementation. */
 extern unsigned ofproto_flow_limit;
 
+/* Determines how long flows may be idle in the datapath before they are
+ * expired. */
+extern unsigned ofproto_max_idle;
+
 /* Number of upcall handler and revalidator threads. Only affects the
  * ofproto-dpif implementation. */
 extern size_t n_handlers, n_revalidators;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 62c97a2..7ea00da 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -307,6 +307,7 @@ static size_t allocated_ofproto_classes;
 struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
 
 unsigned ofproto_flow_limit = OFPROTO_FLOW_LIMIT_DEFAULT;
+unsigned ofproto_max_idle = OFPROTO_MAX_IDLE_DEFAULT;
 enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
 
 size_t n_handlers, n_revalidators;
@@ -698,6 +699,14 @@ ofproto_set_flow_limit(unsigned limit)
 ofproto_flow_limit = limit;
 }
 
+/* Sets the maximum idle time for flows in the datapath before they are
+ * expired. */
+void
+ofproto_set_max_idle(long long int max_idle)
+{
+ofproto_max_idle = max_idle;
+}
+
 /* Sets the path for handling flow misses. */
 void
 ofproto_set_flow_miss_model(unsigned model)
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 0ac4454..a5d7824 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -214,6 +214,7 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
 )
 
 #define OFPROTO_FLOW_LIMIT_DEFAULT 20
+#define OFPROTO_MAX_IDLE_DEFAULT 1200
 
 /* How flow misses should be handled in ofproto-dpif */
 enum ofproto_flow_miss_model {
@@ -243,6 +244,7 @@ void ofproto_set_extra_in_band_remotes(struct ofproto *,
const struct sockaddr_in *, size_t n);
 void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
 void ofproto_set_flow_limit(unsigned limit);
+void ofproto_set_max_idle(long long int max_idle);
 void ofproto_set_flow_miss_model(unsigned model);
 void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
 void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index cde4bd0..455ba14 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -494,6 +494,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch 
*ovs_cfg)
 
 ofproto_set_flow_limit(smap_get_int(&ovs_cfg->other_config, "flow-limit",
 OFPROTO_FLOW_LIMIT_DEFAULT));
+ofproto_set_max_idle(smap_get_int(&ovs_cfg->other_config, "max-idle",
+  OFPROTO_MAX_IDLE_DEFAULT));
 
 ofproto_set_threads(
 smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0),
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index e915caf..5fcfef7 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -136,6 +136,21 @@
 
   
 
+  
+
+The maximum idl

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

[Gurucharan Shetty]

Windows does not have a getrlimit() function. As such,
there is no limit to number of file descriptors that
can be opened. So, set an aritificial limit of 1024.
This may be increased in the future if the limit is too
low.

Co-authored-by: Linda Sun 
Signed-off-by: Linda Sun 
Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |8 
 1 file changed, 8 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index bfa2ce8..cde4d71 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -122,6 +122,7 @@ set_dscp(int fd, uint8_t dscp)
 return 0;
 }
 
+#ifndef _WIN32
 static bool
 rlim_is_finite(rlim_t limit)
 {
@@ -143,11 +144,13 @@ rlim_is_finite(rlim_t limit)
 
 return true;
 }
+#endif
 
 /* Returns the maximum valid FD value, plus 1. */
 int
 get_max_fds(void)
 {
+#ifndef _WIN32
 static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 static int max_fds;
 
@@ -163,6 +166,11 @@ get_max_fds(void)
 }
 
 return max_fds;
+#else
+/* Windows does not have a limitation on the number of file descriptors.
+ * Set an artificial limit at 1024. */
+return 1024;
+#endif
 }
 
 /* Translates 'host_name', which must be a string representation of an IP
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

And add more users.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |   49 +++--
 lib/socket-util.h |   12 
 lib/stream-ssl.c  |   12 
 3 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index d83ddf1..0195093 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -106,7 +106,7 @@ set_dscp(int fd, uint8_t dscp)
 
 val = dscp << 2;
 if (setsockopt(fd, IPPROTO_IP, IP_TOS, &val, sizeof val)) {
-return errno;
+return sock_errno();
 }
 
 return 0;
@@ -239,7 +239,7 @@ lookup_hostname(const char *host_name, struct in_addr *addr)
 #endif
 
 case EAI_SYSTEM:
-return errno;
+return sock_errno();
 
 default:
 return EPROTO;
@@ -762,8 +762,8 @@ inet_open_active(int style, const char *target, uint16_t 
default_port,
 /* Create non-blocking socket. */
 fd = socket(ss.ss_family, style, 0);
 if (fd < 0) {
-VLOG_ERR("%s: socket: %s", target, ovs_strerror(errno));
-error = errno;
+error = sock_errno();
+VLOG_ERR("%s: socket: %s", target, sock_strerror(error));
 goto exit;
 }
 error = set_nonblocking(fd);
@@ -776,15 +776,19 @@ inet_open_active(int style, const char *target, uint16_t 
default_port,
  * connect(), the handshake SYN frames will be sent with a TOS of 0. */
 error = set_dscp(fd, dscp);
 if (error) {
-VLOG_ERR("%s: socket: %s", target, ovs_strerror(error));
+VLOG_ERR("%s: socket: %s", target, sock_strerror(error));
 goto exit;
 }
 
 /* Connect. */
 error = connect(fd, (struct sockaddr *) &ss, ss_length(&ss)) == 0
 ? 0
-: errno;
-if (error == EINPROGRESS) {
+: sock_errno();
+if (error == EINPROGRESS
+#ifdef _WIN32
+| WSAEALREADY | WSAEWOULDBLOCK
+#endif
+) {
 error = EAGAIN;
 }
 
@@ -881,8 +885,8 @@ inet_open_passive(int style, const char *target, int 
default_port,
 /* Create non-blocking socket, set SO_REUSEADDR. */
 fd = socket(ss.ss_family, style, 0);
 if (fd < 0) {
-error = errno;
-VLOG_ERR("%s: socket: %s", target, ovs_strerror(error));
+error = sock_errno();
+VLOG_ERR("%s: socket: %s", target, sock_strerror(error));
 return -error;
 }
 error = set_nonblocking(fd);
@@ -891,16 +895,16 @@ inet_open_passive(int style, const char *target, int 
default_port,
 }
 if (style == SOCK_STREAM
 && setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof yes) < 0) {
-error = errno;
+error = sock_errno();
 VLOG_ERR("%s: setsockopt(SO_REUSEADDR): %s",
- target, ovs_strerror(error));
+ target, sock_strerror(error));
 goto error;
 }
 
 /* Bind. */
 if (bind(fd, (struct sockaddr *) &ss, ss_length(&ss)) < 0) {
-error = errno;
-VLOG_ERR("%s: bind: %s", target, ovs_strerror(error));
+error = sock_errno();
+VLOG_ERR("%s: bind: %s", target, sock_strerror(error));
 goto error;
 }
 
@@ -909,22 +913,22 @@ inet_open_passive(int style, const char *target, int 
default_port,
  * connect(), the handshake SYN frames will be sent with a TOS of 0. */
 error = set_dscp(fd, dscp);
 if (error) {
-VLOG_ERR("%s: socket: %s", target, ovs_strerror(error));
+VLOG_ERR("%s: socket: %s", target, sock_strerror(error));
 goto error;
 }
 
 /* Listen. */
 if (style == SOCK_STREAM && listen(fd, 10) < 0) {
-error = errno;
-VLOG_ERR("%s: listen: %s", target, ovs_strerror(error));
+error = sock_errno();
+VLOG_ERR("%s: listen: %s", target, sock_strerror(error));
 goto error;
 }
 
 if (ssp || kernel_chooses_port) {
 socklen_t ss_len = sizeof ss;
 if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) < 0) {
-error = errno;
-VLOG_ERR("%s: getsockname: %s", target, ovs_strerror(error));
+error = sock_errno();
+VLOG_ERR("%s: getsockname: %s", target, sock_strerror(error));
 goto error;
 }
 if (kernel_chooses_port) {
@@ -1095,8 +1099,8 @@ getsockopt_int(int fd, int level, int option, const char 
*optname, int *valuep)
 
 len = sizeof value;
 if (getsockopt(fd, level, option, &value, &len)) {
-error = errno;
-VLOG_ERR_RL(&rl, "getsockopt(%s): %s", optname, ovs_strerror(error));
+error = sock_errno();
+VLOG_ERR_RL(&rl, "getsockopt(%s): %s", optname, sock_strerror(error));
 } else if (len != sizeof value) {
 error = EINVAL;
 VLOG_ERR_RL(&rl, "getsockopt(%s): value is %u bytes (expected 
%"PRIuSIZE")",
@@ -1256,8 +1260,9 @@ af_inet_ioctl(unsigned long int command, const void *arg)
 if (ovsthread_once_start(&once)) {
 sock = socket(AF

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

[Gurucharan Shetty]

Co-authored-by: Linda Sun 
Signed-off-by: Linda Sun 
Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 0195093..bfa2ce8 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -73,6 +73,7 @@ static int getsockopt_int(int fd, int level, int option, 
const char *optname,
 int
 set_nonblocking(int fd)
 {
+#ifndef _WIN32
 int flags = fcntl(fd, F_GETFL, 0);
 if (flags != -1) {
 if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) != -1) {
@@ -85,6 +86,15 @@ set_nonblocking(int fd)
 VLOG_ERR("fcntl(F_GETFL) failed: %s", ovs_strerror(errno));
 return errno;
 }
+#else
+unsigned long arg = 1;
+if (ioctlsocket(fd, FIONBIO, &arg)) {
+int error = sock_errno();
+VLOG_ERR("set_nonblocking failed: %s", sock_strerror(error));
+return error;
+}
+return 0;
+#endif
 }
 
 void
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |2 ++
 lib/socket-util.h |2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 9095418..d3b580d 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1022,6 +1022,7 @@ error:
 return -error;
 }
 
+#ifndef _WIN32
 /* Returns a readable and writable fd for /dev/null, if successful, otherwise
  * a negative errno value.  The caller must not close the returned fd (because
  * the same fd will be handed out to subsequent callers). */
@@ -1043,6 +1044,7 @@ get_null_fd(void)
 
 return null_fd;
 }
+#endif
 
 int
 read_fully(int fd, void *p_, size_t size, size_t *bytes_read)
diff --git a/lib/socket-util.h b/lib/socket-util.h
index ba4d686..5c32033 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -46,9 +46,9 @@ int drain_rcvbuf(int fd);
 int make_unix_socket(int style, bool nonblock,
  const char *bind_path, const char *connect_path);
 int get_unix_name_len(socklen_t sun_len);
+int get_null_fd(void);
 #endif
 ovs_be32 guess_netmask(ovs_be32 ip);
-int get_null_fd(void);
 
 bool inet_parse_active(const char *target, uint16_t default_port,
struct sockaddr_storage *ssp);
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

drain_rcvbuf() is currenlty called from netlink-socket.c and
netdev-linux.c. As of now, I don't see it being used for Windows.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |2 ++
 lib/socket-util.h |2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 2e3f4a9..88b8c24 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -345,6 +345,7 @@ check_connection_completion(int fd)
 #endif
 }
 
+#ifndef _WIN32
 /* Drain all the data currently in the receive queue of a datagram socket (and
  * possibly additional data).  There is no way to know how many packets are in
  * the receive queue, but we do know that the total number of bytes queued does
@@ -379,6 +380,7 @@ drain_rcvbuf(int fd)
 }
 return 0;
 }
+#endif
 
 /* Returns the size of socket 'sock''s receive buffer (SO_RCVBUF), or a
  * negative errno value if an error occurs. */
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 6e55a4c..b4bd978 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -40,7 +40,9 @@ int lookup_hostname(const char *host_name, struct in_addr *);
 
 int get_socket_rcvbuf(int sock);
 int check_connection_completion(int fd);
+#ifndef _WIN32
 int drain_rcvbuf(int fd);
+#endif
 void drain_fd(int fd, size_t n_packets);
 int make_unix_socket(int style, bool nonblock,
  const char *bind_path, const char *connect_path);
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

There is no corresponding function for Windows.
open() does not work on directories.
There is a function _commit(fd), but that is only meant
for files.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index d3b580d..b085007 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1095,6 +1095,7 @@ int
 fsync_parent_dir(const char *file_name)
 {
 int error = 0;
+#ifndef _WIN32
 char *dir;
 int fd;
 
@@ -1116,6 +1117,7 @@ fsync_parent_dir(const char *file_name)
 VLOG_ERR("%s: open failed (%s)", dir, ovs_strerror(error));
 }
 free(dir);
+#endif
 
 return error;
 }
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

In windows there is no clear way to distinguish between a
socket fd and a file fd.

We use the function, describe_fd() mostly for debugging.
For now, return a generic statement.

Co-authored-by: Linda Sun 
Signed-off-by: Linda Sun 
Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |4 
 1 file changed, 4 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index b085007..3a8a1f7 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1305,6 +1305,7 @@ describe_fd(int fd)
 struct stat s;
 
 ds_init(&string);
+#ifndef _WIN32
 if (fstat(fd, &s)) {
 ds_put_format(&string, "fstat failed (%s)", ovs_strerror(errno));
 } else if (S_ISSOCK(s.st_mode)) {
@@ -1324,6 +1325,9 @@ describe_fd(int fd)
 put_fd_filename(&string, fd);
 #endif
 }
+#else
+ds_put_format(&string,"file descriptor");
+#endif /* _WIN32 */
 return ds_steal_cstr(&string);
 }
 
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Don't try to compile Unix socket related functions for Windows.

Signed-off-by: Gurucharan Shetty 
---
 lib/automake.mk   |4 ++--
 lib/socket-util.c |4 
 lib/socket-util.h |4 ++--
 lib/stream.c  |4 
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/automake.mk b/lib/automake.mk
index 4ecd61d..6c5ad8d 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -193,7 +193,6 @@ lib_libopenvswitch_la_SOURCES = \
lib/stream-provider.h \
lib/stream-ssl.h \
lib/stream-tcp.c \
-   lib/stream-unix.c \
lib/stream.c \
lib/stream.h \
lib/stdio.c \
@@ -244,7 +243,8 @@ lib_libopenvswitch_la_SOURCES += \
 else
 lib_libopenvswitch_la_SOURCES += \
lib/daemon.c \
-   lib/latch.c
+   lib/latch.c \
+   lib/stream-unix.c
 endif
 
 EXTRA_DIST += \
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 88b8c24..4ec7201 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -412,6 +412,7 @@ drain_fd(int fd, size_t n_packets)
 }
 }
 
+#ifndef _WIN32
 /* Attempts to shorten 'name' by opening a file descriptor for the directory
  * part of the name and indirecting through /proc/self/fd//.
  * On systems with Linux-like /proc, this works as long as  isn't too
@@ -672,6 +673,7 @@ get_unix_name_len(socklen_t sun_len)
 ? sun_len - offsetof(struct sockaddr_un, sun_path)
 : 0);
 }
+#endif /* _WIN32 */
 
 ovs_be32
 guess_netmask(ovs_be32 ip_)
@@ -1197,6 +1199,7 @@ describe_sockaddr(struct ds *string, int fd,
 ds_put_format(string, "%s:%"PRIu16,
   ss_format_address(&ss, addrbuf, sizeof addrbuf),
   ss_get_port(&ss));
+#ifndef _WIN32
 } else if (ss.ss_family == AF_UNIX) {
 struct sockaddr_un sun;
 const char *null;
@@ -1207,6 +1210,7 @@ describe_sockaddr(struct ds *string, int fd,
 null = memchr(sun.sun_path, '\0', maxlen);
 ds_put_buffer(string, sun.sun_path,
   null ? null - sun.sun_path : maxlen);
+#endif
 }
 #ifdef HAVE_NETLINK
 else if (ss.ss_family == AF_NETLINK) {
diff --git a/lib/socket-util.h b/lib/socket-util.h
index b4bd978..cf2f96d 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -40,13 +40,13 @@ int lookup_hostname(const char *host_name, struct in_addr 
*);
 
 int get_socket_rcvbuf(int sock);
 int check_connection_completion(int fd);
+void drain_fd(int fd, size_t n_packets);
 #ifndef _WIN32
 int drain_rcvbuf(int fd);
-#endif
-void drain_fd(int fd, size_t n_packets);
 int make_unix_socket(int style, bool nonblock,
  const char *bind_path, const char *connect_path);
 int get_unix_name_len(socklen_t sun_len);
+#endif
 ovs_be32 guess_netmask(ovs_be32 ip);
 int get_null_fd(void);
 
diff --git a/lib/stream.c b/lib/stream.c
index b69f03c..1dfecf0 100644
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -51,7 +51,9 @@ enum stream_state {
 
 static const struct stream_class *stream_classes[] = {
 &tcp_stream_class,
+#ifndef _WIN32
 &unix_stream_class,
+#endif
 #ifdef HAVE_OPENSSL
 &ssl_stream_class,
 #endif
@@ -59,7 +61,9 @@ static const struct stream_class *stream_classes[] = {
 
 static const struct pstream_class *pstream_classes[] = {
 &ptcp_pstream_class,
+#ifndef _WIN32
 &punix_pstream_class,
+#endif
 #ifdef HAVE_OPENSSL
 &pssl_pstream_class,
 #endif
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

For Windows sockets, one has to call closesocket() to
close the sockets.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |4 ++--
 lib/socket-util.h |4 
 lib/stream-ssl.c  |2 --
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4ec7201..9095418 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -870,7 +870,7 @@ exit:
 memset(ssp, 0, sizeof *ssp);
 }
 if (fd >= 0) {
-close(fd);
+closesocket(fd);
 fd = -1;
 }
 } else {
@@ -1018,7 +1018,7 @@ error:
 if (ssp) {
 memset(ssp, 0, sizeof *ssp);
 }
-close(fd);
+closesocket(fd);
 return -error;
 }
 
diff --git a/lib/socket-util.h b/lib/socket-util.h
index cf2f96d..ba4d686 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -113,4 +113,8 @@ static inline int sock_errno(void)
 #endif
 }
 
+#ifndef _WIN32
+#define closesocket close
+#endif
+
 #endif /* socket-util.h */
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 0dc832d..14d62c4 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -60,8 +60,6 @@
  * compiled with /MD is not tested. */
 #include 
 #define SHUT_RDWR SD_BOTH
-#else
-#define closesocket close
 #endif
 
 VLOG_DEFINE_THIS_MODULE(stream_ssl);
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Couple of return values need changes.
* EAI_NODATA is the same as EAI_NONAME. So we prevent duplicate cases.
* Windows does not have a EAI_SYSTEM.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |4 
 1 file changed, 4 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index b3b1ae5..0132133 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -265,13 +265,17 @@ lookup_hostname(const char *host_name, struct in_addr 
*addr)
 case EAI_MEMORY:
 return ENOMEM;
 
+#ifndef _WIN32
 #ifdef EAI_NODATA
 case EAI_NODATA:
 return ENXIO;
 #endif
+#endif
 
+#ifdef EAI_SYSTEM
 case EAI_SYSTEM:
 return sock_errno();
+#endif
 
 default:
 return EPROTO;
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Windows does not have a inet_aton(). But does have a
inet_pton().

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |   14 ++
 1 file changed, 14 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index cde4d71..b3b1ae5 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -173,6 +173,20 @@ get_max_fds(void)
 #endif
 }
 
+#ifdef _WIN32
+static int inet_aton(const char *host_name, struct in_addr *addr)
+{
+int retval = inet_pton(AF_INET, host_name, addr);
+if (retval == -1) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_ERR_RL(&rl, "inet_pton returned error: %s",
+sock_strerror(sock_errno()));
+return 0;
+}
+return retval;
+}
+#endif
+
 /* Translates 'host_name', which must be a string representation of an IP
  * address, into a numeric IP address in '*addr'.  Returns 0 if successful,
  * otherwise a positive errno value. */
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

There is no direct mapping for the ioctl function in
Windows.  As of now, af_inet_ioctl() is being used for Linux
and BSD. So, don't try to compile it for Windows.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |2 ++
 lib/socket-util.h |2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 3a8a1f7..69783a8 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -1331,6 +1331,7 @@ describe_fd(int fd)
 return ds_steal_cstr(&string);
 }
 
+#ifndef _WIN32
 /* Calls ioctl() on an AF_INET sock, passing the specified 'command' and
  * 'arg'.  Returns 0 if successful, otherwise a positive errno value. */
 int
@@ -1369,6 +1370,7 @@ af_inet_ifreq_ioctl(const char *name, struct ifreq *ifr, 
unsigned long int cmd,
 }
 return error;
 }
+#endif
 
 /* sockaddr_storage helpers. */
 
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 2257759..674e266 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -76,11 +76,13 @@ char *describe_fd(int fd);
  * in  is used. */
 #define DSCP_DEFAULT (IPTOS_PREC_INTERNETCONTROL >> 2)
 
+#ifndef _WIN32
 /* Helpers for calling ioctl() on an AF_INET socket. */
 struct ifreq;
 int af_inet_ioctl(unsigned long int command, const void *arg);
 int af_inet_ifreq_ioctl(const char *name, struct ifreq *,
 unsigned long int cmd, const char *cmd_name);
+#endif
 
 /* Functions for working with sockaddr_storage that might contain an IPv4 or
  * IPv6 address. */
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Windows defines the 'optval' argument as char * instead of void *.

Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.h |8 
 1 file changed, 8 insertions(+)

diff --git a/lib/socket-util.h b/lib/socket-util.h
index 5c32033..2257759 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -100,6 +100,14 @@ static inline int rpl_setsockopt(int sock, int level, int 
optname,
 {
 return (setsockopt)(sock, level, optname, optval, optlen);
 }
+
+#define getsockopt(sock, level, optname, optval, optlen) \
+rpl_getsockopt(sock, level, optname, optval, optlen)
+static inline int rpl_getsockopt(int sock, int level, int optname,
+ void *optval, socklen_t *optlen)
+{
+return (getsockopt)(sock, level, optname, optval, optlen);
+}
 #endif
 
 /* In Windows platform, errno is not set for socket calls.
-- 
1.7.9.5

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

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

[Gurucharan Shetty]

Windows send() does not have a MSG_DONTWAIT.
So, use the get_socket_error() function.

Co-authored-by: Linda Sun 
Signed-off-by: Linda Sun 
Signed-off-by: Gurucharan Shetty 
---
 lib/socket-util.c |   34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 0132133..2e3f4a9 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -282,12 +282,28 @@ lookup_hostname(const char *host_name, struct in_addr 
*addr)
 }
 }
 
+#ifdef _WIN32
+/* Returns the error condition associated with socket 'fd' and resets the
+ * socket's error status. */
+static int
+get_socket_error(int fd)
+{
+int error;
+
+if (getsockopt_int(fd, SOL_SOCKET, SO_ERROR, "SO_ERROR", &error)) {
+error = sock_errno();
+}
+return error;
+}
+#endif
+
 int
 check_connection_completion(int fd)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 10);
-struct pollfd pfd;
 int retval;
+#ifndef _WIN32
+struct pollfd pfd;
 
 pfd.fd = fd;
 pfd.events = POLLOUT;
@@ -311,6 +327,22 @@ check_connection_completion(int fd)
 } else {
 return EAGAIN;
 }
+#else
+WSAPOLLFD pfd;
+pfd.fd = fd;
+pfd.events = POLLWRNORM;
+
+retval = WSAPoll(&pfd, 1, 0);
+if (retval == 1) {
+return get_socket_error(fd);
+} else if (retval == SOCKET_ERROR) {
+int error = sock_errno();
+VLOG_ERR_RL(&rl, "poll: %s", sock_strerror(error));
+return error;
+} else {
+return EAGAIN;
+}
+#endif
 }
 
 /* Drain all the data currently in the receive queue of a datagram socket (and
-- 
1.7.9.5

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

Re: [ovs-dev] [PATCH v2 00/13] datapath: Reduce lock contention.

[Pravin Shelar]

Hi Jarno,
Can you send updated patches for current master?

Thanks,
Pravin.

On Tue, Feb 11, 2014 at 4:07 PM, Jarno Rajahalme  wrote:
> This series reduces kernel datapath lock contention.  I have included
> the NUMA stats patches, as the rest do not apply cleanly without them.
> I have added netperf TCP_CRR results (connections/second) as the first
> column in the list below.  These are in a specific 2-socket server,
> YMMV.
>
> I have included the current master as a baseline.  The numbers are for
> the best run after a warmup, representing a case when revalidation is
> not necessary, but stats are still pushed continuously.
>
> Finally, I have manually set the max_idle to 750ms, so that it would
> not skew the results (too high max_idle will cause handlers to not be
> able to push all flows, which leads to more upcalls, and more TCP
> latency -> lower TCP_CRR score.)
>
> Jarno Rajahalme (13):
> 19k Master
> 17k datapath: Remove 5-tuple optimization.
> 45.1k   datapath: Per NUMA node flow stats.
> datapath: Fix race.
> datapath: Avoid assigning a NULL pointer to flow actions.
> datapath: Use TCP flags in the flow key for stats.
> datapath: Clarify locking.
> 45.2k   datapath: Minimize dp and vport critical sections.
> 52.3k   datapath: Minimize ovs_flow_cmd_del critical section.
> 52.3k   datapath: Minimize ovs_flow_cmd_new_or_set critical section.
> datapath: Store alloced size with sw_flow_acts.
> 52.7k   datapath: Remove memory allocations from ovs_flow_cmd_execute.
> 56.2k   datapath: Build netlink reply only if needed.
> 56.8k   datapath: Do not return actions from flow delete.
>
>  datapath/datapath.c |  528 
> +--
>  datapath/flow.c |  181 
>  datapath/flow.h |   19 +-
>  datapath/flow_netlink.c |   78 ++-
>  datapath/flow_netlink.h |3 +-
>  datapath/flow_table.c   |   59 +++---
>  datapath/flow_table.h   |6 +-
>  7 files changed, 445 insertions(+), 429 deletions(-)
>
> --
> 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 v2 00/13] datapath: Reduce lock contention.

[Jarno Rajahalme]

I’m working on it…

  Jarno

On Feb 19, 2014, at 4:47 PM, Pravin Shelar  wrote:

> Hi Jarno,
> Can you send updated patches for current master?
> 
> Thanks,
> Pravin.
> 
> On Tue, Feb 11, 2014 at 4:07 PM, Jarno Rajahalme  
> wrote:
>> This series reduces kernel datapath lock contention.  I have included
>> the NUMA stats patches, as the rest do not apply cleanly without them.
>> I have added netperf TCP_CRR results (connections/second) as the first
>> column in the list below.  These are in a specific 2-socket server,
>> YMMV.
>> 
>> I have included the current master as a baseline.  The numbers are for
>> the best run after a warmup, representing a case when revalidation is
>> not necessary, but stats are still pushed continuously.
>> 
>> Finally, I have manually set the max_idle to 750ms, so that it would
>> not skew the results (too high max_idle will cause handlers to not be
>> able to push all flows, which leads to more upcalls, and more TCP
>> latency -> lower TCP_CRR score.)
>> 
>> Jarno Rajahalme (13):
>> 19k Master
>> 17k datapath: Remove 5-tuple optimization.
>> 45.1k   datapath: Per NUMA node flow stats.
>>datapath: Fix race.
>>datapath: Avoid assigning a NULL pointer to flow actions.
>>datapath: Use TCP flags in the flow key for stats.
>>datapath: Clarify locking.
>> 45.2k   datapath: Minimize dp and vport critical sections.
>> 52.3k   datapath: Minimize ovs_flow_cmd_del critical section.
>> 52.3k   datapath: Minimize ovs_flow_cmd_new_or_set critical section.
>>datapath: Store alloced size with sw_flow_acts.
>> 52.7k   datapath: Remove memory allocations from ovs_flow_cmd_execute.
>> 56.2k   datapath: Build netlink reply only if needed.
>> 56.8k   datapath: Do not return actions from flow delete.
>> 
>> datapath/datapath.c |  528 
>> +--
>> datapath/flow.c |  181 
>> datapath/flow.h |   19 +-
>> datapath/flow_netlink.c |   78 ++-
>> datapath/flow_netlink.h |3 +-
>> datapath/flow_table.c   |   59 +++---
>> datapath/flow_table.h   |6 +-
>> 7 files changed, 445 insertions(+), 429 deletions(-)
>> 
>> --
>> 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] ofp-util: Avoid gcc warning on NetBSD

[YAMAMOTO Takashi]

Avoid the following warning caused by the combination
of NetBSD's htons implementation and gcc bug.
Now --enable-Werror build succeeds on NetBSD-6.

lib/ofp-util.c: In function 'ofputil_match_from_ofp10_match':
lib/ofp-util.c:174:15: error: integer overflow in expression

Signed-off-by: YAMAMOTO Takashi 
---
 lib/ofp-util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index aa25e67..fca18de 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -169,9 +169,11 @@ ofputil_match_from_ofp10_match(const struct ofp10_match 
*ofmatch,
 match->wc.masks.vlan_tci = htons(0x);
 } else {
 ovs_be16 vid, pcp, tci;
+uint16_t hpcp;
 
 vid = ofmatch->dl_vlan & htons(VLAN_VID_MASK);
-pcp = htons((ofmatch->dl_vlan_pcp << VLAN_PCP_SHIFT) & VLAN_PCP_MASK);
+hpcp = (ofmatch->dl_vlan_pcp << VLAN_PCP_SHIFT) & VLAN_PCP_MASK;
+pcp = htons(hpcp);
 tci = vid | pcp | htons(VLAN_CFI);
 match->flow.vlan_tci = tci & match->wc.masks.vlan_tci;
 }
-- 
1.8.3.1

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

[ovs-dev] [PATCH v8] ofproto: Honour Table Mod settings for table-miss handling

[Simon Horman]

This reworks lookup of rules for both table 0 and table action translation.
The result is that Table Mod settings, which can alter the miss-behaviour
of tables, including table 0, on a per-table basis may be honoured.

Previous patches proposed by myself which build on earlier merged patches
by Andy Zhou implement the ofproto side of Table Mod. So with this patch
the feature should be complete.

Neither this patch, nor any other patches it builds on, alter the default
behaviour of Open vSwitch. And in particular the OpenFlow1.1 behaviour is
the default regardless of which OpenFlow version is negotiated between the
switch and the controller.

An implementation detail, which lends itself to future work, is the
handling of OFPTC_TABLE_MISS_CONTINUE. If a table has this behaviour set by
Table Mod and a miss occurs then a loop is created, skipping to the next
table. It is quite easy to create a situation where this loop covers ~255
tables which is very expensive as the lookup for each table involves taking
locks, amongst other things.

Cc: Andy Zhou 
Signed-off-by: Simon Horman 

---
v8
* Rebase

v7
* As suggested by Ben Pfaff
  - Used simplified loop/case logic in rule_dpif_lookup_from_table()
+ I re-introduced next_id so that table_id is not advanced to
  ofproto->up.n_tables.
  - Consolidate logic of xlate_table_action() and rule_dpif_lookup()
into rule_dpif_lookup_from_table()
  - Do not honour table mod settings in the case of a resubmit action

v4 - v6
* Rebase

v3
* Remove bogus fall-through comment in OFPTC_TABLE_MISS_CONTROLLER case
  in rule_dpif_lookup_from_table(). This case always returns.
* Add fall-through from OFPTC_TABLE_MISS_CONTROLLER
  to OFPTC_TABLE_MISS_DROP in xlate_table_action() to handle
  situations where packet_in is not allowed.
* Re-arrange rule_dpif_lookup_from_table() and xlate_table_action()
  to only have one call to choose_miss_rule(). This seems tidier.
* Consistently call ovs-ofctl monitor without a -P argument
* Remove ofproto parameter name from declaration of table_get_config().
  This is in keeping with the coding style of other declarations in
  ofproto.h
* Only allow supported configurations in table_mod message

v2
* Use ofproto->up.n_tables instead of N_TABLES as the number
  of tables present.
* Do not use the presence of rules in a table to indicate that it is
  present. Instead treat all tables, other than TBL_INTERNAL, that
  exist in ofproto.up as present. This simplifies things slightly
  and seems to be a more logical approach.
* As pointed out by Yamamoto-san
  - Read config for each table that is missed rather
than just the first one. A logic bug that somehow I was blind to.
---
 OPENFLOW-1.1+|   5 -
 ofproto/ofproto-dpif-xlate.c |  77 ++
 ofproto/ofproto-dpif.c   | 183 --
 ofproto/ofproto-dpif.h   |  12 +-
 ofproto/ofproto.c|  14 +-
 ofproto/ofproto.h|   5 +
 tests/ofproto-dpif.at| 353 +++
 7 files changed, 600 insertions(+), 49 deletions(-)

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index 1b8a0ee..4363d28 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -54,11 +54,6 @@ OpenFlow 1.1
 The list of remaining work items for OpenFlow 1.1 is below.  It is
 probably incomplete.
 
-* OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
-  implement it.  It should be implemented so that the default OVS
-  behavior does not change.  Simon Horman has posted a patch.
-  [required for OF1.1 and OF1.2]
-
 * MPLS.  Simon Horman maintains a patch series that adds this
   feature.  This is partially merged.
   [optional for OF1.1+]
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ccf0b75..0b3d963 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -225,8 +225,9 @@ static void xlate_actions__(struct xlate_in *, struct 
xlate_out *)
 OVS_REQ_RDLOCK(xlate_rwlock);
 static void xlate_normal(struct xlate_ctx *);
 static void xlate_report(struct xlate_ctx *, const char *);
-static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
-   uint8_t table_id, bool may_packet_in);
+static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
+   uint8_t table_id, bool may_packet_in,
+   bool force_controller_on_miss);
 static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
 static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
 static void output_normal(struct xlate_ctx *, const struct xbundle *,
@@ -1719,14 +1720,14 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 ctx->xout->slow |= special;
 } else if (may_receive(peer, ctx)) {
 if (xport_stp_forward_state(peer)) {
-xlate_table_action(ctx, flow->in_port.ofp_port, 0, true

[ovs-dev] [PATCH v8] ofproto: Honour Table Mod settings for table-miss handling

[Simon Horman]

This reworks lookup of rules for both table 0 and table action translation.
The result is that Table Mod settings, which can alter the miss-behaviour
of tables, including table 0, on a per-table basis may be honoured.

Previous patches proposed by myself which build on earlier merged patches
by Andy Zhou implement the ofproto side of Table Mod. So with this patch
the feature should be complete.

Neither this patch, nor any other patches it builds on, alter the default
behaviour of Open vSwitch. And in particular the OpenFlow1.1 behaviour is
the default regardless of which OpenFlow version is negotiated between the
switch and the controller.

An implementation detail, which lends itself to future work, is the
handling of OFPTC_TABLE_MISS_CONTINUE. If a table has this behaviour set by
Table Mod and a miss occurs then a loop is created, skipping to the next
table. It is quite easy to create a situation where this loop covers ~255
tables which is very expensive as the lookup for each table involves taking
locks, amongst other things.

Cc: Andy Zhou 
Signed-off-by: Simon Horman 

---
v8
* Rebase

v7
* As suggested by Ben Pfaff
  - Used simplified loop/case logic in rule_dpif_lookup_from_table()
+ I re-introduced next_id so that table_id is not advanced to
  ofproto->up.n_tables.
  - Consolidate logic of xlate_table_action() and rule_dpif_lookup()
into rule_dpif_lookup_from_table()
  - Do not honour table mod settings in the case of a resubmit action

v4 - v6
* Rebase

v3
* Remove bogus fall-through comment in OFPTC_TABLE_MISS_CONTROLLER case
  in rule_dpif_lookup_from_table(). This case always returns.
* Add fall-through from OFPTC_TABLE_MISS_CONTROLLER
  to OFPTC_TABLE_MISS_DROP in xlate_table_action() to handle
  situations where packet_in is not allowed.
* Re-arrange rule_dpif_lookup_from_table() and xlate_table_action()
  to only have one call to choose_miss_rule(). This seems tidier.
* Consistently call ovs-ofctl monitor without a -P argument
* Remove ofproto parameter name from declaration of table_get_config().
  This is in keeping with the coding style of other declarations in
  ofproto.h
* Only allow supported configurations in table_mod message

v2
* Use ofproto->up.n_tables instead of N_TABLES as the number
  of tables present.
* Do not use the presence of rules in a table to indicate that it is
  present. Instead treat all tables, other than TBL_INTERNAL, that
  exist in ofproto.up as present. This simplifies things slightly
  and seems to be a more logical approach.
* As pointed out by Yamamoto-san
  - Read config for each table that is missed rather
than just the first one. A logic bug that somehow I was blind to.
---
 OPENFLOW-1.1+|   5 -
 ofproto/ofproto-dpif-xlate.c |  77 ++
 ofproto/ofproto-dpif.c   | 183 --
 ofproto/ofproto-dpif.h   |  12 +-
 ofproto/ofproto.c|  14 +-
 ofproto/ofproto.h|   5 +
 tests/ofproto-dpif.at| 353 +++
 7 files changed, 600 insertions(+), 49 deletions(-)

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index 1b8a0ee..4363d28 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -54,11 +54,6 @@ OpenFlow 1.1
 The list of remaining work items for OpenFlow 1.1 is below.  It is
 probably incomplete.
 
-* OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
-  implement it.  It should be implemented so that the default OVS
-  behavior does not change.  Simon Horman has posted a patch.
-  [required for OF1.1 and OF1.2]
-
 * MPLS.  Simon Horman maintains a patch series that adds this
   feature.  This is partially merged.
   [optional for OF1.1+]
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ccf0b75..0b3d963 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -225,8 +225,9 @@ static void xlate_actions__(struct xlate_in *, struct 
xlate_out *)
 OVS_REQ_RDLOCK(xlate_rwlock);
 static void xlate_normal(struct xlate_ctx *);
 static void xlate_report(struct xlate_ctx *, const char *);
-static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
-   uint8_t table_id, bool may_packet_in);
+static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
+   uint8_t table_id, bool may_packet_in,
+   bool force_controller_on_miss);
 static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
 static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
 static void output_normal(struct xlate_ctx *, const struct xbundle *,
@@ -1719,14 +1720,14 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
 ctx->xout->slow |= special;
 } else if (may_receive(peer, ctx)) {
 if (xport_stp_forward_state(peer)) {
-xlate_table_action(ctx, flow->in_port.ofp_port, 0, true

Re: [ovs-dev] [PATCH v8] ofproto: Honour Table Mod settings for table-miss handling

[Simon Horman]

Apologies for sending this twice, both posts contain the same patch,
so please ignore one of them.

On Thu, Feb 20, 2014 at 11:17:41AM +0900, Simon Horman wrote:
> This reworks lookup of rules for both table 0 and table action translation.
> The result is that Table Mod settings, which can alter the miss-behaviour
> of tables, including table 0, on a per-table basis may be honoured.
> 
> Previous patches proposed by myself which build on earlier merged patches
> by Andy Zhou implement the ofproto side of Table Mod. So with this patch
> the feature should be complete.
> 
> Neither this patch, nor any other patches it builds on, alter the default
> behaviour of Open vSwitch. And in particular the OpenFlow1.1 behaviour is
> the default regardless of which OpenFlow version is negotiated between the
> switch and the controller.
> 
> An implementation detail, which lends itself to future work, is the
> handling of OFPTC_TABLE_MISS_CONTINUE. If a table has this behaviour set by
> Table Mod and a miss occurs then a loop is created, skipping to the next
> table. It is quite easy to create a situation where this loop covers ~255
> tables which is very expensive as the lookup for each table involves taking
> locks, amongst other things.
> 
> Cc: Andy Zhou 
> Signed-off-by: Simon Horman 
> 
> ---
> v8
> * Rebase
> 
> v7
> * As suggested by Ben Pfaff
>   - Used simplified loop/case logic in rule_dpif_lookup_from_table()
> + I re-introduced next_id so that table_id is not advanced to
>   ofproto->up.n_tables.
>   - Consolidate logic of xlate_table_action() and rule_dpif_lookup()
> into rule_dpif_lookup_from_table()
>   - Do not honour table mod settings in the case of a resubmit action
> 
> v4 - v6
> * Rebase
> 
> v3
> * Remove bogus fall-through comment in OFPTC_TABLE_MISS_CONTROLLER case
>   in rule_dpif_lookup_from_table(). This case always returns.
> * Add fall-through from OFPTC_TABLE_MISS_CONTROLLER
>   to OFPTC_TABLE_MISS_DROP in xlate_table_action() to handle
>   situations where packet_in is not allowed.
> * Re-arrange rule_dpif_lookup_from_table() and xlate_table_action()
>   to only have one call to choose_miss_rule(). This seems tidier.
> * Consistently call ovs-ofctl monitor without a -P argument
> * Remove ofproto parameter name from declaration of table_get_config().
>   This is in keeping with the coding style of other declarations in
>   ofproto.h
> * Only allow supported configurations in table_mod message
> 
> v2
> * Use ofproto->up.n_tables instead of N_TABLES as the number
>   of tables present.
> * Do not use the presence of rules in a table to indicate that it is
>   present. Instead treat all tables, other than TBL_INTERNAL, that
>   exist in ofproto.up as present. This simplifies things slightly
>   and seems to be a more logical approach.
> * As pointed out by Yamamoto-san
>   - Read config for each table that is missed rather
> than just the first one. A logic bug that somehow I was blind to.
> ---
>  OPENFLOW-1.1+|   5 -
>  ofproto/ofproto-dpif-xlate.c |  77 ++
>  ofproto/ofproto-dpif.c   | 183 --
>  ofproto/ofproto-dpif.h   |  12 +-
>  ofproto/ofproto.c|  14 +-
>  ofproto/ofproto.h|   5 +
>  tests/ofproto-dpif.at| 353 
> +++
>  7 files changed, 600 insertions(+), 49 deletions(-)
> 
> diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> index 1b8a0ee..4363d28 100644
> --- a/OPENFLOW-1.1+
> +++ b/OPENFLOW-1.1+
> @@ -54,11 +54,6 @@ OpenFlow 1.1
>  The list of remaining work items for OpenFlow 1.1 is below.  It is
>  probably incomplete.
>  
> -* OFPT_TABLE_MOD message.  This is new in OF1.1, so we need to
> -  implement it.  It should be implemented so that the default OVS
> -  behavior does not change.  Simon Horman has posted a patch.
> -  [required for OF1.1 and OF1.2]
> -
>  * MPLS.  Simon Horman maintains a patch series that adds this
>feature.  This is partially merged.
>[optional for OF1.1+]
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index ccf0b75..0b3d963 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -225,8 +225,9 @@ static void xlate_actions__(struct xlate_in *, struct 
> xlate_out *)
>  OVS_REQ_RDLOCK(xlate_rwlock);
>  static void xlate_normal(struct xlate_ctx *);
>  static void xlate_report(struct xlate_ctx *, const char *);
> -static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
> -   uint8_t table_id, bool may_packet_in);
> +static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
> +   uint8_t table_id, bool may_packet_in,
> +   bool force_controller_on_miss);
>  static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
>  static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid)

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

[YAMAMOTO Takashi]

> This commit removes the 'dispatcher' thread by allowing 'handler'
> threads to read upcalls directly from dpif.  vport in dpif will
> open netlink sockets for each handler and will use the 5-tuple
> hash from the missed packet to choose which socket (handler) to
> send the upcall.
> 
> This patch also significantly simplifies the flow miss handling
> code and brings slight improvement to flow setup rate.

doesn't this make dpif-netdev reorder packets in a flow
(eg. a tcp stream) if running with multiple forwarders?

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

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

[Alex Wang]

Thanks a lot for pointing it out, Yamamoto,

I failed to cover this.  will work on solving it.

Alex Wang,

On Wed, Feb 19, 2014 at 6:36 PM, YAMAMOTO Takashi wrote:

> > This commit removes the 'dispatcher' thread by allowing 'handler'
> > threads to read upcalls directly from dpif.  vport in dpif will
> > open netlink sockets for each handler and will use the 5-tuple
> > hash from the missed packet to choose which socket (handler) to
> > send the upcall.
> >
> > This patch also significantly simplifies the flow miss handling
> > code and brings slight improvement to flow setup rate.
>
> doesn't this make dpif-netdev reorder packets in a flow
> (eg. a tcp stream) if running with multiple forwarders?
>
> YAMAMOTO Takashi
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Kmindg G]

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

looks good to me.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Simon Horman]

$logfile is already prefixed by "$sandbox/" and suffixed by ".log"
so do not duplicate this prefix and suffix combination when appending
$logfile to $logs.

Cc: YAMAMOTO Takashi 
Signed-off-by: Simon Horman 
---
 tests/run-ryu | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/run-ryu b/tests/run-ryu
index 05578d4..94fe6bb 100755
--- a/tests/run-ryu
+++ b/tests/run-ryu
@@ -97,7 +97,7 @@ run_app() {
 EOF
 logfile=$sandbox/`echo $app | sed 's,/,.,g'`.log
 logs="$logs
-$sandbox/$logfile.log"
+$logfile"
 ryu-manager "$app" --log-file="$logfile" & pid=$!
 echo $pid > "$sandbox/ryu.pid"
 i=0
-- 
1.8.5.2

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