Follow up debate on the fast path slow path issue...

-----Original Message-----
From: dev-boun...@openvswitch.org [mailto:dev-boun...@openvswitch.org] On
Behalf Of dev-requ...@openvswitch.org
Sent: Tuesday, November 29, 2011 3:43 AM
To: dev@openvswitch.org
Subject: dev Digest, Vol 28, Issue 146

Send dev mailing list submissions to
        dev@openvswitch.org

To subscribe or unsubscribe via the World Wide Web, visit
        http://openvswitch.org/mailman/listinfo/dev
or, via email, send a message with subject or body 'help' to
        dev-requ...@openvswitch.org

You can reach the person managing the list at
        dev-ow...@openvswitch.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of dev digest..."


Today's Topics:

   1. [PATCH] ovs-vlan-bugs: Document driver bug with priority
      tagged packets. (Ben Pfaff)
   2. Re: Statistics Collection Performance Impact (Ben Pfaff)
   3. Re: [GIT PULL v2] Open vSwitch (Jamal Hadi Salim)
   4. Re: Open vSwitch Design (Jamal Hadi Salim)
   5. [PATCH 1/2] ofproto-dpif: Remove redundant
      commit_odp_actions() call. (Ethan Jackson)
   6. [PATCH 2/2] odp-util: Move commit_odp_actions() from
      ofproto-dpif. (Ethan Jackson)


----------------------------------------------------------------------

Message: 1
Date: Mon, 28 Nov 2011 13:49:12 -0800
From: Ben Pfaff <b...@nicira.com>
Subject: [ovs-dev] [PATCH] ovs-vlan-bugs: Document driver bug with
        priority        tagged packets.
To: dev@openvswitch.org
Cc: Ben Pfaff <b...@nicira.com>
Message-ID: <1322516952-24247-1-git-send-email-...@nicira.com>

Reported-by: Jesse Gross <je...@nicira.com>
---
Does this correctly explain the issue?

Thanks,

Ben.

