[ovs-dev] [QUESTION] the flow expire time

2012-10-26 Thread JieYue Ma
Hi,

During my observation, it seems that the flow expire time has some
kind of heuristic algo to compute. I add some log for flow miss, and
start to ping from A to B. At first, the flow from A to B seems to
miss almost every 5 seconds, but as time goes by, the miss interval
enlarges, which means the flow expire time increases itself, doesn't
it?

I don't know how the flow expire mechanism works, hopefully some one
could tell me, thanks. And also, maybe I need to change such heuristic
way in our production environment, could some one point out where the
code is?

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


Re: [ovs-dev] [PATCH 1/4] datapath: Add basic MPLS support to kernel

2012-10-26 Thread Isaku Yamahata
Some inlined comments in case label == 0. I don't find any dependency on it.

On Thu, Oct 18, 2012 at 06:02:18PM +0900, Simon Horman wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index a6915fb..e606ae1 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -74,6 +74,43 @@ int ovs_net_id __read_mostly;
>  int (*ovs_dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);
>  EXPORT_SYMBOL(ovs_dp_ioctl_hook);
>  
> +void skb_cb_set_mpls_bos(struct sk_buff *skb)
> +{
> + struct ethhdr *eth;
> + int nh_ofs;
> + __be16 dl_type = 0;
> +
> + skb_reset_mac_header(skb);
> +
> + eth = eth_hdr(skb);
> + nh_ofs = sizeof(struct ethhdr);
> + if (likely(eth->h_proto >= htons(ETH_TYPE_MIN))) {
> + dl_type = eth->h_proto;
> +
> + while (dl_type == htons(ETH_P_8021Q) &&
> + skb->len >= nh_ofs + sizeof(struct vlan_hdr)) {
> + struct vlan_hdr *vh = (struct vlan_hdr*)(skb->data + 
> nh_ofs);
> + dl_type = vh->h_vlan_encapsulated_proto;
> + nh_ofs += sizeof(struct vlan_hdr);
> + }
> +
> + OVS_CB(skb)->mpls_bos = nh_ofs;
> + } else {
> + OVS_CB(skb)->mpls_bos = 0;
> + }
> +}
> +
> +unsigned char *skb_cb_mpls_bos(const struct sk_buff *skb)
> +{
> + return OVS_CB(skb)->mpls_bos ?
> + skb_mac_header(skb) + OVS_CB(skb)->mpls_bos : 0;
> +}
> +
> +ptrdiff_t skb_cb_mpls_bos_offset(const struct sk_buff *skb)
> +{
> + return OVS_CB(skb)->mpls_bos;
> +}
> +
>  /**
>   * DOC: Locking:
>   *
> @@ -663,6 +700,9 @@ static int validate_actions(const struct nlattr *attr,
>   static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
>   [OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
>   [OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
> + [OVS_ACTION_ATTR_PUSH_MPLS] = sizeof(struct 
> ovs_action_push_mpls),
> + [OVS_ACTION_ATTR_POP_MPLS] = sizeof(__be16),
> + [OVS_ACTION_ATTR_SET_MPLS] = sizeof(__be32),
>   [OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct 
> ovs_action_push_vlan),
>   [OVS_ACTION_ATTR_POP_VLAN] = 0,
>   [OVS_ACTION_ATTR_SET] = (u32)-1,
> @@ -691,6 +731,24 @@ static int validate_actions(const struct nlattr *attr,
>   return -EINVAL;
>   break;
>  
> + case OVS_ACTION_ATTR_PUSH_MPLS: {
> + const struct ovs_action_push_mpls *mpls = nla_data(a);
> + if (mpls->mpls_ethertype != htons(ETH_P_MPLS_UC) &&
> + mpls->mpls_ethertype != htons(ETH_P_MPLS_MC))
> + return -EINVAL;
> + break;
> + }
> +
> + case OVS_ACTION_ATTR_POP_MPLS:
> + break;
> +
> + case OVS_ACTION_ATTR_SET_MPLS: {
> + __be32 mpls_label = nla_get_be32(a);
> + if (mpls_label == htonl(0)) {
> + return -EINVAL;
> + }

Is this necessary?
At least, this is inconsistent with case OVS_ACTION_ATTR_PUSH_MPLS.


> + break;
> + }
>  
>   case OVS_ACTION_ATTR_POP_VLAN:
>   break;
> @@ -805,6 +863,8 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, 
> struct genl_info *info)
>   OVS_CB(packet)->flow = flow;
>   packet->priority = flow->key.phy.priority;
>  
> + skb_cb_set_mpls_bos(packet);
> +
>   rcu_read_lock();
>   dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>   err = -ENODEV;


... snip ...

> diff --git a/datapath/flow.h b/datapath/flow.h
> index 02c563a..eb3fe49 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -53,6 +53,9 @@ struct sw_flow_key {
>   __be16 type;/* Ethernet frame type. */
>   } eth;
>   struct {
> + __be32 top_label;   /* 0 if no MPLS, top label from stack 
> otherwise */

I think that the validity of top_label is guaranteed by ethertype = MPLS.
So top_label can be 0 as valid label value.


> + } mpls;
> + struct {
>   u8 proto;   /* IP protocol or lower 8 bits of ARP 
> opcode. */
>   u8 tos; /* IP ToS. */
>   u8 ttl; /* IP TTL/hop limit. */
> @@ -126,6 +129,10 @@ struct arp_eth_header {
>   unsigned char   ar_tip[4];  /* target IP address*/
>  } __packed;
>  
> +#define ETH_TYPE_MIN 0x600
> +
> +#define MPLS_HLEN 4
> +
>  int ovs_flow_init(void);
>  void ovs_flow_exit(void);
>  

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


Re: [ovs-dev] [PATCH] datapath: Deprecated CAPWAP support.

2012-10-26 Thread Cong Wang

On 10/28/2012 04:07 AM, Pravin B Shelar wrote:

--- a/datapath/vport-capwap.c
+++ b/datapath/vport-capwap.c
@@ -473,6 +473,7 @@ static struct vport *capwap_create(const struct vport_parms 
*parms)
struct vport *vport;
int err;

+   pr_err("%s: capwap support is deprecated\n", __func__);
err = init_socket(ovs_dp_get_net(parms->dp));
if (err)
return ERR_PTR(err);



We usually use a warning for such deprecated feature, not an error.
Thus, pr_warn() is better.

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


Re: [ovs-dev] [PATCH 3/4] User-Space MPLS actions and matches

2012-10-26 Thread Isaku Yamahata
On Thu, Oct 18, 2012 at 06:02:20PM +0900, Simon Horman wrote:
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 1fb0304..13eb367 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -378,6 +378,26 @@ ofpact_from_nxast(const union ofp_action *a, enum 
> ofputil_action_code code,
>  case OFPUTIL_NXAST_CONTROLLER:
>  controller_from_openflow((const struct nx_action_controller *) a, 
> out);
>  break;
> +
> +case OFPUTIL_NXAST_PUSH_MPLS: {
> +struct nx_action_push *nxap = (struct nx_action_push *)a;
> +if (nxap->ethertype != htons(ETH_TYPE_MPLS) &&
> +nxap->ethertype != htons(ETH_TYPE_MPLS_MCAST)) {
> +return OFPERR_OFPBAC_BAD_ARGUMENT;
> +}
> +ofpact_put_PUSH_MPLS(out)->ethertype = nxap->ethertype;
> +break;
> +}
> +
> +case OFPUTIL_NXAST_POP_MPLS: {
> +struct nx_action_pop_mpls *nxapm = (struct nx_action_pop_mpls *)a;
> +if (nxapm->ethertype == htons(ETH_TYPE_MPLS) ||
> +nxapm->ethertype == htons(ETH_TYPE_MPLS_MCAST)) {
> +return OFPERR_OFPBAC_BAD_ARGUMENT;
> +}
> +ofpact_put_POP_MPLS(out)->ethertype = nxapm->ethertype;
> +break;
> +}
>  }
>  
>  return error;
> @@ -722,6 +742,26 @@ ofpact_from_openflow11(const union ofp_action *a, struct 
> ofpbuf *out)
>  return nxm_reg_load_from_openflow12_set_field(
>  (const struct ofp12_action_set_field *)a, out);
>  
> +case OFPUTIL_OFPAT11_PUSH_MPLS: {
> +struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
> +if (oap->ethertype != htons(ETH_TYPE_MPLS) &&
> +oap->ethertype != htons(ETH_TYPE_MPLS_MCAST)) {
> +return OFPERR_OFPBAC_BAD_ARGUMENT;
> +}
> +ofpact_put_PUSH_MPLS(out)->ethertype = oap->ethertype;
> +break;
> +}
> +
> +case OFPUTIL_OFPAT11_POP_MPLS: {
> +struct ofp11_action_pop_mpls *oapm = (struct ofp11_action_pop_mpls 
> *)a;
> +if (oapm->ethertype == htons(ETH_TYPE_MPLS) ||
> +oapm->ethertype == htons(ETH_TYPE_MPLS_MCAST)) {
> +return OFPERR_OFPBAC_BAD_ARGUMENT;
> +}
> +ofpact_put_POP_MPLS(out)->ethertype = oapm->ethertype;
> +break;
> +}
> +
>  #define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
>  #include "ofp-util.def"
>  return ofpact_from_nxast(a, code, out);
> @@ -1052,6 +1092,8 @@ ofpact_check__(const struct ofpact *a, int max_ports)
>  
>  case OFPACT_NOTE:
>  case OFPACT_EXIT:
> +case OFPACT_PUSH_MPLS:
> +case OFPACT_POP_MPLS:
>  return 0;
>  
>  case OFPACT_CLEAR_ACTIONS:
> @@ -1260,6 +1302,16 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf 
> *out)
>  ofputil_put_NXAST_EXIT(out);
>  break;
>  
> +case OFPACT_PUSH_MPLS:
> +ofputil_put_NXAST_PUSH_MPLS(out)->ethertype =
> +ofpact_get_PUSH_MPLS(a)->ethertype;
> +break;
> +
> +case OFPACT_POP_MPLS:
> +ofputil_put_NXAST_POP_MPLS(out)->ethertype =
> +ofpact_get_POP_MPLS(a)->ethertype;
> +break;
> +
>  case OFPACT_OUTPUT:
>  case OFPACT_ENQUEUE:
>  case OFPACT_SET_VLAN_VID:
> @@ -1384,6 +1436,8 @@ ofpact_to_openflow10(const struct ofpact *a, struct 
> ofpbuf *out)
>  case OFPACT_AUTOPATH:
>  case OFPACT_NOTE:
>  case OFPACT_EXIT:
> +case OFPACT_PUSH_MPLS:
> +case OFPACT_POP_MPLS:
>  ofpact_to_nxast(a, out);
>  break;
>  }
> @@ -1476,6 +1530,16 @@ ofpact_to_openflow11(const struct ofpact *a, struct 
> ofpbuf *out)
>  = htons(ofpact_get_SET_L4_DST_PORT(a)->port);
>  break;
>  
> +case OFPACT_PUSH_MPLS:
> +ofputil_put_OFPAT11_PUSH_MPLS(out)->ethertype =
> +ofpact_get_PUSH_MPLS(a)->ethertype;
> +break;
> +
> +case OFPACT_POP_MPLS:
> +ofputil_put_OFPAT11_POP_MPLS(out)->ethertype =
> +ofpact_get_POP_MPLS(a)->ethertype;
> +break;
> +
>  case OFPACT_CLEAR_ACTIONS:
>  case OFPACT_GOTO_TABLE:
>  NOT_REACHED();

Even when NX MPLS actions is used, retrieved actions always results in
OF1.1+ MPLS actions. We can keep it easily.

Set ofpact->compat = OFPUTIL_NX_xxx_MPLS in ofpact_from_nxact().
And then
if (a->compat == OFPUTIL_NXAST_xxx_MPLS) {
NX MPLS action
} else {
OF11+ MPLS action
}
-- 
yamahata
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Protection of domain names

2012-10-26 Thread David Zhao
(Letter to the President or Brand Owner, thanks)
Dear President,
We are the department of Asian Domain Registration Service in China. I have 
something to confirm with you. We formally received an application on October 
25,2012 that a company which self-styled "GOX Investment Ltd" were applying to 
register"openvswitch"as their Net Brand and some domain names through our firm.
Now we are handling this registration, and after our initial checking, we found 
the name were similar to your company's, so we need to check with you whether 
your company has authorized that company to register these names. If you 
authorized this, we will finish the registration at once. If you did not 
authorize, please let us know within 7 workdays, so that we will handle this 
issue better. Out of the time limit we will unconditionally finish the 
registration for "GOX Investment Ltd".Looking forward to your prompt reply.
Best Regards,
David Zhao
Head of Registration Department
Tel:+86-551-3434624 || Fax:+86-551-3434924
Address:No.99 JiuHuaShan Road,Hefei,Anhui,China
<>___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] ovs-vsctl: check if the device name is valid

2012-10-26 Thread Cong Wang

On 10/25/2012 11:20 PM, Ben Pfaff wrote:

Furthermore, ovs-vsctl is meant to be generic in the sense that you can
use an ovs-vsctl built in one place (on one OS) to control a switch and
database running elsewhere (on another OS).  You don't want to apply the
local host's restrictions to changes to a remote db.


Makes sense.

The problem without this patch is that when we add some port whose name 
is longer than IFNAMSIZ, ovs-vsctl doesn't give any error, the error is 
actually "hidden" in the log, which is actually written by ovs-vswitchd. 
So, it is not easy for inexperienced people to find where the error is.


How about raising the console log level? So that such errors will be 
seen on the terminal.


Thanks!

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


Re: [ovs-dev] [PATCH] ovs-vsctl: check if the device name is valid

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 09:22:35PM +0800, Cong Wang wrote:
> On 10/25/2012 11:20 PM, Ben Pfaff wrote:
> >Furthermore, ovs-vsctl is meant to be generic in the sense that you can
> >use an ovs-vsctl built in one place (on one OS) to control a switch and
> >database running elsewhere (on another OS).  You don't want to apply the
> >local host's restrictions to changes to a remote db.
> 
> The problem without this patch is that when we add some port whose
> name is longer than IFNAMSIZ, ovs-vsctl doesn't give any error, the
> error is actually "hidden" in the log, which is actually written by
> ovs-vswitchd. So, it is not easy for inexperienced people to find
> where the error is.
> 
> How about raising the console log level? So that such errors will be
> seen on the terminal.

