From: Ben Pfaff Date: Tue, 15 May 2012 19:50:57 +0000 (-0700) Subject: odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2508ac16defd417b94fb69689b6b1da4fbc76282;p=openvswitch odp-util: Update ODPUTIL_FLOW_KEY_BYTES for current kernel flow format. Before we submitted the kernel module upstream, we updated the flow format by adding two fields to the description of packets with VLAN headers, but we forgot to update ODPUTIL_FLOW_KEY_BYTES to reflect these changes. The result was that a maximum-length flow did not fit in the given space. This fixes a crash processing IPv6 neighbor discovery packets with VLAN headers received in a tunnel configured with key=flow or in_key=flow. This updates some comments to better describe the implications of ODPUTIL_FLOW_KEY_BYTES (suggested by Justin). This also updates test-odp.c so that it would have caught this problem, and updates odp.at to demonstrate that a full 156 bytes are necessary. (To see that, revert the change to ODPUTIL_FLOW_KEY_BYTES and run the test.) Reported-by: Dan Wendlandt Signed-off-by: Ben Pfaff --- diff --git a/lib/odp-util.c b/lib/odp-util.c index 8693d3c8..0574c9f3 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1246,7 +1246,10 @@ ovs_to_odp_frag(uint8_t nw_frag) : OVS_FRAG_TYPE_LATER); } -/* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'. */ +/* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'. + * + * 'buf' must have at least ODPUTIL_FLOW_KEY_BYTES bytes of space, or be + * capable of being expanded to allow for that much space. */ void odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow) { diff --git a/lib/odp-util.h b/lib/odp-util.h index d53f0836..f902b766 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -65,8 +65,16 @@ void format_odp_actions(struct ds *, const struct nlattr *odp_actions, int odp_actions_from_string(const char *, const struct simap *port_names, struct ofpbuf *odp_actions); -/* Upper bound on the length of a nlattr-formatted flow key. The longest - * nlattr-formatted flow key would be: +/* The maximum number of bytes that odp_flow_key_from_flow() appends to a + * buffer. This is the upper bound on the length of a nlattr-formatted flow + * key that ovs-vswitchd fully understands. + * + * OVS doesn't insist that ovs-vswitchd and the datapath have exactly the same + * idea of a flow, so therefore this value isn't necessarily an upper bound on + * the length of a flow key that the datapath can pass to ovs-vswitchd. + * + * The longest nlattr-formatted flow key appended by odp_flow_key_from_flow() + * would be: * * struct pad nl hdr total * ------ --- ------ ----- @@ -74,15 +82,20 @@ int odp_actions_from_string(const char *, const struct simap *port_names, * OVS_KEY_ATTR_TUN_ID 8 -- 4 12 * OVS_KEY_ATTR_IN_PORT 4 -- 4 8 * OVS_KEY_ATTR_ETHERNET 12 -- 4 16 + * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (outer VLAN ethertype) * OVS_KEY_ATTR_8021Q 4 -- 4 8 - * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 + * OVS_KEY_ATTR_ENCAP 0 -- 4 4 (VLAN encapsulation) + * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (inner VLAN ethertype) * OVS_KEY_ATTR_IPV6 40 -- 4 44 * OVS_KEY_ATTR_ICMPV6 2 2 4 8 * OVS_KEY_ATTR_ND 28 -- 4 32 * ------------------------------------------------- - * total 144 + * total 156 + * + * We include some slack space in case the calculation isn't quite right or we + * add another field and forget to adjust this value. */ -#define ODPUTIL_FLOW_KEY_BYTES 144 +#define ODPUTIL_FLOW_KEY_BYTES 200 /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow * key. An array of "struct nlattr" might not, in theory, be sufficiently diff --git a/tests/odp.at b/tests/odp.at index caedc94d..9617af26 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -46,6 +46,12 @@ s/$/)/' odp-base.txt echo '# Valid forms with tun_id and VLAN headers.' sed 's/^/tun_id(0xfedcba9876543210),/ s/\(eth([[^)]]*)\),*/\1,eth_type(0x8100),vlan(vid=99,pcp=7),encap(/ +s/$/)/' odp-base.txt + + echo + echo '# Valid forms with QOS priority, tun_id, and VLAN headers.' + sed 's/^/priority(1234),tun_id(0xfedcba9876543210),/ +s/\(eth([[^)]]*)\),*/\1,eth_type(0x8100),vlan(vid=99,pcp=7),encap(/ s/$/)/' odp-base.txt echo diff --git a/tests/test-odp.c b/tests/test-odp.c index 2b4cfe19..dd80766f 100644 --- a/tests/test-odp.c +++ b/tests/test-odp.c @@ -28,6 +28,7 @@ static int parse_keys(void) { + int exit_code = 0; struct ds in; ds_init(&in); @@ -71,6 +72,12 @@ parse_keys(void) ofpbuf_init(&odp_key, 0); odp_flow_key_from_flow(&odp_key, &flow); + if (odp_key.size > ODPUTIL_FLOW_KEY_BYTES) { + printf ("too long: %zu > %d\n", + odp_key.size, ODPUTIL_FLOW_KEY_BYTES); + exit_code = 1; + } + /* Convert odp_key to string. */ ds_init(&out); odp_flow_key_format(odp_key.data, odp_key.size, &out); @@ -82,7 +89,7 @@ parse_keys(void) } ds_destroy(&in); - return 0; + return exit_code; } static int