diff --git a/utilities/ovs-vlan-bugs.man b/utilities/ovs-vlan-bugs.man
index bdca8fc..1e62d16 100644
--- a/utilities/ovs-vlan-bugs.man
+++ b/utilities/ovs-vlan-bugs.man
@@ -6,6 +6,11 @@ the extracted tag.  Drivers are supposed to only enable
stripping if a
 \fBvlan_group\fR is registered but not all of them do that.
 .
 .IP \(bu
+On receive, some drivers handle priority tagged packets specially and
+don't pass the tag onto the network stack at all, so Open vSwitch
+never has a chance to see it.
+.
+.IP \(bu
 Some drivers size their receive buffers based on whether a
 \fBvlan_group\fR is enabled, meaning that a maximum size packet with a
 VLAN tag will not fit if no \fBvlan_group\fR is configured.
-- 
1.7.4.4



------------------------------

Message: 2
Date: Mon, 28 Nov 2011 14:03:10 -0800
From: Ben Pfaff <b...@nicira.com>
Subject: Re: [ovs-dev] Statistics Collection Performance Impact
To: Simon Horman <ho...@verge.net.au>
Cc: dev@openvswitch.org, d...@midokura.jp
Message-ID: <20111128220310.go3...@nicira.com>
Content-Type: text/plain; charset=us-ascii

On Thu, Nov 10, 2011 at 09:14:50AM -0800, Ben Pfaff wrote:
> On Thu, Nov 10, 2011 at 06:04:07PM +0900, Simon Horman wrote:
> > On Wed, Nov 09, 2011 at 08:14:36PM -0800, Ben Pfaff wrote:
> > > On Thu, Nov 10, 2011 at 10:20:24AM +0900, Simon Horman wrote:
> > > > On Wed, Nov 09, 2011 at 01:19:42PM -0800, Ben Pfaff wrote:
> > > > > On Mon, Nov 07, 2011 at 04:20:19PM +0900, Simon Horman wrote:
> > > > > > Although a very simple and possibly na??ve approach, I wonder if
> > > > > > dynamically extending the interval at which statistics are
collected is
> > > > > > a worthwhile approach to mitigating the performance impact of
statistics
> > > > > > collection.
> > > > > 
> > > > > It's one approach.  It has the nice property of being very simple.
> > > > > 
> > > > > Another approach that I've been pondering myself is, when there
are many
> > > > > datapath flows, to read out the kernel stats for only some of them
at a
> > > > > time and apply the expiration algorithm to just those.  We could
run
> > > > > expiration just as frequently overall, but it would apply to any
given
> > > > > flow less frequently.
> > > > 
> > > > I had also considered that and I think it an approach worth
investigating.
> > > > It seems to me that the main challenge will be arrange things such
that a
> > > > partial statistics update can occur while still allowing all
statistics to
> > > > be updated over time.  I wonder if partitioning the flow hash would
be a
> > > > reasonable way to achieve this.
> > > 
> > > I think that it works OK already.  Just start the flow dump and read
as
> > > many flows as you want to deal with at a time, then stop, keeping the
> > > dump in progress.  Then when you want to keep going later, just keep
> > > reading the dump.
> > 
> > Good point. I'll see about testing that.
> 
> Great, I look forward to hearing how well it works.

Did you get a chance to experiment with this?  I really am interested
in doing it this way, if it is effective.


------------------------------

Message: 3
Date: Mon, 28 Nov 2011 17:21:13 -0500
From: Jamal Hadi Salim <j...@mojatatu.com>
Subject: Re: [ovs-dev] [GIT PULL v2] Open vSwitch
To: Ben Pfaff <b...@nicira.com>
Cc: dev@openvswitch.org, net...@vger.kernel.org,        Herbert Xu
        <herb...@gondor.apana.org.au>,  David Miller <da...@davemloft.net>
Message-ID: <1322518873.6118.7.camel@mojatatu>
Content-Type: text/plain; charset="UTF-8"

On Mon, 2011-11-28 at 08:01 -0800, Ben Pfaff wrote:

> Regarding OpenFlow rate limiting, in addition to Martin's response, Open
> vSwitch has implemented controller rate limiting since day one.  It is
> documented in ovs-vswitchd.conf.db(5):

Ok, I think thats a good start. My experience says just rate limiting
may not be sufficient - unless the rate limiting is adaptive in some
form; or just use strict prio where you let the exception traffic
rot if you have other work - maybe thats what Martin was talking 
about.

The problem is more in the outbound towards the external controller.
You dont have multiple queues (given a single TCP socket) and config,
events, and exception packets are all shared in one queue.

cheers,
jamal




------------------------------

Message: 4
Date: Mon, 28 Nov 2011 17:42:51 -0500
From: Jamal Hadi Salim <j...@mojatatu.com>
Subject: Re: [ovs-dev] Open vSwitch Design
To: Justin Pettit <jpet...@nicira.com>
Cc: dev@openvswitch.org, Chris Wright <chr...@redhat.com>,      Herbert Xu
        <herb...@gondor.apana.org.au>,  Eric Dumazet
<eric.duma...@gmail.com>,
        netdev <net...@vger.kernel.org>,        John Fastabend
        <john.r.fastab...@intel.com>,   Stephen Hemminger
        <shemmin...@vyatta.com>,        David Miller <da...@davemloft.net>
Message-ID: <1322520171.6118.29.camel@mojatatu>
Content-Type: text/plain; charset="UTF-8"

On Mon, 2011-11-28 at 10:34 -0800, Justin Pettit wrote:
> On Nov 25, 2011, at 5:11 PM, Jamal Hadi Salim wrote:

> 
> Are you talking about ASICs on NICs?  

I am indifferent - looking at it entirely from a control
perspective. i.e if i do "ip link blah down" on a port
i want that to work with zero changes to iproute2; the only
way you can achieve that is if you expose those ports as
netdevs.
This is what I said was a good thing the Intel folks were trying to
achieve (and what Lennert has done for the small Marvel switch chips).

> I was referring to integrating Open vSwitch into top-of-rack switches.  
> These typically have a 48x1G or 48x10G switching ASIC and a relatively 
> slow (~800MHz PPC-class) management CPU running an operating system like 
> Linux.  There's no way that these systems can have a standard CPU on the
fastpath.

No, not the datapath; just control of the hardware.  If i run
"ip route add .." i want that to work on the ASIC.
Same with tc action/classification. I want to run those tools and
configure an ACL in the ASIC with no new learning curve.

> 
> I understood the original question to be: Can we make the interface to the

> kernel look like a hardware switch?  My answer had two main parts.  First,

> I don't think we could define a "standard" hardware interface, since
they're
> all very different.  Second, even if we could, I think a software
fastpath's
> strengths and weaknesses are such that the hardware model wouldn't be
ideal.

Not talking about datapath - but control interface to those devices.
We cant define how the low levels look like. But if you expose things
using standard linux interfaces, then user space tools and APIs stay
unchanged.

Then i shouldnt care where the feature runs (hardware NIC, ASIC, pure
kernel level software etc).

> 
> The problem is that DRAM isn't going to cut it on the ACL tables--which
are 
> typically used for flow-based matching--on a 48x10G (or even 48x1G)
switch.

There are vendors who use DRAMS with speacilized interfaces that
interleave requests behind the scenes. Maybe i can point you to one
offline. 

> I've seen a couple of switching ASICs that support many 10s of thousands
of
> ACL entries, but they require expensive external TCAMs for lookup and SRAM

> for counters.  Most of the white box vendors that I've seen that use those

> ASICs don't bother adding the external TCAM and SRAM to their designs.  
> Even when they are added, their matching capabilities are typically
limited 
> in order to keep up with traffic.

I thought SRAM markets have dried up these days. Anyways what you are
refereing to above is generally true.

> > Justin - theres nothing new you need in the kernel to have that feature.
> > Let me rephrase that, that has not been a new feature for at least a
> > decade in Linux.
> > Add exact match filters with higher priority. Have the lowest priority
> > filter to redirect to user space. Let user space lookup some service
> > rule; have it download to the kernel one or more exact matches.
> > Let the packet proceed on its way down the kernel to its destination if
> > thats what is defined.
> 
> My point was that a software fastpath should look different than a
hardware-based one.

And i was pointing  to what your datapath patches which in conjunction
with your user space code.

> > 
> > That bit sounds interesting - I will look at your spec.
> 
> Great!

I am sorry - been overloaded elsewhere havent looked. But i think above
I pretty much spelt out what my desires are.


> Yes, Open vSwitch has been ported to 24x10G ASICs running Linux on their
management CPUs.  
> However, in these cases the datapath is handled by hardware and not the
software forwarding 
> plane, obviously.

Of course.

> > Do the vendors agree to some common interface?
> 
> Yes, if you view ofproto (as described in the porting guide) as that
interface.  Every merchant silicon vendor 
> I've seen views the interfaces to their ASICs as proprietary.  

Yes, the XAL agony (HALS and PALS that run on 26 other OSes).

> Someone (with the appropriate SDK and licenses) needs to write providers
for those different hardware ports.  
> We've helped multiple vendors do this and know a few others that have done
it on their own.

You know what would be really nice is if you achieved what i described
above.
Can I ifconfig an ethernet switch port?

> This really seems besides the point for this discussion, though.  
> We've written an ofproto provider for software switches called "dpif" 
> (this is also described in the porting guide). What we're proposing be 
> included in Linux is the kernel module that speaks to that dpif provider 
> over a well-defined, stable, netlink-based protocol.
> 
> Here's just a quick (somewhat simplified) summary of the different layers.

> At the top, there are controllers and switches that communicate using
OpenFlow.
> OpenFlow gives controller writers the ability to inspect and modify the
switches' 
> flow tables and interfaces.  If a flow entry doesn't match an existing
entry, the 
> packet is forwarded to the controller for further processing.  OpenFlow
1.0 was 
> pretty basic and exposed a single flow table.  OpenFlow 1.1 introduced a
number 
> of new features including multiple table support.  The forthcoming
OpenFlow 1.2 
> will include support for extensible matches, which means that new fields
may be 
> added without requiring a full revision of the specification.  OpenFlow is
defined 
> by the Open Networking Foundation and is not directly related to Open
vSwitch.
> 
> The userspace in Open vSwitch has an OpenFlow library that interacts with
the 
> controllers.  Userspace has its own classifier that supports wildcard
entries 
> and multiple tables.  Many of the changes to the OpenFlow protocol only
require 
> modifying that library and perhaps some of the glue code with the
classifier.  
> (In theory, other software-defined networking protocols could be plugged
in as well.)  
> The classifier interacts with the ofproto layer below it, which implements
a fastpath.

Yes, when i looked at your code i can see that you have gone past
openflow.

> On a hardware switch, since it supports wildcarding, it essentially
becomes a 
> passthrough that just calls the appropriate APIs for the ASIC.  

Are these APIs documented as well? Maybe thats all we need if you dont
have the standard linux tools working.

> On software, 
> as we've discussed, exact-match flows work better.
> 
> For that reason, we've defined the dpif layer, which is an ofproto
provider.  
> It's primary purpose is to take high-level concepts like "treat this group
of 
> interfaces as a LACP bond" or "support this set of wildcard flow entries"
and 
> explode them into exact-match entries on-demand.  We've then implemented a

> Linux dpif provider that takes the exact match entries created by the dpif

> layer and converts them into netlink messages that the kernel module
understands.  
> These messages are well-defined and not specific to Open vSwitch or
OpenFlow.

Useful but that seems more like a service layer - I want just to be able
to ifconfig a port as a basic need.
In any case, I should look at your doc to get some clarity.

> This layering has allowed us to introduce new OpenFlow-like features such
as multiple tables 
> and non-OpenFlow features such as port mirroring, STP, CCM, and new
bonding modes without 
> changes to the kernel module.  In fact, the only changes that should
necessitate a kernel 
> interface change are new matches or actions, such as would be required for
handling MPLS.

I just need the basic cobbling blocks.
If you conform to what Linux already does and i can run standard tools,
we can have a lot of creative things that could be done.


> > Make these vendor switches work with plain Linux. The Intel folks are
> > producing interfaces with L2, ACLs, VIs and are putting some effort to
> > integrate them into plain Linux. I should be able to set the qos rules
> > with tc on an intel chip.
> > You guys can still take advantage of all that and still have your nice
> > control plane.
> 
> Once again, I think we are talking about different things.  I believe you
are 
> discussing interfacing with NICs, which is quite different from a high
fanout 
> switching ASIC.  As I previously mentioned, the point of my original 

> post was that I think it would be best not to model a high fanout switch
in the interface to the kernel.
> 

I hope my clarification above makes more sense.

cheers,
jamal




------------------------------

Message: 5
Date: Mon, 28 Nov 2011 14:43:14 -0800
From: Ethan Jackson <et...@nicira.com>
Subject: [ovs-dev] [PATCH 1/2] ofproto-dpif: Remove redundant
        commit_odp_actions() call.
To: dev@openvswitch.org
Message-ID: <1322520195-10292-1-git-send-email-et...@nicira.com>

---
 ofproto/ofproto-dpif.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1b7bc44..88032bf 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4308,7 +4308,6 @@ flood_packets(struct action_xlate_ctx *ctx, bool all)
 {
     struct ofport_dpif *ofport;
 
-    commit_odp_actions(ctx);
     HMAP_FOR_EACH (ofport, up.hmap_node, &ctx->ofproto->up.ports) {
         uint16_t ofp_port = ofport->up.ofp_port;
 
-- 
1.7.7.1



------------------------------

Message: 6
Date: Mon, 28 Nov 2011 14:43:15 -0800
From: Ethan Jackson <et...@nicira.com>
Subject: [ovs-dev] [PATCH 2/2] odp-util: Move commit_odp_actions()
        from    ofproto-dpif.
To: dev@openvswitch.org
Message-ID: <1322520195-10292-2-git-send-email-et...@nicira.com>

In an effort to simplify ofproto-dpif, this commit moves the
definition of commit_odp_actions() to odp-util.
---
 lib/odp-util.c         |  160
+++++++++++++++++++++++++++++++++++++++++++++++
 lib/odp-util.h         |    3 +
 ofproto/ofproto-dpif.c |  163
+-----------------------------------------------
 3 files changed, 165 insertions(+), 161 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index e2e094e..1b1dc19 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1721,3 +1721,163 @@ odp_put_userspace_action(uint32_t pid, const struct
user_action_cookie *cookie,
 
     return cookie ? odp_actions->size - NLA_ALIGN(sizeof *cookie) : 0;
 }
+

+/* The commit_odp_actions() function and its helpers. */
+
+static void
+commit_set_action(struct ofpbuf *odp_actions, enum ovs_key_attr key_type,
+                  const void *key, size_t key_size)
+{
+    size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
+    nl_msg_put_unspec(odp_actions, key_type, key, key_size);
+    nl_msg_end_nested(odp_actions, offset);
+}
+
+static void
+commit_set_tun_id_action(const struct flow *flow, struct flow *base,
+                         struct ofpbuf *odp_actions)
+{
+    if (base->tun_id == flow->tun_id) {
+        return;
+    }
+    base->tun_id = flow->tun_id;
+
+    commit_set_action(odp_actions, OVS_KEY_ATTR_TUN_ID,
+                      &base->tun_id, sizeof(base->tun_id));
+}
+
+static void
+commit_set_ether_addr_action(const struct flow *flow, struct flow *base,
+                             struct ofpbuf *odp_actions)
+{
+    struct ovs_key_ethernet eth_key;
+
+    if (eth_addr_equals(base->dl_src, flow->dl_src) &&
+        eth_addr_equals(base->dl_dst, flow->dl_dst)) {
+        return;
+    }
+
+    memcpy(base->dl_src, flow->dl_src, ETH_ADDR_LEN);
+    memcpy(base->dl_dst, flow->dl_dst, ETH_ADDR_LEN);
+
+    memcpy(eth_key.eth_src, base->dl_src, ETH_ADDR_LEN);
+    memcpy(eth_key.eth_dst, base->dl_dst, ETH_ADDR_LEN);
+
+    commit_set_action(odp_actions, OVS_KEY_ATTR_ETHERNET,
+                      &eth_key, sizeof(eth_key));
+}
+
+static void
+commit_vlan_action(const struct flow *flow, struct flow *base,
+                   struct ofpbuf *odp_actions)
+{
+    if (base->vlan_tci == flow->vlan_tci) {
+        return;
+    }
+
+    if (base->vlan_tci & htons(VLAN_CFI)) {
+        nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_VLAN);
+    }
+
+    if (flow->vlan_tci & htons(VLAN_CFI)) {
+        struct ovs_action_push_vlan vlan;
+
+        vlan.vlan_tpid = htons(ETH_TYPE_VLAN);
+        vlan.vlan_tci = flow->vlan_tci;
+        nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
+                          &vlan, sizeof vlan);
+    }
+    base->vlan_tci = flow->vlan_tci;
+}
+
+static void
+commit_set_nw_action(const struct flow *flow, struct flow *base,
+                     struct ofpbuf *odp_actions)
+{
+    struct ovs_key_ipv4 ipv4_key;
+
+    if (base->dl_type != htons(ETH_TYPE_IP) ||
+        !base->nw_src || !base->nw_dst) {
+        return;
+    }
+
+    if (base->nw_src == flow->nw_src &&
+        base->nw_dst == flow->nw_dst &&
+        base->nw_tos == flow->nw_tos &&
+        base->nw_ttl == flow->nw_ttl &&
+        base->nw_frag == flow->nw_frag) {
+        return;
+    }
+
+    ipv4_key.ipv4_src = base->nw_src = flow->nw_src;
+    ipv4_key.ipv4_dst = base->nw_dst = flow->nw_dst;
+    ipv4_key.ipv4_tos = base->nw_tos = flow->nw_tos;
+    ipv4_key.ipv4_ttl = base->nw_ttl = flow->nw_ttl;
+    ipv4_key.ipv4_proto = base->nw_proto;
+    ipv4_key.ipv4_frag = (base->nw_frag == 0 ? OVS_FRAG_TYPE_NONE
+                          : base->nw_frag == FLOW_NW_FRAG_ANY
+                          ? OVS_FRAG_TYPE_FIRST : OVS_FRAG_TYPE_LATER);
+
+    commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4,
+                      &ipv4_key, sizeof(ipv4_key));
+}
+
+static void
+commit_set_port_action(const struct flow *flow, struct flow *base,
+                       struct ofpbuf *odp_actions)
+{
+    if (!base->tp_src || !base->tp_dst) {
+        return;
+    }
+
+    if (base->tp_src == flow->tp_src &&
+        base->tp_dst == flow->tp_dst) {
+        return;
+    }
+
+    if (flow->nw_proto == IPPROTO_TCP) {
+        struct ovs_key_tcp port_key;
+
+        port_key.tcp_src = base->tp_src = flow->tp_src;
+        port_key.tcp_dst = base->tp_dst = flow->tp_dst;
+
+        commit_set_action(odp_actions, OVS_KEY_ATTR_TCP,
+                          &port_key, sizeof(port_key));
+
+    } else if (flow->nw_proto == IPPROTO_UDP) {
+        struct ovs_key_udp port_key;
+
+        port_key.udp_src = base->tp_src = flow->tp_src;
+        port_key.udp_dst = base->tp_dst = flow->tp_dst;
+
+        commit_set_action(odp_actions, OVS_KEY_ATTR_UDP,
+                          &port_key, sizeof(port_key));
+    }
+}
+
+static void
+commit_set_priority_action(const struct flow *flow, struct flow *base,
+                           struct ofpbuf *odp_actions)
+{
+    if (base->priority == flow->priority) {
+        return;
+    }
+    base->priority = flow->priority;
+
+    commit_set_action(odp_actions, OVS_KEY_ATTR_PRIORITY,
+                      &base->priority, sizeof(base->priority));
+}
+
+/* Updates 'odp_actions' with the ODP actions required to transform 'base'
into
+ * 'flow'.  Updates 'base' in accordance with the emitted actions. */
+void
+commit_odp_actions(const struct flow *flow, struct flow *base,
+                   struct ofpbuf *odp_actions)
+{
+    commit_set_tun_id_action(flow, base, odp_actions);
+    commit_set_ether_addr_action(flow, base, odp_actions);
+    commit_vlan_action(flow, base, odp_actions);
+    commit_set_nw_action(flow, base, odp_actions);
+    commit_set_port_action(flow, base, odp_actions);
+    commit_set_priority_action(flow, base, odp_actions);
+}
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 57294ec..7c9b588 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -136,4 +136,7 @@ BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) ==
8);
 size_t odp_put_userspace_action(uint32_t pid,
                                 const struct user_action_cookie *,
                                 struct ofpbuf *odp_actions);