ovs-vswitchd doesn't have a fd for the console; it daemonized itself and
closed that fd long ago.  (That's especially important if one started
ovs-vswitchd over an SSH connection; otherwise ovs-vswitchd holds the
SSH connection open.)

I think the key is probably to learn to look at the log if something
goes wrong.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [QUESTION] the flow expire time

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 03:25:46PM +0800, JieYue Ma wrote:
> During my observation, it seems that the flow expire time has some
> kind of heuristic algo to compute. I add some log for flow miss, and
> start to ping from A to B. At first, the flow from A to B seems to
> miss almost every 5 seconds, but as time goes by, the miss interval
> enlarges, which means the flow expire time increases itself, doesn't
> it?

Can you explain what you observe in more detail?  It doesn't match up to
what I expect Open vSwitch to do, but I don't understand exactly what
you're reporting.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Bug#691508: ovs-vsctl: --may-exist --if-exists don't work

2012-10-26 Thread Adam Heath
Package: openvswitch-switch
Version: 1.4.2+git20120612-9

>From the man page:

   [--may-exist] add-br bridge
   [--may-exist] add-br bridge parent vlan
   [--may-exist] add-port bridge port [column[:key]=value]...
   [--if-exists] del-port [bridge] port

>From my script:

++ ovs-vsctl --may-exist add-port bf-public bf-public
ovs-vsctl: unrecognized option '--may-exist'
++ ovs-vsctl --may-exist add-br bf-public
ovs-vsctl: unrecognized option '--may-exist'

I can work around the --may/--if not being supported for bridge
add/remove.  For port add/remove, it's a bit harder, as I have to
parse the output of list-ports to see if the port exists or not.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [openflow-dev] [code] OFTest for OVS

2012-10-26 Thread Dan Talayco
Hi Ben--

Thanks for working on this!

I know there are still some rough edges with OFTest, but there are a number
of changes that have improved things (the control-C issue is a lot better
than it was, but still present in some situations) and we continue to make
progress (especially with contributions like yours).

Locally, we've been working on a port virtualization tool that allows
things like virtual connections across UDP sockets, etc.  What we have is
not quite ready for prime time, but I expect your domain socket approach
should provide some push forward in that.

One more note:  We are also reviewing how to better compartmentalize and
specify for execution sets of tests ("suites").  There are a number of
things (like the "emergency flow cache" tests) that have been put in but
generally aren't implemented by anyone (as the feature was mostly
discredited before it went into 1.0 and then remove by 1.1).

As the number of contributed tests is growing, there have been calls for a
base set of "compliance" tests.  While defining compliance is not in the
scope of OFTest, I'd like to have a default set of tests that maximally
satisfies (1) as much coverage as possible; and (2) agreement that the
tests cover implemented and useful features.

Easy definition, maintenance and selection of other test suites is the next
goal.

-Dan



On Thu, Oct 25, 2012 at 11:14 PM, Ben Pfaff  wrote:

> On Thu, Oct 25, 2012 at 05:12:21PM -0700, Ben Pfaff wrote:
> > It passes a few tests; I've only tried a few.
> >
> > Now that I look at it, there are some bugs here that will prevent
> > traffic from actually passing through, but they are not fundamental to
> > the approach.  I'm mostly passing this along in case anyone wants to
> > comment on the idea; I've already spent more time on it than I should
> > have.
>
> I'm attaching updated dummy.py and start-sandbox scripts.  With these
> versions, the command
>
> OVS_RUNDIR=/home/blp/nicira/ovs/_build/sandbox ./oft -P dummy \
> --log-file=oft.log --verbose -T all
>
> yields:
>
> runTest (load.PacketInLoad) ... ok
> runTest (load.PacketOutLoad) ... ok
> runTest (port_stats.MultiFlowStats) ... (,
> error(9, 'Bad file descriptor'), )
> ok
> runTest (port_stats.SingleFlowStats) ... ok
> runTest (basic.EchoWithData) ... ok
> runTest (basic.DescStatsGet) ... ok
> runTest (basic.PacketOutMC) ... ok
> runTest (basic.PacketOut) ... ok
> runTest (basic.TableStatsGet) ... ok
> runTest (basic.Echo) ... ok
> runTest (basic.PortConfigMod) ... ok
> runTest (basic.FlowMod) ... ok
> runTest (basic.PortConfigModErr) ... ok
> runTest (basic.BadMessage) ... ok
> runTest (basic.PacketIn) ... ok
> runTest (openflow_protocol_messages.ModifyStateModify) ... ok
> runTest (openflow_protocol_messages.ModifyStateDelete) ... FAIL
> runTest (openflow_protocol_messages.ReadState) ... ok
> runTest (openflow_protocol_messages.BarrierRequestReply) ... ok
> runTest (openflow_protocol_messages.PacketOut) ... ok
> runTest (openflow_protocol_messages.ConfigurationRequest) ... ok
> runTest (openflow_protocol_messages.EchoWithoutBody) ... ok
> runTest (openflow_protocol_messages.PacketIn) ... ok
> runTest (openflow_protocol_messages.FeaturesRequest) ... ok
> runTest (openflow_protocol_messages.ModifyStateAdd) ... FAIL
> runTest (openflow_protocol_messages.Hello) ... ok
> runTest (flow_expire.FlowExpire) ... FAIL
> runTest (actions.ModifyL4Dst) ... ok
> runTest (actions.Announcement) ... ok
> runTest (actions.NoAction) ... ok
> runTest (actions.AddVlanTag) ... ok
> runTest (actions.ModifyL2Src) ... ok
> runTest (actions.ModifyTos) ... ok
> runTest (actions.ForwardLocal) ... ok
> runTest (actions.ForwardAll) ... ok
> runTest (actions.ModifyL4Src) ... ok
> runTest (actions.ForwardTable) ... ok
> runTest (actions.ForwardController) ... ok
> runTest (actions.ForwardFlood) ... ok
> runTest (actions.ModifyL2Dst) ... ok
> runTest (actions.ForwardInport) ... ok
> runTest (actions.ModifyL3Dst) ... ok
> runTest (actions.VlanPrio2) ... ok
> runTest (actions.VlanPrio1) ... ok
> runTest (actions.ModifyL3Src) ... ok
> runTest (actions.ModifyVlanTag) ... ok
> runTest (detailed_contr_sw_messages.EmerFlowTimeout) ... FAIL
> runTest (detailed_contr_sw_messages.DeleteNonexistingFlow) ... ok
> runTest (detailed_contr_sw_messages.StrictVsNonstrict) ... FAIL
> runTest (detailed_contr_sw_messages.HardTimeout) ... FAIL
> runTest (detailed_contr_sw_messages.IdleTimeout) ... FAIL
> runTest (detailed_contr_sw_messages.OverlapChecking) ... FAIL
> runTest (detailed_contr_sw_messages.ModifyAction) ... ok
> runTest (detailed_contr_sw_messages.SendFlowRem) ... FAIL
> runTest (detailed_contr_sw_messages.FlowTimeout) ... FAIL
> runTest (detailed_contr_sw_messages.NoOverlapChecking) ... FAIL
> runTest (detailed_contr_s

Re: [ovs-dev] [code] OFTest for OVS

2012-10-26 Thread Ben Pfaff
On Thu, Oct 25, 2012 at 11:14:34PM -0700, Ben Pfaff wrote:
> On Thu, Oct 25, 2012 at 05:12:21PM -0700, Ben Pfaff wrote:
> > It passes a few tests; I've only tried a few.
> > 
> > Now that I look at it, there are some bugs here that will prevent
> > traffic from actually passing through, but they are not fundamental to
> > the approach.  I'm mostly passing this along in case anyone wants to
> > comment on the idea; I've already spent more time on it than I should
> > have.
> 
> I'm attaching updated dummy.py and start-sandbox scripts.  With these
> versions, the command

Well, I meant to attach them.  Here they are.
#! /bin/sh

set -ex

srcdir=$HOME/nicira/ovs
builddir=$srcdir/_build
PATH=$builddir/ovsdb:$builddir/vswitchd:$builddir/utilities:$PATH

cd $builddir
rm -rf sandbox
mkdir sandbox
cd sandbox

OVS_RUNDIR=`pwd`; export OVS_RUNDIR
OVS_LOGDIR=`pwd`; export OVS_LOGDIR
OVS_DBDIR=`pwd`; export OVS_DBDIR
OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR

trap 'kill `cat *.pid`' 0 1 2 3 13 14 15

touch .conf.db.~lock~
rm -f conf.db
ovsdb-tool create conf.db $srcdir/vswitchd/vswitch.ovsschema

ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/db.sock
ovs-vsctl --no-wait init

ovs-vswitchd --detach --no-chdir --pidfile --log-file --enable-dummy 
--disable-system -vvconn -vnetdev_dummy

ovs-vsctl --no-wait \
-- add-br br0 \
-- set bridge br0 datapath-type=dummy fail-mode=secure

for port in p1 p2 p3 p4; do
ovs-vsctl --no-wait \
-- add-port br0 $port \
-- set interface $port type=dummy \
   options:pstream=punix:$OVS_RUNDIR/$port
done

ovs-vsctl \
-- set-controller br0 tcp:127.0.0.1 \
-- set controller br0 connection-mode=out-of-band max-backoff=1000

read line
"""
Dummy platform

This platform uses Open vSwitch dummy interfaces.
"""

import logging
import os
import select
import socket
import struct
import sys
import time
from threading import Thread
from threading import Lock

RCV_TIMEOUT = 1
RUN_DIR = os.environ.get("OVS_RUNDIR", "/var/run/openvswitch")

class DataPlanePortUnix(Thread):
"""
Class defining a port monitoring object that uses Unix domain
sockets for ports, intended for connecting to Open vSwitch "dummy"
netdevs.
"""

def __init__(self, interface_name, port_number, parent, max_pkts=1024):
"""
Set up a port monitor object
@param interface_name The name of the physical interface like eth1
@param port_number The port number associated with this port
@param parent The controlling dataplane object; for pkt wait CV
@param max_pkts Maximum number of pkts to keep in queue
"""
Thread.__init__(self)
self.interface_name = interface_name
self.max_pkts = max_pkts
self.packets_total = 0
self.packets = []
self.packets_discarded = 0
self.port_number = port_number
self.txq = []
self.txq_lock = Lock()
logname = "dp-" + interface_name
self.logger = logging.getLogger(logname)
try:
self.socket = DataPlanePortUnix.interface_open(interface_name)
except:
self.logger.info("Could not open socket")
raise
self.logger.info("Opened port monitor (class %s)", type(self).__name__)
self.parent = parent

@staticmethod
def interface_open(interface_name):
"""
Open a Unix domain socket interface.
@param interface_name port name as a string such as 'eth1'
@retval s socket
"""
s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
s.settimeout(RCV_TIMEOUT)
s.setblocking(0)
s.connect("%s/%s" % (RUN_DIR, interface_name))
return s

def run(self):
"""
Activity function for class
"""
self.running = True
rxbuf = ""
while self.running:
try:
self.txq_lock.acquire()
if self.txq:
wlist = [self.socket]
else:
wlist = []
self.txq_lock.release()

rout, wout, eout = select.select([self.socket], wlist, [], 1)
except:
print sys.exc_info()
self.logger.error("Select error, exiting")
break

if not self.running:
break

if wout:
self.txq_lock.acquire()
if self.txq:
retval = self.socket.send(self.txq[0])
if retval > 0:
self.txq[0] = self.txq[0][retval:]
if len(self.txq[0]) == 0:
self.txq = self.txq[1:]
self.txq_lock.release()

if rout:
if len(rxbuf) < 2:
n = 2 - len(rxbuf)
else:
frame_len = struct.unpack('>h', rxbuf[:2])[0]
 

[ovs-dev] Bug#691508: marked as done (ovs-vsctl: --may-exist --if-exists don't work)

2012-10-26 Thread Debian Bug Tracking System
Your message dated Fri, 26 Oct 2012 09:43:49 -0700
with message-id <20121026164349.gi22...@nicira.com>
and subject line Re: Bug#691508: ovs-vsctl: --may-exist --if-exists don't work
has caused the Debian Bug report #691508,
regarding ovs-vsctl: --may-exist --if-exists don't work
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
691508: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=691508
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
--- Begin Message ---
Package: openvswitch-switch
Version: 1.4.2+git20120612-9

>From the man page:

   [--may-exist] add-br bridge
   [--may-exist] add-br bridge parent vlan
   [--may-exist] add-port bridge port [column[:key]=value]...
   [--if-exists] del-port [bridge] port

>From my script:

++ ovs-vsctl --may-exist add-port bf-public bf-public
ovs-vsctl: unrecognized option '--may-exist'
++ ovs-vsctl --may-exist add-br bf-public
ovs-vsctl: unrecognized option '--may-exist'

I can work around the --may/--if not being supported for bridge
add/remove.  For port add/remove, it's a bit harder, as I have to
parse the output of list-ports to see if the port exists or not.
--- End Message ---
--- Begin Message ---
On Fri, Oct 26, 2012 at 10:29:13AM -0500, Adam Heath wrote:
> Package: openvswitch-switch
> Version: 1.4.2+git20120612-9
> 
> From the man page:
> 
>[--may-exist] add-br bridge
>[--may-exist] add-br bridge parent vlan
>[--may-exist] add-port bridge port [column[:key]=value]...
>[--if-exists] del-port [bridge] port
> 
> From my script:
> 
> ++ ovs-vsctl --may-exist add-port bf-public bf-public
> ovs-vsctl: unrecognized option '--may-exist'
> ++ ovs-vsctl --may-exist add-br bf-public
> ovs-vsctl: unrecognized option '--may-exist'

There's a distinction between global options and options to a particular
command.  These options are for a particular command so you need -- as a
separator:

ovs-vsctl -- --may-exist add-port bf-public bf-public

This is documented near the top of the manpage:

   The ovs-vsctl command line begins with global options (see
   OPTIONS below for details).  The global options are followed by
   one or more commands.  Each command should begin with -- by
   itself as a command- line argument, to separate it from the
   global options and following commands.  (If the first command
   does not have any options, then the first -- may be omitted.)
   The command itself starts with command-spe- cific options, if
   any, followed by the command name and any arguments.  See
   EXAMPLES below for syntax examples.

along with examples:

   Delete bridge br0, reporting an error if it does not exist:

  ovs-vsctl del-br br0

   Delete bridge br0 if it exists (the -- is required to separate
   del-br's options from the global options):

  ovs-vsctl -- --if-exists del-br br0--- End Message ---
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] OF11: push_vlan support

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 01:43:19PM +0900, Isaku Yamahata wrote:
> This implementes push_vlan with 802.1Q.
> NOTE: 802.1AD (QinQ) is not supported. It requires another effort.
> 
> Signed-off-by: Isaku Yamahata 

Applied to master, thank you!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Bug#691508: ovs-vsctl: --may-exist --if-exists don't work

2012-10-26 Thread Adam Heath
On 10/26/2012 11:43 AM, Ben Pfaff wrote:
> 
> [snip]

Right you are, mea-culpa.  You're right that there is an example in
the manpage, showing exactly what I want.  Sorry for the noise, and
thanks for the fast response.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2] datapath: Remove tunnel header caching.

