From patchwork Mon Nov 29 21:02:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Conole X-Patchwork-Id: 1561363 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Bw2UnJYr; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4J2yVm6Y0yz9sRK for ; Tue, 30 Nov 2021 08:03:12 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id C925740290; Mon, 29 Nov 2021 21:03:10 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gvwylaJCfbWm; Mon, 29 Nov 2021 21:03:09 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id B35764021B; Mon, 29 Nov 2021 21:03:08 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 45662C001C; Mon, 29 Nov 2021 21:03:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 562E0C000A for ; Mon, 29 Nov 2021 21:03:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 50D5360A4E for ; Mon, 29 Nov 2021 21:03:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp3.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id iA0cijJrd8rn for ; Mon, 29 Nov 2021 21:03:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 0755E607CB for ; Mon, 29 Nov 2021 21:03:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1638219784; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PnIUlHm/OBq0RJQj5jPdsPDec7/fkgRG/XC11OwVL1o=; b=Bw2UnJYrIEuJiHopPGhzRFjaY6WMXlCIWSOEto+UEAQIKNa6+u2tC5q6Ai4Znco0UZeBbu bSqbzrvxz6H+lM9zQ6z3OX1zNdHn+Szvb5+QTPzixmrduK3A/36oLfFYD1Q2ufUxWdfcdJ tmcBOqqJDvLBTWcSAzGOtfqcXsBWObg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-463-2gA98fQYPjGcXZa-kHS_4A-1; Mon, 29 Nov 2021 16:03:02 -0500 X-MC-Unique: 2gA98fQYPjGcXZa-kHS_4A-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7FC53192CC42; Mon, 29 Nov 2021 21:03:01 +0000 (UTC) Received: from RHTPC1VM0NT.redhat.com (unknown [10.22.9.182]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6CAF05D6BA; Mon, 29 Nov 2021 21:03:00 +0000 (UTC) From: Aaron Conole To: dev@openvswitch.org Date: Mon, 29 Nov 2021 16:02:59 -0500 Message-Id: <20211129210259.1218307-1-aconole@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=aconole@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: Ben Pfaff , Huanle Han , Ilya Maximets Subject: [ovs-dev] [PATCH v3] Revert "odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP." X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" This reverts commit c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP.") Always forcing a slow path action can result in some over-broad flows which swallow all traffic and force them to userspace, as reported in the thread at https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387706.html and at https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387689.html Revert the ODP_FIT_TOO_LITTLE return for IGMP specifically. Additionally, remove the userspace wc mask to prevent revalidator from cycling flows. Fixes: c645550bb249 ("odp-util: Always report ODP_FIT_TOO_LITTLE for IGMP.") Signed-off-by: Aaron Conole Acked-by: Eelco Chaudron Acked-by: Eelco Chaudron --- v1->v2: Add unit test for coverage of mcast traffic when not doing snooping v2->v3: Add a unit test for the non-dummy datapaths as well to demonstrate the matching issue. lib/odp-util.c | 5 --- ofproto/ofproto-dpif-xlate.c | 1 - tests/mcast-snooping.at | 72 ++++++++++++++++++++++++++++++++++++ tests/system-traffic.at | 65 ++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 6 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index fbdfc7ad83..0f21ffe639 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -7132,11 +7132,6 @@ 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) { diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 88cdc09a0e..dacee0642d 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3048,7 +3048,6 @@ xlate_normal(struct xlate_ctx *ctx) */ ctx->xout->slow |= SLOW_ACTION; - memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src); if (mcast_snooping_is_membership(flow->tp_src) || mcast_snooping_is_query(flow->tp_src)) { if (ctx->xin->allow_side_effects && ctx->xin->packet) { diff --git a/tests/mcast-snooping.at b/tests/mcast-snooping.at index 757cf7186e..478aced91d 100644 --- a/tests/mcast-snooping.at +++ b/tests/mcast-snooping.at @@ -216,3 +216,75 @@ AT_CHECK([ovs-appctl mdb/show br0], [0], [dnl ]) AT_CLEANUP + + +AT_SETUP([mcast - igmp flood for non-snoop enabled]) +OVS_VSWITCHD_START([]) + +AT_CHECK([ + ovs-vsctl set bridge br0 \ + datapath_type=dummy], [0]) + +AT_CHECK([ + ovs-vsctl add-port br0 p0 -- set Interface p0 type=dummy \ + other-config:hwaddr=aa:55:aa:55:00:01 ofport_request=1 \ + -- add-port br0 p1 \ + -- set Interface p1 type=dummy other-config:hwaddr=aa:55:aa:55:00:02 ofport_request=2 \ +], [0]) + +AT_CHECK([ovs-ofctl add-flow br0 action=normal]) + +ovs-appctl time/stop + +dnl Basic scenario - needs to flood for IGMP followed by unicast ICMP +dnl in reverse direction +AT_CHECK([ovs-appctl netdev-dummy/receive p0 \ + '0101000c29a0aa55aa550001080046c00028000040000102d3494565eb4ae0000016940400002200f9020000000104000000e00000fb000000000000']) +AT_CHECK([ovs-appctl netdev-dummy/receive p1 \ + 'aa55aa5500010101000c29a008004500001c00010000400164dc0a0101010a0101020800f7ffffffffff']) + +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]]*),/,/'], + [0], [dnl +recirc_id(0),in_port(1),eth(src=aa:55:aa:55:00:01,dst=01:01:00:0c:29:a0),eth_type(0x0800),ipv4(frag=no), actions:100,2 +recirc_id(0),in_port(2),eth(src=01:01:00:0c:29:a0,dst=aa:55:aa:55:00:01),eth_type(0x0800),ipv4(frag=no), actions:1 +]) + +ovs-appctl time/warp 100000 + +dnl Next we should clear the flows and install a complex case +AT_CHECK([ovs-ofctl del-flows br0]) + +AT_DATA([flows.txt], [dnl +table=0, arp actions=NORMAL +table=0, ip,in_port=1 actions=ct(table=1,zone=64000) +table=0, in_port=2 actions=output:1 +table=1, ip,ct_state=+trk+inv actions=drop +table=1 ip,in_port=1,icmp,ct_state=+trk+new actions=output:2 +table=1, in_port=1,ip,ct_state=+trk+new actions=controller(userdata=00.de.ad.be.ef.ca.fe.01) +table=1, in_port=1,ip,ct_state=+trk+est actions=output:2 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +ovs-appctl time/stop + +dnl Send the IGMP, followed by a unicast ICMP - ensure we won't black hole +AT_CHECK([ovs-appctl netdev-dummy/receive p0 \ + '0101000c29a0aa55aa550001080046c00028000040000102d3494565eb4ae0000016940400002200f9020000000104000000e00000fb000000000000']) +AT_CHECK([ovs-appctl netdev-dummy/receive p0 \ + 'aa55aa550001aa55aa55000208004500001c00010000400164dc0a0101010a0101020800f7ffffffffff']) + + +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]]*),/,/'], + [0], [dnl +ct_state(+new-inv+trk),recirc_id(0x1),in_port(1),eth_type(0x0800),ipv4(proto=2,frag=no), actions:userspace(controller(reason=1,dont_send=0,continuation=0,recirc_id=2,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(0),in_port(1),eth_type(0x0800),ipv4(frag=no), actions:ct(zone=64000),recirc(0x1) +ct_state(+new-inv+trk),recirc_id(0x1),in_port(1),eth_type(0x0800),ipv4(proto=1,frag=no), actions:2 +]) + +AT_CLEANUP diff --git a/tests/system-traffic.at b/tests/system-traffic.at index d79753a99a..e77799489d 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6287,6 +6287,71 @@ 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 - flood under normal action]) + +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") + +AT_CHECK([ovs-ofctl add-flow br0 "actions=NORMAL"]) + +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]]*,//'], + [0], [dnl +recirc_id(0),in_port(2),eth(src=f0:00:00:01:01:01,dst=f0:00:00:01:01:02),eth_type(0x0800),ipv4(frag=no), actions:1,3 +]) + +AT_CHECK([ovs-ofctl del-flows br0]) +AT_DATA([flows.txt], [dnl +table=0, arp actions=NORMAL +table=0, ip,in_port=1 actions=ct(table=1,zone=64000) +table=0, in_port=2 actions=output:1 +table=1, ip,ct_state=+trk+inv actions=drop +table=1 ip,in_port=1,icmp,ct_state=+trk+new actions=output:2 +table=1, in_port=1,ip,ct_state=+trk+new actions=controller(userdata=00.de.ad.be.ef.ca.fe.01) +table=1, in_port=1,ip,ct_state=+trk+est actions=output:2 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +OVS_WAIT_UNTIL([ovs-appctl dpctl/dump-flows | wc -l | grep -E '^0$']) + +dnl Send the IGMP, followed by a unicast ICMP - ensure we won't black hole + +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]) + +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 45 00 00 1c 00 01 00 00 40 01 64 dc 0a 01 01 01 0a dnl +01 01 02 08 00 f7 ff ff ff ff ff > /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/recirc_id([[x0-9]]*)/recirc_id()/' dnl + -e 's/recirc_id=[[x0-9]]*/recirc_id=/' dnl + -e 's/recirc([[x0-9]]*)/recirc()/' dnl + -e 's/pid=[[0-9]]*,//' -e 's/ used:[[\.a-zA-Z0-9]]*,//' | sort], + [0], [dnl +recirc_id(),in_port(2),ct_state(+new-inv+trk),eth(),eth_type(0x0800),ipv4(proto=1,frag=no), actions:3 +recirc_id(),in_port(2),ct_state(+new-inv+trk),eth(),eth_type(0x0800),ipv4(proto=2,frag=no), actions:userspace(controller(reason=1,dont_send=0,continuation=0,recirc_id=,rule_cookie=0,controller_id=0,max_len=65535)) +recirc_id(),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), actions:ct(zone=64000),recirc() +recirc_id(),in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), actions:2 +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([802.1ad]) AT_SETUP([802.1ad - vlan_limit])