Merge lp:~gandelman-a/ubuntu/precise/openvswitch/update_key_bytes into lp:ubuntu/precise/openvswitch

Proposed by Adam Gandelman
Status: Merged
Merge reported by: Stéphane Graber
Merged at revision: not available
Proposed branch: lp:~gandelman-a/ubuntu/precise/openvswitch/update_key_bytes
Merge into: lp:ubuntu/precise/openvswitch
Diff against target: 171 lines (+153/-0)
3 files modified
debian/changelog (+8/-0)
debian/patches/series (+1/-0)
debian/patches/update_odputil_key_bytes.patch (+144/-0)
To merge this branch: bzr merge lp:~gandelman-a/ubuntu/precise/openvswitch/update_key_bytes
Reviewer Review Type Date Requested Status
James Page Approve
Ubuntu branches Pending
Review via email: mp+115202@code.launchpad.net

Description of the change

Please note: Merge targeted at precise-proposed/openvswitch (which currently does not exist)

To post a comment you must log in.
Revision history for this message
James Page (james-page) wrote :

LGTM - uploaded to -proposed

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-02-13 09:41:54 +0000
3+++ debian/changelog 2012-07-16 18:42:18 +0000
4@@ -1,3 +1,11 @@
5+openvswitch (1.4.0-1ubuntu1.1) precise-proposed; urgency=low
6+
7+ * debian/patches/update_odputil_key_bytes.patch: Update odp-util flow
8+ key bytes to match the current kernel. Cherry-picked from upstream commit
9+ cf8285a36. (LP: #1021530)
10+
11+ -- Adam Gandelman <adamg@canonical.com> Mon, 16 Jul 2012 11:06:23 -0700
12+
13 openvswitch (1.4.0-1ubuntu1) precise; urgency=low
14
15 * Merged from debian unstable (LP: #925611), remaining changes are:
16
17=== added directory 'debian/patches'
18=== added file 'debian/patches/series'
19--- debian/patches/series 1970-01-01 00:00:00 +0000
20+++ debian/patches/series 2012-07-16 18:42:18 +0000
21@@ -0,0 +1,1 @@
22+update_odputil_key_bytes.patch
23
24=== added file 'debian/patches/update_odputil_key_bytes.patch'
25--- debian/patches/update_odputil_key_bytes.patch 1970-01-01 00:00:00 +0000
26+++ debian/patches/update_odputil_key_bytes.patch 2012-07-16 18:42:18 +0000
27@@ -0,0 +1,144 @@
28+From 2508ac16defd417b94fb69689b6b1da4fbc76282 Mon Sep 17 00:00:00 2001
29+From: Ben Pfaff <blp@nicira.com>
30+Date: Tue, 15 May 2012 12:50:57 -0700
31+Origin, upstream: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commit;h=11f84dce005dd14a374ab2ef5f8c25bcf8285a36
32+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1021530
33+Subject: [PATCH] odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel
34+ flow format.
35+
36+Before we submitted the kernel module upstream, we updated the flow format
37+by adding two fields to the description of packets with VLAN headers, but
38+we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes. The
39+result was that a maximum-length flow did not fit in the given space.
40+
41+This fixes a crash processing IPv6 neighbor discovery packets with VLAN
42+headers received in a tunnel configured with key=flow or in_key=flow.
43+
44+This updates some comments to better describe the implications of
45+ODPUTIL_FLOW_KEY_BYTES (suggested by Justin).
46+
47+This also updates test-odp.c so that it would have caught this problem, and
48+updates odp.at to demonstrate that a full 156 bytes are necessary. (To see
49+that, revert the change to ODPUTIL_FLOW_KEY_BYTES and run the test.)
50+
51+Reported-by: Dan Wendlandt <dan@nicira.com>
52+Signed-off-by: Ben Pfaff <blp@nicira.com>
53+---
54+ lib/odp-util.c | 5 ++++-
55+ lib/odp-util.h | 23 ++++++++++++++++++-----
56+ tests/odp.at | 6 ++++++
57+ tests/test-odp.c | 9 ++++++++-
58+ 4 files changed, 36 insertions(+), 7 deletions(-)
59+
60+Index: openvswitch/lib/odp-util.c
61+===================================================================
62+--- openvswitch.orig/lib/odp-util.c 2012-07-06 10:31:38.619554000 -0700
63++++ openvswitch/lib/odp-util.c 2012-07-06 10:37:11.786772419 -0700
64+@@ -1156,7 +1156,10 @@
65+ : OVS_FRAG_TYPE_NONE);
66+ }
67+
68+-/* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'. */
69++/* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'.
70++ *
71++ * 'buf' must have at least ODPUTIL_FLOW_KEY_BYTES bytes of space, or be
72++ * capable of being expanded to allow for that much space. */
73+ void
74+ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow)
75+ {
76+Index: openvswitch/lib/odp-util.h
77+===================================================================
78+--- openvswitch.orig/lib/odp-util.h 2012-07-06 10:31:38.619554000 -0700
79++++ openvswitch/lib/odp-util.h 2012-07-06 10:37:11.786772419 -0700
80+@@ -65,8 +65,16 @@
81+ int odp_actions_from_string(const char *, const struct shash *port_names,
82+ struct ofpbuf *odp_actions);
83+
84+-/* Upper bound on the length of a nlattr-formatted flow key. The longest
85+- * nlattr-formatted flow key would be:
86++/* The maximum number of bytes that odp_flow_key_from_flow() appends to a
87++ * buffer. This is the upper bound on the length of a nlattr-formatted flow
88++ * key that ovs-vswitchd fully understands.
89++ *
90++ * OVS doesn't insist that ovs-vswitchd and the datapath have exactly the same
91++ * idea of a flow, so therefore this value isn't necessarily an upper bound on
92++ * the length of a flow key that the datapath can pass to ovs-vswitchd.
93++ *
94++ * The longest nlattr-formatted flow key appended by odp_flow_key_from_flow()
95++ * would be:
96+ *
97+ * struct pad nl hdr total
98+ * ------ --- ------ -----
99+@@ -74,15 +82,20 @@
100+ * OVS_KEY_ATTR_TUN_ID 8 -- 4 12
101+ * OVS_KEY_ATTR_IN_PORT 4 -- 4 8
102+ * OVS_KEY_ATTR_ETHERNET 12 -- 4 16
103++ * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (outer VLAN ethertype)
104+ * OVS_KEY_ATTR_8021Q 4 -- 4 8
105+- * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8
106++ * OVS_KEY_ATTR_ENCAP 0 -- 4 4 (VLAN encapsulation)
107++ * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (inner VLAN ethertype)
108+ * OVS_KEY_ATTR_IPV6 40 -- 4 44
109+ * OVS_KEY_ATTR_ICMPV6 2 2 4 8
110+ * OVS_KEY_ATTR_ND 28 -- 4 32
111+ * -------------------------------------------------
112+- * total 144
113++ * total 156
114++ *
115++ * We include some slack space in case the calculation isn't quite right or we
116++ * add another field and forget to adjust this value.
117+ */
118+-#define ODPUTIL_FLOW_KEY_BYTES 144
119++#define ODPUTIL_FLOW_KEY_BYTES 200
120+
121+ /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow
122+ * key. An array of "struct nlattr" might not, in theory, be sufficiently
123+Index: openvswitch/tests/odp.at
124+===================================================================
125+--- openvswitch.orig/tests/odp.at 2012-07-06 10:31:38.619554000 -0700
126++++ openvswitch/tests/odp.at 2012-07-06 10:37:11.786772419 -0700
127+@@ -48,6 +48,12 @@
128+ s/$/)/' odp-base.txt
129+
130+ echo
131++ echo '# Valid forms with QOS priority, tun_id, and VLAN headers.'
132++ sed 's/^/priority(1234),tun_id(0xfedcba9876543210),/
133++s/\(eth([[^)]]*)\),*/\1,eth_type(0x8100),vlan(vid=99,pcp=7),encap(/
134++s/$/)/' odp-base.txt
135++
136++ echo
137+ echo '# Valid forms with IP first fragment.'
138+ sed -n 's/,frag=no),/,frag=first),/p' odp-base.txt
139+
140+Index: openvswitch/tests/test-odp.c
141+===================================================================
142+--- openvswitch.orig/tests/test-odp.c 2012-07-06 10:31:38.619554000 -0700
143++++ openvswitch/tests/test-odp.c 2012-07-06 10:37:11.790772420 -0700
144+@@ -27,6 +27,7 @@
145+ int
146+ main(void)
147+ {
148++ int exit_code = 0;
149+ struct ds in;
150+
151+ ds_init(&in);
152+@@ -85,6 +86,12 @@
153+ ofpbuf_init(&odp_key, 0);
154+ odp_flow_key_from_flow(&odp_key, &flow);
155+
156++ if (odp_key.size > ODPUTIL_FLOW_KEY_BYTES) {
157++ printf ("too long: %zu > %d\n",
158++ odp_key.size, ODPUTIL_FLOW_KEY_BYTES);
159++ exit_code = 1;
160++ }
161++
162+ /* Convert odp_key to string. */
163+ ds_init(&out);
164+ odp_flow_key_format(odp_key.data, odp_key.size, &out);
165+@@ -96,5 +103,5 @@
166+ }
167+ ds_destroy(&in);
168+
169+- return 0;
170++ return exit_code;
171+ }

Subscribers

People subscribed via source and target branches