Re: [ovs-dev] MPLS support in OVS 2.3.2
On Sat, Aug 01, 2015 at 09:42:06AM +0530, tech_kals Kals wrote: Could you please let me know whether the said MPLS actions are supported in OVS or not? To know what Open vSwitch supports, please read the documentation. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 5/6] ovn: Add localnet logical port type.
On Fri, Jul 31, 2015 at 10:19:32PM -0400, Russell Bryant wrote:
On 07/31/2015 07:26 PM, Ben Pfaff wrote:
I'm not sure I correctly understand the model for using these. Maybe
you can flesh out an example to help me. Suppose I have a pair of
hypervisors A and B, and each HV has two VMs (A1 and A2, B1 and B2), and
all four of the VMs are connected to the same Neutron provider network.
How many OVN Logical_Switches would I have and what would their
membership look like? On each HV, what interfaces would be attached to
br-int?
That scenario is really close to what I describe in the overview mail,
except I only had 1 VM per hypervisor.
http://openvswitch.org/pipermail/dev/2015-July/058367.html
A simulation of your scenario would be (with ovs-sandbox):
ovs-vsctl add-br br-eth1
ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth1
for n in 1 2 3 4; do
ovn-nbctl lswitch-add provnet1-$n
ovn-nbctl lport-add provnet1-$n provnet1-$n-port1
ovn-nbctl lport-set-macs provnet1-$n-port1 00:00:00:00:00:0$n
ovn-nbctl lport-set-port-security provnet1-$n-port1 00:00:00:00:00:0$n
ovn-nbctl lport-add provnet1-$n provnet1-$n-physnet1
ovn-nbctl lport-set-macs provnet1-$n-physnet1 unknown
ovn-nbctl lport-set-type provnet1-$n-physnet1 localnet
ovn-nbctl lport-set-options provnet1-$n-physnet1 network_name=physnet1
done
The above gets us the following logical config:
$ ovn-nbctl show
lswitch 457a1810-b3c5-40f9-835e-4f86a65bb60d (provnet1-3)
lport provnet1-3-port1
macs: 00:00:00:00:00:03
lport provnet1-3-physnet1
macs: unknown
lswitch 40afcc6e-464e-44db-bb57-0f13da299cbd (provnet1-2)
lport provnet1-2-port1
macs: 00:00:00:00:00:02
lport provnet1-2-physnet1
macs: unknown
lswitch b603dff3-cd7c-4d82-be19-1bfe2a4f5897 (provnet1-4)
lport provnet1-4-physnet1
macs: unknown
lport provnet1-4-port1
macs: 00:00:00:00:00:04
lswitch 0bd64e33-90f7-4d19-b2c2-2e39ba7b3317 (provnet1-1)
lport provnet1-1-physnet1
macs: unknown
lport provnet1-1-port1
macs: 00:00:00:00:00:01
The output doesn't show it, but the physnet ports are type=localnet.
Now we can bind the first 2 ports locally. For the second two ports, we
can create a fake chassis and update the DB to reflect that the ports
are bound to the fake chassis.
ovs-vsctl add-port br-int lport1 -- set Interface lport1
external_ids:iface-id=provnet1-1-port1
ovs-vsctl add-port br-int lport2 -- set Interface lport1
external_ids:iface-id=provnet1-2-port1
# Create a fake chassis
encaps_uuid=$(ovsdb-client dump OVN_Southbound | grep -A 3 Encap | tail -n
1 | awk '{print $1}')
chassis=$(ovsdb-client transact
'[OVN_Southbound,{op:insert,table:Chassis,row:{name:fakechassis,encaps:[uuid,'$encaps_uuid']}}]')
chassis_uuid=$(echo $chassis | sed -e 's/^.*\uuid\,\\(.*\)\.*/\1/')
uuid=$(ovsdb-client dump OVN_Southbound | grep -A 10 Binding | grep
provnet1-3-port1 | awk '{print $1}')
ovsdb-client transact
'[OVN_Southbound,{op:update,table:Binding,where:[[_uuid,==,[uuid,'$uuid']]],row:{chassis:[uuid,'$chassis_uuid']}}]'
uuid=$(ovsdb-client dump OVN_Southbound | grep -A 10 Binding | grep
provnet1-4-port1 | awk '{print $1}')
ovsdb-client transact
'[OVN_Southbound,{op:update,table:Binding,where:[[_uuid,==,[uuid,'$uuid']]],row:{chassis:[uuid,'$chassis_uuid']}}]'
Does that make sense?
Yes.
Sorry, I forgot that there was a detailed example earlier. I didn't
mean to make you go to a lot of work to repeat it.
I am slightly concerned about having a pair of patch ports per provider
network port (that is what the above implies, right?), but let's go
ahead and try that and if it's a problem we can figure out an
optimization.
Thanks,
Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] classifier versioning and rule_dpif_try_ref()
Now that the classifier is versioned, is it still possible for rule_dpif_try_ref() to return false? Specifically, can we change the loop in rule_dpif_lookup_in_table() to do an ordinary ref instead of a try_ref? My motive is that, if we could change it, then we could drop the 'take_ref' parameter from rule_dpif_lookup_from_table(), instead letting the caller take the ref if it wants it, which seems like a cleaner interface. But, of course, correctness is more important. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 21/22] ofctrl: Negotiate OVN Geneve option.
Is there any documentation for the ovs side geneve negotiation?
Also one question below:
Thanks,
Alex Wang,
On Sun, Jul 19, 2015 at 3:45 PM, Ben Pfaff b...@nicira.com wrote:
This won't really get used until the next commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ovn/controller/ofctrl.c | 470
ovn/controller/ofctrl.h | 5 +-
ovn/controller/ovn-controller.c | 6 +-
ovn/controller/physical.h | 7 +
4 files changed, 348 insertions(+), 140 deletions(-)
diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
index b1c421c..d8a0573 100644
--- a/ovn/controller/ofctrl.c
+++ b/ovn/controller/ofctrl.c
@@ -27,9 +27,10 @@
#include openflow/openflow.h
#include openvswitch/vlog.h
#include ovn-controller.h
-#include vswitch-idl.h
+#include physical.h
#include rconn.h
#include socket-util.h
+#include vswitch-idl.h
VLOG_DEFINE_THIS_MODULE(ofctrl);
@@ -53,6 +54,9 @@ static char *ovn_flow_to_string(const struct ovn_flow *);
static void ovn_flow_log(const struct ovn_flow *, const char *action);
static void ovn_flow_destroy(struct ovn_flow *);
+static ovs_be32 queue_msg(struct ofpbuf *);
+static void queue_flow_mod(struct ofputil_flow_mod *);
+
/* OpenFlow connection to the switch. */
static struct rconn *swconn;
@@ -60,6 +64,25 @@ static struct rconn *swconn;
* rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */
static unsigned int seqno;
+/* Connection state machine. */
+#define STATES \
+STATE(S_NEW)\
+STATE(S_GENEVE_TABLE_REQUESTED) \
+STATE(S_GENEVE_TABLE_MOD_SENT) \
+STATE(S_CLEAR_FLOWS)\
+STATE(S_UPDATE_FLOWS)
+enum ofctrl_state {
+#define STATE(NAME) NAME,
+STATES
+#undef STATE
+};
+
+/* Current state. */
+static enum ofctrl_state state;
+
+/* Transaction IDs for messages in flight to the switch. */
+static ovs_be32 xid, xid2;
+
/* Counter for in-flight OpenFlow messages on 'swconn'. We only send a
new
* round of flow table modifications to the switch when the counter falls
to
* zero, to avoid unbounded buffering. */
@@ -69,11 +92,15 @@ static struct rconn_packet_counter *tx_counter;
* installed in the switch. */
static struct hmap installed_flows;
+/* MFF_* field ID for our Geneve option. In S_GENEVE_TABLE_MOD_SENT,
this is
+ * the option we requested (we don't know whether we obtained it yet). In
+ * S_CLEAR_FLOWS or S_UPDATE_FLOWS, this is really the option we have. */
+static enum mf_field_id mff_ovn_geneve;
+
static void ovn_flow_table_clear(struct hmap *flow_table);
static void ovn_flow_table_destroy(struct hmap *flow_table);
-static void ofctrl_update_flows(struct hmap *desired_flows);
-static void ofctrl_recv(const struct ofpbuf *msg);
+static void ofctrl_recv(const struct ofp_header *, enum ofptype);
void
ofctrl_init(void)
@@ -82,15 +109,244 @@ ofctrl_init(void)
tx_counter = rconn_packet_counter_create();
hmap_init(installed_flows);
}
+
+/* S_NEW, for a new connection.
+ *
+ * Sends NXT_GENEVE_TABLE_REQUEST and transitions to
+ * S_GENEVE_TABLE_REQUESTED. */
-/* Attempts to update the OpenFlow flows in bridge 'br_int' to those in
- * 'flow_table'. Removes all of the flows from 'flow_table' and frees
them.
+static void
+run_S_NEW(void)
+{
+struct ofpbuf *buf = ofpraw_alloc(OFPRAW_NXT_GENEVE_TABLE_REQUEST,
+ rconn_get_version(swconn), 0);
+xid = queue_msg(buf);
+state = S_GENEVE_TABLE_REQUESTED;
+}
+
+static void
+recv_S_NEW(const struct ofp_header *oh OVS_UNUSED,
+ enum ofptype type OVS_UNUSED)
+{
+OVS_NOT_REACHED();
+}
+
+/* S_GENEVE_TABLE_REQUESTED, when NXT_GENEVE_TABLE_REQUEST has been sent
+ * and we're waiting for a reply.
*
- * The flow table will only be updated if we've got an OpenFlow
connection to
- * 'br_int' and it's not backlogged. Otherwise, it'll have to wait until
the
- * next iteration. */
-void
-ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
+ * If we receive an NXT_GENEVE_TABLE_REPLY:
+ *
+ * - If it contains our tunnel metadata option, assign its field ID to
+ * mff_ovn_geneve and transition to S_CLEAR_FLOWS.
+ *
+ * - Otherwise, if there is an unused tunnel metadata field ID, send
+ * NXT_GENEVE_TABLE_MOD and OFPT_BARRIER_REQUEST, and transition to
+ * S_GENEVE_TABLE_MOD_SENT.
+ *
+ * - Otherwise, log an error, disable Geneve, and transition to
+ * S_CLEAR_FLOWS.
+ *
+ * If we receive an OFPT_ERROR:
+ *
+ * - Log an error, disable Geneve, and transition to S_CLEAR_FLOWS. */
+
+static void
+run_S_GENEVE_TABLE_REQUESTED(void)
+{
+}
+
+static void
+recv_S_GENEVE_TABLE_REQUESTED(const struct ofp_header *oh, enum ofptype
type)
+{
+if (oh-xid !=
Re: [ovs-dev] MPLS support in OVS 2.3.2
Thanks Ben for your reply. Could you please let me know whether the said MPLS actions are supported in OVS or not? Thanks in adv, tech kals. On Fri, Jul 31, 2015 at 8:55 PM, Ben Pfaff b...@nicira.com wrote: On Fri, Jul 31, 2015 at 06:30:15PM +0530, tech_kals Kals wrote: Also, is there any plan to support TTP (Table Type Pattern) in the upcoming OVS release ? No. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 21/22] ofctrl: Negotiate OVN Geneve option.
On Fri, Jul 31, 2015 at 10:04:35PM -0700, Alex Wang wrote:
Is there any documentation for the ovs side geneve negotiation?
No. What kind of documentation would help?
+if (state != S_UPDATE_FLOWS
+|| rconn_packet_counter_n_packets(tx_counter)) {
+ovn_flow_table_clear(flow_table);
+return;
}
Should we use maybe a boolean to mark this case, so that ofctrl_wait
could register a wake up event for re-updating the flows? Wonder if there
could be a delayed flow update issue~
Only rconn_run() could cause rconn_packet_counter_n_packets() to
decrease to 0, and only ofctrl_run() calls rconn_run() on swconn, so it
shouldn't happen currently. I guess it could happen if ofctrl_put()
were called *before* ofctrl_run() instead of after; maybe there should
be a note about that.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Fix STT protocol field for sampling packet.
On Fri, Jul 31, 2015 at 10:21 AM, Jesse Gross je...@nicira.com wrote: On Thu, Jul 30, 2015 at 8:51 PM, Pravin B Shelar pshe...@nicira.com wrote: Fixes typo in STT sampling code. Signed-off-by: Pravin B Shelar pshe...@nicira.com --- datapath/vport-stt.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Acked-by: Jesse Gross je...@nicira.com I pushed patch to master and branch-2.4 Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 5/6] ovn: Add localnet logical port type.
On 07/31/2015 07:26 PM, Ben Pfaff wrote:
On Fri, Jul 31, 2015 at 01:14:44PM -0400, Russell Bryant wrote:
Introduce a new logical port type called localnet. A logical port
with this type also has an option called network_name. A localnet
logical port represents a connection to a locally accessible network.
ovn-controller will use the ovn-bridge-mappings configuration to
figure out which patch port on br-int should be used for this port.
Signed-off-by: Russell Bryant rbry...@redhat.com
Since this reserves a new register for OVN, we should remove that
register from the logical pipeline symbol table (see the loop near the
beginning of symtab_init() in pipeline.c) so that logical flows can't
screw with it.
Maybe the new field should be declared in pipeline.h with the others (my
tunnel key series adds MFF_LOG_DATAPATH here too). It's not really a
logical pipeline field, but it might still make sense to group these
together.
sparse spotted that these be64s in physical_run() should be be32s:
struct ofpact_set_field *sf =
ofpact_put_SET_FIELD(ln_flow-ofpacts);
sf-field = mf_from_id(MFF_OVN_FLAGS);
sf-value.be64 = htonl(OVN_FLAG_LOCALNET);
sf-mask.be64 = OVS_BE64_MAX;
Thanks for the feedback. All of the above makes sense and I'll
incorporate it.
I'm not sure I correctly understand the model for using these. Maybe
you can flesh out an example to help me. Suppose I have a pair of
hypervisors A and B, and each HV has two VMs (A1 and A2, B1 and B2), and
all four of the VMs are connected to the same Neutron provider network.
How many OVN Logical_Switches would I have and what would their
membership look like? On each HV, what interfaces would be attached to
br-int?
That scenario is really close to what I describe in the overview mail,
except I only had 1 VM per hypervisor.
http://openvswitch.org/pipermail/dev/2015-July/058367.html
A simulation of your scenario would be (with ovs-sandbox):
ovs-vsctl add-br br-eth1
ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth1
for n in 1 2 3 4; do
ovn-nbctl lswitch-add provnet1-$n
ovn-nbctl lport-add provnet1-$n provnet1-$n-port1
ovn-nbctl lport-set-macs provnet1-$n-port1 00:00:00:00:00:0$n
ovn-nbctl lport-set-port-security provnet1-$n-port1 00:00:00:00:00:0$n
ovn-nbctl lport-add provnet1-$n provnet1-$n-physnet1
ovn-nbctl lport-set-macs provnet1-$n-physnet1 unknown
ovn-nbctl lport-set-type provnet1-$n-physnet1 localnet
ovn-nbctl lport-set-options provnet1-$n-physnet1 network_name=physnet1
done
The above gets us the following logical config:
$ ovn-nbctl show
lswitch 457a1810-b3c5-40f9-835e-4f86a65bb60d (provnet1-3)
lport provnet1-3-port1
macs: 00:00:00:00:00:03
lport provnet1-3-physnet1
macs: unknown
lswitch 40afcc6e-464e-44db-bb57-0f13da299cbd (provnet1-2)
lport provnet1-2-port1
macs: 00:00:00:00:00:02
lport provnet1-2-physnet1
macs: unknown
lswitch b603dff3-cd7c-4d82-be19-1bfe2a4f5897 (provnet1-4)
lport provnet1-4-physnet1
macs: unknown
lport provnet1-4-port1
macs: 00:00:00:00:00:04
lswitch 0bd64e33-90f7-4d19-b2c2-2e39ba7b3317 (provnet1-1)
lport provnet1-1-physnet1
macs: unknown
lport provnet1-1-port1
macs: 00:00:00:00:00:01
The output doesn't show it, but the physnet ports are type=localnet.
Now we can bind the first 2 ports locally. For the second two ports, we
can create a fake chassis and update the DB to reflect that the ports
are bound to the fake chassis.
ovs-vsctl add-port br-int lport1 -- set Interface lport1
external_ids:iface-id=provnet1-1-port1
ovs-vsctl add-port br-int lport2 -- set Interface lport1
external_ids:iface-id=provnet1-2-port1
# Create a fake chassis
encaps_uuid=$(ovsdb-client dump OVN_Southbound | grep -A 3 Encap | tail -n 1
| awk '{print $1}')
chassis=$(ovsdb-client transact
'[OVN_Southbound,{op:insert,table:Chassis,row:{name:fakechassis,encaps:[uuid,'$encaps_uuid']}}]')
chassis_uuid=$(echo $chassis | sed -e 's/^.*\uuid\,\\(.*\)\.*/\1/')
uuid=$(ovsdb-client dump OVN_Southbound | grep -A 10 Binding | grep
provnet1-3-port1 | awk '{print $1}')
ovsdb-client transact
'[OVN_Southbound,{op:update,table:Binding,where:[[_uuid,==,[uuid,'$uuid']]],row:{chassis:[uuid,'$chassis_uuid']}}]'
uuid=$(ovsdb-client dump OVN_Southbound | grep -A 10 Binding | grep
provnet1-4-port1 | awk '{print $1}')
ovsdb-client transact
'[OVN_Southbound,{op:update,table:Binding,where:[[_uuid,==,[uuid,'$uuid']]],row:{chassis:[uuid,'$chassis_uuid']}}]'
Does that make sense?
--
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 6/6] ovn: Automatically create br-int in ovn-controller.
On 07/31/2015 07:27 PM, Ben Pfaff wrote: On Fri, Jul 31, 2015 at 01:14:45PM -0400, Russell Bryant wrote: ovn-controller previously required the integration bridge to be created before running ovn-controller. This patch makes ovn-controller automatically create it if it doesn't already exist. Signed-off-by: Russell Bryant rbry...@redhat.com Seems reasonable, but I'd like to get the tunnel-key series in first if that's OK. Of course. I'll hold on to it and resubmit later. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when adding OVS ports
hi Sorin,
In OvsInitExternalNBLContext(), there’s the following check:
563 if (poolHandle == context-ovsPool.ndisHandle ||
564 nbl-SourceHandle == context-ovsPool.ndisHandle) {
565 return (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(nbl);
When you call OvsPartialCopyToMultipleNBLs(), we end up calling
OvsPartialCopyNBL() - OvsAllocateNBLContext() for each of the NBLs, which
nbl-SourceHandle is set to context-ovsPool.ndisHandle.
So, this check should have caught your case. No?
thanks,
-- Nithin
On Jul 1, 2015, at 8:12 AM, Eitan Eliahu elia...@vmware.com wrote:
Hi Sorin,
Thank you for debugging this issue.
I think that the NBL context signature is used for code validation only
(think about a signature we put in an object for the purpose of validating
this object when executing in a different context).
Can you think about another way to fix this issue (e.g. a status/flag)?
Thanks,
Eitan
-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Wednesday, July 01, 2015 7:02 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when adding OVS ports
This BSOD occurred in the context of a packet (NBL) with multiple
NET_BUFFER(s) (NBs). The reason for the BSOD is due to the marking of NBLs
created by OVS as being external and wrongly completing them.
This patch should be applied both on master and branch 2.4.
Signed-off-by: Sorin Vinturis svintu...@cloudbasesolutions.com
Reported-by: Sorin Vinturis svintu...@cloudbasesolutions.com
Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_82d=BQIGaQc=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEsr=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCYm=yx7HeX13135X7eeWKg3s73H3l711NpzfUes52_NsBqQs=3pshLMQB4SoTOb8sZncux7Bi6G5ADQwH7I0TlH9Is2Ue=
---
datapath-windows/ovsext/BufferMgmt.c | 8
1 file changed, 8 insertions(+)
diff --git a/datapath-windows/ovsext/BufferMgmt.c
b/datapath-windows/ovsext/BufferMgmt.c
index 83d6cde..d36dfa2 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -558,6 +558,12 @@ OvsInitExternalNBLContext(PVOID ovsContext,
NDIS_STATUS status;
UINT16 flags;
+ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(nbl);
+if (ctx ctx-magic == OVS_CTX_MAGIC) {
+OVS_LOG_INFO(nbl context already initialized/allocated by OVS.);
+return ctx;
+}
+
poolHandle = NdisGetPoolFromNetBufferList(nbl);
if (poolHandle == context-ovsPool.ndisHandle) { @@ -801,6 +807,7 @@
OvsPartialCopyNBL(PVOID ovsContext,
OVS_DEFAULT_PORT_NO);
InterlockedIncrement((LONG volatile *)srcCtx-refCount);
+
#ifdef DBG
OvsDumpNetBufferList(nbl);
OvsDumpForwardingDetails(nbl);
@@ -808,6 +815,7 @@ OvsPartialCopyNBL(PVOID ovsContext,
OvsDumpNetBufferList(newNbl);
OvsDumpForwardingDetails(newNbl);
#endif
+
OVS_LOG_LOUD(Partial Copy new NBL: %p, newNbl);
return newNbl;
--
1.9.0.msysgit.0
___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_devd=BQIGaQc=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEsr=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCYm=yx7HeX13135X7eeWKg3s73H3l711NpzfUes52_NsBqQs=M6NSK31Vva4-yAxaCNBMGVI5d9vESZHrNxsIVtrjJEEe=
___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_devd=BQIGaQc=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEsr=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80m=CfEqHqD5AnXmgd1_rqtkOlUi5hqSD_ea5ybq21lxcQAs=TiOSVk4d09Xaoq_FwyrQqOq7q2HF-RkfhTUQxsmVYiEe=
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 2/6] ovn: Add bridge mappings to ovn-controller.
On 07/31/2015 07:07 PM, Ben Pfaff wrote:
On Fri, Jul 31, 2015 at 01:14:41PM -0400, Russell Bryant wrote:
Add a new OVN configuration entry in the Open_vSwitch database called
ovn-bridge-mappings. This allows the configuration of mappings
between a physical network name and an OVS bridge that provides
connectivity to that network.
For example, if you wanted to configure physnet1 to map to br-eth0
and physnet2 to map to br-eth1, the configuration would be:
$ ovs-vsctl set open . \
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
Patch ports between these bridges and the integration bridge are
automatically created and also removed if necessary when the
configuration changes.
Documentation for this configuration value is introduced in a later
patch that makes use of this by introducing a localnet logical port
type.
Signed-off-by: Russell Bryant rbry...@redhat.com
I tested this with:
make sandbox SANDBOXFLAGS=--ovn
ovs-vsctl add-br br-eth0
ovs-vsctl add-br br-eth1
ovs-vsctl set open .
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
ovs-vsctl show
The results:
Bridge br-eth1
Port patch-br-eth1-to-br-int
Interface patch-br-eth1-to-br-int
type: patch
options: {peer=br-int}
Port br-eth1
Interface br-eth1
type: internal
Bridge br-int
fail_mode: secure
Port br-int
Interface br-int
type: internal
Port patch-br-int-to-br-eth0
Interface patch-br-int-to-br-eth0
type: patch
options: {peer=br-eth0}
Port patch-br-int-to-br-eth1
Interface patch-br-int-to-br-eth1
type: patch
options: {peer=br-eth1}
Bridge br-eth0
Port patch-br-eth0-to-br-int
Interface patch-br-eth0-to-br-int
type: patch
options: {peer=br-int}
Port br-eth0
Interface br-eth0
type: internal
This looks close to correct to me. The peer names are all wrong,
though: the peer of a patch port is the name of another patch port, not
the name of another bridge.
Oops, I obviously fundamentally misunderstood how to use peer.
I think that the code is intended to remove the patch ports when they
are not longer needed, but when I followed up with:
ovs-vsctl remove open . external-ids ovn-bridge-mappings
ovs-vsctl show
I still saw them.
Well ... that's embarrassing. Yeah, it doesn't work anymore for me
either. I'll fix all of this up.
--
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 1/5] lib: Add smap_equal().
Add a method to determine of two smaps are equal (have the exact same
set of key-value pairs).
Suggested-by: Ben Pfaff b...@nicira.com
Signed-off-by: Russell Bryant rbry...@redhat.com
---
lib/smap.c | 20
lib/smap.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/lib/smap.c b/lib/smap.c
index 7fe3ce4..8865a88 100644
--- a/lib/smap.c
+++ b/lib/smap.c
@@ -302,6 +302,26 @@ smap_to_json(const struct smap *smap)
}
return json;
}
+
+/* Returns true if the two maps are equal, meaning that they have the same set
+ * of key-value pairs.
+ */
+bool
+smap_equal(const struct smap *smap1, const struct smap *smap2)
+{
+if (smap_count(smap1) != smap_count(smap2)) {
+return false;
+}
+
+const struct smap_node *node;
+SMAP_FOR_EACH (node, smap1) {
+const char *value2 = smap_get(smap2, node-key);
+if (!value2 || strcmp(node-value, value2)) {
+return false;
+}
+}
+return true;
+}
/* Private Helpers. */
diff --git a/lib/smap.h b/lib/smap.h
index caf3efc..cac3878 100644
--- a/lib/smap.h
+++ b/lib/smap.h
@@ -67,4 +67,6 @@ const struct smap_node **smap_sort(const struct smap *);
void smap_from_json(struct smap *, const struct json *);
struct json *smap_to_json(const struct smap *);
+bool smap_equal(const struct smap *, const struct smap *);
+
#endif /* smap.h */
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 4/5] ovn: Get/set lport type and options in ovn-nbctl.
A recent patch added type and options columns to the Logical_Port
table in OVN_Northbound. This patch allows you to get and set those
columns with ovn-nbctl.
ovn-nbctl should eventually get converted to use the common db-ctl
code that was recently added. When that happens, these commands can
just be removed.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/ovn-nbctl.8.xml | 24 ++--
ovn/ovn-nbctl.c | 111
2 files changed, 132 insertions(+), 3 deletions(-)
diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml
index 39ffb35..ba3cc82 100644
--- a/ovn/ovn-nbctl.8.xml
+++ b/ovn/ovn-nbctl.8.xml
@@ -23,9 +23,7 @@
h1Logical Switch Commands/h1
dl
- dtcodelswitch-add/code [varlswitch/var]/dt
- dd
-Creates a new logical switch named varlswitch/var. If
+ dtcodelswitch-add/code [varlswitch/var]/dt dd Creates a
new logical switch named varlswitch/var. If
varlswitch/var is not provided, the switch will not have a
name so other commands must refer to this switch by its UUID.
Initially the switch will have no ports.
@@ -192,6 +190,26 @@
or codedisabled/code.
/dd
+ dtcodelport-set-type/code varlport/var vartype/var/dt
+ dd
+Set the type for the logical port. No special types have been
implemented yet.
+ /dd
+
+ dtcodelport-get-type/code varlport/var/dt
+ dd
+Get the type for the logical port.
+ /dd
+
+ dtcodelport-set-options/code varlport/var
[varkey=value/var].../dt
+ dd
+Set type-specific key-value options for the logical port.
+ /dd
+
+ dtcodelport-get-options/code varlport/var/dt
+ dd
+Get the type-specific options for the logical port.
+ /dd
+
/dl
h1Options/h1
diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c
index 8430122..0bdb3a3 100644
--- a/ovn/ovn-nbctl.c
+++ b/ovn/ovn-nbctl.c
@@ -86,6 +86,11 @@ Logical port commands:\n\
('enabled' or 'disabled')\n\
lport-get-enabled LPORT get administrative state LPORT\n\
('enabled' or 'disabled')\n\
+ lport-set-type LPORT TYPE Set the type for LPORT\n\
+ lport-get-type LPORT Get the type for LPORT\n\
+ lport-set-options LPORT KEY=VALUE [KEY=VALUE]...\n\
+Set options related to the type of LPORT\n\
+ lport-get-options LPORT Get the type specific options for LPORT\n\
\n\
Options:\n\
--db=DATABASE connect to DATABASE\n\
@@ -627,6 +632,84 @@ do_lport_get_enabled(struct ovs_cmdl_context *ctx)
printf(%s\n,
(!lport-enabled || *lport-enabled) ? enabled : disabled);
}
+
+static void
+do_lport_set_type(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const char *type = ctx-argv[2];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+nbrec_logical_port_set_type(lport, type);
+}
+
+static void
+do_lport_get_type(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+printf(%s\n, lport-type);
+}
+
+static void
+do_lport_set_options(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+size_t i;
+struct smap options = SMAP_INITIALIZER(options);
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+for (i = 2; i ctx-argc; i++) {
+char *key, *value;
+value = xstrdup(ctx-argv[i]);
+key = strsep(value, =);
+if (value) {
+smap_add(options, key, value);
+}
+free(key);
+}
+
+nbrec_logical_port_set_options(lport, options);
+
+smap_destroy(options);
+}
+
+static void
+do_lport_get_options(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+struct smap_node *node;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+SMAP_FOR_EACH(node, lport-options) {
+printf(%s=%s\n, node-key, node-value);
+}
+}
static void
parse_options(int argc, char *argv[])
@@ -828,6 +911,34 @@ static const struct ovs_cmdl_command all_commands[] = {
.max_args = 1,
.handler = do_lport_get_enabled,
},
+{
+.name = lport-set-type,
+.usage = LPORT TYPE,
+.min_args = 2,
+.max_args = 2,
+.handler = do_lport_set_type,
+},
+{
+.name = lport-get-type,
+.usage = LPORT,
+.min_args = 1,
+
Re: [ovs-dev] [PATCH net-next 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
Hi Thomas,
I have applied your below mentioned 1ad patch to ovs-master code. Compiled
successfully. I am running ovs in a centos machine.
I have created bridge and configured ports using below commands.
ovs-vsctl --no-wait add-br br0
ovs-vsctl --no-wait add-port br0 eth0 tag=100 vlan_mode=native-tagged /access
ovs-vsctl --no-wait add-port br0 eth1
Configured the bridge to work in legacy bridge mode
$ ovs-ofctl dump-flows br0
NXST_FLOW reply (xid=0x4):
cookie=0x0, duration=15.458s, table=0, n_packets=0, n_bytes=0, idle_age=15,
priority=0 actions=NORMAL
And started running vswitchd
$ ovs-vswitchd --pidfile
Started pumping traffic from outside using ostinato packet generator for
Phy-Phy scenario(sending to eth0 ,receiving back on eth1)
My observation is
1) For untagged packet received on eth0 pushing vid 100 and packet flow is fine
2) For already tagged (1q) packet received on eth0 packet is dropped , I could
see vswitchd log throwing below error(duplicate eth_type attribute in flow key)
2015-07-31T12:13:59Z|1|ovs_numa|INFO|Discovered 4 CPU cores on NUMA node 0
2015-07-31T12:13:59Z|2|ovs_numa|INFO|Discovered 1 NUMA nodes and 4 CPU cores
2015-07-31T12:13:59Z|3|reconnect|INFO|unix:/usr/local/var/run/openvswitch/db.sock:
connecting...
2015-07-31T12:13:59Z|4|reconnect|INFO|unix:/usr/local/var/run/openvswitch/db.sock:
connected
2015-07-31T12:13:59Z|5|ofproto_dpif|INFO|system@ovs-system: Datapath
supports recirculation
2015-07-31T12:13:59Z|6|ofproto_dpif|INFO|system@ovs-system: MPLS label
stack length probed as 1
2015-07-31T12:13:59Z|7|ofproto_dpif|INFO|system@ovs-system: Datapath
supports unique flow ids
2015-07-31T12:13:59Z|1|ofproto_dpif_upcall(handler1)|INFO|received packet
on unassociated datapath port 0
2015-07-31T12:13:59Z|8|bridge|INFO|bridge br0: added interface eth0 on port
1
2015-07-31T12:13:59Z|9|bridge|INFO|bridge br0: added interface br0 on port
65534
2015-07-31T12:13:59Z|00010|bridge|INFO|bridge br0: added interface eth1 on port
2
2015-07-31T12:13:59Z|00011|bridge|INFO|bridge br0: using datapath ID
eac07aea5143
2015-07-31T12:13:59Z|00012|connmgr|INFO|br0: added service controller
punix:/usr/local/var/run/openvswitch/br0.mgmt
2015-07-31T12:13:59Z|00013|bridge|INFO|ovs-vswitchd (Open vSwitch) 2.3.90
2015-07-31T12:14:09Z|00014|memory|INFO|2420 kB peak resident set size after
10.0 seconds
2015-07-31T12:14:09Z|00015|memory|INFO|handlers:2 ports:3 revalidators:2 rules:5
2015-07-31T12:16:31Z|1|odp_util(handler6)|ERR|duplicate eth_type attribute
in flow key
2015-07-31T12:26:51Z|2|odp_util(handler6)|ERR|duplicate eth_type attribute
in flow key
Please, let me know whether I missed anything in the configuration. It would be
helpful if someone could let me
Know how I can insert 1ad tag on ingress.
Thanks Regards,
Uday
-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Thomas F Herbert
Sent: Sunday, July 26, 2015 8:23 PM
To: net...@vger.kernel.org; pshe...@nicira.com
Cc: dev@openvswitch.org; therb...@redhat.com
Subject: [ovs-dev] [PATCH net-next 3/3] openvswitch: 802.1AD: Flow handling,
actions, vlan parsing and netlink attributes
Add support for 802.1ad including the ability to push and pop double tagged
vlans. Add support for 802.1ad to netlink parsing and flow conversion. Uses
double nested encap attributes to represent double tagged vlan. Inner TPID
encoded along with ctci in nested attributes. Allows either 0x8100 or 0x88a8 on
inner or outer tags.
Signed-off-by: Thomas F Herbert thomasfherb...@gmail.com
---
net/openvswitch/flow.c | 84 +++---
net/openvswitch/flow.h | 5 ++
net/openvswitch/flow_netlink.c | 196 ++---
3 files changed, 243 insertions(+), 42 deletions(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index
8db22ef..0abab37 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -298,21 +298,80 @@ static bool icmp6hdr_ok(struct sk_buff *skb) static int
parse_vlan(struct sk_buff *skb, struct sw_flow_key *key) {
struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
__be16 tci;
};
- struct qtag_prefix *qp;
+ struct qtag_prefix *qp = (struct qtag_prefix *)skb-data;
- if (unlikely(skb-len sizeof(struct qtag_prefix) + sizeof(__be16)))
+ struct qinqtag_prefix {
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
+ __be16 tci;
+ __be16 inner_tpid; /* ETH_P_8021Q */
+ __be16 ctci;
+ };
+
+ if (likely(skb_vlan_tag_present(skb))) {
+ key-eth.tci = htons(skb-vlan_tci);
+
+ /* Case where upstream
+* processing has already stripped the outer vlan tag.
+*/
+ if (unlikely(skb-vlan_proto ==
[ovs-dev] MPLS support in OVS 2.3.2
Hi Experts, Could someone clarify me whether the following MPLS actions are supported in latest OVS. MPLS L3 VPN Label group entry bucket actions: Push MPLS shim header (for VPN label; this sets the Ether type to 0x8847) Set-Field MPLS Label Set-Field BOS (bottom of stack) Set-Field MPLS TC (EXP, optional) Decrements and Check TTL (depends on label) Set-Field TTL (value or copy out) Set-Field PCP (in L2 header, 0ptional) Group (MPLS Tunnel Label) For MPLS LSR (MPLS Label Switch Router) The MPLS SWAP Label group entry bucket actions for P include: Push MPLS Label (if LSP label popped) Set-Field MPLS Label (set or swap label) Set-Field TC (EXP, optional) Decrements and check TTL Set-Field TTL (optional) Set-Field PCP (in outermost L2 header) – explicit value or from table. Optional. Group (MPLS Tunnel Label) Also, is there any plan to support TTP (Table Type Pattern) in the upcoming OVS release ? If so, could you please kindly let me know when will be the next release ? Thanks, tech.kals ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 5/5] ovn: Add localnet logical port type.
Introduce a new logical port type called localnet. A logical port
with this type also has an option called network_name. A localnet
logical port represents a connection to a locally accessible network.
ovn-controller will use the ovn-bridge-mappings configuration to
figure out which patch port on br-int should be used for this port.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/controller/ovn-controller.c | 3 +-
ovn/controller/physical.c | 145 +++-
ovn/controller/physical.h | 4 +-
ovn/ovn-nb.xml | 16 -
ovn/ovn-sb.xml | 30 -
5 files changed, 159 insertions(+), 39 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index f643a5d..41cd0c8 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -509,7 +509,8 @@ main(int argc, char *argv[])
struct hmap flow_table = HMAP_INITIALIZER(flow_table);
pipeline_run(ctx, flow_table);
if (chassis_id) {
-physical_run(ctx, br_int, chassis_id, flow_table);
+physical_run(ctx, br_int, chassis_id, bridge_mappings,
+flow_table);
}
ofctrl_run(br_int, flow_table);
hmap_destroy(flow_table);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 9009e2b..386e4ae 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -22,9 +22,18 @@
#include ovn-controller.h
#include ovn/lib/ovn-sb-idl.h
#include pipeline.h
+#include shash.h
#include simap.h
+#include smap.h
#include vswitch-idl.h
+/* A register of bit flags for OVN */
+#define MFF_OVN_FLAGS MFF_REG5
+enum {
+/* Indicates that the packet came in on a localnet port */
+OVN_FLAG_LOCALNET = (1 0),
+};
+
void
physical_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
@@ -42,12 +51,25 @@ physical_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, ovsrec_interface_col_external_ids);
}
+static void
+init_input_match(struct match *match, ofp_port_t ofport, int tag)
+{
+match_init_catchall(match);
+match_set_in_port(match, ofport);
+if (tag) {
+match_set_dl_vlan(match, htons(tag));
+}
+}
+
void
physical_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
- const char *this_chassis_id, struct hmap *flow_table)
+ const char *this_chassis_id, struct smap *bridge_mappings,
+ struct hmap *flow_table)
{
struct simap lport_to_ofport = SIMAP_INITIALIZER(lport_to_ofport);
struct simap chassis_to_ofport = SIMAP_INITIALIZER(chassis_to_ofport);
+struct simap localnet_to_ofport = SIMAP_INITIALIZER(localnet_to_ofport);
+
for (int i = 0; i br_int-n_ports; i++) {
const struct ovsrec_port *port_rec = br_int-ports[i];
if (!strcmp(port_rec-name, br_int-name)) {
@@ -72,8 +94,17 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
continue;
}
-/* Record as chassis or local logical port. */
-if (chassis_id) {
+/* Record as patch to local net, chassis, or local logical port. */
+if (!strcmp(iface_rec-type, patch)) {
+const char *peer = smap_get(iface_rec-options, peer);
+if (!peer) {
+continue;
+}
+const char *localnet = smap_get(bridge_mappings, peer);
+if (localnet) {
+simap_put(localnet_to_ofport, localnet, ofport);
+}
+} else if (chassis_id) {
simap_put(chassis_to_ofport, chassis_id, ofport);
break;
} else {
@@ -89,6 +120,13 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
struct ofpbuf ofpacts;
ofpbuf_init(ofpacts, 0);
+struct localnet_flow {
+struct shash_node node;
+struct match match;
+struct ofpbuf ofpacts;
+};
+struct shash localnet_inputs = SHASH_INITIALIZER(localnet_inputs);
+
/* Set up flows in table 0 for physical-to-logical translation and in table
* 64 for logical-to-physical translation. */
const struct sbrec_binding *binding;
@@ -100,10 +138,15 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
* (and set 'local' to true). When 'parent_port' is set for a binding,
* it implies a container sitting inside a VM reachable via a 'tag'.
*/
-
int tag = 0;
ofp_port_t ofport;
-if (binding-parent_port) {
+if (!strcmp(binding-type, localnet)) {
+const char *network = smap_get(binding-options, network_name);
+if (!network) {
+continue;
+}
+ofport = u16_to_ofp(simap_get(localnet_to_ofport,
[ovs-dev] [PATCH v4 2/5] ovn: Add bridge mappings to ovn-controller.
Add a new OVN configuration entry in the Open_vSwitch database called
ovn-bridge-mappings. This allows the configuration of mappings
between a physical network name and an OVS bridge that provides
connectivity to that network.
For example, if you wanted to configure physnet1 to map to br-eth0
and physnet2 to map to br-eth1, the configuration would be:
$ ovs-vsctl set open . \
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
Patch ports between these bridges and the integration bridge are
automatically created and also removed if necessary when the
configuration changes.
Documentation for this configuration value is introduced in a later
patch that makes use of this by introducing a localnet logical port
type.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/controller/ovn-controller.c | 238 +++-
ovn/controller/ovn-controller.h | 2 +
2 files changed, 236 insertions(+), 4 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 559cb0b..f643a5d 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -71,6 +71,18 @@ get_initial_snapshot(struct ovsdb_idl *idl)
}
static const struct ovsrec_bridge *
+get_bridge(struct ovsdb_idl *ovs_idl, const char *br_name)
+{
+const struct ovsrec_bridge *br;
+OVSREC_BRIDGE_FOR_EACH (br, ovs_idl) {
+if (!strcmp(br-name, br_name)) {
+return br;
+}
+}
+return NULL;
+}
+
+static const struct ovsrec_bridge *
get_br_int(struct ovsdb_idl *ovs_idl)
{
const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
@@ -84,10 +96,9 @@ get_br_int(struct ovsdb_idl *ovs_idl)
}
const struct ovsrec_bridge *br;
-OVSREC_BRIDGE_FOR_EACH (br, ovs_idl) {
-if (!strcmp(br-name, br_int_name)) {
-return br;
-}
+br = get_bridge(ovs_idl, br_int_name);
+if (br) {
+return br;
}
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
@@ -102,6 +113,209 @@ get_chassis_id(const struct ovsdb_idl *ovs_idl)
return cfg ? smap_get(cfg-external_ids, system-id) : NULL;
}
+/*
+ * Return true if the port is a patch port to a given bridge
+ */
+static bool
+match_patch_port(const struct ovsrec_port *port, const struct ovsrec_bridge
*to_br)
+{
+struct ovsrec_interface *iface;
+size_t i;
+
+for (i = 0; i port-n_interfaces; i++) {
+const char *peer;
+iface = port-interfaces[i];
+if (strcmp(iface-type, patch)) {
+continue;
+}
+peer = smap_get(iface-options, peer);
+if (peer !strcmp(peer, to_br-name)) {
+return true;
+}
+}
+
+return false;
+}
+
+static void
+create_patch_port(struct controller_ctx *ctx,
+ const char *network,
+ const struct ovsrec_bridge *b1,
+ const struct ovsrec_bridge *b2)
+{
+struct ovsrec_interface *iface;
+struct ovsrec_port *port, **ports;
+size_t i;
+char *port_name;
+
+port_name = xasprintf(patch-%s-to-%s, b1-name, b2-name);
+
+ovsdb_idl_txn_add_comment(ctx-ovs_idl_txn,
+ovn-controller: creating patch port '%s' from '%s' to '%s',
+port_name, b1-name, b2-name);
+
+iface = ovsrec_interface_insert(ctx-ovs_idl_txn);
+ovsrec_interface_set_name(iface, port_name);
+ovsrec_interface_set_type(iface, patch);
+struct smap options = SMAP_INITIALIZER(options);
+smap_add(options, peer, b2-name);
+ovsrec_interface_set_options(iface, options);
+smap_destroy(options);
+
+port = ovsrec_port_insert(ctx-ovs_idl_txn);
+ovsrec_port_set_name(port, port_name);
+ovsrec_port_set_interfaces(port, iface, 1);
+struct smap ext_ids = SMAP_INITIALIZER(ext_ids);
+smap_add(ext_ids, ovn-patch-port, network);
+ovsrec_port_set_external_ids(port, ext_ids);
+smap_destroy(ext_ids);
+
+ports = xmalloc(sizeof *port * (b1-n_ports + 1));
+for (i = 0; i b1-n_ports; i++) {
+ports[i] = b1-ports[i];
+}
+ports[i] = port;
+ovsrec_bridge_verify_ports(b1);
+ovsrec_bridge_set_ports(b1, ports, b1-n_ports + 1);
+
+free(ports);
+free(port_name);
+}
+
+static void
+create_patch_ports(struct controller_ctx *ctx,
+ const char *network,
+ struct shash *existing_ports,
+ const struct ovsrec_bridge *b1,
+ const struct ovsrec_bridge *b2)
+{
+size_t i;
+
+for (i = 0; i b1-n_ports; i++) {
+if (match_patch_port(b1-ports[i], b2)) {
+/* Patch port already exists on b1 */
+shash_find_and_delete(existing_ports, b1-ports[i]-name);
+break;
+}
+}
+if (i == b1-n_ports) {
+create_patch_port(ctx, network, b1, b2);
+}
+}
+
+static void
+init_existing_ports(struct controller_ctx *ctx,
+struct shash
[ovs-dev] [PATCH v4 3/5] ovn: Add type and options to logical port.
We have started discussing the use of the logical port abstraction in
OVN to represent special types of connections into an OVN logical
switch. This patch proposes some schema updates to reflect these
special types of logical ports. A logical port can have a type and
a set of options specific to that type.
Some examples of logical port types would be vtep for connectivity
to a VTEP gateway or localnet for a connection to a locally
accessible network via an ovs bridge. Actualy support for these (or
other) types will come in later patches.
Signed-off-by: Russell Bryant rbry...@redhat.com
Acked-by: Ben Pfaff b...@nicira.com
---
ovn/northd/ovn-northd.c | 11 +++
ovn/ovn-nb.ovsschema| 6 ++
ovn/ovn-nb.xml | 17 +
ovn/ovn-sb.ovsschema| 6 ++
ovn/ovn-sb.xml | 17 +
5 files changed, 57 insertions(+)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 94e89ee..2a1913e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -558,6 +558,12 @@ set_bindings(struct northd_context *ctx)
sbrec_binding_set_logical_datapath(binding,
*logical_datapath);
}
+if (!strings_equal(binding-type, lport-type)) {
+sbrec_binding_set_type(binding, lport-type);
+}
+if (!smap_equal(binding-options, lport-options)) {
+sbrec_binding_set_options(binding, lport-options);
+}
} else {
/* There is no binding for this logical port, so create one. */
@@ -578,6 +584,9 @@ set_bindings(struct northd_context *ctx)
sbrec_binding_set_tunnel_key(binding, tunnel_key);
sbrec_binding_set_logical_datapath(binding, *logical_datapath);
+sbrec_binding_set_type(binding, lport-type);
+sbrec_binding_set_options(binding, lport-options);
+
/* Add the tunnel key to the tk_hmap so that we don't try to
* use it for another port. (We don't want it in the lp_hmap
* because that would just get the Binding record deleted
@@ -806,6 +815,8 @@ main(int argc, char *argv[])
ovsdb_idl_add_column(ovnsb_idl, sbrec_binding_col_parent_port);
ovsdb_idl_add_column(ovnsb_idl, sbrec_binding_col_logical_datapath);
ovsdb_idl_add_column(ovnsb_idl, sbrec_binding_col_tunnel_key);
+ovsdb_idl_add_column(ovnsb_idl, sbrec_binding_col_type);
+ovsdb_idl_add_column(ovnsb_idl, sbrec_binding_col_options);
ovsdb_idl_add_column(ovnsb_idl, sbrec_pipeline_col_logical_datapath);
ovsdb_idl_omit_alert(ovnsb_idl, sbrec_pipeline_col_logical_datapath);
ovsdb_idl_add_column(ovnsb_idl, sbrec_pipeline_col_table_id);
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 508b6cd..f17b649 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -25,6 +25,12 @@
Logical_Port: {
columns: {
name: {type: string},
+type: {type: string},
+options: {
+ type: {key: string,
+ value: string,
+ min: 0,
+ max: unlimited}},
parent_name: {type: {key: string, min: 0, max: 1}},
tag: {
type: {key: {type: integer,
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index 032e23d..fac6ad0 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -108,6 +108,23 @@
/p
/column
+column name=type
+ p
+ Specify a type for this logical port. Logical ports can be used to model
+ other types of connectivity into an OVN logical switch. Leaving this
column
+ blank maintains the default logical port behavior.
+ /p
+
+ p
+ There are no other logical port types implemented yet.
+ /p
+/column
+
+column name=options
+This column provides key/value settings specific to the logical port
+ref column=type/.
+/column
+
column name=parent_name
When ref column=name/ identifies the interface of a container
spawned inside a tenant VM, this column represents the VM interface
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index f255006..e50e671 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -48,6 +48,12 @@
columns: {
logical_datapath: {type: uuid},
logical_port: {type: string},
+type: {type: string},
+options: {
+ type: {key: string,
+ value: string,
+ min: 0,
+ max: unlimited}},
tunnel_key: {
type: {key: {type: integer,
minInteger: 1,
diff
Re: [ovs-dev] [PATCH net-next 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes
On 7/31/15 5:34 AM, ravulakollu.ku...@wipro.com wrote: Hi Thomas, I have applied your below mentioned 1ad patch to ovs-master code. Compiled successfully. I am running ovs in a centos machine. I have created bridge and configured ports using below commands. ovs-vsctl --no-wait add-br br0 ovs-vsctl --no-wait add-port br0 eth0 tag=100 vlan_mode=native-tagged /access ovs-vsctl --no-wait add-port br0 eth1 Configured the bridge to work in legacy bridge mode $ ovs-ofctl dump-flows br0 NXST_FLOW reply (xid=0x4): cookie=0x0, duration=15.458s, table=0, n_packets=0, n_bytes=0, idle_age=15, priority=0 actions=NORMAL And started running vswitchd $ ovs-vswitchd --pidfile Started pumping traffic from outside using ostinato packet generator for Phy-Phy scenario(sending to eth0 ,receiving back on eth1) My observation is 1) For untagged packet received on eth0 pushing vid 100 and packet flow is fine 2) For already tagged (1q) packet received on eth0 packet is dropped , I could see vswitchd log throwing below error(duplicate eth_type attribute in flow key) 2015-07-31T12:13:59Z|1|ovs_numa|INFO|Discovered 4 CPU cores on NUMA node 0 2015-07-31T12:13:59Z|2|ovs_numa|INFO|Discovered 1 NUMA nodes and 4 CPU cores 2015-07-31T12:13:59Z|3|reconnect|INFO|unix:/usr/local/var/run/openvswitch/db.sock: connecting... 2015-07-31T12:13:59Z|4|reconnect|INFO|unix:/usr/local/var/run/openvswitch/db.sock: connected 2015-07-31T12:13:59Z|5|ofproto_dpif|INFO|system@ovs-system: Datapath supports recirculation 2015-07-31T12:13:59Z|6|ofproto_dpif|INFO|system@ovs-system: MPLS label stack length probed as 1 2015-07-31T12:13:59Z|7|ofproto_dpif|INFO|system@ovs-system: Datapath supports unique flow ids 2015-07-31T12:13:59Z|1|ofproto_dpif_upcall(handler1)|INFO|received packet on unassociated datapath port 0 2015-07-31T12:13:59Z|8|bridge|INFO|bridge br0: added interface eth0 on port 1 2015-07-31T12:13:59Z|9|bridge|INFO|bridge br0: added interface br0 on port 65534 2015-07-31T12:13:59Z|00010|bridge|INFO|bridge br0: added interface eth1 on port 2 2015-07-31T12:13:59Z|00011|bridge|INFO|bridge br0: using datapath ID eac07aea5143 2015-07-31T12:13:59Z|00012|connmgr|INFO|br0: added service controller punix:/usr/local/var/run/openvswitch/br0.mgmt 2015-07-31T12:13:59Z|00013|bridge|INFO|ovs-vswitchd (Open vSwitch) 2.3.90 2015-07-31T12:14:09Z|00014|memory|INFO|2420 kB peak resident set size after 10.0 seconds 2015-07-31T12:14:09Z|00015|memory|INFO|handlers:2 ports:3 revalidators:2 rules:5 2015-07-31T12:16:31Z|1|odp_util(handler6)|ERR|duplicate eth_type attribute in flow key 2015-07-31T12:26:51Z|2|odp_util(handler6)|ERR|duplicate eth_type attribute in flow key I believe this may be a bug in the patch. Also, what you are doing above should work without the patch. Please, let me know whether I missed anything in the configuration. It would be helpful if someone could let me Know how I can insert 1ad tag on ingress. The patch supports pushing and popping outer tags for 802.1ad. The previously submitted user space patch is required as well. To test: take the switch out of NORMAL mode so it won't flood packets, set up veths, bridges and ports. The test should accept untagged traffic in eth p1p1 and double tagged traffic in eth em4 sudo ip link add type veth sudo ifconfig veth0 up sudo ifconfig veth1 up sudo ovs-vsctl add-br br0 sudo ovs-vsctl -- set bridge br0 fail-mode=secure sudo ovs-vsctl -- set bridge br0 protocols=[OpenFlow11] sudo ovs-vsctl add-port br0 em4 sudo ovs-vsctl add-port br0 veth0 # # Add Customer bridge # sudo ovs-vsctl add-br br1 sudo ovs-vsctl -- set bridge br1 fail-mode=secure sudo ovs-vsctl -- set bridge br1 protocols=[OpenFlow11] sudo ovs-vsctl add-port br1 veth1 sudo ovs-vsctl add-port br1 p1p2 Then add flows to push and pop vlans as follows. sudo ovs-ofctl --protocols=OpenFlow11 add-flow br0 in_port=1,dl_vlan=100,actions=pop_vlan,output:2 # # Packets to Core # sudo ovs-ofctl --protocols=OpenFlow11 add-flow br0 in_port=2,actions=push_vlan:0x88a8,load:100-\OXM_OF_VLAN_VID[],output:1 # # Strip c-tag # # Ctag Packets from br0,VID:998 # sudo ovs-ofctl --protocols=OpenFlow11 add-flow br1 in_port=1,dl_vlan=998,actions=pop_vlan,output:2 # # Ctag Packets from cpe to VID:998 # sudo ovs-ofctl --protocols=OpenFlow11 add-flow br1 in_port=2,actions=push_vlan:0x8100,load:998-\OXM_OF_VLAN_VID[],output:1 --TFH Thanks Regards, Uday -Original Message- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Thomas F Herbert Sent: Sunday, July 26, 2015 8:23 PM To: net...@vger.kernel.org; pshe...@nicira.com Cc: dev@openvswitch.org; therb...@redhat.com Subject: [ovs-dev] [PATCH net-next 3/3] openvswitch: 802.1AD: Flow handling, actions, vlan parsing and netlink attributes Add support for 802.1ad including the ability to push and pop double tagged vlans. Add support for 802.1ad to netlink parsing and flow conversion. Uses double
Re: [ovs-dev] MPLS support in OVS 2.3.2
On Fri, Jul 31, 2015 at 06:30:15PM +0530, tech_kals Kals wrote: Also, is there any plan to support TTP (Table Type Pattern) in the upcoming OVS release ? No. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 17/21] ovn: Rename Pipeline table to Rule table.
I have a lot of misgivings about all of the possible names.
Thanks.
On Thu, Jul 30, 2015 at 04:33:23PM -0700, Justin Pettit wrote:
I think Pipeline is more descriptive about what it actually is. I also find
it confusing since we use the term rule in the classifier. I think Flow
(or Logical_Flow) would be clearer than Rule, since we really are talking
about flows, and people may look for a distinction that isn't there. That,
and the fact that we use rule for a different purpose in other parts of the
tree, I think will make it more confusing.
All that said, I haven't looked ahead at the other patches yet, so maybe this
is the right choice. I'll defer to you.
Acked-by: Justin Pettit jpet...@nicira.com
--Justin
On Jul 28, 2015, at 8:44 AM, Ben Pfaff b...@nicira.com wrote:
The OVN pipeline is being split into two phases, which are most naturally
called pipelines. I kept getting very confused trying to call them
anything else, and in the end it seems to make more sense to just rename
the Pipeline table.
It would be even better to call this table Flow or Logical_Flow, but I
am worried that we already have far too many uses of the word flow.
Rule is slightly less overloaded in OVS.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ovn/TODO | 2 +-
ovn/controller/automake.mk| 6 +-
ovn/controller/ovn-controller.c | 8 +-
ovn/controller/physical.c | 2 +-
ovn/controller/{pipeline.c = rule.c} | 50 +-
ovn/controller/{pipeline.h = rule.h} | 18 ++--
ovn/lib/actions.c | 4 +-
ovn/northd/ovn-northd.c | 182
+-
ovn/ovn-architecture.7.xml| 20 ++--
ovn/ovn-nb.xml| 4 +-
ovn/ovn-sb.ovsschema | 2 +-
ovn/ovn-sb.xml| 6 +-
12 files changed, 152 insertions(+), 152 deletions(-)
rename ovn/controller/{pipeline.c = rule.c} (89%)
rename ovn/controller/{pipeline.h = rule.h} (79%)
diff --git a/ovn/TODO b/ovn/TODO
index 07d66da..19c95ca 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -48,7 +48,7 @@
Currently, clients monitor the entire contents of a table. It
might make sense to allow clients to monitor only rows that
satisfy specific criteria, e.g. to allow an ovn-controller to
-receive only Pipeline rows for logical networks on its hypervisor.
+receive only Rule rows for logical networks on its hypervisor.
*** Reducing redundant data and code within ovsdb-server.
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index 9ed6bec..55134a3 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -10,10 +10,10 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/ofctrl.h \
ovn/controller/ovn-controller.c \
ovn/controller/ovn-controller.h \
- ovn/controller/pipeline.c \
- ovn/controller/pipeline.h \
ovn/controller/physical.c \
- ovn/controller/physical.h
+ ovn/controller/physical.h \
+ ovn/controller/rule.c \
+ ovn/controller/rule.h
ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la
lib/libopenvswitch.la
man_MANS += ovn/controller/ovn-controller.8
EXTRA_DIST += ovn/controller/ovn-controller.8.xml
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index 12515c3..cfd6eb9 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -44,7 +44,7 @@
#include chassis.h
#include encaps.h
#include physical.h
-#include pipeline.h
+#include rule.h
VLOG_DEFINE_THIS_MODULE(main);
@@ -224,7 +224,7 @@ main(int argc, char *argv[])
sbrec_init();
ofctrl_init();
-pipeline_init();
+rule_init();
/* Connect to OVS OVSDB instance. We do not monitor all tables by
* default, so modules must register their interest explicitly. */
@@ -266,7 +266,7 @@ main(int argc, char *argv[])
if (br_int) {
struct hmap flow_table = HMAP_INITIALIZER(flow_table);
-pipeline_run(ctx, flow_table);
+rule_run(ctx, flow_table);
if (chassis_id) {
physical_run(ctx, br_int, chassis_id, flow_table);
}
@@ -318,7 +318,7 @@ main(int argc, char *argv[])
}
unixctl_server_destroy(unixctl);
-pipeline_destroy();
+rule_destroy();
ofctrl_destroy();
idl_loop_destroy(ovs_idl_loop);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 55d6107..2dc96ab 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -21,7 +21,7 @@
#include ofpbuf.h
#include ovn-controller.h
#include ovn/lib/ovn-sb-idl.h
-#include pipeline.h
+#include rule.h
#include simap.h
#include vswitch-idl.h
diff --git
Re: [ovs-dev] [PATCH 03/26] ofpbuf: New macro OFPBUF_STUB_INITIALIZER.
Neat!
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/ofpbuf.h | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index b30cbdb..9e82de2 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the License);
* you may not use this file except in compliance with the License.
@@ -64,6 +64,25 @@ struct ofpbuf {
enum ofpbuf_source source; /* Source of memory allocated as 'base'. */
};
+/* An initializer for a struct ofpbuf that will be initially empty and
+ * uses the space in STUB (which should be an array) as a stub.
+ *
+ * Usage example:
+ *
+ * uint64_t stub[1024 / 8]; // 1 kB stub properly aligned for 64-bit
data.
+ * struct ofpbuf ofpbuf = OFPBUF_STUB_INITIALIZER(stub);
+ */
+#define OFPBUF_STUB_INITIALIZER(STUB) { \
+.base = (STUB), \
+.data = (STUB), \
+.size = 0, \
+.allocated = sizeof (STUB), \
+.header = NULL, \
+.msg = NULL,\
+.list_node = OVS_LIST_POISON, \
+.source = OFPBUF_STUB, \
+}
+
void ofpbuf_use(struct ofpbuf *, void *, size_t);
void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 02/26] list: New macro OVS_LIST_POISON for initializing a poisoned list.
On Fri, Jul 31, 2015 at 11:18:22AM -0700, Jarno Rajahalme wrote:
One question below,
One answer below ;-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks.
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/list.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..c1b0891 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,11 @@
#include util.h
#include openvswitch/list.h
+/* Initializer for a list with pointers that will (probably) cause
segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON { (void *) (uintptr_t) 0xULL, \
+ (void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +96,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = (struct ovs_list) OVS_LIST_POISON;
Could the cast be part of the macro?
The short answer is yes. Would you like it that way, then?
For C pedants only, continue reading for pedantry below the fold.
--8--cut here--8--
First, it's not a cast. It's a compound literal, and the C standard
says, in a footnote: Note that [a compound literal] differs from a cast
expression. For example, a cast specifies a conversion to scalar types
or void only, and the result of a cast expression is not an lvalue.
Second, a compound literal isn't an initializer (except in the
degenerate sense that any expression is an initializer). The {...}
bracketed text in OVS_LIST_POISON is really an initializer. So I'd
probably update the comment above OVS_LIST_POISON at the same time, for
better pedantry.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5 5/6] ovn: Add localnet logical port type.
Introduce a new logical port type called localnet. A logical port
with this type also has an option called network_name. A localnet
logical port represents a connection to a locally accessible network.
ovn-controller will use the ovn-bridge-mappings configuration to
figure out which patch port on br-int should be used for this port.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/controller/ovn-controller.c | 3 +-
ovn/controller/physical.c | 145 +++-
ovn/controller/physical.h | 4 +-
ovn/ovn-nb.xml | 16 -
ovn/ovn-sb.xml | 30 -
5 files changed, 159 insertions(+), 39 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index f0b2626..194e8c2 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -515,7 +515,8 @@ main(int argc, char *argv[])
struct hmap flow_table = HMAP_INITIALIZER(flow_table);
pipeline_run(ctx, flow_table);
if (chassis_id) {
-physical_run(ctx, br_int, chassis_id, flow_table);
+physical_run(ctx, br_int, chassis_id, bridge_mappings,
+flow_table);
}
ofctrl_run(br_int, flow_table);
hmap_destroy(flow_table);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 9009e2b..386e4ae 100644
--- a/ovn/controller/physical.c
+++ b/ovn/controller/physical.c
@@ -22,9 +22,18 @@
#include ovn-controller.h
#include ovn/lib/ovn-sb-idl.h
#include pipeline.h
+#include shash.h
#include simap.h
+#include smap.h
#include vswitch-idl.h
+/* A register of bit flags for OVN */
+#define MFF_OVN_FLAGS MFF_REG5
+enum {
+/* Indicates that the packet came in on a localnet port */
+OVN_FLAG_LOCALNET = (1 0),
+};
+
void
physical_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
@@ -42,12 +51,25 @@ physical_register_ovs_idl(struct ovsdb_idl *ovs_idl)
ovsdb_idl_add_column(ovs_idl, ovsrec_interface_col_external_ids);
}
+static void
+init_input_match(struct match *match, ofp_port_t ofport, int tag)
+{
+match_init_catchall(match);
+match_set_in_port(match, ofport);
+if (tag) {
+match_set_dl_vlan(match, htons(tag));
+}
+}
+
void
physical_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
- const char *this_chassis_id, struct hmap *flow_table)
+ const char *this_chassis_id, struct smap *bridge_mappings,
+ struct hmap *flow_table)
{
struct simap lport_to_ofport = SIMAP_INITIALIZER(lport_to_ofport);
struct simap chassis_to_ofport = SIMAP_INITIALIZER(chassis_to_ofport);
+struct simap localnet_to_ofport = SIMAP_INITIALIZER(localnet_to_ofport);
+
for (int i = 0; i br_int-n_ports; i++) {
const struct ovsrec_port *port_rec = br_int-ports[i];
if (!strcmp(port_rec-name, br_int-name)) {
@@ -72,8 +94,17 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
continue;
}
-/* Record as chassis or local logical port. */
-if (chassis_id) {
+/* Record as patch to local net, chassis, or local logical port. */
+if (!strcmp(iface_rec-type, patch)) {
+const char *peer = smap_get(iface_rec-options, peer);
+if (!peer) {
+continue;
+}
+const char *localnet = smap_get(bridge_mappings, peer);
+if (localnet) {
+simap_put(localnet_to_ofport, localnet, ofport);
+}
+} else if (chassis_id) {
simap_put(chassis_to_ofport, chassis_id, ofport);
break;
} else {
@@ -89,6 +120,13 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
struct ofpbuf ofpacts;
ofpbuf_init(ofpacts, 0);
+struct localnet_flow {
+struct shash_node node;
+struct match match;
+struct ofpbuf ofpacts;
+};
+struct shash localnet_inputs = SHASH_INITIALIZER(localnet_inputs);
+
/* Set up flows in table 0 for physical-to-logical translation and in table
* 64 for logical-to-physical translation. */
const struct sbrec_binding *binding;
@@ -100,10 +138,15 @@ physical_run(struct controller_ctx *ctx, const struct
ovsrec_bridge *br_int,
* (and set 'local' to true). When 'parent_port' is set for a binding,
* it implies a container sitting inside a VM reachable via a 'tag'.
*/
-
int tag = 0;
ofp_port_t ofport;
-if (binding-parent_port) {
+if (!strcmp(binding-type, localnet)) {
+const char *network = smap_get(binding-options, network_name);
+if (!network) {
+continue;
+}
+ofport = u16_to_ofp(simap_get(localnet_to_ofport,
Re: [ovs-dev] [PATCH] datapath: Fix STT protocol field for sampling packet.
On Thu, Jul 30, 2015 at 8:51 PM, Pravin B Shelar pshe...@nicira.com wrote: Fixes typo in STT sampling code. Signed-off-by: Pravin B Shelar pshe...@nicira.com --- datapath/vport-stt.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Acked-by: Jesse Gross je...@nicira.com ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 18/21] actions: Allow caller to specify output table.
On Thu, Jul 30, 2015 at 04:55:13PM -0700, Justin Pettit wrote: On Jul 28, 2015, at 8:44 AM, Ben Pfaff b...@nicira.com wrote: When an upcoming commit divides the pipeline up into ingress and egress pipeline, it will become necessary to resubmit to different tables from each of those pipelines to implement output. This commit makes that possible. Signed-off-by: Ben Pfaff b...@nicira.com --- ovn/controller/rule.c | 2 +- ovn/lib/actions.c | 16 +++- ovn/lib/actions.h | 6 -- tests/test-ovn.c | 2 +- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/ovn/controller/rule.c b/ovn/controller/rule.c index 0f5971b..c7281a0 100644 --- a/ovn/controller/rule.c +++ b/ovn/controller/rule.c @@ -283,7 +283,7 @@ rule_run(struct controller_ctx *ctx, struct hmap *flow_table) ofpbuf_use_stub(ofpacts, ofpacts_stub, sizeof ofpacts_stub); next_table_id = rule-table_id 31 ? rule-table_id + 17 : 0; error = actions_parse_string(rule-actions, symtab, ldp-ports, - next_table_id, ofpacts, prereqs); + next_table_id, 64, ofpacts, prereqs); Do you think we should start using #defines or enums for these tables instead of magic numbers? It's hard to keep track of their values, and if we need to shuffle things around it's going to be tough to not miss some. That's probably true. I'd like to hold off and add that as a followup patch, though, if it's OK. Acked-by: Justin Pettit jpet...@nicira.com Thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] rtbsd: support RTM_IFANNOUNCE messages
When devices are created, they are announced using RTM_IFANNOUNCE messages using
PF_ROUTE.
Signed-off-by: Thadeu Lima de Souza Cascardo casca...@redhat.com
---
lib/rtbsd.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/lib/rtbsd.c b/lib/rtbsd.c
index 8fc88e4..33fb9fd 100644
--- a/lib/rtbsd.c
+++ b/lib/rtbsd.c
@@ -124,8 +124,15 @@ rtbsd_notifier_run(void)
if (retval = 0) {
/* received packet from PF_ROUTE socket
* XXX check for bad packets */
-if (msg.ifm_type == RTM_IFINFO) {
+switch (msg.ifm_type) {
+case RTM_IFINFO:
+/* Since RTM_IFANNOUNCE messages are smaller than RTM_IFINFO
+ * messages, the same buffer may be used. */
+case RTM_IFANNOUNCE:
rtbsd_report_change(msg);
+break;
+default:
+break;
}
} else if (errno == EAGAIN) {
ovs_mutex_unlock(rtbsd_mutex);
@@ -161,14 +168,25 @@ rtbsd_report_change(const struct if_msghdr *msg)
{
struct rtbsd_notifier *notifier;
struct rtbsd_change change;
+const struct if_announcemsghdr *ahdr;
COVERAGE_INC(rtbsd_changed);
change.msg_type = msg-ifm_type; //XXX
-change.if_index = msg-ifm_index;
-if_indextoname(msg-ifm_index, change.if_name);
change.master_ifindex = 0; //XXX
+switch (msg-ifm_type) {
+case RTM_IFINFO:
+change.if_index = msg-ifm_index;
+if_indextoname(msg-ifm_index, change.if_name);
+break;
+case RTM_IFANNOUNCE:
+ahdr = (const struct if_announcemsghdr *) msg;
+change.if_index = ahdr-ifan_index;
+strncpy(change.if_name, ahdr-ifan_name, IF_NAMESIZE);
+break;
+}
+
LIST_FOR_EACH (notifier, node, all_notifiers) {
notifier-cb(change, notifier-aux);
}
--
2.4.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5 6/6] ovn: Automatically create br-int in ovn-controller.
ovn-controller previously required the integration bridge to be
created before running ovn-controller. This patch makes
ovn-controller automatically create it if it doesn't already exist.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/controller/ovn-controller.c | 65 ++---
tutorial/ovs-sandbox| 2 --
2 files changed, 54 insertions(+), 13 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 194e8c2..cdb74c8 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -82,10 +82,52 @@ get_bridge(struct ovsdb_idl *ovs_idl, const char *br_name)
return NULL;
}
+static void
+create_br_int(struct controller_ctx *ctx,
+ const struct ovsrec_open_vswitch *cfg,
+ const char *bridge_name)
+{
+if (!ctx-ovs_idl_txn) {
+return;
+}
+
+ovsdb_idl_txn_add_comment(ctx-ovs_idl_txn,
+ovn-controller: creating integration bridge '%s', bridge_name);
+
+struct ovsrec_interface *iface;
+iface = ovsrec_interface_insert(ctx-ovs_idl_txn);
+ovsrec_interface_set_name(iface, bridge_name);
+ovsrec_interface_set_type(iface, internal);
+
+struct ovsrec_port *port;
+port = ovsrec_port_insert(ctx-ovs_idl_txn);
+ovsrec_port_set_name(port, bridge_name);
+ovsrec_port_set_interfaces(port, iface, 1);
+
+struct ovsrec_bridge *bridge;
+bridge = ovsrec_bridge_insert(ctx-ovs_idl_txn);
+ovsrec_bridge_set_name(bridge, bridge_name);
+ovsrec_bridge_set_fail_mode(bridge, secure);
+struct smap other_config = SMAP_INITIALIZER(other_config);
+smap_add(other_config, disable-in-band, true);
+ovsrec_bridge_set_other_config(bridge, other_config);
+smap_destroy(other_config);
+ovsrec_bridge_set_ports(bridge, port, 1);
+
+struct ovsrec_bridge **bridges;
+size_t bytes = sizeof *bridges * cfg-n_bridges;
+bridges = xmalloc(bytes + sizeof *bridges);
+memcpy(bridges, cfg-bridges, bytes);
+bridges[cfg-n_bridges] = bridge;
+ovsrec_open_vswitch_verify_bridges(cfg);
+ovsrec_open_vswitch_set_bridges(cfg, bridges, cfg-n_bridges + 1);
+}
+
static const struct ovsrec_bridge *
-get_br_int(struct ovsdb_idl *ovs_idl)
+get_br_int(struct controller_ctx *ctx)
{
-const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
+const struct ovsrec_open_vswitch *cfg;
+cfg = ovsrec_open_vswitch_first(ctx-ovs_idl);
if (!cfg) {
return NULL;
}
@@ -96,14 +138,11 @@ get_br_int(struct ovsdb_idl *ovs_idl)
}
const struct ovsrec_bridge *br;
-br = get_bridge(ovs_idl, br_int_name);
-if (br) {
-return br;
+br = get_bridge(ctx-ovs_idl, br_int_name);
+if (!br) {
+create_br_int(ctx, cfg, br_int_name);
}
-
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-VLOG_WARN_RL(rl, %s: integration bridge does not exist, br_int_name);
-return NULL;
+return br;
}
static const char *
@@ -463,6 +502,7 @@ main(int argc, char *argv[])
ctx.ovs_idl = ovsdb_idl_create(ovs_remote, ovsrec_idl_class, false, true);
ovsdb_idl_add_table(ctx.ovs_idl, ovsrec_table_open_vswitch);
ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_open_vswitch_col_external_ids);
+ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_open_vswitch_col_bridges);
ovsdb_idl_add_table(ctx.ovs_idl, ovsrec_table_interface);
ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_interface_col_name);
ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_interface_col_type);
@@ -473,6 +513,9 @@ main(int argc, char *argv[])
ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_port_col_external_ids);
ovsdb_idl_add_table(ctx.ovs_idl, ovsrec_table_bridge);
ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_bridge_col_ports);
+ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_bridge_col_name);
+ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_bridge_col_fail_mode);
+ovsdb_idl_add_column(ctx.ovs_idl, ovsrec_bridge_col_other_config);
chassis_register_ovs_idl(ctx.ovs_idl);
encaps_register_ovs_idl(ctx.ovs_idl);
binding_register_ovs_idl(ctx.ovs_idl);
@@ -496,7 +539,7 @@ main(int argc, char *argv[])
ctx.ovnsb_idl_txn = idl_loop_run(ovnsb_idl_loop);
ctx.ovs_idl_txn = idl_loop_run(ovs_idl_loop);
-const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl);
+const struct ovsrec_bridge *br_int = get_br_int(ctx);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
/* Map bridges to local nets from ovn-bridge-mappings */
@@ -546,7 +589,7 @@ main(int argc, char *argv[])
ctx.ovnsb_idl_txn = idl_loop_run(ovnsb_idl_loop);
ctx.ovs_idl_txn = idl_loop_run(ovs_idl_loop);
-const struct ovsrec_bridge *br_int = get_br_int(ctx.ovs_idl);
+const struct ovsrec_bridge *br_int = get_br_int(ctx);
const char *chassis_id = get_chassis_id(ctx.ovs_idl);
/* Run all of the cleanup
Re: [ovs-dev] [PATCH 04/26] ofproto-dpif-xlate: Initialize 'ctx' all in one place.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
As I see it, this has two benefits. First, by using an initializer
rather than a series of assignment statements, the reader can be
assured that everything in the structure is actually initialized.
Second, previously the initialization of 'ctx' was scattered in
a few places in this function, which made it a little harder to be
sure that any given member was not just initialized but actually
initialized before the statement that one was looking at.
It's also nice to get rid of the stub members in xlate_ctx, since
nothing outside of xlate_actions() itself needs direct access to
them. (This is pretty much necessary if we're going to use an
initializer for struct xlate_ctx, because otherwise the compiler
would initialize the whole stub, which is too expensive.)
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 77 ++--
1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b6d5d15..643bde0 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -175,7 +175,6 @@ struct xlate_ctx {
/* Stack for the push and pop actions. Each stack element is of type
* union mf_subvalue. */
-union mf_subvalue init_stack[1024 / sizeof(union mf_subvalue)];
struct ofpbuf stack;
/* The rule that we are currently translating, or NULL. */
@@ -294,7 +293,6 @@ struct xlate_ctx {
* datapath actions. */
bool action_set_has_group; /* Action set contains OFPACT_GROUP? */
struct ofpbuf action_set; /* Action set. */
-uint64_t action_set_stub[1024 / 8];
};
static void xlate_action_set(struct xlate_ctx *ctx);
@@ -4732,16 +4730,53 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
};
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, xcfgp);
+struct xbridge *xbridge = xbridge_lookup(xcfg, xin-ofproto);
+if (!xbridge) {
+return;
+}
+
struct flow_wildcards *wc = NULL;
struct flow *flow = xin-flow;
struct rule_dpif *rule = NULL;
+union mf_subvalue stack_stub[1024 / sizeof(union mf_subvalue)];
+uint64_t action_set_stub[1024 / 8];
+struct xlate_ctx ctx = {
+.xin = xin,
+.xout = xout,
+.base_flow = *flow,
+.orig_tunnel_ip_dst = flow-tunnel.ip_dst,
+.xbridge = xbridge,
+.stack = OFPBUF_STUB_INITIALIZER(stack_stub),
+.rule = xin-rule,
+
+.recurse = 0,
+.resubmits = 0,
+.in_group = false,
+.in_action_set = false,
+
+.table_id = 0,
+.rule_cookie = OVS_BE64_MAX,
+.orig_skb_priority = flow-skb_priority,
+.sflow_n_outputs = 0,
+.sflow_odp_port = 0,
+.user_cookie_offset = 0,
+.exit = false,
+
+.recirc_action_offset = -1,
+.last_unroll_offset = -1,
+
+.was_mpls = false,
+
+.action_set_has_group = false,
+.action_set = OFPBUF_STUB_INITIALIZER(action_set_stub),
+};
+memset(ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
+
enum slow_path_reason special;
const struct ofpact *ofpacts;
-struct xbridge *xbridge;
struct xport *in_port;
struct flow orig_flow;
-struct xlate_ctx ctx;
size_t ofpacts_len;
bool tnl_may_send;
bool is_icmp;
@@ -4769,9 +4804,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
* kernel does. If we wish to maintain the original values an action
* needs to be generated. */
-ctx.xin = xin;
-ctx.xout = xout;
-
xout-odp_actions = xin-odp_actions;
if (!xout-odp_actions) {
xout-odp_actions = xout-odp_actions_buf;
@@ -4780,19 +4812,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
ofpbuf_reserve(xout-odp_actions, NL_A_U32_SIZE);
-xbridge = xbridge_lookup(xcfg, xin-ofproto);
-if (!xbridge) {
-return;
-}
-/* 'ctx.xbridge' may be changed by action processing, whereas 'xbridge'
- * will remain set on the original input bridge. */
-ctx.xbridge = xbridge;
-ctx.rule = xin-rule;
-
-ctx.base_flow = *flow;
-memset(ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
-ctx.orig_tunnel_ip_dst = flow-tunnel.ip_dst;
-
if (!xin-skip_wildcards) {
wc = xout-wc;
flow_wildcards_init_catchall(wc);
@@ -4814,24 +4833,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
tnl_may_send = tnl_xlate_init(flow, wc);
-ctx.recurse = 0;
-ctx.resubmits = 0;
-ctx.in_group = false;
-ctx.in_action_set = false;
-ctx.orig_skb_priority = flow-skb_priority;
-ctx.table_id = 0;
-ctx.rule_cookie = OVS_BE64_MAX;
-ctx.exit = false;
-
[ovs-dev] [PATCH 2/2] bridge: reconfigure when system interfaces change
Whenever system interfaces are removed, added or change state, reconfigure
bridge. This allows late interfaces to be added to the datapath when they are
added to the system after ovs-vswitchd is started.
Signed-off-by: Thadeu Lima de Souza Cascardo casca...@redhat.com
---
lib/automake.mk| 7 -
lib/if-notifier-bsd.c | 69 ++
lib/if-notifier-stub.c | 38 +++
lib/if-notifier.c | 64 ++
lib/if-notifier.h | 32 +++
vswitchd/bridge.c | 24 +-
6 files changed, 232 insertions(+), 2 deletions(-)
create mode 100644 lib/if-notifier-bsd.c
create mode 100644 lib/if-notifier-stub.c
create mode 100644 lib/if-notifier.c
create mode 100644 lib/if-notifier.h
diff --git a/lib/automake.mk b/lib/automake.mk
index faca968..a7e7b9b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -292,6 +292,7 @@ lib_libopenvswitch_la_SOURCES += \
lib/getrusage-windows.c \
lib/latch-windows.c \
lib/route-table-stub.c \
+ lib/if-notifier-stub.c \
lib/strsep.c
else
lib_libopenvswitch_la_SOURCES += \
@@ -338,6 +339,8 @@ if LINUX
lib_libopenvswitch_la_SOURCES += \
lib/dpif-netlink.c \
lib/dpif-netlink.h \
+ lib/if-notifier.c \
+ lib/if-notifier.h \
lib/netdev-linux.c \
lib/netdev-linux.h \
lib/netlink-notifier.c \
@@ -379,11 +382,13 @@ endif
if ESX
lib_libopenvswitch_la_SOURCES += \
-lib/route-table-stub.c
+ lib/route-table-stub.c \
+ lib/if-notifier-stub.c
endif
if HAVE_IF_DL
lib_libopenvswitch_la_SOURCES += \
+ lib/if-notifier-bsd.c \
lib/netdev-bsd.c \
lib/rtbsd.c \
lib/rtbsd.h \
diff --git a/lib/if-notifier-bsd.c b/lib/if-notifier-bsd.c
new file mode 100644
index 000..a1c3d0e
--- /dev/null
+++ b/lib/if-notifier-bsd.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License);
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include config.h
+#include if-notifier.h
+#include rtbsd.h
+#include util.h
+
+struct if_notifier {
+struct rtbsd_notifier notifier;
+if_notify_func *cb;
+void *aux;
+};
+
+static void if_notifier_cb(const struct rtbsd_change *change OVS_UNUSED,
+ void *aux)
+{
+struct if_notifier *notifier;
+notifier = aux;
+notifier-cb(notifier-aux);
+}
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb, void *aux)
+{
+struct if_notifier *notifier;
+int ret;
+notifier = xzalloc(sizeof *notifier);
+notifier-cb = cb;
+notifier-aux = aux;
+ret = rtbsd_notifier_register(notifier-notifier, if_notifier_cb,
+ notifier);
+if (ret) {
+free(notifier);
+return NULL;
+}
+return notifier;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier)
+{
+if (notifier) {
+rtbsd_notifier_unregister(notifier-notifier);
+free(notifier);
+}
+}
+
+void if_notifier_run(void)
+{
+rtbsd_notifier_run();
+}
+
+void if_notifier_wait(void)
+{
+rtbsd_notifier_wait();
+}
diff --git a/lib/if-notifier-stub.c b/lib/if-notifier-stub.c
new file mode 100644
index 000..ffd5876
--- /dev/null
+++ b/lib/if-notifier-stub.c
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License);
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include config.h
+#include if-notifier.h
+#include stddef.h
+#include compiler.h
+
+struct if_notifier *
+if_notifier_create(if_notify_func *cb OVS_UNUSED, void *aux OVS_UNUSED)
+{
+return NULL;
+}
+
+void if_notifier_destroy(struct if_notifier *notifier OVS_UNUSED)
+{
+}
+
+void if_notifier_run(void)
+{
+}
+
+void if_notifier_wait(void)
+{
+}
diff --git a/lib/if-notifier.c b/lib/if-notifier.c
new file mode 100644
index 000..10cd292
--- /dev/null
+++ b/lib/if-notifier.c
@@ -0,0 +1,64 @@
+/*
+ *
Re: [ovs-dev] [PATCH 3/3] tests: Add basic vxlan tunnel sanity test.
On 31/07/2015 01:30, Joe Stringer joestrin...@nicira.com wrote: On 30 July 2015 at 11:37, Daniele Di Proietto diproiet...@vmware.com wrote: I get a warning in the OVS log that causes this test to fail. It appears that when br0 is removed (in OVS_KMOD_VSWITCHD_STOP) OVS gets a rtnetlink message (because br0 had an address in the routing table), but route_table_parse() fails, because if_indextoname() returns an error (the device is not there anymore). Adding AT_CHECK([ip addr del dev br0 10.1.1.100/24]) before OVS_KMOD_VSWITCHD_STOP solves the problem for me. Could you add that? What do you think? I guess that just deleting the datapath before cleaning up the OVS side is a bit aggressive. We could probably do this, or we should remove the bridge from ovs via ovs-vsctl first. I can roll that into the next version. On my system (debian Jessie) if the bridge has an address, even deleting it via ovs-vsctl triggers the warning. Also, I think that instead of creating the veth pair for the tunnel underlay manually, we could use another OVS bridge. This has two advantages: * The code is shorter * Tunnelling in userspace requires an OVS bridge as underlay. This is a summary of the changes I'm proposing: diff --git a/tests/kmod-traffic.at b/tests/kmod-traffic.at index 169078d..346d464 100644 --- a/tests/kmod-traffic.at +++ b/tests/kmod-traffic.at @@ -110,20 +110,18 @@ AT_SETUP([kmod - ping over vxlan tunnel]) AT_SKIP_IF([! ip link help 21 | grep vxlan /dev/null]) OVS_KMOD_VSWITCHD_START( - [set-fail-mode br0 standalone --]) + [set-fail-mode br0 standalone --dnl +add-br br-ovs-p0]) dnl Ensure that vport_* can be removed on exit. ON_EXIT([modprobe -r vport_vxlan]) ON_EXIT([ovs-dpctl del-dp ovs-system]) ADD_NAMESPACES(at_ns0) +ADD_VETH(p0, at_ns0, br-ovs-p0, 172.31.1.1/24) dnl Set up underlay link from host into the namespace using veth pair. -AT_CHECK([ip link add p0 type veth peer name host-p0]) -AT_CHECK([ip addr add dev host-p0 172.31.1.100/24]) -AT_CHECK([ip link set dev host-p0 up]) -AT_CHECK([ip link set p0 netns at_ns0]) -AT_CHECK([ip netns exec at_ns0 ip addr add dev p0 172.31.1.1/24]) -AT_CHECK([ip netns exec at_ns0 ip link set dev p0 up]) +AT_CHECK([ip addr add dev br-ovs-p0 172.31.1.100/24]) +AT_CHECK([ip link set dev br-ovs-p0 up]) dnl Set up remote end of tunnel inside the namespace. ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.1], [172.31.1.100], @@ -152,5 +150,8 @@ AT_CHECK([cat ping.output | grep transmitted | sed 's/time.*ms$/time 0ms/'], [ 3 packets transmitted, 3 received, 0% packet loss, time 0ms ]) +AT_CHECK([ip addr del dev br0 10.1.1.100/24]) +AT_CHECK([ip addr del dev br-ovs-p0 172.31.1.100/24]) + OVS_KMOD_VSWITCHD_STOP AT_CLEANUP I'll leave it up to you, I can also make this change in my next series. Acked-by: Daniele Di Proietto diproiet...@vmware.com This looks like a reasonable change. I'll test it out and roll it in with a v2, thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 01/26] ofproto-dpif-xlate: Initialize '*xout' all together at beginning.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To my mind, this is a good way to ensure that '*xout' gets initialized
properly in every execution. By using an initializer rather than a
series of assignment statements, we can be assured that every member
gets initialized.
This commit makes xlate_actions() more expensive because struct
xlate_out is large and this assignment will initialize all of it due to
C rules. Later commits will fix this up by removing all of the large
members, reducing xlate_out to only a few bytes total.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 125d4c6..b6d5d15 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4720,6 +4720,17 @@ too_many_output_actions(const struct ofpbuf
*odp_actions OVS_UNUSED)
void
xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
{
+*xout = (struct xlate_out) {
+.slow = 0,
+.fail_open = false,
+.has_learn = false,
+.has_normal = false,
+.has_fin_timeout = false,
+.nf_output_iface = NF_OUT_DROP,
+.mirrors = 0,
+.n_recircs = 0,
+};
+
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, xcfgp);
struct flow_wildcards *wc = NULL;
struct flow *flow = xin-flow;
@@ -4760,13 +4771,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ctx.xin = xin;
ctx.xout = xout;
-ctx.xout-slow = 0;
-ctx.xout-has_learn = false;
-ctx.xout-has_normal = false;
-ctx.xout-has_fin_timeout = false;
-ctx.xout-nf_output_iface = NF_OUT_DROP;
-ctx.xout-mirrors = 0;
-ctx.xout-n_recircs = 0;
xout-odp_actions = xin-odp_actions;
if (!xout-odp_actions) {
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 02/26] list: New macro OVS_LIST_POISON for initializing a poisoned list.
One question below,
Jarno
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/list.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..c1b0891 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,11 @@
#include util.h
#include openvswitch/list.h
+/* Initializer for a list with pointers that will (probably) cause segfaults
if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON { (void *) (uintptr_t) 0xULL, \
+ (void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +96,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = (struct ovs_list) OVS_LIST_POISON;
Could the cast be part of the macro?
}
/* Inserts 'elem' just before 'before'. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 05/26] ofproto-dpif-xlate: Make xlate_actions() caller supply flow_wildcards.
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, struct xlate_out has embedded a struct flow_wildcards, which
xlate_actions() filled in during the flow translation process (unless this
was disabled with xin-skip_wildcards to in theory save time). This commit
I have recently noticed with an upcoming test-classifier benchmark” that
computing the wildcards roughly doubles the classifier lookup time.
removes the embedded flow_wildcards and 'skip_wildcards', instead putting
a pointer to a flow_wildcards into struct xlate_in, for a caller to fill
in with a pointer to its own structure if desired.
I was concerned about the scratch wc at first, but then I noticed that no
behavior should be changed, as the xlate_out wc was previously used similarly.
One reason for this change is performance. Until now, the userspace slow
path has done a full copy of a struct flow_wildcards for each upcall in
upcall_cb(). This commit eliminates that copy. I don't know whether this
has a measurable performance impact; it may not, especially since this is
on the slow path.
Struct flow copies used to be noticeable in slow-path stress test traces even
when it was half the current size, so IMO it is good to get rid of unnecessary
init and copy operations.
This commit also eliminates a large data structure from struct xlate_out,
reducing the cost of the initialization of that structure at the beginning
of xlate_actions(). However, there is more size reduction to come in
later commits.
I was a bit baffled by the test case change. Do you know why the change was
necessary, or why the explicit mask was not needed before this patch?
Looking at the end of xlate_actions(), we should have cleared the upper bits of
imp code and type already before?
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Jarno
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-upcall.c | 44 -
ofproto/ofproto-dpif-xlate.c | 109 +-
ofproto/ofproto-dpif-xlate.h | 27 +--
ofproto/ofproto-dpif.c| 5 +-
tests/ofproto-dpif.at | 2 +-
5 files changed, 88 insertions(+), 99 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 440f9e9..6a02d60 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the License);
* you may not use this file except in compliance with the License.
@@ -170,6 +170,7 @@ struct upcall {
bool xout_initialized; /* True if 'xout' must be uninitialized. */
struct xlate_out xout; /* Result of xlate_actions(). */
+struct flow_wildcards wc; /* Dependencies that megaflow must match.
*/
struct ofpbuf put_actions; /* Actions 'put' in the fastpath. */
struct dpif_ipfix *ipfix; /* IPFIX pointer or NULL. */
@@ -250,7 +251,7 @@ static struct ovs_list all_udpifs =
OVS_LIST_INITIALIZER(all_udpifs);
static size_t recv_upcalls(struct handler *);
static int process_upcall(struct udpif *, struct upcall *,
- struct ofpbuf *odp_actions);
+ struct ofpbuf *odp_actions, struct flow_wildcards
*);
static void handle_upcalls(struct udpif *, struct upcall *, size_t n_upcalls);
static void udpif_stop_threads(struct udpif *);
static void udpif_start_threads(struct udpif *, size_t n_handlers,
@@ -278,7 +279,8 @@ static void upcall_unixctl_dump_wait(struct unixctl_conn
*conn, int argc,
static void upcall_unixctl_purge(struct unixctl_conn *conn, int argc,
const char *argv[], void *aux);
-static struct udpif_key *ukey_create_from_upcall(struct upcall *);
+static struct udpif_key *ukey_create_from_upcall(struct upcall *,
+ struct flow_wildcards *);
static int ukey_create_from_dpif_flow(const struct udpif *,
const struct dpif_flow *,
struct udpif_key **);
@@ -704,7 +706,7 @@ recv_upcalls(struct handler *handler)
pkt_metadata_from_flow(dupcall-packet.md, flow);
flow_extract(dupcall-packet, flow);
-error = process_upcall(udpif, upcall, NULL);
+error = process_upcall(udpif, upcall, NULL, upcall-wc);
if (error) {
goto cleanup;
}
@@ -943,7 +945,7 @@ upcall_receive(struct upcall *upcall, const struct
dpif_backer *backer,
static void
upcall_xlate(struct udpif *udpif, struct upcall *upcall,
- struct ofpbuf *odp_actions)
+ struct ofpbuf *odp_actions, struct flow_wildcards *wc)
{
struct dpif_flow_stats stats;
struct xlate_in xin;
@@ -954,7
Re: [ovs-dev] [PATCH 06/26] ofproto-dpif-xlate: Make xlate_actions() caller supply action buffer.
There was a missing dot in the commit message
On Jul 31, 2015, at 1:40 PM, Jarno Rajahalme jrajaha...@nicira.com wrote:
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, struct xlate_out has embedded an ofpbuf for actions and a large
stub for it, which xlate_actions() filled in during the flow translation
process This commit removes the embedded ofpbuf and stub, instead putting a
pointer to an ofpbuf into struct xlate_in, for a caller to fill in with a
pointer to its own structure if desired. (If none is supplied,
xlate_actions() uses an internal scratch buffer and destroys it before
returning.)
This commit eliminates the last large data structure from
struct xlate_out, making the initialization of an entire xlate_out at
the beginning of xlate_actions() now reasonable. More members will be
eliminated in upcoming commits, but this is no longer essential.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-upcall.c | 44 +-
ofproto/ofproto-dpif-xlate.c | 101
+-
ofproto/ofproto-dpif-xlate.h | 11 ++---
ofproto/ofproto-dpif.c| 20 ++---
4 files changed, 91 insertions(+), 85 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 6a02d60..59010c2 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -170,6 +170,7 @@ struct upcall {
bool xout_initialized; /* True if 'xout' must be uninitialized. */
struct xlate_out xout; /* Result of xlate_actions(). */
+struct ofpbuf odp_actions; /* Datapath actions from
xlate_actions(). */
struct flow_wildcards wc; /* Dependencies that megaflow must match.
*/
struct ofpbuf put_actions; /* Actions 'put' in the fastpath. */
@@ -190,6 +191,8 @@ struct upcall {
const struct nlattr *key; /* Datapath flow key. */
size_t key_len;/* Datapath flow key length. */
const struct nlattr *out_tun_key; /* Datapath output tunnel key. */
+
+uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
};
/* 'udpif_key's are responsible for tracking the little bit of state udpif
@@ -706,7 +709,8 @@ recv_upcalls(struct handler *handler)
pkt_metadata_from_flow(dupcall-packet.md, flow);
flow_extract(dupcall-packet, flow);
-error = process_upcall(udpif, upcall, NULL, upcall-wc);
+error = process_upcall(udpif, upcall,
+ upcall-odp_actions, upcall-wc);
if (error) {
goto cleanup;
}
@@ -927,6 +931,8 @@ upcall_receive(struct upcall *upcall, const struct
dpif_backer *backer,
upcall-pmd_id = pmd_id;
upcall-type = type;
upcall-userdata = userdata;
+ofpbuf_use_stub(upcall-odp_actions, upcall-odp_actions_stub,
+sizeof upcall-odp_actions_stub);
ofpbuf_init(upcall-put_actions, 0);
upcall-xout_initialized = false;
@@ -956,8 +962,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
stats.tcp_flags = ntohs(upcall-flow-tcp_flags);
xlate_in_init(xin, upcall-ofproto, upcall-flow, upcall-in_port, NULL,
- stats.tcp_flags, upcall-packet, wc);
-xin.odp_actions = odp_actions;
+ stats.tcp_flags, upcall-packet, wc, odp_actions);
if (upcall-type == DPIF_UC_MISS) {
xin.resubmit_stats = stats;
@@ -1011,8 +1016,7 @@ upcall_xlate(struct udpif *udpif, struct upcall
*upcall,
if (!upcall-xout.slow) {
ofpbuf_use_const(upcall-put_actions,
- upcall-xout.odp_actions-data,
- upcall-xout.odp_actions-size);
+ odp_actions-data, odp_actions-size);
} else {
ofpbuf_init(upcall-put_actions, 0);
compose_slow_path(udpif, upcall-xout, upcall-flow,
@@ -1035,6 +1039,7 @@ upcall_uninit(struct upcall *upcall)
if (upcall-xout_initialized) {
xlate_out_uninit(upcall-xout);
}
+ofpbuf_uninit(upcall-odp_actions);
ofpbuf_uninit(upcall-put_actions);
if (upcall-ukey) {
if (!upcall-ukey_persists) {
@@ -1234,7 +1239,7 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
* actions were composed assuming that the packet contained no
* VLAN. So, we must remove the VLAN header from the packet
before
* trying to execute the actions. */
-if (upcall-xout.odp_actions-size) {
+if (upcall-odp_actions.size) {
eth_pop_vlan(CONST_CAST(struct dp_packet *, upcall-packet));
}
@@ -1272,15 +1277,15 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
op-dop.u.flow_put.actions_len = ukey-actions-size;
}
-if (upcall-xout.odp_actions-size) {
+if
Re: [ovs-dev] [PATCH 07/26] ofproto-dpif-xlate: Calculate 'ofpacts' in more restricted scope.
Thanks.
I applied the series up to this point to master.
On Fri, Jul 31, 2015 at 01:46:58PM -0700, Jarno Rajahalme wrote:
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This moves the calculation of 'ofpacts' closer to its actual use, which
in my opinion makes the code easier to read.
This commit also expands the circumstances in which OVS omits sending
NetFlow records from those where there is exactly one OpenFlow action that
sends to controller, to those where any OpenFlow action sends to
controller. I doubt that this is a big deal.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 68
1 file changed, 31 insertions(+), 37 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eb1e17a..e39e46f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4781,10 +4781,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
enum slow_path_reason special;
-const struct ofpact *ofpacts;
struct xport *in_port;
struct flow orig_flow;
-size_t ofpacts_len;
bool tnl_may_send;
bool is_icmp;
@@ -4932,20 +4930,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
xout-fail_open = ctx.rule rule_dpif_is_fail_open(ctx.rule);
-if (xin-ofpacts) {
-ofpacts = xin-ofpacts;
-ofpacts_len = xin-ofpacts_len;
-} else if (ctx.rule) {
-const struct rule_actions *actions =
rule_dpif_get_actions(ctx.rule);
-
-ofpacts = actions-ofpacts;
-ofpacts_len = actions-ofpacts_len;
-
-ctx.rule_cookie = rule_dpif_get_flow_cookie(ctx.rule);
-} else {
-OVS_NOT_REACHED();
-}
-
if (mbridge_has_mirrors(xbridge-mbridge)) {
/* Do this conditionally because the copy is expensive enough that
it
* shows up in profiles. */
@@ -4994,6 +4978,22 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
if (tnl_may_send (!in_port || may_receive(in_port, ctx))) {
+const struct ofpact *ofpacts;
+size_t ofpacts_len;
+
+if (xin-ofpacts) {
+ofpacts = xin-ofpacts;
+ofpacts_len = xin-ofpacts_len;
+} else if (ctx.rule) {
+const struct rule_actions *actions
+= rule_dpif_get_actions(ctx.rule);
+ofpacts = actions-ofpacts;
+ofpacts_len = actions-ofpacts_len;
+ctx.rule_cookie = rule_dpif_get_flow_cookie(ctx.rule);
+} else {
+OVS_NOT_REACHED();
+}
+
do_xlate_actions(ofpacts, ofpacts_len, ctx);
/* We've let OFPP_NORMAL and the learning action look at the
@@ -5068,28 +5068,22 @@ xlate_actions(struct xlate_in *xin, struct
xlate_out *xout)
}
}
-/* Do netflow only for packets really received by the bridge. */
-if (!xin-recirc xbridge-netflow) {
-/* Only update netflow if we don't have controller flow. We don't
- * report NetFlow expiration messages for such facets because they
- * are just part of the control logic for the network, not real
- * traffic. */
-if (ofpacts_len == 0
-|| ofpacts-type != OFPACT_CONTROLLER
-|| ofpact_next(ofpacts) ofpact_end(ofpacts, ofpacts_len)) {
-if (ctx.xin-resubmit_stats) {
-netflow_flow_update(xbridge-netflow, flow,
-xout-nf_output_iface,
-ctx.xin-resubmit_stats);
-}
-if (ctx.xin-xcache) {
-struct xc_entry *entry;
+/* Do netflow only for packets really received by the bridge and not
sent
+ * to the controller. We consider packets sent to the controller to be
+ * part of the control plane rather than the data plane. */
+if (!xin-recirc xbridge-netflow !(xout-slow
SLOW_CONTROLLER)) {
+if (ctx.xin-resubmit_stats) {
+netflow_flow_update(xbridge-netflow, flow,
+xout-nf_output_iface,
+ctx.xin-resubmit_stats);
+}
+if (ctx.xin-xcache) {
+struct xc_entry *entry;
-entry = xlate_cache_add_entry(ctx.xin-xcache, XC_NETFLOW);
-entry-u.nf.netflow = netflow_ref(xbridge-netflow);
-entry-u.nf.flow = xmemdup(flow, sizeof *flow);
-entry-u.nf.iface = xout-nf_output_iface;
-}
+entry = xlate_cache_add_entry(ctx.xin-xcache,
Re: [ovs-dev] [PATCH 02/26] list: New macro OVS_LIST_POISON for initializing a poisoned list.
On Jul 31, 2015, at 11:37 AM, Ben Pfaff b...@nicira.com wrote:
On Fri, Jul 31, 2015 at 11:18:22AM -0700, Jarno Rajahalme wrote:
One question below,
One answer below ;-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks.
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/list.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..c1b0891 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,11 @@
#include util.h
#include openvswitch/list.h
+/* Initializer for a list with pointers that will (probably) cause
segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON { (void *) (uintptr_t) 0xULL, \
+ (void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +96,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = (struct ovs_list) OVS_LIST_POISON;
Could the cast be part of the macro?
The short answer is yes. Would you like it that way, then?
If the compound literal can still be used as an initializer, then yes, so that
the macro can be used by itself in both situations, as an initializer and as a
compound literal in an assignment.
For C pedants only, continue reading for pedantry below the fold.
Thanks, I’m always open to deepen my understanding of C :-)
Jarno
--8--cut here--8--
First, it's not a cast. It's a compound literal, and the C standard
says, in a footnote: Note that [a compound literal] differs from a cast
expression. For example, a cast specifies a conversion to scalar types
or void only, and the result of a cast expression is not an lvalue.
Second, a compound literal isn't an initializer (except in the
degenerate sense that any expression is an initializer). The {...}
bracketed text in OVS_LIST_POISON is really an initializer. So I'd
probably update the comment above OVS_LIST_POISON at the same time, for
better pedantry.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 07/26] ofproto-dpif-xlate: Calculate 'ofpacts' in more restricted scope.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This moves the calculation of 'ofpacts' closer to its actual use, which
in my opinion makes the code easier to read.
This commit also expands the circumstances in which OVS omits sending
NetFlow records from those where there is exactly one OpenFlow action that
sends to controller, to those where any OpenFlow action sends to
controller. I doubt that this is a big deal.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 68
1 file changed, 31 insertions(+), 37 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index eb1e17a..e39e46f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4781,10 +4781,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
enum slow_path_reason special;
-const struct ofpact *ofpacts;
struct xport *in_port;
struct flow orig_flow;
-size_t ofpacts_len;
bool tnl_may_send;
bool is_icmp;
@@ -4932,20 +4930,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
xout-fail_open = ctx.rule rule_dpif_is_fail_open(ctx.rule);
-if (xin-ofpacts) {
-ofpacts = xin-ofpacts;
-ofpacts_len = xin-ofpacts_len;
-} else if (ctx.rule) {
-const struct rule_actions *actions = rule_dpif_get_actions(ctx.rule);
-
-ofpacts = actions-ofpacts;
-ofpacts_len = actions-ofpacts_len;
-
-ctx.rule_cookie = rule_dpif_get_flow_cookie(ctx.rule);
-} else {
-OVS_NOT_REACHED();
-}
-
if (mbridge_has_mirrors(xbridge-mbridge)) {
/* Do this conditionally because the copy is expensive enough that it
* shows up in profiles. */
@@ -4994,6 +4978,22 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
if (tnl_may_send (!in_port || may_receive(in_port, ctx))) {
+const struct ofpact *ofpacts;
+size_t ofpacts_len;
+
+if (xin-ofpacts) {
+ofpacts = xin-ofpacts;
+ofpacts_len = xin-ofpacts_len;
+} else if (ctx.rule) {
+const struct rule_actions *actions
+= rule_dpif_get_actions(ctx.rule);
+ofpacts = actions-ofpacts;
+ofpacts_len = actions-ofpacts_len;
+ctx.rule_cookie = rule_dpif_get_flow_cookie(ctx.rule);
+} else {
+OVS_NOT_REACHED();
+}
+
do_xlate_actions(ofpacts, ofpacts_len, ctx);
/* We've let OFPP_NORMAL and the learning action look at the
@@ -5068,28 +5068,22 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
}
-/* Do netflow only for packets really received by the bridge. */
-if (!xin-recirc xbridge-netflow) {
-/* Only update netflow if we don't have controller flow. We don't
- * report NetFlow expiration messages for such facets because they
- * are just part of the control logic for the network, not real
- * traffic. */
-if (ofpacts_len == 0
-|| ofpacts-type != OFPACT_CONTROLLER
-|| ofpact_next(ofpacts) ofpact_end(ofpacts, ofpacts_len)) {
-if (ctx.xin-resubmit_stats) {
-netflow_flow_update(xbridge-netflow, flow,
-xout-nf_output_iface,
-ctx.xin-resubmit_stats);
-}
-if (ctx.xin-xcache) {
-struct xc_entry *entry;
+/* Do netflow only for packets really received by the bridge and not sent
+ * to the controller. We consider packets sent to the controller to be
+ * part of the control plane rather than the data plane. */
+if (!xin-recirc xbridge-netflow !(xout-slow SLOW_CONTROLLER))
{
+if (ctx.xin-resubmit_stats) {
+netflow_flow_update(xbridge-netflow, flow,
+xout-nf_output_iface,
+ctx.xin-resubmit_stats);
+}
+if (ctx.xin-xcache) {
+struct xc_entry *entry;
-entry = xlate_cache_add_entry(ctx.xin-xcache, XC_NETFLOW);
-entry-u.nf.netflow = netflow_ref(xbridge-netflow);
-entry-u.nf.flow = xmemdup(flow, sizeof *flow);
-entry-u.nf.iface = xout-nf_output_iface;
-}
+entry = xlate_cache_add_entry(ctx.xin-xcache, XC_NETFLOW);
+entry-u.nf.netflow = netflow_ref(xbridge-netflow);
+entry-u.nf.flow = xmemdup(flow, sizeof *flow);
+entry-u.nf.iface = xout-nf_output_iface;
}
}
--
2.1.3
Re: [ovs-dev] [PATCH 08/26] ofproto-dpif-xlate: Eliminate 'rule' local variable.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This variable was only used as a temporary within a small scope, so it
worked just as well to just use ctx.rule there instead.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e39e46f..1ecc1e8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4738,7 +4738,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
struct flow *flow = xin-flow;
-struct rule_dpif *rule = NULL;
union mf_subvalue stack_stub[1024 / sizeof(union mf_subvalue)];
uint64_t action_set_stub[1024 / 8];
@@ -4907,25 +4906,22 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ctx.tables_version =
ofproto_dpif_get_tables_version(ctx.xbridge-ofproto);
if (!xin-ofpacts !ctx.rule) {
-rule = rule_dpif_lookup_from_table(ctx.xbridge-ofproto,
- ctx.tables_version, flow, xin-wc,
- ctx.xin-xcache != NULL,
- ctx.xin-resubmit_stats,
- ctx.table_id,
- flow-in_port.ofp_port, true,
true);
+ctx.rule = rule_dpif_lookup_from_table(
+ctx.xbridge-ofproto, ctx.tables_version, flow, xin-wc,
+ctx.xin-xcache != NULL, ctx.xin-resubmit_stats, ctx.table_id,
+flow-in_port.ofp_port, true, true);
if (ctx.xin-resubmit_stats) {
-rule_dpif_credit_stats(rule, ctx.xin-resubmit_stats);
+rule_dpif_credit_stats(ctx.rule, ctx.xin-resubmit_stats);
}
if (ctx.xin-xcache) {
struct xc_entry *entry;
entry = xlate_cache_add_entry(ctx.xin-xcache, XC_RULE);
-entry-u.rule = rule;
+entry-u.rule = ctx.rule;
}
-ctx.rule = rule;
if (OVS_UNLIKELY(ctx.xin-resubmit_hook)) {
-ctx.xin-resubmit_hook(ctx.xin, rule, 0);
+ctx.xin-resubmit_hook(ctx.xin, ctx.rule, 0);
}
}
xout-fail_open = ctx.rule rule_dpif_is_fail_open(ctx.rule);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 02/26] list: New macro OVS_LIST_POISON for initializing a poisoned list.
On Fri, Jul 31, 2015 at 01:03:24PM -0700, Jarno Rajahalme wrote:
On Jul 31, 2015, at 11:37 AM, Ben Pfaff b...@nicira.com wrote:
On Fri, Jul 31, 2015 at 11:18:22AM -0700, Jarno Rajahalme wrote:
One question below,
One answer below ;-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks.
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/list.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..c1b0891 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,11 @@
#include util.h
#include openvswitch/list.h
+/* Initializer for a list with pointers that will (probably) cause
segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON { (void *) (uintptr_t) 0xULL, \
+ (void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +96,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = (struct ovs_list) OVS_LIST_POISON;
Could the cast be part of the macro?
The short answer is yes. Would you like it that way, then?
If the compound literal can still be used as an initializer, then yes,
so that the macro can be used by itself in both situations, as an
initializer and as a compound literal in an assignment.
It can be used both ways, so I changed this to:
--8--cut here--8--
From: Ben Pfaff b...@nicira.com
Date: Fri, 31 Jul 2015 13:14:20 -0700
Subject: [PATCH] list: New macro OVS_LIST_POISON for initializing a poisoned
list.
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
---
lib/list.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..5ad54b8 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,12 @@
#include util.h
#include openvswitch/list.h
+/* struct ovs_list with pointers that will (probably) cause segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON \
+(struct ovs_list) { (void *) (uintptr_t) 0xULL, \
+(void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +97,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = OVS_LIST_POISON;
}
/* Inserts 'elem' just before 'before'. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 02/26] list: New macro OVS_LIST_POISON for initializing a poisoned list.
Thanks!
Jarno
On Jul 31, 2015, at 1:14 PM, Ben Pfaff b...@nicira.com wrote:
On Fri, Jul 31, 2015 at 01:03:24PM -0700, Jarno Rajahalme wrote:
On Jul 31, 2015, at 11:37 AM, Ben Pfaff b...@nicira.com wrote:
On Fri, Jul 31, 2015 at 11:18:22AM -0700, Jarno Rajahalme wrote:
One question below,
One answer below ;-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks.
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/list.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..c1b0891 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,11 @@
#include util.h
#include openvswitch/list.h
+/* Initializer for a list with pointers that will (probably) cause
segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON { (void *) (uintptr_t) 0xULL, \
+ (void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +96,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = (struct ovs_list) OVS_LIST_POISON;
Could the cast be part of the macro?
The short answer is yes. Would you like it that way, then?
If the compound literal can still be used as an initializer, then yes,
so that the macro can be used by itself in both situations, as an
initializer and as a compound literal in an assignment.
It can be used both ways, so I changed this to:
--8--cut here--8--
From: Ben Pfaff b...@nicira.com mailto:b...@nicira.com
Date: Fri, 31 Jul 2015 13:14:20 -0700
Subject: [PATCH] list: New macro OVS_LIST_POISON for initializing a poisoned
list.
To be used in an upcoming commit.
Signed-off-by: Ben Pfaff b...@nicira.com mailto:b...@nicira.com
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
mailto:jrajaha...@nicira.com
---
lib/list.h | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/list.h b/lib/list.h
index 7ba1e35..5ad54b8 100644
--- a/lib/list.h
+++ b/lib/list.h
@@ -23,6 +23,12 @@
#include util.h
#include openvswitch/list.h
+/* struct ovs_list with pointers that will (probably) cause segfaults if
+ * dereferenced and, better yet, show up clearly in a debugger. */
+#define OVS_LIST_POISON \
+(struct ovs_list) { (void *) (uintptr_t) 0xULL, \
+(void *) (uintptr_t) 0xULL }
+
static inline void list_init(struct ovs_list *);
static inline void list_poison(struct ovs_list *);
@@ -91,7 +97,7 @@ list_init(struct ovs_list *list)
static inline void
list_poison(struct ovs_list *list)
{
-memset(list, 0xcc, sizeof *list);
+*list = OVS_LIST_POISON;
}
/* Inserts 'elem' just before 'before'. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 06/26] ofproto-dpif-xlate: Make xlate_actions() caller supply action buffer.
Thanks, I've fixed that now.
On Fri, Jul 31, 2015 at 01:41:42PM -0700, Jarno Rajahalme wrote:
There was a missing dot in the commit message
On Jul 31, 2015, at 1:40 PM, Jarno Rajahalme jrajaha...@nicira.com wrote:
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, struct xlate_out has embedded an ofpbuf for actions and a large
stub for it, which xlate_actions() filled in during the flow translation
process This commit removes the embedded ofpbuf and stub, instead putting a
pointer to an ofpbuf into struct xlate_in, for a caller to fill in with a
pointer to its own structure if desired. (If none is supplied,
xlate_actions() uses an internal scratch buffer and destroys it before
returning.)
This commit eliminates the last large data structure from
struct xlate_out, making the initialization of an entire xlate_out at
the beginning of xlate_actions() now reasonable. More members will be
eliminated in upcoming commits, but this is no longer essential.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-upcall.c | 44 +-
ofproto/ofproto-dpif-xlate.c | 101
+-
ofproto/ofproto-dpif-xlate.h | 11 ++---
ofproto/ofproto-dpif.c| 20 ++---
4 files changed, 91 insertions(+), 85 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 6a02d60..59010c2 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -170,6 +170,7 @@ struct upcall {
bool xout_initialized; /* True if 'xout' must be uninitialized.
*/
struct xlate_out xout; /* Result of xlate_actions(). */
+struct ofpbuf odp_actions; /* Datapath actions from
xlate_actions(). */
struct flow_wildcards wc; /* Dependencies that megaflow must
match. */
struct ofpbuf put_actions; /* Actions 'put' in the fastpath. */
@@ -190,6 +191,8 @@ struct upcall {
const struct nlattr *key; /* Datapath flow key. */
size_t key_len;/* Datapath flow key length. */
const struct nlattr *out_tun_key; /* Datapath output tunnel key. */
+
+uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
};
/* 'udpif_key's are responsible for tracking the little bit of state udpif
@@ -706,7 +709,8 @@ recv_upcalls(struct handler *handler)
pkt_metadata_from_flow(dupcall-packet.md, flow);
flow_extract(dupcall-packet, flow);
-error = process_upcall(udpif, upcall, NULL, upcall-wc);
+error = process_upcall(udpif, upcall,
+ upcall-odp_actions, upcall-wc);
if (error) {
goto cleanup;
}
@@ -927,6 +931,8 @@ upcall_receive(struct upcall *upcall, const struct
dpif_backer *backer,
upcall-pmd_id = pmd_id;
upcall-type = type;
upcall-userdata = userdata;
+ofpbuf_use_stub(upcall-odp_actions, upcall-odp_actions_stub,
+sizeof upcall-odp_actions_stub);
ofpbuf_init(upcall-put_actions, 0);
upcall-xout_initialized = false;
@@ -956,8 +962,7 @@ upcall_xlate(struct udpif *udpif, struct upcall
*upcall,
stats.tcp_flags = ntohs(upcall-flow-tcp_flags);
xlate_in_init(xin, upcall-ofproto, upcall-flow, upcall-in_port,
NULL,
- stats.tcp_flags, upcall-packet, wc);
-xin.odp_actions = odp_actions;
+ stats.tcp_flags, upcall-packet, wc, odp_actions);
if (upcall-type == DPIF_UC_MISS) {
xin.resubmit_stats = stats;
@@ -1011,8 +1016,7 @@ upcall_xlate(struct udpif *udpif, struct upcall
*upcall,
if (!upcall-xout.slow) {
ofpbuf_use_const(upcall-put_actions,
- upcall-xout.odp_actions-data,
- upcall-xout.odp_actions-size);
+ odp_actions-data, odp_actions-size);
} else {
ofpbuf_init(upcall-put_actions, 0);
compose_slow_path(udpif, upcall-xout, upcall-flow,
@@ -1035,6 +1039,7 @@ upcall_uninit(struct upcall *upcall)
if (upcall-xout_initialized) {
xlate_out_uninit(upcall-xout);
}
+ofpbuf_uninit(upcall-odp_actions);
ofpbuf_uninit(upcall-put_actions);
if (upcall-ukey) {
if (!upcall-ukey_persists) {
@@ -1234,7 +1239,7 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
* actions were composed assuming that the packet contained no
* VLAN. So, we must remove the VLAN header from the packet
before
* trying to execute the actions. */
-if (upcall-xout.odp_actions-size) {
+if (upcall-odp_actions.size) {
eth_pop_vlan(CONST_CAST(struct dp_packet *,
upcall-packet));
}
@@ -1272,15
Re: [ovs-dev] [PATCH 05/26] ofproto-dpif-xlate: Make xlate_actions() caller supply flow_wildcards.
On Fri, Jul 31, 2015 at 01:28:43PM -0700, Jarno Rajahalme wrote:
On Jul 31, 2015, at 1:06 PM, Ben Pfaff b...@nicira.com wrote:
On Fri, Jul 31, 2015 at 11:39:07AM -0700, Jarno Rajahalme wrote:
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, struct xlate_out has embedded a struct flow_wildcards, which
xlate_actions() filled in during the flow translation process (unless this
was disabled with xin-skip_wildcards to in theory save time). This
commit
I have recently noticed with an upcoming test-classifier benchmark”
that computing the wildcards roughly doubles the classifier lookup
time.
Thanks, I'll add that info to the commit message.
removes the embedded flow_wildcards and 'skip_wildcards', instead putting
a pointer to a flow_wildcards into struct xlate_in, for a caller to fill
in with a pointer to its own structure if desired.
I was concerned about the scratch wc at first, but then I noticed that
no behavior should be changed, as the xlate_out wc was previously used
similarly.
That's correct, I'll clarify that in the commit message too.
One reason for this change is performance. Until now, the userspace slow
path has done a full copy of a struct flow_wildcards for each upcall in
upcall_cb(). This commit eliminates that copy. I don't know whether this
has a measurable performance impact; it may not, especially since this is
on the slow path.
Struct flow copies used to be noticeable in slow-path stress test
traces even when it was half the current size, so IMO it is good to
get rid of unnecessary init and copy operations.
Great.
This commit also eliminates a large data structure from struct xlate_out,
reducing the cost of the initialization of that structure at the beginning
of xlate_actions(). However, there is more size reduction to come in
later commits.
I was a bit baffled by the test case change. Do you know why the
change was necessary, or why the explicit mask was not needed before
this patch?
Looking at the end of xlate_actions(), we should have cleared the
upper bits of imp code and type already before?
The test case change is due to the following. It was necessary because
xout.wc went away. Also, I couldn't figure out what the motivation for
it was, either by thinking about it or by examining the history.
I could break that change out as a separate commit, if you like. It
would make it more obvious why the test case changed.
@@ -4548,7 +4548,6 @@ trace_format_megaflow(struct ds *result, int level,
const char *title,
ds_put_char_multiple(result, '\t', level);
ds_put_format(result, %s: , title);
-flow_wildcards_or(trace-wc, trace-xout.wc, trace-wc);
match_init(match, trace-key, trace-wc);
match_format(match, result, OFP_DEFAULT_PRIORITY);
ds_put_char(result, '\n');
This seems like an attempt to accumulate the masks across resubmits, but the
xout.wc was already properly accumulated, so this is not necessary, and
actually turned out to be “harmful” after we added the clearing of the upper
bits for ICMP, which this added back.
No need to split this, but could mention this in commit message. This is a
bug fix in trace output, IMO.
Too late ;-) I broke it out as:
--8--cut here--8--
From: Ben Pfaff b...@nicira.com
Date: Fri, 31 Jul 2015 13:33:16 -0700
Subject: [PATCH] ofproto-dpif: Fix inaccurate wildcard output in
ofproto/trace.
Until now, the ofproto/trace command has tried to accumulate wildcards
independently of flow translation. This was unnecessary, and in a few
cases where flow translation drops wildcards, it meant that ofproto/trace
printed inaccurate wildcards (because it keep the wildcards that flow
translation dropped).
This updates a test case whose output is now more accurate.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif.c | 6 +-
tests/ofproto-dpif.at | 2 +-
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index d06d765..5826c9f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4464,7 +4464,6 @@ struct trace_ctx {
struct xlate_in xin;
const struct flow *key;
struct flow flow;
-struct flow_wildcards wc;
struct ds *result;
};
@@ -4548,8 +4547,7 @@ trace_format_megaflow(struct ds *result, int level, const
char *title,
ds_put_char_multiple(result, '\t', level);
ds_put_format(result, %s: , title);
-flow_wildcards_or(trace-wc, trace-xout.wc, trace-wc);
-match_init(match, trace-key, trace-wc);
+match_init(match, trace-key, trace-xout.wc);
match_format(match, result, OFP_DEFAULT_PRIORITY);
ds_put_char(result, '\n');
}
@@ -4891,8 +4889,6 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow
*flow,
flow_format(ds, flow);
ds_put_char(ds,
Re: [ovs-dev] [PATCH 05/26] ofproto-dpif-xlate: Make xlate_actions() caller supply flow_wildcards.
On Fri, Jul 31, 2015 at 11:39:07AM -0700, Jarno Rajahalme wrote: On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote: Until now, struct xlate_out has embedded a struct flow_wildcards, which xlate_actions() filled in during the flow translation process (unless this was disabled with xin-skip_wildcards to in theory save time). This commit I have recently noticed with an upcoming test-classifier benchmark” that computing the wildcards roughly doubles the classifier lookup time. Thanks, I'll add that info to the commit message. removes the embedded flow_wildcards and 'skip_wildcards', instead putting a pointer to a flow_wildcards into struct xlate_in, for a caller to fill in with a pointer to its own structure if desired. I was concerned about the scratch wc at first, but then I noticed that no behavior should be changed, as the xlate_out wc was previously used similarly. That's correct, I'll clarify that in the commit message too. One reason for this change is performance. Until now, the userspace slow path has done a full copy of a struct flow_wildcards for each upcall in upcall_cb(). This commit eliminates that copy. I don't know whether this has a measurable performance impact; it may not, especially since this is on the slow path. Struct flow copies used to be noticeable in slow-path stress test traces even when it was half the current size, so IMO it is good to get rid of unnecessary init and copy operations. Great. This commit also eliminates a large data structure from struct xlate_out, reducing the cost of the initialization of that structure at the beginning of xlate_actions(). However, there is more size reduction to come in later commits. I was a bit baffled by the test case change. Do you know why the change was necessary, or why the explicit mask was not needed before this patch? Looking at the end of xlate_actions(), we should have cleared the upper bits of imp code and type already before? The test case change is due to the following. It was necessary because xout.wc went away. Also, I couldn't figure out what the motivation for it was, either by thinking about it or by examining the history. I could break that change out as a separate commit, if you like. It would make it more obvious why the test case changed. @@ -4548,7 +4548,6 @@ trace_format_megaflow(struct ds *result, int level, const char *title, ds_put_char_multiple(result, '\t', level); ds_put_format(result, %s: , title); -flow_wildcards_or(trace-wc, trace-xout.wc, trace-wc); match_init(match, trace-key, trace-wc); match_format(match, result, OFP_DEFAULT_PRIORITY); ds_put_char(result, '\n'); Acked-by: Jarno Rajahalme jrajaha...@nicira.com Thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 05/26] ofproto-dpif-xlate: Make xlate_actions() caller supply flow_wildcards.
On Jul 31, 2015, at 1:06 PM, Ben Pfaff b...@nicira.com wrote: On Fri, Jul 31, 2015 at 11:39:07AM -0700, Jarno Rajahalme wrote: On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote: Until now, struct xlate_out has embedded a struct flow_wildcards, which xlate_actions() filled in during the flow translation process (unless this was disabled with xin-skip_wildcards to in theory save time). This commit I have recently noticed with an upcoming test-classifier benchmark” that computing the wildcards roughly doubles the classifier lookup time. Thanks, I'll add that info to the commit message. removes the embedded flow_wildcards and 'skip_wildcards', instead putting a pointer to a flow_wildcards into struct xlate_in, for a caller to fill in with a pointer to its own structure if desired. I was concerned about the scratch wc at first, but then I noticed that no behavior should be changed, as the xlate_out wc was previously used similarly. That's correct, I'll clarify that in the commit message too. One reason for this change is performance. Until now, the userspace slow path has done a full copy of a struct flow_wildcards for each upcall in upcall_cb(). This commit eliminates that copy. I don't know whether this has a measurable performance impact; it may not, especially since this is on the slow path. Struct flow copies used to be noticeable in slow-path stress test traces even when it was half the current size, so IMO it is good to get rid of unnecessary init and copy operations. Great. This commit also eliminates a large data structure from struct xlate_out, reducing the cost of the initialization of that structure at the beginning of xlate_actions(). However, there is more size reduction to come in later commits. I was a bit baffled by the test case change. Do you know why the change was necessary, or why the explicit mask was not needed before this patch? Looking at the end of xlate_actions(), we should have cleared the upper bits of imp code and type already before? The test case change is due to the following. It was necessary because xout.wc went away. Also, I couldn't figure out what the motivation for it was, either by thinking about it or by examining the history. I could break that change out as a separate commit, if you like. It would make it more obvious why the test case changed. @@ -4548,7 +4548,6 @@ trace_format_megaflow(struct ds *result, int level, const char *title, ds_put_char_multiple(result, '\t', level); ds_put_format(result, %s: , title); -flow_wildcards_or(trace-wc, trace-xout.wc, trace-wc); match_init(match, trace-key, trace-wc); match_format(match, result, OFP_DEFAULT_PRIORITY); ds_put_char(result, '\n'); This seems like an attempt to accumulate the masks across resubmits, but the xout.wc was already properly accumulated, so this is not necessary, and actually turned out to be “harmful” after we added the clearing of the upper bits for ICMP, which this added back. No need to split this, but could mention this in commit message. This is a bug fix in trace output, IMO. Jarno Acked-by: Jarno Rajahalme jrajaha...@nicira.com Thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 06/26] ofproto-dpif-xlate: Make xlate_actions() caller supply action buffer.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, struct xlate_out has embedded an ofpbuf for actions and a large
stub for it, which xlate_actions() filled in during the flow translation
process This commit removes the embedded ofpbuf and stub, instead putting a
pointer to an ofpbuf into struct xlate_in, for a caller to fill in with a
pointer to its own structure if desired. (If none is supplied,
xlate_actions() uses an internal scratch buffer and destroys it before
returning.)
This commit eliminates the last large data structure from
struct xlate_out, making the initialization of an entire xlate_out at
the beginning of xlate_actions() now reasonable. More members will be
eliminated in upcoming commits, but this is no longer essential.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-upcall.c | 44 +-
ofproto/ofproto-dpif-xlate.c | 101 +-
ofproto/ofproto-dpif-xlate.h | 11 ++---
ofproto/ofproto-dpif.c| 20 ++---
4 files changed, 91 insertions(+), 85 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 6a02d60..59010c2 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -170,6 +170,7 @@ struct upcall {
bool xout_initialized; /* True if 'xout' must be uninitialized. */
struct xlate_out xout; /* Result of xlate_actions(). */
+struct ofpbuf odp_actions; /* Datapath actions from xlate_actions().
*/
struct flow_wildcards wc; /* Dependencies that megaflow must match.
*/
struct ofpbuf put_actions; /* Actions 'put' in the fastpath. */
@@ -190,6 +191,8 @@ struct upcall {
const struct nlattr *key; /* Datapath flow key. */
size_t key_len;/* Datapath flow key length. */
const struct nlattr *out_tun_key; /* Datapath output tunnel key. */
+
+uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
};
/* 'udpif_key's are responsible for tracking the little bit of state udpif
@@ -706,7 +709,8 @@ recv_upcalls(struct handler *handler)
pkt_metadata_from_flow(dupcall-packet.md, flow);
flow_extract(dupcall-packet, flow);
-error = process_upcall(udpif, upcall, NULL, upcall-wc);
+error = process_upcall(udpif, upcall,
+ upcall-odp_actions, upcall-wc);
if (error) {
goto cleanup;
}
@@ -927,6 +931,8 @@ upcall_receive(struct upcall *upcall, const struct
dpif_backer *backer,
upcall-pmd_id = pmd_id;
upcall-type = type;
upcall-userdata = userdata;
+ofpbuf_use_stub(upcall-odp_actions, upcall-odp_actions_stub,
+sizeof upcall-odp_actions_stub);
ofpbuf_init(upcall-put_actions, 0);
upcall-xout_initialized = false;
@@ -956,8 +962,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
stats.tcp_flags = ntohs(upcall-flow-tcp_flags);
xlate_in_init(xin, upcall-ofproto, upcall-flow, upcall-in_port, NULL,
- stats.tcp_flags, upcall-packet, wc);
-xin.odp_actions = odp_actions;
+ stats.tcp_flags, upcall-packet, wc, odp_actions);
if (upcall-type == DPIF_UC_MISS) {
xin.resubmit_stats = stats;
@@ -1011,8 +1016,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
if (!upcall-xout.slow) {
ofpbuf_use_const(upcall-put_actions,
- upcall-xout.odp_actions-data,
- upcall-xout.odp_actions-size);
+ odp_actions-data, odp_actions-size);
} else {
ofpbuf_init(upcall-put_actions, 0);
compose_slow_path(udpif, upcall-xout, upcall-flow,
@@ -1035,6 +1039,7 @@ upcall_uninit(struct upcall *upcall)
if (upcall-xout_initialized) {
xlate_out_uninit(upcall-xout);
}
+ofpbuf_uninit(upcall-odp_actions);
ofpbuf_uninit(upcall-put_actions);
if (upcall-ukey) {
if (!upcall-ukey_persists) {
@@ -1234,7 +1239,7 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
* actions were composed assuming that the packet contained no
* VLAN. So, we must remove the VLAN header from the packet
before
* trying to execute the actions. */
-if (upcall-xout.odp_actions-size) {
+if (upcall-odp_actions.size) {
eth_pop_vlan(CONST_CAST(struct dp_packet *, upcall-packet));
}
@@ -1272,15 +1277,15 @@ handle_upcalls(struct udpif *udpif, struct upcall
*upcalls,
op-dop.u.flow_put.actions_len = ukey-actions-size;
}
-if (upcall-xout.odp_actions-size) {
+if (upcall-odp_actions.size) {
op = ops[n_ops++];
op-ukey = NULL;
Re: [ovs-dev] [PATCH 19/26] ofproto-dpif-xlate: Move 'nf_output_iface' from xlate_out to xlate_ctx.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This member is used internally during translation but none of the callers
used as an output of translation.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 35 ++-
ofproto/ofproto-dpif-xlate.h | 1 -
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index ac7ded7..2c9092d 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -205,6 +205,7 @@ struct xlate_ctx {
uint32_t orig_skb_priority; /* Priority when packet arrived. */
uint32_t sflow_n_outputs; /* Number of output ports. */
odp_port_t sflow_odp_port; /* Output port for composing sFlow action. */
+ofp_port_t nf_output_iface; /* Output interface index for NetFlow. */
bool exit; /* No further actions should be processed. */
mirror_mask_t mirrors; /* Bitmap of associated mirrors. */
@@ -2245,7 +2246,7 @@ xlate_normal_flood(struct xlate_ctx *ctx, struct
xbundle *in_xbundle,
output_normal(ctx, xbundle, vlan);
}
}
-ctx-xout-nf_output_iface = NF_OUT_FLOOD;
+ctx-nf_output_iface = NF_OUT_FLOOD;
}
static void
@@ -3040,7 +3041,7 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
ctx-sflow_odp_port = odp_port;
ctx-sflow_n_outputs++;
-ctx-xout-nf_output_iface = ofp_port;
+ctx-nf_output_iface = ofp_port;
}
out:
@@ -3427,7 +3428,7 @@ flood_packets(struct xlate_ctx *ctx, bool all)
}
}
-ctx-xout-nf_output_iface = NF_OUT_FLOOD;
+ctx-nf_output_iface = NF_OUT_FLOOD;
}
static void
@@ -3667,9 +3668,9 @@ static void
xlate_output_action(struct xlate_ctx *ctx,
ofp_port_t port, uint16_t max_len, bool may_packet_in)
{
-ofp_port_t prev_nf_output_iface = ctx-xout-nf_output_iface;
+ofp_port_t prev_nf_output_iface = ctx-nf_output_iface;
-ctx-xout-nf_output_iface = NF_OUT_DROP;
+ctx-nf_output_iface = NF_OUT_DROP;
switch (port) {
case OFPP_IN_PORT:
@@ -3708,12 +3709,12 @@ xlate_output_action(struct xlate_ctx *ctx,
}
if (prev_nf_output_iface == NF_OUT_FLOOD) {
-ctx-xout-nf_output_iface = NF_OUT_FLOOD;
-} else if (ctx-xout-nf_output_iface == NF_OUT_DROP) {
-ctx-xout-nf_output_iface = prev_nf_output_iface;
+ctx-nf_output_iface = NF_OUT_FLOOD;
+} else if (ctx-nf_output_iface == NF_OUT_DROP) {
+ctx-nf_output_iface = prev_nf_output_iface;
} else if (prev_nf_output_iface != NF_OUT_DROP
- ctx-xout-nf_output_iface != NF_OUT_FLOOD) {
-ctx-xout-nf_output_iface = NF_OUT_MULTI;
+ ctx-nf_output_iface != NF_OUT_FLOOD) {
+ctx-nf_output_iface = NF_OUT_MULTI;
}
}
@@ -3763,10 +3764,10 @@ xlate_enqueue_action(struct xlate_ctx *ctx,
ctx-xin-flow.skb_priority = flow_priority;
/* Update NetFlow output port. */
-if (ctx-xout-nf_output_iface == NF_OUT_DROP) {
-ctx-xout-nf_output_iface = ofp_port;
-} else if (ctx-xout-nf_output_iface != NF_OUT_FLOOD) {
-ctx-xout-nf_output_iface = NF_OUT_MULTI;
+if (ctx-nf_output_iface == NF_OUT_DROP) {
+ctx-nf_output_iface = ofp_port;
+} else if (ctx-nf_output_iface != NF_OUT_FLOOD) {
+ctx-nf_output_iface = NF_OUT_MULTI;
}
}
@@ -4715,7 +4716,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
*xout = (struct xlate_out) {
.slow = 0,
.fail_open = false,
-.nf_output_iface = NF_OUT_DROP,
.n_recircs = 0,
};
@@ -4753,6 +4753,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
.orig_skb_priority = flow-skb_priority,
.sflow_n_outputs = 0,
.sflow_odp_port = 0,
+.nf_output_iface = NF_OUT_DROP,
.exit = false,
.mirrors = 0,
@@ -5027,7 +5028,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
if (!xin-recirc xbridge-netflow !(xout-slow SLOW_CONTROLLER)) {
if (ctx.xin-resubmit_stats) {
netflow_flow_update(xbridge-netflow, flow,
-xout-nf_output_iface,
+ctx.nf_output_iface,
ctx.xin-resubmit_stats);
}
if (ctx.xin-xcache) {
@@ -5036,7 +5037,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
entry = xlate_cache_add_entry(ctx.xin-xcache, XC_NETFLOW);
entry-u.nf.netflow = netflow_ref(xbridge-netflow);
entry-u.nf.flow = xmemdup(flow, sizeof *flow);
-entry-u.nf.iface = xout-nf_output_iface;
+entry-u.nf.iface = ctx.nf_output_iface;
}
}
diff --git a/ofproto/ofproto-dpif-xlate.h
Re: [ovs-dev] [PATCH 24/26] ofp-actions: Add action debug_recirc for testing recirculation.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
It isn't otherwise useful and in fact hurts performance so it's disabled
without --enable-dummy.
An upcoming commit will make use of this.
Signed-off-by: Ben Pfaff b...@nicira.com
---
lib/dummy.c | 1 +
lib/dummy.h | 1 +
lib/ofp-actions.c| 61
lib/ofp-actions.h| 6 +
ofproto/ofproto-dpif-xlate.c | 6 +
5 files changed, 75 insertions(+)
diff --git a/lib/dummy.c b/lib/dummy.c
index ef36578..5caceee 100644
--- a/lib/dummy.c
+++ b/lib/dummy.c
@@ -46,5 +46,6 @@ dummy_enable(const char *arg)
dpif_dummy_register(level);
timeval_dummy_register();
vlandev_dummy_enable();
+ofpact_dummy_enable();
}
diff --git a/lib/dummy.h b/lib/dummy.h
index a94658b..5b8a841 100644
--- a/lib/dummy.h
+++ b/lib/dummy.h
@@ -38,5 +38,6 @@ void dpif_dummy_register(enum dummy_level);
void netdev_dummy_register(enum dummy_level);
void timeval_dummy_register(void);
void vlandev_dummy_enable(void);
+void ofpact_dummy_enable(void);
#endif /* dummy.h */
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 14a2802..ad88c6e 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -19,6 +19,7 @@
#include bundle.h
#include byte-order.h
#include compiler.h
+#include dummy.h
#include dynamic-string.h
#include hmap.h
#include learn.h
@@ -284,6 +285,16 @@ enum ofp_raw_action_type {
/* NX1.0+(34): struct nx_action_conjunction. */
NXAST_RAW_CONJUNCTION,
+
+/* ## -- ## */
+/* ## Debugging actions. ## */
+/* ## -- ## */
+
+/* These are intentionally undocumented, subject to change, and ovs-vswitchd
*/
+/* accepts them only if started with --enable-dummy. */
+
+/* NX1.0+(255): void. */
+NXAST_RAW_DEBUG_RECIRC,
};
/* OpenFlow actions are always a multiple of 8 bytes in length. */
@@ -4384,6 +4395,49 @@ format_SAMPLE(const struct ofpact_sample *a, struct ds
*s)
a-obs_domain_id, a-obs_point_id);
}
+/* debug_recirc instruction. */
+
+static bool enable_debug;
+
+void
+ofpact_dummy_enable(void)
+{
+enable_debug = true;
+}
+
+static enum ofperr
+decode_NXAST_RAW_DEBUG_RECIRC(struct ofpbuf *out)
+{
+if (!enable_debug) {
+return OFPERR_OFPBAC_BAD_VENDOR_TYPE;
+}
+
+ofpact_put_DEBUG_RECIRC(out);
+return 0;
+}
+
+static void
+encode_DEBUG_RECIRC(const struct ofpact_null *n OVS_UNUSED,
+enum ofp_version ofp_version OVS_UNUSED,
+struct ofpbuf *out)
+{
+put_NXAST_DEBUG_RECIRC(out);
+}
+
+static char * OVS_WARN_UNUSED_RESULT
+parse_DEBUG_RECIRC(char *arg OVS_UNUSED, struct ofpbuf *ofpacts,
+ enum ofputil_protocol *usable_protocols OVS_UNUSED)
+{
+ofpact_put_DEBUG_RECIRC(ofpacts);
+return NULL;
+}
+
+static void
+format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED, struct ds *s)
+{
+ds_put_cstr(s, debug_recirc);
+}
+
/* Meter instruction. */
static void
@@ -4790,6 +4844,7 @@ ofpact_is_set_or_move_action(const struct ofpact *a)
case OFPACT_STRIP_VLAN:
case OFPACT_WRITE_ACTIONS:
case OFPACT_WRITE_METADATA:
+case OFPACT_DEBUG_RECIRC:
return false;
default:
OVS_NOT_REACHED();
@@ -4850,6 +4905,7 @@ ofpact_is_allowed_in_actions_set(const struct ofpact *a)
case OFPACT_SAMPLE:
case OFPACT_STACK_POP:
case OFPACT_STACK_PUSH:
+case OFPACT_DEBUG_RECIRC:
/* The action set may only include actions and thus
* may not include any instructions */
@@ -5063,6 +5119,7 @@ ovs_instruction_type_from_ofpact_type(enum ofpact_type
type)
case OFPACT_EXIT:
case OFPACT_UNROLL_XLATE:
case OFPACT_SAMPLE:
+case OFPACT_DEBUG_RECIRC:
default:
return OVSINST_OFPIT11_APPLY_ACTIONS;
}
@@ -5659,6 +5716,9 @@ ofpact_check__(enum ofputil_protocol *usable_protocols,
struct ofpact *a,
* OpenFlow. */
return OFPERR_OFPBAC_BAD_TYPE;
+case OFPACT_DEBUG_RECIRC:
+return 0;
+
default:
OVS_NOT_REACHED();
}
@@ -6061,6 +6121,7 @@ ofpact_outputs_to_port(const struct ofpact *ofpact,
ofp_port_t port)
case OFPACT_GOTO_TABLE:
case OFPACT_METER:
case OFPACT_GROUP:
+case OFPACT_DEBUG_RECIRC:
default:
return false;
}
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 785c814..b26845d 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -107,6 +107,12 @@
OFPACT(SAMPLE, ofpact_sample, ofpact, sample) \
OFPACT(UNROLL_XLATE,ofpact_unroll_xlate, ofpact, unroll_xlate) \
\
+/* Debugging actions. \
+ *
[ovs-dev] [RFC] dpdk: support multiple queues in vhost
This RFC is based on the vhost multiple queues work on
dpdk-dev: http://dpdk.org/ml/archives/dev/2015-June/019345.html
Signed-off-by: Flavio Leitner f...@redhat.com
---
lib/netdev-dpdk.c | 61 ---
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5ae805e..493172c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -215,12 +215,9 @@ struct netdev_dpdk {
* If the numbers match, 'txq_needs_locking' is false, otherwise it is
* true and we will take a spinlock on transmission */
int real_n_txq;
+int real_n_rxq;
bool txq_needs_locking;
-/* Spinlock for vhost transmission. Other DPDK devices use spinlocks in
- * dpdk_tx_queue */
-rte_spinlock_t vhost_tx_lock;
-
/* virtio-net structure for vhost device */
OVSRCU_TYPE(struct virtio_net *) virtio_dev;
@@ -602,13 +599,10 @@ dpdk_dev_parse_name(const char dev_name[], const char
prefix[],
static int
vhost_construct_helper(struct netdev *netdev_) OVS_REQUIRES(dpdk_mutex)
{
-struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
-
if (rte_eal_init_ret) {
return rte_eal_init_ret;
}
-rte_spinlock_init(netdev-vhost_tx_lock);
return netdev_dpdk_init(netdev_, -1, DPDK_DEV_VHOST);
}
@@ -791,9 +785,16 @@ netdev_dpdk_vhost_set_multiq(struct netdev *netdev_,
unsigned int n_txq,
ovs_mutex_lock(dpdk_mutex);
ovs_mutex_lock(netdev-mutex);
+rte_free(netdev-tx_q);
+/* FIXME: the number of vqueues needs to match */
netdev-up.n_txq = n_txq;
-netdev-real_n_txq = 1;
-netdev-up.n_rxq = 1;
+netdev-up.n_rxq = n_rxq;
+
+/* vring has txq = rxq */
+netdev-real_n_txq = n_rxq;
+netdev-real_n_rxq = n_rxq;
+netdev-txq_needs_locking = netdev-real_n_txq != netdev-up.n_txq;
+netdev_dpdk_alloc_txq(netdev, netdev-up.n_txq);
ovs_mutex_unlock(netdev-mutex);
ovs_mutex_unlock(dpdk_mutex);
@@ -904,14 +905,14 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_,
struct netdev *netdev = rx-up.netdev;
struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
-int qid = 1;
+int qid = rxq_-queue_id;
uint16_t nb_rx = 0;
if (OVS_UNLIKELY(!is_vhost_running(virtio_dev))) {
return EAGAIN;
}
-nb_rx = rte_vhost_dequeue_burst(virtio_dev, qid,
+nb_rx = rte_vhost_dequeue_burst(virtio_dev, VIRTIO_TXQ + qid * 2,
vhost_dev-dpdk_mp-mp,
(struct rte_mbuf **)packets,
NETDEV_MAX_BURST);
@@ -958,8 +959,9 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct
dp_packet **packets,
}
static void
-__netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts,
- int cnt, bool may_steal)
+__netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
+ struct dp_packet **pkts, int cnt,
+ bool may_steal)
{
struct netdev_dpdk *vhost_dev = netdev_dpdk_cast(netdev);
struct virtio_net *virtio_dev = netdev_dpdk_get_virtio(vhost_dev);
@@ -974,13 +976,16 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct
dp_packet **pkts,
goto out;
}
-/* There is vHost TX single queue, So we need to lock it for TX. */
-rte_spinlock_lock(vhost_dev-vhost_tx_lock);
+if (vhost_dev-txq_needs_locking) {
+qid = qid % vhost_dev-real_n_txq;
+rte_spinlock_lock(vhost_dev-tx_q[qid].tx_lock);
+}
do {
+int vhost_qid = VIRTIO_RXQ + qid * VIRTIO_QNUM;
unsigned int tx_pkts;
-tx_pkts = rte_vhost_enqueue_burst(virtio_dev, VIRTIO_RXQ,
+tx_pkts = rte_vhost_enqueue_burst(virtio_dev, vhost_qid,
cur_pkts, cnt);
if (OVS_LIKELY(tx_pkts)) {
/* Packets have been sent.*/
@@ -999,7 +1004,7 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct
dp_packet **pkts,
* Unable to enqueue packets to vhost interface.
* Check available entries before retrying.
*/
-while (!rte_vring_available_entries(virtio_dev, VIRTIO_RXQ)) {
+while (!rte_vring_available_entries(virtio_dev, vhost_qid)) {
if (OVS_UNLIKELY((rte_get_timer_cycles() - start) timeout)) {
expired = 1;
break;
@@ -1011,7 +1016,10 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct
dp_packet **pkts,
}
}
} while (cnt);
-rte_spinlock_unlock(vhost_dev-vhost_tx_lock);
+
+if (vhost_dev-txq_needs_locking) {
+rte_spinlock_unlock(vhost_dev-tx_q[qid].tx_lock);
+}
rte_spinlock_lock(vhost_dev-stats_lock);
vhost_dev-stats.tx_packets += (total_pkts - cnt);
@@ -1116,7 +1124,7 @@ dpdk_do_tx_copy(struct
[ovs-dev] [PATCH v5 1/6] lib: Add smap_equal().
Add a method to determine of two smaps are equal (have the exact same
set of key-value pairs).
Suggested-by: Ben Pfaff b...@nicira.com
Signed-off-by: Russell Bryant rbry...@redhat.com
---
lib/smap.c | 20
lib/smap.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/lib/smap.c b/lib/smap.c
index 7fe3ce4..8865a88 100644
--- a/lib/smap.c
+++ b/lib/smap.c
@@ -302,6 +302,26 @@ smap_to_json(const struct smap *smap)
}
return json;
}
+
+/* Returns true if the two maps are equal, meaning that they have the same set
+ * of key-value pairs.
+ */
+bool
+smap_equal(const struct smap *smap1, const struct smap *smap2)
+{
+if (smap_count(smap1) != smap_count(smap2)) {
+return false;
+}
+
+const struct smap_node *node;
+SMAP_FOR_EACH (node, smap1) {
+const char *value2 = smap_get(smap2, node-key);
+if (!value2 || strcmp(node-value, value2)) {
+return false;
+}
+}
+return true;
+}
/* Private Helpers. */
diff --git a/lib/smap.h b/lib/smap.h
index caf3efc..cac3878 100644
--- a/lib/smap.h
+++ b/lib/smap.h
@@ -67,4 +67,6 @@ const struct smap_node **smap_sort(const struct smap *);
void smap_from_json(struct smap *, const struct json *);
struct json *smap_to_json(const struct smap *);
+bool smap_equal(const struct smap *, const struct smap *);
+
#endif /* smap.h */
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v5 0/6] OVN support for Neutron provider networks
v4-v5: - Fix some bugs where code assume br_int existed and that there was an active ovs_idl transaction that could be used. - Add patch for automatically creating br-int if it does not already exist. I added it to this series because it would conflict if I split it out. v3-v4: - Drop patches that either merged or are no longer necessary. - Rework patches so that there are no longer any blocking transactions. Everything is now done in the transactions committed in the main loop. Russell Bryant (6): lib: Add smap_equal(). ovn: Add bridge mappings to ovn-controller. ovn: Add type and options to logical port. ovn: Get/set lport type and options in ovn-nbctl. ovn: Add localnet logical port type. ovn: Automatically create br-int in ovn-controller. lib/smap.c | 20 +++ lib/smap.h | 2 + ovn/controller/ovn-controller.c | 306 ++-- ovn/controller/ovn-controller.h | 2 + ovn/controller/physical.c | 145 +++ ovn/controller/physical.h | 4 +- ovn/northd/ovn-northd.c | 11 ++ ovn/ovn-nb.ovsschema| 6 + ovn/ovn-nb.xml | 27 ovn/ovn-nbctl.8.xml | 24 +++- ovn/ovn-nbctl.c | 111 +++ ovn/ovn-sb.ovsschema| 6 + ovn/ovn-sb.xml | 41 ++ tutorial/ovs-sandbox| 2 - 14 files changed, 657 insertions(+), 50 deletions(-) OpenStack Neutron as an API extension called provider networks which allows an administrator to specify that it would like ports directly attached to some pre-existing network in their environment. There was a previous thread where we got into the details of this here: http://openvswitch.org/pipermail/dev/2015-June/056765.html The case where this would be used is an environment that isn't actually interested in virtual networks and just wants all of their compute resources connected up to externally managed networks. Even in this environment, OVN still has a lot of value to add. OVN implements port security and ACLs for all ports connected to these networks. OVN also provides the configuration interface and control plane to manage this across many hypervisors. Let's start from how this would be used from Neutron and go down through OVN to show how it works in OVN. Imagine an environment where every hypervisor has a NIC attached to the same physical network that you would like all of your VMs connected to. We'll refer to this physical network as physnet1. Let's also assume that the interface to physne1 is eth1 on every hypervisor. You would need to first create an OVS bridge and add eth1 to it by doing something like: $ ovs-vsctl add-br br-eth1 $ ovs-vsctl add-port br-eth1 eth1 Now you must also configure ovn-controller to tell it that it can get traffic to physnet1 by sending it to the bridge br-eth1. $ ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth1 When ovn-controller starts up, it parses the bridge mappings and automatically creates patch ports between the OVN integration bridge and the bridges specified in bridge mappings. Now that ovn-controller on every hypervisor understands what physnet1 is, you can create this network in Neutron. The following command defines a network in Neutron called provnet1 which is implemented as connecting to a physical network called physnet1. The type is set to flat meaning that the traffic is not tagged. $ neutron net-create provnet1 --shared \ --provider:physical_network physnet1 \ --provider:network_type flat (Note that the Neutron API supports specifying a VLAN tag here, but that is not yet supported in this patch series but will be added later as an addition.) At this point an OpenStack user can start creating Neutron ports for VMs to be attached to this network. $ neutron port-create provnet1 When the Neutron network is defined, nothing is actually created in OVN_Northbound. Instead, every time a Neutron port is created on this Neutron provider network, this connection is modeled as a 2-port OVN logical switch. At this point, we can model what would happen by using ovn-nbctl. Consider the following script, which sets up what Neutron would create for 2 Neutron ports connected to the same Neutron provider network. Further, it simulates an environment with 2 hypervisors, with one logical port bound to each hypervisor. ovs-vsctl add-br br-eth1 ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth1 for n in 1 2 ; do ovn-nbctl lswitch-add provnet1-$n ovn-nbctl lport-add provnet1-$n provnet1-$n-port1 ovn-nbctl lport-add provnet1-$n provnet1-$n-port1 ovn-nbctl lport-set-macs provnet1-$n-port1 00:00:00:00:00:0$n ovn-nbctl lport-set-port-security provnet1-$n-port1 00:00:00:00:00:0$n ovn-nbctl lport-add provnet1-$n provnet1-$n-physnet1 ovn-nbctl lport-set-macs
[ovs-dev] [PATCH v5 2/6] ovn: Add bridge mappings to ovn-controller.
Add a new OVN configuration entry in the Open_vSwitch database called
ovn-bridge-mappings. This allows the configuration of mappings
between a physical network name and an OVS bridge that provides
connectivity to that network.
For example, if you wanted to configure physnet1 to map to br-eth0
and physnet2 to map to br-eth1, the configuration would be:
$ ovs-vsctl set open . \
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
Patch ports between these bridges and the integration bridge are
automatically created and also removed if necessary when the
configuration changes.
Documentation for this configuration value is introduced in a later
patch that makes use of this by introducing a localnet logical port
type.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/controller/ovn-controller.c | 244 +++-
ovn/controller/ovn-controller.h | 2 +
2 files changed, 242 insertions(+), 4 deletions(-)
diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index 559cb0b..f0b2626 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -71,6 +71,18 @@ get_initial_snapshot(struct ovsdb_idl *idl)
}
static const struct ovsrec_bridge *
+get_bridge(struct ovsdb_idl *ovs_idl, const char *br_name)
+{
+const struct ovsrec_bridge *br;
+OVSREC_BRIDGE_FOR_EACH (br, ovs_idl) {
+if (!strcmp(br-name, br_name)) {
+return br;
+}
+}
+return NULL;
+}
+
+static const struct ovsrec_bridge *
get_br_int(struct ovsdb_idl *ovs_idl)
{
const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
@@ -84,10 +96,9 @@ get_br_int(struct ovsdb_idl *ovs_idl)
}
const struct ovsrec_bridge *br;
-OVSREC_BRIDGE_FOR_EACH (br, ovs_idl) {
-if (!strcmp(br-name, br_int_name)) {
-return br;
-}
+br = get_bridge(ovs_idl, br_int_name);
+if (br) {
+return br;
}
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
@@ -102,6 +113,213 @@ get_chassis_id(const struct ovsdb_idl *ovs_idl)
return cfg ? smap_get(cfg-external_ids, system-id) : NULL;
}
+/*
+ * Return true if the port is a patch port to a given bridge
+ */
+static bool
+match_patch_port(const struct ovsrec_port *port, const struct ovsrec_bridge
*to_br)
+{
+struct ovsrec_interface *iface;
+size_t i;
+
+for (i = 0; i port-n_interfaces; i++) {
+const char *peer;
+iface = port-interfaces[i];
+if (strcmp(iface-type, patch)) {
+continue;
+}
+peer = smap_get(iface-options, peer);
+if (peer !strcmp(peer, to_br-name)) {
+return true;
+}
+}
+
+return false;
+}
+
+static void
+create_patch_port(struct controller_ctx *ctx,
+ const char *network,
+ const struct ovsrec_bridge *b1,
+ const struct ovsrec_bridge *b2)
+{
+if (!ctx-ovs_idl_txn) {
+return;
+}
+
+char *port_name;
+port_name = xasprintf(patch-%s-to-%s, b1-name, b2-name);
+
+ovsdb_idl_txn_add_comment(ctx-ovs_idl_txn,
+ovn-controller: creating patch port '%s' from '%s' to '%s',
+port_name, b1-name, b2-name);
+
+struct ovsrec_interface *iface;
+iface = ovsrec_interface_insert(ctx-ovs_idl_txn);
+ovsrec_interface_set_name(iface, port_name);
+ovsrec_interface_set_type(iface, patch);
+struct smap options = SMAP_INITIALIZER(options);
+smap_add(options, peer, b2-name);
+ovsrec_interface_set_options(iface, options);
+smap_destroy(options);
+
+struct ovsrec_port *port;
+port = ovsrec_port_insert(ctx-ovs_idl_txn);
+ovsrec_port_set_name(port, port_name);
+ovsrec_port_set_interfaces(port, iface, 1);
+struct smap ext_ids = SMAP_INITIALIZER(ext_ids);
+smap_add(ext_ids, ovn-patch-port, network);
+ovsrec_port_set_external_ids(port, ext_ids);
+smap_destroy(ext_ids);
+
+struct ovsrec_port **ports;
+size_t i;
+ports = xmalloc(sizeof *port * (b1-n_ports + 1));
+for (i = 0; i b1-n_ports; i++) {
+ports[i] = b1-ports[i];
+}
+ports[i] = port;
+ovsrec_bridge_verify_ports(b1);
+ovsrec_bridge_set_ports(b1, ports, b1-n_ports + 1);
+
+free(ports);
+free(port_name);
+}
+
+static void
+create_patch_ports(struct controller_ctx *ctx,
+ const char *network,
+ struct shash *existing_ports,
+ const struct ovsrec_bridge *b1,
+ const struct ovsrec_bridge *b2)
+{
+size_t i;
+
+for (i = 0; i b1-n_ports; i++) {
+if (match_patch_port(b1-ports[i], b2)) {
+/* Patch port already exists on b1 */
+shash_find_and_delete(existing_ports, b1-ports[i]-name);
+break;
+}
+}
+if (i == b1-n_ports) {
+create_patch_port(ctx, network, b1, b2);
+}
+}
+
+static void
[ovs-dev] [PATCH v5 4/6] ovn: Get/set lport type and options in ovn-nbctl.
A recent patch added type and options columns to the Logical_Port
table in OVN_Northbound. This patch allows you to get and set those
columns with ovn-nbctl.
ovn-nbctl should eventually get converted to use the common db-ctl
code that was recently added. When that happens, these commands can
just be removed.
Signed-off-by: Russell Bryant rbry...@redhat.com
---
ovn/ovn-nbctl.8.xml | 24 ++--
ovn/ovn-nbctl.c | 111
2 files changed, 132 insertions(+), 3 deletions(-)
diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml
index 39ffb35..ba3cc82 100644
--- a/ovn/ovn-nbctl.8.xml
+++ b/ovn/ovn-nbctl.8.xml
@@ -23,9 +23,7 @@
h1Logical Switch Commands/h1
dl
- dtcodelswitch-add/code [varlswitch/var]/dt
- dd
-Creates a new logical switch named varlswitch/var. If
+ dtcodelswitch-add/code [varlswitch/var]/dt dd Creates a
new logical switch named varlswitch/var. If
varlswitch/var is not provided, the switch will not have a
name so other commands must refer to this switch by its UUID.
Initially the switch will have no ports.
@@ -192,6 +190,26 @@
or codedisabled/code.
/dd
+ dtcodelport-set-type/code varlport/var vartype/var/dt
+ dd
+Set the type for the logical port. No special types have been
implemented yet.
+ /dd
+
+ dtcodelport-get-type/code varlport/var/dt
+ dd
+Get the type for the logical port.
+ /dd
+
+ dtcodelport-set-options/code varlport/var
[varkey=value/var].../dt
+ dd
+Set type-specific key-value options for the logical port.
+ /dd
+
+ dtcodelport-get-options/code varlport/var/dt
+ dd
+Get the type-specific options for the logical port.
+ /dd
+
/dl
h1Options/h1
diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c
index 8430122..0bdb3a3 100644
--- a/ovn/ovn-nbctl.c
+++ b/ovn/ovn-nbctl.c
@@ -86,6 +86,11 @@ Logical port commands:\n\
('enabled' or 'disabled')\n\
lport-get-enabled LPORT get administrative state LPORT\n\
('enabled' or 'disabled')\n\
+ lport-set-type LPORT TYPE Set the type for LPORT\n\
+ lport-get-type LPORT Get the type for LPORT\n\
+ lport-set-options LPORT KEY=VALUE [KEY=VALUE]...\n\
+Set options related to the type of LPORT\n\
+ lport-get-options LPORT Get the type specific options for LPORT\n\
\n\
Options:\n\
--db=DATABASE connect to DATABASE\n\
@@ -627,6 +632,84 @@ do_lport_get_enabled(struct ovs_cmdl_context *ctx)
printf(%s\n,
(!lport-enabled || *lport-enabled) ? enabled : disabled);
}
+
+static void
+do_lport_set_type(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const char *type = ctx-argv[2];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+nbrec_logical_port_set_type(lport, type);
+}
+
+static void
+do_lport_get_type(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+printf(%s\n, lport-type);
+}
+
+static void
+do_lport_set_options(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+size_t i;
+struct smap options = SMAP_INITIALIZER(options);
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+for (i = 2; i ctx-argc; i++) {
+char *key, *value;
+value = xstrdup(ctx-argv[i]);
+key = strsep(value, =);
+if (value) {
+smap_add(options, key, value);
+}
+free(key);
+}
+
+nbrec_logical_port_set_options(lport, options);
+
+smap_destroy(options);
+}
+
+static void
+do_lport_get_options(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx-pvt;
+const char *id = ctx-argv[1];
+const struct nbrec_logical_port *lport;
+struct smap_node *node;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+SMAP_FOR_EACH(node, lport-options) {
+printf(%s=%s\n, node-key, node-value);
+}
+}
static void
parse_options(int argc, char *argv[])
@@ -828,6 +911,34 @@ static const struct ovs_cmdl_command all_commands[] = {
.max_args = 1,
.handler = do_lport_get_enabled,
},
+{
+.name = lport-set-type,
+.usage = LPORT TYPE,
+.min_args = 2,
+.max_args = 2,
+.handler = do_lport_set_type,
+},
+{
+.name = lport-get-type,
+.usage = LPORT,
+.min_args = 1,
+
Re: [ovs-dev] [PATCH 6/6] datapath: Add support for 4.1 kernel.
On Thu, Jul 30, 2015 at 4:30 PM, Joe Stringer joestrin...@nicira.com wrote: On 30 July 2015 at 12:24, Jesse Gross je...@nicira.com wrote: On Wed, Jul 29, 2015 at 12:53 PM, Joe Stringer joestrin...@nicira.com wrote: diff --git a/.travis.yml b/.travis.yml index 70cc14b..f3236db 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ env: - TESTSUITE=1 KERNEL=3.18.1 - TESTSUITE=1 OPTS=--enable-shared - BUILD_ENV=-m32 OPTS=--disable-ssl + - KERNEL=4.1.3 - KERNEL=4.0.2 I wonder if we should just replace 4.0.2? I don't know if we need to check every kernel version and it will increase build times. Sure, this was on my mind too. Do we have some heuristic to figure out which kernels to test? The ones that make the most sense to me would be LTS or those included in major releases of popular distros. Perhaps the existing selection of versions is more intentional than I can see, but it looks like a random scattering to me. I don't think there is much in the way of a pattern at this point. I agree with your logic - earliest and latest kernels that we support, common distribution kernels or LTS releases, anywhere where we know there was a major change, etc. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 11/26] ofproto-dpif-xlate: Move declaration of 'orig_flow' near its first use.
C99 liberties…
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 3430a57..6bad7f9 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4784,7 +4784,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
struct xport *in_port;
-struct flow orig_flow;
bool tnl_may_send;
COVERAGE_INC(xlate_actions);
@@ -4927,6 +4926,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
xout-fail_open = ctx.rule rule_dpif_is_fail_open(ctx.rule);
+struct flow orig_flow;
if (mbridge_has_mirrors(xbridge-mbridge)) {
/* Do this conditionally because the copy is expensive enough that it
* shows up in profiles. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 16/26] ofproto-dpif-xlate: Set up 'base_flow' when we initialize 'ctx'.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
The initialization of 'base_flow' was previously split into a few pieces,
and I think it's easier to understand if it's all in one place.
This also moves and rewrites the comment describing 'base_flow'. I think
that the perspective of the new comment is a little more useful.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 50 +++-
1 file changed, 22 insertions(+), 28 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7726a40..8a208a4 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4770,7 +4770,29 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
.action_set_has_group = false,
.action_set = OFPBUF_STUB_INITIALIZER(action_set_stub),
};
+
+/* 'base_flow' reflects the packet as it came in, but we need it to
reflect
+ * the packet as the datapath will treat it for output actions:
+ *
+ * - Our datapath doesn't retain tunneling information without us
+ * re-setting it, so clear the tunnel data.
+ *
+ * - For VLAN splinters, a higher layer may pretend that the packet
+ * came in on 'flow-in_port.ofp_port' with 'flow-vlan_tci'
+ * attached, because that's how we want to treat it from an
OpenFlow
+ * perspective. But from the datapath's perspective it actually
came
+ * in on a VLAN device without any VLAN attached. So here we put
the
+ * datapath's view of the VLAN information in 'base_flow' to ensure
+ * correct treatment.
+ */
memset(ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
+if (flow-in_port.ofp_port
+!= vsp_realdev_to_vlandev(xbridge-ofproto,
+ flow-in_port.ofp_port,
+ flow-vlan_tci)) {
+ctx.base_flow.vlan_tci = 0;
+}
+
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
if (xin-wc) {
xlate_wc_init(ctx);
@@ -4780,27 +4802,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
COVERAGE_INC(xlate_actions);
-/* Flow initialization rules:
- * - 'base_flow' must match the kernel's view of the packet at the
- * time that action processing starts. 'flow' represents any
- * transformations we wish to make through actions.
- * - By default 'base_flow' and 'flow' are the same since the input
- * packet matches the output before any actions are applied.
- * - When using VLAN splinters, 'base_flow''s VLAN is set to the value
- * of the received packet as seen by the kernel. If we later output
- * to another device without any modifications this will cause us to
- * insert a new tag since the original one was stripped off by the
- * VLAN device.
- * - Tunnel metadata as received is retained in 'flow'. This allows
- * tunnel metadata matching also in later tables.
- * Since a kernel action for setting the tunnel metadata will only be
- * generated with actual tunnel output, changing the tunnel metadata
- * values in 'flow' (such as tun_id) will only have effect with a later
- * tunnel output action.
- * - Tunnel 'base_flow' is completely cleared since that is what the
- * kernel does. If we wish to maintain the original values an action
- * needs to be generated. */
-
/* The in_port of the original packet before recirculation. */
in_port = get_ofp_port(xbridge, flow-in_port.ofp_port);
@@ -4926,13 +4927,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Do not perform special processing on recirculated packets,
* as recirculated packets are not really received by the bridge. */
if (xin-recirc || !process_special(ctx, in_port)) {
-if (flow-in_port.ofp_port
-!= vsp_realdev_to_vlandev(xbridge-ofproto,
- flow-in_port.ofp_port,
- flow-vlan_tci)) {
-ctx.base_flow.vlan_tci = 0;
-}
-
/* Sampling is done only for packets really received by the bridge. */
unsigned int user_cookie_offset = 0;
if (!xin-recirc) {
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 17/26] ofproto-dpif-xlate: Move 'mirrors' from xlate_out to xlate_ctx.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Nothing outside of ofproto-dpif-xlate.c referenced this member.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 21 +++--
ofproto/ofproto-dpif-xlate.h | 1 -
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8a208a4..2a91cfc 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -206,6 +206,7 @@ struct xlate_ctx {
uint32_t sflow_n_outputs; /* Number of output ports. */
odp_port_t sflow_odp_port; /* Output port for composing sFlow action. */
bool exit; /* No further actions should be processed. */
+mirror_mask_t mirrors; /* Bitmap of associated mirrors. */
/* These are used for non-bond recirculation. The recirculation IDs are
* stored in xout and must be associated with a datapath flow (ukey),
@@ -1536,8 +1537,8 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct
flow *orig_flow)
uint16_t vlan;
uint16_t vid;
-mirrors = ctx-xout-mirrors;
-ctx-xout-mirrors = 0;
+mirrors = ctx-mirrors;
+ctx-mirrors = 0;
in_xbundle = lookup_input_bundle(xbridge, orig_flow-in_port.ofp_port,
ctx-xin-packet != NULL, NULL);
@@ -1595,7 +1596,7 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct
flow *orig_flow)
}
mirrors = ~dup_mirrors;
-ctx-xout-mirrors |= dup_mirrors;
+ctx-mirrors |= dup_mirrors;
if (out) {
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, xcfgp);
struct xbundle *out_xbundle = xbundle_lookup(xcfg, out);
@@ -2825,8 +2826,8 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
}
if (mbridge_has_mirrors(ctx-xbridge-mbridge) xport-xbundle) {
-ctx-xout-mirrors |= xbundle_mirror_dst(xport-xbundle-xbridge,
- xport-xbundle);
+ctx-mirrors |= xbundle_mirror_dst(xport-xbundle-xbridge,
+ xport-xbundle);
}
if (xport-peer) {
@@ -2871,10 +2872,10 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
* the learning action look at the packet, then drop it. */
struct flow old_base_flow = ctx-base_flow;
size_t old_size = ctx-odp_actions-size;
-mirror_mask_t old_mirrors = ctx-xout-mirrors;
+mirror_mask_t old_mirrors = ctx-mirrors;
xlate_table_action(ctx, flow-in_port.ofp_port, 0, true,
true);
-ctx-xout-mirrors = old_mirrors;
+ctx-mirrors = old_mirrors;
ctx-base_flow = old_base_flow;
ctx-odp_actions-size = old_size;
@@ -4722,7 +4723,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
.has_normal = false,
.has_fin_timeout = false,
.nf_output_iface = NF_OUT_DROP,
-.mirrors = 0,
.n_recircs = 0,
};
@@ -4761,6 +4761,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
.sflow_n_outputs = 0,
.sflow_odp_port = 0,
.exit = false,
+.mirrors = 0,
.recirc_action_offset = -1,
.last_unroll_offset = -1,
@@ -5014,7 +5015,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Update mirror stats only for packets really received by the bridge. */
if (!xin-recirc mbridge_has_mirrors(xbridge-mbridge)) {
if (ctx.xin-resubmit_stats) {
-mirror_update_stats(xbridge-mbridge, xout-mirrors,
+mirror_update_stats(xbridge-mbridge, ctx.mirrors,
ctx.xin-resubmit_stats-n_packets,
ctx.xin-resubmit_stats-n_bytes);
}
@@ -5023,7 +5024,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
entry = xlate_cache_add_entry(ctx.xin-xcache, XC_MIRROR);
entry-u.mirror.mbridge = mbridge_ref(xbridge-mbridge);
-entry-u.mirror.mirrors = xout-mirrors;
+entry-u.mirror.mirrors = ctx.mirrors;
}
}
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 9c9d2f2..346c735 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -44,7 +44,6 @@ struct xlate_out {
bool has_normal;/* Actions output to OFPP_NORMAL? */
bool has_fin_timeout; /* Actions include NXAST_FIN_TIMEOUT? */
ofp_port_t nf_output_iface; /* Output interface index for NetFlow. */
-mirror_mask_t mirrors; /* Bitmap of associated mirrors. */
/* Recirculation IDs on which references are held. */
unsigned n_recircs;
--
2.1.3
Re: [ovs-dev] [PATCH 18/26] ofproto-dpif-xlate: Remove multiple members from struct xlate_out.
Must have been historical reasons :-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Nothing used them.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 7 ---
ofproto/ofproto-dpif-xlate.h | 3 ---
2 files changed, 10 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2a91cfc..ac7ded7 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2260,8 +2260,6 @@ xlate_normal(struct xlate_ctx *ctx)
uint16_t vlan;
uint16_t vid;
-ctx-xout-has_normal = true;
-
memset(wc-masks.dl_src, 0xff, sizeof wc-masks.dl_src);
memset(wc-masks.dl_dst, 0xff, sizeof wc-masks.dl_dst);
wc-masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
@@ -3835,7 +3833,6 @@ xlate_learn_action__(struct xlate_ctx *ctx, const
struct ofpact_learn *learn,
static void
xlate_learn_action(struct xlate_ctx *ctx, const struct ofpact_learn *learn)
{
-ctx-xout-has_learn = true;
learn_mask(learn, ctx-wc);
if (ctx-xin-xcache) {
@@ -4423,7 +4420,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t
ofpacts_len,
case OFPACT_FIN_TIMEOUT:
CHECK_MPLS_RECIRCULATION();
memset(wc-masks.nw_proto, 0xff, sizeof wc-masks.nw_proto);
-ctx-xout-has_fin_timeout = true;
xlate_fin_timeout(ctx, ofpact_get_FIN_TIMEOUT(a));
break;
@@ -4719,9 +4715,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
*xout = (struct xlate_out) {
.slow = 0,
.fail_open = false,
-.has_learn = false,
-.has_normal = false,
-.has_fin_timeout = false,
.nf_output_iface = NF_OUT_DROP,
.n_recircs = 0,
};
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 346c735..6dd990a 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -40,9 +40,6 @@ struct xlate_cache;
struct xlate_out {
enum slow_path_reason slow; /* 0 if fast path may be used. */
bool fail_open; /* Initial rule is fail open? */
-bool has_learn; /* Actions include NXAST_LEARN? */
-bool has_normal;/* Actions output to OFPP_NORMAL? */
-bool has_fin_timeout; /* Actions include NXAST_FIN_TIMEOUT? */
ofp_port_t nf_output_iface; /* Output interface index for NetFlow. */
/* Recirculation IDs on which references are held. */
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 12/26] ofproto-dpif-xlate: Simplify 'sample_actions_len' calculation.
On Fri, Jul 31, 2015 at 02:03:24PM -0700, Jarno Rajahalme wrote:
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
It's always the size of 'odp_actions' following adding the sample actions.
Is this a bug fix?
No, there should be no visible change. I'll note that in the commit
message.
Jarno
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 6bad7f9..e69605e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4953,8 +4953,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Do not perform special processing on recirculated packets,
* as recirculated packets are not really received by the bridge. */
if (xin-recirc || !process_special(ctx, in_port)) {
-size_t sample_actions_len;
-
if (flow-in_port.ofp_port
!= vsp_realdev_to_vlandev(xbridge-ofproto,
flow-in_port.ofp_port,
@@ -4966,10 +4964,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
if (!xin-recirc) {
add_sflow_action(ctx);
add_ipfix_action(ctx);
-sample_actions_len = ctx.odp_actions-size;
-} else {
-sample_actions_len = 0;
}
+size_t sample_actions_len = ctx.odp_actions-size;
if (tnl_may_send (!in_port || may_receive(in_port, ctx))) {
const struct ofpact *ofpacts;
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 15/26] ofproto-dpif-xlate: Clean up sFlow and IPFIX sampling code.
On Fri, Jul 31, 2015 at 02:25:22PM -0700, Jarno Rajahalme wrote:
I’m starting to dig the designated initializers, too :-)
My favorite use of them is the following example C99 standard. I
haven't actually used this style myself yet, though:
EXAMPLE 3 Initializers with designations can be combined with compound
literals. Structure objects created using compound literals can be
passed to functions without depending on member order:
drawline((struct point){.x=1, .y=1},
(struct point){.x=3, .y=4});
Or, if drawline instead expected pointers to struct point:
drawline((struct point){.x=1, .y=1},
(struct point){.x=3, .y=4});
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks!
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This code was a twisty maze of tiny functions, but what it actually needed
to do was simple. This makes it look that simple.
Among more stylistic changes, this removes 'user_cookie_offset' from
xlate_ctx. This member was used to communicate between two sections of
code that are both in xlate_actions() and close together, so it's better to
simply use a local variable than to put it into a shared context structure.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 255
---
1 file changed, 97 insertions(+), 158 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 03bca1b..7726a40 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -205,7 +205,6 @@ struct xlate_ctx {
uint32_t orig_skb_priority; /* Priority when packet arrived. */
uint32_t sflow_n_outputs; /* Number of output ports. */
odp_port_t sflow_odp_port; /* Output port for composing sFlow action.
*/
-uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
bool exit; /* No further actions should be processed.
*/
/* These are used for non-bond recirculation. The recirculation IDs are
@@ -2440,211 +2439,147 @@ xlate_normal(struct xlate_ctx *ctx)
}
}
-/* Compose SAMPLE action for sFlow or IPFIX. The given probability is
- * the number of packets out of UINT32_MAX to sample. The given
- * cookie is passed back in the callback for each sampled packet.
+/* Appends a sample action for sFlow or IPFIX to 'ctx-odp_actions'. The
+ * 'probability' is the number of packets out of UINT32_MAX to sample. The
+ * 'cookie' (of length 'cookie_size' bytes) is passed back in the callback
for
+ * each sampled packet. 'tunnel_out_port', if not ODPP_NONE, is added as
the
+ * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions', an
+ * OVS_USERSPACE_ATTR_ACTIONS attribute is added.
*/
static size_t
-compose_sample_action(const struct xbridge *xbridge,
- struct ofpbuf *odp_actions,
- const struct flow *flow,
+compose_sample_action(struct xlate_ctx *ctx,
const uint32_t probability,
const union user_action_cookie *cookie,
const size_t cookie_size,
const odp_port_t tunnel_out_port,
bool include_actions)
{
-size_t sample_offset, actions_offset;
-odp_port_t odp_port;
-int cookie_offset;
-uint32_t pid;
+size_t sample_offset = nl_msg_start_nested(ctx-odp_actions,
+ OVS_ACTION_ATTR_SAMPLE);
-sample_offset = nl_msg_start_nested(odp_actions,
OVS_ACTION_ATTR_SAMPLE);
+nl_msg_put_u32(ctx-odp_actions, OVS_SAMPLE_ATTR_PROBABILITY,
probability);
-nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability);
+size_t actions_offset = nl_msg_start_nested(ctx-odp_actions,
+OVS_SAMPLE_ATTR_ACTIONS);
-actions_offset = nl_msg_start_nested(odp_actions,
OVS_SAMPLE_ATTR_ACTIONS);
+odp_port_t odp_port = ofp_port_to_odp_port(
+ctx-xbridge, ctx-xin-flow.in_port.ofp_port);
+uint32_t pid = dpif_port_get_pid(ctx-xbridge-dpif, odp_port,
+ flow_hash_5tuple(ctx-xin-flow, 0));
+int cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size,
+ tunnel_out_port,
+ include_actions,
+ ctx-odp_actions);
-odp_port = ofp_port_to_odp_port(xbridge, flow-in_port.ofp_port);
-pid = dpif_port_get_pid(xbridge-dpif, odp_port,
-flow_hash_5tuple(flow, 0));
-cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size,
- tunnel_out_port,
-
Re: [ovs-dev] [PATCH 21/26] ofproto-dpif-xlate: Drop packets received from mirror output ports earlier.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Packets should never be received on mirror output ports. We drop them
when we do receive them. But by putting them through the processing that
we did until now, we made it possible for MAC learning, etc. to happen
based on these packets. This commit drops them earlier to prevent that.
Found by inspection.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index be0fd13..8c8da9a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1548,18 +1548,6 @@ add_mirror_actions(struct xlate_ctx *ctx, const struct
flow *orig_flow)
}
mirrors |= xbundle_mirror_src(xbridge, in_xbundle);
-/* Drop frames on bundles reserved for mirroring. */
-if (xbundle_mirror_out(xbridge, in_xbundle)) {
-if (ctx-xin-packet != NULL) {
-static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-VLOG_WARN_RL(rl, bridge %s: dropping packet received on port
- %s, which is reserved exclusively for mirroring,
- ctx-xbridge-name, in_xbundle-name);
-}
-ofpbuf_clear(ctx-odp_actions);
-return;
-}
-
/* Check VLAN. */
vid = vlan_tci_to_vid(orig_flow-vlan_tci);
if (!input_vid_is_valid(vid, in_xbundle, ctx-xin-packet != NULL)) {
@@ -4919,9 +4907,20 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
}
-/* Do not perform special processing on recirculated packets,
- * as recirculated packets are not really received by the bridge. */
-if (xin-recirc || !process_special(ctx, in_port)) {
+if (!xin-recirc process_special(ctx, in_port)) {
+/* process_special() did all the processing for this packet.
+ *
+ * We do not perform special processing on recirculated packets, as
+ * recirculated packets are not really received by the bridge.*/
+} else if (in_port in_port-xbundle
+xbundle_mirror_out(xbridge, in_port-xbundle)) {
+if (ctx.xin-packet != NULL) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(rl, bridge %s: dropping packet received on port
+ %s, which is reserved exclusively for mirroring,
+ ctx.xbridge-name, in_port-xbundle-name);
+}
+} else {
/* Sampling is done only for packets really received by the bridge. */
unsigned int user_cookie_offset = 0;
if (!xin-recirc) {
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 22/26] ofproto-dpif-xlate: Rewrite mirroring to better fit flow translation.
Always learn something new when reviewing code ;-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Until now, mirroring has been implemented by accumulating, across the whole
translation process, a set of mirrors that should receive a mirrored
packet. After translation was complete, mirroring restored the original
version of the packet and sent that version to the mirrors.
That implementation was ugly for multiple reasons. First, it means that
we have to keep a copy of the original packet (or its headers, actually),
which is expensive. Second, it doesn't really make sense to mirror a
version of a packet that is different from the one originally output.
Third, it interacted with recirculation; mirroring needed to happen only
after recirculation was complete, but this was never properly implemented,
so that (I think) mirroring never happened for packets that were
recirculated.
This commit changes how mirroring works. Now, a packet is mirrored at the
point in translation when it becomes eligible for it: for mirrors based on
ingress port, this is at ingress; for mirrors based on egress port, this
is at egress. (Duplicates are dropped.) Mirroring happens on the version
of the packet as it exists when it becomes eligible. Finally, since
mirroring happens immediately, it interacts better with recirculation
(it still isn't perfect, since duplicate mirroring will occur if a packet
is eligible for mirroring both before and after recirculation; this is
not difficult to fix and an upcoming commit later in this series will do so).
Finally, this commit removes more code from xlate_actions() than it adds,
which in my opinion makes it easier to understand.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 113 +++
tests/ofproto-dpif.at| 12 ++---
vswitchd/vswitch.xml | 26 ++
3 files changed, 82 insertions(+), 69 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8c8da9a..cbcd2a3 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1530,56 +1530,55 @@ lookup_input_bundle(const struct xbridge *xbridge,
ofp_port_t in_port,
}
static void
-add_mirror_actions(struct xlate_ctx *ctx, const struct flow *orig_flow)
+mirror_packet(struct xlate_ctx *ctx, struct xbundle *xbundle,
+ mirror_mask_t mirrors)
{
-const struct xbridge *xbridge = ctx-xbridge;
-mirror_mask_t mirrors;
-struct xbundle *in_xbundle;
-uint16_t vlan;
-uint16_t vid;
-
-mirrors = ctx-mirrors;
-ctx-mirrors = 0;
-
-in_xbundle = lookup_input_bundle(xbridge, orig_flow-in_port.ofp_port,
- ctx-xin-packet != NULL, NULL);
-if (!in_xbundle) {
+bool warn = ctx-xin-packet != NULL;
+uint16_t vid = vlan_tci_to_vid(ctx-xin-flow.vlan_tci);
+if (!input_vid_is_valid(vid, xbundle, warn)) {
return;
}
-mirrors |= xbundle_mirror_src(xbridge, in_xbundle);
+uint16_t vlan = input_vid_to_vlan(xbundle, vid);
-/* Check VLAN. */
-vid = vlan_tci_to_vid(orig_flow-vlan_tci);
-if (!input_vid_is_valid(vid, in_xbundle, ctx-xin-packet != NULL)) {
-return;
-}
-vlan = input_vid_to_vlan(in_xbundle, vid);
+const struct xbridge *xbridge = ctx-xbridge;
+/* Don't mirror to destinations that we've already mirrored to. */
+mirrors = ~ctx-mirrors;
if (!mirrors) {
return;
}
-/* Restore the original packet before adding the mirror actions. */
-ctx-xin-flow = *orig_flow;
+/* Record these mirrors so that we don't mirror to them again. */
+ctx-mirrors |= mirrors;
+
+if (ctx-xin-resubmit_stats) {
+mirror_update_stats(xbridge-mbridge, mirrors,
+ctx-xin-resubmit_stats-n_packets,
+ctx-xin-resubmit_stats-n_bytes);
+}
+if (ctx-xin-xcache) {
+struct xc_entry *entry;
+
+entry = xlate_cache_add_entry(ctx-xin-xcache, XC_MIRROR);
+entry-u.mirror.mbridge = mbridge_ref(xbridge-mbridge);
+entry-u.mirror.mirrors = mirrors;
+}
while (mirrors) {
+const unsigned long *vlans;
mirror_mask_t dup_mirrors;
struct ofbundle *out;
-const unsigned long *vlans;
-bool vlan_mirrored;
-bool has_mirror;
int out_vlan;
-has_mirror = mirror_get(xbridge-mbridge, raw_ctz(mirrors),
-vlans, dup_mirrors, out, out_vlan);
+bool has_mirror = mirror_get(xbridge-mbridge, raw_ctz(mirrors),
+ vlans, dup_mirrors, out, out_vlan);
ovs_assert(has_mirror);
if (vlans) {
ctx-wc-masks.vlan_tci |= htons(VLAN_CFI | VLAN_VID_MASK);
}
-
Re: [ovs-dev] [PATCH 23/26] ofproto-dpif-rid: Factor recirculation state out as new structure.
It’s almost like having named function arguments in C!
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This greatly reduces the number of arguments to many of the functions
involved in recirculation, which to my eye makes the code clearer. It
will also make it easier to add new recirculation state in an upcoming
commit.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-rid.c | 162 ---
ofproto/ofproto-dpif-rid.h | 43 +++-
ofproto/ofproto-dpif-xlate.c | 57 ---
3 files changed, 117 insertions(+), 145 deletions(-)
diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index f1b3bdc..e2867b7 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -124,62 +124,52 @@ recirc_id_node_find(uint32_t id)
}
static uint32_t
-recirc_metadata_hash(struct ofproto_dpif *ofproto, uint8_t table_id,
- struct recirc_metadata *md, struct ofpbuf *stack,
- uint32_t action_set_len, uint32_t ofpacts_len,
- const struct ofpact *ofpacts)
+recirc_metadata_hash(const struct recirc_state *state)
{
uint32_t hash;
-BUILD_ASSERT(OFPACT_ALIGNTO == sizeof(uint64_t));
-
-hash = hash_pointer(ofproto, 0);
-hash = hash_int(table_id, hash);
-hash = hash_words64((const uint64_t *)md, sizeof *md / sizeof(uint64_t),
+hash = hash_pointer(state-ofproto, 0);
+hash = hash_int(state-table_id, hash);
+hash = hash_words64((const uint64_t *) state-metadata,
+sizeof state-metadata / sizeof(uint64_t),
hash);
-if (stack stack-size != 0) {
-hash = hash_words64((const uint64_t *)stack-data,
-stack-size / sizeof(uint64_t), hash);
+if (state-stack state-stack-size != 0) {
+hash = hash_words64((const uint64_t *) state-stack-data,
+state-stack-size / sizeof(uint64_t), hash);
}
-hash = hash_int(action_set_len, hash);
-if (ofpacts_len) {
-hash = hash_words64(ALIGNED_CAST(const uint64_t *, ofpacts),
-OFPACT_ALIGN(ofpacts_len) / sizeof(uint64_t),
+hash = hash_int(state-action_set_len, hash);
+if (state-ofpacts_len) {
+hash = hash_words64(ALIGNED_CAST(const uint64_t *, state-ofpacts),
+state-ofpacts_len / sizeof(uint64_t),
hash);
}
return hash;
}
static bool
-recirc_metadata_equal(const struct recirc_id_node *node,
- struct ofproto_dpif *ofproto, uint8_t table_id,
- struct recirc_metadata *md, struct ofpbuf *stack,
- uint32_t action_set_len, uint32_t ofpacts_len,
- const struct ofpact *ofpacts)
+recirc_metadata_equal(const struct recirc_state *a,
+ const struct recirc_state *b)
{
-return node-ofproto == ofproto
- node-table_id == table_id
- !memcmp(node-metadata, md, sizeof *md)
- ((!node-stack (!stack || stack-size == 0))
-|| (node-stack stack ofpbuf_equal(node-stack, stack)))
- node-action_set_len == action_set_len
- node-ofpacts_len == ofpacts_len
- (ofpacts_len == 0 || !memcmp(node-ofpacts, ofpacts,
ofpacts_len));
+return (a-table_id == b-table_id
+ a-ofproto == b-ofproto
+ !memcmp(a-metadata, b-metadata, sizeof a-metadata)
+ (((!a-stack || !a-stack-size)
+ (!b-stack || !b-stack-size))
+|| (a-stack b-stack ofpbuf_equal(a-stack,
b-stack)))
+ a-action_set_len == b-action_set_len
+ ofpacts_equal(a-ofpacts, a-ofpacts_len,
+ b-ofpacts, b-ofpacts_len));
}
/* Lockless RCU protected lookup. If node is needed accross RCU quiescent
* state, caller should take a reference. */
static struct recirc_id_node *
-recirc_find_equal(struct ofproto_dpif *ofproto, uint8_t table_id,
- struct recirc_metadata *md, struct ofpbuf *stack,
- uint32_t action_set_len, uint32_t ofpacts_len,
- const struct ofpact *ofpacts, uint32_t hash)
+recirc_find_equal(const struct recirc_state *target, uint32_t hash)
{
struct recirc_id_node *node;
-CMAP_FOR_EACH_WITH_HASH(node, metadata_node, hash, metadata_map) {
-if (recirc_metadata_equal(node, ofproto, table_id, md, stack,
- action_set_len, ofpacts_len, ofpacts)) {
+CMAP_FOR_EACH_WITH_HASH (node, metadata_node, hash, metadata_map) {
+if (recirc_metadata_equal(node-state, target)) {
return node;
}
}
@@ -187,16 +177,12 @@ recirc_find_equal(struct ofproto_dpif *ofproto, uint8_t
table_id,
}
Re: [ovs-dev] [PATCH 26/26] ofproto-dpif-xlate: Fix mirroring interaction with recirculation.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
Thanks for the clean-up, will be easier to maintain this code going forward.
Appreciate the C lessons as well :-)
Jarno
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
Before this commit, mirroring state was not preserved across recirculation,
which could result in a packet being mirrored to the same destination both
before and after recirculation. This commit fixes the problem and adds a
test to avoid regression.
Found by inspection.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-rid.c | 2 ++
ofproto/ofproto-dpif-rid.h | 2 ++
ofproto/ofproto-dpif-xlate.c | 4
tests/ofproto-dpif.at| 28
4 files changed, 36 insertions(+)
diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index e2867b7..b8207c4 100644
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -137,6 +137,7 @@ recirc_metadata_hash(const struct recirc_state *state)
hash = hash_words64((const uint64_t *) state-stack-data,
state-stack-size / sizeof(uint64_t), hash);
}
+hash = hash_int(state-mirrors, hash);
hash = hash_int(state-action_set_len, hash);
if (state-ofpacts_len) {
hash = hash_words64(ALIGNED_CAST(const uint64_t *, state-ofpacts),
@@ -156,6 +157,7 @@ recirc_metadata_equal(const struct recirc_state *a,
(((!a-stack || !a-stack-size)
(!b-stack || !b-stack-size))
|| (a-stack b-stack ofpbuf_equal(a-stack, b-stack)))
+ a-mirrors == b-mirrors
a-action_set_len == b-action_set_len
ofpacts_equal(a-ofpacts, a-ofpacts_len,
b-ofpacts, b-ofpacts_len));
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 9caa662..11ae486 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -23,6 +23,7 @@
#include cmap.h
#include list.h
#include ofp-actions.h
+#include ofproto-dpif-mirror.h
#include ovs-thread.h
struct ofproto_dpif;
@@ -135,6 +136,7 @@ struct recirc_state {
struct ofproto_dpif *ofproto; /* Post-recirculation bridge. */
struct recirc_metadata metadata; /* Flow metadata. */
struct ofpbuf *stack; /* Stack if any. */
+mirror_mask_t mirrors;/* Mirrors already output. */
/* Actions to be translated on recirculation. */
uint32_t action_set_len; /* How much of 'ofpacts' consists of an
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index dc16a39..e2596d9 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3522,6 +3522,7 @@ compose_recirculate_action(struct xlate_ctx *ctx)
.ofproto = ctx-xbridge-ofproto,
.metadata = md,
.stack = ctx-stack,
+.mirrors = ctx-mirrors,
.action_set_len = ctx-recirc_action_offset,
.ofpacts_len = ctx-action_set.size,
.ofpacts = ctx-action_set.data,
@@ -4868,6 +4869,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
ofpbuf_put(ctx.stack, state-stack-data, state-stack-size);
}
+/* Restore mirror state. */
+ctx.mirrors = state-mirrors;
+
/* Restore action set, if any. */
if (state-action_set_len) {
const struct ofpact *a;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 9bfb794..58c426b 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -4144,6 +4144,34 @@ AT_CHECK([ovs-dpctl normalize-actions $flow
$actual], [0], [expout])
OVS_VSWITCHD_STOP
AT_CLEANUP
+# This test verifies that mirror state is preserved across recirculation.
+#
+# Otherwise, post-recirculation the ingress and the output to port 4
+# would cause the packet to be mirrored to port 3 a second time.
+AT_SETUP([ofproto-dpif - mirroring with recirculation])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], 1, 2, 3, 4)
+ovs-vsctl \
+set Bridge br0 mirrors=@m --\
+--id=@p3 get Port p3 --\
+--id=@m create Mirror name=mymirror select_all=true output_port=@p3
+
+AT_DATA([flows.txt], [dnl
+in_port=1 actions=2,debug_recirc,4
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+flow=in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy $flow -generate], [0],
[stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: 3,2,recirc(0x1)
+])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy $flow,recirc_id(1)
-generate], [0], [stdout])
+AT_CHECK_UNQUOTED([tail -1 stdout], [0], [Datapath actions: 4
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
# Two testcases below are for the ofproto/trace command
# The first one tests all correct syntax:
#
Re: [ovs-dev] [PATCH v2 17/21] ovn: Rename Pipeline table to Rule table.
Just to mention that I also think Flow is a better name for this table.
On Fri, Jul 31, 2015 at 11:19 AM, Ben Pfaff b...@nicira.com wrote:
I have a lot of misgivings about all of the possible names.
Thanks.
On Thu, Jul 30, 2015 at 04:33:23PM -0700, Justin Pettit wrote:
I think Pipeline is more descriptive about what it actually is. I also
find it confusing since we use the term rule in the classifier. I think
Flow (or Logical_Flow) would be clearer than Rule, since we really are
talking about flows, and people may look for a distinction that isn't
there. That, and the fact that we use rule for a different purpose in
other parts of the tree, I think will make it more confusing.
All that said, I haven't looked ahead at the other patches yet, so maybe
this is the right choice. I'll defer to you.
Acked-by: Justin Pettit jpet...@nicira.com
--Justin
On Jul 28, 2015, at 8:44 AM, Ben Pfaff b...@nicira.com wrote:
The OVN pipeline is being split into two phases, which are most
naturally
called pipelines. I kept getting very confused trying to call them
anything else, and in the end it seems to make more sense to just
rename
the Pipeline table.
It would be even better to call this table Flow or Logical_Flow, but I
am worried that we already have far too many uses of the word flow.
Rule is slightly less overloaded in OVS.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ovn/TODO | 2 +-
ovn/controller/automake.mk| 6 +-
ovn/controller/ovn-controller.c | 8 +-
ovn/controller/physical.c | 2 +-
ovn/controller/{pipeline.c = rule.c} | 50 +-
ovn/controller/{pipeline.h = rule.h} | 18 ++--
ovn/lib/actions.c | 4 +-
ovn/northd/ovn-northd.c | 182
+-
ovn/ovn-architecture.7.xml| 20 ++--
ovn/ovn-nb.xml| 4 +-
ovn/ovn-sb.ovsschema | 2 +-
ovn/ovn-sb.xml| 6 +-
12 files changed, 152 insertions(+), 152 deletions(-)
rename ovn/controller/{pipeline.c = rule.c} (89%)
rename ovn/controller/{pipeline.h = rule.h} (79%)
diff --git a/ovn/TODO b/ovn/TODO
index 07d66da..19c95ca 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -48,7 +48,7 @@
Currently, clients monitor the entire contents of a table. It
might make sense to allow clients to monitor only rows that
satisfy specific criteria, e.g. to allow an ovn-controller to
-receive only Pipeline rows for logical networks on its hypervisor.
+receive only Rule rows for logical networks on its hypervisor.
*** Reducing redundant data and code within ovsdb-server.
diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk
index 9ed6bec..55134a3 100644
--- a/ovn/controller/automake.mk
+++ b/ovn/controller/automake.mk
@@ -10,10 +10,10 @@ ovn_controller_ovn_controller_SOURCES = \
ovn/controller/ofctrl.h \
ovn/controller/ovn-controller.c \
ovn/controller/ovn-controller.h \
- ovn/controller/pipeline.c \
- ovn/controller/pipeline.h \
ovn/controller/physical.c \
- ovn/controller/physical.h
+ ovn/controller/physical.h \
+ ovn/controller/rule.c \
+ ovn/controller/rule.h
ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/
libopenvswitch.la
man_MANS += ovn/controller/ovn-controller.8
EXTRA_DIST += ovn/controller/ovn-controller.8.xml
diff --git a/ovn/controller/ovn-controller.c
b/ovn/controller/ovn-controller.c
index 12515c3..cfd6eb9 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -44,7 +44,7 @@
#include chassis.h
#include encaps.h
#include physical.h
-#include pipeline.h
+#include rule.h
VLOG_DEFINE_THIS_MODULE(main);
@@ -224,7 +224,7 @@ main(int argc, char *argv[])
sbrec_init();
ofctrl_init();
-pipeline_init();
+rule_init();
/* Connect to OVS OVSDB instance. We do not monitor all tables by
* default, so modules must register their interest explicitly. */
@@ -266,7 +266,7 @@ main(int argc, char *argv[])
if (br_int) {
struct hmap flow_table = HMAP_INITIALIZER(flow_table);
-pipeline_run(ctx, flow_table);
+rule_run(ctx, flow_table);
if (chassis_id) {
physical_run(ctx, br_int, chassis_id, flow_table);
}
@@ -318,7 +318,7 @@ main(int argc, char *argv[])
}
unixctl_server_destroy(unixctl);
-pipeline_destroy();
+rule_destroy();
ofctrl_destroy();
idl_loop_destroy(ovs_idl_loop);
diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
index 55d6107..2dc96ab 100644
--- a/ovn/controller/physical.c
+++
Re: [ovs-dev] [PATCH 09/26] ofproto-dpif-xlate: Simplify invocation of process_special().
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This takes advantage of common properties of the invocation of this
function in both callers (both supply the same 'flow' and 'packet',
although they write it differently) and avoids the need for a local
variable in each place.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1ecc1e8..017ed06 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2647,20 +2647,22 @@ fix_sflow_action(struct xlate_ctx *ctx)
ctx-sflow_odp_port, ctx-sflow_n_outputs, cookie);
}
-static enum slow_path_reason
-process_special(struct xlate_ctx *ctx, const struct flow *flow,
-const struct xport *xport, const struct dp_packet *packet)
+static bool
+process_special(struct xlate_ctx *ctx, const struct xport *xport)
{
+const struct flow *flow = ctx-xin-flow;
struct flow_wildcards *wc = ctx-wc;
const struct xbridge *xbridge = ctx-xbridge;
+const struct dp_packet *packet = ctx-xin-packet;
+enum slow_path_reason slow;
if (!xport) {
-return 0;
+slow = 0;
} else if (xport-cfm cfm_should_process_flow(xport-cfm, flow, wc)) {
if (packet) {
cfm_process_heartbeat(xport-cfm, packet);
}
-return SLOW_CFM;
+slow = SLOW_CFM;
} else if (xport-bfd bfd_should_process_flow(xport-bfd, flow, wc)) {
if (packet) {
bfd_process_packet(xport-bfd, flow, packet);
@@ -2669,13 +2671,13 @@ process_special(struct xlate_ctx *ctx, const struct
flow *flow,
ofproto_dpif_monitor_port_send_soon(xport-ofport);
}
}
-return SLOW_BFD;
+slow = SLOW_BFD;
} else if (xport-xbundle xport-xbundle-lacp
flow-dl_type == htons(ETH_TYPE_LACP)) {
if (packet) {
lacp_process_packet(xport-xbundle-lacp, xport-ofport, packet);
}
-return SLOW_LACP;
+slow = SLOW_LACP;
} else if ((xbridge-stp || xbridge-rstp)
stp_should_process_flow(flow, wc)) {
if (packet) {
@@ -2683,14 +2685,21 @@ process_special(struct xlate_ctx *ctx, const struct
flow *flow,
? stp_process_packet(xport, packet)
: rstp_process_packet(xport, packet);
}
-return SLOW_STP;
+slow = SLOW_STP;
} else if (xport-lldp lldp_should_process_flow(xport-lldp, flow)) {
if (packet) {
lldp_process_packet(xport-lldp, packet);
}
-return SLOW_LLDP;
+slow = SLOW_LLDP;
} else {
-return 0;
+slow = 0;
+}
+
+if (slow) {
+ctx-xout-slow |= slow;
+return true;
+} else {
+return false;
}
}
@@ -2890,7 +2899,6 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
struct flow old_flow = ctx-xin-flow;
bool old_was_mpls = ctx-was_mpls;
cls_version_t old_version = ctx-tables_version;
-enum slow_path_reason special;
struct ofpbuf old_stack = ctx-stack;
union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)];
struct ofpbuf old_action_set = ctx-action_set;
@@ -2909,11 +2917,7 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
ctx-tables_version
= ofproto_dpif_get_tables_version(ctx-xbridge-ofproto);
-special = process_special(ctx, ctx-xin-flow, peer,
- ctx-xin-packet);
-if (special) {
-ctx-xout-slow |= special;
-} else if (may_receive(peer, ctx)) {
+if (!process_special(ctx, peer) may_receive(peer, ctx)) {
if (xport_stp_forward_state(peer)
xport_rstp_forward_state(peer)) {
xlate_table_action(ctx, flow-in_port.ofp_port, 0, true,
true);
if (ctx-action_set.size) {
@@ -4779,7 +4783,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
memset(ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
-enum slow_path_reason special;
struct xport *in_port;
struct flow orig_flow;
bool tnl_may_send;
@@ -4951,10 +4954,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Do not perform special processing on recirculated packets,
* as recirculated packets are not really received by the bridge. */
-if (!xin-recirc
-(special = process_special(ctx, flow, in_port, ctx.xin-packet))) {
-ctx.xout-slow |= special;
-} else {
+if (xin-recirc || !process_special(ctx, in_port)) {
Re: [ovs-dev] [PATCH 10/26] ofproto-dpif-xlate: Eliminate 'is_icmp' from xlate_actions().
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This is only used in one place and action processing can't change the
result, so only calculate it where it's needed.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 017ed06..3430a57 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4786,7 +4786,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
struct xport *in_port;
struct flow orig_flow;
bool tnl_may_send;
-bool is_icmp;
COVERAGE_INC(xlate_actions);
@@ -4827,7 +4826,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
netflow_mask_wc(flow, ctx.wc);
}
}
-is_icmp = is_icmpv4(flow) || is_icmpv6(flow);
tnl_may_send = tnl_xlate_init(flow, xin-wc);
@@ -5098,7 +5096,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
* Avoid the problem here by making sure that only the low 8 bits of
* either field can be unwildcarded for ICMP.
*/
-if (is_icmp) {
+if (is_icmpv4(flow) || is_icmpv6(flow)) {
ctx.wc-masks.tp_src = htons(UINT8_MAX);
ctx.wc-masks.tp_dst = htons(UINT8_MAX);
}
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 15/26] ofproto-dpif-xlate: Clean up sFlow and IPFIX sampling code.
I’m starting to dig the designated initializers, too :-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This code was a twisty maze of tiny functions, but what it actually needed
to do was simple. This makes it look that simple.
Among more stylistic changes, this removes 'user_cookie_offset' from
xlate_ctx. This member was used to communicate between two sections of
code that are both in xlate_actions() and close together, so it's better to
simply use a local variable than to put it into a shared context structure.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 255 ---
1 file changed, 97 insertions(+), 158 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 03bca1b..7726a40 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -205,7 +205,6 @@ struct xlate_ctx {
uint32_t orig_skb_priority; /* Priority when packet arrived. */
uint32_t sflow_n_outputs; /* Number of output ports. */
odp_port_t sflow_odp_port; /* Output port for composing sFlow action. */
-uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */
bool exit; /* No further actions should be processed. */
/* These are used for non-bond recirculation. The recirculation IDs are
@@ -2440,211 +2439,147 @@ xlate_normal(struct xlate_ctx *ctx)
}
}
-/* Compose SAMPLE action for sFlow or IPFIX. The given probability is
- * the number of packets out of UINT32_MAX to sample. The given
- * cookie is passed back in the callback for each sampled packet.
+/* Appends a sample action for sFlow or IPFIX to 'ctx-odp_actions'. The
+ * 'probability' is the number of packets out of UINT32_MAX to sample. The
+ * 'cookie' (of length 'cookie_size' bytes) is passed back in the callback
for
+ * each sampled packet. 'tunnel_out_port', if not ODPP_NONE, is added as the
+ * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions', an
+ * OVS_USERSPACE_ATTR_ACTIONS attribute is added.
*/
static size_t
-compose_sample_action(const struct xbridge *xbridge,
- struct ofpbuf *odp_actions,
- const struct flow *flow,
+compose_sample_action(struct xlate_ctx *ctx,
const uint32_t probability,
const union user_action_cookie *cookie,
const size_t cookie_size,
const odp_port_t tunnel_out_port,
bool include_actions)
{
-size_t sample_offset, actions_offset;
-odp_port_t odp_port;
-int cookie_offset;
-uint32_t pid;
+size_t sample_offset = nl_msg_start_nested(ctx-odp_actions,
+ OVS_ACTION_ATTR_SAMPLE);
-sample_offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SAMPLE);
+nl_msg_put_u32(ctx-odp_actions, OVS_SAMPLE_ATTR_PROBABILITY,
probability);
-nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability);
+size_t actions_offset = nl_msg_start_nested(ctx-odp_actions,
+OVS_SAMPLE_ATTR_ACTIONS);
-actions_offset = nl_msg_start_nested(odp_actions,
OVS_SAMPLE_ATTR_ACTIONS);
+odp_port_t odp_port = ofp_port_to_odp_port(
+ctx-xbridge, ctx-xin-flow.in_port.ofp_port);
+uint32_t pid = dpif_port_get_pid(ctx-xbridge-dpif, odp_port,
+ flow_hash_5tuple(ctx-xin-flow, 0));
+int cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size,
+ tunnel_out_port,
+ include_actions,
+ ctx-odp_actions);
-odp_port = ofp_port_to_odp_port(xbridge, flow-in_port.ofp_port);
-pid = dpif_port_get_pid(xbridge-dpif, odp_port,
-flow_hash_5tuple(flow, 0));
-cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size,
- tunnel_out_port,
- include_actions,
- odp_actions);
+nl_msg_end_nested(ctx-odp_actions, actions_offset);
+nl_msg_end_nested(ctx-odp_actions, sample_offset);
-nl_msg_end_nested(odp_actions, actions_offset);
-nl_msg_end_nested(odp_actions, sample_offset);
return cookie_offset;
}
-static void
-compose_sflow_cookie(const struct xbridge *xbridge, ovs_be16 vlan_tci,
- odp_port_t odp_port, unsigned int n_outputs,
- union user_action_cookie *cookie)
-{
-int ifindex;
-
-cookie-type = USER_ACTION_COOKIE_SFLOW;
-cookie-sflow.vlan_tci = vlan_tci;
-
-/* See
Re: [ovs-dev] [PATCH 20/26] ofproto-dpif-xlate: Move initialization of 'in_port' closer to first use.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This seems to be a little clearer to me.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2c9092d..be0fd13 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4793,13 +4793,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
xlate_wc_init(ctx);
}
-struct xport *in_port;
-
COVERAGE_INC(xlate_actions);
-/* The in_port of the original packet before recirculation. */
-in_port = get_ofp_port(xbridge, flow-in_port.ofp_port);
-
if (xin-recirc) {
const struct recirc_id_node *recirc = xin-recirc;
@@ -4902,6 +4897,11 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
orig_flow = *flow;
}
+/* Get the proximate input port of the packet. (If xin-recirc,
+ * flow-in_port is the ultimate input port of the packet.) */
+struct xport *in_port = get_ofp_port(xbridge,
+ ctx.base_flow.in_port.ofp_port);
+
/* Tunnel stats only for non-recirculated packets. */
if (!xin-recirc in_port in_port-is_tunnel) {
if (ctx.xin-resubmit_stats) {
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 20/21] ofctrl: Negotiate OVN Geneve option.
On Jul 28, 2015, at 8:44 AM, Ben Pfaff b...@nicira.com wrote:
+static void
+recv_S_GENEVE_TABLE_MOD_SENT(const struct ofp_header *oh, enum ofptype type)
+{
+if (oh-xid != xid oh-xid != xid2) {
+ofctrl_recv(oh, type);
+} else if (oh-xid == xid2 type == OFPTYPE_BARRIER_REPLY) {
+state = S_CLEAR_FLOWS;
+} else if (oh-xid == xid type == OFPTYPE_ERROR) {
+enum ofperr error = ofperr_decode_msg(oh, NULL);
+if (error == OFPERR_NXGTMFC_ALREADY_MAPPED ||
+error == OFPERR_NXGTMFC_DUP_ENTRY) {
+VLOG_INFO(raced with another controller adding
+ Geneve option (%s); trying again,
+ ofperr_to_string(error));
+state = S_NEW;
I think in this case, we'll still get the barrier message which may log an
error in ofctrl_recv(). I think we may just want to ignore barriers in
ofctrl_recv().
+/* S_UPDATE_FLOWS, for maintaining the flow table over time.
+ *
+ * Compare the installed flows to the ones we want. Send OFPT_FLOW_MOD as
+ * necessary.
+ *
+ * This is a terminal state. We only transition out of it if the connection
+ * drops. */
+
+static void
+run_S_UPDATE_FLOWS(void)
+{
+}
+
+
+static void
+recv_S_UPDATE_FLOWS(const struct ofp_header *oh, enum ofptype type)
+{
+ofctrl_recv(oh, type);
+}
The comment makes it sound like this state itself is maintaining these flows as
opposed to it happening by calls to ofctrl_put(). It might be useful to
clarify that in the comment.
@@ -377,26 +586,13 @@ queue_flow_mod(struct ofputil_flow_mod *fm)
queue_msg(ofputil_encode_flow_mod(fm, OFPUTIL_P_OF13_OXM));
}
-static void
-ofctrl_update_flows(struct hmap *desired_flows)
+void
+ofctrl_put(struct hmap *flow_table)
I think it might be nice to add a comment to this function--especially that it
will take care of removing all the members from flow_table.
+if (state != S_UPDATE_FLOWS
+|| rconn_packet_counter_n_packets(tx_counter)) {
+ovn_flow_table_clear(flow_table);
+return;
}
It might be nice to explain why we're bail out if there are any packets queued
in the rconn.
if (br_int) {
+enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int);
+
struct hmap flow_table = HMAP_INITIALIZER(flow_table);
rule_run(ctx, flow_table);
-if (chassis_id) {
physical_run(ctx, br_int, chassis_id, flow_table);
+if (chassis_id mff_ovn_geneve) {
Won't this prevent OVN from coming up properly if Geneve isn't a supported
protocol on the host or negotiation fails? It would be nice to still come up
if STT is configured and there are any Geneve problems.
Less importantly, do we want to bother with all the Geneve negotiation if only
STT is needed on the host?
diff --git a/ovn/controller/physical.h b/ovn/controller/physical.h
index 9de76de..82baa2f 100644
--- a/ovn/controller/physical.h
+++ b/ovn/controller/physical.h
@@ -29,6 +29,13 @@ struct hmap;
struct ovsdb_idl;
struct ovsrec_bridge;
+/* OVN Geneve option information.
+ *
+ * Keep these in sync with the documentation in ovn-architecture(7). */
+#define OVN_GENEVE_CLASS 0x
+#define OVN_GENEVE_TYPE 0
+#define OVN_GENEVE_LEN 4
Are these values just placeholders until we have assigned numbers? If so, it
might be worth noting that they should change in the future.
This isn't particular to your patch, but I wonder if we should introduce a
#define for 0x that indicates it's the Geneve Experimental class.
--Justin
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 1/6] lib: Add smap_equal().
On Fri, Jul 31, 2015 at 01:14:40PM -0400, Russell Bryant wrote: Add a method to determine of two smaps are equal (have the exact same set of key-value pairs). Suggested-by: Ben Pfaff b...@nicira.com Signed-off-by: Russell Bryant rbry...@redhat.com Applied to master, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [dpdk-dev] Fwd: OVS with DPDK ..Error packets
-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Srikanth Akula
Sent: Wednesday, July 29, 2015 10:32 PM
To: d...@dpdk.org; dev@openvswitch.org
Subject: [dpdk-dev] Fwd: OVS with DPDK ..Error packets
(+DPDK dev team )
Hello ,
I am trying to test the OVS_DPDK performance and found that lot of packets
being treated as error packets .
ovs-vsctl get Interface dpdk0 statistics
{collisions=0, rx_bytes=38915076374, rx_crc_err=0, rx_dropped=0,
*rx_errors=3840287219
3840287219, *rx_frame_err=0, rx_over_err=0, rx_packets=292972799,
tx_bytes=38935883904, tx_dropped=0, tx_errors=0, tx_packets=293068162}
I am running DPDK application inside my VM .
Looks like there is a buffer issue ( 64Bytes - 10Gbps)
Could somebody let me know if i have missed any configuration in DPDK/OVS ?
Errors can show here when you are sending traffic in at a rate higher than can
be handled, so it can be normal to see that in some cases.
First thing I would check is that you have your PMD(s) and qemu threads doing
the fwding core affinitised to different cores so they get the max amount of
cycles they can.
I would also check a simple phy-phy test to eliminate any test equipment/NIC
setup issues.
-Srikanth
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 2/6] ovn: Add bridge mappings to ovn-controller.
On Fri, Jul 31, 2015 at 01:14:41PM -0400, Russell Bryant wrote:
Add a new OVN configuration entry in the Open_vSwitch database called
ovn-bridge-mappings. This allows the configuration of mappings
between a physical network name and an OVS bridge that provides
connectivity to that network.
For example, if you wanted to configure physnet1 to map to br-eth0
and physnet2 to map to br-eth1, the configuration would be:
$ ovs-vsctl set open . \
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
Patch ports between these bridges and the integration bridge are
automatically created and also removed if necessary when the
configuration changes.
Documentation for this configuration value is introduced in a later
patch that makes use of this by introducing a localnet logical port
type.
Signed-off-by: Russell Bryant rbry...@redhat.com
I tested this with:
make sandbox SANDBOXFLAGS=--ovn
ovs-vsctl add-br br-eth0
ovs-vsctl add-br br-eth1
ovs-vsctl set open .
external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1
ovs-vsctl show
The results:
Bridge br-eth1
Port patch-br-eth1-to-br-int
Interface patch-br-eth1-to-br-int
type: patch
options: {peer=br-int}
Port br-eth1
Interface br-eth1
type: internal
Bridge br-int
fail_mode: secure
Port br-int
Interface br-int
type: internal
Port patch-br-int-to-br-eth0
Interface patch-br-int-to-br-eth0
type: patch
options: {peer=br-eth0}
Port patch-br-int-to-br-eth1
Interface patch-br-int-to-br-eth1
type: patch
options: {peer=br-eth1}
Bridge br-eth0
Port patch-br-eth0-to-br-int
Interface patch-br-eth0-to-br-int
type: patch
options: {peer=br-int}
Port br-eth0
Interface br-eth0
type: internal
This looks close to correct to me. The peer names are all wrong,
though: the peer of a patch port is the name of another patch port, not
the name of another bridge.
I think that the code is intended to remove the patch ports when they
are not longer needed, but when I followed up with:
ovs-vsctl remove open . external-ids ovn-bridge-mappings
ovs-vsctl show
I still saw them.
Thanks,
Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 4/6] ovn: Get/set lport type and options in ovn-nbctl.
On Fri, Jul 31, 2015 at 01:14:43PM -0400, Russell Bryant wrote: A recent patch added type and options columns to the Logical_Port table in OVN_Northbound. This patch allows you to get and set those columns with ovn-nbctl. ovn-nbctl should eventually get converted to use the common db-ctl code that was recently added. When that happens, these commands can just be removed. Signed-off-by: Russell Bryant rbry...@redhat.com Applied to master, thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 3/6] ovn: Add type and options to logical port.
On Fri, Jul 31, 2015 at 01:14:42PM -0400, Russell Bryant wrote: We have started discussing the use of the logical port abstraction in OVN to represent special types of connections into an OVN logical switch. This patch proposes some schema updates to reflect these special types of logical ports. A logical port can have a type and a set of options specific to that type. Some examples of logical port types would be vtep for connectivity to a VTEP gateway or localnet for a connection to a locally accessible network via an ovs bridge. Actualy support for these (or other) types will come in later patches. Signed-off-by: Russell Bryant rbry...@redhat.com Acked-by: Ben Pfaff b...@nicira.com Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 6/6] ovn: Automatically create br-int in ovn-controller.
On Fri, Jul 31, 2015 at 01:14:45PM -0400, Russell Bryant wrote: ovn-controller previously required the integration bridge to be created before running ovn-controller. This patch makes ovn-controller automatically create it if it doesn't already exist. Signed-off-by: Russell Bryant rbry...@redhat.com Seems reasonable, but I'd like to get the tunnel-key series in first if that's OK. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v5 5/6] ovn: Add localnet logical port type.
On Fri, Jul 31, 2015 at 01:14:44PM -0400, Russell Bryant wrote: Introduce a new logical port type called localnet. A logical port with this type also has an option called network_name. A localnet logical port represents a connection to a locally accessible network. ovn-controller will use the ovn-bridge-mappings configuration to figure out which patch port on br-int should be used for this port. Signed-off-by: Russell Bryant rbry...@redhat.com Since this reserves a new register for OVN, we should remove that register from the logical pipeline symbol table (see the loop near the beginning of symtab_init() in pipeline.c) so that logical flows can't screw with it. Maybe the new field should be declared in pipeline.h with the others (my tunnel key series adds MFF_LOG_DATAPATH here too). It's not really a logical pipeline field, but it might still make sense to group these together. sparse spotted that these be64s in physical_run() should be be32s: struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ln_flow-ofpacts); sf-field = mf_from_id(MFF_OVN_FLAGS); sf-value.be64 = htonl(OVN_FLAG_LOCALNET); sf-mask.be64 = OVS_BE64_MAX; I'm not sure I correctly understand the model for using these. Maybe you can flesh out an example to help me. Suppose I have a pair of hypervisors A and B, and each HV has two VMs (A1 and A2, B1 and B2), and all four of the VMs are connected to the same Neutron provider network. How many OVN Logical_Switches would I have and what would their membership look like? On each HV, what interfaces would be attached to br-int? Thanks a lot, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 13/26] tunnel: Break tnl_xlate_init() into two separate functions.
That was surprisingly difficult to read :-)
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
It seems to me that tnl_xlate_init() has two almost-separate tasks. First,
it marks most of the 'wc' bits for tunnels. Second, it checks and updates
ECN bits. This commit breaks tnl_xlate_init() into two separate functions,
one for each of those tasks.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 3 +-
ofproto/tunnel.c | 85
ofproto/tunnel.h | 3 +-
3 files changed, 42 insertions(+), 49 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e69605e..8acb908 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4824,9 +4824,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
if (xbridge-netflow) {
netflow_mask_wc(flow, ctx.wc);
}
+tnl_wc_init(flow, xin-wc);
}
-tnl_may_send = tnl_xlate_init(flow, xin-wc);
+tnl_may_send = tnl_process_ecn(flow);
/* The in_port of the original packet before recirculation. */
in_port = get_ofp_port(xbridge, flow-in_port.ofp_port);
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 372b385..a7df772 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -332,67 +332,58 @@ out:
return ofport;
}
-static bool
-tnl_ecn_ok(struct flow *flow, struct flow_wildcards *wc)
-{
-if (is_ip_any(flow)) {
-if ((flow-tunnel.ip_tos IP_ECN_MASK) == IP_ECN_CE) {
-if (wc) {
-wc-masks.nw_tos |= IP_ECN_MASK;
-}
-if ((flow-nw_tos IP_ECN_MASK) == IP_ECN_NOT_ECT) {
-VLOG_WARN_RL(rl, dropping tunnel packet marked ECN CE
- but is not ECN capable);
-return false;
-} else {
-/* Set the ECN CE value in the tunneled packet. */
-flow-nw_tos |= IP_ECN_CE;
-}
-}
-}
-
-return true;
-}
-
/* Should be called at the beginning of action translation to initialize
* wildcards and perform any actions based on receiving on tunnel port.
*
* Returns false if the packet must be dropped. */
bool
-tnl_xlate_init(struct flow *flow, struct flow_wildcards *wc)
+tnl_process_ecn(struct flow *flow)
{
-/* tnl_port_should_receive() examines the 'tunnel.ip_dst' field to
- * determine the presence of the tunnel metadata. However, since
tunnels'
- * datapath port numbers are different from the non-tunnel ports, and we
- * always unwildcard the 'in_port', we do not need to unwildcard
- * the 'tunnel.ip_dst' for non-tunneled packets. */
-if (tnl_port_should_receive(flow)) {
-if (wc) {
-wc-masks.tunnel.tun_id = OVS_BE64_MAX;
-wc-masks.tunnel.ip_src = OVS_BE32_MAX;
-wc-masks.tunnel.ip_dst = OVS_BE32_MAX;
-wc-masks.tunnel.flags = (FLOW_TNL_F_DONT_FRAGMENT |
- FLOW_TNL_F_CSUM |
- FLOW_TNL_F_KEY);
-wc-masks.tunnel.ip_tos = UINT8_MAX;
-wc-masks.tunnel.ip_ttl = UINT8_MAX;
-/* The tp_src and tp_dst members in flow_tnl are set to be always
- * wildcarded, not to unwildcard them here. */
-wc-masks.tunnel.tp_src = 0;
-wc-masks.tunnel.tp_dst = 0;
-
-memset(wc-masks.pkt_mark, 0xff, sizeof wc-masks.pkt_mark);
-}
-if (!tnl_ecn_ok(flow, wc)) {
+if (!tnl_port_should_receive(flow)) {
+return true;
+}
+
+if (is_ip_any(flow) (flow-tunnel.ip_tos IP_ECN_MASK) == IP_ECN_CE)
{
+if ((flow-nw_tos IP_ECN_MASK) == IP_ECN_NOT_ECT) {
+VLOG_WARN_RL(rl, dropping tunnel packet marked ECN CE
+ but is not ECN capable);
return false;
}
-flow-pkt_mark = ~IPSEC_MARK;
+/* Set the ECN CE value in the tunneled packet. */
+flow-nw_tos |= IP_ECN_CE;
}
+flow-pkt_mark = ~IPSEC_MARK;
return true;
}
+void
+tnl_wc_init(struct flow *flow, struct flow_wildcards *wc)
+{
+if (tnl_port_should_receive(flow)) {
+wc-masks.tunnel.tun_id = OVS_BE64_MAX;
+wc-masks.tunnel.ip_src = OVS_BE32_MAX;
+wc-masks.tunnel.ip_dst = OVS_BE32_MAX;
+wc-masks.tunnel.flags = (FLOW_TNL_F_DONT_FRAGMENT |
+ FLOW_TNL_F_CSUM |
+ FLOW_TNL_F_KEY);
+wc-masks.tunnel.ip_tos = UINT8_MAX;
+wc-masks.tunnel.ip_ttl = UINT8_MAX;
+/* The tp_src and tp_dst members in flow_tnl are set to be always
+ * wildcarded, not to unwildcard them here. */
+wc-masks.tunnel.tp_src = 0;
+
Re: [ovs-dev] [PATCH 14/26] ofproto-dpif-xlate: Factor wildcard processing out of xlate_actions().
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
I think that this makes xlate_actions() easier to read.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 102 ---
1 file changed, 58 insertions(+), 44 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8acb908..03bca1b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4717,6 +4717,58 @@ too_many_output_actions(const struct ofpbuf
*odp_actions OVS_UNUSED)
#endif
}
+static void
+xlate_wc_init(struct xlate_ctx *ctx)
+{
+flow_wildcards_init_catchall(ctx-wc);
+
+/* Some fields we consider to always be examined. */
+memset(ctx-wc-masks.in_port, 0xff, sizeof ctx-wc-masks.in_port);
+memset(ctx-wc-masks.dl_type, 0xff, sizeof ctx-wc-masks.dl_type);
+if (is_ip_any(ctx-xin-flow)) {
+ctx-wc-masks.nw_frag |= FLOW_NW_FRAG_MASK;
+}
+
+if (ctx-xbridge-support.odp.recirc) {
+/* Always exactly match recirc_id when datapath supports
+ * recirculation. */
+ctx-wc-masks.recirc_id = UINT32_MAX;
+}
+
+if (ctx-xbridge-netflow) {
+netflow_mask_wc(ctx-xin-flow, ctx-wc);
+}
+
+tnl_wc_init(ctx-xin-flow, ctx-wc);
+}
+
+static void
+xlate_wc_finish(struct xlate_ctx *ctx)
+{
+/* Clear the metadata and register wildcard masks, because we won't
+ * use non-header fields as part of the cache. */
+flow_wildcards_clear_non_packet_fields(ctx-wc);
+
+/* ICMPv4 and ICMPv6 have 8-bit type and code fields. struct flow
+ * uses the low 8 bits of the 16-bit tp_src and tp_dst members to
+ * represent these fields. The datapath interface, on the other hand,
+ * represents them with just 8 bits each. This means that if the high
+ * 8 bits of the masks for these fields somehow become set, then they
+ * will get chopped off by a round trip through the datapath, and
+ * revalidation will spot that as an inconsistency and delete the flow.
+ * Avoid the problem here by making sure that only the low 8 bits of
+ * either field can be unwildcarded for ICMP.
+ */
+if (is_icmpv4(ctx-xin-flow) || is_icmpv6(ctx-xin-flow)) {
+ctx-wc-masks.tp_src = htons(UINT8_MAX);
+ctx-wc-masks.tp_dst = htons(UINT8_MAX);
+}
+/* VLAN_TCI CFI bit must be matched if any of the TCI is matched. */
+if (ctx-wc-masks.vlan_tci) {
+ctx-wc-masks.vlan_tci |= htons(VLAN_CFI);
+}
+}
+
/* Translates the flow, actions, or rule in 'xin' into datapath actions in
* 'xout'.
* The caller must take responsibility for eventually freeing 'xout', with
@@ -4782,9 +4834,11 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
};
memset(ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
ofpbuf_reserve(ctx.odp_actions, NL_A_U32_SIZE);
+if (xin-wc) {
+xlate_wc_init(ctx);
+}
struct xport *in_port;
-bool tnl_may_send;
COVERAGE_INC(xlate_actions);
@@ -4809,26 +4863,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
* kernel does. If we wish to maintain the original values an action
* needs to be generated. */
-if (xin-wc) {
-flow_wildcards_init_catchall(ctx.wc);
-memset(ctx.wc-masks.in_port, 0xff, sizeof ctx.wc-masks.in_port);
-memset(ctx.wc-masks.dl_type, 0xff, sizeof ctx.wc-masks.dl_type);
-if (is_ip_any(flow)) {
-ctx.wc-masks.nw_frag |= FLOW_NW_FRAG_MASK;
-}
-if (xbridge-support.odp.recirc) {
-/* Always exactly match recirc_id when datapath supports
- * recirculation. */
-ctx.wc-masks.recirc_id = UINT32_MAX;
-}
-if (xbridge-netflow) {
-netflow_mask_wc(flow, ctx.wc);
-}
-tnl_wc_init(flow, xin-wc);
-}
-
-tnl_may_send = tnl_process_ecn(flow);
-
/* The in_port of the original packet before recirculation. */
in_port = get_ofp_port(xbridge, flow-in_port.ofp_port);
@@ -4968,7 +5002,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
size_t sample_actions_len = ctx.odp_actions-size;
-if (tnl_may_send (!in_port || may_receive(in_port, ctx))) {
+if (tnl_process_ecn(flow)
+ (!in_port || may_receive(in_port, ctx))) {
const struct ofpact *ofpacts;
size_t ofpacts_len;
@@ -5079,28 +5114,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
}
if (xin-wc) {
-/* Clear the metadata and register wildcard masks, because we won't
- * use non-header fields as part of the cache. */
-flow_wildcards_clear_non_packet_fields(ctx.wc);
-
-/* ICMPv4 and ICMPv6 have 8-bit type and code
Re: [ovs-dev] [PATCH 25/26] ofproto-dpif-xlate: Add recirculation information to ofproto/trace.
Acked-by: Jarno Rajahalme jrajaha...@nicira.com
On Jul 29, 2015, at 11:42 PM, Ben Pfaff b...@nicira.com wrote:
This makes it possible to understand what happens recirculation-wise in
translation.
Signed-off-by: Ben Pfaff b...@nicira.com
---
ofproto/ofproto-dpif-xlate.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 68f4b37..dc16a39 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -530,6 +530,18 @@ xlate_report(struct xlate_ctx *ctx, const char *format,
...)
}
}
+static inline void
+xlate_report_actions(struct xlate_ctx *ctx, const char *title,
+ const struct ofpact *ofpacts, size_t ofpacts_len)
+{
+if (OVS_UNLIKELY(ctx-xin-report_hook)) {
+struct ds s = DS_EMPTY_INITIALIZER;
+ofpacts_format(ofpacts, ofpacts_len, s);
+xlate_report(ctx, %s: %s, title, ds_cstr(s));
+ds_destroy(s);
+}
+}
+
static void
xlate_xbridge_init(struct xlate_cfg *xcfg, struct xbridge *xbridge)
{
@@ -4814,13 +4826,14 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
if (xin-recirc) {
const struct recirc_state *state = xin-recirc-state;
+xlate_report(ctx, Restoring state post-recirculation:);
+
if (xin-ofpacts_len 0 || ctx.rule) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+const char *conflict = xin-ofpacts_len ? actions : rule;
-VLOG_WARN_RL(rl, Recirculation conflict (%s)!,
- xin-ofpacts_len 0
- ? actions
- : rule);
+VLOG_WARN_RL(rl, Recirculation conflict (%s)!, conflict);
+xlate_report(ctx, - Recirculation conflict (%s)!, conflict);
goto exit;
}
@@ -4834,6 +4847,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Drop the packet if the bridge cannot be found. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
VLOG_WARN_RL(rl, Recirculation bridge no longer exists.);
+xlate_report(ctx, - Recirculation bridge no longer
exists.);
goto exit;
}
ctx.xbridge = new_bridge;
@@ -4842,6 +4856,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
/* Set the post-recirculation table id. Note: A table lookup is done
* only if there are no post-recirculation actions. */
ctx.table_id = state-table_id;
+xlate_report(ctx, - Resuming from table %PRIu8, ctx.table_id);
/* Restore pipeline metadata. May change flow's in_port and other
* metadata to the values that existed when recirculation was
@@ -4857,6 +4872,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
if (state-action_set_len) {
const struct ofpact *a;
+xlate_report_actions(ctx, - Restoring action set,
+ state-ofpacts, state-action_set_len);
+
ofpbuf_put(ctx.action_set, state-ofpacts,
state-action_set_len);
OFPACT_FOR_EACH(a, state-ofpacts, state-action_set_len) {
@@ -4873,6 +4891,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
xin-ofpacts_len = state-ofpacts_len - state-action_set_len;
xin-ofpacts = state-ofpacts +
state-action_set_len / sizeof *state-ofpacts;
+
+xlate_report_actions(ctx, - Restoring actions,
+ xin-ofpacts, xin-ofpacts_len);
}
} else if (OVS_UNLIKELY(flow-recirc_id)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 26/26] ofproto-dpif-xlate: Fix mirroring interaction with recirculation.
On Fri, Jul 31, 2015 at 03:10:47PM -0700, Jarno Rajahalme wrote: Acked-by: Jarno Rajahalme jrajaha...@nicira.com Thanks for the clean-up, will be easier to maintain this code going forward. Thanks for all the reviews! I applied this series to master. Appreciate the C lessons as well :-) :-) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 2/2] ofproto: Implement OF1.4 Group Meter change notification messages
Hi Ben, Thanks for the review. All the comments have been incorporated in Version 3 of the patch. Thanks Niti Rohilla On Tue, Jul 28, 2015 at 8:59 PM, Ben Pfaff b...@nicira.com wrote: On Tue, Jul 28, 2015 at 05:22:25PM +0530, niti1...@gmail.com wrote: From: Niti Rohilla niti.rohi...@tcs.com This patch adds support for Openflow1.4 Group meter change notification messages. In a multi controller environment, when a controller modifies the state of group and meter table, the request that successfully modifies this state is forwarded to other controllers. Other controllers are informed with the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per controller channel basis using the Set Asynchronous Configuration Message. Signed-off-by: Niti Rohilla niti.rohi...@tcs.com Your function for re-encoding requests doesn't actually do it! It only changes the version number. That won't properly account for differences between OF1.4 and later versions of the protocol, for example in changes to numbering of OXM fields. The code needs to use ofputil_*() functions to encode the group_mod and meter_mod requests properly for the protocol in use. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 15/15] net: Drop unlikely before IS_ERR(_OR_NULL)
IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag and there
is no need to do that again from its callers. Drop it.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
net/openvswitch/datapath.c | 2 +-
net/sctp/socket.c | 2 +-
net/socket.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ff8c4a4c1609..01d69680ba5d 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -1143,7 +1143,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct
genl_info *info)
info, OVS_FLOW_CMD_NEW, false,
ufid_flags);
- if (unlikely(IS_ERR(reply))) {
+ if (IS_ERR(reply)) {
error = PTR_ERR(reply);
goto err_unlock_ovs;
}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 1425ec2bbd5a..c1569432235e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4481,7 +4481,7 @@ static int sctp_getsockopt_peeloff(struct sock *sk, int
len, char __user *optval
}
newfile = sock_alloc_file(newsock, 0, NULL);
- if (unlikely(IS_ERR(newfile))) {
+ if (IS_ERR(newfile)) {
put_unused_fd(retval);
sock_release(newsock);
return PTR_ERR(newfile);
diff --git a/net/socket.c b/net/socket.c
index 9963a0b53a64..dd2c247c99e3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -373,7 +373,7 @@ struct file *sock_alloc_file(struct socket *sock, int
flags, const char *dname)
file = alloc_file(path, FMODE_READ | FMODE_WRITE,
socket_file_ops);
- if (unlikely(IS_ERR(file))) {
+ if (IS_ERR(file)) {
/* drop dentry, keep inode */
ihold(d_inode(path.dentry));
path_put(path);
@@ -1303,7 +1303,7 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int,
protocol,
}
newfile1 = sock_alloc_file(sock1, flags, NULL);
- if (unlikely(IS_ERR(newfile1))) {
+ if (IS_ERR(newfile1)) {
err = PTR_ERR(newfile1);
goto out_put_unused_both;
}
@@ -1467,7 +1467,7 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user
*, upeer_sockaddr,
goto out_put;
}
newfile = sock_alloc_file(newsock, flags,
sock-sk-sk_prot_creator-name);
- if (unlikely(IS_ERR(newfile))) {
+ if (IS_ERR(newfile)) {
err = PTR_ERR(newfile);
put_unused_fd(newfd);
sock_release(newsock);
--
2.4.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v4 2/2] ofproto: Implement OF1.4 Group Meter change notification messages
From: Niti Rohilla niti.rohi...@tcs.com
This patch adds support for Openflow1.4 Group meter change notification
messages. In a multi controller environment, when a controller modifies the
state of group and meter table, the request that successfully modifies this
state is forwarded to other controllers. Other controllers are informed with
the OFPT_REQUESTFORWARD message. Request forwarding is enabled on a per
controller channel basis using the Set Asynchronous Configuration Message.
Signed-off-by: Niti Rohilla niti.rohi...@tcs.com
---
Comments recevied on 28th July,2015 have been incorporated in this version.
Following are the changes between v3 and v4:
ofputil_re_encode_requestforward() has been changed to encode the group_mod and
meter_mod requests properly for the protocol in use in ofconn.
include/openflow/openflow-1.4.h | 6 +++
lib/learning-switch.c | 1 +
lib/ofp-msgs.h | 6 +++
lib/ofp-print.c | 36 +++
lib/ofp-util.c | 99 +
lib/ofp-util.h | 16 +++
lib/rconn.c | 1 +
ofproto/connmgr.c | 47 +++
ofproto/connmgr.h | 4 ++
ofproto/ofproto.c | 27 ---
ovn/controller/ofctrl.c | 1 +
tests/ofp-print.at | 46 +++
tests/ofproto.at| 76 +++
13 files changed, 361 insertions(+), 5 deletions(-)
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 37eef4a..5202fe1 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -355,6 +355,12 @@ struct ofp14_role_prop_experimenter {
};
OFP_ASSERT(sizeof(struct ofp14_role_prop_experimenter) == 12);
+/* Group/Meter request forwarding. */
+struct ofp14_requestforward {
+struct ofp_header request; /* Request being forwarded. */
+};
+OFP_ASSERT(sizeof(struct ofp14_requestforward) == 8);
+
/* Bundle control message types */
enum ofp14_bundle_ctrl_type {
OFPBCT_OPEN_REQUEST= 0,
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 1753cea..7ddf69b 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -419,6 +419,7 @@ lswitch_process_packet(struct lswitch *sw, const struct
ofpbuf *msg)
case OFPTYPE_ROLE_REQUEST:
case OFPTYPE_ROLE_REPLY:
case OFPTYPE_ROLE_STATUS:
+case OFPTYPE_REQUESTFORWARD:
case OFPTYPE_SET_FLOW_FORMAT:
case OFPTYPE_FLOW_MOD_TABLE_ID:
case OFPTYPE_SET_PACKET_IN_FORMAT:
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 1358b21..52a74d1 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -247,6 +247,9 @@ enum ofpraw {
/* OFPT 1.4+ (30): struct ofp14_role_status, uint8_t[8][]. */
OFPRAW_OFPT14_ROLE_STATUS,
+/* OFPT 1.4+ (32): struct ofp14_requestforward, uint8_t[8][]. */
+OFPRAW_OFPT14_REQUESTFORWARD,
+
/* OFPT 1.4+ (33): struct ofp14_bundle_ctrl_msg, uint8_t[8][]. */
OFPRAW_OFPT14_BUNDLE_CONTROL,
@@ -559,6 +562,9 @@ enum ofptype {
/* Controller role change event messages. */
OFPTYPE_ROLE_STATUS, /* OFPRAW_OFPT14_ROLE_STATUS. */
+/* Request forwarding by the switch. */
+OFPTYPE_REQUESTFORWARD, /* OFPRAW_OFPT14_REQUESTFORWARD. */
+
OFPTYPE_BUNDLE_CONTROL, /* OFPRAW_OFPT14_BUNDLE_CONTROL. */
OFPTYPE_BUNDLE_ADD_MESSAGE, /* OFPRAW_OFPT14_BUNDLE_ADD_MESSAGE. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index b8088f3..6a55e21 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -3050,6 +3050,38 @@ ofp_print_geneve_table_reply(struct ds *s, const struct
ofp_header *oh)
ofputil_uninit_geneve_table(gtr.mappings);
}
+/* This function will print the request forward message. The reason for
+ * request forward is taken from rf.request.type */
+static void
+ofp_print_requestforward(struct ds *string, const struct ofp_header *oh)
+{
+struct ofputil_requestforward rf;
+enum ofperr error;
+enum ofptype type;
+
+error = ofputil_decode_requestforward(oh, rf);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+error = ofptype_decode(type, rf.request);
+if (error) {
+ofp_print_error(string, error);
+return;
+}
+
+ds_put_cstr(string, reason=);
+
+if (type == OFPTYPE_GROUP_MOD) {
+ds_put_cstr(string, group_mod);
+ofp_print_group_mod(string, rf.request);
+} else if (type == OFPTYPE_METER_MOD) {
+ds_put_cstr(string, meter_mod);
+ofp_print_meter_mod(string, rf.request);
+}
+}
+
static void
ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
struct ds *string, int verbosity)
@@ -3179,6 +3211,10 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw
raw,
ofp_print_role_status_message(string, oh);
break;
+case OFPTYPE_REQUESTFORWARD:
+
