[ovs-discuss] kernel datapath discards LLDP packet

Ben Pfaff blp at nicira.com
Thu Dec 22 17:25:41 UTC 2011


On Thu, Dec 22, 2011 at 06:24:27PM +0800, Liang Liang wrote:
> With nice help from Ben, I setup Open vSwitch with NOX successfully. But
> one new problem :(
> I started discovery component of NOX, which uses LLDP protocol to build up
> network topology. NOX sends LLDP packet to vswitch and requests vswitch to
> output the packet to specified port. Now the problem is vswitch discards
> it.....
> I turned on ovs-vswitchd log(netlink_socket and dpif) using ovs-appctl, the
> log shows that it receives one error reply from kernel and error no. is
> 22(invalid arguments).

I think you said earlier you were using 1.2.1.  This is probably the
bug fixed in the 1.2.2 release by the following commit.  Does
upgrading to 1.2.2 fix the problem?

--8<--------------------------cut here-------------------------->8--

>From d6b37e427aca7158f169ef8015be4139dea258d8 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Thu, 8 Sep 2011 16:36:57 -0700
Subject: [PATCH] datapath: Allow a packet with no input port to omit ODP_KEY_ATTR_IN_PORT.

When ovs-vswitchd executes actions on a synthesized packet, that is, on a
packet that is not being forwarded from any particular port but is being
generated by ovs-vswitchd itself or by an OpenFlow controller (using a
OFPT_PACKET_OUT message with an in_port of OFPP_NONE), there is no good
choice for the in_port to pass to the kernel in the flow in the
ODP_PACKET_CMD_EXECUTE message.  This commit allows ovs-vswitchd to omit
the in_port entirely in this case.

This fixes a bug in OFPT_PACKET_OUT: using an in_port of OFPP_NONE would
cause the packet to be dropped by the kernel, since that's an invalid
input port.

Signed-off-by: Ben Pfaff <blp at nicira.com>
Acked-by: Jesse Gross <jesse at nicira.com>
Reported-by: Aaron Rosen <arosen at clemson.edu>
---
 datapath/flow.c   |   11 ++++++++---
 datapath/flow.h   |    2 +-
 lib/dpif-netdev.c |   14 +++++++++++---
 lib/odp-util.c    |    9 +++++++--
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index 27038ec..d9e3602 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -624,7 +624,7 @@ static const u32 key_lens[ODP_KEY_ATTR_MAX + 1] = {
  * This state machine accepts the following forms, with [] for optional
  * elements and | for alternatives:
  *
- * [tun_id] in_port ethernet [8021q] [ethertype \
+ * [tun_id] [in_port] ethernet [8021q] [ethertype \
  *              [IPv4 [TCP|UDP|ICMP] | IPv6 [TCP|UDP|ICMPv6 [ND]] | ARP]]
  */
 int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
@@ -637,6 +637,7 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
 	int key_len;
 
 	memset(swkey, 0, sizeof(*swkey));
+	swkey->eth.in_port = USHRT_MAX;
 	swkey->eth.type = htons(ETH_P_802_2);
 	key_len = SW_FLOW_KEY_OFFSET(eth);
 
@@ -671,6 +672,8 @@ int flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
 			swkey->eth.in_port = nla_get_u32(nla);
 			break;
 
+		case TRANSITION(ODP_KEY_ATTR_UNSPEC, ODP_KEY_ATTR_ETHERNET):
+		case TRANSITION(ODP_KEY_ATTR_TUN_ID, ODP_KEY_ATTR_ETHERNET):
 		case TRANSITION(ODP_KEY_ATTR_IN_PORT, ODP_KEY_ATTR_ETHERNET):
 			eth_key = nla_data(nla);
 			memcpy(swkey->eth.src, eth_key->eth_src, ETH_ALEN);
@@ -888,6 +891,7 @@ int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id,
 	u16 prev_type;
 	int rem;
 
+	*in_port = USHRT_MAX;
 	*tun_id = 0;
 
 	prev_type = ODP_KEY_ATTR_UNSPEC;
@@ -910,7 +914,7 @@ int flow_metadata_from_nlattrs(u16 *in_port, __be64 *tun_id,
 			break;
 
 		default:
-			goto done;
+			return 0;
 		}
 
 		prev_type = type;
@@ -938,7 +942,8 @@ int flow_to_nlattrs(const struct sw_flow_key *swkey, struct sk_buff *skb)
 	if (swkey->eth.tun_id != cpu_to_be64(0))
 		NLA_PUT_BE64(skb, ODP_KEY_ATTR_TUN_ID, swkey->eth.tun_id);
 
-	NLA_PUT_U32(skb, ODP_KEY_ATTR_IN_PORT, swkey->eth.in_port);
+	if (swkey->eth.in_port != USHRT_MAX)
+		NLA_PUT_U32(skb, ODP_KEY_ATTR_IN_PORT, swkey->eth.in_port);
 
 	nla = nla_reserve(skb, ODP_KEY_ATTR_ETHERNET, sizeof(*eth_key));
 	if (!nla)
diff --git a/datapath/flow.h b/datapath/flow.h
index 6a3c539..1802d18 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -33,7 +33,7 @@ struct sw_flow_actions {
 struct sw_flow_key {
 	struct {
 		__be64 tun_id;		/* Encapsulating tunnel ID. */
-		u16    in_port;		/* Input switch port. */
+		u16    in_port;		/* Input switch port (or USHRT_MAX). */
 		u8     src[ETH_ALEN];	/* Ethernet source address. */
 		u8     dst[ETH_ALEN];	/* Ethernet destination address. */
 		__be16 tci;		/* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d48d7ae..1bd7b91 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -652,6 +652,12 @@ dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
         return EINVAL;
     }
 
+    if (flow->in_port < OFPP_MAX
+        ? flow->in_port >= MAX_PORTS
+        : flow->in_port != OFPP_LOCAL && flow->in_port != OFPP_NONE) {
+        return EINVAL;
+    }
+
     return 0;
 }
 
@@ -977,9 +983,11 @@ dpif_netdev_execute(struct dpif *dpif,
     }
 
     flow_extract(&copy, 0, -1, &key);
-    dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
-
-    error = dp_netdev_execute_actions(dp, &copy, &key, actions, actions_len);
+    error = dpif_netdev_flow_from_nlattrs(key_attrs, key_len, &key);
+    if (!error) {
+        error = dp_netdev_execute_actions(dp, &copy, &key,
+                                          actions, actions_len);
+    }
     if (mutates) {
         ofpbuf_uninit(&copy);
     }
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 93f8f8a..bf530ca 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -399,8 +399,10 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow)
         nl_msg_put_be64(buf, ODP_KEY_ATTR_TUN_ID, flow->tun_id);
     }
 
-    nl_msg_put_u32(buf, ODP_KEY_ATTR_IN_PORT,
-                   ofp_port_to_odp_port(flow->in_port));
+    if (flow->in_port != OFPP_NONE) {
+        nl_msg_put_u32(buf, ODP_KEY_ATTR_IN_PORT,
+                       ofp_port_to_odp_port(flow->in_port));
+    }
 
     eth_key = nl_msg_put_unspec_uninit(buf, ODP_KEY_ATTR_ETHERNET,
                                        sizeof *eth_key);
@@ -516,6 +518,7 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
 
     memset(flow, 0, sizeof *flow);
     flow->dl_type = htons(FLOW_DL_TYPE_NONE);
+    flow->in_port = OFPP_NONE;
 
     prev_type = ODP_KEY_ATTR_UNSPEC;
     NL_ATTR_FOR_EACH (nla, left, key, key_len) {
@@ -551,6 +554,8 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
             flow->in_port = odp_port_to_ofp_port(nl_attr_get_u32(nla));
             break;
 
+        case TRANSITION(ODP_KEY_ATTR_UNSPEC, ODP_KEY_ATTR_ETHERNET):
+        case TRANSITION(ODP_KEY_ATTR_TUN_ID, ODP_KEY_ATTR_ETHERNET):
         case TRANSITION(ODP_KEY_ATTR_IN_PORT, ODP_KEY_ATTR_ETHERNET):
             eth_key = nl_attr_get(nla);
             memcpy(flow->dl_src, eth_key->eth_src, ETH_ADDR_LEN);
-- 
1.7.2.5




More information about the discuss mailing list