[ovs-dev] [QUESTION] the flow expire time
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
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.
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
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
(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
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
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
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
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
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
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)
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
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
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.
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
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.
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.
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.
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
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.
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.
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.
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
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
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
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.
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.
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
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.
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.
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'
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.
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.
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.
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.
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.
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
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
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
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().
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
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
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
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
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
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
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.
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