On Wed, Jul 3, 2013 at 9:18 AM, Andy Zhou <az...@nicira.com> wrote:
> 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 <az...@nicira.com>

I applied this with the follow patch, can you please check it over?:

diff --git a/datapath/datapath.h b/datapath/datapath.h
index 17b1db9..d14a162 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -206,6 +206,6 @@ 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 " netlink: ", ##__VA_ARGS__)
+ pr_info_once(fmt "netlink: ", ##__VA_ARGS__)

 #endif /* datapath.h */
diff --git a/datapath/flow.c b/datapath/flow.c
index 89bb496..c76c18d 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -207,14 +207,14 @@ static bool ovs_match_validate(const struct
sw_flow_match *match,

  if ((key_attrs & key_expected) != key_expected) {
  /* Key attributes check failed. */
- OVS_NLERR("Missing expected key attributes. (key_attrs=%llx,
expected = %llx)\n",
+ 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) {
  /* Mask attributes check failed. */
- OVS_NLERR("Contain more than allowed mask fields. (mask_attr = %llx,
mask_allowed = %llx)\n",
+ OVS_NLERR("Contain more than allowed mask fields (mask_attrs=%llx,
mask_allowed=%llx).\n",
  mask_attrs, mask_allowed);
  return false;
  }
@@ -1132,15 +1132,21 @@ 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)) {
- OVS_NLERR("Duplicate key attribute type %d.\n", type);
+ if (type > OVS_KEY_ATTR_MAX) {
+ OVS_NLERR("Unknown key attribute (type=%d, max=%d).\n",
+  type, OVS_KEY_ATTR_MAX);
+ }
+
+ if (attrs & (1ULL << type)) {
+ OVS_NLERR("Duplicate key attribute (type %d).\n", type);
  return -EINVAL;
  }

  expected_len = ovs_key_lens[type];
  if (nla_len(nla) != expected_len && expected_len != -1) {
- OVS_NLERR("Key attribute type %d length %d, not matching expected
length %d.\n",
- type, nla_len(nla), expected_len);
+ OVS_NLERR("Key attribute has unexpected length (type=%d"
+  ", length=%d, expected=%d).\n", type,
+  nla_len(nla), expected_len);
  return -EINVAL;
  }

@@ -1150,7 +1156,7 @@ static int __parse_flow_nlattrs(const struct nlattr *attr,
  }
  }
  if (rem) {
- OVS_NLERR("netlink message has %d unknown bytes.\n", rem);
+ OVS_NLERR("Message has %d unknown bytes.\n", rem);
  return -EINVAL;
  }

@@ -1191,14 +1197,15 @@ int ipv4_tun_from_nlattr(const struct nlattr *attr,
  };

  if (type > OVS_TUNNEL_KEY_ATTR_MAX) {
- OVS_NLERR("Unexpected IPv4 tunnel attribute. (type = %d, MAX=%d)\n",
+ OVS_NLERR("Unknown IPv4 tunnel attribute (type=%d, max=%d)\n",
  type, OVS_TUNNEL_KEY_ATTR_MAX);
  return -EINVAL;
  }

  if (ovs_tunnel_key_lens[type] != nla_len(a)) {
- OVS_NLERR("IPv4 tunnel attribute type %d length %d does not equal to
expected length %d.\n",
- type, nla_len(a), ovs_tunnel_key_lens[type]);
+ OVS_NLERR("IPv4 tunnel attribute type has unexpected "
+  " legnth (type=%d, length=%d, expected=%d.)\n",
+  type, nla_len(a), ovs_tunnel_key_lens[type]);
  return -EINVAL;
  }

@@ -1313,7 +1320,7 @@ static int metadata_from_nlattrs(struct
sw_flow_match *match,  u64 *attrs,
  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) {
- OVS_NLERR("Flow meta data mark is not zero.\n");
+ OVS_NLERR("skb->mark must be zero on this kernel (mark=%d).\n", mark);
  return -EINVAL;
  }
 #endif
@@ -1356,7 +1363,7 @@ 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))) {
- OVS_NLERR("VLAN tci does not have VLAN_TAG_PRESENT bit set.\n");
+ OVS_NLERR("VLAN TCI does not have VLAN_TAG_PRESENT bit set.\n");
  return -EINVAL;
  }

