[ovs-dev] [PATCH ovn] ovn-northd: Skip conntrack for MLD packets.
Numan Siddique
numans at ovn.org
Fri Sep 11 09:46:37 UTC 2020
On Fri, Sep 11, 2020 at 2:25 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> We currently skip conntrack for IPv6 Neighbor Discovery packets because
> conntrack marks all ND packets as invalid [0].
>
> The same thing should be done for MLD packets. Otherwise, as soon as an
> allow-related ACL or load balancer is added, MLD packets will go to
> conntrack and get dropped because they are marked "invalid".
>
> This commit also fixes the MLD test to use a link local IPv6 source
> address.
>
> [0] https://bugzilla.kernel.org/show_bug.cgi?id=11797
>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
Thanks Dumitru. I applied this patch to master. Does it need a
backport to 20.06 ?
Numan
> ---
> northd/ovn-northd.8.xml | 16 +++++++++-------
> northd/ovn-northd.c | 12 ++++++------
> tests/ovn.at | 23 +++++++++++++++++++----
> 3 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index a275442..e09b28a 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -318,7 +318,8 @@
> <code>Pre-stateful</code> to send IP packets to the connection tracker
> before eventually advancing to ingress table <code>ACLs</code>. If
> special ports such as route ports or localnet ports can't use ct(), a
> - priority-110 flow is added to skip over stateful ACLs.
> + priority-110 flow is added to skip over stateful ACLs. IPv6 Neighbor
> + Discovery and MLD traffic also skips stateful ACLs.
> </p>
>
> <p>
> @@ -337,11 +338,12 @@
> This table prepares flows for possible stateful load balancing processing
> in ingress table <code>LB</code> and <code>Stateful</code>. It contains
> a priority-0 flow that simply moves traffic to the next table. Moreover
> - it contains a priority-110 flow to move IPv6 Neighbor Discovery traffic
> - to the next table. If load balancing rules with virtual IP addresses
> - (and ports) are configured in <code>OVN_Northbound</code> database for a
> - logical switch datapath, a priority-100 flow is added for each configured
> - virtual IP address <var>VIP</var>. For IPv4 <var>VIPs</var>, the match is
> + it contains a priority-110 flow to move IPv6 Neighbor Discovery and MLD
> + traffic to the next table. If load balancing rules with virtual IP
> + addresses (and ports) are configured in <code>OVN_Northbound</code>
> + database for a logical switch datapath, a priority-100 flow is added for
> + each configured virtual IP address <var>VIP</var>. For IPv4
> + <var>VIPs</var>, the match is
> <code>ip && ip4.dst == <var>VIP</var></code>. For IPv6
> <var>VIPs</var>, the match is <code>ip &&
> ip6.dst == <var>VIP</var></code>. The flow sets an action
> @@ -478,7 +480,7 @@
>
> <li>
> A priority-65535 flow that allows IPv6 Neighbor solicitation,
> - Neighbor discover, Router solicitation and Router advertisement
> + Neighbor discover, Router solicitation, Router advertisement and MLD
> packets.
> </li>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b95d6cd..48d17a4 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4924,10 +4924,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
> * Not to do conntrack on ND and ICMP destination
> * unreachable packets. */
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> - "nd || nd_rs || nd_ra || "
> + "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
> "(udp && udp.src == 546 && udp.dst == 547)", "next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
> - "nd || nd_rs || nd_ra || "
> + "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
> "(udp && udp.src == 546 && udp.dst == 547)", "next;");
>
> /* Ingress and Egress Pre-ACL Table (Priority 100).
> @@ -5040,10 +5040,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows,
> {
> /* Do not send ND packets to conntrack */
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> - "nd || nd_rs || nd_ra",
> + "nd || nd_rs || nd_ra || mldv1 || mldv2",
> "next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
> - "nd || nd_rs || nd_ra",
> + "nd || nd_rs || nd_ra || mldv1 || mldv2",
> "next;");
>
> /* Do not send service monitor packets to conntrack. */
> @@ -5575,9 +5575,9 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows,
> *
> * Not to do conntrack on ND packets. */
> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX,
> - "nd || nd_ra || nd_rs", "next;");
> + "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX,
> - "nd || nd_ra || nd_rs", "next;");
> + "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;");
> }
>
> /* Ingress or Egress ACL Table (Various priorities). */
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 4e58722..1898728 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17339,7 +17339,7 @@ store_mld_query() {
> local mld_type=82
> local mld_code=00
> local max_resp=03e8
> - local mld_chksum=59be
> + local mld_chksum=7b3d
> local addr=00000000000000000000000000000000
>
> local eth=${eth_dst}${eth_src}86dd
> @@ -17419,6 +17419,21 @@ ovn-nbctl lsp-add sw3 sw3-rtr \
> -- lsp-set-addresses sw3-rtr 00:00:00:00:03:00 \
> -- lsp-set-options sw3-rtr router-port=rtr-sw3
>
> +# Conntrack marks all IPv6 Neighbor Discovery and MLD packets as invalid,
> +# make sure to test that conntrack is bypassed for MLD by adding an empty
> +# allow-related ACL and an empty load balancer.
> +ovn-nbctl acl-add sw1 from-lport 1 "1" allow-related
> +ovn-nbctl acl-add sw2 from-lport 1 "1" allow-related
> +ovn-nbctl acl-add sw3 from-lport 1 "1" allow-related
> +ovn-nbctl acl-add sw1 to-lport 1 "1" allow-related
> +ovn-nbctl acl-add sw2 to-lport 1 "1" allow-related
> +ovn-nbctl acl-add sw3 to-lport 1 "1" allow-related
> +
> +ovn-nbctl lb-add lb0 [[4242::1]]:80 ""
> +ovn-nbctl ls-lb-add sw1 lb0
> +ovn-nbctl ls-lb-add sw2 lb0
> +ovn-nbctl ls-lb-add sw3 lb0
> +
> net_add n1
> sim_add hv1
> as hv1
> @@ -17614,12 +17629,12 @@ ovn-nbctl set Logical_Switch sw2 \
> other_config:mcast_querier="true" \
> other_config:mcast_query_interval=1 \
> other_config:mcast_eth_src="00:00:00:00:02:fe" \
> - other_config:mcast_ip6_src="2000::fe"
> + other_config:mcast_ip6_src="fe80::fe"
>
> # Wait for 1 query interval (1 sec) and check that two queries are generated.
> > expected
> -store_mld_query 0000000002fe 200000000000000000000000000000fe expected
> -store_mld_query 0000000002fe 200000000000000000000000000000fe expected
> +store_mld_query 0000000002fe fe8000000000000000000000000000fe expected
> +store_mld_query 0000000002fe fe8000000000000000000000000000fe expected
> sleep 1
>
> OVN_CHECK_PACKETS([hv1/vif3-tx.pcap], [expected])
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list