[ovs-dev] [PATCH] datapath: add netlink error message to help kernel userspace integration.

Andy Zhou azhou at nicira.com
Mon Jul 1 22:43:36 UTC 2013


When kernel rejects a netlink message, it usually returns EINVAL
error code to the userspace. The actual reason for rejecting the
netlinke message is not available, making it harder to debug netlink
issues.  This patch adds kernel log messages whenever a netlink message
is rejected with reasons. Those messages are logged at the info level.

Those messages are logged only once per message, to keep kernel log noise
level down. Reload the kernel module to re-enable already logged
messages.

The messages are meant to help developers to debug userspace and kernel
intergration issues. The actual message may change or be removed over time.
These messages are not expected to show up in a production environment.

Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 datapath/datapath.c |   11 ++++++--
 datapath/datapath.h |    4 +++
 datapath/flow.c     |   74 ++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3680391..2797e2e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1353,12 +1353,15 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 		 */
 		error = -EEXIST;
 		if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
-		    info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
+		    info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL)) {
+			OVS_NLERR("Flow creation message rejected, duplicate flow found.\n");
 			goto err_unlock_ovs;
+		}
 
 		/* The unmasked key has to be the same for flow updates. */
 		error = -EINVAL;
 		if (!ovs_flow_cmp_unmasked_key(flow, &key, match.range.end))
+			OVS_NLERR("Flow modification message rejected, umasked key does not match.\n");
 			goto err_unlock_ovs;
 
 		/* Update actions. */
@@ -1407,8 +1410,10 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	struct sw_flow_match match;
 	int err;
 
-	if (!a[OVS_FLOW_ATTR_KEY])
+	if (!a[OVS_FLOW_ATTR_KEY]) {
+		OVS_NLERR("Flow get message rejected, Key attribute missing.\n");
 		return -EINVAL;
+	}
 
 	ovs_match_init(&match, &key, NULL);
 	err = ovs_match_from_nlattrs(&match, a[OVS_FLOW_ATTR_KEY], NULL);
@@ -1432,6 +1437,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	reply = ovs_flow_cmd_build_info(flow, dp, info->snd_portid,
 					info->snd_seq, OVS_FLOW_CMD_NEW);
 	if (IS_ERR(reply)) {
+		OVS_NLERR("Flow get message rejected, failed to construct the reply message.\n");
 		err = PTR_ERR(reply);
 		goto unlock;
 	}
@@ -1475,6 +1481,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	table = ovsl_dereference(dp->table);
 	flow = ovs_flow_lookup_unmasked_key(table, &match);
 	if (!flow) {
+		OVS_NLERR("Flow del message rejected, flow not found.\n");
 		err = -ENOENT;
 		goto unlock;
 	}
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 559df69..0eb0aca 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -204,4 +204,8 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport *, u32 portid, u32 seq,
 
 int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb);
 void ovs_dp_notify_wq(struct work_struct *work);
+
+#define OVS_NLERR(fmt, ...) \
+	pr_info_once(fmt " NLERR: ", ##__VA_ARGS__)
+
 #endif /* datapath.h */
diff --git a/datapath/flow.c b/datapath/flow.c
index 2ac36b6..32fe86b 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -205,13 +205,19 @@ static bool ovs_match_validate(const struct sw_flow_match *match,
 		}
 	}
 
-	if ((key_attrs & key_expected) != key_expected)
+	if ((key_attrs & key_expected) != key_expected) {
 		/* Key attributes check failed. */
+		OVS_NLERR("Missing expected key attributes. (key_attrs=%llx, expected = %llx)\n",
+				key_attrs, key_expected);
 		return false;
+	}
 
-	if ((mask_attrs & mask_allowed) != mask_attrs)
+	if ((mask_attrs & mask_allowed) != mask_attrs) {
 		/* Mask attributes check failed. */
+		OVS_NLERR("Contain more than allowed mask fields. (mask_attr = %llx, mask_allowed = %llx)\n",
+				mask_attrs, mask_allowed);
 		return false;
+	}
 
 	return true;
 }