2012-10-26 Thread Pravin B Shelar
v1-v2:
- Fixed capwap fragment case.
- simplified tnl_send.
--8<--cut here-->8--

Tunnel caching was added to reduce CPU utilization on TX path
by caching packet header, So performance gain is directly proportional
to number of skbs transferred. But with help of offloads skb are getting
larger. So there are less number of skbs. Therefore header caching does
not shows similar gains we seen in past. And now kernel 3.6 has removed
dst caching from networking which makes header caching even more tricky.
So this commit removes header caching from OVS tunnelling.

Signed-off-by: Pravin B Shelar 
---
 datapath/flow.c  |   15 +-
 datapath/flow.h  |4 -
 datapath/tunnel.c|  462 +++---
 datapath/tunnel.h|  100 +
 datapath/vport-capwap.c  |   40 +---
 datapath/vport-gre.c |   59 +-
 include/openvswitch/tunnel.h |1 -
 lib/netdev-vport.c   |   11 +-
 vswitchd/vswitch.xml |   10 -
 9 files changed, 56 insertions(+), 646 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index c70daee..8b3f21b 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -226,9 +226,7 @@ struct sw_flow *ovs_flow_alloc(void)
return ERR_PTR(-ENOMEM);
 
spin_lock_init(&flow->lock);
-   atomic_set(&flow->refcnt, 1);
flow->sf_acts = NULL;
-   flow->dead = false;
 
return flow;
 }
@@ -292,7 +290,6 @@ struct flow_table *ovs_flow_tbl_alloc(int new_size)
 
 static void flow_free(struct sw_flow *flow)
 {
-   flow->dead = true;
ovs_flow_put(flow);
 }
 
@@ -423,7 +420,6 @@ static void rcu_free_flow_callback(struct rcu_head *rcu)
 {
struct sw_flow *flow = container_of(rcu, struct sw_flow, rcu);
 
-   flow->dead = true;
ovs_flow_put(flow);
 }
 
@@ -434,20 +430,13 @@ void ovs_flow_deferred_free(struct sw_flow *flow)
call_rcu(&flow->rcu, rcu_free_flow_callback);
 }
 
-void ovs_flow_hold(struct sw_flow *flow)
-{
-   atomic_inc(&flow->refcnt);
-}
-
 void ovs_flow_put(struct sw_flow *flow)
 {
if (unlikely(!flow))
return;
 
-   if (atomic_dec_and_test(&flow->refcnt)) {
-   kfree((struct sf_flow_acts __force *)flow->sf_acts);
-   kmem_cache_free(flow_cache, flow);
-   }
+   kfree((struct sf_flow_acts __force *)flow->sf_acts);
+   kmem_cache_free(flow_cache, flow);
 }
 
 /* RCU callback used by ovs_flow_deferred_free_acts. */
diff --git a/datapath/flow.h b/datapath/flow.h
index f4ef285..373df67 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -104,9 +104,6 @@ struct sw_flow {
struct sw_flow_key key;
struct sw_flow_actions __rcu *sf_acts;
 
-   atomic_t refcnt;
-   bool dead;
-
spinlock_t lock;/* Lock for values below. */
unsigned long used; /* Last used time (in jiffies). */
u64 packet_count;   /* Number of packets matched. */
@@ -137,7 +134,6 @@ void ovs_flow_deferred_free(struct sw_flow *);
 struct sw_flow_actions *ovs_flow_actions_alloc(const struct nlattr *);
 void ovs_flow_deferred_free_acts(struct sw_flow_actions *);
 
-void ovs_flow_hold(struct sw_flow *);
 void ovs_flow_put(struct sw_flow *);
 
 int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *,
diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index bfb09ce..a5222d1 100644
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -52,43 +52,9 @@
 #include "vport-generic.h"
 #include "vport-internal_dev.h"
 
-#ifdef NEED_CACHE_TIMEOUT
-/*
- * On kernels where we can't quickly detect changes in the rest of the system
- * we use an expiration time to invalidate the cache.  A shorter expiration
- * reduces the length of time that we may potentially blackhole packets while
- * a longer time increases performance by reducing the frequency that the
- * cache needs to be rebuilt.  A variety of factors may cause the cache to be
- * invalidated before the expiration time but this is the maximum.  The time
- * is expressed in jiffies.
- */
-#define MAX_CACHE_EXP HZ
-#endif
-
-/*
- * Interval to check for and remove caches that are no longer valid.  Caches
- * are checked for validity before they are used for packet encapsulation and
- * old caches are removed at that time.  However, if no packets are sent 
through
- * the tunnel then the cache will never be destroyed.  Since it holds
- * references to a number of system objects, the cache will continue to use
- * system resources by not allowing those objects to be destroyed.  The cache
- * cleaner is periodically run to free invalid caches.  It does not
- * significantly affect system performance.  A lower interval will release
- * resources faster but will itself consume resources by requiring more 
frequent
- * checks.  A longer interval may result in messages being printed to the 
kernel
- * message buffer about unreleased res

[ovs-dev] Bug#691508: ovs-vsctl: --may-exist --if-exists don't work

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 01:00:32PM -0500, Adam Heath wrote:
> On 10/26/2012 11:43 AM, Ben Pfaff wrote:
> > 
> > [snip]
> 
> Right you are, mea-culpa.  You're right that there is an example in
> the manpage, showing exactly what I want.  Sorry for the noise, and
> thanks for the fast response.

It's counterintuitive and confusing.  I need to do something about it,
at least improving the error message.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] datapath: Remove tunnel header caching.

2012-10-26 Thread Ben Pfaff
On Thu, Oct 25, 2012 at 11:14:38AM -0700, Pravin B Shelar wrote:
> v1-v2:
>   - Fixed capwap fragment case.
>   - simplified tnl_send.
> --8<--cut here-->8--
> 
> Tunnel caching was added to reduce CPU utilization on TX path
> by caching packet header, So performance gain is directly proportional
> to number of skbs transferred. But with help of offloads skb are getting
> larger. So there are less number of skbs. Therefore header caching does
> not shows similar gains we seen in past. And now kernel 3.6 has removed
> dst caching from networking which makes header caching even more tricky.
> So this commit removes header caching from OVS tunnelling.
> 
> Signed-off-by: Pravin B Shelar 

Is there any benefit in adding a comment to tunnel.h remarking that
the bit 1<<6 is reserved because it was previously used?  (Maybe not,
if we're deleting this interface entirely in another version.)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] datapath: Remove tunnel header caching.

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 11:19:37AM -0700, Ben Pfaff wrote:
> On Thu, Oct 25, 2012 at 11:14:38AM -0700, Pravin B Shelar wrote:
> > v1-v2:
> > - Fixed capwap fragment case.
> > - simplified tnl_send.
> > --8<--cut here-->8--
> > 
> > Tunnel caching was added to reduce CPU utilization on TX path
> > by caching packet header, So performance gain is directly proportional
> > to number of skbs transferred. But with help of offloads skb are getting
> > larger. So there are less number of skbs. Therefore header caching does
> > not shows similar gains we seen in past. And now kernel 3.6 has removed
> > dst caching from networking which makes header caching even more tricky.
> > So this commit removes header caching from OVS tunnelling.
> > 
> > Signed-off-by: Pravin B Shelar 
> 
> Is there any benefit in adding a comment to tunnel.h remarking that
> the bit 1<<6 is reserved because it was previously used?  (Maybe not,
> if we're deleting this interface entirely in another version.)

Oh, and in netdev-vport.c, should we silently ignore the header_cache
option instead of warning that it's unknown?  Or instead warn that
it's not useful anymore?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] datapath: Remove tunnel header caching.

2012-10-26 Thread Jesse Gross
On Fri, Oct 26, 2012 at 11:21 AM, Ben Pfaff  wrote:
> On Fri, Oct 26, 2012 at 11:19:37AM -0700, Ben Pfaff wrote:
>> On Thu, Oct 25, 2012 at 11:14:38AM -0700, Pravin B Shelar wrote:
>> > v1-v2:
>> > - Fixed capwap fragment case.
>> > - simplified tnl_send.
>> > --8<--cut here-->8--
>> >
>> > Tunnel caching was added to reduce CPU utilization on TX path
>> > by caching packet header, So performance gain is directly proportional
>> > to number of skbs transferred. But with help of offloads skb are getting
>> > larger. So there are less number of skbs. Therefore header caching does
>> > not shows similar gains we seen in past. And now kernel 3.6 has removed
>> > dst caching from networking which makes header caching even more tricky.
>> > So this commit removes header caching from OVS tunnelling.
>> >
>> > Signed-off-by: Pravin B Shelar 
>>
>> Is there any benefit in adding a comment to tunnel.h remarking that
>> the bit 1<<6 is reserved because it was previously used?  (Maybe not,
>> if we're deleting this interface entirely in another version.)
>
> Oh, and in netdev-vport.c, should we silently ignore the header_cache
> option instead of warning that it's unknown?  Or instead warn that
> it's not useful anymore?

We warn for all unknown options so it should happen automatically.
It's also probably not that important for header caching because it's
really supposed to be a transparent optimization.  Not having it
shouldn't change any behavior.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [openflow-dev] [code] OFTest for OVS

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 09:24:54AM -0700, Dan Talayco wrote:
> I know there are still some rough edges with OFTest, but there are a number
> of changes that have improved things (the control-C issue is a lot better
> than it was, but still present in some situations) and we continue to make
> progress (especially with contributions like yours).

I hope that my message didn't sound too much like criticism of OFTest.
If I'm criticizing anything, it's that OFTest is a little hard to get
up and running, because of the specific issues that I'm trying to
address with the code that I've written.  Those issues are the main
reasons that we've never really even tried OFTest against OVS.

My goal here is to, from an OVS tree, be able to just type "make
check-oftest" and have OFTest run against the OVS build tree without
any further setup.  That's not much more work than I've already done.
If OFTest is this easy to run, then we (speaking mainly of myself)
will be more likely to run it.

> Locally, we've been working on a port virtualization tool that allows
> things like virtual connections across UDP sockets, etc.  What we have is
> not quite ready for prime time, but I expect your domain socket approach
> should provide some push forward in that.

What kind of protocol are you using?  (Or perhaps no protocol at all,
just raw Ethernet frames over UDP?)  My choice of Unix domain stream
sockets is not essential, although it's convenient on the OVS side,
but it does require a tiny protocol definition (each Ethernet frame is
preceded by a 2-byte length field).

> One more note:  We are also reviewing how to better compartmentalize and
> specify for execution sets of tests ("suites").  There are a number of
> things (like the "emergency flow cache" tests) that have been put in but
> generally aren't implemented by anyone (as the feature was mostly
> discredited before it went into 1.0 and then remove by 1.1).
>
> As the number of contributed tests is growing, there have been calls for a
> base set of "compliance" tests.  While defining compliance is not in the
> scope of OFTest, I'd like to have a default set of tests that maximally
> satisfies (1) as much coverage as possible; and (2) agreement that the
> tests cover implemented and useful features.
> 
> Easy definition, maintenance and selection of other test suites is the next
> goal.

All of that sounds like worthwhile goals.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Add Upstream id for GRE type.

2012-10-26 Thread Jesse Gross
On Sat, Oct 27, 2012 at 1:06 PM, Pravin B Shelar  wrote:
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index 89feb61..d4a043c 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -182,6 +182,7 @@ enum ovs_vport_type {
> OVS_VPORT_TYPE_UNSPEC,
> OVS_VPORT_TYPE_NETDEV,   /* network device */
> OVS_VPORT_TYPE_INTERNAL, /* network device implemented by datapath */
> +   OVS_VPORT_TYPE_FT_GRE,

I'm not sure what FT stands for.  Future?  In any case, can you add a
comment next to it?

> OVS_VPORT_TYPE_PATCH = 100, /* virtual tunnel connecting two vports */
> OVS_VPORT_TYPE_GRE,  /* GRE tunnel */
> OVS_VPORT_TYPE_CAPWAP,   /* CAPWAP tunnel */

> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 621abd1..a864341 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -154,6 +154,9 @@ netdev_vport_get_netdev_type(const struct 
> dpif_linux_vport *vport)
>  case OVS_VPORT_TYPE_PATCH:
>  return "patch";
>
> +case OVS_VPORT_TYPE_FT_GRE:
> +return "gre";

I don't think that we want to introduce any uses of the symbolic name
in userspace.  In the next release, we'll drop the FT part of the name
(but keep the same constant) so I think it's easier if we don't touch
userspace.

Also, did you look at the userspace code to see whether it can handle
the transition by removing port types that it doesn't understand and
recreating them?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] ovs-save: Use timeout option for ovs-vsctl.

2012-10-26 Thread Gurucharan Shetty
Running ovs-save directly to save datapath configuration can
wait forever if ovsdb-server is not running. Use a timeout.

Signed-off-by: Gurucharan Shetty 
---
 utilities/ovs-save |   12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/utilities/ovs-save b/utilities/ovs-save
index 9ed14eb..01e5791 100755
--- a/utilities/ovs-save
+++ b/utilities/ovs-save
@@ -177,6 +177,10 @@ save_flows () {
 done
 }
 