+
+void commit_odp_actions(const struct flow *, struct flow *base,
+                        struct ofpbuf *odp_actions);
 #endif /* odp-util.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 88032bf..14b5447 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4027,165 +4027,6 @@ fix_sflow_action(struct action_xlate_ctx *ctx)
 }
 
 static void
-commit_set_action(struct ofpbuf *odp_actions, enum ovs_key_attr key_type,
-                  const void *key, size_t key_size)
-{
-    size_t offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SET);
-    nl_msg_put_unspec(odp_actions, key_type, key, key_size);
-    nl_msg_end_nested(odp_actions, offset);
-}
-
-static void
-commit_set_tun_id_action(const struct flow *flow, struct flow *base,
-                         struct ofpbuf *odp_actions)
-{
-    if (base->tun_id == flow->tun_id) {
-        return;
-    }
-    base->tun_id = flow->tun_id;
-
-    commit_set_action(odp_actions, OVS_KEY_ATTR_TUN_ID,
-                      &base->tun_id, sizeof(base->tun_id));
-}
-
-static void
-commit_set_ether_addr_action(const struct flow *flow, struct flow *base,
-                             struct ofpbuf *odp_actions)
-{
-    struct ovs_key_ethernet eth_key;
-
-    if (eth_addr_equals(base->dl_src, flow->dl_src) &&
-        eth_addr_equals(base->dl_dst, flow->dl_dst)) {
-        return;
-    }
-
-    memcpy(base->dl_src, flow->dl_src, ETH_ADDR_LEN);
-    memcpy(base->dl_dst, flow->dl_dst, ETH_ADDR_LEN);
-
-    memcpy(eth_key.eth_src, base->dl_src, ETH_ADDR_LEN);
-    memcpy(eth_key.eth_dst, base->dl_dst, ETH_ADDR_LEN);
-
-    commit_set_action(odp_actions, OVS_KEY_ATTR_ETHERNET,
-                      &eth_key, sizeof(eth_key));
-}
-
-static void
-commit_vlan_action(const struct flow *flow, struct flow *base,
-                   struct ofpbuf *odp_actions)
-{
-    if (base->vlan_tci == flow->vlan_tci) {
-        return;
-    }
-
-    if (base->vlan_tci & htons(VLAN_CFI)) {
-        nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_VLAN);
-    }
-
-    if (flow->vlan_tci & htons(VLAN_CFI)) {
-        struct ovs_action_push_vlan vlan;
-
-        vlan.vlan_tpid = htons(ETH_TYPE_VLAN);
-        vlan.vlan_tci = flow->vlan_tci;
-        nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
-                          &vlan, sizeof vlan);
-    }
-    base->vlan_tci = flow->vlan_tci;
-}
-
-static void
-commit_set_nw_action(const struct flow *flow, struct flow *base,
-                     struct ofpbuf *odp_actions)
-{
-    struct ovs_key_ipv4 ipv4_key;
-
-    if (base->dl_type != htons(ETH_TYPE_IP) ||
-        !base->nw_src || !base->nw_dst) {
-        return;
-    }
-
-    if (base->nw_src == flow->nw_src &&
-        base->nw_dst == flow->nw_dst &&
-        base->nw_tos == flow->nw_tos &&
-        base->nw_ttl == flow->nw_ttl &&
-        base->nw_frag == flow->nw_frag) {
-        return;
-    }
-
-    ipv4_key.ipv4_src = base->nw_src = flow->nw_src;
-    ipv4_key.ipv4_dst = base->nw_dst = flow->nw_dst;
-    ipv4_key.ipv4_tos = base->nw_tos = flow->nw_tos;
-    ipv4_key.ipv4_ttl = base->nw_ttl = flow->nw_ttl;
-    ipv4_key.ipv4_proto = base->nw_proto;
-    ipv4_key.ipv4_frag = (base->nw_frag == 0 ? OVS_FRAG_TYPE_NONE
-                          : base->nw_frag == FLOW_NW_FRAG_ANY
-                          ? OVS_FRAG_TYPE_FIRST : OVS_FRAG_TYPE_LATER);
-
-    commit_set_action(odp_actions, OVS_KEY_ATTR_IPV4,
-                      &ipv4_key, sizeof(ipv4_key));
-}
-
-static void
-commit_set_port_action(const struct flow *flow, struct flow *base,
-                       struct ofpbuf *odp_actions)
-{
-    if (!base->tp_src || !base->tp_dst) {
-        return;
-    }
-
-    if (base->tp_src == flow->tp_src &&
-        base->tp_dst == flow->tp_dst) {
-        return;
-    }
-
-    if (flow->nw_proto == IPPROTO_TCP) {
-        struct ovs_key_tcp port_key;
-
-        port_key.tcp_src = base->tp_src = flow->tp_src;
-        port_key.tcp_dst = base->tp_dst = flow->tp_dst;
-
-        commit_set_action(odp_actions, OVS_KEY_ATTR_TCP,
-                          &port_key, sizeof(port_key));
-
-    } else if (flow->nw_proto == IPPROTO_UDP) {
-        struct ovs_key_udp port_key;
-
-        port_key.udp_src = base->tp_src = flow->tp_src;
-        port_key.udp_dst = base->tp_dst = flow->tp_dst;
-
-        commit_set_action(odp_actions, OVS_KEY_ATTR_UDP,
-                          &port_key, sizeof(port_key));
-    }
-}
-
-static void
-commit_set_priority_action(const struct flow *flow, struct flow *base,
-                           struct ofpbuf *odp_actions)
-{
-    if (base->priority == flow->priority) {
-        return;
-    }
-    base->priority = flow->priority;
-
-    commit_set_action(odp_actions, OVS_KEY_ATTR_PRIORITY,
-                      &base->priority, sizeof(base->priority));
-}
-
-static void
-commit_odp_actions(struct action_xlate_ctx *ctx)
-{
-    const struct flow *flow = &ctx->flow;
-    struct flow *base = &ctx->base_flow;
-    struct ofpbuf *odp_actions = ctx->odp_actions;
-
-    commit_set_tun_id_action(flow, base, odp_actions);
-    commit_set_ether_addr_action(flow, base, odp_actions);
-    commit_vlan_action(flow, base, odp_actions);
-    commit_set_nw_action(flow, base, odp_actions);
-    commit_set_port_action(flow, base, odp_actions);
-    commit_set_priority_action(flow, base, odp_actions);
-}
-
-static void
 compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port,
                         bool check_stp)
 {
@@ -4219,7 +4060,7 @@ compose_output_action__(struct action_xlate_ctx *ctx,
uint16_t ofp_port,
     if (out_port != odp_port) {
         ctx->flow.vlan_tci = htons(0);
     }
-    commit_odp_actions(ctx);
+    commit_odp_actions(&ctx->flow, &ctx->base_flow, ctx->odp_actions);
     nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_OUTPUT, out_port);
 
     ctx->sflow_odp_port = odp_port;
@@ -4330,7 +4171,7 @@ compose_controller_action(struct action_xlate_ctx
*ctx, int len)
 {
     struct user_action_cookie cookie;
 
-    commit_odp_actions(ctx);
+    commit_odp_actions(&ctx->flow, &ctx->base_flow, ctx->odp_actions);
     cookie.type = USER_ACTION_COOKIE_CONTROLLER;
     cookie.data = len;
     cookie.n_output = 0;
-- 
1.7.7.1



------------------------------

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


End of dev Digest, Vol 28, Issue 146
************************************

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

Reply via email to