Re: [ovs-dev] [PATCH/RFC 4/8] ofproto-dpif: Add recirc_id field to struct rule_dpif

2014-03-26 Thread Simon Horman
On Tue, Mar 25, 2014 at 09:18:19PM -0700, Andy Zhou wrote:
> On Tue, Mar 25, 2014 at 7:19 PM, Simon Horman  wrote:
> > Hi Andy,
> >
> > to date, the implementation of MPLS has not provided any entity other
> > than the rules themselves.
> 
> I see. I need to step back to understand the bigger picture more. I
> have not been closely following the MPLS patches in the past.
> >
> > This being the case it is not the label that defines the behaviour after
> > recirculation but rather what is done to a packet after the label stack is
> > modified. It is quite conceivable (though possibly not useful) to construct
> > different rules which use the same label in different ways.  Or to have
> > rules which require recirculation for MPLS but are not tied to a specific
> > label. An example of the latter is match=mpls 
> > actions=pop_mpls:0x800,dec_ttl.
> 
> I understand and agree.  What would be the best way to describe how
> recirc_id is used in implementing mpls?  My best understanding of
> reading this patch series is each recirc_id represent a MPLS (or vlan)
> label stack change. Recirculation helps to break up actions that
> applies at different label stack level.  Is this correct?
> >
> > To turn the tables around. I was somewhat surprised when I initially read
> > your implementation to see that you were using a higher-level entity than
> > rules in conjunction with recirculation for bonding. But I see that it
> > makes quite a lot of sense in the case of bonding.
> >
> Point taken.  I am still not sure I understand why there should be only one
> recirc_id per rule.

I'm not sure that there should be only one recirc_id per rule.
But rather one every time a rule makes a transition that
requires recirculation.

I'm not sure that my implementation meets that need but that
was the intention.