+ovs_vsctl () {
+ovs-vsctl --no-wait --timeout=5 "$@"
+}
+
 save_datapaths () {
 if missing_program ovs-dpctl; then
 echo "$0: ovs-dpctl not found in $PATH" >&2
@@ -225,20 +229,20 @@ save_datapaths () {
 # For ipsec, ovs-dpctl does not show the key value pairs related
 # to certificates. Get that information from ovs-vsctl.
 if [ "${type}" = "ipsec_gre" ] ; then
-if peer_cert=`ovs-vsctl get interface \
+if peer_cert=`ovs_vsctl get interface \
 "${netdev}" options:peer_cert 2>/dev/null`; 
then
 # The option peer_cert comes with an accompanying
 # "certificate" or "use_ssl_cert"
-if certificate=`ovs-vsctl get interface "${netdev}" \
+if certificate=`ovs_vsctl get interface "${netdev}" \
 options:certificate 2>/dev/null` ; then
 
command="${command},peer_cert=${peer_cert},certificate=${certificate}"
 else
-use_ssl_cert=`ovs-vsctl get interface "${netdev}" \
+use_ssl_cert=`ovs_vsctl get interface "${netdev}" \
 options:use_ssl_cert 2>/dev/null`
 
command="${command},peer_cert=${peer_cert},use_ssl_cert=${use_ssl_cert}"
 fi
 else
-psk=`ovs-vsctl get interface "${netdev}" \
+psk=`ovs_vsctl get interface "${netdev}" \
 options:psk 2>/dev/null`
 command="${command},psk=${psk}"
 fi
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH] ovs-save: Use timeout option for ovs-vsctl.

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 11:28:42AM -0700, Gurucharan Shetty wrote:
> Running ovs-save directly to save datapath configuration can
> wait forever if ovsdb-server is not running. Use a timeout.
> 
> Signed-off-by: Gurucharan Shetty 

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


[ovs-dev] Bug#691508: ovs-vsctl: --may-exist --if-exists don't work

2012-10-26 Thread Adam Heath
On 10/26/2012 01:11 PM, Ben Pfaff wrote:
> On Fri, Oct 26, 2012 at 01:00:32PM -0500, Adam Heath wrote:
>> On 10/26/2012 11:43 AM, Ben Pfaff wrote:
>>>
>>> [snip]
>>
>> Right you are, mea-culpa.  You're right that there is an example in
>> the manpage, showing exactly what I want.  Sorry for the noise, and
>> thanks for the fast response.
> 
> It's counterintuitive and confusing.  I need to do something about it,
> at least improving the error message.

So re-open the bug, change the title, and lower the priority?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] Bug#691508: ovs-vsctl: --may-exist --if-exists don't work

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 01:35:34PM -0500, Adam Heath wrote:
> On 10/26/2012 01:11 PM, Ben Pfaff wrote:
> > On Fri, Oct 26, 2012 at 01:00:32PM -0500, Adam Heath wrote:
> >> On 10/26/2012 11:43 AM, Ben Pfaff wrote:
> >>>
> >>> [snip]
> >>
> >> Right you are, mea-culpa.  You're right that there is an example in
> >> the manpage, showing exactly what I want.  Sorry for the noise, and
> >> thanks for the fast response.
> > 
> > It's counterintuitive and confusing.  I need to do something about it,
> > at least improving the error message.
> 
> So re-open the bug, change the title, and lower the priority?

Sure, I'm happy with that, thanks!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [openflow-dev] [code] OFTest for OVS

2012-10-26 Thread Rich Lane
Hi Ben,

Would it be ok if I added dummy.py (as platforms/ovs-dummy.py) to the
OFTest repository? In the future I want to support maintaining platform
modules separately from OFTest, but the DataPlanePort interface is going to
change quite a bit before then. If ovs-dummy.py is versioned with the rest
of OFTest I will keep it up to date.

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


[ovs-dev] [PATCH] timeval: Fix occasional backtrace() related deadlock.

2012-10-26 Thread Ethan Jackson
Occasionally, backtrace() will deadlock in the signal handler
because it does some non signal safe initialization.  Specifically,
it opens a shared object.  As a work around, this patch forces
backtrace() to run outside of a signal handler, so that future
calls will perform as expected.

Signed-off-by: Ethan Jackson 
---
 lib/timeval.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/lib/timeval.c b/lib/timeval.c
index 05f1c2d..d853989 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -121,6 +121,16 @@ time_init(void)
 }
 inited = true;
 
+/* The implementation of backtrace() in glibc does some one time
+ * initialization which is not signal safe.  This can cause deadlocks if
+ * run from the signal handler.  As a workaround, force the initialization
+ * to happen here. */
+if (HAVE_EXECINFO_H) {
+void *bt[1];
+
+backtrace(bt, ARRAY_SIZE(bt));
+}
+
 memset(traces, 0, sizeof traces);
 
 if (HAVE_EXECINFO_H && CACHE_TIME) {
-- 
1.7.9.5

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


Re: [ovs-dev] [PATCH] timeval: Fix occasional backtrace() related deadlock.

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 12:36:41PM -0700, Ethan Jackson wrote:
> Occasionally, backtrace() will deadlock in the signal handler
> because it does some non signal safe initialization.  Specifically,
> it opens a shared object.  As a work around, this patch forces
> backtrace() to run outside of a signal handler, so that future
> calls will perform as expected.
> 
> Signed-off-by: Ethan Jackson 

I noticed the hang too but hadn't yet investigated.  Thanks for
finding and fixing the problem.  The patch looks good.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [openflow-dev] [code] OFTest for OVS

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 12:24:47PM -0700, Rich Lane wrote:
> Would it be ok if I added dummy.py (as platforms/ovs-dummy.py) to the
> OFTest repository? In the future I want to support maintaining platform
> modules separately from OFTest, but the DataPlanePort interface is going to
> change quite a bit before then. If ovs-dummy.py is versioned with the rest
> of OFTest I will keep it up to date.

That's OK with me.

Feel free to rename DataPlanePortUnix to something more OVS-specific
if you like.

Here's a separate patch that I applied to suppress the "Missing
pypcap" warning when the DataPlanePortUnix is used (since it doesn't
need pypcap).

diff --git a/src/python/oftest/dataplane.py b/src/python/oftest/dataplane.py
index 828f584..e23251f 100644
--- a/src/python/oftest/dataplane.py
+++ b/src/python/oftest/dataplane.py
@@ -314,17 +314,6 @@ class DataPlane:
 
 
 #
-# We use the DataPlanePort class defined here by 
-# default for all port traffic:
-#
-if have_pypcap:
-self.dppclass = DataPlanePortPcap
-else:
-self.logger.warning("Missing pypcap, VLAN tests may fail. See 
README for installation instructions.")
-self.dppclass = DataPlanePort
-
-
-#
 # The platform/config can provide a custom DataPlanePort class
 # here if you have a custom implementation with different
 # behavior. 
@@ -336,6 +325,11 @@ class DataPlane:
 if "dataplane" in self.config:
 if "portclass" in self.config["dataplane"]:
 self.dppclass = self.config["dataplane"]["portclass"]
+elif have_pypcap:
+self.dppclass = DataPlanePortPcap
+else:
+self.logger.warning("Missing pypcap, VLAN tests may fail. See 
README for installation instructions.")
+self.dppclass = DataPlanePort
 
 if self.dppclass == None:
 raise Exception("Problem determining DataPlanePort class.")
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Deprecated CAPWAP support.

2012-10-26 Thread Jesse Gross
On Sat, Oct 27, 2012 at 1:07 PM, Pravin B Shelar  wrote:
> Signed-off-by: Pravin B Shelar 

I think we should probably add a little bit of information about the
rationale and timeline to the commit message.  I would also mention
that it is deprecated in the schema documentation for CAPWAP.

>  v1.8.0 - xx xxx 
> diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
> index 39aec42..a113af6 100644
> --- a/datapath/vport-capwap.c
> +++ b/datapath/vport-capwap.c
> @@ -473,6 +473,7 @@ static struct vport *capwap_create(const struct 
> vport_parms *parms)
> struct vport *vport;
> int err;
>
> +   pr_err("%s: capwap support is deprecated\n", __func__);

It might be better to warn in userspace since more people will are
likely to look at ovs-vswitchd.log for this type of thing.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: enable encap for capwap.

2012-10-26 Thread Jesse Gross
On Sat, Oct 27, 2012 at 1:07 PM, Pravin B Shelar  wrote:
> kernel 3.5 added a switch to turn on UDP encap, capwap needs
> to enable it.
>
> Signed-off-by: Pravin B Shelar 
> ---
>  datapath/linux/compat/include/linux/udp.h |5 +
>  datapath/vport-capwap.c   |2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/datapath/linux/compat/include/linux/udp.h 
> b/datapath/linux/compat/include/linux/udp.h
> index 6fe4721..6a805b5 100644
> --- a/datapath/linux/compat/include/linux/udp.h
> +++ b/datapath/linux/compat/include/linux/udp.h
> @@ -10,4 +10,9 @@ static inline struct udphdr *udp_hdr(const struct sk_buff 
> *skb)
>  }
>  #endif /* HAVE_SKBUFF_HEADER_HELPERS */
>
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(3,5,0)
> +static inline void udp_encap_enable(void)
> +{
> +}
> +#endif
>  #endif
> diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
> index a113af6..a6d194a 100644
> --- a/datapath/vport-capwap.c
> +++ b/datapath/vport-capwap.c
> @@ -445,8 +445,8 @@ static int init_socket(struct net *net)
> capwap_net->frag_state.low_thresh   = CAPWAP_FRAG_PRUNE_MEM;
>
> inet_frags_init_net(&capwap_net->frag_state);
> -
> capwap_net->n_tunnels++;
> +   udp_encap_enable();

Can we just move this up a few lines so that incrementing the refcount
is the last thing that we do?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath: Fix sparse warning for symbol 'BUILD_BUG_ON_NOT_POWER_OF_2'

2012-10-26 Thread Jesse Gross
On Thu, Oct 25, 2012 at 1:28 PM, Pravin Shelar  wrote:
> On Thu, Oct 25, 2012 at 1:19 PM, Ben Pfaff  wrote:
>> On Sat, Oct 27, 2012 at 01:07:24PM -0700, Pravin B Shelar wrote:
>>> Signed-off-by: Pravin B Shelar 
>>
>> I'm curious, what warning does sparse give?  (You could add this to
>> the commit message.)
>>
> This symbol was moved from kernel.h to bug.h in 3.4.
> warning is as follows, I will add it to commit msg.
>
> include/linux/bug.h:15:9: warning: preprocessor token
> BUILD_BUG_ON_NOT_POWER_OF_2 redefined
> /root/ovs-cl1/stt-fl/datapath/linux/compat/include/linux/kernel.h:44:9:
> this was the original definition

Since the "correct" place for this on new kernels is in bug.h, it's
probably better to just move the whole definition there to make it
easier to find.  That would also solve this problem since bug.h
includes kernel.h
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


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

2012-10-26 Thread Jesse Gross
On Thu, Oct 25, 2012 at 1:34 PM, Ben Pfaff  wrote:
> On Thu, Oct 25, 2012 at 01:33:32PM -0700, Pravin Shelar wrote:
>> On Thu, Oct 25, 2012 at 1:22 PM, Ben Pfaff  wrote:
>> > On Sat, Oct 27, 2012 at 01:07:35PM -0700, Pravin B Shelar wrote:
>> >> Signed-off-by: Pravin B Shelar 
>> >
>> > ...
>> >
>> >> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0)
>> >> +#define NETLINK_CB_PORT_ID(skb) (NETLINK_CB((skb)).portid)
>> >> +#else
>> >> +#define NETLINK_CB_PORT_ID(skb) (NETLINK_CB((skb)).pid)
>> >> +#endif
>> >
>> > I'd be at least tempted to write:
>> >
>> > #if LINUX_VERSION_CODE < KERNEL_VERSION(3,6,0)
>> > #define portid pid
>> > #endif
>>
>> portid is pretty common name, I am not sure if we can make this type
>> of renaming cross kernel datapath.
>
> I won't argue with that.

It's worth checking whether it is possible to use portid directly
since it is nicer.

>> > and similarly for snd_portid vs. snd_pid in genetlink.h, which would
>> > skip the indirection macro.
>>
>> ok, I will change it.
>
> Well, it's up to you (and Jesse), just pointing it out.

Certainly to the extent possible, it's nice to do this with macros
that completely hide the difference.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] debian: Automatically up the slave interfaces of a bond.

2012-10-26 Thread Gurucharan Shetty
When physical interfaces are added to a bond through the
/etc/network/interfaces script, administratively 'up' the
slave interfaces.

Signed-off-by: Gurucharan Shetty 
---
 debian/ifupdown.sh |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/debian/ifupdown.sh b/debian/ifupdown.sh
index bd6aec8..f6a7750 100755
--- a/debian/ifupdown.sh
+++ b/debian/ifupdown.sh
@@ -61,6 +61,10 @@ if [ "${MODE}" = "start" ]; then
 ${OVS_EXTRA+-- $OVS_EXTRA}
 
 ifconfig "${IFACE}" up
+for slave in ${IF_OVS_BONDS}
+do
+ifconfig "${slave}" up
+done
 ;;
 *)
 exit 0
-- 
1.7.2.5

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


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

2012-10-26 Thread Jesse Gross
On Sat, Oct 27, 2012 at 1:07 PM, Pravin B Shelar  wrote:
> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> index 5b74877..04ff55f 100644
> --- a/datapath/tunnel.c
> +++ b/datapath/tunnel.c
> @@ -1007,10 +1011,9 @@ int ovs_tnl_send(struct vport *vport, struct sk_buff 
> *skb)
> iph = ip_hdr(skb);
> iph->version= 4;
> iph->ihl= sizeof(struct iphdr) >> 2;
> -   iph->frag_off   = htons(IP_DF);

I think this change belongs as part of the previous patch.

> diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
> index e466323..83c4b03 100644
> --- a/datapath/vport-capwap.c
> +++ b/datapath/vport-capwap.c
> @@ -791,8 +791,11 @@ static struct sk_buff *defrag(struct sk_buff *skb, bool 
> frag_last)
> struct frag_queue *fq;
>
> if (atomic_read(&ns_frag_state->mem) > ns_frag_state->high_thresh)
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0)
> +   inet_frag_evictor(ns_frag_state, &frag_state, true);
> +#else
> inet_frag_evictor(ns_frag_state, &frag_state);
> -
> +#endif