@@ -1369,8 +1376,8 @@ static int ovs_key_from_nlattrs(struct
sw_flow_match *match,  u64 attrs,

  eth_type = nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]);
  if (!is_mask && ntohs(eth_type) < ETH_P_802_3_MIN) {
- OVS_NLERR("Ethertype value %x is less than %x.\n",
- eth_type, ETH_P_802_3_MIN);
+ OVS_NLERR("EtherType is less than mimimum (type=%x, min=%x).\n",
+ ntohs(eth_type), ETH_P_802_3_MIN);
  return -EINVAL;
  }

@@ -1385,7 +1392,7 @@ static int ovs_key_from_nlattrs(struct
sw_flow_match *match,  u64 attrs,

  ipv4_key = nla_data(a[OVS_KEY_ATTR_IPV4]);
  if (!is_mask && ipv4_key->ipv4_frag > OVS_FRAG_TYPE_MAX) {
- OVS_NLERR("IPv4 frag type value %d is larger than %d.\n",
+ OVS_NLERR("Unknown IPv4 fragment type (value=%d, max=%d).\n",
  ipv4_key->ipv4_frag, OVS_FRAG_TYPE_MAX);
  return -EINVAL;
  }
@@ -1409,7 +1416,8 @@ static int ovs_key_from_nlattrs(struct
sw_flow_match *match,  u64 attrs,

  ipv6_key = nla_data(a[OVS_KEY_ATTR_IPV6]);
  if (!is_mask && ipv6_key->ipv6_frag > OVS_FRAG_TYPE_MAX) {
- OVS_NLERR("IPv6 frag type value is larger than OVS_FRAG_TYPE_MAX.\n");
+ OVS_NLERR("Unknown IPv6 fragment type (value=%d, max=%d).\n",
+ ipv6_key->ipv6_frag, OVS_FRAG_TYPE_MAX);
  return -EINVAL;
  }
  SW_FLOW_KEY_PUT(match, ipv6.label,
@@ -1439,7 +1447,8 @@ static int ovs_key_from_nlattrs(struct
sw_flow_match *match,  u64 attrs,

  arp_key = nla_data(a[OVS_KEY_ATTR_ARP]);
  if (!is_mask && (arp_key->arp_op & htons(0xff00))) {
- OVS_NLERR("ARP op value high 16-bits are not clear.\n");
+ OVS_NLERR("Unknown ARP opcode (opcode=%d).\n",
+  arp_key->arp_op);
  return -EINVAL;
  }

@@ -1606,7 +1615,9 @@ int ovs_match_from_nlattrs(struct sw_flow_match *match,
  encap = a[OVS_KEY_ATTR_ENCAP];
  err = parse_flow_mask_nlattrs(encap, a, &mask_attrs);
  } else {
- OVS_NLERR("Ethertype 8021Q is not exact match.\n");
+ OVS_NLERR("VLAN frames must have an exact match"
+ " on the TPID (mask=%x).\n",
+ ntohs(eth_type));
  err = -EINVAL;
  }

diff --git a/datapath/linux/compat/include/linux/kernel.h
b/datapath/linux/compat/include/linux/kernel.h
index 16c25fe..fdd2005 100644
--- a/datapath/linux/compat/include/linux/kernel.h
+++ b/datapath/linux/compat/include/linux/kernel.h
@@ -43,7 +43,6 @@
  * Print a one-time message (analogous to WARN_ONCE() et al):
  */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 38)
-#ifdef CONFIG_PRINTK
 #undef printk_once
 #define printk_once(fmt, ...) \
 ({ \
@@ -54,13 +53,6 @@
  printk(fmt, ##__VA_ARGS__); \
  } \
 })
-#else
-#define printk_once(fmt, ...) \
- no_printk(fmt, ##__VA_ARGS__)
-#endif
-
-#undef pr_fmt
-#define pr_fmt(fmt) fmt

 #define pr_emerg_once(fmt, ...) \
  printk_once(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__)
@@ -78,14 +70,6 @@
  printk_once(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_cont_once(fmt, ...) \
  printk_once(KERN_CONT pr_fmt(fmt), ##__VA_ARGS__)
-/* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug_once(fmt, ...) \
- printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#else
-#define pr_debug_once(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#endif

 #endif
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to