> > Assuming I have understood it correctly, I think that the infrastructure
> > you have created should work quite nicely both with and without a
> > higher-level entity. As I hope my patchset demonstrates.
> 
> The design intent is that the recirculation infrastructure should be
> general enough for multiple features.  I'd like to study the patch
> series in more detail to make sure it works for mpls.  Having
> higher-level entity or not is a detailed  issue that we should be able
> flush out soon.
> 
> >
> > On Tue, Mar 25, 2014 at 02:54:46PM -0700, Andy Zhou wrote:
> >> Simon, I am not sure If I understand how recirc_id should be managed for 
> >> MPLS.
> >>
> >> Why should rule allocate and free recirc_id by itself, rather that
> >> some other entity managing them?  In the bond case, bond is one
> >> managing recirc_ids. one recirc_id per bond.  I'd imaging some entity
> >> in the user space should keep track of the mpls labels and their
> >> mapping to recirc_id?
> >>
> >>
> >> On Tue, Mar 25, 2014 at 2:24 PM, Simon Horman  wrote:
> >> > This is to allow a recirculation id to be associated with a rule
> >> > in the case that its actions cause recirculation.
> >> >
> >> > In such a case if the recirc_id field is non-zero then that value should 
> >> > be
> >> > used, otherwise a value should be obtained using
> >> > ofproto_dpif_alloc_recirc_id and saved in recirc_id field.
> >> >
> >> > When destructing the rule if the recirc_id field is non-zero then
> >> > the associated internal flow should be deleted.
> >> >
> >> > This is in preparation for using the same helper as part of support
> >> > for using recirculation in conjunction series of actions including
> >> > with MPLS actions that are currently not able to be translated.
> >> >
> >> > Signed-off-by: Simon Horman 
> >> > ---
> >> >  ofproto/ofproto-dpif.c | 27 +++
> >> >  ofproto/ofproto-dpif.h |  1 +
> >> >  2 files changed, 28 insertions(+)
> >> >
> >> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> >> > index 05f7bca..f239735 100644
> >> > --- a/ofproto/ofproto-dpif.c
> >> > +++ b/ofproto/ofproto-dpif.c
> >> > @@ -89,6 +89,12 @@ struct rule_dpif {
> >> >   *   recently been processed by a revalidator. */
> >> >  struct ovs_mutex stats_mutex;
> >> >  struct dpif_flow_stats stats OVS_GUARDED;
> >> > +
> >> > +/* If non-zero then the recirculation id that has
> >> > + * been allocated for use with this rule.
> >> > + * The recirculation id and associated internal flow should
> >> > + * be freed when the rule is freed */
> >> > +uint32_t recirc_id;
> >> >  };
> >> >
> >> >  static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t 
> >> > *bytes,
> >> > @@ -3154,6 +3160,19 @@ rule_dpif_get_actions(const struct rule_dpif 
> >> > *rule)
> >> >  return rule_get_actions(&rule->up);
> >> >  }
> >> >
> >> > +/* Returns 'rule''s recirculation id. */
> >> > +uint32_t
> >> > +rule_dpif_get_recirc_id(struct rule_dpif *rule)
> >> > +OVS_REQUIRES(rule->up.mutex)
> >> > +{
> >> > +if (!rule->recirc_id) {
> >> > +struct ofproto_dpif *ofproto = 
> >> > ofproto_dpif_cast(rule-

Re: [ovs-dev] hackathon idea list

2014-03-26 Thread Flavio Leitner
On Tue, Mar 25, 2014 at 10:11:23PM -0700, Ben Pfaff wrote:
> Bash Command Completion
> ---
> 
> ovs-vsctl and other programs would be easier to use if bash command
> completion (with ``tab'', etc.) were supported. Alex Wang
>  is planning to lead a team for this project.

Feel free to use my bash completion script as you want:
http://people.redhat.com/~fleitner/ovs-vsctl/ovs-vsctl.sh

That was my first round, so it needs work to clean up, etc..

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


Re: [ovs-dev] hackathon idea list

2014-03-26 Thread Alex Wang
Thanks Flavio,

In fact, I have learned a lot from your post about how to write compgen
functions. ;D

Will keep using your script as reference ~

I'm thinking of writing a script that parses the manpage and generates the
script of compgen functions.

I have some initial implementation here for appctl
https://github.com/yew011/ovs_command_comp

Alex Wang,


On Wed, Mar 26, 2014 at 5:22 AM, Flavio Leitner  wrote:

> On Tue, Mar 25, 2014 at 10:11:23PM -0700, Ben Pfaff wrote:
> > Bash Command Completion
> > ---
> >
> > ovs-vsctl and other programs would be easier to use if bash command
> > completion (with ``tab'', etc.) were supported. Alex Wang
> >  is planning to lead a team for this project.
>
> Feel free to use my bash completion script as you want:
> http://people.redhat.com/~fleitner/ovs-vsctl/ovs-vsctl.sh
>
> That was my first round, so it needs work to clean up, etc..
>
> fbl
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] hackathon idea list

2014-03-26 Thread Flavio Leitner
On Wed, Mar 26, 2014 at 07:25:34AM -0700, Alex Wang wrote:
> Thanks Flavio,
> 
> In fact, I have learned a lot from your post about how to write compgen
> functions. ;D

Same here, it was my first attempt to write bash completion scripts. :)

 
> Will keep using your script as reference ~
> 
> I'm thinking of writing a script that parses the manpage and generates the
> script of compgen functions.
> 
> I have some initial implementation here for appctl
> https://github.com/yew011/ovs_command_comp

That would be even better. I will take a look.
Flavio

> Alex Wang,
> 
> 
> On Wed, Mar 26, 2014 at 5:22 AM, Flavio Leitner  wrote:
> 
> > On Tue, Mar 25, 2014 at 10:11:23PM -0700, Ben Pfaff wrote:
> > > Bash Command Completion
> > > ---
> > >
> > > ovs-vsctl and other programs would be easier to use if bash command
> > > completion (with ``tab'', etc.) were supported. Alex Wang
> > >  is planning to lead a team for this project.
> >
> > Feel free to use my bash completion script as you want:
> > http://people.redhat.com/~fleitner/ovs-vsctl/ovs-vsctl.sh
> >
> > That was my first round, so it needs work to clean up, etc..
> >
> > fbl
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [patch net-next RFC v2 0/6] introduce infrastructure for support of switch chip datapath

2014-03-26 Thread Jiri Pirko
This is second version of RFC. Here are the main differences from the first one:
-There is no special swdev of swport structure. The switch and its ports are
 now represented only by net_device structures. There are couple of 
switch-specific
 ndos added (inserting and removing flows).

-Regarding the flows, driver marks skb with "missing flow" flag now. That would
 give indication to a user (OVS datapath of af_packet userspace application).
 On the opposite direction, skb can be xmitted by a port.

-dummyswitch module has now rtnetlink iface for easy creation of dummy switches
 and ports.

The basic idea is to introduce a generic infractructure to support various
switch chips in kernel. Also the idea is to benefit of currently existing
Open vSwitch userspace infrastructure.


The first two patches are just minor skb flag and packet_type modifications.


The third patch does a split of structures which are not specific to OVS
into more generic ones that can be reused.


The fourth patch introduces the "switchdev" API itself. It should serve as
a glue between chip drivers on the one side and the user on the other.
That user might be OVS datapath but in future it might be just userspace
application interacting via af_packet and Netlink iface.

The infrastructure is designed to be similar to for example linux bridge.
There is one netdevice representing a switch chip and one netdevice per every
port. These are bound together in classic slave-master way. The reason
to reuse the netdevices for port representation is that userspace can use
standard tools to get information about the ports, statistics, tcpdump, etc.

Note that the netdevices are just representations of the ports in the switch.
Therefore **no actual data** goes though, only missed flow skbs and, if drivers
supports it, when ETH_P_ALL packet_type is hooked on (tcpdump).


The fifth patch introduces a support for switchdev vports into OVS datapath.
After that, userspace would be able to create a switchdev DP for a switch chip,
to add switchdev ports to it and to use it in the same way as it would be
OVS SW datapath.


The sixth patch adds a dummy switch module. It is just an example of
switchdev driver implementation.


Jiri Pirko (6):
  net: make packet_type->ak_packet_priv generic
  skbuff: add "missed_flow" flag
  openvswitch: split flow structures into ovs specific and generic ones
  net: introduce switchdev API
  openvswitch: Introduce support for switchdev based datapath
  net: introduce dummy switch

 drivers/net/Kconfig|   7 +
 drivers/net/Makefile   |   1 +
 drivers/net/dummyswitch.c  | 235 +
 include/linux/filter.h |   1 +
 include/linux/netdevice.h  |  26 +++-
 include/linux/skbuff.h |  13 ++
 include/linux/sw_flow.h| 105 +
 include/linux/switchdev.h  |  30 
 include/uapi/linux/if_link.h   |   9 ++
 include/uapi/linux/openvswitch.h   |   4 +
 net/Kconfig|  10 ++
 net/core/Makefile  |   1 +
 net/core/dev.c |   4 +-
 net/core/filter.c  |   3 +
 net/core/switchdev.c   | 172 +
 net/openvswitch/Makefile   |   4 +
 net/openvswitch/datapath.c |  90 +++
 net/openvswitch/datapath.h |  12 +-
 net/openvswitch/dp_notify.c|   3 +-
 net/openvswitch/flow.c |  14 +-
 net/openvswitch/flow.h | 123 +++
 net/openvswitch/flow_netlink.c |  24 +--
 net/openvswitch/flow_netlink.h |   4 +-
 net/openvswitch/flow_table.c   | 100 ++--
 net/openvswitch/flow_table.h   |  18 +--
 net/openvswitch/vport-gre.c|   4 +-
 net/openvswitch/vport-internal_switchdev.c | 179 ++
 net/openvswitch/vport-internal_switchdev.h |  28 
 net/openvswitch/vport-netdev.c |   4 +-
 net/openvswitch/vport-switchportdev.c  | 205 +
 net/openvswitch/vport-switchportdev.h  |  24 +++
 net/openvswitch/vport-vxlan.c  |   2 +-
 net/openvswitch/vport.c|   6 +-
 net/openvswitch/vport.h|   4 +-
 net/packet/af_packet.c |  22 ++-
 35 files changed, 1269 insertions(+), 222 deletions(-)
 create mode 100644 drivers/net/dummyswitch.c
 create mode 100644 include/linux/sw_flow.h
 create mode 100644 include/linux/switchdev.h
 create mode 100644 net/core/switchdev.c
 create mode 100644 net/openvswitch/vport-internal_switchdev.c
 create mode 100644 net/openvswitch/vport-internal_switchdev.h
 create mode 100644 net/openvswitch/vport-switchportdev.c
 create mode 100644 net/openvswitch/vport-switchportdev.h

-- 
1.8.5.3


[ovs-dev] [patch net-next RFC v2 6/6] net: introduce dummy switch

2014-03-26 Thread Jiri Pirko
Dummy switch implementation using switchdev API

Signed-off-by: Jiri Pirko 
---
 drivers/net/Kconfig  |   7 ++
 drivers/net/Makefile |   1 +
 drivers/net/dummyswitch.c| 235 +++
 include/uapi/linux/if_link.h |   9 ++
 4 files changed, 252 insertions(+)
 create mode 100644 drivers/net/dummyswitch.c

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 89402c3..a9629a7 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -71,6 +71,13 @@ config DUMMY
  To compile this driver as a module, choose M here: the module
  will be called dummy.
 
+config NET_DUMMY_SWITCH
+   tristate "Dummy switch net driver support"
+   depends on NET_SWITCHDEV
+   ---help---
+ To compile this driver as a module, choose M here: the module
+ will be called dummyswitch.
+
 config EQUALIZER
tristate "EQL (serial line load balancing) support"
---help---
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 3fef8a8..d5d4ce6 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -7,6 +7,7 @@
 #
 obj-$(CONFIG_BONDING) += bonding/
 obj-$(CONFIG_DUMMY) += dummy.o
+obj-$(CONFIG_NET_DUMMY_SWITCH) += dummyswitch.o
 obj-$(CONFIG_EQUALIZER) += eql.o
 obj-$(CONFIG_IFB) += ifb.o
 obj-$(CONFIG_MACVLAN) += macvlan.o
diff --git a/drivers/net/dummyswitch.c b/drivers/net/dummyswitch.c
new file mode 100644
index 000..58c65a9
--- /dev/null
+++ b/drivers/net/dummyswitch.c
@@ -0,0 +1,235 @@
+/*
+ * drivers/net/dummyswitch.c - Dummy switch device
+ * Copyright (c) 2014 Jiri Pirko 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/*
+ * Dummy switch
+ */
+
+static void dummysw_uninit(struct net_device *dev)
+{
+   __swdev_unregister(dev);
+}
+
+static netdev_tx_t dummysw_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+   dev_kfree_skb(skb);
+   return NETDEV_TX_OK;
+}
+
+static int dummysw_flow_insert(struct net_device *dev, struct sw_flow *flow)
+{
+   netdev_dbg(dev, "Flow inserted");
+   return 0;
+}
+static int dummysw_flow_remove(struct net_device *dev, struct sw_flow *flow)
+{
+   netdev_dbg(dev, "Flow removed");
+   return 0;
+}
+
+static const struct net_device_ops dummysw_netdev_ops = {
+   .ndo_uninit = dummysw_uninit,
+   .ndo_start_xmit = dummysw_xmit,
+   .ndo_swdev_flow_insert  = dummysw_flow_insert,
+   .ndo_swdev_flow_remove  = dummysw_flow_remove,
+};
+
+static void dummysw_setup(struct net_device *dev)
+{
+   ether_setup(dev);
+
+   /* Initialize the device structure. */
+   dev->netdev_ops = &dummysw_netdev_ops;
+   dev->destructor = free_netdev;
+
+   /* Fill in device structure with ethernet-generic values. */
+   dev->tx_queue_len = 0;
+   dev->flags |= IFF_NOARP;
+   dev->flags &= ~IFF_MULTICAST;
+   dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+   dev->features   |= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO;
+   dev->features   |= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
+   eth_hw_addr_random(dev);
+}
+
+static int dummysw_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+   if (tb[IFLA_ADDRESS]) {
+   if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+   return -EINVAL;
+   if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+   return -EADDRNOTAVAIL;
+   }
+   return 0;
+}
+
+static int dummysw_newlink(struct net *src_net, struct net_device *dev,
+  struct nlattr *tb[], struct nlattr *data[])
+{
+   int err;
+
+   err = register_netdevice(dev);
+   if (err)
+   return err;
+   err = __swdev_register(dev);
+   if (err)
+   goto err_swdev_register;
+   netif_carrier_on(dev);
+   return 0;
+
+err_swdev_register:
+   unregister_netdevice(dev);
+   return err;
+}
+
+static struct rtnl_link_ops dummysw_link_ops __read_mostly = {
+   .kind   = "dummysw",
+   .setup  = dummysw_setup,
+   .validate   = dummysw_validate,
+   .newlink= dummysw_newlink,
+};
+
+
+/*
+ * Dummy switch port
+ */
+
+static void dummyswport_uninit(struct net_device *dev)
+{
+   __swportdev_unregister(dev);
+}
+
+static netdev_tx_t dummyswport_xmit(struct sk_buff *skb, struct net_device 
*dev)
+{
+   dev_kfree_skb(skb);
+   return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops dummyswport_netdev_ops = {
+   .ndo_uninit = dummyswport_uninit,
+   .ndo_start_xmit = dummyswport_xmit,
+};
+
+static void dummyswport_setup(struct net_device *dev)
+{
+   

[ovs-dev] [patch net-next RFC v2 4/6] net: introduce switchdev API

2014-03-26 Thread Jiri Pirko
switchdev API is designed to allow kernel support for various switch
chips.

It is the responsibility of a driver to create netdevice instances which
represents every port and for the switch master itself. Driver uses
swdev_register and swportdev_register functions to make the core aware
of the fact these netdevices are representing switch and switch ports.

Signed-off-by: Jiri Pirko 
---
 include/linux/netdevice.h |  24 +++
 include/linux/switchdev.h |  30 
 net/Kconfig   |  10 +++
 net/core/Makefile |   1 +
 net/core/switchdev.c  | 172 ++
 5 files changed, 237 insertions(+)
 create mode 100644 include/linux/switchdev.h
 create mode 100644 net/core/switchdev.c

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6b70e6f..5ad80da 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -49,6 +49,8 @@
 
 #include 
 #include 
+#include 
+
 #include 
 
 struct netpoll_info;
@@ -998,6 +1000,18 @@ typedef u16 (*select_queue_fallback_t)(struct net_device 
*dev,
  * Callback to use for xmit over the accelerated station. This
  * is used in place of ndo_start_xmit on accelerated net
  * devices.
+ *
+ * int (*ndo_swdev_flow_insert)(struct net_device *dev,
+ * struct sw_flow *flow);
+ * Called to insert a flow into switch device. If driver does
+ * not implement this, it is assumed that the hw does not have
+ * a capability to work with flows.
+ *
+ * int (*ndo_swdev_flow_remove)(struct net_device *dev,
+ * struct sw_flow *flow);
+ * Called to remove a flow from switch device. If driver does
+ * not implement this, it is assumed that the hw does not have
+ * a capability to work with flows.
  */
 struct net_device_ops {
int (*ndo_init)(struct net_device *dev);
@@ -1145,6 +1159,12 @@ struct net_device_ops {
netdev_tx_t (*ndo_dfwd_start_xmit) (struct sk_buff *skb,
struct net_device *dev,
void *priv);
+#if IS_ENABLED(CONFIG_NET_SWITCHDEV)
+   int (*ndo_swdev_flow_insert)(struct net_device *dev,
+struct sw_flow *flow);
+   int (*ndo_swdev_flow_remove)(struct net_device *dev,
+struct sw_flow *flow);
+#endif
 };
 
 /**
@@ -1205,6 +1225,8 @@ enum netdev_priv_flags {
IFF_SUPP_NOFCS  = 1<<19,
IFF_LIVE_ADDR_CHANGE= 1<<20,
IFF_MACVLAN = 1<<21,
+   IFF_SWITCH  = 1<<22,
+   IFF_SWITCH_PORT = 1<<23,
 };
 
 #define IFF_802_1Q_VLANIFF_802_1Q_VLAN
@@ -1229,6 +1251,8 @@ enum netdev_priv_flags {
 #define IFF_SUPP_NOFCS IFF_SUPP_NOFCS
 #define IFF_LIVE_ADDR_CHANGE   IFF_LIVE_ADDR_CHANGE
 #define IFF_MACVLANIFF_MACVLAN
+#define IFF_SWITCH IFF_SWITCH
+#define IFF_SWITCH_PORTIFF_SWITCH_PORT
 
 /*
  * The DEVICE structure.
diff --git a/include/linux/switchdev.h b/include/linux/switchdev.h
new file mode 100644
index 000..e4aeaba
--- /dev/null
+++ b/include/linux/switchdev.h
@@ -0,0 +1,30 @@
+/*
+ * include/linux/switchdev.h - Switch device API
+ * Copyright (c) 2014 Jiri Pirko 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#ifndef _LINUX_SWITCHDEV_H_
+#define _LINUX_SWITCHDEV_H_
+
+#include 
+#include 
+
+bool swdev_dev_check(const struct net_device *dev);
+int swdev_flow_insert(struct net_device *dev, struct sw_flow *flow);
+int swdev_flow_remove(struct net_device *dev, struct sw_flow *flow);
+int __swdev_register(struct net_device *dev);
+int swdev_register(struct net_device *dev);
+void __swdev_unregister(struct net_device *dev);
+void swdev_unregister(struct net_device *dev);
+
+bool swportdev_dev_check(const struct net_device *dev);
+int __swportdev_register(struct net_device *port_dev, struct net_device *dev);
+int swportdev_register(struct net_device *port_dev, struct net_device *dev);
+void __swportdev_unregister(struct net_device *port_dev);
+void swportdev_unregister(struct net_device *port_dev);
+
+#endif /* _LINUX_SWITCHDEV_H_ */
diff --git a/net/Kconfig b/net/Kconfig
index e411046..e02ab8d 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -285,6 +285,16 @@ config NET_FLOW_LIMIT
  with many clients some protection against DoS by a single (spoofed)
  flow that greatly exceeds average workload.
 
+config NET_SWITCHDEV
+   tristate "Switch device"

[ovs-dev] [patch net-next RFC v2 2/6] skbuff: add "missed_flow" flag

2014-03-26 Thread Jiri Pirko
This flag sets incoming switch device in order to let know that the flow
which the skb is part of is missed. Listener may react to it
appropriately,

Signed-off-by: Jiri Pirko 
---
 include/linux/filter.h |  1 +
 include/linux/skbuff.h | 13 +
 net/core/filter.c  |  3 +++
 3 files changed, 17 insertions(+)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index e568c8e..38c7c04 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -154,6 +154,7 @@ enum {
BPF_S_ANC_VLAN_TAG,
BPF_S_ANC_VLAN_TAG_PRESENT,
BPF_S_ANC_PAY_OFFSET,
+   BPF_S_ANC_MISSED_FLOW,
 };
 
 #endif /* __LINUX_FILTER_H__ */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 03db95a..0100c2f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -567,6 +567,7 @@ struct sk_buff {
 * headers if needed
 */
__u8encapsulation:1;
+   __u8missed_flow:1;
/* 6/8 bit hole (depending on ndisc_nodetype presence) */
kmemcheck_bitfield_end(flags2);
 
@@ -2993,5 +2994,17 @@ static inline unsigned int skb_gso_network_seglen(const 
struct sk_buff *skb)
   skb_network_header(skb);
return hdr_len + skb_gso_transport_seglen(skb);
 }
+
+/**
+ * skb_mark_missed_flow - marks skb as a part of missed flow
+ * @skb: buffer
+ *
+ * Marks skb as a part of missed flow.
+ */
+static inline void skb_mark_missed_flow(struct sk_buff *skb)
+{
+   skb->missed_flow = 1;
+}
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_SKBUFF_H */
diff --git a/net/core/filter.c b/net/core/filter.c
index ad30d62..048f11a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -350,6 +350,9 @@ load_b:
case BPF_S_ANC_PAY_OFFSET:
A = __skb_get_poff(skb);
continue;
+   case BPF_S_ANC_MISSED_FLOW:
+   A = !!skb->missed_flow;
+   continue;
case BPF_S_ANC_NLATTR: {
struct nlattr *nla;
 
-- 
1.8.5.3

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


[ovs-dev] [patch net-next RFC v2 5/6] openvswitch: Introduce support for switchdev based datapath

2014-03-26 Thread Jiri Pirko
Signed-off-by: Jiri Pirko 
---
 include/uapi/linux/openvswitch.h   |   4 +
 net/openvswitch/Makefile   |   4 +
 net/openvswitch/datapath.c |  45 ++-
 net/openvswitch/datapath.h |   8 ++
 net/openvswitch/dp_notify.c|   3 +-
 net/openvswitch/vport-internal_switchdev.c | 179 +
 net/openvswitch/vport-internal_switchdev.h |  28 
 net/openvswitch/vport-netdev.c |   4 +-
 net/openvswitch/vport-switchportdev.c  | 205 +
 net/openvswitch/vport-switchportdev.h  |  24 
 net/openvswitch/vport.c|   4 +
 net/openvswitch/vport.h|   2 +
 12 files changed, 504 insertions(+), 6 deletions(-)
 create mode 100644 net/openvswitch/vport-internal_switchdev.c
 create mode 100644 net/openvswitch/vport-internal_switchdev.h
 create mode 100644 net/openvswitch/vport-switchportdev.c
 create mode 100644 net/openvswitch/vport-switchportdev.h

diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 970553c..8df1a49 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -189,6 +189,10 @@ enum ovs_vport_type {
OVS_VPORT_TYPE_INTERNAL, /* network device implemented by datapath */
OVS_VPORT_TYPE_GRE,  /* GRE tunnel. */
OVS_VPORT_TYPE_VXLAN,/* VXLAN tunnel. */
+   OVS_VPORT_TYPE_INTERNAL_SWITCHDEV, /* network device which represents
+ a hardware switch */
+   OVS_VPORT_TYPE_SWITCHPORTDEV, /* network device which represents
+a port of a hardware switch */
__OVS_VPORT_TYPE_MAX
 };
 
diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
index 3591cb5..6e9fb2a 100644
--- a/net/openvswitch/Makefile
+++ b/net/openvswitch/Makefile
@@ -22,3 +22,7 @@ endif
 ifneq ($(CONFIG_OPENVSWITCH_GRE),)
 openvswitch-y += vport-gre.o
 endif
+
+ifneq ($(CONFIG_NET_SWITCHDEV),)
+openvswitch-y += vport-internal_switchdev.o vport-switchportdev.o
+endif
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f229ab6..6056325 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -58,7 +58,9 @@
 #include "flow_table.h"
 #include "flow_netlink.h"
 #include "vport-internal_dev.h"
+#include "vport-internal_switchdev.h"
 #include "vport-netdev.h"
+#include "vport-switchportdev.h"
 
 int ovs_net_id __read_mostly;
 
@@ -124,6 +126,9 @@ static struct datapath *get_dp(struct net *net, int 
dp_ifindex)
dev = dev_get_by_index_rcu(net, dp_ifindex);
if (dev) {
struct vport *vport = ovs_internal_dev_get_vport(dev);
+
+   if (!vport)
+   vport = ovs_internal_swdev_get_vport(dev);
if (vport)
dp = vport->dp;
}
@@ -768,6 +773,19 @@ static struct sk_buff *ovs_flow_cmd_build_info(struct 
ovs_flow *flow,
return skb;
 }
 
+static int ovs_dp_flow_insert(struct datapath *dp, struct sw_flow *flow)
+{
+   if (dp->ops && dp->ops->flow_insert)
+   return dp->ops->flow_insert(dp, flow);
+   return 0;
+}
+
+static void ovs_dp_flow_remove(struct datapath *dp, struct sw_flow *flow)
+{
+   if (dp->ops && dp->ops->flow_remove)
+   dp->ops->flow_remove(dp, flow);
+}
+
 static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 {
struct nlattr **a = info->attrs;
@@ -836,13 +854,15 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, 
struct genl_info *info)
flow->flow.key = masked_key;
flow->flow.unmasked_key = key;
rcu_assign_pointer(flow->sf_acts, acts);
+   acts = NULL;
 
/* Put flow in bucket. */
error = ovs_flow_tbl_insert(&dp->table, flow, &mask);
-   if (error) {
-   acts = NULL;
+   if (error)
goto err_flow_free;
-   }
+   error = ovs_dp_flow_insert(dp, &flow->flow);
+   if (error)
+   goto err_flow_tbl_remove;
 
reply = ovs_flow_cmd_build_info(flow, dp, info, 
OVS_FLOW_CMD_NEW);
} else {
@@ -884,6 +904,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, 
struct genl_info *info)
 0, PTR_ERR(reply));
return 0;
 
+err_flow_tbl_remove:
+   ovs_flow_tbl_remove(&dp->table, flow);
 err_flow_free:
ovs_flow_free(flow, false);
 err_unlock_ovs:
@@ -981,6 +1003,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
goto unlock;
}
 
+   ovs_dp_flow_remove(dp, &flow->flow);
ovs_flow_tbl_remove(&dp->table, flow);
 
err = ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
@@ -1234,7 +1257,10 @@ static int ovs_dp_cmd_new(struc

[ovs-dev] [patch net-next RFC v2 3/6] openvswitch: split flow structures into ovs specific and generic ones

2014-03-26 Thread Jiri Pirko
After this, flow related structures can be used in other code.

Signed-off-by: Jiri Pirko 
---
 include/linux/sw_flow.h| 105 +++
 net/openvswitch/datapath.c |  45 +++
 net/openvswitch/datapath.h |   4 +-
 net/openvswitch/flow.c |  14 ++---
 net/openvswitch/flow.h | 123 +
 net/openvswitch/flow_netlink.c |  24 
 net/openvswitch/flow_netlink.h |   4 +-
 net/openvswitch/flow_table.c   | 100 +
 net/openvswitch/flow_table.h   |  18 +++---
 net/openvswitch/vport-gre.c|   4 +-
 net/openvswitch/vport-vxlan.c  |   2 +-
 net/openvswitch/vport.c|   2 +-
 net/openvswitch/vport.h|   2 +-
 13 files changed, 242 insertions(+), 205 deletions(-)
 create mode 100644 include/linux/sw_flow.h

diff --git a/include/linux/sw_flow.h b/include/linux/sw_flow.h
new file mode 100644
index 000..e7b1ef9
--- /dev/null
+++ b/include/linux/sw_flow.h
@@ -0,0 +1,105 @@
+/*
+ * Copyright (c) 2007-2012 Nicira, Inc.
+ * Copyright (c) 2014 Jiri Pirko 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA
+ */
+
+#ifndef _LINUX_SW_FLOW_H_
+#define _LINUX_SW_FLOW_H_
+
+struct sw_flow_key_ipv4_tunnel {
+   __be64 tun_id;
+   __be32 ipv4_src;
+   __be32 ipv4_dst;
+   __be16 tun_flags;
+   u8   ipv4_tos;
+   u8   ipv4_ttl;
+};
+
+struct sw_flow_key {
+   struct sw_flow_key_ipv4_tunnel tun_key;  /* Encapsulating tunnel key. */
+   struct {
+   u32 priority;   /* Packet QoS priority. */
+   u32 skb_mark;   /* SKB mark. */
+   u16 in_port;/* Input switch port (or DP_MAX_PORTS). 
*/
+   } phy;
+   struct {
+   u8 src[ETH_ALEN];   /* Ethernet source address. */
+   u8 dst[ETH_ALEN];   /* Ethernet destination address. */
+   __be16 tci; /* 0 if no VLAN, VLAN_TAG_PRESENT set 
otherwise. */
+   __be16 type;/* Ethernet frame type. */
+   } eth;
+   struct {
+   u8 proto;   /* IP protocol or lower 8 bits of ARP 
opcode. */
+   u8 tos; /* IP ToS. */
+   u8 ttl; /* IP TTL/hop limit. */
+   u8 frag;/* One of OVS_FRAG_TYPE_*. */
+   } ip;
+   union {
+   struct {
+   struct {
+   __be32 src; /* IP source address. */
+   __be32 dst; /* IP destination address. */
+   } addr;
+   union {
+   struct {
+   __be16 src; /* TCP/UDP/SCTP 
source port. */
+   __be16 dst; /* TCP/UDP/SCTP 
destination port. */
+   __be16 flags;   /* TCP flags. */
+   } tp;
+   struct {
+   u8 sha[ETH_ALEN];   /* ARP source 
hardware address. */
+   u8 tha[ETH_ALEN];   /* ARP target 
hardware address. */
+   } arp;
+   };
+   } ipv4;
+   struct {
+   struct {
+   struct in6_addr src;/* IPv6 source address. 
*/
+   struct in6_addr dst;/* IPv6 destination 
address. */
+   } addr;
+   __be32 label;   /* IPv6 flow label. */
+   struct {
+   __be16 src; /* TCP/UDP/SCTP source 
port. */
+   __be16 dst; /* TCP/UDP/SCTP 
destination port. */
+   __be16 flags;   /* TCP flags. */
+   } tp;
+   struct {
+   struct in6_addr target; /* ND target address. */
+   u8 sll[ETH_ALEN];   /* ND source link layer 
address. */
+   u8 tll[ETH_ALEN];   /* ND target link layer 
address. */
+   } nd;
+  

Re: [ovs-dev] [PATCH] netdev-bsd: compilation fixes

2014-03-26 Thread Ben Pfaff
On Wed, Mar 26, 2014 at 02:26:15PM +0900, YAMAMOTO Takashi wrote:
> > On Wed, Mar 26, 2014 at 10:29:03AM +0900, YAMAMOTO Takashi wrote:
> >> > On Wed, Mar 26, 2014 at 05:13:43AM +0900, YAMAMOTO Takashi wrote:
> >> >> This fixes regressions from commit f7791740
> >> >> ("netdev: Rename netdev_rx to netdev_rxq")
> >> >> and commit df1e5a3b.
> >> >> ("netdev: Extend rx_recv to pass multiple packets.")
> >> >> 
> >> >> Signed-off-by: YAMAMOTO Takashi 
> >> > 
> >> > Applied, thanks.
> >> > 
> >> > By the way, I sent you an invitation to become an Open vSwitch
> >> > committer some time ago, following the process described on the
> >> > website, but I never heard back.  Do you want direct commit rights?
> >> 
> >> i think missed it, sorry.  yes, i'm interested in it.
> >> can you resend?
> > 
> > It's simple.  Following the procedure described at
> > http://openvswitch.org/development/committer-grant-revocation/ the
> > existing committers voted to accept you as a new committer.  So, if you
> > agree to follow the guidelines at
> > http://openvswitch.org/development/committer-responsibilities/ we'll
> > grant you commit access.  I think those guidelines are
> > straightforward--they only describe what we have done for a long time
> > anyway--but this is a new process, so they're also somewhat open to
> > discussion.
> 
> thanks.
> the guidelines seem reasonable.  i agree to follow.

Thanks!  I think that to proceed I need an SSH public key and your
preferred username.  After that, I'll ask the right person to get it set
up.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [patch net-next RFC v2 2/6] skbuff: add "missed_flow" flag

2014-03-26 Thread Alexei Starovoitov
On Wed, Mar 26, 2014 at 9:31 AM, Jiri Pirko  wrote:
> This flag sets incoming switch device in order to let know that the flow
> which the skb is part of is missed. Listener may react to it
> appropriately,
>
> Signed-off-by: Jiri Pirko 
> ---
>  include/linux/filter.h |  1 +
>  include/linux/skbuff.h | 13 +
>  net/core/filter.c  |  3 +++
>  3 files changed, 17 insertions(+)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index e568c8e..38c7c04 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -154,6 +154,7 @@ enum {
> BPF_S_ANC_VLAN_TAG,
> BPF_S_ANC_VLAN_TAG_PRESENT,
> BPF_S_ANC_PAY_OFFSET,
> +   BPF_S_ANC_MISSED_FLOW,

Interesting use case for bpf...
looks like it's not connected to userspace.
Are you trying to extend ovs with switchdev api and bpf at once?
How do you plan to use this bpf extension?

Thanks
Alexei

>  };
>
>  #endif /* __LINUX_FILTER_H__ */
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 03db95a..0100c2f 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -567,6 +567,7 @@ struct sk_buff {
>  * headers if needed
>  */
> __u8encapsulation:1;
> +   __u8missed_flow:1;
> /* 6/8 bit hole (depending on ndisc_nodetype presence) */
> kmemcheck_bitfield_end(flags2);
>
> @@ -2993,5 +2994,17 @@ static inline unsigned int 
> skb_gso_network_seglen(const struct sk_buff *skb)
>skb_network_header(skb);
> return hdr_len + skb_gso_transport_seglen(skb);
>  }
> +
> +/**
> + * skb_mark_missed_flow - marks skb as a part of missed flow
> + * @skb: buffer
> + *
> + * Marks skb as a part of missed flow.
> + */
> +static inline void skb_mark_missed_flow(struct sk_buff *skb)
> +{
> +   skb->missed_flow = 1;
> +}
> +
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_SKBUFF_H */
> diff --git a/net/core/filter.c b/net/core/filter.c
> index ad30d62..048f11a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -350,6 +350,9 @@ load_b:
> case BPF_S_ANC_PAY_OFFSET:
> A = __skb_get_poff(skb);
> continue;
> +   case BPF_S_ANC_MISSED_FLOW:
> +   A = !!skb->missed_flow;
> +   continue;
> case BPF_S_ANC_NLATTR: {
> struct nlattr *nla;
>
> --
> 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 net-next RFC v2 2/6] skbuff: add "missed_flow" flag

2014-03-26 Thread Jiri Pirko
Wed, Mar 26, 2014 at 05:59:12PM CET, alexei.starovoi...@gmail.com wrote:
>On Wed, Mar 26, 2014 at 9:31 AM, Jiri Pirko  wrote:
>> This flag sets incoming switch device in order to let know that the flow
>> which the skb is part of is missed. Listener may react to it
>> appropriately,
>>
>> Signed-off-by: Jiri Pirko 
>> ---
>>  include/linux/filter.h |  1 +
>>  include/linux/skbuff.h | 13 +
>>  net/core/filter.c  |  3 +++
>>  3 files changed, 17 insertions(+)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index e568c8e..38c7c04 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -154,6 +154,7 @@ enum {
>> BPF_S_ANC_VLAN_TAG,
>> BPF_S_ANC_VLAN_TAG_PRESENT,
>> BPF_S_ANC_PAY_OFFSET,
>> +   BPF_S_ANC_MISSED_FLOW,
>
>Interesting use case for bpf...
>looks like it's not connected to userspace.
>Are you trying to extend ovs with switchdev api and bpf at once?
>How do you plan to use this bpf extension?

The motive fot this is to provides a user (userspace app) possibility to
get all skbs which were "flow-missed". He can adjust the filter so that
only these can get through.



>
>Thanks
>Alexei
>
>>  };
>>
>>  #endif /* __LINUX_FILTER_H__ */
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 03db95a..0100c2f 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -567,6 +567,7 @@ struct sk_buff {
>>  * headers if needed
>>  */
>> __u8encapsulation:1;
>> +   __u8missed_flow:1;
>> /* 6/8 bit hole (depending on ndisc_nodetype presence) */
>> kmemcheck_bitfield_end(flags2);
>>
>> @@ -2993,5 +2994,17 @@ static inline unsigned int 
>> skb_gso_network_seglen(const struct sk_buff *skb)
>>skb_network_header(skb);
>> return hdr_len + skb_gso_transport_seglen(skb);
>>  }
>> +
>> +/**
>> + * skb_mark_missed_flow - marks skb as a part of missed flow
>> + * @skb: buffer
>> + *
>> + * Marks skb as a part of missed flow.
>> + */
>> +static inline void skb_mark_missed_flow(struct sk_buff *skb)
>> +{
>> +   skb->missed_flow = 1;
>> +}
>> +
>>  #endif /* __KERNEL__ */
>>  #endif /* _LINUX_SKBUFF_H */
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index ad30d62..048f11a 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -350,6 +350,9 @@ load_b:
>> case BPF_S_ANC_PAY_OFFSET:
>> A = __skb_get_poff(skb);
>> continue;
>> +   case BPF_S_ANC_MISSED_FLOW:
>> +   A = !!skb->missed_flow;
>> +   continue;
>> case BPF_S_ANC_NLATTR: {
>> struct nlattr *nla;
>>
>> --
>> 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] datapath: Add support for kernel 3.13

2014-03-26 Thread Kyle Mestery
On Tue, Mar 25, 2014 at 3:36 PM, Pravin Shelar  wrote:

> On Fri, Mar 21, 2014 at 10:41 AM, Kyle Mestery
>  wrote:
> > Add support for building the in-tree kernel datapath for Linux kernel
> 3.13.
> > There were some changes in the netlink area which required adding new
> > compatibility code for this layer.
> >
> > Signed-off-by: Kyle Mestery 
> > ---
> >  FAQ   |  2 +-
> >  acinclude.m4  |  4 +--
> >  datapath/datapath.c   | 27 +++-
> >  datapath/datapath.h   |  1 +
> >  datapath/dp_notify.c  | 11 +++
> >  datapath/linux/compat/genetlink-openvswitch.c | 20 +---
> >  datapath/linux/compat/include/net/genetlink.h | 45
> +--
> >  datapath/linux/compat/include/net/ip.h|  4 +++
> >  datapath/linux/compat/utils.c |  3 ++
> >  datapath/vport-lisp.c |  7 +++--
> >  datapath/vport-vxlan.c|  3 +-
> >  11 files changed, 100 insertions(+), 27 deletions(-)
> >
> > diff --git a/FAQ b/FAQ
> > index a54bbf9..040cdf7 100644
> > --- a/FAQ
> > +++ b/FAQ
> > @@ -148,7 +148,7 @@ A: The following table lists the Linux kernel
> versions against which the
> > 1.10.x 2.6.18 to 3.8
> > 1.11.x 2.6.18 to 3.8
> > 2.0.x  2.6.32 to 3.10
> > -   2.1.x  2.6.32 to 3.12
> > +   2.1.x  2.6.32 to 3.13
> >
> > Open vSwitch userspace should also work with the Linux kernel module
> > built into Linux 3.3 and later.
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 8f41e33..bd8714c 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
> >  AC_MSG_RESULT([$kversion])
> >
> >  if test "$version" -ge 3; then
> > -   if test "$version" = 3 && test "$patchlevel" -le 12; then
> > +   if test "$version" = 3 && test "$patchlevel" -le 13; then
> >: # Linux 3.x
> > else
> > - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.12.x is not supported])
> > + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.13.x is not supported])
> > fi
> >  else
> > if test "$version" -le 1 || test "$patchlevel" -le 5 || test
> "$sublevel" -le 31; then
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index f7c3391..b4f09b7 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -64,11 +64,13 @@
> >
> >  int ovs_net_id __read_mostly;
> >
> > +static struct genl_family dp_packet_genl_family;
> > +
> >  static void ovs_notify(struct sk_buff *skb, struct genl_info *info,
> >struct genl_multicast_group *grp)
> >  {
> > -   genl_notify(skb, genl_info_net(info), info->snd_portid,
> > -   grp->id, info->nlhdr, GFP_KERNEL);
> > +   genl_notify(&dp_packet_genl_family, skb, genl_info_net(info),
> > +   info->snd_portid, 0, info->nlhdr, GFP_KERNEL);
> >  }
> >
> >  /**
> > @@ -883,8 +885,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff
> *skb, struct genl_info *info)
> > if (!IS_ERR(reply))
> > ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
> > else
> > -   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -   ovs_dp_flow_multicast_group.id,
> PTR_ERR(reply));
> > +   genl_set_err(&dp_flow_genl_family, sock_net(skb->sk), 0,
> > +0, PTR_ERR(reply));
> > return 0;
> >
> >  err_flow_free:
> > @@ -1365,8 +1367,8 @@ static int ovs_dp_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> > reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
> > if (IS_ERR(reply)) {
> > err = PTR_ERR(reply);
> > -   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -   ovs_dp_datapath_multicast_group.id,
> err);
> > +   genl_set_err(&dp_datapath_genl_family,
> sock_net(skb->sk), 0,
> > +0, err);
> > err = 0;
> > goto unlock;
> > }
> > @@ -1463,7 +1465,7 @@ static const struct nla_policy
> vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
> > [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
> >  };
> >
> > -static struct genl_family dp_vport_genl_family = {
> > +struct genl_family dp_vport_genl_family = {
> > .id = GENL_ID_GENERATE,
> > .hdrsize = sizeof(struct ovs_header),
> > .name = OVS_VPORT_FAMILY,
> > @@ -1862,6 +1864,7 @@ static int dp_register_genl(void)
> > for (i = 0; i < ARRAY_SIZE(dp_genl_families); i++) {
> > const struct genl_family_and_ops *f =
> &dp_genl_families[i];
> >
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> > er

Re: [ovs-dev] [PATCH V2 09/10] dpif-netdev: Add DPDK netdev.

2014-03-26 Thread Ed Maste
On 21 March 2014 14:03, Pravin  wrote:
> Following patch adds DPDK netdev-class to userspace datapath. Now
> OVS can use DPDK port for IO by just configuring DPDK port and then
> adding dpdk type port to userspace datapath.

Intel recently released DPDK also for FreeBSD, so it may be worth
referring explicitly to Linux DPDK in INSTALL.DPDK (until we can port
/ test it on FreeBSD).

Most of the existing DPDK docs also assume Linux, but I assume this
will be updated over time.  The FreeBSD DPDK getting started guide is
here:
http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/intel-dpdk-freebsd-getting-started-guide.html

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


Re: [ovs-dev] [PATCH v4] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors

2014-03-26 Thread David Miller
From: Zoltan Kiss 
Date: Fri, 21 Mar 2014 10:31:34 +

> skb_zerocopy can copy elements of the frags array between skbs, but it doesn't
> orphan them. Also, it doesn't handle errors, so this patch takes care of that
> as well, and modify the callers accordingly. skb_tx_error() is also added to
> the callers so they will signal the failed delivery towards the creator of the
> skb.
> 
> Signed-off-by: Zoltan Kiss 

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


Re: [ovs-dev] [PATCH v4] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors

2014-03-26 Thread David Miller
From: David Miller 
Date: Wed, 26 Mar 2014 15:59:58 -0400 (EDT)

> From: Zoltan Kiss 
> Date: Fri, 21 Mar 2014 10:31:34 +
> 
>> skb_zerocopy can copy elements of the frags array between skbs, but it 
>> doesn't
>> orphan them. Also, it doesn't handle errors, so this patch takes care of that
>> as well, and modify the callers accordingly. skb_tx_error() is also added to
>> the callers so they will signal the failed delivery towards the creator of 
>> the
>> skb.
>> 
>> Signed-off-by: Zoltan Kiss 
> 
> Applied, thanks Zoltan.

Actually, Zoltan, you have to fix this:

net/core/skbuff.c: In function ‘skb_zerocopy’:
net/core/skbuff.c:2172:2: warning: passing argument 1 of ‘skb_orphan_frags’ 
discards ‘const’ qualifi
er from pointer target type [enabled by default]
In file included from include/linux/tcp.h:21:0,
 from net/core/skbuff.c:50:
include/linux/skbuff.h:1904:19: note: expected ‘struct sk_buff *’ but argument 
is of type ‘const str
uct sk_buff *’
net/core/skbuff.c:2173:3: warning: passing argument 1 of ‘skb_tx_error’ 
discards ‘const’ qualifier f
rom pointer target type [enabled by default]
net/core/skbuff.c:642:6: note: expected ‘struct sk_buff *’ but argument is of 
type ‘const struct sk_
buff *’
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] hackathon idea list

2014-03-26 Thread Ansis Atteka
Here is another idea:

Use strongSwan to negotiate IPsec_* tunnels

Currently ovs-monitor-ipsec uses racoon to negotiate IPsec flavored
tunnels. Besides racoon, there are also some other IPsec keying
daemons (e.g. strongSwan, racoon2, openSwan).

The main cause to switch to strongSwan would be to leverage
aes-gcm cipher (currently, racoon limits us to aes-cbc). aes-gcm would
allow OVS to achieve much better performance when tunneling packets.

On Tue, Mar 25, 2014 at 10:11 PM, Ben Pfaff  wrote:
> Anyone is welcome to work on anything relevant to Open vSwitch at the
> hackathon.  For those who are looking for something to do, here is a
> list of specific project ideas.  I will bring this list, broken down
> into individual printed items, to Cisco with me in the morning, so
> consider this a preview to scan through and provoke thoughts if you
> happen to read it before the hackathon begins.
>
> Most of this list is from me, with contributions from others, so feel
> free to direct questions toward me.
>
>
> Programming Project Ideas
> =
>
> Each of these projects would ideally result in a patch or a short
> series of them posted to ovs-dev.
>
> Please read SubmittingPatches and CodingStyle in the top of the source
> tree before you begin work.  The OPENFLOW-1.1+ file also has an
> introduction to how OpenFlow is implemented in Open vSwitch. It is
> also a good idea to look around the source tree for related code, and
> back through the Git history for commits on related subjects, to allow
> you to follow existing patterns and conventions.
>
> Improve linking speed
> -
>
> Changing one of the files in the Open vSwitch ``lib'' directory
> causes 43 binaries to be relinked, which takes a lot of time even with
> parallel ``make''.  31 of those binaries are in the ``tests''
> directory.  This project would combine most of those binaries into a
> single test program that just takes a subcommand name as its first
> command-line argument, because linking one larger program or even half
> a dozen would be much faster than linking 31 small programs.  (Some of
> the test programs already take a subcommand name.)  One would not want
> to combine the source code for all the programs into a single C file,
> however.
>
> Meters
> --
>
> Open vSwitch has OpenFlow protocol support for meters, but it does not
> have an implementation in the kernel or userspace datapaths.  An
> implementation was proposed some time ago (I recommend looking for the
> discussion in the ovs-dev mailing list archives), but for a few
> different reasons it was not accepted.  Some of those reasons apply
> only to a kernel implementation of meters.  At the time, a userspace
> implementation wasn't as interesting, because the userspace switch
> did not perform at a production speed, but with the advent of
> multithreaded forwarding and, now, DPDK support, userspace-only meters
> would be a great way to get started.
>
> This project might take longer than two days.
>
> Improve SSL/TLS Security
> 
>
> Open vSwitch allows some weak ciphers to be used for its secure
> connections.  Security audits often suggest that the project remove
> those ciphers, but there's not a clean way to modify the acceptable
> ciphers.  At the very least, the cipher list should be audited, but it
> would be nice to make it configurable.
>
> Open vSwitch does not insist on perfect forward security via ephemeral
> Diffie-Hellman key exchange when it establishes an SSL/TLS connection.
> Given the wiretapping revelations over the last year, it seems wise to
> turn this on.  (This would probably amount to finding the right
> OpenSSL function to call or just reducing the acceptable ciphers
> further.)
>
> These changes might have backward-compatibility implications; one
> would have to test the behavior of the reduced cipher list OVS against
> older versions.
>
> Improve ovs-vsctl error reporting
> -
>
> ovs-vsctl is a command-line interface to the Open vSwitch database,
> and as such it just modifies the desired Open vSwitch configuration in
> the database.  ovs-vswitchd, on the other hand, monitors the database
> and implements the actual configuration specified in the database.
> This can lead to surprises when the user makes a change to the
> database, with ovs-vsctl, that ovs-vswitchd cannot actually
> implement. In such a case, the ovs-vsctl command silently succeeds
> (because the database was successfully updated) but its desired
> effects don't actually take place. One good example of such a change
> is attempting to add a port with a misspelled name (e.g. ``ovs-vsctl
> add-port br0 fth0'', where fth0 should be eth0); another is creating
> a bridge or a port whose name is longer than supported
> (e.g. ``ovs-vsctl add-br'' with a 16-character bridge name on
> Linux). It can take users a long time to realize the err

Re: [ovs-dev] [patch net-next RFC v2 0/6] introduce infrastructure for support of switch chip datapath

2014-03-26 Thread Jamal Hadi Salim

Jiri,

The flow extensions may be distracting - note there are many
tables (L3, L2, etc) in such chips not just ACLs. And there's likely no
OneWay(tm) to add a flow. My view is probably to solve or reach an
agreement on the ports. Then resolve the different tables control/data
exposure.
On the switchdev - You are still exposing it; do you expect these
things to be created from user space? Probably thats one approach, but
I would suspect the majority would result in the driver itself creating
these devices after discovering the resources from the control
interfaces (PCIE etc).

cheers,
jamal


On 03/26/14 12:31, Jiri Pirko wrote:

This is second version of RFC. Here are the main differences from the first one:
-There is no special swdev of swport structure. The switch and its ports are
  now represented only by net_device structures. There are couple of 
switch-specific
  ndos added (inserting and removing flows).

-Regarding the flows, driver marks skb with "missing flow" flag now. That would
  give indication to a user (OVS datapath of af_packet userspace application).
  On the opposite direction, skb can be xmitted by a port.

-dummyswitch module has now rtnetlink iface for easy creation of dummy switches
  and ports.

The basic idea is to introduce a generic infractructure to support various
switch chips in kernel. Also the idea is to benefit of currently existing
Open vSwitch userspace infrastructure.


The first two patches are just minor skb flag and packet_type modifications.


The third patch does a split of structures which are not specific to OVS
into more generic ones that can be reused.


The fourth patch introduces the "switchdev" API itself. It should serve as
a glue between chip drivers on the one side and the user on the other.
That user might be OVS datapath but in future it might be just userspace
application interacting via af_packet and Netlink iface.

The infrastructure is designed to be similar to for example linux bridge.
There is one netdevice representing a switch chip and one netdevice per every
port. These are bound together in classic slave-master way. The reason
to reuse the netdevices for port representation is that userspace can use
standard tools to get information about the ports, statistics, tcpdump, etc.

Note that the netdevices are just representations of the ports in the switch.
Therefore **no actual data** goes though, only missed flow skbs and, if drivers
supports it, when ETH_P_ALL packet_type is hooked on (tcpdump).


The fifth patch introduces a support for switchdev vports into OVS datapath.
After that, userspace would be able to create a switchdev DP for a switch chip,
to add switchdev ports to it and to use it in the same way as it would be
OVS SW datapath.


The sixth patch adds a dummy switch module. It is just an example of
switchdev driver implementation.


Jiri Pirko (6):
   net: make packet_type->ak_packet_priv generic
   skbuff: add "missed_flow" flag
   openvswitch: split flow structures into ovs specific and generic ones
   net: introduce switchdev API
   openvswitch: Introduce support for switchdev based datapath
   net: introduce dummy switch

  drivers/net/Kconfig|   7 +
  drivers/net/Makefile   |   1 +
  drivers/net/dummyswitch.c  | 235 +
  include/linux/filter.h |   1 +
  include/linux/netdevice.h  |  26 +++-
  include/linux/skbuff.h |  13 ++
  include/linux/sw_flow.h| 105 +
  include/linux/switchdev.h  |  30 
  include/uapi/linux/if_link.h   |   9 ++
  include/uapi/linux/openvswitch.h   |   4 +
  net/Kconfig|  10 ++
  net/core/Makefile  |   1 +
  net/core/dev.c |   4 +-
  net/core/filter.c  |   3 +
  net/core/switchdev.c   | 172 +
  net/openvswitch/Makefile   |   4 +
  net/openvswitch/datapath.c |  90 +++
  net/openvswitch/datapath.h |  12 +-
  net/openvswitch/dp_notify.c|   3 +-
  net/openvswitch/flow.c |  14 +-
  net/openvswitch/flow.h | 123 +++
  net/openvswitch/flow_netlink.c |  24 +--
  net/openvswitch/flow_netlink.h |   4 +-
  net/openvswitch/flow_table.c   | 100 ++--
  net/openvswitch/flow_table.h   |  18 +--
  net/openvswitch/vport-gre.c|   4 +-
  net/openvswitch/vport-internal_switchdev.c | 179 ++
  net/openvswitch/vport-internal_switchdev.h |  28 
  net/openvswitch/vport-netdev.c |   4 +-
  net/openvswitch/vport-switchportdev.c  | 205 +
  net/openvswitch/vport-switchportdev.h  |  24 +++
  net/openvswitch/vport-vxlan.c  

Re: [ovs-dev] [patch net-next RFC v2 0/6] introduce infrastructure for support of switch chip datapath

2014-03-26 Thread Florian Fainelli
2014-03-26 14:44 GMT-07:00 Jamal Hadi Salim :
> Jiri,
>
> The flow extensions may be distracting - note there are many
> tables (L3, L2, etc) in such chips not just ACLs. And there's likely no
> OneWay(tm) to add a flow. My view is probably to solve or reach an
> agreement on the ports. Then resolve the different tables control/data
> exposure.

Agreed.

> On the switchdev - You are still exposing it; do you expect these
> things to be created from user space? Probably thats one approach, but
> I would suspect the majority would result in the driver itself creating
> these devices after discovering the resources from the control
> interfaces (PCIE etc).

It seems to me like, minus the strong MDIO dependency, DSA is probably
the closest and most ready piece of software we have in the kernel to
start building Ethernet switch port net_device as it already contains
pretty much everything we want:

- per-port ethtool operations
- per-port xmit/rcv handlers
- existing drivers

The missing bits are roughly:

- adding IFF_SWITCH_PORT flags to the slave net_device created
- creating the switch master net_device: sw1
- creating the Switch CPU port net_device: sw1p

>
> cheers,
> jamal
>
>
>
> On 03/26/14 12:31, Jiri Pirko wrote:
>>
>> This is second version of RFC. Here are the main differences from the
>> first one:
>> -There is no special swdev of swport structure. The switch and its ports
>> are
>>   now represented only by net_device structures. There are couple of
>> switch-specific
>>   ndos added (inserting and removing flows).
>>
>> -Regarding the flows, driver marks skb with "missing flow" flag now. That
>> would
>>   give indication to a user (OVS datapath of af_packet userspace
>> application).
>>   On the opposite direction, skb can be xmitted by a port.
>>
>> -dummyswitch module has now rtnetlink iface for easy creation of dummy
>> switches
>>   and ports.
>>
>> The basic idea is to introduce a generic infractructure to support various
>> switch chips in kernel. Also the idea is to benefit of currently existing
>> Open vSwitch userspace infrastructure.
>>
>>
>> The first two patches are just minor skb flag and packet_type
>> modifications.
>>
>>
>> The third patch does a split of structures which are not specific to OVS
>> into more generic ones that can be reused.
>>
>>
>> The fourth patch introduces the "switchdev" API itself. It should serve as
>> a glue between chip drivers on the one side and the user on the other.
>> That user might be OVS datapath but in future it might be just userspace
>> application interacting via af_packet and Netlink iface.
>>
>> The infrastructure is designed to be similar to for example linux bridge.
>> There is one netdevice representing a switch chip and one netdevice per
>> every
>> port. These are bound together in classic slave-master way. The reason
>> to reuse the netdevices for port representation is that userspace can use
>> standard tools to get information about the ports, statistics, tcpdump,
>> etc.
>>
>> Note that the netdevices are just representations of the ports in the
>> switch.
>> Therefore **no actual data** goes though, only missed flow skbs and, if
>> drivers
>> supports it, when ETH_P_ALL packet_type is hooked on (tcpdump).
>>
>>
>> The fifth patch introduces a support for switchdev vports into OVS
>> datapath.
>> After that, userspace would be able to create a switchdev DP for a switch
>> chip,
>> to add switchdev ports to it and to use it in the same way as it would be
>> OVS SW datapath.
>>
>>
>> The sixth patch adds a dummy switch module. It is just an example of
>> switchdev driver implementation.
>>
>>
>> Jiri Pirko (6):
>>net: make packet_type->ak_packet_priv generic
>>skbuff: add "missed_flow" flag
>>openvswitch: split flow structures into ovs specific and generic ones
>>net: introduce switchdev API
>>openvswitch: Introduce support for switchdev based datapath
>>net: introduce dummy switch
>>
>>   drivers/net/Kconfig|   7 +
>>   drivers/net/Makefile   |   1 +
>>   drivers/net/dummyswitch.c  | 235
>> +
>>   include/linux/filter.h |   1 +
>>   include/linux/netdevice.h  |  26 +++-
>>   include/linux/skbuff.h |  13 ++
>>   include/linux/sw_flow.h| 105 +
>>   include/linux/switchdev.h  |  30 
>>   include/uapi/linux/if_link.h   |   9 ++
>>   include/uapi/linux/openvswitch.h   |   4 +
>>   net/Kconfig|  10 ++
>>   net/core/Makefile  |   1 +
>>   net/core/dev.c |   4 +-
>>   net/core/filter.c  |   3 +
>>   net/core/switchdev.c   | 172 +
>>   net/openvswitch/Makefile   |   4 +
>>   net/openvswitch/datapath.c |  90 +++
>>   net/

Re: [ovs-dev] [BUG]: kernel crash when use gre vport of openvswitch!

2014-03-26 Thread Jesse Gross
On Tue, Mar 25, 2014 at 7:28 PM, wei zhang  wrote:
> Hi
>
> When I use gre vport of openvswitch on the Centos6.4, kernel crashed!
> the crash log is attached at the last.
>
> I found out that openvswitch register a gre_cisco_protocol but
> does not supply a err_handler with it. The gre_cisco_err() call the
> err_handler without existence check, cause the kernel crash.
>
> I send a patch which adding the existence check, this is the reply of David 
> Miller:
>
>>Rather, openvswitch should provide an appropriate ->err_handler() that
>>returns PACKET_RCVD or PACKET_REJECT.
>
> So maybe openvswitch need supply a dummy err_handler() ?

That sounds fine to me. Can you write a patch to do this?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] Null Pointer / Kernel Panic

2014-03-26 Thread Jesse Gross
On Tue, Feb 25, 2014 at 12:56 PM, Sean Swehla  wrote:
> Hello,
>
> I'm currently hitting a null pointer dereference and kernel panic that seems
> to be in ovs. The problem is sporadic. I have one production machine that's
> hit it four times in the past 24hrs, and one lab machine that I can't get to
> hit it at all.
>
> We rebuilt openvswitch with debugging symbols turned on, and traced the null
> pointer dereference to datapath/linux/flow.c:814 . Do you have any advice on
> how to trace this back to a root cause (or, ideally, a fix) ? I've scoured
> Google for related issues but come up short. (I'll happily accept that my
> google-fu is lacking, though.)
>
> I would greatly appreciate any guidance you could offer. Here's some more
> information about my system, for context.

This looks suspiciously similar to the problem addressed by this
commit. It doesn't apply to OVS 1.10, so your kernel might have the
issue mentioned here.

commit a98ea90e1e86fc7f2bbdc6afd4ad9dc78a06e889
Author: Pravin B Shelar 
Date:   Fri Jul 26 13:52:24 2013 -0700

datapath: list: Fix double fetch of pointer in hlist_entry_safe()

Following patch backports commit f65846a1800ef8c48d (list: Fix double
fetch of pointer in hlist_entry_safe()) from upstream kernel.
This patch fixes following panic. Thanks to Jesse for helping to
debug this issue.

BUG: unable to handle kernel NULL pointer dereference at 0118
[129608.216422] IP: [] ovs_masked_flow_lookup+0xda/0x140 [
[129608.216918] PGD 11500a067 PUD 120851067 PMD 0
[129608.216994] Oops:  [#1] SMP
[129608.217049] CPU 0
[129608.218697]
[129608.218726] Pid: 0, comm: swapper/0 Tainted: G   O
3.2.39-server-nn21 #1 VMware, Inc. VMware Virtual Platform/440BX Desktop
Reference Platform
[129608.219288] RIP: 0010:[]  []
ovs_masked_flow_lookup+0xda/0x140 [openvswitch]
[129608.219454] RSP: 0018:88013fc03b60  EFLAGS: 00010282
[129608.219536] RAX: 0020 RBX: 880123087100 RCX:
88012098e000
[129608.219719] RDX: 8800b3b0ca30 RSI: 010a RDI:
88011df8c000
[129608.220121] RBP: 88013fc03c30 R08: 0001 R09:
20069825
[129608.220287] R10: 0020 R11: 0001 R12:
880036e1c6c0
[129608.220451] R13: 88013fc03b98 R14: 0024 R15:
ffe0
[129608.220618] FS:  () GS:88013fc0()
knlGS:
[129608.220794] CS:  0010 DS:  ES:  CR0: 8005003b
[129608.220911] CR2: 0118 CR3: 0001190c9000 CR4:
000406f0
[129608.221122] DR0:  DR1:  DR2:

[129608.221320] DR3:  DR6: 0ff0 DR7:
0400
[129608.221488] Process swapper/0 (pid: 0, threadinfo 81c0,
task 81c0d020)
[129608.221669] Stack:
[129608.221725]  0044 0020 88013fc03c60

[129608.221906]    
f01423220002
[129608.222069]  1973f0142322015a 00060008 1973f0140579f414
2f1dc7ec
[129608.11] Call Trace:
[129608.64]  
[129608.222316]  [] ovs_flow_lookup+0x5d/0x70
[openvswitch]
[129608.222411]  [] ovs_dp_process_received_packet+0x70/0x
[129608.222541]  [] ? resched_task+0x2c/0x80
[129608.222644]  [] ? netdev_create+0x120/0x120
[openvswitch]
[129608.222743]  [] ovs_vport_receive+0x38/0x40
[openvswitch]
[129608.222838]  [] netdev_frame_hook+0xa3/0xf0
[openvswitch]
[129608.222933]  [] ? netdev_create+0x120/0x120
[openvswitch]
[129608.223029]  [] __netif_receive_skb+0x1c8/0x620
[129608.223114]  [] ? map_single+0x60/0x60
[129608.223192]  [] process_backlog+0xb1/0x190
[129608.223274]  [] net_rx_action+0x134/0x290
[129608.223355]  [] __do_softirq+0xa8/0x210
[129608.223433]  [] ? _raw_spin_lock+0xe/0x20
[129608.223513]  [] call_softirq+0x1c/0x30
[129608.223590]  [] do_softirq+0x65/0xa0
[129608.223665]  [] irq_exit+0x8e/0xb0
[129608.223738]  [] do_IRQ+0x63/0xe0
[129608.223808]  [] common_interrupt+0x6e/0x6e
[129608.223887]  
[129608.223933]  [] ? native_safe_halt+0x6/0x10
[129608.224014]  [] default_idle+0x53/0x1d0
[129608.224092]  [] cpu_idle+0xd6/0x120
[129608.224167]  [] rest_init+0x72/0x74
[129608.224252]  [] start_kernel+0x3b5/0x3c2
[129608.224331]  []
x86_64_start_reservations+0x132/0x136
[129608.224421]  [] ? early_idt_handlers+0x140/0x140
[129608.224506]  [] x86_64_start_kernel+0x102/0x111
[129608.224589] Code: 25 48 63 53 28 48 8d 42 01 48 c1 e0 04 49 01 c7 49
8b 07 48 85 c0 74 61 4d 8b 3f 48 c1 e2 04 48 83 c2 10 49 29 d7 4d 85 ff
74 26 <4d> 39 a7 38 01 00 00 75 cd 48 8b 95 38 ff ff ff 4c 89 ee 49 8d
[129608.224949] RIP  [] o

[ovs-dev] [PATCH v5] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors

2014-03-26 Thread Zoltan Kiss
skb_zerocopy can copy elements of the frags array between skbs, but it doesn't
orphan them. Also, it doesn't handle errors, so this patch takes care of that
as well, and modify the callers accordingly. skb_tx_error() is also added to
the callers so they will signal the failed delivery towards the creator of the
skb.

Signed-off-by: Zoltan Kiss 
---
v2: orphan the frags right before touching the frags

v3:
- orphan 'from' instead of 'to'
- call skb_tx_error() in the callers if something went wrong

v4: correctly use error path in queue_userspace_packet

v5: remove const qualifier of "from" in skb_zerocopy parameters

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 03db95a..0db91fa 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2508,8 +2508,8 @@ int skb_splice_bits(struct sk_buff *skb, unsigned int 
offset,
unsigned int flags);
 void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to);
 unsigned int skb_zerocopy_headlen(const struct sk_buff *from);
-void skb_zerocopy(struct sk_buff *to, const struct sk_buff *from,
- int len, int hlen);
+int skb_zerocopy(struct sk_buff *to, struct sk_buff *from,
+int len, int hlen);
 void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len);
 int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen);
 void skb_scrub_packet(struct sk_buff *skb, bool xnet);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3f14c63..908ad98 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2127,25 +2127,31 @@ EXPORT_SYMBOL_GPL(skb_zerocopy_headlen);
  *
  * The `hlen` as calculated by skb_zerocopy_headlen() specifies the
  * headroom in the `to` buffer.
+ *
+ * Return value:
+ * 0: everything is OK
+ * -ENOMEM: couldn't orphan frags of @from due to lack of memory
+ * -EFAULT: skb_copy_bits() found some problem with skb geometry
  */
-void
-skb_zerocopy(struct sk_buff *to, const struct sk_buff *from, int len, int hlen)
+int
+skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
 {
int i, j = 0;
int plen = 0; /* length of skb->head fragment */
+   int ret;
struct page *page;
unsigned int offset;
 
BUG_ON(!from->head_frag && !hlen);
 
/* dont bother with small payloads */
-   if (len <= skb_tailroom(to)) {
-   skb_copy_bits(from, 0, skb_put(to, len), len);
-   return;
-   }
+   if (len <= skb_tailroom(to))
+   return skb_copy_bits(from, 0, skb_put(to, len), len);
 
if (hlen) {
-   skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+   ret = skb_copy_bits(from, 0, skb_put(to, hlen), hlen);
+   if (unlikely(ret))
+   return ret;
len -= hlen;
} else {
plen = min_t(int, skb_headlen(from), len);
@@ -2163,6 +2169,11 @@ skb_zerocopy(struct sk_buff *to, const struct sk_buff 
*from, int len, int hlen)
to->len += len + plen;
to->data_len += len + plen;
 
+   if (unlikely(skb_orphan_frags(from, GFP_ATOMIC))) {
+   skb_tx_error(from);
+   return -ENOMEM;
+   }
+
for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
if (!len)
break;
@@ -2173,6 +2184,8 @@ skb_zerocopy(struct sk_buff *to, const struct sk_buff 
*from, int len, int hlen)
j++;
}
skb_shinfo(to)->nr_frags = j;
+
+   return 0;
 }
 EXPORT_SYMBOL_GPL(skb_zerocopy);
 
diff --git a/net/netfilter/nfnetlink_queue_core.c 
b/net/netfilter/nfnetlink_queue_core.c
index f072fe8..108120f 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -354,13 +354,16 @@ nfqnl_build_packet_message(struct net *net, struct 
nfqnl_instance *queue,
 
skb = nfnetlink_alloc_skb(net, size, queue->peer_portid,
  GFP_ATOMIC);
-   if (!skb)
+   if (!skb) {
+   skb_tx_error(entskb);
return NULL;
+   }
 
nlh = nlmsg_put(skb, 0, 0,
NFNL_SUBSYS_QUEUE << 8 | NFQNL_MSG_PACKET,
sizeof(struct nfgenmsg), 0);
if (!nlh) {
+   skb_tx_error(entskb);
kfree_skb(skb);
return NULL;
}
@@ -488,13 +491,15 @@ nfqnl_build_packet_message(struct net *net, struct 
nfqnl_instance *queue,
nla->nla_type = NFQA_PAYLOAD;
nla->nla_len = nla_attr_size(data_len);
 
-   skb_zerocopy(skb, entskb, data_len, hlen);
+   if (skb_zerocopy(skb, entskb, data_len, hlen))
+   goto nla_put_failure;
}
 
nlh->nlmsg_len = skb->len;
return skb;
 
 nla_put_failure:
+   skb_tx_error(entskb);
kfree_skb(skb);
net_err_ratelimited("nf_queue: error creating packet message\n");
  

Re: [ovs-dev] [PATCH v4] core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors

2014-03-26 Thread Zoltan Kiss

On 26/03/14 20:12, David Miller wrote:

From: David Miller 
Date: Wed, 26 Mar 2014 15:59:58 -0400 (EDT)


From: Zoltan Kiss 
Date: Fri, 21 Mar 2014 10:31:34 +


skb_zerocopy can copy elements of the frags array between skbs, but it doesn't
orphan them. Also, it doesn't handle errors, so this patch takes care of that
as well, and modify the callers accordingly. skb_tx_error() is also added to
the callers so they will signal the failed delivery towards the creator of the
skb.

Signed-off-by: Zoltan Kiss 


Applied, thanks Zoltan.


Actually, Zoltan, you have to fix this:

net/core/skbuff.c: In function ‘skb_zerocopy’:
net/core/skbuff.c:2172:2: warning: passing argument 1 of ‘skb_orphan_frags’ 
discards ‘const’ qualifi
er from pointer target type [enabled by default]
In file included from include/linux/tcp.h:21:0,
  from net/core/skbuff.c:50:
include/linux/skbuff.h:1904:19: note: expected ‘struct sk_buff *’ but argument 
is of type ‘const str
uct sk_buff *’
net/core/skbuff.c:2173:3: warning: passing argument 1 of ‘skb_tx_error’ 
discards ‘const’ qualifier f
rom pointer target type [enabled by default]
net/core/skbuff.c:642:6: note: expected ‘struct sk_buff *’ but argument is of 
type ‘const struct sk_
buff *’



Ok, resubmitted. 'from' is now not a const parameter, because 
skb->pfmemalloc might change.


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


Re: [ovs-dev] [PATCH] ofproto-dpif: Remove the flow_dumper thread.

2014-03-26 Thread Joe Stringer
Hey Ethan,

On 6 March 2014 13:33, Joe Stringer  wrote:

>  static void
> -revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
> +revalidate(struct revalidator *revalidator)
>  {
>  struct udpif *udpif = revalidator->udpif;
>
>  struct dump_op ops[REVALIDATE_MAX_BATCH];
> -struct udpif_flow_dump *udump, *next_udump;
> -size_t n_ops, n_flows;
> +const struct nlattr *key, *mask, *actions;
> +size_t key_len, mask_len, actions_len;
> +const struct dpif_flow_stats *stats;
> +long long int now;
>  unsigned int flow_limit;
> -long long int max_idle;
> -bool must_del;
> +size_t n_ops;
> +void *state;
>
> +n_ops = 0;
> +now = time_msec();
>  atomic_read(&udpif->flow_limit, &flow_limit);
>
> -n_flows = udpif_get_n_flows(udpif);
> -
> -must_del = false;
> -max_idle = MAX_IDLE;
> -if (n_flows > flow_limit) {
> -must_del = n_flows > 2 * flow_limit;
> -max_idle = 100;
> -}
> -
> -n_ops = 0;
> -LIST_FOR_EACH_SAFE (udump, next_udump, list_node, udumps) {
> -long long int used, now;
> +dpif_flow_dump_state_init(udpif->dpif, &state);
> +while (dpif_flow_dump_next(&udpif->dump, state, &key, &key_len, &mask,
> +   &mask_len, &actions, &actions_len,
> &stats)) {
>

Above, the current version of this "Remove the flow dumper thread" patch
removes the code to modify the flow limit and max_idle. I was trying to
remember why this is---whether it was a deliberate omission or perhaps I've
introduced it along the way as I rebased.

I don't have access to a test setup right now, so can't easily observe the
behaviour with/without this snippet. I suspect it would change the
behaviour significantly when around the flow_limit, but it's not obvious to
me that the behaviour is more desirable with this limit/idle code, or
without. I believe that all of my previous testing would have been without.

Do you remember why this is removed?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] boot.sh is failing on latest master -- need help

2014-03-26 Thread Ben Pfaff
On Tue, Mar 25, 2014 at 11:39 AM, Sabyasachi Sengupta
 wrote:
>
> I'm getting the following error while trying to build openvswitch. Can
> anyone suggest what may be missing? I have autoconf-2.65, m4 and libtool
> installed. The failure does not occur when using master from last week.
>
> [sabyasse@vmOnNFS openvswitch]$ sudo sh ./boot.sh
> configure.ac:43: error: possibly undefined macro: LT_INIT
>   If this token and others are legitimate, please use m4_pattern_allow.
>   See the Autoconf documentation.
> autoreconf: /usr/local/bin/autoconf failed with exit status: 1

libtool isn't correctly installed.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Add support for kernel 3.13

2014-03-26 Thread Kyle Mestery
On Tue, Mar 25, 2014 at 3:36 PM, Pravin Shelar  wrote:

> On Fri, Mar 21, 2014 at 10:41 AM, Kyle Mestery
>  wrote:
> > Add support for building the in-tree kernel datapath for Linux kernel
> 3.13.
> > There were some changes in the netlink area which required adding new
> > compatibility code for this layer.
> >
> > Signed-off-by: Kyle Mestery 
> > ---
> >  FAQ   |  2 +-
> >  acinclude.m4  |  4 +--
> >  datapath/datapath.c   | 27 +++-
> >  datapath/datapath.h   |  1 +
> >  datapath/dp_notify.c  | 11 +++
> >  datapath/linux/compat/genetlink-openvswitch.c | 20 +---
> >  datapath/linux/compat/include/net/genetlink.h | 45
> +--
> >  datapath/linux/compat/include/net/ip.h|  4 +++
> >  datapath/linux/compat/utils.c |  3 ++
> >  datapath/vport-lisp.c |  7 +++--
> >  datapath/vport-vxlan.c|  3 +-
> >  11 files changed, 100 insertions(+), 27 deletions(-)
> >
> > diff --git a/FAQ b/FAQ
> > index a54bbf9..040cdf7 100644
> > --- a/FAQ
> > +++ b/FAQ
> > @@ -148,7 +148,7 @@ A: The following table lists the Linux kernel
> versions against which the
> > 1.10.x 2.6.18 to 3.8
> > 1.11.x 2.6.18 to 3.8
> > 2.0.x  2.6.32 to 3.10
> > -   2.1.x  2.6.32 to 3.12
> > +   2.1.x  2.6.32 to 3.13
> >
> > Open vSwitch userspace should also work with the Linux kernel module
> > built into Linux 3.3 and later.
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 8f41e33..bd8714c 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -134,10 +134,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
> >  AC_MSG_RESULT([$kversion])
> >
> >  if test "$version" -ge 3; then
> > -   if test "$version" = 3 && test "$patchlevel" -le 12; then
> > +   if test "$version" = 3 && test "$patchlevel" -le 13; then
> >: # Linux 3.x
> > else
> > - AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.12.x is not supported])
> > + AC_ERROR([Linux kernel in $KBUILD is version $kversion, but
> version newer than 3.13.x is not supported])
> > fi
> >  else
> > if test "$version" -le 1 || test "$patchlevel" -le 5 || test
> "$sublevel" -le 31; then
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index f7c3391..b4f09b7 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -64,11 +64,13 @@
> >
> >  int ovs_net_id __read_mostly;
> >
> > +static struct genl_family dp_packet_genl_family;
> > +
> >  static void ovs_notify(struct sk_buff *skb, struct genl_info *info,
> >struct genl_multicast_group *grp)
> >  {
> > -   genl_notify(skb, genl_info_net(info), info->snd_portid,
> > -   grp->id, info->nlhdr, GFP_KERNEL);
> > +   genl_notify(&dp_packet_genl_family, skb, genl_info_net(info),
> > +   info->snd_portid, 0, info->nlhdr, GFP_KERNEL);
> >  }
> >
> >  /**
> > @@ -883,8 +885,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff
> *skb, struct genl_info *info)
> > if (!IS_ERR(reply))
> > ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
> > else
> > -   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -   ovs_dp_flow_multicast_group.id,
> PTR_ERR(reply));
> > +   genl_set_err(&dp_flow_genl_family, sock_net(skb->sk), 0,
> > +0, PTR_ERR(reply));
> > return 0;
> >
> >  err_flow_free:
> > @@ -1365,8 +1367,8 @@ static int ovs_dp_cmd_set(struct sk_buff *skb,
> struct genl_info *info)
> > reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
> > if (IS_ERR(reply)) {
> > err = PTR_ERR(reply);
> > -   netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
> > -   ovs_dp_datapath_multicast_group.id,
> err);
> > +   genl_set_err(&dp_datapath_genl_family,
> sock_net(skb->sk), 0,
> > +0, err);
> > err = 0;
> > goto unlock;
> > }
> > @@ -1463,7 +1465,7 @@ static const struct nla_policy
> vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
> > [OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
> >  };
> >
> > -static struct genl_family dp_vport_genl_family = {
> > +struct genl_family dp_vport_genl_family = {
> > .id = GENL_ID_GENERATE,
> > .hdrsize = sizeof(struct ovs_header),
> > .name = OVS_VPORT_FAMILY,
> > @@ -1862,6 +1864,7 @@ static int dp_register_genl(void)
> > for (i = 0; i < ARRAY_SIZE(dp_genl_families); i++) {
> > const struct genl_family_and_ops *f =
> &dp_genl_families[i];
> >
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,13,0)
> > er

[ovs-dev] [PATCH net] openvswitch: fix a possible deadlock and lockdep warning

2014-03-26 Thread Flavio Leitner
There are two problematic situations.

A deadlock can happen when is_percpu is false because it can get
interrupted while holding the spinlock. Then it executes
ovs_flow_stats_update() in softirq context which tries to get
the same lock.

The second sitation is that when is_percpu is true, the code
correctly disables BH only for the local CPU, but that confuses
lockdep enough to trigger the warning below.

This patch disables BH for both cases fixing the real deadlock
and the lockdep warning message.

=
[ INFO: inconsistent lock state ]
3.14.0-rc8-7-g632b06a #1 Tainted: G  I
-
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
swapper/0/0 [HC0[0]:SC1[5]:HE1:SE0] takes:
(&(&cpu_stats->lock)->rlock){+.?...}, at: [] 
ovs_flow_stats_update+0x51/0xd0 [openvswitch]
{SOFTIRQ-ON-W} state was registered at:
[] __lock_acquire+0x68f/0x1c40
[] lock_acquire+0xa2/0x1d0
[] _raw_spin_lock+0x3e/0x80
[] ovs_flow_stats_get+0xc4/0x1e0 [openvswitch]
[] ovs_flow_cmd_fill_info+0x185/0x360 [openvswitch]
[] ovs_flow_cmd_build_info.constprop.27+0x55/0x90 
[openvswitch]
[] ovs_flow_cmd_new_or_set+0x4dd/0x570 [openvswitch]
[] genl_family_rcv_msg+0x1cd/0x3f0
[] genl_rcv_msg+0x8e/0xd0
[] netlink_rcv_skb+0xa9/0xc0
[] genl_rcv+0x28/0x40
[] netlink_unicast+0x100/0x1e0
[] netlink_sendmsg+0x347/0x770
[] sock_sendmsg+0x9c/0xe0
[] ___sys_sendmsg+0x3a9/0x3c0
[] __sys_sendmsg+0x51/0x90
[] SyS_sendmsg+0x12/0x20
[] system_call_fastpath+0x16/0x1b
irq event stamp: 1740726
hardirqs last  enabled at (1740726): [] 
ip6_finish_output2+0x4f0/0x840
hardirqs last disabled at (1740725): [] 
ip6_finish_output2+0x4ab/0x840
softirqs last  enabled at (1740674): [] 
_local_bh_enable+0x22/0x50
softirqs last disabled at (1740675): [] irq_exit+0xc5/0xd0

other info that might help us debug this:
 Possible unsafe locking scenario:

   CPU0
   
  lock(&(&cpu_stats->lock)->rlock);
  
lock(&(&cpu_stats->lock)->rlock);

 *** DEADLOCK ***

5 locks held by swapper/0/0:
 #0:  (((&ifa->dad_timer))){+.-...}, at: [] 
call_timer_fn+0x5/0x320
 #1:  (rcu_read_lock){.+.+..}, at: [] mld_sendpack+0x5/0x4a0
 #2:  (rcu_read_lock_bh){.+}, at: [] 
ip6_finish_output2+0x59/0x840
 #3:  (rcu_read_lock_bh){.+}, at: [] 
__dev_queue_xmit+0x5/0x9b0
 #4:  (rcu_read_lock){.+.+..}, at: [] 
internal_dev_xmit+0x5/0x110 [openvswitch]

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G  I  3.14.0-rc8-7-g632b06a 
#1
Hardware name:  /DX58SO, BIOS SOX5810J.86A.5599.2012.0529.2218 
05/29/2012
  0fcf20709903df0c 88042d603808 817cfe3c
 81c134c0 88042d603858 817cb6da 0005
 0001 8804 0006 81c134c0
Call Trace:
   [] dump_stack+0x4d/0x66
 [] print_usage_bug+0x1f4/0x205
 [] ? check_usage_backwards+0x180/0x180
 [] mark_lock+0x223/0x2b0
 [] __lock_acquire+0x623/0x1c40
 [] ? __lock_is_held+0x57/0x80
 [] ? masked_flow_lookup+0x236/0x250 [openvswitch]
 [] lock_acquire+0xa2/0x1d0
 [] ? ovs_flow_stats_update+0x51/0xd0 [openvswitch]
 [] _raw_spin_lock+0x3e/0x80
 [] ? ovs_flow_stats_update+0x51/0xd0 [openvswitch]
 [] ovs_flow_stats_update+0x51/0xd0 [openvswitch]
 [] ovs_dp_process_received_packet+0x84/0x120 [openvswitch]
 [] ? __lock_acquire+0x347/0x1c40
 [] ovs_vport_receive+0x2a/0x30 [openvswitch]
 [] internal_dev_xmit+0x68/0x110 [openvswitch]
 [] ? internal_dev_xmit+0x5/0x110 [openvswitch]
 [] dev_hard_start_xmit+0x2e6/0x8b0
 [] __dev_queue_xmit+0x417/0x9b0
 [] ? __dev_queue_xmit+0x5/0x9b0
 [] ? ip6_finish_output2+0x4f0/0x840
 [] dev_queue_xmit+0x10/0x20
 [] ip6_finish_output2+0x551/0x840
 [] ? ip6_finish_output+0x9a/0x220
 [] ip6_finish_output+0x9a/0x220
 [] ip6_output+0x4f/0x1f0
 [] mld_sendpack+0x1d9/0x4a0
 [] mld_send_initial_cr.part.32+0x88/0xa0
 [] ? addrconf_dad_completed+0x220/0x220
 [] ipv6_mc_dad_complete+0x31/0x50
 [] addrconf_dad_completed+0x147/0x220
 [] ? addrconf_dad_completed+0x220/0x220
 [] addrconf_dad_timer+0x19f/0x1c0
 [] call_timer_fn+0x99/0x320
 [] ? call_timer_fn+0x5/0x320
 [] ? addrconf_dad_completed+0x220/0x220
 [] run_timer_softirq+0x254/0x3b0
 [] __do_softirq+0x12d/0x480
 [] irq_exit+0xc5/0xd0
 [] do_IRQ+0x58/0xf0
 [] common_interrupt+0x72/0x72
   [] ? cpuidle_enter_state+0x54/0xd0
 [] cpuidle_idle_call+0xb9/0x380
 [] arch_cpu_idle+0xe/0x40
 [] cpu_startup_entry+0xf5/0x420
 [] rest_init+0x13d/0x150
 [] ? rest_init+0x5/0x150
 [] start_kernel+0x493/0x4b4
 [] ? repair_env_string+0x5c/0x5c
 [] ? early_idt_handlers+0x120/0x120
 [] x86_64_start_reservations+0x2a/0x2c
 [] x86_64_start_kernel+0x14d/0x170

Signed-off-by: Flavio Leitner 
---
 net/openvswitch/flow.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index dda451f..2998989 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -103,30 +103,24 @@ static void stats_read(struct flow_stats *stats,

[ovs-dev] [PATCH] recirculation: Some cosmetic fixes

2014-03-26 Thread YAMAMOTO Takashi
Wrap long lines, fix whitespaces, and fix a typo in a comment.
No functional changes are intended.

Cc: Andy Zhou 
Signed-off-by: YAMAMOTO Takashi 
---
 include/linux/openvswitch.h |  3 ++-
 lib/dpif-netdev.c   |  3 ++-
 ofproto/ofproto-dpif.h  | 50 -
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index d9282d6..3fc4978 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -549,7 +549,8 @@ enum ovs_recirc_hash_alg {
  * struct ovs_action_recirc - %OVS_ACTION_ATTR_RECIRC action argument.
  * @recirc_id: The Recirculation label, Zero is invalid.
  * @hash_alg: Algorithm used to compute hash prior to recirculation.
- * @hash_bias: bias used for computing hash.  used to compute hash prior to 
recirculation.
+ * @hash_bias: bias used for computing hash.  used to compute hash prior to
+ * recirculation.
  */
 struct ovs_action_recirc {
uint32_t  hash_alg; /* One of ovs_dp_hash_alg. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8687a47..60e024e 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2117,8 +2117,9 @@ dp_execute_cb(void *aux_, struct ofpbuf *packet,
 
 case OVS_ACTION_ATTR_RECIRC: {
 const struct ovs_action_recirc *act;
+
 act = nl_attr_get(a);
-md->recirc_id =act->recirc_id;
+md->recirc_id = act->recirc_id;
 md->dp_hash = 0;
 
 if (act->hash_alg == OVS_RECIRC_HASH_ALG_L4) {
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 93e6ec0..088ff89 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -139,32 +139,35 @@ struct ofport_dpif *odp_port_to_ofport(const struct 
dpif_backer *, odp_port_t);
  * Recirculation
  * =
  *
- * Recirculation is a technique to allow a frame to re-enter the packet 
processing
- * path for one or multiple times to achieve more flexible packet processing 
in the
- * data path. MPLS handling and selecting bond slave port of a bond ports.
+ * Recirculation is a technique to allow a frame to re-enter the packet
+ * processing path for one or multiple times to achieve more flexible packet
+ * processing in the data path. MPLS handling and selecting bond slave port
+ * of a bond ports.
  *
  * Data path and user space interface
  * ---
  *
- * Two new fields, recirc_id and dp_hash, are added to the current flow data 
structure.
- * They are both both of type uint32_t. In addition, a new action, RECIRC, are 
added.
+ * Two new fields, recirc_id and dp_hash, are added to the current flow data
+ * structure. They are both of type uint32_t. In addition, a new action,
+ * RECIRC, are added.
  *
- * The value recirc_id is used to distinguish a packet from multiple 
iterations of
- * recirculation. A packet initially received is considered of having 
recirc_id of 0.
- * Recirc_id is managed by the user space, opaque to the data path.
+ * The value recirc_id is used to distinguish a packet from multiple
+ * iterations of recirculation. A packet initially received is considered of
+ * having recirc_id of 0. Recirc_id is managed by the user space, opaque to
+ * the data path.
  *
  * On the other hand, dp_hash can only be computed by the data path, opaque to
- * the user space. In fact, user space may not able to recompute the hash 
value.
- * The dp_hash value should be wildcarded when for a newly received packet.
- * RECIRC action specifies whether the hash is computed. If computed, how many
- * fields to be included in the hash computation. The computed hash value is
- * stored into the dp_hash field prior to recirculation.
- *
- * The RECIRC action computes and set the dp_hash field, set the recirc_id 
field
- * and then reprocess the packet as if it was received on the same input port.
- * RECIRC action works like a function call; actions listed behind the RECIRC
- * action will be executed after its execution.  RECIRC action can be nested,
- * data path implementation limits the number of recirculation executed
+ * the user space. In fact, user space may not able to recompute the hash
+ * value. The dp_hash value should be wildcarded when for a newly received
+ * packet. RECIRC action specifies whether the hash is computed. If computed,
+ * how many fields to be included in the hash computation. The computed hash
+ * value is stored into the dp_hash field prior to recirculation.
+ *
+ * The RECIRC action computes and set the dp_hash field, set the recirc_id
+ * field and then reprocess the packet as if it was received on the same input
+ * port. RECIRC action works like a function call; actions listed behind the
+ * RECIRC action will be executed after its execution.  RECIRC action can be
+ * nested, data path implementation limits the number of recirculation executed
  * to prevent unreasonable nesting depth or infinite loop.
  *
  * Both flow fields and the REC