It looks to me like this change is coming in 3.7, not 3.6.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH v2] datapath: Remove tunnel header caching.

2012-10-26 Thread Jesse Gross
On Thu, Oct 25, 2012 at 11:14 AM, Pravin B Shelar  wrote:
> Tunnel caching was added to reduce CPU utilization on TX path
> by caching packet header, So performance gain is directly proportional
> to number of skbs transferred. But with help of offloads skb are getting
> larger. So there are less number of skbs. Therefore header caching does
> not shows similar gains we seen in past. And now kernel 3.6 has removed
> dst caching from networking which makes header caching even more tricky.
> So this commit removes header caching from OVS tunnelling.
>
> Signed-off-by: Pravin B Shelar 

We should probably add a NEWS item.

> diff --git a/datapath/tunnel.h b/datapath/tunnel.h
> index 951a6f1..f296ecb 100644
> --- a/datapath/tunnel.h
> +++ b/datapath/tunnel.h
> -   /*
> -* Updates the cached header of a packet to match the actual packet
> +* Updates the header of a packet to match the actual packet
>  * data.  Typical things that might need to be updated are length,
>  * checksum, etc.  The IP header will have already been updated and 
> this
>  * is the final step before transmission.  Returns a linked list of

Is this still really updating the header now?  It's really creating it
from scratch.

> diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
> index d02d4ec..3a877f3 100644
> --- a/datapath/vport-gre.c
> +++ b/datapath/vport-gre.c
> @@ -196,7 +162,6 @@ static struct sk_buff *gre_update_header(const struct 
> vport *vport,
>  * packet originally had DF set.
>  */
> skb->local_df = 1;
> -   __ip_select_ident(ip_hdr(skb), dst, 0);

Why is this no longer needed?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] debian: Automatically up the slave interfaces of a bond.

2012-10-26 Thread Ben Pfaff
On Fri, Oct 26, 2012 at 02:29:30PM -0700, Gurucharan Shetty wrote:
> When physical interfaces are added to a bond through the
> /etc/network/interfaces script, administratively 'up' the
> slave interfaces.
> 
> Signed-off-by: Gurucharan Shetty 

This seems reasonable.

Thanks,

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


Re: [ovs-dev] [PATCH 1/4] datapath: Add basic MPLS support to kernel

2012-10-26 Thread Simon Horman
On Fri, Oct 26, 2012 at 05:08:31PM +0900, Isaku Yamahata wrote:
> Some inlined comments in case label == 0. I don't find any dependency on it.

Thanks Yamahata-san,

I agree with both of your comments, I should have fixed these up
when adding tracking of the stack depth (which was also suggested by you).
O will incorporate the changes you suggest below in the next version of
this patch.

> On Thu, Oct 18, 2012 at 06:02:18PM +0900, Simon Horman wrote:
> > diff --git a/datapath/datapath.c b/datapath/datapath.c
> > index a6915fb..e606ae1 100644
> > --- a/datapath/datapath.c
> > +++ b/datapath/datapath.c
> > @@ -74,6 +74,43 @@ int ovs_net_id __read_mostly;
> >  int (*ovs_dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int 
> > cmd);
> >  EXPORT_SYMBOL(ovs_dp_ioctl_hook);
> >  
> > +void skb_cb_set_mpls_bos(struct sk_buff *skb)
> > +{
> > +   struct ethhdr *eth;
> > +   int nh_ofs;
> > +   __be16 dl_type = 0;
> > +
> > +   skb_reset_mac_header(skb);
> > +
> > +   eth = eth_hdr(skb);
> > +   nh_ofs = sizeof(struct ethhdr);
> > +   if (likely(eth->h_proto >= htons(ETH_TYPE_MIN))) {
> > +   dl_type = eth->h_proto;
> > +
> > +   while (dl_type == htons(ETH_P_8021Q) &&
> > +   skb->len >= nh_ofs + sizeof(struct vlan_hdr)) {
> > +   struct vlan_hdr *vh = (struct vlan_hdr*)(skb->data + 
> > nh_ofs);
> > +   dl_type = vh->h_vlan_encapsulated_proto;
> > +   nh_ofs += sizeof(struct vlan_hdr);
> > +   }
> > +
> > +   OVS_CB(skb)->mpls_bos = nh_ofs;
> > +   } else {
> > +   OVS_CB(skb)->mpls_bos = 0;
> > +   }
> > +}
> > +
> > +unsigned char *skb_cb_mpls_bos(const struct sk_buff *skb)
> > +{
> > +   return OVS_CB(skb)->mpls_bos ?
> > +   skb_mac_header(skb) + OVS_CB(skb)->mpls_bos : 0;
> > +}
> > +
> > +ptrdiff_t skb_cb_mpls_bos_offset(const struct sk_buff *skb)
> > +{
> > +   return OVS_CB(skb)->mpls_bos;
> > +}
> > +
> >  /**
> >   * DOC: Locking:
> >   *
> > @@ -663,6 +700,9 @@ static int validate_actions(const struct nlattr *attr,
> > static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
> > [OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
> > [OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
> > +   [OVS_ACTION_ATTR_PUSH_MPLS] = sizeof(struct 
> > ovs_action_push_mpls),
> > +   [OVS_ACTION_ATTR_POP_MPLS] = sizeof(__be16),
> > +   [OVS_ACTION_ATTR_SET_MPLS] = sizeof(__be32),
> > [OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct 
> > ovs_action_push_vlan),
> > [OVS_ACTION_ATTR_POP_VLAN] = 0,
> > [OVS_ACTION_ATTR_SET] = (u32)-1,
> > @@ -691,6 +731,24 @@ static int validate_actions(const struct nlattr *attr,
> > return -EINVAL;
> > break;
> >  
> > +   case OVS_ACTION_ATTR_PUSH_MPLS: {
> > +   const struct ovs_action_push_mpls *mpls = nla_data(a);
> > +   if (mpls->mpls_ethertype != htons(ETH_P_MPLS_UC) &&
> > +   mpls->mpls_ethertype != htons(ETH_P_MPLS_MC))
> > +   return -EINVAL;
> > +   break;
> > +   }
> > +
> > +   case OVS_ACTION_ATTR_POP_MPLS:
> > +   break;
> > +
> > +   case OVS_ACTION_ATTR_SET_MPLS: {
> > +   __be32 mpls_label = nla_get_be32(a);
> > +   if (mpls_label == htonl(0)) {
> > +   return -EINVAL;
> > +   }
> 
> Is this necessary?
> At least, this is inconsistent with case OVS_ACTION_ATTR_PUSH_MPLS.
> 
> 
> > +   break;
> > +   }
> >  
> > case OVS_ACTION_ATTR_POP_VLAN:
> > break;
> > @@ -805,6 +863,8 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, 
> > struct genl_info *info)
> > OVS_CB(packet)->flow = flow;
> > packet->priority = flow->key.phy.priority;
> >  
> > +   skb_cb_set_mpls_bos(packet);
> > +
> > rcu_read_lock();
> > dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
> > err = -ENODEV;
> 
> 
> ... snip ...
> 
> > diff --git a/datapath/flow.h b/datapath/flow.h
> > index 02c563a..eb3fe49 100644
> > --- a/datapath/flow.h
> > +++ b/datapath/flow.h
> > @@ -53,6 +53,9 @@ struct sw_flow_key {
> > __be16 type;/* Ethernet frame type. */
> > } eth;
> > struct {
> > +   __be32 top_label;   /* 0 if no MPLS, top label from stack 
> > otherwise */
> 
> I think that the validity of top_label is guaranteed by ethertype = MPLS.
> So top_label can be 0 as valid label value.
> 
> 
> > +   } mpls;
> > +   struct {
> > u8 proto;   /* IP protocol or lower 8 bits of ARP 
> > opcode. */
> > u8 tos; /* IP ToS. */
> > u8 ttl; /* IP TTL/hop limit. */
> > @@ -126,6 +129,10 @@ struct arp_eth_

Re: [ovs-dev] [PATCH 3/4] User-Space MPLS actions and matches

2012-10-26 Thread Simon Horman
On Fri, Oct 26, 2012 at 05:22:13PM +0900, Isaku Yamahata wrote:
> On Thu, Oct 18, 2012 at 06:02:20PM +0900, Simon Horman wrote:
> > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > index 1fb0304..13eb367 100644
> > --- a/lib/ofp-actions.c
> > +++ b/lib/ofp-actions.c
> > @@ -378,6 +378,26 @@ ofpact_from_nxast(const union ofp_action *a, enum 
> > ofputil_action_code code,
> >  case OFPUTIL_NXAST_CONTROLLER:
> >  controller_from_openflow((const struct nx_action_controller *) a, 
> > out);
> >  break;
> > +
> > +case OFPUTIL_NXAST_PUSH_MPLS: {
> > +struct nx_action_push *nxap = (struct nx_action_push *)a;
> > +if (nxap->ethertype != htons(ETH_TYPE_MPLS) &&
> > +nxap->ethertype != htons(ETH_TYPE_MPLS_MCAST)) {
> > +return OFPERR_OFPBAC_BAD_ARGUMENT;
> > +}
> > +ofpact_put_PUSH_MPLS(out)->ethertype = nxap->ethertype;
> > +break;
> > +}
> > +
> > +case OFPUTIL_NXAST_POP_MPLS: {
> > +struct nx_action_pop_mpls *nxapm = (struct nx_action_pop_mpls *)a;
> > +if (nxapm->ethertype == htons(ETH_TYPE_MPLS) ||
> > +nxapm->ethertype == htons(ETH_TYPE_MPLS_MCAST)) {
> > +return OFPERR_OFPBAC_BAD_ARGUMENT;
> > +}
> > +ofpact_put_POP_MPLS(out)->ethertype = nxapm->ethertype;
> > +break;
> > +}
> >  }
> >  
> >  return error;
> > @@ -722,6 +742,26 @@ ofpact_from_openflow11(const union ofp_action *a, 
> > struct ofpbuf *out)
> >  return nxm_reg_load_from_openflow12_set_field(
> >  (const struct ofp12_action_set_field *)a, out);
> >  
> > +case OFPUTIL_OFPAT11_PUSH_MPLS: {
> > +struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
> > +if (oap->ethertype != htons(ETH_TYPE_MPLS) &&
> > +oap->ethertype != htons(ETH_TYPE_MPLS_MCAST)) {
> > +return OFPERR_OFPBAC_BAD_ARGUMENT;
> > +}
> > +ofpact_put_PUSH_MPLS(out)->ethertype = oap->ethertype;
> > +break;
> > +}
> > +
> > +case OFPUTIL_OFPAT11_POP_MPLS: {
> > +struct ofp11_action_pop_mpls *oapm = (struct ofp11_action_pop_mpls 
> > *)a;
> > +if (oapm->ethertype == htons(ETH_TYPE_MPLS) ||
> > +oapm->ethertype == htons(ETH_TYPE_MPLS_MCAST)) {
> > +return OFPERR_OFPBAC_BAD_ARGUMENT;
> > +}
> > +ofpact_put_POP_MPLS(out)->ethertype = oapm->ethertype;
> > +break;
> > +}
> > +
> >  #define NXAST_ACTION(ENUM, STRUCT, EXTENSIBLE, NAME) case OFPUTIL_##ENUM:
> >  #include "ofp-util.def"
> >  return ofpact_from_nxast(a, code, out);
> > @@ -1052,6 +1092,8 @@ ofpact_check__(const struct ofpact *a, int max_ports)
> >  
> >  case OFPACT_NOTE:
> >  case OFPACT_EXIT:
> > +case OFPACT_PUSH_MPLS:
> > +case OFPACT_POP_MPLS:
> >  return 0;
> >  
> >  case OFPACT_CLEAR_ACTIONS:
> > @@ -1260,6 +1302,16 @@ ofpact_to_nxast(const struct ofpact *a, struct 
> > ofpbuf *out)
> >  ofputil_put_NXAST_EXIT(out);
> >  break;
> >  
> > +case OFPACT_PUSH_MPLS:
> > +ofputil_put_NXAST_PUSH_MPLS(out)->ethertype =
> > +ofpact_get_PUSH_MPLS(a)->ethertype;
> > +break;
> > +
> > +case OFPACT_POP_MPLS:
> > +ofputil_put_NXAST_POP_MPLS(out)->ethertype =
> > +ofpact_get_POP_MPLS(a)->ethertype;
> > +break;
> > +
> >  case OFPACT_OUTPUT:
> >  case OFPACT_ENQUEUE:
> >  case OFPACT_SET_VLAN_VID:
> > @@ -1384,6 +1436,8 @@ ofpact_to_openflow10(const struct ofpact *a, struct 
> > ofpbuf *out)
> >  case OFPACT_AUTOPATH:
> >  case OFPACT_NOTE:
> >  case OFPACT_EXIT:
> > +case OFPACT_PUSH_MPLS:
> > +case OFPACT_POP_MPLS:
> >  ofpact_to_nxast(a, out);
> >  break;
> >  }
> > @@ -1476,6 +1530,16 @@ ofpact_to_openflow11(const struct ofpact *a, struct 
> > ofpbuf *out)
> >  = htons(ofpact_get_SET_L4_DST_PORT(a)->port);
> >  break;
> >  
> > +case OFPACT_PUSH_MPLS:
> > +ofputil_put_OFPAT11_PUSH_MPLS(out)->ethertype =
> > +ofpact_get_PUSH_MPLS(a)->ethertype;
> > +break;
> > +
> > +case OFPACT_POP_MPLS:
> > +ofputil_put_OFPAT11_POP_MPLS(out)->ethertype =
> > +ofpact_get_POP_MPLS(a)->ethertype;
> > +break;
> > +
> >  case OFPACT_CLEAR_ACTIONS:
> >  case OFPACT_GOTO_TABLE:
> >  NOT_REACHED();
> 
> Even when NX MPLS actions is used, retrieved actions always results in
> OF1.1+ MPLS actions. We can keep it easily.
> 
> Set ofpact->compat = OFPUTIL_NX_xxx_MPLS in ofpact_from_nxact().
> And then
> if (a->compat == OFPUTIL_NXAST_xxx_MPLS) {
> NX MPLS action
> } else {
> OF11+ MPLS action
> }

