On 9/15/21 21:04, Aaron Conole wrote: > When OVS starts with default settings, it will have no existing datapath > flows configured, and it will not explicitly handle IGMP packets. This > means that all traffic will hit the datapath, and then follow the > xlate_normal processing path. > > Unfortunately for some users of IGMP-aware applications (such as > 'keepalived'), IGMP packets will arrive, go through processing and a > default flow like following will be installed: > > recirc_id(0),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), > actions:userspace(pid=xxxxxxx,slow_path(match)) > > This is a very broad match - and will force all IPv4 traffic to userspace. > > To combat this, force the wildcard initialization to always include an > IGMP protocol match. An existing IGMP check is only run when multicast > snooping is configured. Now we will always run the check during wildcard > init. A unit test is added that works for kernel and userspace datapaths. > > Reported-by: Lorenzo Bianconi <[email protected]> > Reported-by: Mohamed Mahmoud <[email protected]> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2002888 > Signed-off-by: Aaron Conole <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 8 ++++++++ > tests/system-traffic.at | 28 ++++++++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 8723cb4e85..dc3971cdf9 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -7381,6 +7381,14 @@ xlate_wc_init(struct xlate_ctx *ctx) > WC_MASK_FIELD_MASK(ctx->wc, nw_frag, FLOW_NW_FRAG_MASK); > } > > + /* Always check for igmp type in the packet. This will ensure that > + * the igmp nw type will properly be set as a match field. */ > + if (get_dl_type(&ctx->xin->flow) == htons(ETH_TYPE_IP)) { > + if (ctx->xin->flow.nw_proto == IPPROTO_IGMP && ctx->wc) { > + WC_MASK_FIELD(ctx->wc, nw_proto); > + } > + } > + > if (ctx->xbridge->support.odp.recirc) { > /* Always exactly match recirc_id when datapath supports > * recirculation. */ > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index de9108ac20..e0836839d6 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at
Hi, Aaron. I'm not going deep into the issue and the actual change for now, but it seems to be very generic. Can we have a test in a main testsuite instead of a system one? e.g. in odproto-dpif.at. Packet can be injected with 'ovs-appctl netdev-dummy/receive' in this case. Best regards, Ilya Maximets. > @@ -6147,6 +6147,34 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | > OFPROTO_CLEAR_DURATION_IDLE > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_BANNER([IGMP]) > + > +AT_SETUP([IGMP - VRRP VSS padded]) > + > +OVS_TRAFFIC_VSWITCHD_START() > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") > + > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 dnl > +f0 00 00 01 01 01 08 00 46 c0 00 28 00 00 40 00 01 02 d3 49 45 65 eb 4a e0 > dnl > +00 00 16 94 04 00 00 22 00 f9 02 00 00 00 01 04 00 00 00 e0 00 00 fb 00 00 > dnl > +00 00 00 00 > /dev/null]) > + > +AT_CHECK([ovs-appctl dpctl/dump-flows | grep -e .*ipv4 | dnl > + sed -e 's/ packets:[[0-9]]*,//' -e 's/ bytes:[[0-9]]*,//' dnl > + -e 's/ used:[[a-zA-Z0-9]]*,//' -e 's/pid=[[0-9]]*,//' dnl > + -e 's/,packet_type(ns=[[0-9]]*,id=[[0-9]]*),/,eth(),/' dnl > + -e 's/actions:drop/actions:userspace(slow_path(match))/'], > + [0], [dnl > +recirc_id(0),in_port(2),eth(),eth_type(0x0800),ipv4(proto=2,frag=no), > actions:userspace(slow_path(match)) > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > + > AT_BANNER([802.1ad]) > > AT_SETUP([802.1ad - vlan_limit]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
