[ovs-dev] [PATCH v2] dpif-netlink: convert packet_type netlink attribute
Zoltán Balogh
zoltan.balogh at ericsson.com
Tue Jul 4 21:23:17 UTC 2017
By introducing packet type-aware pipeline, match on ethertype was
removed when packet type is not Ethernet. As pointed out by Eric Garver,
this could have a side effect on the kernel datapath:
https://patchwork.ozlabs.org/patch/782991/
This patch does approach the problem from a different perspective.
Instead of re-introducing the unconditional match on Ethernet type in all
megaflow entries, as suggested by Eric, mapping of packet_type != PT_ETH to
dl_type could be handled in the put_exclude_packet_type() in dpif-netlink.c.
Put_exclude_packet_type() filters the packet_type netlink attribute out,
before all attributes are sent from userspace to kernel. This commit modifies
the put_exclude_packet_type() function to do a proper conversation and add the
missing OVS_KEY_ATTR_ETHERTYPE attribute if it's needed.
Signed-off-by: Zoltán Balogh <zoltan.balogh at ericsson.com>
Reported-by: Eric Garver <e at erig.me>
---
lib/dpif-netlink.c | 120 +++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 90 insertions(+), 30 deletions(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 562f613..cf4e98f 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3433,34 +3433,101 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow,
}
+static bool
+put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
+ const struct nlattr *data, uint16_t data_len,
+ const struct nlattr *packet_type)
+{
+ ovs_assert(NLA_ALIGN(packet_type->nla_len) == NL_A_U32_SIZE);
+ size_t packet_type_len = NL_A_U32_SIZE;
+ size_t first_chunk_size = (uint8_t *)packet_type - (uint8_t *)data;
+ size_t second_chunk_size = data_len - first_chunk_size
+ - packet_type_len;
+ uint8_t *first_attr = NULL;
+ struct nlattr *next_attr = nl_attr_next(packet_type);
+
+ bool ethernet_present = nl_attr_find__(data, data_len,
+ OVS_KEY_ATTR_ETHERNET);
+
+ first_attr = nl_msg_put_unspec_uninit(buf, type,
+ data_len - packet_type_len);
+ memcpy(first_attr, data, first_chunk_size);
+ memcpy(first_attr + first_chunk_size, next_attr, second_chunk_size);
+
+ return ethernet_present;
+}
+
/*
- * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out,
- * then puts 'data' to 'buf'.
+ * If PACKET_TYPE attribute is present in 'data', converts it to ETHERNET and
+ * ETHERTYPE attributes, then puts 'data' to 'buf'.
*/
static void
-put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
- const struct nlattr *data, uint16_t data_len)
+put_convert_packet_type(struct ofpbuf *buf,
+ const struct dpif_netlink_flow *flow)
{
const struct nlattr *packet_type;
+ const struct nlattr *data = flow->key;
+ uint16_t data_len = flow->key_len;
+ const struct nlattr *mask_data = flow->mask;
+ uint16_t mask_len = flow->mask_len;
+ ovs_be32 value = htonl(PT_ETH);
+ bool ethernet_present = false;
+
+ /* Verify KEY attributes. */
+ if (data_len) {
+ packet_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_PACKET_TYPE);
+ if (packet_type) {
+ /* Convert PACKET_TYPE Netlink attribute. */
+ value = nl_attr_get_be32(packet_type);
+ ethernet_present = put_exclude_packet_type(buf, OVS_FLOW_ATTR_KEY,
+ data, data_len,
+ packet_type);
+ /* Put OVS_KEY_ATTR_ETHERTYPE if needed. */
+ if (ntohl(value) == PT_ETH) {
+ ovs_assert(ethernet_present);
+ } else {
+ const struct nlattr *eth_type;
+ ovs_be16 ns_type = pt_ns_type_be(value);
- packet_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_PACKET_TYPE);
-
- if (packet_type) {
- /* exclude PACKET_TYPE Netlink attribute. */
- ovs_assert(NLA_ALIGN(packet_type->nla_len) == NL_A_U32_SIZE);
- size_t packet_type_len = NL_A_U32_SIZE;
- size_t first_chunk_size = (uint8_t *)packet_type - (uint8_t *)data;
- size_t second_chunk_size = data_len - first_chunk_size
- - packet_type_len;
- uint8_t *first_attr = NULL;
- struct nlattr *next_attr = nl_attr_next(packet_type);
-
- first_attr = nl_msg_put_unspec_uninit(buf, type,
- data_len - packet_type_len);
- memcpy(first_attr, data, first_chunk_size);
- memcpy(first_attr + first_chunk_size, next_attr, second_chunk_size);
- } else {
- nl_msg_put_unspec(buf, type, data, data_len);
+ ovs_assert(ethernet_present == false);
+
+ eth_type = nl_attr_find__(data, data_len,
+ OVS_KEY_ATTR_ETHERTYPE);
+ if (eth_type) {
+ ovs_assert(nl_attr_get_be16(eth_type) == ns_type);
+ } else {
+ nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, ns_type);
+ }
+ }
+ } else {
+ nl_msg_put_unspec(buf, OVS_FLOW_ATTR_KEY, data, data_len);
+ }
+ }
+
+ /* Verify MASK attributes. */
+ if (mask_len) {
+ packet_type = nl_attr_find__(mask_data, mask_len,
+ OVS_KEY_ATTR_PACKET_TYPE);
+ if (packet_type) {
+ /* Convert PACKET_TYPE Netlink attribute. */
+ ethernet_present = put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK,
+ mask_data, mask_len,
+ packet_type);
+ /* Put OVS_KEY_ATTR_ETHERTYPE if needed. */
+ if (ntohl(value) != PT_ETH) {
+ const struct nlattr *eth_type;
+
+ ovs_assert(ethernet_present == false);
+
+ eth_type = nl_attr_find__(mask_data, mask_len,
+ OVS_KEY_ATTR_ETHERTYPE);
+ if (!eth_type) {
+ nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
+ }
+ }
+ } else {
+ nl_msg_put_unspec(buf, OVS_FLOW_ATTR_MASK, mask_data, mask_len);
+ }
}
}
@@ -3488,14 +3555,7 @@ dpif_netlink_flow_to_ofpbuf(const struct dpif_netlink_flow *flow,
| OVS_UFID_F_OMIT_ACTIONS);
}
if (!flow->ufid_terse || !flow->ufid_present) {
- if (flow->key_len) {
- put_exclude_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
- flow->key_len);
- }
- if (flow->mask_len) {
- put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
- flow->mask_len);
- }
+ put_convert_packet_type(buf, flow);
if (flow->actions || flow->actions_len) {
nl_msg_put_unspec(buf, OVS_FLOW_ATTR_ACTIONS,
flow->actions, flow->actions_len);
--
1.9.1
More information about the dev
mailing list