Hi,

I'm not sure that this is a problem as the behaviour of the actions is the
same. Am I missing something?

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


[ovs-dev] [PATCH 0/8 v2.5] Basic MPLS actions and matches

2012-10-26 Thread Simon Horman
Hi,

This series implements basic MPLS actions and matches based on work by
Ravi K, Leo Alterman and Yamahata-san.

The main limitation of this implementation is that it only
supports manipulating the outer-most MPLS label.

Key difference between the previous post, v2.4:

* Various clean-ups and fises to
  "User-Space MPLS actions and matches"
  as noted in its changelog.

* New patches to add support for TLS actions

* Inclusion of Justin Pettit's patch
  "flow: Set ttl in flow_compose()",
  which is a dependency of the
  "Add support for copy_ttl_out action" patch.


The following changes since commit f43e80e023378a2c1ef18d3caee9b76d6d2a6d23:

  poll-loop: Log backtraces when CPU usage is high. (2012-10-25 11:14:07 -0700)

are available in the git repository at:

  git://github.com/horms/openvswitch.git devel/mpls

for you to fetch changes up to 0d2c5e8efbf05c132ff273c3013c933df2b9759a:

  Add support for copy_ttl_in action (2012-10-27 14:52:02 +0900)


Justin Pettit (1):
  flow: Set ttl in flow_compose().

Simon Horman (7):
  datapath: Add basic MPLS support to kernel
  nx-match: Do not check pre-requisites for load actions
  User-Space MPLS actions and matches
  Add support for dec_mpls_ttl action
  Add support for set_mpls_ttl action
  Add support for copy_ttl_out action
  Add support for copy_ttl_in action

 datapath/actions.c  |   81 
 datapath/datapath.c |   53 
 datapath/datapath.h |7 +
 datapath/flow.c |   30 +
 datapath/flow.h |7 +
 datapath/vport.c|2 +
 include/linux/openvswitch.h |   34 -
 include/openflow/nicira-ext.h   |   39 ++
 include/openflow/openflow-1.2.h |2 +
 lib/autopath.c  |6 +-
 lib/autopath.h  |3 +-
 lib/bundle.c|7 +-
 lib/bundle.h|3 +-
 lib/dpif-netdev.c   |   13 ++
 lib/flow.c  |  123 --
 lib/flow.h  |   17 ++-
 lib/learn.c |   10 +-
 lib/learn.h |2 +-
 lib/match.c |   88 -
 lib/match.h |6 +
 lib/meta-flow.c |  138 ++--
 lib/meta-flow.h |   13 +-
 lib/multipath.c |7 +-
 lib/multipath.h |3 +-
 lib/nx-match.c  |   36 --
 lib/nx-match.h  |6 +-
 lib/odp-util.c  |  171 -
 lib/ofp-actions.c   |  186 +--
 lib/ofp-actions.h   |   33 -
 lib/ofp-parse.c |   46 +++
 lib/ofp-print.c |9 +-
 lib/ofp-util.c  |   28 +++-
 lib/ofp-util.def|   12 ++
 lib/ofpbuf.c|8 +-
 lib/ofpbuf.h|1 +
 lib/packets.c   |  268 ++-
 lib/packets.h   |   91 +
 ofproto/ofproto-dpif.c  |  166 +++-
 ofproto/ofproto.c   |4 +-
 tests/odp.at|   13 ++
 tests/ofproto-dpif.at   |  268 ---
 tests/test-bundle.c |1 +
 tests/test-multipath.c  |1 +
 utilities/ovs-dpctl.c   |   18 ++-
 utilities/ovs-ofctl.8.in|   46 +++
 45 files changed, 1987 insertions(+), 119 deletions(-)
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH 1/8] flow: Set ttl in flow_compose().

2012-10-26 Thread Simon Horman
From: Justin Pettit 

Thanks to Ben Pfaff for immediately pinpointing the likely location of
the issue.

Signed-off-by: Justin Pettit 
Signed-off-by: Simon Horman 
---
 lib/flow.c|1 +
 tests/ofproto-dpif.at |   36 ++--
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 06478da..ad59934 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -767,6 +767,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
 b->l3 = ip = ofpbuf_put_zeros(b, sizeof *ip);
 ip->ip_ihl_ver = IP_IHL_VER(5, 4);
 ip->ip_tos = flow->nw_tos;
+ip->ip_ttl = flow->nw_ttl;
 ip->ip_proto = flow->nw_proto;
 ip->ip_src = flow->nw_src;
 ip->ip_dst = flow->nw_dst;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bc2362d..8920d6f 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -263,13 +263,13 @@ done
 OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via no_match) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=9
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9
 tcp_csum:0
 dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via no_match) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=9
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9
 tcp_csum:0
 dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via no_match) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=9
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=9
 tcp_csum:0
 ])
 
 dnl Singleton controller action.
@@ -282,13 +282,13 @@ done
 OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via action) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=10
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10
 tcp_csum:0
 dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via action) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=10
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10
 tcp_csum:0
 dnl
 OFPT_PACKET_IN (xid=0x0): total_len=60 in_port=1 (via action) data_len=60 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=10
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,vlan_tci=0x,dl_src=10:11:11:11:11:11,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10
 tcp_csum:0
 ])
 
 dnl Modified controller action.
@@ -301,13 +301,13 @@ done
 OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 OFPT_PACKET_IN (xid=0x0): total_len=64 in_port=1 (via action) data_len=64 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,dl_vlan=15,dl_vlan_pcp=0,dl_src=30:33:33:33:33:33,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=8,tp_dst=10
 tcp_csum:0
+priority=0,tcp,metadata=0,in_port=0,dl_vlan=15,dl_vlan_pcp=0,dl_src=30:33:33:33:33:33,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=8,tp_dst=10
 tcp_csum:0
 dnl
 OFPT_PACKET_IN (xid=0x0): total_len=64 in_port=1 (via action) data_len=64 
(unbuffered)
-priority=0,tcp,metadata=0,in_port=0,dl_vlan=15,dl_vlan_pcp=0,dl_src=30:33:33:33:33:33,

[ovs-dev] [PATCH 3/8] nx-match: Do not check pre-requisites for load actions

2012-10-26 Thread Simon Horman
There are (or at least will be) cases where this check can produce false
positives.  For example, a flow which matches a non-MPLS packet and then
applies an MPLS push action followed by an action to load the MPLS label.

I believe this is the same problem that was recently discussed in
relation to set-field. In that case my understanding is that consensus
was to omit the check and potentially revisit the problem later.

Signed-off-by: Simon Horman 

---

v2.2
* No change

v2.1
* Initial post
---
 lib/autopath.c|6 +++---
 lib/autopath.h|3 +--
 lib/bundle.c  |7 +++
 lib/bundle.h  |3 +--
 lib/learn.c   |   10 +-
 lib/learn.h   |2 +-
 lib/meta-flow.c   |   14 +-
 lib/meta-flow.h   |4 ++--
 lib/multipath.c   |7 +++
 lib/multipath.h   |3 +--
 lib/nx-match.c|   16 
 lib/nx-match.h|6 ++
 lib/ofp-actions.c |   23 +++
 lib/ofp-actions.h |2 +-
 ofproto/ofproto.c |4 ++--
 15 files changed, 49 insertions(+), 61 deletions(-)

diff --git a/lib/autopath.c b/lib/autopath.c
index 9da6463..35f9737 100644
--- a/lib/autopath.c
+++ b/lib/autopath.c
@@ -81,16 +81,16 @@ autopath_from_openflow(const struct nx_action_autopath *nap,
 return OFPERR_OFPBAC_BAD_ARGUMENT;
 }
 
-return autopath_check(autopath, NULL);
+return autopath_check(autopath);
 }
 
 enum ofperr
-autopath_check(const struct ofpact_autopath *autopath, const struct flow *flow)
+autopath_check(const struct ofpact_autopath *autopath)
 {
 VLOG_WARN_ONCE("The autopath action is deprecated and may be removed in"
" February 2013.  Please email dev@openvswitch.org with"
" concerns.");
-return mf_check_dst(&autopath->dst, flow);
+return mf_check_dst(&autopath->dst);
 }
 
 void
diff --git a/lib/autopath.h b/lib/autopath.h
index 337e7d1..ef46456 100644
--- a/lib/autopath.h
+++ b/lib/autopath.h
@@ -33,8 +33,7 @@ void autopath_parse(struct ofpact_autopath *, const char *);
 
 enum ofperr autopath_from_openflow(const struct nx_action_autopath *,
struct ofpact_autopath *);
-enum ofperr autopath_check(const struct ofpact_autopath *,
-   const struct flow *);
+enum ofperr autopath_check(const struct ofpact_autopath *);
 void autopath_to_nxast(const struct ofpact_autopath *,
struct ofpbuf *openflow);
 
diff --git a/lib/bundle.c b/lib/bundle.c
index 92ac1e1..1ba5be5 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -173,20 +173,19 @@ bundle_from_openflow(const struct nx_action_bundle *nab,
 ofpact_update_len(ofpacts, &bundle->ofpact);
 
 if (!error) {
-error = bundle_check(bundle, OFPP_MAX, NULL);
+error = bundle_check(bundle, OFPP_MAX);
 }
 return error;
 }
 
 enum ofperr
-bundle_check(const struct ofpact_bundle *bundle, int max_ports,
- const struct flow *flow)
+bundle_check(const struct ofpact_bundle *bundle, int max_ports)
 {
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 size_t i;
 
 if (bundle->dst.field) {
-enum ofperr error = mf_check_dst(&bundle->dst, flow);
+enum ofperr error = mf_check_dst(&bundle->dst);
 if (error) {
 return error;
 }
diff --git a/lib/bundle.h b/lib/bundle.h
index 5b6bb67..a41f3f6 100644
--- a/lib/bundle.h
+++ b/lib/bundle.h
@@ -39,8 +39,7 @@ uint16_t bundle_execute(const struct ofpact_bundle *, const 
struct flow *,
 void *aux);
 enum ofperr bundle_from_openflow(const struct nx_action_bundle *,
  struct ofpbuf *ofpact);
-enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports,
- const struct flow *);
+enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports);
 void bundle_to_nxast(const struct ofpact_bundle *, struct ofpbuf *of10);
 void bundle_parse(const char *, struct ofpbuf *ofpacts);
 void bundle_parse_load(const char *, struct ofpbuf *ofpacts);
diff --git a/lib/learn.c b/lib/learn.c
index b9bbc97..f9d5527 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -168,7 +168,7 @@ learn_from_openflow(const struct nx_action_learn *nal, 
struct ofpbuf *ofpacts)
 /* Checks that 'learn' is a valid action on 'flow'.  Returns 0 if it is valid,
  * otherwise an OFPERR_*. */
 enum ofperr
-learn_check(const struct ofpact_learn *learn, const struct flow *flow)
+learn_check(const struct ofpact_learn *learn)
 {
 const struct ofpact_learn_spec *spec;
 struct match match;
@@ -179,7 +179,7 @@ learn_check(const struct ofpact_learn *learn, const struct 
flow *flow)
 
 /* Check the source. */
 if (spec->src_type == NX_LEARN_SRC_FIELD) {
-error = mf_check_src(&spec->src, flow);
+error = mf_check_src(&spec->src);
 if (error) {
 return error;
 }
@@ -188,7 +188,7 @@ learn_check(const str

[ovs-dev] [PATCH 2/8] datapath: Add basic MPLS support to kernel

2012-10-26 Thread Simon Horman
Allow datapath to recognize and extract MPLS labels into flow keys
and execute actions which push, pop, and set labels on packets.

Based heavily on work by Leo Alterman and Ravi K.

Cc: Isaku Yamahata 
Cc: Ravi K 
Cc: Leo Alterman 
Signed-off-by: Simon Horman 

---

v2.3
* s/mpls_stack/mpls_bos/
  This is in keeping with the naming used in the OpenFlow 1.3 specification

v2.2
* Call skb_reset_mac_header() in skb_cb_set_mpls_stack()
  eth_hdr(skb) is non-NULL when called in skb_cb_set_mpls_stack().
* Add a call to skb_cb_set_mpls_stack() in ovs_packet_cmd_execute().
  I apologise that I have mislaid my notes on this but
  it avoids a kernel panic. I can investigate again if necessary.
* Use struct ovs_action_push_mpls instead of
  __be16 to decode OVS_ACTION_ATTR_PUSH_MPLS in validate_actions(). This is
  consistent with the data format for the attribute.
* Indentation fix in skb_cb_mpls_stack(). [cosmetic]

v2.1
* Manual rebase

Previous version(s) by Leo Alterman
---
 datapath/actions.c  |   81 +++
 datapath/datapath.c |   60 
 datapath/datapath.h |7 
 datapath/flow.c |   30 
 datapath/flow.h |7 
 datapath/vport.c|2 ++
 include/linux/openvswitch.h |   34 +-
 lib/dpif-netdev.c   |4 +++
 lib/odp-util.c  |9 +
 9 files changed, 233 insertions(+), 1 deletion(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 4649c05..b73b4a7 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -48,6 +48,67 @@ static int make_writable(struct sk_buff *skb, int write_len)
return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
 }
 
+static __be16 get_ethertype(const struct sk_buff *skb)
+{
+   struct ethhdr *hdr = (struct ethhdr *)(skb_cb_mpls_bos(skb) - ETH_HLEN);
+   return hdr->h_proto;
+}
+
+static void set_ethertype(struct sk_buff *skb, const __be16 ethertype)
+{
+   struct ethhdr *hdr = (struct ethhdr *)(skb_cb_mpls_bos(skb) - ETH_HLEN);
+   hdr->h_proto = ethertype;
+}
+
+static int push_mpls(struct sk_buff *skb, const struct ovs_action_push_mpls 
*mpls)
+{
+   u32 l2_size;
+   __be32 *new_mpls_label;
+
+   if (skb_cow_head(skb, MPLS_HLEN) < 0) {
+   kfree_skb(skb);
+   return -ENOMEM;
+   }
+
+   l2_size = skb_cb_mpls_bos_offset(skb);
+   skb_push(skb, MPLS_HLEN);
+   memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb), l2_size);
+   skb_reset_mac_header(skb);
+
+   new_mpls_label = (__be32 *)(skb_mac_header(skb) + l2_size);
+   *new_mpls_label = mpls->mpls_label;
+
+   set_ethertype(skb, mpls->mpls_ethertype);
+   return 0;
+}
+
+static int pop_mpls(struct sk_buff *skb, const __be16 *ethertype)
+{
+   __be16 current_ethertype = get_ethertype(skb);
+   if (current_ethertype == htons(ETH_P_MPLS_UC) ||
+   current_ethertype == htons(ETH_P_MPLS_MC)) {
+   u32 l2_size = skb_cb_mpls_bos_offset(skb);
+
+   memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb), 
l2_size);
+
+   skb_pull(skb, MPLS_HLEN);
+   skb_reset_mac_header(skb);
+
+   set_ethertype(skb, *ethertype);
+   }
+   return 0;
+}
+
+static int set_mpls(struct sk_buff *skb, const __be32 *mpls_label)
+{
+   __be16 current_ethertype = get_ethertype(skb);
+   if (current_ethertype == htons(ETH_P_MPLS_UC) ||
+   current_ethertype == htons(ETH_P_MPLS_MC)) {
+   memcpy(skb_cb_mpls_bos(skb), mpls_label, sizeof(__be32));
+   }
+   return 0;
+}
+
 /* remove VLAN header from packet and update csum accordingly. */
 static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 {
@@ -101,6 +162,9 @@ static int pop_vlan(struct sk_buff *skb)
return err;
 
__vlan_hwaccel_put_tag(skb, ntohs(tci));
+
+   /* update pointer to MPLS label stack */
+   skb_cb_set_mpls_bos(skb);
return 0;
 }
 
