Hi Ben,
The mentioned fix below is causing a regression, actually two related
ones:
1) If IGMP snooping is disabled, IGMP related traffic is still sent slow
path for processing with the NORMAL rule. In addition, even without the
NORMAL rule, the DP rule gets modified.
Here are some examples:
a) default config, one bridge (kernel dp), two ports (i.e. only a single
NORMAL rule, and mcast_snooping_enable: false), send an IGMP join:
- With your fix the dp rule looks like this:
recirc_id(0),in_port(2),eth(src=04:f4:bc:28:57:01,dst=01:00:5e:00:00:16),eth_type(0x0800),ipv4(frag=no),
packets:8, bytes:576, used:0.169s,
actions:userspace(pid=4220974525,slow_path(match))
- Without the fix, which is correct:
recirc_id(0),in_port(2),eth(src=04:f4:bc:28:57:01,dst=01:00:5e:00:00:16),eth_type(0x0800),ipv4(frag=no),
packets:8, bytes:576, used:0.004s, actions:1
b) with a more specific OpenFlow entry, no DP entry gets created:
ovs-ofctl del-flows ovs_pvp_br0 && ovs-ofctl add-flow ovs_pvp_br0
"in_port=enp5s0f0,eth_type(0x800),ip_proto=2,action=drop"
ovs-ofctl del-flows ovs_pvp_br0 && ovs-ofctl add-flow ovs_pvp_br0
"in_port=enp5s0f0,eth_type(0x800),ip_proto=2,action=vnet6"
2) With the netdev (DPDK) interface all seems to work as expected, i.e.,
the flow gets installed correctly without the userspace upcall in both
cases above. However, the verifier generates an error as it would like
to update the dp rule with a slow_path variant. Which is failing as
netdev is using the odp_flow_key_to_flow() utility function, which has
the IGMP check.
Looking at the change it assumes you filter on igmp type/code when you
filter on the protocol type igmp. But this is not true, you can just
filter on the protocol type only, and this is what is actually causing
the problems above. So I think your patch needs to be reversed!
However, looking at fixing Huanle's original problem,
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343665.html,
looking at his patch, this might be the right solution. All IGMP packets
independent of their type need to be processed by a SLOW_ACTION upcall.
I've copied in Flavio, who worked on the IGMP snooping part before, as
he might still remember what I argue above is true :)
If you have any other ideas, thoughts please let me know.
Cheers,
Eelco
============================================================
commit c645550bb2498fb3816b6a39b22bffeb3154dca3
Author: Ben Pfaff <[email protected]>
Date: Wed Jan 24 11:40:20 2018 -0800
odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP.
OVS datapaths don't understand or parse IGMP fields, but OVS
userspace
does, so this commit updates odp_flow_key_to_flow() to report that
properly
to the caller.
Reported-by: Huanle Han <[email protected]>
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/343665.html
Signed-off-by: Ben Pfaff <[email protected]>
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 14d5af097..5da83b4c6 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -6210,6 +6210,11 @@ parse_l2_5_onward(const struct nlattr
*attrs[OVS_KEY_ATTR_MAX + 1],
}
}
}
+ } else if (src_flow->nw_proto == IPPROTO_IGMP
+ && src_flow->dl_type == htons(ETH_TYPE_IP)) {
+ /* OVS userspace parses the IGMP type, code, and group, but its
+ * datapaths do not, so there is always missing information. */
+ return ODP_FIT_TOO_LITTLE;
}
if (is_mask && expected_bit != OVS_KEY_ATTR_UNSPEC) {
if ((flow->tp_src || flow->tp_dst) && flow->nw_proto != 0xff)
{
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