@@ -1126,24 +1132,32 @@ static int __parse_flow_nlattrs(const struct nlattr *attr,
 		u16 type = nla_type(nla);
 		int expected_len;
 
-		if (type > OVS_KEY_ATTR_MAX || attrs & (1ULL << type))
+		if (type > OVS_KEY_ATTR_MAX || attrs & (1ULL << type)) {
+			OVS_NLERR("Key or mask attr type error.\n");
 			return -EINVAL;
+		}
 
 		expected_len = ovs_key_lens[type];
-		if (nla_len(nla) != expected_len && expected_len != -1)
+		if (nla_len(nla) != expected_len && expected_len != -1) {
+			OVS_NLERR("Key or mask attr length error.\n");
 			return -EINVAL;
+		}
 
-		if (attrs & (1ULL << type))
+		if (attrs & (1ULL << type)) {
+			OVS_NLERR("Key or mask has duplicate fields.\n");
 			/* Duplicated field. */
 			return -EINVAL;
+		}
 
 		if (!nz || !is_all_zero(nla_data(nla), expected_len)) {
 			attrs |= 1ULL << type;
 			a[type] = nla;
 		}
 	}
-	if (rem)
+	if (rem) {
+		OVS_NLERR("Key or mask message longer than expected.\n");
 		return -EINVAL;
+	}
 
 	*attrsp = attrs;
 	return 0;