@@ -121,6 +185,9 @@ static int push_vlan(struct sk_buff *skb, const struct 
ovs_action_push_vlan *vla
 
}
__vlan_hwaccel_put_tag(skb, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
+
+   /* update pointer to MPLS label stack */
+   skb_cb_set_mpls_bos(skb);
return 0;
 }
 
@@ -414,6 +481,20 @@ static int do_execute_actions(struct datapath *dp, struct 
sk_buff *skb,
output_userspace(dp, skb, a);
break;
 
+   case OVS_ACTION_ATTR_PUSH_MPLS:
+   err = push_mpls(skb, nla_data(a));
+   if (unlikely(err)) /* skb already freed. */
+   return err;
+   break;
+
+   case OVS_ACTION_ATTR_POP_MPLS:
+   err = pop_mpls(skb, nla_data(a));
+   break;
+
+   case OVS_ACTION_AT

[ovs-dev] [PATCH 5/8] Add support for dec_mpls_ttl action

2012-10-26 Thread Simon Horman
This adds support for the OpenFlow 1.1+ dec_mpls_ttl action.
And also adds an NX dec_mpls_ttl action.

The handling of the TTL modification is entirely handled in userspace.

Signed-off-by: Simon Horman 
---
 include/openflow/nicira-ext.h |1 +
 lib/flow.c|8 
 lib/ofp-actions.c |   20 
 lib/ofp-actions.h |1 +
 lib/ofp-parse.c   |5 +
 lib/ofp-util.def  |2 ++
 lib/packets.c |2 +-
 lib/packets.h |1 +
 ofproto/ofproto-dpif.c|   28 
 tests/ofproto-dpif.at |   21 +
 utilities/ovs-ofctl.8.in  |9 +
 11 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 65bae3b..209e7dc 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,7 @@ enum nx_action_subtype {
 NXAST_WRITE_METADATA,   /* struct nx_action_write_metadata */
 NXAST_PUSH_MPLS,/* struct nx_action_push_mpls */
 NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+NXAST_DEC_MPLS_TTL, /* struct nx_action_header */
 };
 
 /* Header for Nicira-defined actions. */
diff --git a/lib/flow.c b/lib/flow.c
index 7995a7b..0d7bbf4 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -798,6 +798,14 @@ flow_set_mpls_label(struct flow *flow, ovs_be32 label)
 set_mpls_lse_label(&flow->mpls_lse, label);
 }
 
+/* Sets the MPLS TTL that 'flow' matches to 'ttl', which should be in the
+ * range 0...255. */
+void
+flow_set_mpls_ttl(struct flow *flow, uint8_t ttl)
+{
+set_mpls_lse_ttl(&flow->mpls_lse, ttl);
+}
+
 /* Sets the MPLS TC that 'flow' matches to 'tc', which should be in the
  * range 0...7. */
 void
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 9376ce9..475901b 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -412,6 +412,10 @@ ofpact_from_nxast(const union ofp_action *a, enum 
ofputil_action_code code,
 break;
 }
 
