In ovs kernel datapath upcall, the *key* and packet are passed to
userspace. The key contains the fields/meta extracted from packet.
Once the ovs-vswitchd receives the upcall, the packet is extracted
again into *flow*. Next, the flow is used to match openflow rules to
generate the wildcard(wc). At last, vswitchd installs a mega_flow in
datapath(mega_flow = key/wc,action)
We can see that vswitchd generate wc from flow while it installs dp
flow with key. If the key is not consistent with the flow [1], we get
bad mega_flow.
Let's assume we have the flowing rules, means to block tcp port 0-0xf,
but allow other ports.
"table=0,priority=100,tcp,tp_dst=0x0/0xfff0 actions=drop"
"table=0,priority=90,tcp actions=p1"
good case:
If a packet has tcp dst=0x10, generated `mega_flow=0x10/0xfff0,out:p1`,
this is expected.
bad case:
If a packet has tcp dst=0x10 but not pass tcphdr_ok [1], generated wc
and action are `0xfff0,out:p1`. The mega_flow will be
`0x0/0xfff0,out:p1`, bacause mega_flow=key/wc,action. This allows
packets with tcp port 0-0xf pass by mistake.
The following scapy3 script triggers the issue:
```py
eth=Ether(src="fa:16:3e:5e:e3:57",dst="be:95:df:40:fb:57")
ip=IP(src="10.10.10.10",dst="20.20.20.20")
tcp=TCP(sport=100,dport=16,dataofs=1)
sendp(eth/ip/tcp)
```
This patch is to prevent from installing datapath flow if the key is
not consistant with the flow.
[1] https://github.com/openvswitch/ovs/blob/v2.16.1/datapath/flow.c#L601
Signed-off-by: lic121
---
ofproto/ofproto-dpif-upcall.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 1c9c720..81f297d 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -244,6 +244,7 @@ struct upcall {
size_t key_len;/* Datapath flow key length. */
const struct nlattr *out_tun_key; /* Datapath output tunnel key. */
+const struct flow *key_as_flow; /* converted from key. */
struct user_action_cookie cookie;
uint64_t odp_actions_stub[1024 / 8]; /* Stub for odp_actions. */
@@ -810,6 +811,7 @@ recv_upcalls(struct handler *handler)
struct dpif_upcall dupcalls[UPCALL_MAX_BATCH];
struct upcall upcalls[UPCALL_MAX_BATCH];
struct flow flows[UPCALL_MAX_BATCH];
+struct flow key_as_flows[UPCALL_MAX_BATCH];
size_t n_upcalls, i;
n_upcalls = 0;
@@ -818,6 +820,7 @@ recv_upcalls(struct handler *handler)
struct dpif_upcall *dupcall = [n_upcalls];
struct upcall *upcall = [n_upcalls];
struct flow *flow = [n_upcalls];
+struct flow *key_as_flow = _as_flows[n_upcalls];
unsigned int mru = 0;
uint64_t hash = 0;
int error;
@@ -830,7 +833,7 @@ recv_upcalls(struct handler *handler)
}
upcall->fitness = odp_flow_key_to_flow(dupcall->key, dupcall->key_len,
- flow, NULL);
+ key_as_flow, NULL);
if (upcall->fitness == ODP_FIT_ERROR) {
goto free_dupcall;
}
@@ -843,6 +846,9 @@ recv_upcalls(struct handler *handler)
hash = nl_attr_get_u64(dupcall->hash);
}
+/* Fill flow with key_as_flow as upcall_receive needs
+ * packet flow info. */
+*flow = *key_as_flow;
error = upcall_receive(upcall, udpif->backer, >packet,
dupcall->type, dupcall->userdata, flow, mru,
>ufid, PMD_ID_NULL);
@@ -856,20 +862,21 @@ recv_upcalls(struct handler *handler)
dupcall->key_len, NULL, 0, NULL, 0,
>ufid, PMD_ID_NULL, NULL);
VLOG_INFO_RL(, "received packet on unassociated datapath "
- "port %"PRIu32, flow->in_port.odp_port);
+ "port %"PRIu32, key_as_flow->in_port.odp_port);
}
goto free_dupcall;
}
upcall->key = dupcall->key;
upcall->key_len = dupcall->key_len;
+upcall->key_as_flow = key_as_flow;
upcall->ufid = >ufid;
upcall->hash = hash;
upcall->out_tun_key = dupcall->out_tun_key;
upcall->actions = dupcall->actions;
-pkt_metadata_from_flow(>packet.md, flow);
+pkt_metadata_from_flow(>packet.md, key_as_flow);
flow_extract(>packet, flow);
error = process_upcall(udpif, upcall,
@@ -1332,6 +1339,19 @@ should_install_flow(struct udpif *udpif, struct upcall
*upcall)
return false;
}
+/* For linux kernel datapath, the "key" extracted by kernel may be
+ * inconsistent with the flow extracted from packet by ovs. If that
+ * is the case, twe can't install the datapth flow (key/wc) */
+if (upcall->key_len && !flow_equal_except(upcall->key_as_flow,
+