@@ -1182,8 +1196,10 @@ int ipv4_tun_from_nlattr(const struct nlattr *attr,
 		};
 
 		if (type > OVS_TUNNEL_KEY_ATTR_MAX ||
-			ovs_tunnel_key_lens[type] != nla_len(a))
+			ovs_tunnel_key_lens[type] != nla_len(a)) {
+			OVS_NLERR("Unexpected IPv4 tunnel attribute. (type = %d)\n", type);
 			return -EINVAL;
+		}
 
 		switch (type) {
 		case OVS_TUNNEL_KEY_ATTR_ID:
@@ -1221,14 +1237,20 @@ int ipv4_tun_from_nlattr(const struct nlattr *attr,
 
 	SW_FLOW_KEY_PUT(match, tun_key.tun_flags, tun_flags, is_mask);
 
-	if (rem > 0)
+	if (rem > 0) {
+		OVS_NLERR("IPv4 tunnel attribute longer than expected.\n");
 		return -EINVAL;
+	}
 
-	if (!match->key->tun_key.ipv4_dst)
+	if (!match->key->tun_key.ipv4_dst) {
+		OVS_NLERR("IPv4 tunnel destination address is zero.\n");
 		return -EINVAL;
+	}
 
-	if (!ttl)
+	if (!ttl) {
+		OVS_NLERR("IPv4 tunnel ttl is zero.\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
@@ -1289,8 +1311,10 @@ static int metadata_from_nlattrs(struct sw_flow_match *match,  u64 *attrs,
 	if (*attrs & (1ULL << OVS_KEY_ATTR_SKB_MARK)) {
 		uint32_t mark = nla_get_u32(a[OVS_KEY_ATTR_SKB_MARK]);
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,20) && !defined(CONFIG_NETFILTER)
-		if (!is_mask && mark != 0)
+		if (!is_mask && mark != 0) {
+			OVS_NLERR("Flow metadata mark is zero.\n");
 			return -EINVAL;
+		}
 #endif
 		SW_FLOW_KEY_PUT(match, phy.skb_mark, mark, is_mask);
 		*attrs &= ~(1ULL << OVS_KEY_ATTR_SKB_MARK);
@@ -1330,8 +1354,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match,  u64 attrs,
 
 		tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
 		if (!is_mask)
-			if (!(tci & htons(VLAN_TAG_PRESENT)))
+			if (!(tci & htons(VLAN_TAG_PRESENT))) {
+				OVS_NLERR("Vlan tci value error.\n");
 				return -EINVAL;
+			}
 
 		SW_FLOW_KEY_PUT(match, eth.tci, tci, is_mask);
 		attrs &= ~(1ULL << OVS_KEY_ATTR_VLAN);
@@ -1341,8 +1367,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match,  u64 attrs,
 		__be16 eth_type;
 
 		eth_type = nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]);
-		if (!is_mask && ntohs(eth_type) < ETH_P_802_3_MIN)
+		if (!is_mask && ntohs(eth_type) < ETH_P_802_3_MIN) {
+			OVS_NLERR("Ethertype value is less than 0x800.\n");
 			return -EINVAL;
+		}
 
 		SW_FLOW_KEY_PUT(match, eth.type, eth_type, is_mask);
 		attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE);
@@ -1354,8 +1382,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match,  u64 attrs,
 		const struct ovs_key_ipv4 *ipv4_key;
 
 		ipv4_key = nla_data(a[OVS_KEY_ATTR_IPV4]);
-		if (!is_mask && ipv4_key->ipv4_frag > OVS_FRAG_TYPE_MAX)
+		if (!is_mask && ipv4_key->ipv4_frag > OVS_FRAG_TYPE_MAX) {
+			OVS_NLERR("Ipv4 frag value error.\n");
 			return -EINVAL;
+		}
 		SW_FLOW_KEY_PUT(match, ip.proto,
 				ipv4_key->ipv4_proto, is_mask);
 		SW_FLOW_KEY_PUT(match, ip.tos,
@@ -1375,8 +1405,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match,  u64 attrs,
 		const struct ovs_key_ipv6 *ipv6_key;
 
 		ipv6_key = nla_data(a[OVS_KEY_ATTR_IPV6]);
-		if (!is_mask && ipv6_key->ipv6_frag > OVS_FRAG_TYPE_MAX)
+		if (!is_mask && ipv6_key->ipv6_frag > OVS_FRAG_TYPE_MAX) {
+			OVS_NLERR("Ipv6 frag value error.\n");
 			return -EINVAL;
+		}
 		SW_FLOW_KEY_PUT(match, ipv6.label,
 				ipv6_key->ipv6_label, is_mask);
 		SW_FLOW_KEY_PUT(match, ip.proto,
@@ -1403,8 +1435,10 @@ static int ovs_key_from_nlattrs(struct sw_flow_match *match,  u64 attrs,
 		const struct ovs_key_arp *arp_key;
 
 		arp_key = nla_data(a[OVS_KEY_ATTR_ARP]);
-		if (!is_mask && (arp_key->arp_op & htons(0xff00)))
+		if (!is_mask && (arp_key->arp_op & htons(0xff00))) {
+			OVS_NLERR("ARP op vlaue error.\n");
 			return -EINVAL;
+		}
 
 		SW_FLOW_KEY_PUT(match, ipv4.addr.src,
 				arp_key->arp_sip, is_mask);
@@ -1539,8 +1573,10 @@ int ovs_match_from_nlattrs(struct sw_flow_match *match,
 				encap_valid = true;
 				key_attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE);
 				err = parse_flow_nlattrs(encap, a, &key_attrs);
-			} else
+			} else {
+				OVS_NLERR("Encap frame error.\n");
 				err = -EINVAL;
+			}
 
 			if (err)
 				return err;
@@ -1566,8 +1602,10 @@ int ovs_match_from_nlattrs(struct sw_flow_match *match,
 				mask_attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE);
 				encap = a[OVS_KEY_ATTR_ENCAP];
 				err = parse_flow_mask_nlattrs(encap, a, &mask_attrs);
-			} else
+			} else {
+				OVS_NLERR("Ethertype 8021Q is not exact match.\n");
 				err = -EINVAL;
+			}
 
 			if (err)
 				return err;
-- 
1.7.9.5




More information about the dev mailing list