[ovs-dev] Processed: jessie
Processing commands for cont...@bugs.debian.org:
> tags 719665 + sid jessie
Bug #719665 [src:libterralib] libterralib: FTBFS: Impossible to satisfy
Build-Depends
Added tag(s) sid and jessie.
> tags 719736 + sid jessie
Bug #719736 [openvswitch] openvswitch: build FTBFS "make check" failures on
most archs
Added tag(s) sid and jessie.
> tags 720374 + sid jessie
Bug #720374 [libgrib-api-1.10.4,libgrib-api-1.9.16] libgrib-api-1.9.16 and
libgrib-api-1.10.4: error when trying to install together
Added tag(s) sid and jessie.
> tags 704784 + sid jessie
Bug #704784 {Done: gregor herrmann } [src:libogre-perl]
libogre-perl: Please upgrade OGRE dependency to 1.8 or greater
Added tag(s) sid and jessie.
> tags 704786 + sid jessie
Bug #704786 [src:ember] ember: Please upgrade OGRE dependency to 1.8 or greater
Added tag(s) sid and jessie.
> tags 707079 + sid jessie
Bug #707079 [chef] chef: Please package new upstream version
Added tag(s) sid and jessie.
> tags 704781 + sid jessie
Bug #704781 {Done: Muammar El Khatib } [src:cegui-mk2]
cegui-mk2: Please upgrade OGRE dependency to 1.8 or greater
Added tag(s) sid and jessie.
> thanks
Stopping processing here.
Please contact me if you need assistance.
--
704781: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=704781
704784: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=704784
704786: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=704786
707079: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=707079
719665: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719665
719736: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719736
720374: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=720374
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [clang-atomics 1/4] ovs-atomic: Fix typo in comment.
On 20 August 2013 14:10, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff Acked-by: Ed Maste ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 19/08/13 17:23, Jesse Gross wrote: >> Is this the case? Is the delta between DKMS and native kernel >>> disappearing? Can I just ship the userspace tooling for 13.10 >>> of Ubuntu? > It's going away but it's not quite there yet. The 3.11 upstream > kernel supports GRE but not VXLAN (that's hopefully coming in 3.12) > so I think it probably makes sense to go with the out of tree > version for one more release and then switch over. That makes sense to me as well; roll on 14.04! >>> If not, some help diagnosing the issue I'm seeing would be >>> great: >>> >>> https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/1213879 > >>> Here's an upstream commit that is definitely needed as part of 3.11 > support and looks at least somewhat related, so it might be a good > place to start: > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/openvswitch?id=351638e7deeed2ec8ce451b53d33921b3da68f83 Thank > for the pointer - this got me over the immediate kernel panic I saw with the master branch on 3.11; see attached patch file for the full delta I'm using. I'm currently hitting a WARN_ON in net/core/dev.c:5078 /* Notifier chain MUST detach us all upper devices. */ WARN_ON(netdev_has_any_upper_dev(dev)); This occurs when devices are being destroyed in openvswitch (I'm using mininet with a two switch, 14 host topology for testing). Any ideas? The stock openvswitch module in the 3.11 kernel does not exhibit the same behaviour so I've been looking there for clues... Full trace: Aug 21 10:13:42 armstrong kernel: [ 2423.059333] [ cut here ] Aug 21 10:13:42 armstrong kernel: [ 2423.059351] WARNING: CPU: 2 PID: 5286 at /build/buildd/linux-3.11.0/net/core/dev.c:5078 rollback_registered_many+0x219/0x250() Aug 21 10:13:42 armstrong kernel: [ 2423.059355] Modules linked in: openvswitch(OF) veth(F) xt_conntrack(F) ipt_REJECT(F) xt_CHECKSUM(F) iptable_mangle(F) xt_tcpudp(F) ip6table_filter(F) ip6_tables(F) iptable_filter(F) ebtable_nat(F) ebtables(F) ipt_MASQUERADE(F) iptable_nat(F) nf_conntrack_ipv4(F) nf_defrag_ipv4(F) nf_nat_ipv4(F) nf_nat(F) nf_conntrack(F) ip_tables(F) x_tables(F) bridge(F) stp(F) llc(F) dm_crypt(F) gre(F) x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel(F) kvm(F) crc32_pclmul(F) ghash_clmulni_intel(F) aesni_intel(F) aes_x86_64(F) lrw(F) gf128mul(F) glue_helper(F) ablk_helper(F) cryptd(F) snd_usb_audio snd_usbmidi_lib microcode(F) psmouse(F) serio_raw(F) thinkpad_acpi nvram(F) snd_seq_midi(F) snd_seq_midi_event(F) uvcvideo snd_hda_codec_hdmi snd_rawmidi(F) videobuf2_vmalloc arc4(F) videobuf2_memops joydev(F) parport_pc(F) snd_hda_codec_realtek videobuf2_core videodev ppdev(F) snd_seq(F) iwldvm snd_hda_intel mac80211 btusb snd_hda_codec snd_hwdep(F) iwlwifi snd_pcm(F) lpc_ich bnep cfg80211 r Aug 21 10:13:42 armstrong kernel: fcomm bluetooth snd_seq_device(F) snd_page_alloc(F) snd_timer(F) snd(F) tpm_tis soundcore(F) intel_rst mei_me mei mac_hid lp(F) parport(F) nls_iso8859_1(F) usb_storage(F) hid_generic hid_microsoft usbhid hid mmc_block i915 i2c_algo_bit drm_kms_helper drm sdhci_pci ahci(F) libahci(F) sdhci e1000e(F) ptp(F) pps_core(F) wmi video(F) [last unloaded: openvswitch] Aug 21 10:13:42 armstrong kernel: [ 2423.059516] CPU: 2 PID: 5286 Comm: ip Tainted: GF W O 3.11.0-3-generic #7-Ubuntu Aug 21 10:13:42 armstrong kernel: [ 2423.059521] Hardware name: LENOVO 2324CTO/2324CTO, BIOS G2ET91WW (2.51 ) 01/14/2013 Aug 21 10:13:42 armstrong kernel: [ 2423.059524] 0009 88037b8fd8a8 816f0d91 Aug 21 10:13:42 armstrong kernel: [ 2423.059532] 88037b8fd8e0 81061ced 880387b6a000 88037b8fd8d8 Aug 21 10:13:42 armstrong kernel: [ 2423.059539] 88037b8fd948 81cd3880 88037b8fd8f0 Aug 21 10:13:42 armstrong kernel: [ 2423.059546] Call Trace: Aug 21 10:13:42 armstrong kernel: [ 2423.059557] [] dump_stack+0x45/0x56 Aug 21 10:13:42 armstrong kernel: [ 2423.059567] [] warn_slowpath_common+0x7d/0xa0 Aug 21 10:13:42 armstrong kernel: [ 2423.059575] [] warn_slowpath_null+0x1a/0x20 Aug 21 10:13:42 armstrong kernel: [ 2423.059582] [] rollback_registered_many+0x219/0x250 Aug 21 10:13:42 armstrong kernel: [ 2423.059590] [] unregister_netdevice_many+0x17/0x70 Aug 21 10:13:42 armstrong kernel: [ 2423.059599] [] rtnl_dellink+0xbf/0x130 Aug 21 10:13:42 armstrong kernel: [ 2423.059608] [] rtnetlink_rcv_msg+0x99/0x260 Aug 21 10:13:42 armstrong kernel: [ 2423.059617] [] ? __mutex_lock_slowpath+0xb3/0x1c0 Aug 21 10:13:42 armstrong kernel: [ 2423.059624] [] ? rtnetlink_rcv+0x30/0x30 Aug 21 10:13:42 armstrong kernel: [ 2423.059632] [] netlink_rcv_skb+0xa9/0xc0 Aug 21 10:13:42 armstrong kernel: [ 2423.059639] [] rtnetlink_rcv+0x28/0x30 Aug 21 10:13:42 armstrong kernel: [ 2423.059646] [] netlink_unicast+0xdd/0x190 Aug 21 10:13:42 armstrong kernel: [ 2423.059655] [] ? memcpy_fromiovec+0x4d/0x90 Aug
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 21/08/13 10:26, James Page wrote: >> support and looks at least somewhat related, so it might be a >> good >>> place to start: >>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/openvswitch?id=351638e7deeed2ec8ce451b53d33921b3da68f83 > >>> Thank >>> > for the pointer - this got me over the immediate kernel panic I saw > with the master branch on 3.11; see attached patch file for the > full delta I'm using. > > I'm currently hitting a WARN_ON in net/core/dev.c:5078 I tried the same patches against branch-1.12 and I don't see the same issue with the 3.11 kernel - which might just dig me out of a hole for Ubuntu 13.10. What's the status of OVS 1.12? Is this just a dev branch or is it planned for release? Cheers James - -- James Page Technical Lead Ubuntu Server Team james.p...@canonical.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCAAGBQJSFI+zAAoJEL/srsug59jD0BcQANP/ixE0yVAOo1Zd9YmJu/o6 kcTkSLB+N8MZPdn9LOvPxVJMMUXEHWQALxq+KsmXS9VfrdgaoJBFAMlgZ1zaC3KC XPsrfyx4hC7ReFus2F0IZ5jPjpSBRiiZWys7Xv3/LUxtBifp6E5D8ANGDX/AA1X/ aIQ8PxHk0WhI2igx4aflJs+gMIC/SbKByMTFzJQ8qQv8nNEiG2ULAw4ecvV9jgpl +QFdXDA3m6dZUvotjexYNjXu2Ble2slDEqRYkCqmEUiL5pmaIWVo3Nf2x5OQv+Fo wGmMqskTXFZu8DJA28Tuy9wEvqCyihavGGTSARF9s+WVYxgGqAKbD3CrWZXxDqkh xy709ES2g+KatSRUXD7nGEoFz7LXlfsvfy05E/Je2Qrg/ePaYIqsPfwRGRihmbld YsjUGtDa2DWpfhEYpCGPtCqdtIJL72zafqFhioF13rxZcu0Wx+GbX6b5Hs+Jt1p6 g4Nv7DHqgr2hNRExaRAXrzxH2d47AobEpnexveGzzDUt2gvW6WAfyVtq4FGcMVNy glw0VA4rSWtros3RhUnW+M1Q+v7G30HkN5tV/+G3r/iB2TxtG6/YmvGlhirzszat kYfG0VWiYXCf8JkKn5tO3i5chW4RiKZkFrpqTkxTQN7ZFXkRUubIA7PxG4FlG90s Dq2ZkJKdvvApXSUYdnWv =lOf6 -END PGP SIGNATURE- ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
On Wed, Aug 21, 2013 at 3:00 AM, James Page wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > On 21/08/13 10:26, James Page wrote: >>> support and looks at least somewhat related, so it might be a >>> good place to start: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/net/openvswitch?id=351638e7deeed2ec8ce451b53d33921b3da68f83 >> > Thank >> for the pointer - this got me over the immediate kernel panic I saw >> with the master branch on 3.11; see attached patch file for the >> full delta I'm using. >> >> I'm currently hitting a WARN_ON in net/core/dev.c:5078 > > I tried the same patches against branch-1.12 and I don't see the same > issue with the 3.11 kernel - which might just dig me out of a hole for > Ubuntu 13.10. Interesting. Here's a likely candidate for the difference: 2b51596fdeba7fbf4caff323dd6af375e7f84596 (datapath: link upper device for port devices) and the follow up: fa40777195548ade99d22b4f8ec9a795be682504 (datapath: Do not set dev->master for XEN.) Although it's not immediately obvious why this would cause the warning because on branch-1.11 the master device isn't set at all. > What's the status of OVS 1.12? Is this just a dev branch or is it > planned for release? It's actually a dead branch. It looks like we're going to get a few more things in and call the version after 1.11 2.0. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Hi Jesse On 21/08/13 17:16, Jesse Gross wrote: >>> What's the status of OVS 1.12? Is this just a dev branch or is >>> it planned for release? > It's actually a dead branch. It looks like we're going to get a > few more things in and call the version after 1.11 2.0. OK - so it sounds like I should be landing a snapshot based on the 1.11 branch into Ubuntu Saucy - albeit with the cherry picks for 3.10 and my patch for 3.11 ontop... Is that correct? Cheers James - -- James Page Ubuntu and Debian Developer james.p...@ubuntu.com jamesp...@debian.org -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCAAGBQJSFPDQAAoJEL/srsug59jDlJYQAIRubCEp43P/wQDtOmHP16dX LI2mGPGH1fgpDrfjlznT6AOmgxWOiyKkN7JN1mYmCnSKUPKt5bEmTxOV+5p1HD4k NDwk53AFShslk0Y1DLUsJ2QD4s6gj6LWOAynAxLUf2kol0IFD8/ArWSHHvmJzXNi tgfoPip9Mwj+lDZyayGirmbQEW2krlMgPClCugITWVWuOD8ojsPr8G0quRTMdVAI np7+5Rix+MfqS33zoge3VsKfi45p0AW8szkhiFa+5B4x4OsJat8UNK/jQ7wlMhi/ WjX3tQT2h0HkwzUyGyowbI7vpOEd6m98OrMdlg4Mm4qoKOVN90PKc6v43Q6VBoLD lR0x8WDlk36u8vVxlRHM09egqXc+qKw9mzYs/5Izlr79g1vPZ6DkblJF4skgNxZ7 k9CHxbgZztSlxFIM7NuphL04OfUUhmO186vLnwLRYSCTXtXRQfWC3r+uHkk87BL7 Vns96Da8D/HKpr1QHsPmRiKCrKqjHt2KLjuzU/Gm2iWLoA/eEnlFkZpCxuRqwEaP LC0Wh6fyWU2hbZpixhrS4vIMkBmlZJddfc5NvC88Kb5euD3jxcf1uzUXNiAnrbqB o1zCOR8iVJC7HRZ57tDvVKh+wT9tW+UOnxHJWAfV3PS3SpB/G1Kq2y6GQUoX/94B sYOUZeLdUc+FMEjct6qc =tZhj -END PGP SIGNATURE- ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [clang-atomics 1/4] ovs-atomic: Fix typo in comment.
On Wed, Aug 21, 2013 at 05:05:01AM -0400, Ed Maste wrote: > On 20 August 2013 14:10, Ben Pfaff wrote: > > Signed-off-by: Ben Pfaff > > Acked-by: Ed Maste Thanks, applied. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
On Wed, Aug 21, 2013 at 9:54 AM, James Page wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > Hi Jesse > > On 21/08/13 17:16, Jesse Gross wrote: What's the status of OVS 1.12? Is this just a dev branch or is it planned for release? >> It's actually a dead branch. It looks like we're going to get a >> few more things in and call the version after 1.11 2.0. > > OK - so it sounds like I should be landing a snapshot based on the > 1.11 branch into Ubuntu Saucy - albeit with the cherry picks for 3.10 > and my patch for 3.11 ontop... > > Is that correct? That sounds right to me. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 21/08/13 18:01, Jesse Gross wrote: >>> On 21/08/13 17:16, Jesse Gross wrote: > What's the status of OVS 1.12? Is this just a dev > branch or is it planned for release? > It's actually a dead branch. It looks like we're going to > get a few more things in and call the version after 1.11 > 2.0. >>> >>> OK - so it sounds like I should be landing a snapshot based on >>> the 1.11 branch into Ubuntu Saucy - albeit with the cherry >>> picks for 3.10 and my patch for 3.11 ontop... >>> >>> Is that correct? > That sounds right to me. > Whats the timescale around 2.0? - -- James Page Ubuntu and Debian Developer james.p...@ubuntu.com jamesp...@debian.org -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBCAAGBQJSFPN7AAoJEL/srsug59jDrBoP/02O88Ju4V4a0agaYJarkufk ZGjZDR1KTL7f7cVN8FXLpDM5ENWBLF5G3wuK1VD31Z7U9Yz0JughfyB/+MRlDztW CBETp3mI4g/ekeftH18Gi+nqXXi1pQJNaXbZz48yx7mG+8pJpHDXEWW+ZRPvA7sp M9jAc+hY9knUSuX+uvfiZKO00B5Op0Ml/PqSuWr6Cmw2xPyB5tUlK89HOmwJzFbG 9uNR1pLg/ZQnK0bkT69Ffka0zbO6z5u0agM6lSnfKCrLX7fD50pJES9wxQfUMY21 UAujrs3oa0cQsl3QSLlCsQ2m0BSGWI0/QVjBkn5w85SZr5XLZMLXuqkh9sXT/tJn fKP3flD9F5c/tAKcQW8Z3wf3axD0fyjczfP8dCVj3MoWaBRS+2i6TsoIKp6Nspv+ 6M51xBYDzHaIcvUkDnU3Qi9o0M+M2YU2rZJp3K1D9RUYwZvmIlFGZIvzlwgaWcyP qaJmNPfoO2tCMWstvl87/Krq9bkS2efSQBL+krCC/0FH2T4Q4xQ/Rl/45bVFfnI/ gkwJtBARj6amsuj4EflkZlAFj1RsG9UFTd7vtRWqSKAe+ZHdM0ExLnG2TET6/tgR A1AC/zyRf7NBWr7aesp8fAi+B2Q3yor+y6S33eLK9LGBLg7Nb7zLUoOR6mLBYdku 1X1ZRbpsruJK8brrtjxg =Qil5 -END PGP SIGNATURE- ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [clang-atomics 4/4] ovs-thread: Work around apparent Clang bug.
On Tue, Aug 20, 2013 at 03:12:16PM -0700, Ben Pfaff wrote: > On Tue, Aug 20, 2013 at 05:05:26PM -0400, Ed Maste wrote: > > On 20 August 2013 14:10, Ben Pfaff wrote: > > > Without this patch, I get Clang warnings that I don't understand for each > > > of the non-static functions in dirs.c: > > > > > > warning: control reaches end of non-void function > > > > This is a clang bug; in your example the call to bar() is omitted by > > clang, and the warning is thus actually true. I encountered the bug > > while looking at other issues in FreeBSD's stdatomic.h and my > > colleague distilled a simple test case and submitted a bug report[1] > > for it yesterday. As of this morning it's fixed in clang svn[2]. > > Thanks, that's good to know. I'm using a Clang nightly build, so I'll > upgrade tomorrow and see if it fixes the problem for me, and I'll plan > to drop this patch. The new nightly fixes this problem but introduces new -Wthread-safety warnings. So, I'm going to drop this patch and go look at those warnings. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command
This commit changes the "ovs-appctl coverage/show" command to show the
per-second, per-minute and per-hour rates of function invocation. More
importantly, this makes using coverage counter an easy way to monitor
the execution of specific functions.
Signed-off-by: Alex Wang
---
lib/coverage-unixctl.man |3 +-
lib/coverage.c | 106 --
lib/coverage.h | 16 ++-
tests/ofproto-dpif.at| 34 +++
vswitchd/ovs-vswitchd.c |2 +
5 files changed, 155 insertions(+), 6 deletions(-)
diff --git a/lib/coverage-unixctl.man b/lib/coverage-unixctl.man
index 9718894..31935cc 100644
--- a/lib/coverage-unixctl.man
+++ b/lib/coverage-unixctl.man
@@ -8,4 +8,5 @@ main loop takes unusually long to run.
Coverage counters are useful mainly for performance analysis and
debugging.
.IP "\fBcoverage/show\fR"
-Displays the values of all of the coverage counters.
+Displays the per-second, per-minute and per-hour rates, and total counts
+of all of the coverage counters.
diff --git a/lib/coverage.c b/lib/coverage.c
index 23e2997..153b6ac 100644
--- a/lib/coverage.c
+++ b/lib/coverage.c
@@ -63,7 +63,17 @@ struct coverage_counter *coverage_counters[] = {
static struct ovs_mutex coverage_mutex = OVS_MUTEX_INITIALIZER;
+static long long int coverage_run_time = LLONG_MIN;
+
+/* Defines the moving average array index variables. */
+static unsigned int min_idx = 0;
+static unsigned int hr_idx = 0;
+/* Index counter used to compute the moving average array's index. */
+static unsigned int idx_count = 0;
+
static void coverage_read(struct svec *);
+static unsigned int coverage_array_sum(const unsigned int *arr,
+ const unsigned int len);
static void
coverage_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
@@ -206,6 +216,7 @@ coverage_log(void)
static void
coverage_read(struct svec *lines)
{
+struct coverage_counter **c = coverage_counters;
unsigned long long int *totals;
size_t n_never_hit;
uint32_t hash;
@@ -220,15 +231,24 @@ coverage_read(struct svec *lines)
totals = xmalloc(n_coverage_counters * sizeof *totals);
ovs_mutex_lock(&coverage_mutex);
for (i = 0; i < n_coverage_counters; i++) {
-totals[i] = coverage_counters[i]->total;
+totals[i] = c[i]->total;
}
ovs_mutex_unlock(&coverage_mutex);
for (i = 0; i < n_coverage_counters; i++) {
if (totals[i]) {
-svec_add_nocopy(lines, xasprintf("%-24s %9llu",
- coverage_counters[i]->name,
- totals[i]));
+/* The per second rate is calculated via averaging the count in
+ * the most recent COVERAGE_RUN_INTERVAL interval. */
+svec_add_nocopy(lines,
+xasprintf("%-24s %5.1f/sec %7u/min "
+ "%9u/hr total: %llu",
+ c[i]->name,
+ ((double) c[i]->min[(idx_count - 1) % MIN_AVG_LEN])
+ * 1000 / COVERAGE_RUN_INTERVAL,
+ coverage_array_sum(c[i]->min, MIN_AVG_LEN),
+ coverage_array_sum(c[i]->hr, HR_AVG_LEN),
+ totals[i]));
+
} else {
n_never_hit++;
}
@@ -249,3 +269,81 @@ coverage_clear(void)
}
ovs_mutex_unlock(&coverage_mutex);
}
+
+/* Runs approximately every COVERAGE_RUN_INTERVAL amount of time to update the
+ * coverage counters' 'min' and 'hr' array. 'min' array is for cumulating
+ * per second counts into per minute count. 'hr' array is for cumulating per
+ * minute counts into per hour count. */
+void
+coverage_run(void)
+{
+struct coverage_counter **c = coverage_counters;
+long long int now;
+
+/* Initialize the coverage_run_time. */
+if (coverage_run_time == LLONG_MIN) {
+coverage_run_time = time_msec() + COVERAGE_RUN_INTERVAL;
+}
+
+ovs_mutex_lock(&coverage_mutex);
+now = time_msec();
+if (now >= coverage_run_time) {
+size_t i, j;
+/* Computes the number of COVERAGE_RUN_INTERVAL slots, since
+ * it is possible that the actual run interval is multiple of
+ * COVERAGE_RUN_INTERVAL. */
+int slots = (now - coverage_run_time) / COVERAGE_RUN_INTERVAL + 1;
+
+for (i = 0; i < n_coverage_counters; i++) {
+unsigned int count, portion;
+unsigned int m_idx = min_idx;
+unsigned int h_idx = hr_idx;
+unsigned int idx = idx_count;
+
+/* Computes the differences between the current total and the one
+ * recorded in last invocation of coverage_run(). */
+count = c[i]->total - c[i]->last_total;
+c[i]->last_total = c[i]->total;
+/* The count over the time interval is evenly distributed
+ * among slots by calculating the
[ovs-dev] [PATCH 2/2] ofproto-dpif: Compute the subfacet add/del rate using coverage counters
The subfacet rates (e.g. add rate, del rate) were computed by exponential
moving averaging function in ofproto-dpif.c. This patch replaces that
logic with coverage counters. And the rates can be checked by running
"ovs-appctl coverage/show" command.
Signed-off-by: Alex Wang
---
ofproto/ofproto-dpif.c | 103 +---
tests/ofproto-dpif.at |4 --
tests/tunnel.at| 26 ++--
3 files changed, 23 insertions(+), 110 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5a64336..a2988a4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -71,6 +71,11 @@ COVERAGE_DEFINE(facet_changed_rule);
COVERAGE_DEFINE(facet_revalidate);
COVERAGE_DEFINE(facet_unexpected);
COVERAGE_DEFINE(facet_suppress);
+COVERAGE_DEFINE(facet_create);
+COVERAGE_DEFINE(facet_remove);
+COVERAGE_DEFINE(handle_flow_miss);
+COVERAGE_DEFINE(subfacet_create);
+COVERAGE_DEFINE(subfacet_destroy);
COVERAGE_DEFINE(subfacet_install_fail);
COVERAGE_DEFINE(packet_in_overflow);
COVERAGE_DEFINE(flow_mod_overflow);
@@ -416,20 +421,6 @@ struct dpif_backer {
unsigned avg_n_subfacet; /* Average number of flows. */
long long int avg_subfacet_life; /* Average life span of subfacets. */
-/* The average number of subfacets... */
-struct avg_subfacet_rates hourly; /* ...over the last hour. */
-struct avg_subfacet_rates daily;/* ...over the last day. */
-struct avg_subfacet_rates lifetime; /* ...over the switch lifetime. */
-long long int last_minute; /* Last time 'hourly' was updated. */
-
-/* Number of subfacets added or deleted since 'last_minute'. */
-unsigned subfacet_add_count;
-unsigned subfacet_del_count;
-
-/* Number of subfacets added or deleted from 'created' to 'last_minute.' */
-unsigned long long int total_subfacet_add_count;
-unsigned long long int total_subfacet_del_count;
-
/* Number of upcall handling threads. */
unsigned int n_handler_threads;
};
@@ -438,7 +429,6 @@ struct dpif_backer {
static struct shash all_dpif_backers = SHASH_INITIALIZER(&all_dpif_backers);
static void drop_key_clear(struct dpif_backer *);
-static void update_moving_averages(struct dpif_backer *backer);
struct ofproto_dpif {
struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
@@ -1229,14 +1219,6 @@ open_dpif_backer(const char *type, struct dpif_backer
**backerp)
backer->max_n_subfacet = 0;
backer->created = time_msec();
-backer->last_minute = backer->created;
-memset(&backer->hourly, 0, sizeof backer->hourly);
-memset(&backer->daily, 0, sizeof backer->daily);
-memset(&backer->lifetime, 0, sizeof backer->lifetime);
-backer->subfacet_add_count = 0;
-backer->subfacet_del_count = 0;
-backer->total_subfacet_add_count = 0;
-backer->total_subfacet_del_count = 0;
backer->avg_n_subfacet = 0;
backer->avg_subfacet_life = 0;
@@ -3384,6 +3366,7 @@ handle_flow_miss(struct flow_miss *miss, struct
flow_miss_op *ops,
{
struct facet *facet;
+COVERAGE_INC(handle_flow_miss);
miss->ofproto->n_missed += list_size(&miss->packets);
facet = facet_lookup_valid(miss->ofproto, &miss->flow);
@@ -3804,8 +3787,6 @@ update_stats(struct dpif_backer *backer)
run_fast_rl();
}
dpif_flow_dump_done(&dump);
-
-update_moving_averages(backer);
}
/* Calculates and returns the number of milliseconds of idle time after which
@@ -3994,6 +3975,7 @@ facet_create(const struct flow_miss *miss)
struct facet *facet;
struct match match;
+COVERAGE_INC(facet_create);
facet = xzalloc(sizeof *facet);
facet->ofproto = miss->ofproto;
facet->used = miss->stats.used;
@@ -4057,6 +4039,7 @@ facet_remove(struct facet *facet)
{
struct subfacet *subfacet, *next_subfacet;
+COVERAGE_INC(facet_remove);
ovs_assert(!list_is_empty(&facet->subfacets));
/* First uninstall all of the subfacets to get final statistics. */
@@ -4557,6 +4540,7 @@ subfacet_create(struct facet *facet, struct flow_miss
*miss)
subfacet = xmalloc(sizeof *subfacet);
}
+COVERAGE_INC(subfacet_create);
hmap_insert(&backer->subfacets, &subfacet->hmap_node, key_hash);
list_push_back(&facet->subfacets, &subfacet->list_node);
subfacet->facet = facet;
@@ -4570,7 +4554,6 @@ subfacet_create(struct facet *facet, struct flow_miss
*miss)
subfacet->path = SF_NOT_INSTALLED;
subfacet->backer = backer;
-backer->subfacet_add_count++;
return subfacet;
}
@@ -4580,11 +4563,8 @@ static void
subfacet_destroy__(struct subfacet *subfacet)
{
struct facet *facet = subfacet->facet;
-struct ofproto_dpif *ofproto = facet->ofproto;
-
-/* Update ofproto stats before uninstall the subfacet. */
-ofproto->backer->subfacet_del_count++;
+COVERAGE_INC(subfacet_destroy);
subfacet_uninstall(subfacet);
hmap_remove(&subfacet->back
Re: [ovs-dev] Reinterpret base for meter band types bitmap.
On Aug 12, 2013, at 9:57 PM, Ben Pfaff wrote:
> OpenFlow 1.3 says that the band_types member of struct ofp_meter_features
> is a bitmap of OFPMBT_* values. The OFPMBT_* values are 1-based, so
> until now, to avoid wasting bit 0, OVS mapped an OFPMBT_* with value 1 to
> bit 0, value 2 to bit 1, and so on. However, according to
> http://openvswitch.org/pipermail/dev/2013-July/029666.html,
> other OpenFlow implementations directly translate value 1 to bit 1,
> value 2 to bit 2, and so on. This commit changes Open vSwitch to use this
> more common interpretation.
>
> A request for clarification of this issue in the OpenFlow standard has been
> filed with the ONF Extensibility Working Group as issue EXT-345.
>
Should have raised the issue while I was writing that..
> Reported-by: YAMAMOTO Takashi
> Signed-off-by: Ben Pfaff
>
Acked-by: Jarno Rajahalme
> ---
> I'm pretty sure I sent this out before but I can't find it in
> patchwork. Reviews welcome.
>
> lib/ofp-print.c|8 ++--
> tests/ofp-print.at |2 +-
> 2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 1a4dd9c..e914b0a 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1102,13 +1102,9 @@ ofputil_meter_capabilities_to_name(uint32_t bit)
> static const char *
> ofputil_meter_band_types_to_name(uint32_t bit)
> {
> -/*
> - * Note: Meter band types start from 1. We assume that the lowest bit
> - * in the band_types corresponds to DROP band type (1).
> - */
> switch (bit) {
> -case 1 << (OFPMBT13_DROP - 1): return "drop";
> -case 1 << (OFPMBT13_DSCP_REMARK - 1): return "dscp_remark";
> +case 1 << OFPMBT13_DROP: return "drop";
> +case 1 << OFPMBT13_DSCP_REMARK: return "dscp_remark";
> }
>
> return NULL;
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 986b931..266af6c 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -1670,7 +1670,7 @@ AT_SETUP([OFPST_METER_FEATURES reply - OF1.3])
> AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
> AT_CHECK([ovs-ofctl ofp-print "\
> 04 13 00 20 00 00 00 02 00 0b 00 00 00 00 00 00 \
> -00 01 00 00 00 00 00 03 00 00 00 0F 10 02 00 00 \
> +00 01 00 00 00 00 00 06 00 00 00 0F 10 02 00 00 \
> "], [0], [dnl
> OFPST_METER_FEATURES reply (OF1.3) (xid=0x2):
> max_meter:65536 max_bands:16 max_color:2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] ofproto-dpif-upcall: Increase upcall stub size.
The existing upcall stub size is not large enough even for the
smallest miss upcalls.
Signed-off-by: Ethan Jackson
Reported-by: Guolin Yang
---
ofproto/ofproto-dpif-upcall.h |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index f742060..f597672 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -66,7 +66,7 @@ struct upcall {
/* Raw upcall plus data for keeping track of the memory backing it. */
struct dpif_upcall dpif_upcall; /* As returned by dpif_recv() */
struct ofpbuf upcall_buf; /* Owns some data in 'dpif_upcall'. */
-uint64_t upcall_stub[256 / 8]; /* Buffer to reduce need for malloc(). */
+uint64_t upcall_stub[512 / 8]; /* Buffer to reduce need for malloc(). */
};
struct upcall *upcall_next(struct udpif *);
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] netdev: Make run and wait functions optional as documented.
Signed-off-by: Ethan Jackson
Reported-by: Guolin Yang
---
lib/netdev.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lib/netdev.c b/lib/netdev.c
index 088aea9..bf942a0 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -126,7 +126,9 @@ netdev_run(void)
ovs_rwlock_rdlock(&netdev_class_rwlock);
HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
-rc->class->run();
+if (rc->class->run) {
+rc->class->run();
+}
}
ovs_rwlock_unlock(&netdev_class_rwlock);
}
@@ -143,7 +145,9 @@ netdev_wait(void)
ovs_rwlock_rdlock(&netdev_class_rwlock);
HMAP_FOR_EACH (rc, hmap_node, &netdev_classes) {
-rc->class->wait();
+if (rc->class->wait) {
+rc->class->wait();
+}
}
ovs_rwlock_unlock(&netdev_class_rwlock);
}
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [clang 3/4] ofproto-dpif: Mark rule_release() as no_thread_safety_analysis.
Otherwise new Clang complains about this function because it only sometimes
releases the lock (that is, it only does it when there is a lock to
release).
I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif.c |1 +
1 file changed, 1 insertion(+)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6f87aa6..4f1da53 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4838,6 +4838,7 @@ choose_miss_rule(enum ofputil_port_config config, struct
rule_dpif *miss_rule,
void
rule_release(struct rule_dpif *rule)
+OVS_NO_THREAD_SAFETY_ANALYSIS
{
if (rule) {
ovs_rwlock_unlock(&rule->up.evict);
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [clang 0/4] Fix new warnings with most recent Clang
I upgraded from Clang 1:3.4~svn187484-1~exp1 to 1:3.4~svn188890-1~exp1 this morning and got new thread safety warnings. This series fixes them. All the new warnings appear to be false positives. Ben Pfaff (4): ovs-thread: Mark lock and unlock functions as no_thread_safety_analysis. ofproto-dpif: Fix thread safety annotation on rule_dpif_lookup_in_table(). ofproto-dpif: Mark rule_release() as no_thread_safety_analysis. ofproto-dpif-xlate: Refactor xlate_table_action() to avoid Clang warnings. lib/compiler.h |3 +++ lib/ovs-thread.c |3 +++ ofproto/ofproto-dpif-xlate.c | 56 +++--- ofproto/ofproto-dpif.c |3 ++- ofproto/ofproto-dpif.h |2 +- 5 files changed, 40 insertions(+), 27 deletions(-) -- 1.7.10.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [clang 1/4] ovs-thread: Mark lock and unlock functions as no_thread_safety_analysis.
I don't see any other way to make Clang realize that these are the real
mutex implementation functions.
I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.
Signed-off-by: Ben Pfaff
---
lib/compiler.h |3 +++
lib/ovs-thread.c |3 +++
2 files changed, 6 insertions(+)
diff --git a/lib/compiler.h b/lib/compiler.h
index 519b832..fb4d46c 100644
--- a/lib/compiler.h
+++ b/lib/compiler.h
@@ -128,6 +128,8 @@
#define OVS_EXCLUDED(...) __attribute__((locks_excluded(__VA_ARGS__)))
#define OVS_ACQ_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__)))
#define OVS_ACQ_AFTER(...) __attribute__((acquired_after(__VA_ARGS__)))
+#define OVS_NO_THREAD_SAFETY_ANALYSIS \
+__attribute__((no_thread_safety_analysis))
#else /* not Clang */
#define OVS_LOCKABLE
#define OVS_REQ_RDLOCK(...)
@@ -145,6 +147,7 @@
#define OVS_RELEASES(...)
#define OVS_ACQ_BEFORE(...)
#define OVS_ACQ_AFTER(...)
+#define OVS_NO_THREAD_SAFETY_ANALYSIS
#endif
/* ISO C says that a C implementation may choose any integer type for an enum
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index e9366e2..b3a87bb 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -47,6 +47,7 @@ static bool multithreaded;
void \
ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \
const char *where) \
+OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
int error = pthread_##TYPE##_##FUN(&l->lock); \
@@ -63,6 +64,7 @@ LOCK_FUNCTION(rwlock, wrlock);
int \
ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \
const char *where) \
+OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
int error = pthread_##TYPE##_##FUN(&l->lock); \
@@ -81,6 +83,7 @@ TRY_LOCK_FUNCTION(rwlock, trywrlock);
#define UNLOCK_FUNCTION(TYPE, FUN) \
void \
ovs_##TYPE##_##FUN(const struct ovs_##TYPE *l_) \
+OVS_NO_THREAD_SAFETY_ANALYSIS \
{ \
struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
int error; \
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [clang 4/4] ofproto-dpif-xlate: Refactor xlate_table_action() to avoid Clang warnings.
I get a bunch of thread-safety warnings with the latest Clang without this
patch, because Clang is smart enough to see locking and unlocking but not
smart enough to figure out the relationships. This refactoring avoids the
warnings.
I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif-xlate.c | 56 +++---
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4578675..1b5e9eb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1664,6 +1664,26 @@ compose_output_action(struct xlate_ctx *ctx, ofp_port_t
ofp_port)
}
static void
+xlate_recursively(struct xlate_ctx *ctx, struct rule_dpif *rule)
+OVS_RELEASES(rule->up.evict)
+{
+struct rule_dpif *old_rule = ctx->rule;
+
+if (ctx->xin->resubmit_stats) {
+rule_credit_stats(rule, ctx->xin->resubmit_stats);
+}
+
+
+ctx->recurse++;
+ctx->rule = rule;
+do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx);
+ctx->rule = old_rule;
+ctx->recurse--;
+
+rule_release(rule);
+}
+
+static void
xlate_table_action(struct xlate_ctx *ctx,
ofp_port_t in_port, uint8_t table_id, bool may_packet_in)
{
@@ -1671,28 +1691,28 @@ xlate_table_action(struct xlate_ctx *ctx,
struct rule_dpif *rule;
ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port;
uint8_t old_table_id = ctx->table_id;
+bool got_rule;
ctx->table_id = table_id;
-/* Look up a flow with 'in_port' as the input port. */
+/* Look up a flow with 'in_port' as the input port. Then restore the
+ * original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will
+ * have surprising behavior). */
ctx->xin->flow.in_port.ofp_port = in_port;
-rule_dpif_lookup_in_table(ctx->xbridge->ofproto, &ctx->xin->flow,
- &ctx->xout->wc, table_id, &rule);
-
-/* Restore the original input port. Otherwise OFPP_NORMAL and
- * OFPP_IN_PORT will have surprising behavior. */
+got_rule = rule_dpif_lookup_in_table(ctx->xbridge->ofproto,
+ &ctx->xin->flow, &ctx->xout->wc,
+ table_id, &rule);
ctx->xin->flow.in_port.ofp_port = old_in_port;
if (ctx->xin->resubmit_hook) {
ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
}
-if (rule == NULL && may_packet_in) {
+if (got_rule) {
+xlate_recursively(ctx, rule);
+} else if (may_packet_in) {
struct xport *xport;
-/* Makes clang's thread safety analysis happy. */
-rule_release(rule);
-
/* XXX
* check if table configuration flags
* OFPTC_TABLE_MISS_CONTROLLER, default.
@@ -1704,23 +1724,9 @@ xlate_table_action(struct xlate_ctx *ctx,
ctx->xbridge->miss_rule,
ctx->xbridge->no_packet_in_rule);
ovs_rwlock_rdlock(&rule->up.evict);
+xlate_recursively(ctx, rule);
}
-if (rule && ctx->xin->resubmit_stats) {
-rule_credit_stats(rule, ctx->xin->resubmit_stats);
-}
-
-if (rule) {
-struct rule_dpif *old_rule = ctx->rule;
-
-ctx->recurse++;
-ctx->rule = rule;
-do_xlate_actions(rule->up.ofpacts, rule->up.ofpacts_len, ctx);
-ctx->rule = old_rule;
-ctx->recurse--;
-}
-rule_release(rule);
-
ctx->table_id = old_table_id;
} else {
static struct vlog_rate_limit recurse_rl = VLOG_RATE_LIMIT_INIT(1, 1);
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [clang 2/4] ofproto-dpif: Fix thread safety annotation on rule_dpif_lookup_in_table().
New Clang versions raise warnings about the incorrect old annotation.
I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
I previously used version 1:3.4~svn187484-1~exp1.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif.c |2 +-
ofproto/ofproto-dpif.h |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5a64336..6f87aa6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4781,7 +4781,7 @@ bool
rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
const struct flow *flow, struct flow_wildcards *wc,
uint8_t table_id, struct rule_dpif **rule)
-OVS_ACQ_RDLOCK((*rule)->up.evict)
+OVS_TRY_RDLOCK(true, (*rule)->up.evict)
{
struct cls_rule *cls_rule;
struct classifier *cls;
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 6a4ae07..15e58e9 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -91,7 +91,7 @@ void rule_dpif_lookup(struct ofproto_dpif *, const struct
flow *,
bool rule_dpif_lookup_in_table(struct ofproto_dpif *, const struct flow *,
struct flow_wildcards *, uint8_t table_id,
struct rule_dpif **rule)
-OVS_ACQ_RDLOCK((*rule)->up.evict);
+OVS_TRY_RDLOCK(true, (*rule)->up.evict);
void rule_release(struct rule_dpif *rule) OVS_RELEASES(rule->up.evict);
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/2] netdev: Make run and wait functions optional as documented.
On Wed, Aug 21, 2013 at 01:00:19PM -0700, Ethan Jackson wrote: > Signed-off-by: Ethan Jackson > Reported-by: Guolin Yang Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/2] ofproto-dpif-upcall: Increase upcall stub size.
On Wed, Aug 21, 2013 at 01:00:18PM -0700, Ethan Jackson wrote: > The existing upcall stub size is not large enough even for the > smallest miss upcalls. > > Signed-off-by: Ethan Jackson > Reported-by: Guolin Yang Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Reinterpret base for meter band types bitmap.
On Wed, Aug 21, 2013 at 11:26:13AM -0700, Jarno Rajahalme wrote: > On Aug 12, 2013, at 9:57 PM, Ben Pfaff wrote: > > > OpenFlow 1.3 says that the band_types member of struct ofp_meter_features > > is a bitmap of OFPMBT_* values. The OFPMBT_* values are 1-based, so > > until now, to avoid wasting bit 0, OVS mapped an OFPMBT_* with value 1 to > > bit 0, value 2 to bit 1, and so on. However, according to > > http://openvswitch.org/pipermail/dev/2013-July/029666.html, > > other OpenFlow implementations directly translate value 1 to bit 1, > > value 2 to bit 2, and so on. This commit changes Open vSwitch to use this > > more common interpretation. > > > > A request for clarification of this issue in the OpenFlow standard has been > > filed with the ONF Extensibility Working Group as issue EXT-345. > > > > Should have raised the issue while I was writing that.. > > > Reported-by: YAMAMOTO Takashi > > Signed-off-by: Ben Pfaff > > > > Acked-by: Jarno Rajahalme Thanks, applied to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ipfix: implement flow caching and aggregation in exporter
On Tue, Aug 20, 2013 at 05:54:27PM -0700, Romain Lenglet wrote: > Implement a per-exporter flow cache with active timeout expiration. > Add columns "cache_active_timeout" and "cache_max_flows" into table > "IPFIX" to configure each cache. > > Add per-flow elements "octetDeltaSumOfSquares", > "minimumIpTotalLength", and "maximumIpTotalLength" to replace > "ethernetTotalLength". Add per-flow element "flowEndReason" to > indicate whether a flow has expired because of an active timeout, the > cache size limit being reached, or the exporter being stopped. > > Signed-off-by: Romain Lenglet I'm happy with the code. I don't exactly understand what this provides to users, so can you provide a brief item to add to NEWS that explains it? (Is the benefit directly user-visible?) Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ipfix: implement flow caching and aggregation in exporter
- Original Message - > From: "Ben Pfaff" > To: "Romain Lenglet" > Cc: dev@openvswitch.org > Sent: Wednesday, August 21, 2013 1:25:59 PM > Subject: Re: [PATCH] ipfix: implement flow caching and aggregation in exporter > > On Tue, Aug 20, 2013 at 05:54:27PM -0700, Romain Lenglet wrote: > > Implement a per-exporter flow cache with active timeout expiration. > > Add columns "cache_active_timeout" and "cache_max_flows" into table > > "IPFIX" to configure each cache. > > > > Add per-flow elements "octetDeltaSumOfSquares", > > "minimumIpTotalLength", and "maximumIpTotalLength" to replace > > "ethernetTotalLength". Add per-flow element "flowEndReason" to > > indicate whether a flow has expired because of an active timeout, the > > cache size limit being reached, or the exporter being stopped. > > > > Signed-off-by: Romain Lenglet > > I'm happy with the code. I don't exactly understand what this > provides to users, so can you provide a brief item to add to NEWS that > explains it? (Is the benefit directly user-visible?) This basically makes IPFIX usable. Without caching it generates one message for every sampled packet, which can be a lot of traffic. This can be useful as it makes it behave like sFlow, but it is not what users usually expect from an IPFIX exporter. I added this item into NEWS: - Added configurable flow caching support to IPFIX exporter. Updated patch following shortly. Thanks for your reviews! -- Romain Lenglet ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ipfix: implement flow caching and aggregation in exporter
Implement a per-exporter flow cache with active timeout expiration.
Add columns "cache_active_timeout" and "cache_max_flows" into table
"IPFIX" to configure each cache.
Add per-flow elements "octetDeltaSumOfSquares",
"minimumIpTotalLength", and "maximumIpTotalLength" to replace
"ethernetTotalLength". Add per-flow element "flowEndReason" to
indicate whether a flow has expired because of an active timeout, the
cache size limit being reached, or the exporter being stopped.
Signed-off-by: Romain Lenglet
---
NEWS |1 +
ofproto/ofproto-dpif-ipfix.c | 704 --
ofproto/ofproto-dpif-ipfix.h |3 +
ofproto/ofproto-dpif.c | 23 +-
ofproto/ofproto.h|4 +
utilities/ovs-vsctl.8.in |5 +-
vswitchd/bridge.c| 10 +
vswitchd/vswitch.ovsschema | 12 +-
vswitchd/vswitch.xml | 12 +
9 files changed, 676 insertions(+), 98 deletions(-)
diff --git a/NEWS b/NEWS
index 1246383..05c3d8f 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,7 @@ v1.12.0 - xx xxx
- Support for Linux kernels up to 3.10
- ovs-ofctl:
* New "ofp-parse" for printing OpenFlow messages read from a file.
+- Added configurable flow caching support to IPFIX exporter.
v1.11.0 - xx xxx
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 8e8e7a2..a9cc73e 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -16,14 +16,17 @@
#include
#include "ofproto-dpif-ipfix.h"
+#include
#include "byte-order.h"
#include "collectors.h"
#include "flow.h"
#include "hash.h"
#include "hmap.h"
+#include "list.h"
#include "ofpbuf.h"
#include "ofproto.h"
#include "packets.h"
+#include "poll-loop.h"
#include "sset.h"
#include "util.h"
#include "timeval.h"
@@ -42,6 +45,10 @@ struct dpif_ipfix_exporter {
struct collectors *collectors;
uint32_t seq_number;
time_t last_template_set_time;
+struct hmap cache_flow_key_map; /* ipfix_flow_cache_entry. */
+struct list cache_flow_start_timestamp_list; /* ipfix_flow_cache_entry. */
+uint32_t cache_active_timeout; /* In seconds. */
+uint32_t cache_max_flows;
};
struct dpif_ipfix_bridge_exporter {
@@ -62,7 +69,7 @@ struct dpif_ipfix_flow_exporter_map_node {
struct dpif_ipfix {
struct dpif_ipfix_bridge_exporter bridge_exporter;
-struct hmap flow_exporter_map; /* dpif_ipfix_flow_exporter_map_nodes. */
+struct hmap flow_exporter_map; /* dpif_ipfix_flow_exporter_map_node. */
atomic_int ref_cnt;
};
@@ -143,32 +150,30 @@ struct ipfix_template_field_specifier {
});
BUILD_ASSERT_DECL(sizeof(struct ipfix_template_field_specifier) == 4);
-/* Part of data record for common metadata and Ethernet entities. */
+/* Part of data record flow key for common metadata and Ethernet entities. */
OVS_PACKED(
-struct ipfix_data_record_common {
+struct ipfix_data_record_flow_key_common {
ovs_be32 observation_point_id; /* OBSERVATION_POINT_ID */
-ovs_be64 packet_delta_count; /* PACKET_DELTA_COUNT */
-ovs_be64 layer2_octet_delta_count; /* LAYER2_OCTET_DELTA_COUNT */
uint8_t source_mac_address[6]; /* SOURCE_MAC_ADDRESS */
uint8_t destination_mac_address[6]; /* DESTINATION_MAC_ADDRESS */
ovs_be16 ethernet_type; /* ETHERNET_TYPE */
-ovs_be16 ethernet_total_length; /* ETHERNET_TOTAL_LENGTH */
uint8_t ethernet_header_length; /* ETHERNET_HEADER_LENGTH */
});
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_common) == 37);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_common) == 19);
-/* Part of data record for VLAN entities. */
+/* Part of data record flow key for VLAN entities. */
OVS_PACKED(
-struct ipfix_data_record_vlan {
+struct ipfix_data_record_flow_key_vlan {
ovs_be16 vlan_id; /* VLAN_ID */
ovs_be16 dot1q_vlan_id; /* DOT1Q_VLAN_ID */
uint8_t dot1q_priority; /* DOT1Q_PRIORITY */
});
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_vlan) == 5);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_vlan) == 5);
-/* Part of data record for IP entities. */
+/* Part of data record flow key for IP entities. */
+/* XXX: Replace IP_TTL with MINIMUM_TTL and MAXIMUM_TTL? */
OVS_PACKED(
-struct ipfix_data_record_ip {
+struct ipfix_data_record_flow_key_ip {
uint8_t ip_version; /* IP_VERSION */
uint8_t ip_ttl; /* IP_TTL */
uint8_t protocol_identifier; /* PROTOCOL_IDENTIFIER */
@@ -176,32 +181,116 @@ struct ipfix_data_record_ip {
uint8_t ip_precedence; /* IP_PRECEDENCE */
uint8_t ip_class_of_service; /* IP_CLASS_OF_SERVICE */
});
-BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_ip) == 6);
+BUILD_ASSERT_DECL(sizeof(struct ipfix_data_record_flow_key_ip) == 6);
-/* Part of data record for IPv4 entities. */
+/* Part of data record flow key for IPv4 entities. */
OVS_PACKED(
-struct ipfix_data_record_ipv4 {
+struct ipfix_data_record_flow_key_ipv4 {
ovs_be32 source_ipv
Re: [ovs-dev] [PATCH] ipfix: implement flow caching and aggregation in exporter
On Wed, Aug 21, 2013 at 01:47:45PM -0700, Romain Lenglet wrote: > - Original Message - > > From: "Ben Pfaff" > > To: "Romain Lenglet" > > Cc: dev@openvswitch.org > > Sent: Wednesday, August 21, 2013 1:25:59 PM > > Subject: Re: [PATCH] ipfix: implement flow caching and aggregation in > > exporter > > > > On Tue, Aug 20, 2013 at 05:54:27PM -0700, Romain Lenglet wrote: > > > Implement a per-exporter flow cache with active timeout expiration. > > > Add columns "cache_active_timeout" and "cache_max_flows" into table > > > "IPFIX" to configure each cache. > > > > > > Add per-flow elements "octetDeltaSumOfSquares", > > > "minimumIpTotalLength", and "maximumIpTotalLength" to replace > > > "ethernetTotalLength". Add per-flow element "flowEndReason" to > > > indicate whether a flow has expired because of an active timeout, the > > > cache size limit being reached, or the exporter being stopped. > > > > > > Signed-off-by: Romain Lenglet > > > > I'm happy with the code. I don't exactly understand what this > > provides to users, so can you provide a brief item to add to NEWS that > > explains it? (Is the benefit directly user-visible?) > > This basically makes IPFIX usable. Without caching it generates one > message for every sampled packet, which can be a lot of traffic. > This can be useful as it makes it behave like sFlow, but it is not what > users usually expect from an IPFIX exporter. > > I added this item into NEWS: > - Added configurable flow caching support to IPFIX exporter. Thanks, that should help our users (and does help me). ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto: Fix typo in comment.
Signed-off-by: Ben Pfaff
---
ofproto/ofproto.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 1e09c56..7b4ef41 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5124,7 +5124,7 @@ choose_rule_to_evict(struct oftable *table, struct rule
**rulep)
* group has no evictable rules.
*
* - The outer loop can exit only if table's 'max_flows' is all filled up
- * by unevictable rules'. */
+ * by unevictable rules. */
HEAP_FOR_EACH (evg, size_node, &table->eviction_groups_by_size) {
struct rule *rule;
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofproto: Fix typo in comment.
On Aug 21, 2013, at 4:15 PM, Ben Pfaff wrote: > Signed-off-by: Ben Pfaff Looks good. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 2/2] bfd: Implements forwarding_if_rx
This patch is basically ready, two minor comments and we'll get it merged today. First the title line should be changed to "bfd: Implement forwarding_if_rx." Note the period. > > -return bfd->state == STATE_UP > -&& bfd->rmt_diag != DIAG_PATH_DOWN > -&& bfd->rmt_diag != DIAG_CPATH_DOWN > -&& bfd->rmt_diag != DIAG_RCPATH_DOWN; > +return (bfd->forwarding_if_rx > + ? bfd->forwarding_if_rx_detect_time > time_msec() > + : bfd->state == STATE_UP) > + && bfd->rmt_diag != DIAG_PATH_DOWN > + && bfd->rmt_diag != DIAG_CPATH_DOWN > + && bfd->rmt_diag != DIAG_RCPATH_DOWN; The indentation here isn't entirely correct. Also the logic doesn't seem entirely correct. I think we want something more like bfd->state == STATE_UP || (bfd->forwarding_if_rx && bfd->forwarding_if_rx_detect_time > time_msec()) I.E. if the state is up, we should be up no matter what forwarding if rx says. Btw this code reminds me, there's a bug with this logic which Paul fixed in the CFM code a while back. We should probably port the fix to BFD as well (in a separate patch). Could you have a look at it after this is merged? commit 016953a Author: Paul Ingram Date: Sat Aug 3 07:12:36 2013 + cfm: update remote opstate only when a CCM is received. Basically if the remote endpoint signaled cpath down, and forwarding_if_rx is enabled, that state should be sticky even if we lose some of their packets. Thanks, Ethan ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH V2 2/2] bfd: Implements forwarding_if_rx
On Wed, Aug 21, 2013 at 2:26 PM, Ethan Jackson wrote: > This patch is basically ready, two minor comments and we'll get it merged > today. > > First the title line should be changed to "bfd: Implement > forwarding_if_rx." Note the period. > > Yes, how could I miss that. > > > > -return bfd->state == STATE_UP > > -&& bfd->rmt_diag != DIAG_PATH_DOWN > > -&& bfd->rmt_diag != DIAG_CPATH_DOWN > > -&& bfd->rmt_diag != DIAG_RCPATH_DOWN; > > +return (bfd->forwarding_if_rx > > + ? bfd->forwarding_if_rx_detect_time > time_msec() > > + : bfd->state == STATE_UP) > > + && bfd->rmt_diag != DIAG_PATH_DOWN > > + && bfd->rmt_diag != DIAG_CPATH_DOWN > > + && bfd->rmt_diag != DIAG_RCPATH_DOWN; > > The indentation here isn't entirely correct. > > Also the logic doesn't seem entirely correct. I think we want > something more like bfd->state == STATE_UP || (bfd->forwarding_if_rx > && bfd->forwarding_if_rx_detect_time > time_msec()) I.E. if the state > is up, we should be up no matter what forwarding if rx says. > Makes sense, I'll fix accordingly, > Btw this code reminds me, there's a bug with this logic which Paul > fixed in the CFM code a while back. We should probably port the fix > to BFD as well (in a separate patch). Could you have a look at it > after this is merged? > > Absolutely! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [clang 1/4] ovs-thread: Mark lock and unlock functions as no_thread_safety_analysis.
Acked-by: Ethan Jackson
On Wed, Aug 21, 2013 at 1:01 PM, Ben Pfaff wrote:
> I don't see any other way to make Clang realize that these are the real
> mutex implementation functions.
>
> I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
> I previously used version 1:3.4~svn187484-1~exp1.
>
> Signed-off-by: Ben Pfaff
> ---
> lib/compiler.h |3 +++
> lib/ovs-thread.c |3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/lib/compiler.h b/lib/compiler.h
> index 519b832..fb4d46c 100644
> --- a/lib/compiler.h
> +++ b/lib/compiler.h
> @@ -128,6 +128,8 @@
> #define OVS_EXCLUDED(...) __attribute__((locks_excluded(__VA_ARGS__)))
> #define OVS_ACQ_BEFORE(...) __attribute__((acquired_before(__VA_ARGS__)))
> #define OVS_ACQ_AFTER(...) __attribute__((acquired_after(__VA_ARGS__)))
> +#define OVS_NO_THREAD_SAFETY_ANALYSIS \
> +__attribute__((no_thread_safety_analysis))
> #else /* not Clang */
> #define OVS_LOCKABLE
> #define OVS_REQ_RDLOCK(...)
> @@ -145,6 +147,7 @@
> #define OVS_RELEASES(...)
> #define OVS_ACQ_BEFORE(...)
> #define OVS_ACQ_AFTER(...)
> +#define OVS_NO_THREAD_SAFETY_ANALYSIS
> #endif
>
> /* ISO C says that a C implementation may choose any integer type for an enum
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index e9366e2..b3a87bb 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -47,6 +47,7 @@ static bool multithreaded;
> void \
> ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \
> const char *where) \
> +OVS_NO_THREAD_SAFETY_ANALYSIS \
> { \
> struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
> int error = pthread_##TYPE##_##FUN(&l->lock); \
> @@ -63,6 +64,7 @@ LOCK_FUNCTION(rwlock, wrlock);
> int \
> ovs_##TYPE##_##FUN##_at(const struct ovs_##TYPE *l_, \
> const char *where) \
> +OVS_NO_THREAD_SAFETY_ANALYSIS \
> { \
> struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
> int error = pthread_##TYPE##_##FUN(&l->lock); \
> @@ -81,6 +83,7 @@ TRY_LOCK_FUNCTION(rwlock, trywrlock);
> #define UNLOCK_FUNCTION(TYPE, FUN) \
> void \
> ovs_##TYPE##_##FUN(const struct ovs_##TYPE *l_) \
> +OVS_NO_THREAD_SAFETY_ANALYSIS \
> { \
> struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
> int error; \
> --
> 1.7.10.4
>
> ___
> 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] [PATCH] ofp-parse: Silence uninitialized use warnings with optimized gcc.
GCC 4.6.3 gets confused by the str_to_*() functions in ofp-parse and
spits out the following warning.
error: ‘priority’ may be used uninitialized in this function
Signed-off-by: Ethan Jackson
---
lib/ofp-parse.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 5cb39f5..2983474 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -132,7 +132,7 @@ str_to_u64(const char *str, uint64_t *valuep)
static char * WARN_UNUSED_RESULT
str_to_be64(const char *str, ovs_be64 *valuep)
{
-uint64_t value;
+uint64_t value = 0;
char *error;
error = str_to_u64(str, &value);
@@ -246,7 +246,7 @@ parse_resubmit(char *arg, struct ofpbuf *ofpacts)
table_s = strsep(&arg, ",");
if (table_s && table_s[0]) {
-uint32_t table_id;
+uint32_t table_id = 0;
char *error;
error = str_to_u32(table_s, &table_id);
@@ -598,10 +598,10 @@ parse_named_action(enum ofputil_action_code code,
size_t orig_size = ofpacts->size;
struct ofpact_tunnel *tunnel;
char *error = NULL;
-uint16_t ethertype;
-uint16_t vid;
-uint8_t pcp;
-uint8_t tos;
+uint16_t ethertype = 0;
+uint16_t vid = 0;
+uint8_t tos = 0;
+uint8_t pcp = 0;
switch (code) {
case OFPUTIL_ACTION_INVALID:
@@ -1184,7 +1184,7 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command,
char *string)
value);
}
} else if (fields & F_PRIORITY && !strcmp(name, "priority")) {
-uint16_t priority;
+uint16_t priority = 0;
error = str_to_u16(value, name, &priority);
fm->priority = priority;
--
1.7.9.5
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofp-parse: Silence uninitialized use warnings with optimized gcc.
On Wed, Aug 21, 2013 at 02:50:59PM -0700, Ethan Jackson wrote: > GCC 4.6.3 gets confused by the str_to_*() functions in ofp-parse and > spits out the following warning. > > error: ???priority??? may be used uninitialized in this function > > Signed-off-by: Ethan Jackson Seems reasonable. Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofp-parse: Silence uninitialized use warnings with optimized gcc.
Thanks, merged. Ethan On Wed, Aug 21, 2013 at 2:54 PM, Ben Pfaff wrote: > On Wed, Aug 21, 2013 at 02:50:59PM -0700, Ethan Jackson wrote: >> GCC 4.6.3 gets confused by the str_to_*() functions in ofp-parse and >> spits out the following warning. >> >> error: ???priority??? may be used uninitialized in this function >> >> Signed-off-by: Ethan Jackson > > Seems reasonable. > > Acked-by: Ben Pfaff ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH V3 2/2] bfd: Implement forwarding_if_rx.
This commit adds a new boolean option "forwarding_if_rx" to bfd.
When forwarding_if_rx is true the interface will be considered
capabale of packet I/O as long as there is packet received at
interface. This is important in that when link becomes temporarily
conjested, consecutive BFD control packets can be lost. And the
forwarding_if_rx can prevent link failover by detecting non-control
packets received at interface.
Signed-off-by: Alex Wang
---
v2 -> v3:
- fix typo in commit log.
- fix indentation in bfd_forwarding__().
- refine the return logic in bfd_forwarding__().
v1 -> v2:
- update the forwarding_if_rx_detect_time when netdev is changed.
- use rate-limited log function.
- refine the commit log and vswitchd.xml.
- add unit test for situation when both forwarding_if_rx and bfd
decay are enabled.
---
lib/bfd.c| 72 +--
tests/bfd.at | 251 +-
vswitchd/vswitch.xml |8 ++
3 files changed, 321 insertions(+), 10 deletions(-)
diff --git a/lib/bfd.c b/lib/bfd.c
index 47e67df..c782481 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -191,12 +191,19 @@ struct bfd {
atomic_bool check_tnl_key;/* Verify tunnel key of inbound packets? */
atomic_int ref_cnt;
+/* When forward_if_rx is true, bfd_forwarding() will return
+ * true as long as there are incoming packets received.
+ * Note, forwarding_override still has higher priority. */
+bool forwarding_if_rx;
+long long int forwarding_if_rx_detect_time;
+
/* BFD decay related variables. */
bool in_decay;/* True when bfd is in decay. */
int decay_min_rx; /* min_rx is set to decay_min_rx when */
/* in decay. */
int decay_rx_ctl; /* Count bfd packets received within decay */
/* detect interval. */
+uint64_t decay_rx_packets;/* Packets received by 'netdev'. */
long long int decay_detect_time; /* Decay detection time. */
};
@@ -223,6 +230,8 @@ static void bfd_put_details(struct ds *, const struct bfd *)
static uint64_t bfd_rx_packets(const struct bfd *) OVS_REQUIRES(mutex);
static void bfd_try_decay(struct bfd *) OVS_REQUIRES(mutex);
static void bfd_decay_update(struct bfd *) OVS_REQUIRES(mutex);
+static void bfd_check_rx(struct bfd *) OVS_REQUIRES(mutex);
+static void bfd_forwarding_if_rx_update(struct bfd *) OVS_REQUIRES(mutex);
static void bfd_unixctl_show(struct unixctl_conn *, int argc,
const char *argv[], void *aux OVS_UNUSED);
static void bfd_unixctl_set_forwarding_override(struct unixctl_conn *,
@@ -280,7 +289,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
long long int min_tx, min_rx;
bool need_poll = false;
bool cfg_min_rx_changed = false;
-bool cpath_down;
+bool cpath_down, forwarding_if_rx;
const char *hwaddr;
uint8_t ea[ETH_ADDR_LEN];
@@ -311,6 +320,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->mult = 3;
atomic_init(&bfd->ref_cnt, 1);
bfd->netdev = netdev_ref(netdev);
+bfd->rx_packets = bfd_rx_packets(bfd);
bfd->in_decay = false;
/* RFC 5881 section 4
@@ -384,6 +394,16 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->eth_dst_set = false;
}
+forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
+if (bfd->forwarding_if_rx != forwarding_if_rx) {
+bfd->forwarding_if_rx = forwarding_if_rx;
+if (bfd->state == STATE_UP && bfd->forwarding_if_rx) {
+bfd_forwarding_if_rx_update(bfd);
+} else {
+bfd->forwarding_if_rx_detect_time = 0;
+}
+}
+
if (need_poll) {
bfd_poll(bfd);
}
@@ -458,6 +478,9 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
bfd_try_decay(bfd);
}
+/* Always checks the reception of any packet. */
+bfd_check_rx(bfd);
+
if (bfd->min_tx != bfd->cfg_min_tx
|| (bfd->min_rx != bfd->cfg_min_rx && bfd->min_rx != bfd->decay_min_rx)
|| bfd->in_decay != old_in_decay) {
@@ -752,9 +775,13 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev
*netdev)
if (bfd->netdev != netdev) {
netdev_close(bfd->netdev);
bfd->netdev = netdev_ref(netdev);
-if (bfd->decay_min_rx) {
+if (bfd->decay_min_rx && bfd->state == STATE_UP) {
bfd_decay_update(bfd);
}
+if (bfd->forwarding_if_rx && bfd->state == STATE_UP) {
+bfd_forwarding_if_rx_update(bfd);
+}
+bfd->rx_packets = bfd_rx_packets(bfd);
}
ovs_mutex_unlock(&mutex);
}
@@ -767,10 +794,12 @@ bfd_forwarding__(const struct bfd *bfd)
OVS_REQUIRES(mutex)
return bfd->forwarding_override == 1;
}
-return bfd->state == STATE_UP
-&& bfd->rmt_diag != DIAG_PATH_DOWN
Re: [ovs-dev] OpenvSwitch DKMS compatibility with Linux 3.11
On Wed, Aug 21, 2013 at 10:06 AM, James Page wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA256 > > On 21/08/13 18:01, Jesse Gross wrote: On 21/08/13 17:16, Jesse Gross wrote: >> What's the status of OVS 1.12? Is this just a dev >> branch or is it planned for release? >> It's actually a dead branch. It looks like we're going to >> get a few more things in and call the version after 1.11 >> 2.0. OK - so it sounds like I should be landing a snapshot based on the 1.11 branch into Ubuntu Saucy - albeit with the cherry picks for 3.10 and my patch for 3.11 ontop... Is that correct? >> That sounds right to me. >> > > Whats the timescale around 2.0? It's somewhat hard to say at this point since we haven't branched yet and therefore really started stabilizing it. There's a lot of new multithreading work in userspace which could be exciting from a stability perspective in the interim, so I think 1.11 is probably the right version for Saucy. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [clang 3/4] ofproto-dpif: Mark rule_release() as no_thread_safety_analysis.
looks good.
Kais
On Wed, Aug 21, 2013 at 1:01 PM, Ben Pfaff wrote:
> Otherwise new Clang complains about this function because it only sometimes
> releases the lock (that is, it only does it when there is a lock to
> release).
>
> I first noticed these warnings with Clang 1:3.4~svn188890-1~exp1.
> I previously used version 1:3.4~svn187484-1~exp1.
>
> Signed-off-by: Ben Pfaff
> ---
> ofproto/ofproto-dpif.c |1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 6f87aa6..4f1da53 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -4838,6 +4838,7 @@ choose_miss_rule(enum ofputil_port_config config,
> struct rule_dpif *miss_rule,
>
> void
> rule_release(struct rule_dpif *rule)
> +OVS_NO_THREAD_SAFETY_ANALYSIS
> {
> if (rule) {
> ovs_rwlock_unlock(&rule->up.evict);
> --
> 1.7.10.4
>
> ___
> 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 V3 2/2] bfd: Implement forwarding_if_rx.
Really sorry, this patch does not include all the changes. Please ignore
this patch.
On Wed, Aug 21, 2013 at 3:24 PM, Alex Wang wrote:
> This commit adds a new boolean option "forwarding_if_rx" to bfd.
>
> When forwarding_if_rx is true the interface will be considered
> capabale of packet I/O as long as there is packet received at
> interface. This is important in that when link becomes temporarily
> conjested, consecutive BFD control packets can be lost. And the
> forwarding_if_rx can prevent link failover by detecting non-control
> packets received at interface.
>
> Signed-off-by: Alex Wang
>
> ---
>
> v2 -> v3:
> - fix typo in commit log.
> - fix indentation in bfd_forwarding__().
> - refine the return logic in bfd_forwarding__().
>
> v1 -> v2:
> - update the forwarding_if_rx_detect_time when netdev is changed.
> - use rate-limited log function.
> - refine the commit log and vswitchd.xml.
> - add unit test for situation when both forwarding_if_rx and bfd
> decay are enabled.
>
> ---
> lib/bfd.c| 72 +--
> tests/bfd.at | 251
> +-
> vswitchd/vswitch.xml |8 ++
> 3 files changed, 321 insertions(+), 10 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index 47e67df..c782481 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -191,12 +191,19 @@ struct bfd {
> atomic_bool check_tnl_key;/* Verify tunnel key of inbound
> packets? */
> atomic_int ref_cnt;
>
> +/* When forward_if_rx is true, bfd_forwarding() will return
> + * true as long as there are incoming packets received.
> + * Note, forwarding_override still has higher priority. */
> +bool forwarding_if_rx;
> +long long int forwarding_if_rx_detect_time;
> +
> /* BFD decay related variables. */
> bool in_decay;/* True when bfd is in decay. */
> int decay_min_rx; /* min_rx is set to decay_min_rx when */
>/* in decay. */
> int decay_rx_ctl; /* Count bfd packets received within
> decay */
>/* detect interval. */
> +uint64_t decay_rx_packets;/* Packets received by 'netdev'. */
> long long int decay_detect_time; /* Decay detection time. */
> };
>
> @@ -223,6 +230,8 @@ static void bfd_put_details(struct ds *, const struct
> bfd *)
> static uint64_t bfd_rx_packets(const struct bfd *) OVS_REQUIRES(mutex);
> static void bfd_try_decay(struct bfd *) OVS_REQUIRES(mutex);
> static void bfd_decay_update(struct bfd *) OVS_REQUIRES(mutex);
> +static void bfd_check_rx(struct bfd *) OVS_REQUIRES(mutex);
> +static void bfd_forwarding_if_rx_update(struct bfd *) OVS_REQUIRES(mutex);
> static void bfd_unixctl_show(struct unixctl_conn *, int argc,
> const char *argv[], void *aux OVS_UNUSED);
> static void bfd_unixctl_set_forwarding_override(struct unixctl_conn *,
> @@ -280,7 +289,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> struct smap *cfg,
> long long int min_tx, min_rx;
> bool need_poll = false;
> bool cfg_min_rx_changed = false;
> -bool cpath_down;
> +bool cpath_down, forwarding_if_rx;
> const char *hwaddr;
> uint8_t ea[ETH_ADDR_LEN];
>
> @@ -311,6 +320,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
> struct smap *cfg,
> bfd->mult = 3;
> atomic_init(&bfd->ref_cnt, 1);
> bfd->netdev = netdev_ref(netdev);
> +bfd->rx_packets = bfd_rx_packets(bfd);
> bfd->in_decay = false;
>
> /* RFC 5881 section 4
> @@ -384,6 +394,16 @@ bfd_configure(struct bfd *bfd, const char *name,
> const struct smap *cfg,
> bfd->eth_dst_set = false;
> }
>
> +forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
> +if (bfd->forwarding_if_rx != forwarding_if_rx) {
> +bfd->forwarding_if_rx = forwarding_if_rx;
> +if (bfd->state == STATE_UP && bfd->forwarding_if_rx) {
> +bfd_forwarding_if_rx_update(bfd);
> +} else {
> +bfd->forwarding_if_rx_detect_time = 0;
> +}
> +}
> +
> if (need_poll) {
> bfd_poll(bfd);
> }
> @@ -458,6 +478,9 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
> bfd_try_decay(bfd);
> }
>
> +/* Always checks the reception of any packet. */
> +bfd_check_rx(bfd);
> +
> if (bfd->min_tx != bfd->cfg_min_tx
> || (bfd->min_rx != bfd->cfg_min_rx && bfd->min_rx !=
> bfd->decay_min_rx)
> || bfd->in_decay != old_in_decay) {
> @@ -752,9 +775,13 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev
> *netdev)
> if (bfd->netdev != netdev) {
> netdev_close(bfd->netdev);
> bfd->netdev = netdev_ref(netdev);
> -if (bfd->decay_min_rx) {
> +if (bfd->decay_min_rx && bfd->state == STATE_UP) {
> bfd_decay_update(bfd);
> }
> +if (bfd->forwarding_if_rx && bfd->state == STATE_UP
[ovs-dev] [PATCH V3 2/2] bfd: Implement forwarding_if_rx.
This commit adds a new boolean option "forwarding_if_rx" to bfd.
When forwarding_if_rx is true the interface will be considered
capable of packet I/O as long as there is packet received at
interface. This is important in that when link becomes temporarily
conjested, consecutive BFD control packets can be lost. And the
forwarding_if_rx can prevent link failover by detecting non-control
packets received at interface.
Signed-off-by: Alex Wang
---
v2 -> v3:
- fix typo in commit log.
- fix indentation in bfd_forwarding__().
- refine the return logic in bfd_forwarding__().
v1 -> v2:
- update the forwarding_if_rx_detect_time when netdev is changed.
- use rate-limited log function.
- refine the commit log and vswitchd.xml.
- add unit test for situation when both forwarding_if_rx and bfd
decay are enabled.
---
lib/bfd.c| 74 +--
tests/bfd.at | 251 +-
vswitchd/vswitch.xml |8 ++
3 files changed, 323 insertions(+), 10 deletions(-)
diff --git a/lib/bfd.c b/lib/bfd.c
index 47e67df..7da6fd9 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -191,12 +191,19 @@ struct bfd {
atomic_bool check_tnl_key;/* Verify tunnel key of inbound packets? */
atomic_int ref_cnt;
+/* When forward_if_rx is true, bfd_forwarding() will return
+ * true as long as there are incoming packets received.
+ * Note, forwarding_override still has higher priority. */
+bool forwarding_if_rx;
+long long int forwarding_if_rx_detect_time;
+
/* BFD decay related variables. */
bool in_decay;/* True when bfd is in decay. */
int decay_min_rx; /* min_rx is set to decay_min_rx when */
/* in decay. */
int decay_rx_ctl; /* Count bfd packets received within decay */
/* detect interval. */
+uint64_t decay_rx_packets;/* Packets received by 'netdev'. */
long long int decay_detect_time; /* Decay detection time. */
};
@@ -223,6 +230,8 @@ static void bfd_put_details(struct ds *, const struct bfd *)
static uint64_t bfd_rx_packets(const struct bfd *) OVS_REQUIRES(mutex);
static void bfd_try_decay(struct bfd *) OVS_REQUIRES(mutex);
static void bfd_decay_update(struct bfd *) OVS_REQUIRES(mutex);
+static void bfd_check_rx(struct bfd *) OVS_REQUIRES(mutex);
+static void bfd_forwarding_if_rx_update(struct bfd *) OVS_REQUIRES(mutex);
static void bfd_unixctl_show(struct unixctl_conn *, int argc,
const char *argv[], void *aux OVS_UNUSED);
static void bfd_unixctl_set_forwarding_override(struct unixctl_conn *,
@@ -280,7 +289,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
long long int min_tx, min_rx;
bool need_poll = false;
bool cfg_min_rx_changed = false;
-bool cpath_down;
+bool cpath_down, forwarding_if_rx;
const char *hwaddr;
uint8_t ea[ETH_ADDR_LEN];
@@ -311,6 +320,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->mult = 3;
atomic_init(&bfd->ref_cnt, 1);
bfd->netdev = netdev_ref(netdev);
+bfd->rx_packets = bfd_rx_packets(bfd);
bfd->in_decay = false;
/* RFC 5881 section 4
@@ -384,6 +394,16 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->eth_dst_set = false;
}
+forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false);
+if (bfd->forwarding_if_rx != forwarding_if_rx) {
+bfd->forwarding_if_rx = forwarding_if_rx;
+if (bfd->state == STATE_UP && bfd->forwarding_if_rx) {
+bfd_forwarding_if_rx_update(bfd);
+} else {
+bfd->forwarding_if_rx_detect_time = 0;
+}
+}
+
if (need_poll) {
bfd_poll(bfd);
}
@@ -458,6 +478,9 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
bfd_try_decay(bfd);
}
+/* Always checks the reception of any packet. */
+bfd_check_rx(bfd);
+
if (bfd->min_tx != bfd->cfg_min_tx
|| (bfd->min_rx != bfd->cfg_min_rx && bfd->min_rx != bfd->decay_min_rx)
|| bfd->in_decay != old_in_decay) {
@@ -752,9 +775,13 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev
*netdev)
if (bfd->netdev != netdev) {
netdev_close(bfd->netdev);
bfd->netdev = netdev_ref(netdev);
-if (bfd->decay_min_rx) {
+if (bfd->decay_min_rx && bfd->state == STATE_UP) {
bfd_decay_update(bfd);
}
+if (bfd->forwarding_if_rx && bfd->state == STATE_UP) {
+bfd_forwarding_if_rx_update(bfd);
+}
+bfd->rx_packets = bfd_rx_packets(bfd);
}
ovs_mutex_unlock(&mutex);
}
@@ -763,14 +790,18 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev
*netdev)
static bool
bfd_forwarding__(const struct bfd *bfd) OVS_REQUIRES(mutex)
{
+long long int time;
+
if (bfd->forwarding_overr
Re: [ovs-dev] [PATCH V2 2/2] bfd: Implements forwarding_if_rx
Hey Ethan,
> Basically if the remote endpoint signaled cpath down, and
> forwarding_if_rx is enabled, that state should be sticky even if we
> lose some of their packets.
>
We do have a similar issue in BFD. I'm not sure if I fully understand your
suggestion. But I have a intuitive fix below:
"""
+++ b/lib/bfd.c
@@ -953,8 +953,10 @@ bfd_set_state(struct bfd *bfd, enum state state, enum
diag diag)
bfd->diag = diag;
if (bfd->state <= STATE_DOWN) {
+if (!bfd->forwarding_if_rx) {
+bfd->rmt_diag = DIAG_NONE;
+}
bfd->rmt_state = STATE_DOWN;
-bfd->rmt_diag = DIAG_NONE;
bfd->rmt_min_rx = 1;
bfd->rmt_flags = 0;
bfd->rmt_disc = 0;
"""
So, when "bfd->forwarding_if_rx" is enabled, we always retain the
"rmt_diag". And it can only be changed by processing the bfd control
packets received.
Thanks,
Alex Wang,
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] ofproto-dpif: add thread-safety annotation to vsp_{remove, add}
This corrects an minor oversight in "ofproto-dpif: Make vlan splinters thread
safe." which add thread-safety to these functions.
Signed-off-by: Simon Horman
---
ofproto/ofproto-dpif.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5a64336..1a23955 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6165,6 +6165,7 @@ vsp_adjust_flow(const struct ofproto_dpif *ofproto,
struct flow *flow)
static void
vsp_remove(struct ofport_dpif *port)
+OVS_EXCLUDED(ofproto->vsp_mutex)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
struct vlan_splinter *vsp;
@@ -6185,6 +6186,7 @@ vsp_remove(struct ofport_dpif *port)
static void
vsp_add(struct ofport_dpif *port, ofp_port_t realdev_ofp_port, int vid)
+OVS_EXCLUDED(ofproto->vsp_mutex)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
--
1.8.3.2
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