+case OFPUTIL_NXAST_DEC_MPLS_TTL:
+ofpact_put_DEC_MPLS_TTL(out);
+break;
+
 case OFPUTIL_NXAST_POP_MPLS: {
 struct nx_action_pop_mpls *nxapm = (struct nx_action_pop_mpls *)a;
 if (nxapm->ethertype == htons(ETH_TYPE_MPLS) ||
@@ -779,6 +783,10 @@ ofpact_from_openflow11(const union ofp_action *a, struct 
ofpbuf *out)
 return nxm_reg_load_from_openflow12_set_field(
 (const struct ofp12_action_set_field *)a, out);
 
+case OFPUTIL_OFPAT11_DEC_MPLS_TTL:
+ofpact_put_DEC_MPLS_TTL(out);
+break;
+
 case OFPUTIL_OFPAT11_PUSH_MPLS: {
 struct ofp11_action_push *oap = (struct ofp11_action_push *)a;
 if (oap->ethertype != htons(ETH_TYPE_MPLS) &&
@@ -1121,6 +1129,7 @@ ofpact_check__(const struct ofpact *a, int max_ports)
 return nxm_reg_load_check(ofpact_get_REG_LOAD(a));
 
 case OFPACT_DEC_TTL:
+case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_SET_QUEUE:
 case OFPACT_POP_QUEUE:
@@ -1349,6 +1358,10 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf 
*out)
 ofpact_dec_ttl_to_nxast(ofpact_get_DEC_TTL(a), out);
 break;
 
+case OFPACT_DEC_MPLS_TTL:
+ofputil_put_NXAST_DEC_MPLS_TTL(out);
+break;
+
 case OFPACT_SET_TUNNEL:
 ofpact_set_tunnel_to_nxast(ofpact_get_SET_TUNNEL(a), out);
 break;
@@ -1518,6 +1531,7 @@ ofpact_to_openflow10(const struct ofpact *a, struct 
ofpbuf *out)
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
 case OFPACT_DEC_TTL:
+case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_WRITE_METADATA:
 case OFPACT_SET_QUEUE:
@@ -1664,6 +1678,7 @@ ofpact_to_openflow11(const struct ofpact *a, struct 
ofpbuf *out)
 case OFPACT_BUNDLE:
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
+case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_SET_QUEUE:
 case OFPACT_POP_QUEUE:
@@ -1785,6 +1800,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, 
uint16_t port)
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
 case OFPACT_DEC_TTL:
+case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_WRITE_METADATA:
 case OFPACT_SET_QUEUE:
@@ -2004,6 +2020,10 @@ ofpact_format(const struct ofpact *a, struct ds *s)
 print_dec_ttl(ofpact_get_DEC_TTL(a), s);
 break;
 
+case OFPACT_DEC_MPLS_TTL:
+ds_put_cstr(s, "dec_mpls_ttl");
+break;
+
 case OFPACT_SET_TUNNEL:
 tunnel = ofpact_get_SET_TUNNEL(a);
 ds_put_format(s, "set_tunnel%s:%#"PRIx64,
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index bfa4617..f051255 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -70,6 +70,7 @@
 DEFINE_OFPACT(REG_MOVE,ofpact_reg_move,  ofpact)\
 DEFINE_OFPACT(REG_LOAD,ofpact_reg_load,  of

[ovs-dev] [PATCH 6/8] Add support for set_mpls_ttl action

2012-10-26 Thread Simon Horman
This adds support for the OpenFlow 1.1+ set_mpls_ttl action.
And also adds an NX set_mpls_ttl action.

The handling of the TTL modification is entirely handled in userspace.

Signed-off-by: Simon Horman 
---
 include/openflow/nicira-ext.h |   12 
 lib/ofp-actions.c |   26 +
 lib/ofp-actions.h |   10 +++
 lib/ofp-parse.c   |   17 +++
 lib/ofp-util.def  |2 ++
 ofproto/ofproto-dpif.c|   13 +
 tests/ofproto-dpif.at |   63 +
 utilities/ovs-ofctl.8.in  |4 +++
 8 files changed, 147 insertions(+)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 209e7dc..fd07357 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,7 @@ enum nx_action_subtype {
 NXAST_WRITE_METADATA,   /* struct nx_action_write_metadata */
 NXAST_PUSH_MPLS,/* struct nx_action_push_mpls */
 NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+NXAST_SET_MPLS_TTL, /* struct nx_action_ttl */
 NXAST_DEC_MPLS_TTL, /* struct nx_action_header */
 };
 
@@ -2235,4 +2236,15 @@ struct nx_action_pop_mpls {
 };
 OFP_ASSERT(sizeof(struct nx_action_pop_mpls) == 16);
 
+/* Action structure for NXAST_SET_MPLS_TTL. */
+struct nx_action_mpls_ttl {
+ovs_be16 type;  /* OFPAT_POP_MPLS. */
+ovs_be16 len;   /* Length is 8. */
+ovs_be32 vendor;/* NX_VENDOR_ID. */
+ovs_be16 subtype;   /* NXAST_SET_MPLS_TTL. */
+uint8_t  ttl;   /* TTL */
+uint8_t  pad[5];
+};
+OFP_ASSERT(sizeof(struct nx_action_pop_mpls) == 16);
+
 #endif /* openflow/nicira-ext.h */
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 475901b..dea97da 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -412,6 +412,12 @@ ofpact_from_nxast(const union ofp_action *a, enum 
ofputil_action_code code,
 break;
 }
 
+case OFPUTIL_NXAST_SET_MPLS_TTL: {
+struct nx_action_mpls_ttl *nxamt = (struct nx_action_mpls_ttl *)a;
+ofpact_put_SET_MPLS_TTL(out)->ttl = nxamt->ttl;
+break;
+}
+
 case OFPUTIL_NXAST_DEC_MPLS_TTL:
 ofpact_put_DEC_MPLS_TTL(out);
 break;
@@ -783,6 +789,12 @@ ofpact_from_openflow11(const union ofp_action *a, struct 
ofpbuf *out)
 return nxm_reg_load_from_openflow12_set_field(
 (const struct ofp12_action_set_field *)a, out);
 
+case OFPUTIL_OFPAT11_SET_MPLS_TTL: {
+struct ofp11_action_mpls_ttl *oamt = (struct ofp11_action_mpls_ttl *)a;
+ofpact_put_SET_MPLS_TTL(out)->ttl = oamt->mpls_ttl;
+break;
+}
+
 case OFPUTIL_OFPAT11_DEC_MPLS_TTL:
 ofpact_put_DEC_MPLS_TTL(out);
 break;
@@ -1129,6 +1141,7 @@ ofpact_check__(const struct ofpact *a, int max_ports)
 return nxm_reg_load_check(ofpact_get_REG_LOAD(a));
 
 case OFPACT_DEC_TTL:
+case OFPACT_SET_MPLS_TTL:
 case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_SET_QUEUE:
@@ -1358,6 +1371,11 @@ ofpact_to_nxast(const struct ofpact *a, struct ofpbuf 
*out)
 ofpact_dec_ttl_to_nxast(ofpact_get_DEC_TTL(a), out);
 break;
 
+case OFPACT_SET_MPLS_TTL:
+ofputil_put_NXAST_SET_MPLS_TTL(out)->ttl
+= ofpact_get_SET_MPLS_TTL(a)->ttl;
+break;
+
 case OFPACT_DEC_MPLS_TTL:
 ofputil_put_NXAST_DEC_MPLS_TTL(out);
 break;
@@ -1531,6 +1549,7 @@ ofpact_to_openflow10(const struct ofpact *a, struct 
ofpbuf *out)
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
 case OFPACT_DEC_TTL:
+case OFPACT_SET_MPLS_TTL:
 case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_WRITE_METADATA:
@@ -1678,6 +1697,7 @@ ofpact_to_openflow11(const struct ofpact *a, struct 
ofpbuf *out)
 case OFPACT_BUNDLE:
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
+case OFPACT_SET_MPLS_TTL:
 case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_SET_QUEUE:
@@ -1800,6 +1820,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact, 
uint16_t port)
 case OFPACT_REG_MOVE:
 case OFPACT_REG_LOAD:
 case OFPACT_DEC_TTL:
+case OFPACT_SET_MPLS_TTL:
 case OFPACT_DEC_MPLS_TTL:
 case OFPACT_SET_TUNNEL:
 case OFPACT_WRITE_METADATA:
@@ -2020,6 +2041,11 @@ ofpact_format(const struct ofpact *a, struct ds *s)
 print_dec_ttl(ofpact_get_DEC_TTL(a), s);
 break;
 
+case OFPACT_SET_MPLS_TTL:
+ds_put_format(s, "set_mpls_ttl(%"PRIu8")",
+  ofpact_get_SET_MPLS_TTL(a)->ttl);
+break;
+
 case OFPACT_DEC_MPLS_TTL:
 ds_put_cstr(s, "dec_mpls_ttl");
 break;
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index f051255..01bd1a4 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -70,6 +70,7 @@
 DEFINE_OFPACT(REG_MOVE,ofpact_reg_mov

[ovs-dev] [PATCH 8/8] Add support for copy_ttl_in action

2012-10-26 Thread Simon Horman
This adds support for the OpenFlow 1.1+ copy_ttl_out action.
And also adds an NX copy_ttl_out action.

The implementation does not support copying in the case where the outermost
header is IP as it is unclear to me that Open vSwtich has a notion of an
inner IP header to copy the TLL from.

The implementation does not support copying in the case where the
outermost and next-to-otermost header is MPLS due to limitations
in the current MPLS implementation which only supports modifying
the outermost MPLS header.

The handling of the TTL modification is entirely handled in userspace.

Signed-off-by: Simon Horman 
---
 include/openflow/nicira-ext.h |1 +
 lib/flow.c|   15 +--
 lib/match.c   |3 ++-
 lib/ofp-actions.c |   20 
 lib/ofp-actions.h |1 +
 lib/ofp-parse.c   |5 +
 lib/ofp-print.c   |5 -
 lib/ofp-util.def  |2 ++
 ofproto/ofproto-dpif.c|   22 ++
 tests/ofproto-dpif.at |   21 +
 utilities/ovs-ofctl.8.in  |5 +
 11 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index c43c51a..81df350 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,7 @@ enum nx_action_subtype {
 NXAST_WRITE_METADATA,   /* struct nx_action_write_metadata */
 NXAST_PUSH_MPLS,/* struct nx_action_push_mpls */
 NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+NXAST_COPY_TTL_IN,  /* struct nx_action_header */
 NXAST_COPY_TTL_OUT, /* struct nx_action_header */
 NXAST_SET_MPLS_TTL, /* struct nx_action_ttl */
 NXAST_DEC_MPLS_TTL, /* struct nx_action_header */
diff --git a/lib/flow.c b/lib/flow.c
index d44050d..def0305 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -368,6 +368,7 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
 {
 struct ofpbuf b = *packet;
 struct eth_header *eth;
+ovs_be16 dl_type;
 
 COVERAGE_INC(flow_extract);
 
@@ -400,11 +401,11 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
 if (eth->eth_type == htons(ETH_TYPE_VLAN)) {
 parse_vlan(&b, flow);
 }
-flow->dl_type = parse_ethertype(&b);
+dl_type = flow->dl_type = parse_ethertype(&b);
 
 /* Parse mpls, copy l3 ttl. */
-if (flow->dl_type == htons(ETH_TYPE_MPLS) ||
-flow->dl_type == htons(ETH_TYPE_MPLS_MCAST)) {
+if (dl_type == htons(ETH_TYPE_MPLS) ||
+dl_type == htons(ETH_TYPE_MPLS_MCAST)) {
 struct ip_header *ih;
 struct ip6_hdr *ih6;
 
@@ -419,15 +420,17 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
 if (packet->size >= sizeof *ih &&
 IP_VER(ih->ip_ihl_ver) == IP_VERSION) {
 flow->nw_ttl = ih->ip_ttl;
+dl_type = htons(ETH_TYPE_IP);
 } else if (packet->size >= sizeof *ih6 &&
IP6_VER(ih6->ip6_vfc) == IP6_VERSION) {
 flow->nw_ttl = ih6->ip6_hlim;
+dl_type = htons(ETH_TYPE_IPV6);
 }
 }
 
 /* Network layer. */
 packet->l3 = b.data;
-if (flow->dl_type == htons(ETH_TYPE_IP)) {
+if (dl_type == htons(ETH_TYPE_IP)) {
 const struct ip_header *nh = pull_ip(&b);
 if (nh) {
 packet->l4 = b.data;
@@ -460,7 +463,7 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
 }
 }
 }
-} else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+} else if (dl_type == htons(ETH_TYPE_IPV6)) {
 if (parse_ipv6(&b, flow)) {
 return;
 }
@@ -475,7 +478,7 @@ flow_extract(struct ofpbuf *packet, uint32_t skb_priority,
 packet->l7 = b.data;
 }
 }
-} else if (flow->dl_type == htons(ETH_TYPE_ARP)) {
+} else if (dl_type == htons(ETH_TYPE_ARP)) {
 const struct arp_eth_header *arp = pull_arp(&b);
 if (arp && arp->ar_hrd == htons(1)
 && arp->ar_pro == htons(ETH_TYPE_IP)
diff --git a/lib/match.c b/lib/match.c
index 6f28378..5a51312 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -861,7 +861,8 @@ match_format(const struct match *match, struct ds *s, 
unsigned int priority)
 if (!skip_proto && wc->masks.nw_proto) {
 if (f->dl_type == htons(ETH_TYPE_ARP)) {
 ds_put_format(s, "arp_op=%"PRIu8",", f->nw_proto);
-} else {
+} else if (f->dl_type != htons(ETH_TYPE_MPLS) &&
+   f->dl_type != htons(ETH_TYPE_MPLS_MCAST)) {
 ds_put_format(s, "nw_proto=%"PRIu8",", f->nw_proto);
 }
 }
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index d51114d..10ed312 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -412,6 +412,10 @@ ofpact_from_nxast(const union ofp_action *a, enum 
ofputil_action_code code,
 break;

[ovs-dev] [PATCH 7/8] Add support for copy_ttl_out action

2012-10-26 Thread Simon Horman
This adds support for the OpenFlow 1.1+ copy_ttl_out action.
And also adds an NX copy_ttl_out action.

The implementation does not support copying in the case where the outermost
header is IP as it is unclear to me that Open vSwtich has a notion of an
inner IP header to copy the TLL from.

The handling of the TTL modification is entirely handled in userspace.

Signed-off-by: Simon Horman 

---

This patch depends on "flow: Set ttl in flow_compose()."
---
 include/openflow/nicira-ext.h |1 +
 lib/flow.c|6 --
 lib/flow.h|9 +
 lib/match.c   |2 +-
 lib/nx-match.c|2 +-
 lib/ofp-actions.c |   20 +++
 lib/ofp-actions.h |1 +
 lib/ofp-parse.c   |5 +
 lib/ofp-util.c|4 ++--
 lib/ofp-util.def  |2 ++
 ofproto/ofproto-dpif.c|   23 +
 tests/ofproto-dpif.at |   44 -
 utilities/ovs-ofctl.8.in  |7 +++
 13 files changed, 115 insertions(+), 11 deletions(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index fd07357..c43c51a 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -306,6 +306,7 @@ enum nx_action_subtype {
 NXAST_WRITE_METADATA,   /* struct nx_action_write_metadata */
 NXAST_PUSH_MPLS,/* struct nx_action_push_mpls */
 NXAST_POP_MPLS, /* struct nx_action_pop_mpls */
+NXAST_COPY_TTL_OUT, /* struct nx_action_header */
 NXAST_SET_MPLS_TTL, /* struct nx_action_ttl */
 NXAST_DEC_MPLS_TTL, /* struct nx_action_header */
 };
diff --git a/lib/flow.c b/lib/flow.c
index 0d7bbf4..d44050d 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -100,7 +100,9 @@ parse_remaining_mpls(struct ofpbuf *b, struct flow *flow)
 struct mpls_hdr *mh;
 while ((mh = ofpbuf_try_pull(b, sizeof *mh)) &&
!(mh->mpls_lse & htonl(MPLS_BOS_MASK))) {
-flow->mpls_depth++;
+if (flow->mpls_depth++ == 1) {
+flow->inner_mpls_lse = mh->mpls_lse;
+}
 }
 }
 
@@ -510,7 +512,7 @@ flow_zero_wildcards(struct flow *flow, const struct 
flow_wildcards *wildcards)
 void
 flow_get_metadata(const struct flow *flow, struct flow_metadata *fmd)
 {
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 18);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19);
 
 fmd->tun_id = flow->tunnel.tun_id;
 fmd->metadata = flow->metadata;
diff --git a/lib/flow.h b/lib/flow.h
index 94d5a98..05d6d08 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -36,7 +36,7 @@ struct ofpbuf;
 /* This sequence number should be incremented whenever anything involving flows
  * or the wildcarding of flows changes.  This will cause build assertion
  * failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 18
+#define FLOW_WC_SEQ 19
 
 #define FLOW_N_REGS 8
 BUILD_ASSERT_DECL(FLOW_N_REGS <= NXM_NX_MAX_REGS);
@@ -77,6 +77,7 @@ struct flow {
 ovs_be32 nw_dst;/* IPv4 destination address. */
 ovs_be32 ipv6_label;/* IPv6 flow label. */
 ovs_be32 mpls_lse;  /* MPLS label stack entry. */
+ovs_be32 inner_mpls_lse;/* Inner MPLS label stack entry. */
 uint16_t in_port;   /* OpenFlow port number of input port. */
 ovs_be16 vlan_tci;  /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
 ovs_be16 dl_type;   /* Ethernet frame type. */
@@ -92,15 +93,15 @@ struct flow {
 uint8_t nw_ttl; /* IP TTL/Hop Limit. */
 uint8_t nw_frag;/* FLOW_FRAG_* flags. */
 uint16_t mpls_depth;/* Depth of MPLS stack. */
-uint8_t zeros[2];   /* Must be zero. */
+uint8_t zeros[6];   /* Must be zero. */
 };
 BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
 
 #define FLOW_U32S (sizeof(struct flow) / 4)
 
 /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
-BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 152 &&
-  FLOW_WC_SEQ == 18);
+BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 160 &&
+  FLOW_WC_SEQ == 19);
 
 /* Represents the metadata fields of struct flow. */
 struct flow_metadata {
diff --git a/lib/match.c b/lib/match.c
index a959843..6f28378 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -722,7 +722,7 @@ match_format(const struct match *match, struct ds *s, 
unsigned int priority)
 
 int i;
 
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 18);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19);
 
 if (priority != OFP_DEFAULT_PRIORITY) {
 ds_put_format(s, "priority=%u,", priority);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 8999bfa..98be3b0 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -547,7 +547,7 @@ nx_put_raw(struct ofpbuf *b, bool oxm, const struct match 
*match,
 int match_len;
 int i;
 
-BUILD_ASSERT_DECL(FLOW_WC_SEQ ==

[ovs-dev] [PATCH] tests: Add support for running OFTest.

2012-10-26 Thread Ben Pfaff
Signed-off-by: Ben Pfaff 
---
This is an additional commit that, applied on top of the series that
I posted yesterday, makes it possible to run OFTest against an
Open vSwitch build tree with just "make check-oftest".

The previous series starts here:
http://openvswitch.org/pipermail/dev/2012-October/022300.html

 Makefile.am   |1 +
 NEWS  |2 ++
 README-OFTest |   68 ++
 tests/automake.mk |6 
 tests/run-oftest  |   94 +
 5 files changed, 171 insertions(+)
 create mode 100644 README-OFTest
 create mode 100755 tests/run-oftest

diff --git a/Makefile.am b/Makefile.am
index dafba2b..13309a7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -54,6 +54,7 @@ EXTRA_DIST = \
IntegrationGuide \
NOTICE \
PORTING \
+   README-OFTest \
README-gcov \
REPORTING-BUGS \
SubmittingPatches \
diff --git a/NEWS b/NEWS
index fa0a249..f138d0f 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 post-v1.9.0
 
+- New "check-oftest" Makefile target for running OFTest against Open
+  vSwitch.  See README-OFTest for details.
 
 
 v1.9.0 - xx xxx 
diff --git a/README-OFTest b/README-OFTest
new file mode 100644
index 000..0155894
--- /dev/null
+++ b/README-OFTest
@@ -0,0 +1,68 @@
+How to Use OFTest With Open vSwitch
+===
+
+This document describes how to use the OFTest OpenFlow protocol
+testing suite with Open vSwitch in "dummy mode".  In this mode of
+testing, no packets travel across physical or virtual networks.
+Instead, Unix domain sockets stand in as simulated networks.  This
+simulation is imperfect, but it is much easier to set up, does not
+require extra physical or virtual hardware, and does not require
+supervisor privileges.
+
+Prerequisites
+-
+
+First, build Open vSwitch according to the instructions in INSTALL.
+You need not install it.
+
+Second, obtain a copy of OFTest, then install the prerequisites and
+build the OpenFlow message classes described in its top-level README.
+The version of OFTest you use must be new enough to include a file
+named platforms/ovs-dummy.py, which provides OVS dummy mode support.
+Testing OVS in dummy mode does not require root privilege, so you may
+ignore that requirement.
+
+Optionally, add the top-level OFTest directory (containing the "oft"
+program) to your $PATH.  This slightly simplifies running OFTest later.
+
+Running OFTest
+--
+
+To run OFTest in dummy mode, run the following command from your Open
+vSwitch build directory:
+
+make check-oftest OFT=
+
+where  is the absolute path to the "oft" program in
+OFTest.
+
+If you added "oft" to your $PATH, you may omit the OFT variable
+assignment:
+
+make check-oftest
+
+By default, "check-oftest" passes "oft" just enough options to enable
+dummy mode.  You can use OFTFLAGS to pass additional options.  For
+example, to run just the basic.Echo test instead of all tests (the
+default) and enable verbose logging:
+
+make check-oftest OFT= OFTFLAGS='--verbose -T basic.Echo'
+
+Interpreting OFTest Results
+---
+
+Please interpret OFTest results cautiously.  Open vSwitch can fail a
+given test in OFTest for many reasons, including bugs in Open vSwitch,
+bugs in OFTest, bugs in the "dummy mode" integration, and differing
+interpretations of the OpenFlow standard and other standards.
+
+Open vSwitch has not been validated against OFTest.  Please do report
+test failures that you believe to represent bugs in Open vSwitch.
+Include the precise versions of Open vSwitch and OFTest in your bug
+report, plus any other information needed to reproduce the problem.
+
+Contact 
+---
+
+b...@openvswitch.org
+http://openvswitch.org/
diff --git a/tests/automake.mk b/tests/automake.mk
index 600056e..d7fcc15 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -144,6 +144,12 @@ check-valgrind: all tests/atconfig tests/atlocal 
$(TESTSUITE) \
@echo 'Valgrind output can be found in tests/testsuite.dir/*/valgrind.*'
@echo 
'--'
 
+# OFTest support.
+
+check-oftest: all
+   srcdir='$(srcdir)' $(SHELL) $(srcdir)/tests/run-oftest
+EXTRA_DIST += tests/run-oftest
+
 clean-local:
test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
 
diff --git a/tests/run-oftest b/tests/run-oftest
new file mode 100755
index 000..d12a22f
--- /dev/null
+++ b/tests/run-oftest
@@ -0,0 +1,94 @@
+#! /bin/sh
+
+set -e
+
+run () {
+echo "$@"
+"$@" || exit 1
+}
+
+# Put built tools early in $PATH.
+builddir=`pwd`
+if test ! -e vswitchd/ovs-vswitchd; then
+echo >&2 'not in build directory, please change directory or run via 
\"make check-oftest'
+exit 1
+fi
+PATH=$builddir/ovsdb:$builddir/vswitchd:$builddir/utilities:$PATH; e