Re: [ovs-dev] [PATCH 4/5] ofproto.at: Fix races in rule eviciton tests
On Mon, Mar 31, 2014 at 2:24 PM, YAMAMOTO Takashi
wrote:
> Bump timeout differences, because timeouts different by 1s might end up
> to have the same position in the heap as rule_eviction_priority() uses
> 1024ms as a unit.
>
> Also, use time/stop to avoid relying on how long an add-flow would take.
>
> These tests were introduced by commit 6d56c1f1.
> ("ofproto: Update rule's priority in eviction group.")
>
> Signed-off-by: YAMAMOTO Takashi
> ---
> tests/ofproto.at | 42 ++
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index e98b526..46ae9e9 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1375,27 +1375,28 @@ AT_CHECK(
> | ${PERL} $srcdir/uuidfilt.pl],
>[0], [<0>
> ])
> +ovs-appctl time/stop
> # Add 4 flows.
> for in_port in 4 3 2 1; do
> -ovs-ofctl add-flow br0
> hard_timeout=1${in_port},in_port=$in_port,actions=drop
> +ovs-ofctl add-flow br0
> hard_timeout=1${in_port}0,in_port=$in_port,actions=drop
> done
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=12, in_port=2 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=110, in_port=1 actions=drop
> + hard_timeout=120, in_port=2 actions=drop
> + hard_timeout=130, in_port=3 actions=drop
> + hard_timeout=140, in_port=4 actions=drop
> NXST_FLOW reply:
> ])
> # Sleep and modify the one that expires soonest
> -sleep 2
> +ovs-appctl time/warp 2
I think we should warp as small as possible to verify precision.
What do you think about warp 12000?
> AT_CHECK([ovs-ofctl mod-flows br0 in_port=1,actions=drop])
> -sleep 2
> +ovs-appctl time/warp 2
FYI: This warp is fine as far as it's greater than 1000, but is no
harm to choose a bigger one.
> # Adding another flow will cause the one that expires soonest to be evicted.
> AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=110, in_port=1 actions=drop
> + hard_timeout=130, in_port=3 actions=drop
> + hard_timeout=140, in_port=4 actions=drop
> in_port=5 actions=drop
> NXST_FLOW reply:
> ])
> @@ -1414,26 +1415,27 @@ AT_CHECK(
> ])
> # Add 4 flows.
> for in_port in 4 3 2 1; do
> -ovs-ofctl add-flow br0
> idle_timeout=1${in_port},in_port=$in_port,actions=drop
> +ovs-ofctl add-flow br0
> idle_timeout=1${in_port}0,in_port=$in_port,actions=drop
> done
> +ovs-appctl time/stop
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=11, in_port=1 actions=drop
> - idle_timeout=12, in_port=2 actions=drop
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=110, in_port=1 actions=drop
> + idle_timeout=120, in_port=2 actions=drop
> + idle_timeout=130, in_port=3 actions=drop
> + idle_timeout=140, in_port=4 actions=drop
> NXST_FLOW reply:
> ])
> # Sleep and receive on the flow that expires soonest
> -sleep 2
> +ovs-appctl time/warp 2
Same comment as above.
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1)'])
> -sleep 2
> +ovs-appctl time/warp 2
> # Adding another flow will cause the one that expires soonest to be evicted.
> AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=130, in_port=3 actions=drop
> + idle_timeout=140, in_port=4 actions=drop
> in_port=5 actions=drop
> - n_packets=1, n_bytes=60, idle_timeout=11, in_port=1 actions=drop
> + n_packets=1, n_bytes=60, idle_timeout=110, in_port=1 actions=drop
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
> --
> 1.8.3.1
>
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] toil
these fleeting phenomenal discor , and partly to the more prominent actors in the war. The contrast between the American colonies of Great Britain throwing off their allegiance to the Old Country because they saw fit to do so for their own interests, and the government of the Federation of these same ex-colonies insisting that some of them, which in their turn see fit to break loose from the Federal pact, shall not do so, under the alternative of war and the pains of treason,--this contrast is assuredly a glaring one; many people considered that it amounted to a positive anomaly,--not a few to a barefaced act of tyrannic apostasy. The personal feeling of the English people, their national _amour propre_, conspired to lead towards this harshest construction of the facts: it was so tempting to convict our old adversaries out of their own mouths, and make them, by the logic of events, read out either their recantation of the Colonial Revolution, or their self-condemnation for the Anti-Secession War. I have already explained to what extent these views appear to me to be tenable, and where their weak point lies: that both the insurrection of the colonies against England, and that of the South against the Federation,--both the repressive measures of England against the colonies, and of the Federation against the South,--were in themselves founded on an indefeasible right, and abstractly defensible; and that the "casting vote," (so to speak,) in both cases, depends, not upon any wordy denial of the right, but upon a thorough estimate of all the attendant conditions, and prominently of the "mights of man." So far for one phase of the personal question. The other phase pertained to the character and the deeds of some leading actors in the war-drama. To most English apprehensions, _the_ hero of the war, from an early stage of it up to his tragic death, was Stonewall Jackson, whose place was afterwards taken, in popular esteem, though not in coequal enthusiasm, by General Lee, both of them Southerners; while the _bete noire_ of the story was General Butler, the Northerner. It would be futile to expound<>___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] from Mr Catherine
My dear I sent you an important information that required urgent your action but until now you have not reply the message why? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] Rapid Spanning Protocol Implementation (IEEE 802.1D) + functional tests
This is the diff between RSTP patch v3 and v2 (the one sent on February 24th).
---
diff --git a/v1/lib/rstp.c b/v2/lib/rstp.c
index 2725707..d226967 100644
--- a/v1/lib/rstp.c
+++ b/v2/lib/rstp.c
@@ -1076,6 +1076,20 @@ rstp_port_get_aux(struct rstp_port *p)
return aux;
}
+/* Returns true if 'state' is one in which BPDU packets should be received
+ * and transmitted on a port, false otherwise.
+ */
+ bool
+ rstp_should_manage_bpdu(enum rstp_state state)
+ {
+ if (state == RSTP_DISCARDING || state == RSTP_LEARNING ||
+ state == RSTP_FORWARDING) {
+ return true;
+ } else {
+ return false;
+ }
+ }
+
/* Returns true if 'state' is one in which packets received on a port should
* be forwarded, false otherwise.
*
@@ -1144,4 +1158,3 @@ rstp_unixctl_tcn(struct unixctl_conn *conn, int argc,
out:
ovs_mutex_unlock(&mutex);
}
---
diff --git a/v1/lib/rstp.h b/v2/lib/rstp.h
index 31cee13..d3eb6a6 100644
--- a/v1/lib/rstp.h
+++ b/v2/lib/rstp.h
@@ -107,6 +107,7 @@ char *get_id_string_from_uint8_t(uint8_t *, int);
char *rstp_state_name(enum rstp_state);
bool rstp_forward_in_state(enum rstp_state);
bool rstp_learn_in_state(enum rstp_state);
+bool rstp_should_manage_bpdu(enum rstp_state state);
char *rstp_port_role_name(enum rstp_port_role);
void rstp_init(void);
diff --git a/v1/lib/rstp-state-machines.c b/v2/lib/rstp-state-machines.c
index e8a263d..9f96f79 100644
--- a/v1/lib/rstp-state-machines.c
+++ b/v2/lib/rstp-state-machines.c
@@ -531,7 +531,7 @@ OVS_REQUIRES(mutex)
pkt = ofpbuf_new(ETH_HEADER_LEN + LLC_HEADER_LEN + bpdu_size);
pkt->l2 = eth = ofpbuf_put_zeros(pkt, sizeof *eth);
llc = ofpbuf_put_zeros(pkt, sizeof *llc);
-pkt->l3 = ofpbuf_put(pkt, bpdu, bpdu_size);
+ofpbuf_set_l3(pkt, ofpbuf_put(pkt, bpdu, bpdu_size));
/* 802.2 header. */
memcpy(eth->eth_dst, eth_addr_rstp, ETH_ADDR_LEN);
---
diff --git a/v1/ofproto/ofproto-dpif-upcall.c b/v2/ofproto/ofproto-dpif-upcall.c
index 244e0f8..8a330f1 100644
--- a/v1/ofproto/ofproto-dpif-upcall.c
+++ b/v2/ofproto/ofproto-dpif-upcall.c
@@ -921,7 +921,7 @@ compose_slow_path(struct udpif *udpif, struct
xlate_out *xout,
cookie.slow_path.unused = 0;
cookie.slow_path.reason = xout->slow;
-port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP)
+port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP |
SLOW_RSTP)
? ODPP_NONE
: odp_in_port;
pid = dpif_port_get_pid(udpif->dpif, port, 0);
---
diff --git a/v1/ofproto/ofproto-dpif-xlate.c b/v2/ofproto/ofproto-dpif-xlate.c
index 0a6b65b..9a0c718 100644
--- a/v1/ofproto/ofproto-dpif-xlate.c
+++ b/v2/ofproto/ofproto-dpif-xlate.c
@@ -739,7 +739,7 @@ xport_get_rstp_port(const struct xport *xport)
: NULL;
}
-static enum rstp_state
+static bool
xport_rstp_learn_state(const struct xport *xport)
{
struct rstp_port *rp = xport_get_rstp_port(xport);
@@ -753,6 +753,13 @@ xport_rstp_forward_state(const struct xport *xport)
return rstp_forward_in_state(rp ? rstp_port_get_state(rp) : RSTP_DISABLED);
}
+static bool
+xport_rstp_should_manage_bpdu(const struct xport *xport)
+{
+struct rstp_port *rp = xport_get_rstp_port(xport);
+return rstp_should_manage_bpdu(rp ? rstp_port_get_state(rp) :
RSTP_DISABLED);
+}
+
/* Returns true if RSTP should process 'flow'. Sets fields in 'wc' that
* were used to make the determination.*/
static bool
@@ -1770,21 +1777,23 @@ compose_output_action__(struct xlate_ctx *ctx,
ofp_port_t ofp_port,
} else if (xport->config & OFPUTIL_PC_NO_FWD) {
xlate_report(ctx, "OFPPC_NO_FWD set, skipping output");
return;
-} else if (check_stp) {
-if (eth_addr_equals(ctx->base_flow.dl_dst, eth_addr_stp)) {
-if (!xport_stp_listen_state(xport)) {
+} else if (check_stp && check_rstp) {
+if (eth_addr_equals(ctx->base_flow.dl_dst, eth_addr_stp) &&
+eth_addr_equals(ctx->base_flow.dl_dst, eth_addr_rstp)) {
+if (!xport_stp_listen_state(xport) &&
+ !xport_rstp_should_manage_bpdu(xport)) {
xlate_report(ctx, "STP not in listening state, "
+"RSTP does not manage BPDU in this state, "
"skipping bpdu output");
return;
-}
-} else if (!xport_stp_forward_state(xport)) {
+} else if (!xport_stp_forward_state(xport) &&
+!xport_rstp_forward_state(xport)) {
xlate_report(ctx, "STP not in forwarding state, "
+"RSTP not in forwarding state, "
"skipping output");
return;
}
-} else if (check_rstp && !xport_rstp_forward_state(xport)) {
-xlate_report(ctx, "RSTP not int forwarding state, skipping output");
-return;
+}
}
if (mbridge_has_mirrors(ctx->xbridge->mbridge) && xport->xbundle) {
@@ -1811,7 +1820,7 @@ c
[ovs-dev] waimaoie ，帮你定位开发全球目标客户！与您共享了相册。
6674570558665051175975807744913939379765982978793377940695601954545212831445384076376468553050251523Shenzhen Technology Co.,Ltd is a high-tech company specializing in the development, production and sale of hydraulic reinforced concrete cutting machine.Main products are fully hydraulic wall saw, hydraulic wire saw. The main models include hape107, 207,307 With light, sharp, timeliness characteristic, can greatly improve the efficiency of the construction company. The company has strong technical strength，Asia"s leading manufacturing technique ，can reach the world advanced level,and get more than 10 patents in China( can offer the patents list for your reference).To ensure the quality of the machine,many importance parts of the machine are imported from Europe countries base on our special design,the whole machine quality can reach European product quality standard,but with much competitive price. The hydraulic concrete wall saw,wire saw are widely used in the construction, building modification, and demolition; such as tunnel, metro, seaport/airport, dam, hydropower station, and so on; our hydraulic wall saw and wire saw have been widely used in Chinese government projects, such as Beijing Olympics Project Line 5 metro, Shanghai Pongdong metro, Guangzhou Metro, Shenzhen Metro, Hangzhou Metro, Shanghai Hongqiao airport, Jinan Train Station, Wuguang High way Metro, Jingzhu Highway, Jinghang Great River, and so on. (We can offer all case photos,videos for your reference). As the great performance of our machines known by more and more people,our products continuously export to Southeast Asia,Middle East,Europe,and get much reputation from our clients. Sincerely looking forward to our further communication and cooperation ,make your work much more efficiency. Please feel free to contact me if you have any question,I will get back to you in time. Best wishes Joe (Sales executive) Shenzhen Technology Co.,Ltd Address: Shenzhen, China LzwqIFcyXcZknCYZjsnRgzTmhWCLLKPAtAerrHKNFetmWEGaDIVgBOBoPlkMuLxBbBNGZWtOnfbnEuQDNkfJLrLRZOuDECHelsWjjzCFxWleOwyTvAOKtGggHQcEYDbtTtFzRAXGRXTfwLSUlJEhjQWpxnSOONSDJRhIHXNQIhKomUJdUKYjRREEgoBdwcASeSPJcZvfqwsDUVrsCZGkAgmGODjeGFKvovZAlBFIAZCgQMBVMCQbJJwjKgtHoGswWKHBURnJiokvzNjkuEycsKeyshNWsqvgZgLYXnrulKZS https://picasaweb.google.com/lh/sredir?uname=109336229090790138640&target=ALBUM&id=5996964977861004593&authkey=Gv1sRgCKyPnIKMrsPjGA&invite=CIv1x5gD&feat=email <><>___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/5] ofproto: Fix a race in pause and resume test
On Mon, Mar 31, 2014 at 03:24:28PM +0900, YAMAMOTO Takashi wrote: > Signed-off-by: YAMAMOTO Takashi Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/5] ofproto-dpif: Fix races in MPLS tests
On Mon, Mar 31, 2014 at 03:24:29PM +0900, YAMAMOTO Takashi wrote: > Depending on how output buffers are flushed, there might > be still races. But this at least makes the race windows smaller. > > Signed-off-by: YAMAMOTO Takashi Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/5] ofproto-dpif.at: Fix races in table-miss tests
On Mon, Mar 31, 2014 at 03:24:27PM +0900, YAMAMOTO Takashi wrote: > Signed-off-by: YAMAMOTO Takashi Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/5] ofproto.at: Fix races in rule eviciton tests
On Mon, Mar 31, 2014 at 03:24:30PM +0900, YAMAMOTO Takashi wrote:
> Bump timeout differences, because timeouts different by 1s might end up
> to have the same position in the heap as rule_eviction_priority() uses
> 1024ms as a unit.
>
> Also, use time/stop to avoid relying on how long an add-flow would take.
>
> These tests were introduced by commit 6d56c1f1.
> ("ofproto: Update rule's priority in eviction group.")
>
> Signed-off-by: YAMAMOTO Takashi
Kmindg's comments sound reasonable to me, otherwise:
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 5/5] ofproto-dpif.at: Sprinkle --overwrite-pidfile
On Mon, Mar 31, 2014 at 03:24:31PM +0900, YAMAMOTO Takashi wrote: > These tests invokes ovs-ofctl monitor twice or more. > Because "ovs-appctl -t ofctl exit" does not wait for the target > process exit, there are chances to see the pid file from the previous > incarnation. > > Signed-off-by: YAMAMOTO Takashi Good catch. As an alternative, one could OVS_WAIT_UNTIL the pidfile disappears. Thank you for improving all these tests! Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Bug#740983: Bug #740983: use the upstream kernel module instead
severity 740983 normal thanks This bug was submitted with: Severity: grave Justification: renders package unusable because the DKMS module does not build with Linux kernel 3.13. While this is true, it does not render the package unusable by any stretch of the imagination, because Linux kernel 3.13, like every kernel from version 3.3 onward, has a built-in openvswitch kernel module that works perfectly well. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Processed: Bug #740983: use the upstream kernel module instead
Processing commands for cont...@bugs.debian.org: > severity 740983 normal Bug #740983 [openvswitch-datapath-dkms] openvswitch-datapath-dkms: fails to build dkms module Severity set to 'normal' from 'grave' > thanks Stopping processing here. Please contact me if you need assistance. -- 740983: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740983 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] debian: Stop openvswitch after VMs managed by libvirt.
When openvswitch stops before libvirt shuts down VMs, it makes it hard for libvirt to remove virtual network interfaces (ovs-vsctl cannot access the database socket, which has been removed). This commit should ensure that the VMs get shut down before openvswitch. CC: 701...@bugs.debian.org Reported-by: Ernesto Domato Suggested-by: Lukasz Szotek Signed-off-by: Ben Pfaff --- AUTHORS| 2 ++ debian/openvswitch-switch.init | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index b189957..4945e7d 100644 --- a/AUTHORS +++ b/AUTHORS @@ -164,6 +164,7 @@ Derek Cormier derek.corm...@lab.ntt.co.jp Dhaval Badiani dbadi...@vmware.com DK Moon dkm...@nicira.com Edwin Chiu ec...@nicira.com +Ernesto Domato edo...@gmail.com Eivind Bulie Haanaes Eric Lopez elo...@nicira.com Frido Roose fr.ro...@gmail.com @@ -209,6 +210,7 @@ Len Gao l...@vmware.com Logan Rosen logatron...@gmail.com Luca Falavigna dktrkr...@debian.org Luiz Henrique Ozaki luiz.oz...@gmail.com +Lukasz Szotek szot...@gmail.com Maxime Brun m.b...@alphalink.fr Michael A. Collins mike.a.coll...@ark-net.org Michael Hu m...@nicira.com diff --git a/debian/openvswitch-switch.init b/debian/openvswitch-switch.init index 481b29c..98a69f7 100755 --- a/debian/openvswitch-switch.init +++ b/debian/openvswitch-switch.init @@ -1,6 +1,6 @@ #! /bin/sh # -# Copyright (C) 2011, 2012 Nicira, Inc. +# Copyright (C) 2011, 2012, 2014 Nicira, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ # Provides: openvswitch-switch # Required-Start:$network $named $remote_fs $syslog # Required-Stop: $remote_fs +# X-Stop-After: libvirt-guests # Default-Start: 2 3 4 5 # Default-Stop: 0 1 6 # Short-Description: Open vSwitch switch -- 1.8.5.3 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Processed: make similar bug reports against the same package
Processing commands for cont...@bugs.debian.org: > # 740983, for Linux 3.13, is against openvswitch-datapath-dkms > # 732260, for Linux 3.11, is against openvswitch > # > # They are essentially the same bug, so make them both against > # the same package. > # > # For now, not merging them: 732260 is likely to be fixed earlier > # because OVS 2.1 supports building against 3.11 but not 3.13. > reassign 732260 openvswitch-datapath-dkms Bug #732260 [openvswitch] openvswitch: datapath kernel module fails to compile with linux kernel version 3.11 Bug reassigned from package 'openvswitch' to 'openvswitch-datapath-dkms'. No longer marked as found in versions 1.9.3+git20131029-1.1. Ignoring request to alter fixed versions of bug #732260 to the same values previously set > thanks Stopping processing here. Please contact me if you need assistance. -- 732260: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732260 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] debian: Depend on 'kmod' instead of module-init-tools.
CC: 733...@bugs.debian.org
Reported-by: m...@linux.it (Marco d'Itri)
Signed-off-by: Ben Pfaff
---
AUTHORS| 1 +
debian/control | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/AUTHORS b/AUTHORS
index b189957..e853887 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -209,6 +209,7 @@ Len Gao l...@vmware.com
Logan Rosen logatron...@gmail.com
Luca Falavigna dktrkr...@debian.org
Luiz Henrique Ozaki luiz.oz...@gmail.com
+Marco d'Itrim...@linux.it
Maxime Brun m.b...@alphalink.fr
Michael A. Collins mike.a.coll...@ark-net.org
Michael Hu m...@nicira.com
diff --git a/debian/control b/debian/control
index b3e89df..1aed8e7 100644
--- a/debian/control
+++ b/debian/control
@@ -65,7 +65,7 @@ Description: Open vSwitch common components
Package: openvswitch-switch
Architecture: linux-any
Suggests: openvswitch-datapath-module
-Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
openvswitch-common (= ${binary:Version}), module-init-tools, procps,
uuid-runtime, netbase, python-argparse
+Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
openvswitch-common (= ${binary:Version}), kmod, procps, uuid-runtime, netbase,
python-argparse
Description: Open vSwitch switch implementations
Open vSwitch is a production quality, multilayer, software-based,
Ethernet virtual switch. It is designed to enable massive network
--
1.8.5.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovstest 1/2] unit-test: Add ovstest
On Sun, Mar 30, 2014 at 06:45:09PM -0700, Andy Zhou wrote:
> Changing one of the files in the Open vSwitch ``lib'' directory
> causes 43 binaries to be relinked, which takes a lot of time even with
> parallel ``make''. 31 of those binaries are in the ``tests''
> directory. ovs-test attemps to combine most of those binaries into a
> single test program that just takes a subcommand name as its first
> command-line argument.
>
> The following patch makes use of this infrastructure.
>
> Signed-off-by: Andy Zhou
I like it.
The user interface could be improved a little. Running without any
arguments at all exits without doing anything and a status of zero, but
I'd prefer that it prints a message on stderr and exits with a nonzero
status. (Otherwise this could look like a "false pass" if some test has
a bug that passes no arguments to ovstest.)
The --help output is not very helpful, I would provide a little more
context:
--help, 0, 0
The OVSTEST_REGISTER macro includes a cast for the function type. I
think it would be better to require the function to have the correct
type, that is, to omit the cast:
> +#define OVSTEST_REGISTER(name, function, sub_commands) \
> +OVS_CONSTRUCTOR(register_##function) { \
> +ovstest_register(#name, (ovstest_func)function, \
> + sub_commands); \
> +}
Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovstest 2/2] unit-test: merge test-heap into ovstest
On Sun, Mar 30, 2014 at 06:45:10PM -0700, Andy Zhou wrote: > Modify test-heap.c to use ovstest framework. > > Signed-off-by: Andy Zhou Looking at this, I think it would be better to require the test name to be passed in as a string, that is, "test-heap" instead of test-heap. It looks less odd to the casual reader: > +OVSTEST_REGISTER(test-heap, test_heap_main, commands); Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovstest v2 2/2] unit-test: merge test-heap into ovstest
On Sun, Mar 30, 2014 at 07:15:47PM -0700, Andy Zhou wrote: > Modify test-heap.c to use ovstest framework. > > Signed-off-by: Andy Zhou > > --- > v1-v2: Coding style fixes I inadvertently reviewed v1. I guess v1->v2 isn't a big deal so feel free to use my original reviews. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Processing of openvswitch_2.1.0+git20140325-1_i386.changes
openvswitch_2.1.0+git20140325-1_i386.changes uploaded successfully to localhost along with the files: openvswitch_2.1.0+git20140325-1.dsc openvswitch_2.1.0+git20140325.orig.tar.xz openvswitch_2.1.0+git20140325-1.debian.tar.xz openvswitch-common_2.1.0+git20140325-1_i386.deb openvswitch-switch_2.1.0+git20140325-1_i386.deb openvswitch-ipsec_2.1.0+git20140325-1_i386.deb openvswitch-dbg_2.1.0+git20140325-1_i386.deb openvswitch-vtep_2.1.0+git20140325-1_i386.deb openvswitch-datapath-source_2.1.0+git20140325-1_all.deb openvswitch-datapath-dkms_2.1.0+git20140325-1_all.deb openvswitch-pki_2.1.0+git20140325-1_all.deb python-openvswitch_2.1.0+git20140325-1_all.deb ovsdbmonitor_2.1.0+git20140325-1_all.deb openvswitch-test_2.1.0+git20140325-1_all.deb Greetings, Your Debian queue daemon (running on host franck.debian.org) ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] openvswitch_2.1.0+git20140325-1_i386.changes is NEW
binary:openvswitch-vtep is NEW. Your package contains new components which requires manual editing of the override file. It is ok otherwise, so please be patient. New packages are usually added to the override file about once a week. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v6 4/6] datapath: Minimize ovs_flow_cmd_del critical section.
ovs_flow_cmd_del() now allocates reply (if needed) after the flow has
already been removed from the flow table. If the reply allocation
fails, a netlink error is signaled with netlink_set_err(), as is
already done in ovs_flow_cmd_new_or_set() in the similar situation.
Signed-off-by: Jarno Rajahalme
---
datapath/datapath.c | 57 +++
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 6f818f4..aa0ce9e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -986,51 +986,54 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
struct sw_flow_match match;
int err;
+ if (likely(a[OVS_FLOW_ATTR_KEY])) {
+ ovs_match_init(&match, &key, NULL);
+ err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL);
+ if (unlikely(err))
+ return err;
+ }
+
ovs_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
- if (!dp) {
+ if (unlikely(!dp)) {
err = -ENODEV;
goto unlock;
}
-
- if (!a[OVS_FLOW_ATTR_KEY]) {
+ if (unlikely(!a[OVS_FLOW_ATTR_KEY])) {
err = ovs_flow_tbl_flush(&dp->table);
goto unlock;
}
-
- ovs_match_init(&match, &key, NULL);
- err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL);
- if (err)
- goto unlock;
-
flow = ovs_flow_tbl_lookup(&dp->table, &key);
- if (!flow || !ovs_flow_cmp_unmasked_key(flow, &match)) {
+ if (unlikely(!flow || !ovs_flow_cmp_unmasked_key(flow, &match))) {
err = -ENOENT;
goto unlock;
}
- reply = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info,
- false);
- if (IS_ERR(reply)) {
- err = PTR_ERR(reply);
- goto unlock;
- }
-
ovs_flow_tbl_remove(&dp->table, flow);
+ ovs_unlock();
- if (reply) {
- err = ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex,
-reply, info->snd_portid,
-info->snd_seq, 0,
-OVS_FLOW_CMD_DEL);
- BUG_ON(err < 0);
+ reply = ovs_flow_cmd_alloc_info((const struct sw_flow_actions __force
*)flow->sf_acts,
+ info, false);
+
+ if (likely(reply)) {
+ if (likely(!IS_ERR(reply))) {
+ rcu_read_lock(); /* Keep RCU checker happy. */
+ err = ovs_flow_cmd_fill_info(flow,
+ovs_header->dp_ifindex,
+reply, info->snd_portid,
+info->snd_seq, 0,
+OVS_FLOW_CMD_DEL);
+ rcu_read_unlock();
+ BUG_ON(err < 0);
+ ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
+ } else {
+ netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
+ ovs_dp_flow_multicast_group.id,
+ PTR_ERR(reply));
+ }
}
ovs_flow_free(flow, true);
- ovs_unlock();
-
- if (reply)
- ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
return 0;
unlock:
ovs_unlock();
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v6 2/6] datapath/flow: Fix ovs_flow_stats_get/clear RCU dereference.
For ovs_flow_stats_get() using ovsl_dereference() was wrong, since
flow dumps call this with RCU read lock.
ovs_flow_stats_clear() is always called with ovs_mutex, so can use
ovsl_dereference().
Also, make the ovs_flow_stats_get() 'flow' argument const to make
later patches cleaner.
Signed-off-by: Jarno Rajahalme
---
datapath/flow.c | 10 ++
datapath/flow.h |6 +++---
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/datapath/flow.c b/datapath/flow.c
index 5d1d2f2..f270f3a 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -123,8 +123,9 @@ unlock:
spin_unlock(&stats->lock);
}
-/* Called with ovs_mutex. */
-void ovs_flow_stats_get(struct sw_flow *flow, struct ovs_flow_stats *ovs_stats,
+/* Must be called with rcu_read_lock or ovs_mutex. */
+void ovs_flow_stats_get(const struct sw_flow *flow,
+ struct ovs_flow_stats *ovs_stats,
unsigned long *used, __be16 *tcp_flags)
{
int node;
@@ -134,7 +135,7 @@ void ovs_flow_stats_get(struct sw_flow *flow, struct
ovs_flow_stats *ovs_stats,
memset(ovs_stats, 0, sizeof(*ovs_stats));
for_each_node(node) {
- struct flow_stats *stats = ovsl_dereference(flow->stats[node]);
+ struct flow_stats *stats =
rcu_dereference_ovsl(flow->stats[node]);
if (stats) {
/* Local CPU may write on non-local stats, so we must
@@ -151,12 +152,13 @@ void ovs_flow_stats_get(struct sw_flow *flow, struct
ovs_flow_stats *ovs_stats,
}
}
+/* Called with ovs_mutex. */
void ovs_flow_stats_clear(struct sw_flow *flow)
{
int node;
for_each_node(node) {
- struct flow_stats *stats = rcu_dereference(flow->stats[node]);
+ struct flow_stats *stats = ovsl_dereference(flow->stats[node]);
if (stats) {
spin_lock_bh(&stats->lock);
diff --git a/datapath/flow.h b/datapath/flow.h
index 5587577..5043a6e 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -182,10 +182,10 @@ struct arp_eth_header {
unsigned char ar_tip[4]; /* target IP address*/
} __packed;
-void ovs_flow_stats_update(struct sw_flow *flow, struct sk_buff *skb);
-void ovs_flow_stats_get(struct sw_flow *flow, struct ovs_flow_stats *stats,
+void ovs_flow_stats_update(struct sw_flow *, struct sk_buff *);
+void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
unsigned long *used, __be16 *tcp_flags);
-void ovs_flow_stats_clear(struct sw_flow *flow);
+void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *);
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v6 1/6] datapath: Fix typo.
Incorrect struct name was confusing, even though otherwise
inconsequental.
Signed-off-by: Jarno Rajahalme
---
datapath/flow_table.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index a680895..159572b 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -141,7 +141,7 @@ static void flow_free(struct sw_flow *flow)
{
int node;
- kfree((struct sf_flow_acts __force *)flow->sf_acts);
+ kfree((struct sw_flow_actions __force *)flow->sf_acts);
for_each_node(node)
if (flow->stats[node])
kmem_cache_free(flow_stats_cache,
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v6 5/6] datapath: Split ovs_flow_cmd_new_or_set().
Following patch will be easier to reason about with separate
ovs_flow_cmd_new() and ovs_flow_cmd_set() functions.
Signed-off-by: Jarno Rajahalme
---
datapath/datapath.c | 161 +--
1 file changed, 117 insertions(+), 44 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index aa0ce9e..f88147e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -794,16 +794,16 @@ static struct sk_buff *ovs_flow_cmd_build_info(const
struct sw_flow *flow,
return skb;
}
-static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
+static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
{
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key, masked_key;
- struct sw_flow *flow = NULL;
+ struct sw_flow *flow;
struct sw_flow_mask mask;
struct sk_buff *reply;
struct datapath *dp;
- struct sw_flow_actions *acts = NULL;
+ struct sw_flow_actions *acts;
struct sw_flow_match match;
int error;
@@ -819,23 +819,21 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
goto error;
/* Validate actions. */
- if (a[OVS_FLOW_ATTR_ACTIONS]) {
- acts =
ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
- error = PTR_ERR(acts);
- if (IS_ERR(acts))
- goto error;
+ error = -EINVAL;
+ if (!a[OVS_FLOW_ATTR_ACTIONS])
+ goto error;
- ovs_flow_mask_key(&masked_key, &key, &mask);
- error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
-&masked_key, 0, &acts);
- if (error) {
- OVS_NLERR("Flow actions may not be safe on all matching
packets.\n");
- goto err_kfree;
- }
- } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
- /* OVS_FLOW_CMD_NEW must have actions. */
- error = -EINVAL;
+ acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
+ error = PTR_ERR(acts);
+ if (IS_ERR(acts))
goto error;
+
+ ovs_flow_mask_key(&masked_key, &key, &mask);
+ error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
+&masked_key, 0, &acts);
+ if (error) {
+ OVS_NLERR("Flow actions may not be safe on all matching
packets.\n");
+ goto err_kfree;
}
ovs_lock();
@@ -847,11 +845,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
/* Check if this is a duplicate flow */
flow = ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow) {
- /* Bail out if we're not allowed to create a new flow. */
- error = -ENOENT;
- if (info->genlhdr->cmd == OVS_FLOW_CMD_SET)
- goto err_unlock_ovs;
-
/* Allocate flow. */
flow = ovs_flow_alloc();
if (IS_ERR(flow)) {
@@ -869,11 +862,9 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
acts = NULL;
goto err_flow_free;
}
-
- reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
- info, OVS_FLOW_CMD_NEW, false);
} else {
- /* We found a matching flow. */
+ struct sw_flow_actions *old_acts;
+
/* Bail out if we're not allowed to modify an existing flow.
* We accept NLM_F_CREATE in place of the intended NLM_F_EXCL
* because Generic Netlink treats the latter as a dump
@@ -881,30 +872,114 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
* gets fixed.
*/
error = -EEXIST;
- if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
- info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
+ if (info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
goto err_unlock_ovs;
/* The unmasked key has to be the same for flow updates. */
if (!ovs_flow_cmp_unmasked_key(flow, &match))
goto err_unlock_ovs;
- /* Update actions, if present. */
- if (acts) {
- struct sw_flow_actions *old_acts;
+ /* Update actions. */
+ old_acts = ovsl_dereference(flow->sf_acts);
+ rcu_assign_pointer(flow->sf_acts, acts);
+ ovs_nla_free_flow_actions(old_acts);
+ }
+
+ reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifin
[ovs-dev] [PATCH v6 3/6] datapath: Reduce locking requirements.
Reduce and clarify locking requirements for ovs_flow_cmd_alloc_info(),
ovs_flow_cmd_fill_info() and ovs_flow_cmd_build_info().
A datapath pointer is available only when holding a lock. Change
ovs_flow_cmd_fill_info() and ovs_flow_cmd_build_info() to take a
dp_ifindex directly, rather than a datapath pointer that is then
(only) used to get the dp_ifindex. This is useful, since the
dp_ifindex is available even when the datapath pointer is not, both
before and after taking a lock, which makes further critical section
reduction possible.
Make ovs_flow_cmd_alloc_info() take an 'acts' argument instead a
'flow' pointer. This allows some future patches to do the allocation
before acquiring the flow pointer.
The locking requirements after this patch are:
ovs_flow_cmd_alloc_info(): May be called without locking, must not be
called while holding the RCU read lock (due to memory allocation).
If 'acts' belong to a flow in the flow table, however, then the
caller must hold ovs_mutex.
ovs_flow_cmd_fill_info(): Either ovs_mutex or RCU read lock must be held.
ovs_flow_cmd_build_info(): This calls both of the above, so the caller
must hold ovs_mutex.
Signed-off-by: Jarno Rajahalme
---
datapath/datapath.c | 54 +++
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 0c77045..6f818f4 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -664,7 +664,7 @@ static size_t ovs_flow_cmd_msg_size(const struct
sw_flow_actions *acts)
}
/* Called with ovs_mutex or RCU read lock. */
-static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
+static int ovs_flow_cmd_fill_info(const struct sw_flow *flow, int dp_ifindex,
struct sk_buff *skb, u32 portid,
u32 seq, u32 flags, u8 cmd)
{
@@ -681,7 +681,7 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
if (!ovs_header)
return -EMSGSIZE;
- ovs_header->dp_ifindex = get_dpifindex(dp);
+ ovs_header->dp_ifindex = dp_ifindex;
/* Fill flow key. */
nla = nla_nest_start(skb, OVS_FLOW_ATTR_KEY);
@@ -704,6 +704,7 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
nla_nest_end(skb, nla);
ovs_flow_stats_get(flow, &stats, &used, &tcp_flags);
+
if (used &&
nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used)))
goto nla_put_failure;
@@ -731,9 +732,9 @@ static int ovs_flow_cmd_fill_info(struct sw_flow *flow,
struct datapath *dp,
const struct sw_flow_actions *sf_acts;
sf_acts = rcu_dereference_ovsl(flow->sf_acts);
-
err = ovs_nla_put_actions(sf_acts->actions,
sf_acts->actions_len, skb);
+
if (!err)
nla_nest_end(skb, start);
else {
@@ -754,20 +755,17 @@ error:
return err;
}
-/* Must be called with ovs_mutex. */
-static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
+/* May not be called with RCU read lock. */
+static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions
*acts,
struct genl_info *info,
bool always)
{
struct sk_buff *skb;
- size_t len;
if (!always && !ovs_must_notify(info, &ovs_dp_flow_multicast_group))
return NULL;
- len = ovs_flow_cmd_msg_size(ovsl_dereference(flow->sf_acts));
-
- skb = genlmsg_new_unicast(len, info, GFP_KERNEL);
+ skb = genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info,
GFP_KERNEL);
if (!skb)
return ERR_PTR(-ENOMEM);
@@ -775,21 +773,23 @@ static struct sk_buff *ovs_flow_cmd_alloc_info(struct
sw_flow *flow,
return skb;
}
-/* Must be called with ovs_mutex. */
-static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
- struct datapath *dp,
- struct genl_info *info,
- u8 cmd, bool always)
+/* Called with ovs_mutex. */
+static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
+ int dp_ifindex,
+ struct genl_info *info, u8 cmd,
+ bool always)
{
struct sk_buff *skb;
int retval;
- skb = ovs_flow_cmd_alloc_info(flow, info, always);
+ skb = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info,
+ always);
if (!skb || IS_ERR(skb))
return skb;
- retval = ovs_flow_cmd_fill_info(flow, dp, skb, info->snd_portid
[ovs-dev] [PATCH v6 6/6] datapath: Minimize ovs_flow_cmd_new|set critical sections.
Signed-off-by: Jarno Rajahalme
---
v6: Use key fields of the newly allocated flow directly in ovs_flow_cmd_new().
Correctly handle the case of no acts in ovs_flow_cmd_set().
datapath/datapath.c | 193 ++-
1 file changed, 115 insertions(+), 78 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index f88147e..52c76b4 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -798,8 +798,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct
genl_info *info)
{
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
- struct sw_flow_key key, masked_key;
- struct sw_flow *flow;
+ struct sw_flow *flow, *new_flow;
struct sw_flow_mask mask;
struct sk_buff *reply;
struct datapath *dp;
@@ -807,61 +806,76 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct
genl_info *info)
struct sw_flow_match match;
int error;
- /* Extract key. */
+ /* Must have key and actions. */
error = -EINVAL;
if (!a[OVS_FLOW_ATTR_KEY])
goto error;
+ if (!a[OVS_FLOW_ATTR_ACTIONS])
+ goto error;
- ovs_match_init(&match, &key, &mask);
+ /* Most of the time we need to allocate a new flow, do it before
+* locking. */
+ new_flow = ovs_flow_alloc();
+ if (IS_ERR(new_flow)) {
+ error = PTR_ERR(new_flow);
+ goto error;
+ }
+
+ /* Extract key. */
+ ovs_match_init(&match, &new_flow->unmasked_key, &mask);
error = ovs_nla_get_match(&match,
a[OVS_FLOW_ATTR_KEY], a[OVS_FLOW_ATTR_MASK]);
if (error)
- goto error;
+ goto err_kfree_flow;
- /* Validate actions. */
- error = -EINVAL;
- if (!a[OVS_FLOW_ATTR_ACTIONS])
- goto error;
+ ovs_flow_mask_key(&new_flow->key, &new_flow->unmasked_key, &mask);
+ /* Validate actions. */
acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
error = PTR_ERR(acts);
if (IS_ERR(acts))
- goto error;
+ goto err_kfree_flow;
- ovs_flow_mask_key(&masked_key, &key, &mask);
- error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
-&masked_key, 0, &acts);
+ error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &new_flow->key,
+0, &acts);
if (error) {
OVS_NLERR("Flow actions may not be safe on all matching
packets.\n");
- goto err_kfree;
+ goto err_kfree_acts;
+ }
+
+ reply = ovs_flow_cmd_alloc_info(acts, info, false);
+ if (IS_ERR(reply)) {
+ error = PTR_ERR(reply);
+ goto err_kfree_acts;
}
ovs_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
- error = -ENODEV;
- if (!dp)
+ if (unlikely(!dp)) {
+ error = -ENODEV;
goto err_unlock_ovs;
-
+ }
/* Check if this is a duplicate flow */
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
- if (!flow) {
- /* Allocate flow. */
- flow = ovs_flow_alloc();
- if (IS_ERR(flow)) {
- error = PTR_ERR(flow);
- goto err_unlock_ovs;
- }
-
- flow->key = masked_key;
- flow->unmasked_key = key;
- rcu_assign_pointer(flow->sf_acts, acts);
+ flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->unmasked_key);
+ if (likely(!flow)) {
+ rcu_assign_pointer(new_flow->sf_acts, acts);
/* Put flow in bucket. */
- error = ovs_flow_tbl_insert(&dp->table, flow, &mask);
- if (error) {
+ error = ovs_flow_tbl_insert(&dp->table, new_flow, &mask);
+ if (unlikely(error)) {
acts = NULL;
- goto err_flow_free;
+ goto err_unlock_ovs;
}
+
+ if (unlikely(reply)) {
+ error = ovs_flow_cmd_fill_info(new_flow,
+ ovs_header->dp_ifindex,
+ reply, info->snd_portid,
+ info->snd_seq, 0,
+ OVS_FLOW_CMD_NEW);
+ BUG_ON(error < 0);
+ }
+ ovs_unlock();
} else {
struct sw_flow_actions *old_acts;
@@ -871,40 +885,45 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct
genl_info *info)
* request. We also accept NLM_F_EXCL in case that bug ever
* gets fixed.
[ovs-dev] [PATCH] vswitchd: Add external_ids to Flow_Table table in database schema.
Every other table has an external_ids column, which can be useful to
controller writers for integration purposes, so add one to Flow_Table also.
Reported-by: Ariel Tubaltsev
Signed-off-by: Ben Pfaff
---
AUTHORS|1 +
vswitchd/vswitch.ovsschema |9 ++---
vswitchd/vswitch.xml |7 +++
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index b189957..89a9ad8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -146,6 +146,7 @@ Andrei Andone andrei.and...@softvision.ro
Anshuman Manral anshuman.man...@outlook.com
Anton Matsiuk anton.mats...@gmail.com
Anuprem Chalvadiachalv...@vmware.com
+Ariel Tubaltsev atubalt...@vmware.com
Atzm Watanabe a...@stratosphere.co.jp
Bastian Blank wa...@debian.org
Ben Basler bbas...@nicira.com
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index efaa1da..3fb45d1 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
- "version": "7.4.0",
- "cksum": "2387737815 20431",
+ "version": "7.5.0",
+ "cksum": "1448369194 20560",
"tables": {
"Open_vSwitch": {
"columns": {
@@ -303,7 +303,10 @@
"groups": {
"type": {"key": "string", "min": 0, "max": "unlimited"}},
"prefixes": {
- "type": {"key": "string", "min": 0, "max": 3,
+ "type": {"key": "string", "min": 0, "max": 3}},
+ "external_ids": {
+ "type": {"key": "string", "value": "string",
+ "min": 0, "max": "unlimited",
"QoS": {
"columns": {
"type": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b0a8577..78594e7 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2628,6 +2628,13 @@
one flow table. Currently this limit is 3.
+
+
+ The overall purpose of these columns is described under Common
+ Columns at the beginning of this document.
+
+
+
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [RFC PATCH 4/4] lib/ofpbuf: Compact
On Sat, Mar 29, 2014 at 05:47:18PM -0700, Jarno Rajahalme wrote:
> Thanks for the review! Pushed to master with the proposed changes. See a
> small comment below, though.
>
> Jarno
>
> On Mar 25, 2014, at 5:16 PM, Ben Pfaff wrote:
>
> > On Mon, Mar 24, 2014 at 10:59:06AM -0700, Jarno Rajahalme wrote:
> >> This patch shrinks the struct ofpbuf from 104 to 48 bytes on 64-bit
> >> systems, or from 52 to 36 bytes on 32-bit systems (counting in the
> >> 'l7' removal by the previous patch). This may help contribute to
> >> cache efficiency, and will speed up initializing, copying and
> >> manipulating ofpbufs. This is potentially important for the DPDK
> >> datapath, but the rest of the code base may also see a little benefit.
> >>
> >> Changes are:
> >>
> >> - Remove 'l7' pointer (previous patch).
> >> - Use offsets instead of layer pointers for l2_5, l3, and l4 using
> >> 'l2' as basis. Usually 'data' is the same as 'l2', but this is not
> >> always the case (e.g., when parsing or constructing a packet), so it
> >> can not be easily used as the offset basis. Also, packet parsing is
> >> faster if we do not need to maintain the offsets each time we pull
> >> data from the ofpbuf.
> >> - Use uint32_t for 'allocated' and 'size', as 2^32 is enough even for
> >> largest possible messages/packets.
> >> - Use packed enum for 'source'.
> >> - Rearrange to avoid unnecessary padding.
> >> - Remove 'private_p', which was used only in two cases, both of which
> >> had the invariant ('l2' == 'data'), so we can temporarily use 'l2'
> >> as a private pointer.
> >>
> >> Signed-off-by: Jarno Rajahalme
> >
> > ofpbuf_resize_l2() and ofpbuf_resize_l2_5() look kind of big to me for
> > inlining.
>
> I moved them to ofpbuf.c. However, looks can be deceiving, as the
> 'increment? is always a compile-time constant, so only one of the ofpbuf
> calls will be compiled in for an inlined function.
I didn't realize that. We can always move them back if it makes a
difference.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] apply on branch-2.1: rhel: Enable DHCP support for internal ports
Done. Sorry it took me a few days. On Tue, Mar 25, 2014 at 05:48:15PM -0300, Flavio Leitner wrote: > > Ben, > > Could you cherry-pick the commit below to branch-2.1 as well? > > commit 490db96efaf89c63656b192d5ca287b0908a6c77 > Author: Flavio Leitner > Date: Tue Jan 14 00:22:07 2014 -0200 > > rhel: Enable DHCP support for internal ports. > > The current initscripts ifup-ovs brings up internal ports as > an ordinary ethernet device, so BOOTPROTO=dhcp|none does not > consider any OVS/bridge detail. > > Since DHCP requires a port in the bridge to reach the server, > bring up the required port before in the same way it does for > OVS bridge. > > Signed-off-by: Flavio Leitner > Signed-off-by: Gurucharan Shetty > > Thanks > fbl ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] bridge: don't bring up internal ports by default.
On Tue, Mar 25, 2014 at 02:50:17PM -0300, Flavio Leitner wrote: > It should be an administrator task to bring up devices as they > are configured properly. > > Currently, Fedora is deleting the bridges when the interface is > brought down. Therefore, there is no bridge on the next boot and > the initscripts can apply the networking configuration properly > for a new bridge. > > However, if the system didn't execute ifdown for some reason, the > bridge is left in the ovsdb and since internal ports are brought > up by default, there is no way for initscripts to known if the > adminitrator has configured it or not already. > > This patch partially reverts the commit > bef071a5fdf8e2dd87677b04b3cf7a8f5094edcb > > Signed-off-by: Flavio Leitner I think you're right, that this is correct, but let's put the NEWS item at the top and give a little more explanation, something like this: - Internal ports are no longer brought up by default, because it should be an administrator task to bring up devices as they are configured properly. This is almost certain to break some people's setups, but I guess it's better than what we do now. Please give a title when referring to a commit in a log message, e.g.: This patch partially reverts the commit bef071a5fdf8e2dd87677b04b3cf7a8f5094edcb (bridge: Always "up" internal devices.). Finally, I would have just fixed all of the above and pushed it, except that it also breaks half a dozen tests in the testsuite. Please fix those and submit a v3. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vswitchd: Add external_ids to Flow_Table table in database schema.
Acked-by: Justin Pettit
--Justin
Ben Pfaff wrote:
Every other table has an external_ids column, which can be useful to
controller writers for integration purposes, so add one to Flow_Table also.
Reported-by: Ariel Tubaltsev
Signed-off-by: Ben Pfaff
---
AUTHORS|1 +
vswitchd/vswitch.ovsschema |9 ++---
vswitchd/vswitch.xml |7 +++
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index b189957..89a9ad8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -146,6 +146,7 @@ Andrei Andone andrei.and...@softvision.ro
Anshuman Manral anshuman.man...@outlook.com
Anton Matsiuk anton.mats...@gmail.com
Anuprem Chalvadiachalv...@vmware.com
+Ariel Tubaltsev atubalt...@vmware.com
Atzm Watanabe a...@stratosphere.co.jp
Bastian Blank wa...@debian.org
Ben Basler bbas...@nicira.com
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index efaa1da..3fb45d1 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
- "version": "7.4.0",
- "cksum": "2387737815 20431",
+ "version": "7.5.0",
+ "cksum": "1448369194 20560",
"tables": {
"Open_vSwitch": {
"columns": {
@@ -303,7 +303,10 @@
"groups": {
"type": {"key": "string", "min": 0, "max": "unlimited"}},
"prefixes": {
- "type": {"key": "string", "min": 0, "max": 3,
+ "type": {"key": "string", "min": 0, "max": 3}},
+ "external_ids": {
+ "type": {"key": "string", "value": "string",
+ "min": 0, "max": "unlimited",
"QoS": {
"columns": {
"type": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b0a8577..78594e7 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2628,6 +2628,13 @@
one flow table. Currently this limit is 3.
+
+
+ The overall purpose of these columns is described underCommon
+ Columns at the beginning of this document.
+
+
+
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] debian: Depend on 'kmod' instead of module-init-tools.
Acked-by: Justin Pettit
--Justin
Ben Pfaff wrote:
CC: 733...@bugs.debian.org
Reported-by: m...@linux.it (Marco d'Itri)
Signed-off-by: Ben Pfaff
---
AUTHORS| 1 +
debian/control | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/AUTHORS b/AUTHORS
index b189957..e853887 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -209,6 +209,7 @@ Len Gao l...@vmware.com
Logan Rosen logatron...@gmail.com
Luca Falavigna dktrkr...@debian.org
Luiz Henrique Ozaki luiz.oz...@gmail.com
+Marco d'Itrim...@linux.it
Maxime Brun m.b...@alphalink.fr
Michael A. Collins mike.a.coll...@ark-net.org
Michael Hu m...@nicira.com
diff --git a/debian/control b/debian/control
index b3e89df..1aed8e7 100644
--- a/debian/control
+++ b/debian/control
@@ -65,7 +65,7 @@ Description: Open vSwitch common components
Package: openvswitch-switch
Architecture: linux-any
Suggests: openvswitch-datapath-module
-Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
openvswitch-common (= ${binary:Version}), module-init-tools, procps,
uuid-runtime, netbase, python-argparse
+Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
openvswitch-common (= ${binary:Version}), kmod, procps, uuid-runtime, netbase,
python-argparse
Description: Open vSwitch switch implementations
Open vSwitch is a production quality, multilayer, software-based,
Ethernet virtual switch. It is designed to enable massive network
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [patch net-next RFC v2 2/6] skbuff: add "missed_flow" flag
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 03db95a..0100c2f 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -567,6 +567,7 @@ struct sk_buff {
>* headers if needed
>*/
> __u8encapsulation:1;
> + __u8missed_flow:1;
> /* 6/8 bit hole (depending on ndisc_nodetype presence) */
> kmemcheck_bitfield_end(flags2);
Presumably you need to update the "6/8 bit hole" comment here.
Regards,
Neil
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] debian: Depend on 'kmod' instead of module-init-tools.
Thanks, applied to master and branch-2.1.
On Mon, Mar 31, 2014 at 01:29:06PM -0700, Justin Pettit wrote:
> Acked-by: Justin Pettit
>
> --Justin
>
>
> Ben Pfaff wrote:
> >CC: 733...@bugs.debian.org
> >Reported-by: m...@linux.it (Marco d'Itri)
> >Signed-off-by: Ben Pfaff
> >---
> > AUTHORS| 1 +
> > debian/control | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/AUTHORS b/AUTHORS
> >index b189957..e853887 100644
> >--- a/AUTHORS
> >+++ b/AUTHORS
> >@@ -209,6 +209,7 @@ Len Gao l...@vmware.com
> > Logan Rosen logatron...@gmail.com
> > Luca Falavigna dktrkr...@debian.org
> > Luiz Henrique Ozaki luiz.oz...@gmail.com
> >+Marco d'Itrim...@linux.it
> > Maxime Brun m.b...@alphalink.fr
> > Michael A. Collins mike.a.coll...@ark-net.org
> > Michael Hu m...@nicira.com
> >diff --git a/debian/control b/debian/control
> >index b3e89df..1aed8e7 100644
> >--- a/debian/control
> >+++ b/debian/control
> >@@ -65,7 +65,7 @@ Description: Open vSwitch common components
> > Package: openvswitch-switch
> > Architecture: linux-any
> > Suggests: openvswitch-datapath-module
> >-Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
> >openvswitch-common (= ${binary:Version}), module-init-tools, procps,
> >uuid-runtime, netbase, python-argparse
> >+Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends},
> >openvswitch-common (= ${binary:Version}), kmod, procps, uuid-runtime,
> >netbase, python-argparse
> > Description: Open vSwitch switch implementations
> > Open vSwitch is a production quality, multilayer, software-based,
> > Ethernet virtual switch. It is designed to enable massive network
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] vswitchd: Add external_ids to Flow_Table table in database schema.
Thanks, applied to master.
On Mon, Mar 31, 2014 at 01:25:18PM -0700, Justin Pettit wrote:
> Acked-by: Justin Pettit
>
> --Justin
>
>
> Ben Pfaff wrote:
> >Every other table has an external_ids column, which can be useful to
> >controller writers for integration purposes, so add one to Flow_Table also.
> >
> >Reported-by: Ariel Tubaltsev
> >Signed-off-by: Ben Pfaff
> >---
> > AUTHORS|1 +
> > vswitchd/vswitch.ovsschema |9 ++---
> > vswitchd/vswitch.xml |7 +++
> > 3 files changed, 14 insertions(+), 3 deletions(-)
> >
> >diff --git a/AUTHORS b/AUTHORS
> >index b189957..89a9ad8 100644
> >--- a/AUTHORS
> >+++ b/AUTHORS
> >@@ -146,6 +146,7 @@ Andrei Andone andrei.and...@softvision.ro
> > Anshuman Manral anshuman.man...@outlook.com
> > Anton Matsiuk anton.mats...@gmail.com
> > Anuprem Chalvadiachalv...@vmware.com
> >+Ariel Tubaltsev atubalt...@vmware.com
> > Atzm Watanabe a...@stratosphere.co.jp
> > Bastian Blank wa...@debian.org
> > Ben Basler bbas...@nicira.com
> >diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
> >index efaa1da..3fb45d1 100644
> >--- a/vswitchd/vswitch.ovsschema
> >+++ b/vswitchd/vswitch.ovsschema
> >@@ -1,6 +1,6 @@
> > {"name": "Open_vSwitch",
> >- "version": "7.4.0",
> >- "cksum": "2387737815 20431",
> >+ "version": "7.5.0",
> >+ "cksum": "1448369194 20560",
> > "tables": {
> > "Open_vSwitch": {
> > "columns": {
> >@@ -303,7 +303,10 @@
> > "groups": {
> > "type": {"key": "string", "min": 0, "max": "unlimited"}},
> > "prefixes": {
> >- "type": {"key": "string", "min": 0, "max": 3,
> >+ "type": {"key": "string", "min": 0, "max": 3}},
> >+ "external_ids": {
> >+ "type": {"key": "string", "value": "string",
> >+ "min": 0, "max": "unlimited",
> > "QoS": {
> > "columns": {
> > "type": {
> >diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> >index b0a8577..78594e7 100644
> >--- a/vswitchd/vswitch.xml
> >+++ b/vswitchd/vswitch.xml
> >@@ -2628,6 +2628,13 @@
> > one flow table. Currently this limit is 3.
> >
> >
> >+
> >+
> >+ The overall purpose of these columns is described underCommon
> >+ Columns at the beginning of this document.
> >+
> >+
> >+
> >
> >
> >
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [patch net-next RFC v2 4/6] net: introduce switchdev API
> diff --git a/net/Kconfig b/net/Kconfig > index e411046..e02ab8d 100644 > --- a/net/Kconfig > +++ b/net/Kconfig > @@ -285,6 +285,16 @@ config NET_FLOW_LIMIT > with many clients some protection against DoS by a single (spoofed) > flow that greatly exceeds average workload. > > +config NET_SWITCHDEV > + tristate "Switch device" > + depends on INET > + ---help--- > + This module provides glue for hardware switch chips so they can be > + accessed from userspace via Open vSwitch Netlink API. > + > + To compile this code as a module, choose M here: the > + module will be called pktgen. > + "pktgen" cut'n'paste typo. Neil ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [patch net-next RFC v2 4/6] net: introduce switchdev API
Hi Jiri,
Apologies for possibly newbie questions here, but:
> +bool swdev_dev_check(const struct net_device *dev)
> +{
> + return dev->priv_flags & IFF_SWITCH;
> +}
> +EXPORT_SYMBOL(swdev_dev_check);
[...]
> + BUG_ON(!swdev_dev_check(dev));
[...]
> +int __swdev_register(struct net_device *dev)
> +{
> + if (dev->priv_flags & IFF_SWITCH) {
> + netdev_err(dev, "Device is already registered as a switch
> device\n");
> + return -EBUSY;
> + }
[...]
1. What's the benefit of requiring and policing the IFF_SWITCH flag, and
requiring a switch netdev to call swdev_register, as opposed to just checking
whether the netdev provides the relevant ops?
> +int swdev_register(struct net_device *dev)
> +{
> + int err;
> +
> + rtnl_lock();
> + err = __swdev_register(dev);
> + rtnl_unlock();
> + return err;
> +}
> +EXPORT_SYMBOL(swdev_register);
2. Why grab and release the rtnl lock, given that there appears to be nothing
rtnl-related in between?
Regards,
Neil
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] apply on branch-2.1: rhel: Enable DHCP support for internal ports
On Mon, Mar 31, 2014 at 01:04:53PM -0700, Ben Pfaff wrote: > Done. Sorry it took me a few days. No worries, thanks! fbl > On Tue, Mar 25, 2014 at 05:48:15PM -0300, Flavio Leitner wrote: > > > > Ben, > > > > Could you cherry-pick the commit below to branch-2.1 as well? > > > > commit 490db96efaf89c63656b192d5ca287b0908a6c77 > > Author: Flavio Leitner > > Date: Tue Jan 14 00:22:07 2014 -0200 > > > > rhel: Enable DHCP support for internal ports. [...] ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] rhel: Add Patch Port support to initscripts
Allows setting up type=patch ports through sysconfig ifcfg-* files.
Signed-off-by: Jason Kölker
---
rhel/README.RHEL | 25 +
rhel/etc_sysconfig_network-scripts_ifdown-ovs | 3 +++
rhel/etc_sysconfig_network-scripts_ifup-ovs | 4
3 files changed, 32 insertions(+)
diff --git a/rhel/README.RHEL b/rhel/README.RHEL
index cb6ab88..2620674 100644
--- a/rhel/README.RHEL
+++ b/rhel/README.RHEL
@@ -25,6 +25,8 @@ assignments. The following OVS-specific variable names are
supported:
* "OVSTunnel", if is an OVS tunnel.
+* "OVSPatchPort", if is a patch port
+
- OVS_BRIDGE: If TYPE is anything other than "OVSBridge", set to
the name of the OVS bridge to which the port should be attached.
@@ -47,6 +49,9 @@ assignments. The following OVS-specific variable names are
supported:
- OVS_TUNNEL_OPTIONS: For "OVSTunnel" interfaces, this field should be
used to specify the tunnel options like remote_ip, key, etc.
+- OVS_PATCH_PEER: For "OVSPatchPort" devices, this field specifies
+ the patch's peer on the other bridge.
+
Note
@@ -182,6 +187,26 @@ OVS_BRIDGE=ovsbridge0
OVS_TUNNEL_TYPE=gre
OVS_TUNNEL_OPTIONS="options:remote_ip=A.B.C.D"
+
+Patch Ports:
+
+==> ifcfg-patch-ovs-0 <==
+DEVICE=patch-ovs-0
+ONBOOT=yes
+DEVICETYPE=ovs
+TYPE=OVSPatchPort
+OVS_BRIDGE=ovsbridge0
+OVS_PATCH_PEER=patch-ovs-1
+
+==> ifcfg-patch-ovs-1 <==
+DEVICE=patch-ovs-1
+ONBOOT=yes
+DEVICETYPE=ovs
+TYPE=OVSPatchPort
+OVS_BRIDGE=ovsbridge1
+OVS_PATCH_PEER=patch-ovs-0
+
+
Reporting Bugs
--
diff --git a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
index daa5786..6e96d62 100755
--- a/rhel/etc_sysconfig_network-scripts_ifdown-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifdown-ovs
@@ -56,6 +56,9 @@ case "$TYPE" in
retval=$?
ovs-vsctl -t ${TIMEOUT} -- --if-exists del-port "$OVS_BRIDGE"
"$DEVICE"
;;
+ OVSPatchPort)
+ ovs-vsctl -t ${TIMEOUT} -- --if-exists del-port "$OVS_BRIDGE"
"$DEVICE"
+ ;;
*)
echo $"Invalid OVS interface type $TYPE"
exit 1
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs
b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index 0ee7b21..45143f6 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -136,6 +136,10 @@ case "$TYPE" in
ovs-vsctl -t ${TIMEOUT} -- --may-exist add-port "$OVS_BRIDGE"
"$DEVICE" $OVS_OPTIONS -- set Interface "$DEVICE" type=$OVS_TUNNEL_TYPE
$OVS_TUNNEL_OPTIONS ${OVS_EXTRA+-- $OVS_EXTRA}
${OTHERSCRIPT} ${CONFIG} ${2}
;;
+ OVSPatchPort)
+ ifup_ovs_bridge
+ ovs-vsctl -t ${TIMEOUT} -- --may-exist add-port "$OVS_BRIDGE"
"$DEVICE" $OVS_OPTIONS -- set Interface "$DEVICE" type=patch
options:peer=${OVS_PATCH_PEER} ${OVS_EXTRA+-- $OVS_EXTRA}
+ ;;
*)
echo $"Invalid OVS interface type $TYPE"
exit 1
--
1.9.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3 04/10] datapath: Compact sw_flow_key.
On Sat, Mar 29, 2014 at 7:33 PM, Jarno Rajahalme wrote: > > On Mar 28, 2014, at 2:52 PM, Jesse Gross wrote: > >> On Fri, Mar 28, 2014 at 8:50 AM, Jarno Rajahalme >> wrote: >>> On Mar 27, 2014, at 3:59 PM, Jesse Gross wrote: > On Fri, Feb 21, 2014 at 11:41 AM, Jarno Rajahalme > wrote: > Minimize padding in sw_flow_key and move 'tp' top the main struct. > These changes simplify code when accessing the transport port numbers > and the tcp flags, and makes the sw_flow_key 8 bytes smaller on 64-bit > systems (128->120 bytes). These changes also make the keys for IPv4 > packets to fit in one cache line. > > There is a valid concern for safety of packing the struct > ovs_key_ipv4_tunnel, as it would be possible to take the address of > the tun_id member as a __be64 * which could result in unaligned access > in some systems. However: > > - sw_flow_key itself is 64-bit aligned, so the tun_id within is always > 64-bit aligned. > - We never make arrays of ovs_key_ipv4_tunnel (which would force every > second tun_key to be misaligned). > - We never take the address of the tun_id in to a __be64 *. > - Whereever we use struct ovs_key_ipv4_tunnel outside the sw_flow_key, > it is in stack (on tunnel input functions), where compiler has full > control of the alignment. I'm not sure that I understand the last comment here. On the stack, the compiler does have control over the layout but it will presumably use the alignment specified here when doing that layout. >>> >>> Maybe the last comment is just redundant, then. >> >> But doesn't it mean that we could now have unaligned accesses? > > > Access to tun_id member can be unaligned, but the compiler should handle it, > so it is safe. It seems we do it exactly once for ingress > (ovs_tun_key_init()) and egress from/to a tunnel in the fast path, so if it > needs to be as two 32-bit units in some targets, it should not matter. > > I see the commit message could have been clearer, though :-( There are two > concerns: safety and performance. As we never pass a tun_id member pointer as > __be64 *, this should be safe. For performance, some architectures must use > multiple instructions to access a “4-aligned” tun_id, but as we have such > accesses once per ingress/egress, it does not matter in practice. Other > architectures use one instruction and face a small penalty for unaligned > access. At the time of writing the commit message I was trying to reason when > that unaligned access would occur. OK, I see now. Thanks for walking through the analysis. > Also, now that I looked into this again, it seems to me that a tun_key in the > action list can be unaligned, as netlink attribute alignment is 4. If set > tunnel action follows pop vlan action, for example, the tun_key would not be > 8-aligned. This would be a safety issue with the tun_key as we had it before > this patch as we do the following in actions.c/execute_set_action(): > > case OVS_KEY_ATTR_IPV4_TUNNEL: > OVS_CB(skb)->tun_key = nla_data(nested_attr); > break; > Hmm, that is an interesting point. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] openvswitch: supply a dummy err_handler of gre_cisco_protocol to prevent kernel crash
On Sun, Mar 30, 2014 at 5:12 AM, wei zhang wrote: > At 2014-03-29 06:02:25,"Jesse Gross" wrote: > >>I'm not sure that rejecting all ICMP packets is the correct thing do >>here since it means that we could pass them onto a later caller even >>though they are intended for us. We should probably use the same logic >>as for receiving packets and just discard them here. > > Thank you very much for your advice, did you mean this logic? Yes, that's what I was thinking. [...] > Maybe I misunderstand something? I think if we discard all packet pass to us > when we use gre vport, new gre_cisco_protocol which has lower priority could > not see the packet intended to it. That's true but in this case it would also not see any data packets, so I don't think that situation would work well anyways. > I checked the implementation of the ipgre_err(), which has be called before > the err_handler of gre vport. It use the the (local address, remote address, > key) > to distinguish the packet which is realy intended to it, although it could not > always get the key from the icmp packet. Should we do as the same as it? > I'm not sure this is feasible, any advice is appreciate. OVS does flow based matching rather than using a static set of configuration parameters, so everything "matches" in some way (although the result might be to drop). This generally means that OVS is the receiver of last resort and nothing currently has a lower priority. That actually means the difference between the patches is somewhat academic but it seems more robust for the logic to be consistent. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [ovstest 1/2] unit-test: Add ovstest
Thanks Ben for the review.
I will push with following incremental changes.
The first two issues, argument check and --help took a bit more code
which I don't feel comfortable check-in without review. I will
incorporate them into the follow on patch series.
index 300a0d9..6d10a5d 100644
--- a/tests/ovstest.h
+++ b/tests/ovstest.h
@@ -73,13 +73,12 @@ ovstest_register(const char *test_name, ovstest_func f,
* // not have sub commands. Otherwise, its command
* // array can replace the NULL here.
*
- * OVSTEST_REGISTER(my-test, my_test_main, NULL);
+ * OVSTEST_REGISTER("my-test", my_test_main, NULL);
*/
#define OVSTEST_REGISTER(name, function, sub_commands) \
OVS_CONSTRUCTOR(register_##function) { \
-ovstest_register(#name, (ovstest_func)function, \
- sub_commands); \
+ovstest_register(name, function, sub_commands); \
}
#endif
diff --git a/tests/test-heap.c b/tests/test-heap.c
index dfb0981..3e0940c 100644
--- a/tests/test-heap.c
+++ b/tests/test-heap.c
@@ -485,4 +485,4 @@ test_heap_main(int argc, char *argv[])
run_command(argc - 1, argv + 1, commands);
}
-OVSTEST_REGISTER(test-heap, test_heap_main, commands);
+OVSTEST_REGISTER("test-heap", test_heap_main, commands);
On Mon, Mar 31, 2014 at 9:16 AM, Ben Pfaff wrote:
> On Sun, Mar 30, 2014 at 06:45:09PM -0700, Andy Zhou wrote:
>> Changing one of the files in the Open vSwitch ``lib'' directory
>> causes 43 binaries to be relinked, which takes a lot of time even with
>> parallel ``make''. 31 of those binaries are in the ``tests''
>> directory. ovs-test attemps to combine most of those binaries into a
>> single test program that just takes a subcommand name as its first
>> command-line argument.
>>
>> The following patch makes use of this infrastructure.
>>
>> Signed-off-by: Andy Zhou
>
> I like it.
>
> The user interface could be improved a little. Running without any
> arguments at all exits without doing anything and a status of zero, but
> I'd prefer that it prints a message on stderr and exits with a nonzero
> status. (Otherwise this could look like a "false pass" if some test has
> a bug that passes no arguments to ovstest.)
>
> The --help output is not very helpful, I would provide a little more
> context:
> --help, 0, 0
>
> The OVSTEST_REGISTER macro includes a cast for the function type. I
> think it would be better to require the function to have the correct
> type, that is, to omit the cast:
>> +#define OVSTEST_REGISTER(name, function, sub_commands) \
>> +OVS_CONSTRUCTOR(register_##function) { \
>> +ovstest_register(#name, (ovstest_func)function, \
>> + sub_commands); \
>> +}
>
> Acked-by: Ben Pfaff
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] rhel: Add Patch Port support to initscripts
On Mon, Mar 31, 2014 at 11:34:14PM +, Jason Kölker wrote: > Allows setting up type=patch ports through sysconfig ifcfg-* files. > > Signed-off-by: Jason Kölker > --- Looks good, works here. Acked-by: Flavio Leitner ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/5] ofpbuf: Add private pointer for dpdk
From: Pravin Shelar
netdev-dpdk uses this pointer to store dpdk mbuf. This patch fixes
compilation error in dpdk.
Signed-off-by: Pravin B Shelar
---
lib/ofpbuf.h |4
1 file changed, 4 insertions(+)
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index 8d1cb11..9becedd 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -22,6 +22,7 @@
#include "list.h"
#include "packets.h"
#include "util.h"
+#include "netdev-dpdk.h"
#ifdef __cplusplus
extern "C" {
@@ -52,6 +53,9 @@ struct ofpbuf {
UINT16_MAX. */
enum ofpbuf_source source; /* Source of memory allocated as 'base'. */
struct list list_node; /* Private list element for use by owner. */
+#ifdef DPDK_NETDEV
+void *private_p;/* private pointer for use by dpdk */
+#endif
};
void * ofpbuf_resize_l2(struct ofpbuf *, int increment);
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 4/5] ofpbuf: Add DPDK mbuf to ofpbuf.
From: Pravin Shelar
Define data, base and size access APIs for DPDK.
Signed-off-by: Pravin B Shelar
---
lib/ofpbuf.h | 47 ++-
1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index df7f189..94651e4 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -39,10 +39,15 @@ enum OVS_PACKED_ENUM ofpbuf_source {
/* Buffer for holding arbitrary data. An ofpbuf is automatically reallocated
* as necessary if it grows too large for the available memory. */
struct ofpbuf {
+#ifdef DPDK_NETDEV
+struct rte_mbuf mbuf; /* DPDK mbuf */
+void *private_p;/* private pointer for use by dpdk */
+#else
void *base; /* First byte of allocated space. */
-uint32_t allocated; /* Number of bytes allocated. */
-uint32_t size; /* Number of bytes in use. */
void *data; /* First byte actually in use. */
+uint32_t size; /* Number of bytes in use. */
+#endif
+uint32_t allocated; /* Number of bytes allocated. */
void *l2; /* Link-level header. */
uint16_t l2_5_ofs; /* MPLS label stack offset from l2, or
@@ -53,9 +58,6 @@ struct ofpbuf {
UINT16_MAX. */
enum ofpbuf_source source; /* Source of memory allocated as 'base'. */
struct list list_node; /* Private list element for use by owner. */
-#ifdef DPDK_NETDEV
-void *private_p;/* private pointer for use by dpdk */
-#endif
};
static inline void * ofpbuf_get_data(const struct ofpbuf *);
@@ -310,6 +312,40 @@ static inline const void *ofpbuf_get_icmp_payload(const
struct ofpbuf *b)
? (const char *)ofpbuf_get_l4(b) + ICMP_HEADER_LEN : NULL;
}
+#ifdef DPDK_NETDEV
+static inline void * ofpbuf_get_data(const struct ofpbuf *b)
+{
+return b->mbuf.pkt.data;
+}
+
+static inline void ofpbuf_set_data(struct ofpbuf *b, void *d)
+{
+b->mbuf.pkt.data = d;
+}
+
+static inline void * ofpbuf_get_base(const struct ofpbuf *b)
+{
+return b->mbuf.buf_addr;
+}
+
+static inline void ofpbuf_set_base(struct ofpbuf *b, void *d)
+{
+b->mbuf.buf_addr = d;
+}
+
+static inline uint32_t ofpbuf_get_size(const struct ofpbuf *b)
+{
+return b->mbuf.pkt.pkt_len;
+}
+
+static inline void ofpbuf_set_size(struct ofpbuf *b, uint32_t v)
+{
+b->mbuf.pkt.data_len = v;/* Current seg length. */
+b->mbuf.pkt.pkt_len = v; /* Total length of all segments linked to
+ * this segment. */
+}
+
+#else
static inline void * ofpbuf_get_data(const struct ofpbuf *b)
{
return b->data;
@@ -339,6 +375,7 @@ static inline void ofpbuf_set_size(struct ofpbuf *b,
uint32_t v)
{
b->size = v;
}
+#endif
#ifdef __cplusplus
}
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 5/5] netdev-dpdk: Remove alloc from packet recv.
From: Pravin Shelar
On DPDK packet recv, ovs is given pointer to mbuf which has
information about a packet, for example pointer to data and size.
By moving mbuf to ofpbuf we can let dpdk allocate ofpbuf and
pass that to ovs for processing the packet.
Signed-off-by: Pravin B Shelar
---
lib/netdev-dpdk.c | 99 +++--
lib/ofpbuf.c |4 +--
lib/ofpbuf.h |6 +++-
3 files changed, 57 insertions(+), 52 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5db4b49..facf220 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -209,16 +209,53 @@ dpdk_rte_mzalloc(size_t sz)
void
free_dpdk_buf(struct ofpbuf *b)
{
-struct rte_mbuf *pkt;
-
-pkt = b->private_p;
-if (!pkt) {
-return;
-}
+struct rte_mbuf *pkt = (struct rte_mbuf *) b;
rte_mempool_put(pkt->pool, pkt);
}
+static void
+__rte_pktmbuf_init(struct rte_mempool *mp,
+ __attribute__((unused)) void *opaque_arg,
+ void *_m,
+ __attribute__((unused)) unsigned i)
+{
+struct rte_mbuf *m = _m;
+uint32_t buf_len = mp->elt_size - sizeof(struct ofpbuf);
+
+RTE_MBUF_ASSERT(mp->elt_size >= sizeof(struct ofpbuf));
+
+memset(m, 0, mp->elt_size);
+
+/* start of buffer is just after mbuf structure */
+m->buf_addr = (char *)m + sizeof(struct ofpbuf);
+m->buf_physaddr = rte_mempool_virt2phy(mp, m) +
+sizeof(struct ofpbuf);
+m->buf_len = (uint16_t)buf_len;
+
+/* keep some headroom between start of buffer and data */
+m->pkt.data = (char*) m->buf_addr + RTE_MIN(RTE_PKTMBUF_HEADROOM,
m->buf_len);
+
+/* init some constant fields */
+m->type = RTE_MBUF_PKT;
+m->pool = mp;
+m->pkt.nb_segs = 1;
+m->pkt.in_port = 0xff;
+}
+
+static void
+ovs_rte_pktmbuf_init(struct rte_mempool *mp,
+ __attribute__((unused)) void *opaque_arg,
+ void *_m,
+ __attribute__((unused)) unsigned i)
+{
+struct rte_mbuf *m = _m;
+
+__rte_pktmbuf_init(mp, opaque_arg, _m, i);
+
+ofpbuf_init_dpdk((struct ofpbuf *) m, m->buf_len);
+}
+
static struct dpdk_mp *
dpdk_mp_get(int socket_id, int mtu) OVS_REQUIRES(dpdk_mutex)
{
@@ -242,7 +279,7 @@ dpdk_mp_get(int socket_id, int mtu) OVS_REQUIRES(dpdk_mutex)
MP_CACHE_SZ,
sizeof(struct rte_pktmbuf_pool_private),
rte_pktmbuf_pool_init, NULL,
- rte_pktmbuf_init, NULL,
+ ovs_rte_pktmbuf_init, NULL,
socket_id, 0);
if (dmp->mp == NULL) {
@@ -550,47 +587,22 @@ dpdk_queue_flush(struct netdev_dpdk *dev, int qid)
rte_spinlock_unlock(&txq->tx_lock);
}
-inline static struct ofpbuf *
-build_ofpbuf(struct rte_mbuf *pkt)
-{
-struct ofpbuf *b;
-
-b = ofpbuf_new(0);
-b->private_p = pkt;
-
-ofpbuf_set_data(b, pkt->pkt.data);
-ofpbuf_set_base(b, (char *)ofpbuf_get_data(b) - DP_NETDEV_HEADROOM -
VLAN_ETH_HEADER_LEN);
-b->allocated = pkt->buf_len;
-ofpbuf_set_size(b, rte_pktmbuf_data_len(pkt));
-b->source = OFPBUF_DPDK;
-
-dp_packet_pad(b);
-
-return b;
-}
-
static int
netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, struct ofpbuf **packet, int *c)
{
struct netdev_rxq_dpdk *rx = netdev_rxq_dpdk_cast(rxq_);
struct netdev *netdev = rx->up.netdev;
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
-struct rte_mbuf *burst_pkts[MAX_RX_QUEUE_LEN];
int nb_rx;
-int i;
dpdk_queue_flush(dev, rxq_->queue_id);
nb_rx = rte_eth_rx_burst(rx->port_id, rxq_->queue_id,
- burst_pkts, MAX_RX_QUEUE_LEN);
+ (struct rte_mbuf **) packet, MAX_RX_QUEUE_LEN);
if (!nb_rx) {
return EAGAIN;
}
-for (i = 0; i < nb_rx; i++) {
-packet[i] = build_ofpbuf(burst_pkts[i]);
-}
-
*c = nb_rx;
return 0;
@@ -677,32 +689,23 @@ netdev_dpdk_send(struct netdev *netdev,
goto out;
}
-rte_prefetch0(&ofpbuf->private_p);
-if (!may_steal ||
-!ofpbuf->private_p || ofpbuf->source != OFPBUF_DPDK) {
+if (!may_steal || ofpbuf->source != OFPBUF_DPDK) {
dpdk_do_tx_copy(netdev, (char *) ofpbuf_get_data(ofpbuf),
ofpbuf_get_size(ofpbuf));
+
+if (may_steal) {
+ofpbuf_delete(ofpbuf);
+}
} else {
-struct rte_mbuf *pkt;
int qid;
-pkt = ofpbuf->private_p;
-ofpbuf->private_p = NULL;
-rte_pktmbuf_data_len(pkt) = ofpbuf_get_size(ofpbuf);
-rte_pktmbuf_pkt_len(pkt) = ofpbuf_get_size(ofpbuf);
-
qid = rte_lcore_id() % NR_QUEUE;
-dpdk_queue_pkt(dev, qid, pkt);
+dpdk_queue_pkt(dev, qid, (struct rte_mbuf *)ofpbuf);
-ofpbuf->private_p = NULL;
}
ret = 0;
out:
-if (may_s
[ovs-dev] [PATCH 3/5] ofpbuf: Add ofpbuf_init_dpdk()
From: Pravin Shelar
Signed-off-by: Pravin B Shelar
---
lib/ofpbuf.c | 27 ++-
lib/ofpbuf.h |2 ++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 8763e49..f616b1c 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -23,6 +23,16 @@
#include "util.h"
static void
+ofpbuf_init__(struct ofpbuf *b, size_t allocated, enum ofpbuf_source source)
+{
+b->allocated = allocated;
+b->source = source;
+b->l2 = NULL;
+b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX;
+list_poison(&b->list_node);
+}
+
+static void
ofpbuf_use__(struct ofpbuf *b, void *base, size_t allocated,
enum ofpbuf_source source)
{
@@ -30,11 +40,7 @@ ofpbuf_use__(struct ofpbuf *b, void *base, size_t allocated,
ofpbuf_set_data(b, base);
ofpbuf_set_size(b, 0);
-b->allocated = allocated;
-b->source = source;
-b->l2 = NULL;
-b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX;
-list_poison(&b->list_node);
+ofpbuf_init__(b, allocated, source);
}
/* Initializes 'b' as an empty ofpbuf that contains the 'allocated' bytes of
@@ -101,6 +107,17 @@ ofpbuf_use_const(struct ofpbuf *b, const void *data,
size_t size)
ofpbuf_set_size(b, size);
}
+/* Initializes 'b' as an empty ofpbuf that contains the 'allocated' bytes of
+ * memory starting at 'base'. DPDK allocated ofpbuf and *data is allocated
+ * from one continous memory region, so in memory data start right after
+ * ofpbuf. Therefore there is special method to free this type of
+ * buffer. */
+void
+ofpbuf_init_dpdk(struct ofpbuf *b, size_t allocated)
+{
+ofpbuf_init__(b, allocated, OFPBUF_DPDK);
+}
+
/* Initializes 'b' as an empty ofpbuf with an initial capacity of 'size'
* bytes. */
void
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index a261a44..df7f189 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -85,6 +85,8 @@ void ofpbuf_use_stack(struct ofpbuf *, void *, size_t);
void ofpbuf_use_stub(struct ofpbuf *, void *, size_t);
void ofpbuf_use_const(struct ofpbuf *, const void *, size_t);
+void ofpbuf_init_dpdk(struct ofpbuf *b, size_t allocated);
+
void ofpbuf_init(struct ofpbuf *, size_t);
void ofpbuf_uninit(struct ofpbuf *);
static inline void *ofpbuf_get_uninit_pointer(struct ofpbuf *);
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 5/5] ofproto-dpif.at: Sprinkle --overwrite-pidfile
> On Mon, Mar 31, 2014 at 03:24:31PM +0900, YAMAMOTO Takashi wrote: >> These tests invokes ovs-ofctl monitor twice or more. >> Because "ovs-appctl -t ofctl exit" does not wait for the target >> process exit, there are chances to see the pid file from the previous >> incarnation. >> >> Signed-off-by: YAMAMOTO Takashi > > Good catch. > > As an alternative, one could OVS_WAIT_UNTIL the pidfile disappears. it sounds like a better solution. i'll take a look. YAMAMOTO Takashi > > Thank you for improving all these tests! > > Acked-by: Ben Pfaff > ___ > 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 4/5] ofproto.at: Fix races in rule eviciton tests
>> # Sleep and modify the one that expires soonest
>> -sleep 2
>> +ovs-appctl time/warp 2
>
> I think we should warp as small as possible to verify precision.
> What do you think about warp 12000?
i agree smaller is better.
how about the following?
YAMAMOTO Takashi
commit b4a624761fe90aa0d57264013287f4236077ac9c
Author: YAMAMOTO Takashi
Date: Mon Mar 31 14:04:35 2014 +0900
ofproto.at: Fix races in rule eviciton tests
Bump timeout differences, because timeouts different by 1s might end up
to have the same position in the heap as rule_eviction_priority() uses
1024ms as a unit.
Also, use time/stop to avoid relying on how long an add-flow would take.
These tests were introduced by commit 6d56c1f1.
("ofproto: Update rule's priority in eviction group.")
Signed-off-by: YAMAMOTO Takashi
Cc: Kmindg G
Acked-by: Ben Pfaff
diff --git a/tests/ofproto.at b/tests/ofproto.at
index e98b526..a5116db 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1375,27 +1375,28 @@ AT_CHECK(
| ${PERL} $srcdir/uuidfilt.pl],
[0], [<0>
])
+ovs-appctl time/stop
# Add 4 flows.
for in_port in 4 3 2 1; do
-ovs-ofctl add-flow br0
hard_timeout=1${in_port},in_port=$in_port,actions=drop
+ovs-ofctl add-flow br0 hard_timeout=$((10 + in_port *
2)),in_port=$in_port,actions=drop
done
AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
- hard_timeout=11, in_port=1 actions=drop
- hard_timeout=12, in_port=2 actions=drop
- hard_timeout=13, in_port=3 actions=drop
- hard_timeout=14, in_port=4 actions=drop
+ hard_timeout=12, in_port=1 actions=drop
+ hard_timeout=14, in_port=2 actions=drop
+ hard_timeout=16, in_port=3 actions=drop
+ hard_timeout=18, in_port=4 actions=drop
NXST_FLOW reply:
])
# Sleep and modify the one that expires soonest
-sleep 2
+ovs-appctl time/warp 3000
AT_CHECK([ovs-ofctl mod-flows br0 in_port=1,actions=drop])
-sleep 2
+ovs-appctl time/warp 2000
# Adding another flow will cause the one that expires soonest to be evicted.
AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
- hard_timeout=11, in_port=1 actions=drop
- hard_timeout=13, in_port=3 actions=drop
- hard_timeout=14, in_port=4 actions=drop
+ hard_timeout=12, in_port=1 actions=drop
+ hard_timeout=16, in_port=3 actions=drop
+ hard_timeout=18, in_port=4 actions=drop
in_port=5 actions=drop
NXST_FLOW reply:
])
@@ -1414,26 +1415,27 @@ AT_CHECK(
])
# Add 4 flows.
for in_port in 4 3 2 1; do
-ovs-ofctl add-flow br0
idle_timeout=1${in_port},in_port=$in_port,actions=drop
+ovs-ofctl add-flow br0 idle_timeout=$((10 + in_port *
2)),in_port=$in_port,actions=drop
done
+ovs-appctl time/stop
AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
- idle_timeout=11, in_port=1 actions=drop
- idle_timeout=12, in_port=2 actions=drop
- idle_timeout=13, in_port=3 actions=drop
- idle_timeout=14, in_port=4 actions=drop
+ idle_timeout=12, in_port=1 actions=drop
+ idle_timeout=14, in_port=2 actions=drop
+ idle_timeout=16, in_port=3 actions=drop
+ idle_timeout=18, in_port=4 actions=drop
NXST_FLOW reply:
])
# Sleep and receive on the flow that expires soonest
-sleep 2
+ovs-appctl time/warp 3000
AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1)'])
-sleep 2
+ovs-appctl time/warp 2000
# Adding another flow will cause the one that expires soonest to be evicted.
AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
- idle_timeout=13, in_port=3 actions=drop
- idle_timeout=14, in_port=4 actions=drop
+ idle_timeout=16, in_port=3 actions=drop
+ idle_timeout=18, in_port=4 actions=drop
in_port=5 actions=drop
- n_packets=1, n_bytes=60, idle_timeout=11, in_port=1 actions=drop
+ n_packets=1, n_bytes=60, idle_timeout=12, in_port=1 actions=drop
NXST_FLOW reply:
])
OVS_VSWITCHD_STOP
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
Hi Jesse, Hi Pravin, Hi All, This patchset makes use of a whitelist to only allow mpls_push actions to be applied to packets with an ethertype where the tag order is well defined and implemented. This avoids the problem of where an MPLS LSE should be added in relation to a VLAN or similar tag. The list of allowed ethertypes is currently: - ETH_P_IP (0x0800) - ETH_P_ARP (0x0806) - ETH_P_RARP (0x0835) - ETH_P_IPV6 (0x86DD) - ETH_P_MPLS_UC (0x8847) - ETH_P_MPLS_MC (0x8847) I have updated this patch since v2.55 so that: * The MPLS ethertypes are included in the whitelist of ethertypes that allow mpls_push actions. * It once again applies on top of the master branch Simon Horman (1): datapath: Add basic MPLS support to kernel OPENFLOW-1.1+ | 4 - datapath/Modules.mk | 1 + datapath/actions.c | 119 +- datapath/datapath.c | 4 +- datapath/flow.c | 29 +++ datapath/flow.h | 17 +- datapath/flow_netlink.c | 298 ++-- datapath/flow_netlink.h | 2 +- datapath/linux/compat/gso.c | 70 +- datapath/linux/compat/gso.h | 41 +++- datapath/linux/compat/include/linux/netdevice.h | 6 +- datapath/linux/compat/netdevice.c | 10 +- datapath/mpls.h | 15 ++ include/linux/openvswitch.h | 9 +- 14 files changed, 568 insertions(+), 57 deletions(-) create mode 100644 datapath/mpls.h -- 1.8.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] [RFC] flow: Do not clear L3+ fields of flow in flow_push_mpls()
On Thu, Mar 20, 2014 at 10:05:44AM -0700, Ben Pfaff wrote:
> On Fri, Mar 14, 2014 at 04:19:52PM +0900, Simon Horman wrote:
> > When creating a flow in the datapath as the result of an upcall
> > the match itself is the match supplied in the upcall while
> > the mask of the match, if supplied, is generated based on the
> > flow and mask composed during action translation.
> >
> > In the case of, for example a UDP packet, the match will include
> > of L2, L3 and L4 fields. However, if the flow is cleared in
> > flow_push_mpls() then the mask that is synthesised from it will
> > not include L3 and L4 fields. This seems incorrect and the kernel
> > datapath complains about this mismatch.
> >
> > Signed-off-by: Simon Horman
>
> The goal of clearing the fields is to ensure that later flow tables
> can't match on fields that aren't visible anymore. That's important for
> accurate OpenFlow implementation, so I'd rather not change it. On the
> other hand, I see the point you're making, but I don't immediately
> understand why it happens that way. After all, I can change fields with
> OpenFlow actions and the datapath flows work out OK, why doesn't this
> work out OK too? Do you understand the reason?
Hi Ben,
I have taken a closer look at this and I think that I now understand the
problem. I would like to propose the following:
From: Simon Horman
odp-util: Do not set port mask of non-IP packets
In the case that an flow for an IP packet has an mpls_push action applied
the L3 and L4 portions of the flow will be cleared in flow_push_mpls().
Without this change commit_set_port_action() will set the tp_src and tp_dst
mask for the flow to all-ones because the base and flow port values no
longer match. Even though there will be no corresponding set action for the
ports; because the flow is no longer IP.
In this case where nw_proto is not part of the match this manifests
in a problem because the userspace datapath rejects flows whose masks
have non-zero values for tp_src or dp_dst if the nw_proto mask is
not all-ones.
This patch resolves this problem by having commit_set_port_action() return
without doing anything if flow->nw_proto is zero. The same logic is present
in commit_set_nw_action().
Signed-off-by: Simon Horman
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 956fef1..b40f9bc 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3778,6 +3778,11 @@ static void
commit_set_port_action(const struct flow *flow, struct flow *base,
struct ofpbuf *odp_actions, struct flow_wildcards *wc)
{
+/* Check if 'flow' really has an L3 header. */
+if (!flow->nw_proto) {
+return;
+}
+
if (!is_ip_any(base) || (!base->tp_src && !base->tp_dst)) {
return;
}
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH v2.56] datapath: Add basic MPLS support to kernel
Allow datapath to recognize and extract MPLS labels into flow keys
and execute actions which push, pop, and set labels on packets.
Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.
Cc: Ravi K
Cc: Leo Alterman
Cc: Isaku Yamahata
Cc: Joe Stringer
Signed-off-by: Simon Horman
---
v2.56
* Update whitelist of ethtypes where mpls_push may be used to include
the MPLS ethtypes. The whitelist is now:
- ETH_P_IP (0x0800)
- ETH_P_ARP (0x0806)
- ETH_P_RARP (0x0835)
- ETH_P_IPV6 (0x86DD)
- ETH_P_MPLS_UC (0x8847)
- ETH_P_MPLS_MC (0x8847)
* Rebase for
- 6d328fa23ddf5c75
("ofproto: Honour Table Mod settings for table-miss handling")
- 708fb4c50aa5547f
("datapath: Compact sw_flow_key.")
- 0962036c0ec3db8a
("recirculation: Adjust ovs_key_attr ABI")
v2.55
* Use a whitelist of ethtypes where mpls_push may be used
rather than a blacklist of ethtypes where mpls_push may not be used.
This is a more restrictive and more conservative approach that guarantees
that the tag order is known and defined.
The new whitelist is:
- ETH_P_IP (0x0800)
- ETH_P_ARP (0x0806)
- ETH_P_RARP (0x0835)
- ETH_P_IPV6 (0x86DD)
The old blacklist was:
- ETH_P_8021Q (0x8100)
- ETH_P_8021AD (0x88A8)
- ETH_P_QINQ1 (0x0x9100)
- ETH_P_QINQ2 (0x0x9200)
- ETH_P_QINQ3 (0x0x9300)
* Rebase for
29c71cfa0c137abd ("datapath: Add support for Linux 3.12")
982a47eceac1be71 ("datapath: Use ether_addr_copy")
v2.54
* Do not allow push MPLS in the presence of VLANs
* Remove support for push MPLS in the presence of VLANs from actions.c
v2.53
* Push MPLS labels after VLAN tags
- This is consistent with OF1.2 and plans for OF1.3.4, and OF1.5+.
It is inconsistent with OF1.4, which appears to be an aberration
v2.52
* Do not guard __skb_network_protocol with KERNEL_VERSION(3.11.0)
It was not guarded before this patch and should not be guarded
afterwards as it is currently needed regardless of the kernel version
v2.50 - v2.51
* No change
v2.49
* Remove MPLS items from OPENFLOW-1.1+. They should now be complete.
v2.47
* Rebase for HAVE_RHEL_OVS_HOOK and OVS_KEY_ATTR_TCP_FLAGS
v2.43 - v2.46
* No change
v2.42
* Rebase for:
+ 0585f7a ("datapath: Simplify mega-flow APIs.")
+ a097c0b ("datapath: Restructure datapath.c and flow.c")
* As suggested by Jesse Gross
+ Take into account that push_mpls() will have freed the skb on error
+ Remove dubious !eth_p_mpls(skb->protocol) condition from push_mpls
The !eth_p_mpls(skb->protocol) condition on setting inner_protocol
has no effect. Its motivation was to ensure that inner_protocol was
only set the first time that mpls_push occured. However this is already
ensured by the !ovs_skb_get_inner_protocol(skb) condition.
+ Return -EINVAL instead of -ENOMEM from pop_mpls() if the skb is too short
+ Do not add @inner_protocol to kernel doc for struct ovs_skb_cb.
The patch no longer adds an inner_protocol member to struct ovs_skb_cb
+ Do not add and set otherwise unsued inner_protocol variable in
rpl_dev_queue_xmit()
* As suggested by Pravin Shelar
+ Implement compatibility code in existing rpl_skb_gso_segment
rather than introducing to use rpl___skb_gso_segment
v2.41
* No change
v2.40
* Rebase for:
+ New dev_queue_xmit compat code
+ Updated put_vlan()
* As suggested by Jesse Gross
+ Remove bogus mac_len update from push_mpls()
+ Slightly simplify push_mpls() by using eth_hdr()
+ Remove dubious condition !eth_p_mpls(inner_protocol) on
an skb being considered to be MPLS in netdev_send()
+ Only use compatibility code for MPLS GSO segmentation on kernels
older than 3.11
+ Revamp setting of inner_protocol
1. Do not unconditionally set inner_protocol to the value of
skb->protocol in ovs_execute_actions().
2. Initialise inner_protocol it to zero only if compatibility code is in
use. In the case where compatibility code is not in use it will either
be zero due since the allocation of the skb or some other value set
by some other user.
3. Conditionally set the inner_protocol in push_mpls() to the value of
skb->protocol when entering push_mpls(). The condition is that
inner_protocol is zero and the value of skb->protocol is not an MPLS
ethernet type.
- This new scheme:
+ Pushes logic to set inner_protocol closer to the case where it is
needed.
+ Avoids over-writing values set by other users.
* As suggested by Pravin Shelar
+ Only set and restore skb->protocol in rpl___skb_gso_segment() in the
case of MPLS
+ Add inner_protocol field to struct ovs_gso_cb instead of ovs_skb_cb.
This moves compatibility code closer to where it is used
and creates fewer differences with mainline.
* Update comment on mac_len updates in datapath/actions.c
* Remove HAVE_INNER_PROCOTOL and instead just check
against kernel version 3.11 directly.
HAVE_INNER_PROCOTOL is a hang-over from work done prior
Re: [ovs-dev] [bond megaflow v4 4/4 rebased] ofproto/bond: Implement bond megaflow using recirculation
On Tue, Mar 25, 2014 at 01:48:39PM -0700, Andy Zhou wrote: > Infrastructure to enable megaflow support for bond ports using > recirculation. This patch adds the following features: > * Generate RECIRC action when bond can benefit from recirculation. > * Populate post recirculation rules in a hidden table. Currently table 254. > * Uses post recirculation rules for bond rebalancing > * A recirculation implementation in dpif-netdev. > > Signed-off-by: Andy Zhou Hi Andy, it appears that this does not apply cleanly, Would you care to rebase it (again)? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/5] ofproto.at: Fix races in rule eviciton tests
On Tue, Apr 1, 2014 at 1:12 PM, YAMAMOTO Takashi wrote:
>>> # Sleep and modify the one that expires soonest
>>> -sleep 2
>>> +ovs-appctl time/warp 2
>>
>> I think we should warp as small as possible to verify precision.
>> What do you think about warp 12000?
>
> i agree smaller is better.
> how about the following?
>
> YAMAMOTO Takashi
>
>
> commit b4a624761fe90aa0d57264013287f4236077ac9c
> Author: YAMAMOTO Takashi
> Date: Mon Mar 31 14:04:35 2014 +0900
>
> ofproto.at: Fix races in rule eviciton tests
>
> Bump timeout differences, because timeouts different by 1s might end up
> to have the same position in the heap as rule_eviction_priority() uses
> 1024ms as a unit.
>
> Also, use time/stop to avoid relying on how long an add-flow would take.
>
> These tests were introduced by commit 6d56c1f1.
> ("ofproto: Update rule's priority in eviction group.")
>
> Signed-off-by: YAMAMOTO Takashi
> Cc: Kmindg G
> Acked-by: Ben Pfaff
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index e98b526..a5116db 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1375,27 +1375,28 @@ AT_CHECK(
> | ${PERL} $srcdir/uuidfilt.pl],
>[0], [<0>
> ])
> +ovs-appctl time/stop
> # Add 4 flows.
> for in_port in 4 3 2 1; do
> -ovs-ofctl add-flow br0
> hard_timeout=1${in_port},in_port=$in_port,actions=drop
> +ovs-ofctl add-flow br0 hard_timeout=$((10 + in_port *
> 2)),in_port=$in_port,actions=drop
> done
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=12, in_port=2 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=12, in_port=1 actions=drop
> + hard_timeout=14, in_port=2 actions=drop
> + hard_timeout=16, in_port=3 actions=drop
> + hard_timeout=18, in_port=4 actions=drop
> NXST_FLOW reply:
> ])
> # Sleep and modify the one that expires soonest
> -sleep 2
> +ovs-appctl time/warp 3000
> AT_CHECK([ovs-ofctl mod-flows br0 in_port=1,actions=drop])
at this point, flow table would be:
hard_timeout=12, in_port=1 actions=drop
hard_timeout=11, in_port=2 actions=drop
hard_timeout=13, in_port=3 actions=drop
hard_timeout=15, in_port=4 actions=drop
And according to your comment, 1s interval may not be suitable.
I would like to change (10 + in_port * 2) to (10 + in_port * 3), and warp 4000.
What do you think?
Thanks.
> -sleep 2
> +ovs-appctl time/warp 2000
> # Adding another flow will cause the one that expires soonest to be evicted.
> AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=12, in_port=1 actions=drop
> + hard_timeout=16, in_port=3 actions=drop
> + hard_timeout=18, in_port=4 actions=drop
> in_port=5 actions=drop
> NXST_FLOW reply:
> ])
> @@ -1414,26 +1415,27 @@ AT_CHECK(
> ])
> # Add 4 flows.
> for in_port in 4 3 2 1; do
> -ovs-ofctl add-flow br0
> idle_timeout=1${in_port},in_port=$in_port,actions=drop
> +ovs-ofctl add-flow br0 idle_timeout=$((10 + in_port *
> 2)),in_port=$in_port,actions=drop
> done
> +ovs-appctl time/stop
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=11, in_port=1 actions=drop
> - idle_timeout=12, in_port=2 actions=drop
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=12, in_port=1 actions=drop
> + idle_timeout=14, in_port=2 actions=drop
> + idle_timeout=16, in_port=3 actions=drop
> + idle_timeout=18, in_port=4 actions=drop
> NXST_FLOW reply:
> ])
> # Sleep and receive on the flow that expires soonest
> -sleep 2
> +ovs-appctl time/warp 3000
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1)'])
> -sleep 2
> +ovs-appctl time/warp 2000
> # Adding another flow will cause the one that expires soonest to be evicted.
> AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
> AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=16, in_port=3 actions=drop
> + idle_timeout=18, in_port=4 actions=drop
> in_port=5 actions=drop
> - n_packets=1, n_bytes=60, idle_timeout=11, in_port=1 actions=drop
> + n_packets=1, n_bytes=60, idle_timeout=12, in_port=1 actions=drop
> NXST_FLOW reply:
> ])
> OVS_VSWITCHD_STOP
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
