On 4/13/23 23:35, Ihar Hrachyshka wrote:
> When setting flows for LS, OVN distinguishes between two states: where
> there’s a stateful ACL present in its list (has_stateful == true *)
> and when it’s missing (all ACLs are stateless).
>
> When has_stateful == true, the following is done (among other things):
> - ct handling flows are set;
> - they are omitted by a higher priority flow for “service” protocols:
> NA, RA, MLD.
>
> The latter is done because of a known issue in kernel ct
> implementation for the protocols:
>
> * https://bugzilla.kernel.org/show_bug.cgi?id=11797
>
> The assumption is that by default OVN allows all traffic unless
> explicitly forbidden, so omitting ct flows only avoids ct machinery
> but doesn't affect functional behavior of flow tables for the
> protocols.
>
> But if an ACL that forbids these protocols is configured, because of
> the ct omittance, this ACL is not in effect. (But only when
> has_stateful == true.)
>
> This behavior results in inconsistent and confusing behavior in
> OpenStack Neutron where
>
> (1) the default security group behavior is drop all IP traffic
> (achieved with default "drop" Port_Group); and
> (2) ports that have stateful and stateless ACLs configured can
> co-exist in the same network.
>
> In which case, depending on other "stateful" ports present in the
> network, "stateless" ports may or may not observe RA / NA / MLD
> traffic. Which affects their IPv6 address configuration.
>
> In this patch, I suggest that we don't make RA / NA / MLD behavior
> dependent on whether "stateful" ACLs are present. Instead, make the
> protocols always allowed, regardless of ACLs configured (whether
> stateful ACLs or ACLs that forbid packets of these protocols).
>
> Note: an argument can be made that the same "always-on" behavior
> should be guaranteed for ARP protocol that serves a similar goal in a
> IPv4 network as RA / NA do for IPv6 networks. This scenario is not
> directly related to the inconsistency between "purely stateless" and
> "mixed-stateful" networks and hence is left for a follow-up patch.
>
> Note: this patch carries a test case that utilizes scapy tool to
> construct packets for the protocols under test. A proper backport may
> demand backporting scapy related patches too.
>
> Reported-At: https://bugs.launchpad.net/neutron/+bug/2006949
> Reported-At: https://bugzilla.redhat.com/show_bug.cgi?id=2149731
> Signed-off-by: Ihar Hrachyshka <[email protected]>
> ---
> v1: initial version
> v2: remove debug ov?-*ctl commands from the new test case
> v2: adjust failing test cases that didn't expect new flows
> ---
Hi Ihar,
Thanks for this new revision. I spotted a problem with apply-after-lb
ACLs however, please see below.
> NEWS | 2 +
> northd/northd.c | 20 ++++---
> northd/ovn-northd.8.xml | 18 ++++--
> tests/ovn-northd.at | 26 +++++++++
> tests/ovn.at | 124 ++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 176 insertions(+), 14 deletions(-)
>
[...]
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 8e80765c6..2d92e96a9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -13898,6 +13898,130 @@ OVN_CLEANUP([gw1],[gw2],[hv1])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([allow IPv6 RA / NA / MLD by default])
> +AT_SKIP_IF([test $HAVE_SCAPY = no])
> +ovn_start
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +lsp_mac_prefix=50:64:00:00:00:0
> +lsp_ip_prefix=10.0.0.
> +lsp_ip6_prefix=aef0::5264:00ff:fe00:000
> +
> +check ovn-nbctl ls-add ls0
> +for i in 1 2; do
> + check ovn-nbctl lsp-add ls0 lsp$i
> + check ovn-nbctl lsp-set-addresses lsp$i \
> + "${lsp_mac_prefix}$i ${lsp_ip_prefix}$i ${lsp_ip6_prefix}$i"
> +
> + # forbid all traffic for the ports
> + for direction in from to; do
> + check ovn-nbctl acl-add ls0 $direction-lport 1000 "outport == \"lsp$i\""
> drop
> + done
This doesn't make sense in the "from" direction. We'll never match it
because "outport" is probably not set by the time we're in the IN_ACL
stages.
I changed it to:
# forbid all traffic for the ports
check ovn-nbctl acl-add ls0 from-lort 1000 "inport == \"lsp$i\"" drop
check ovn-nbctl --apply-after-lb acl-add ls0 from-lport 1000 "inport ==
\"lsp$i\"" drop
check ovn-nbctl acl-add ls0 to-lport 1000 "outport == \"lsp$i\"" drop
And this uncovered the fact that we're missing a flow in the
S_SWITCH_IN_ACL_AFTER_LB stage.
> +
> + ovs-vsctl -- add-port br-int vif$i -- \
> + set interface vif$i external-ids:iface-id=lsp$i \
> + options:tx_pcap=hv1/vif$i-tx.pcap \
> + options:rxq_pcap=hv1/vif$i-rx.pcap
> + : > $i.expected
> +done
> +
> +router_mac=fa:16:3e:00:00:01
> +router_prefix=fdad:1234:5678::
> +router_ip=${router_prefix}1
> +ovn-nbctl lr-add lr0
> +ovn-nbctl lrp-add lr0 lrp0 ${router_mac} ${router_ip}/64
> +ovn-nbctl set Logical_Router_Port lrp0 ipv6_ra_configs:address_mode="slaac"
> +ovn-nbctl \
> + -- lsp-add ls0 rp0 \
> + -- set Logical_Switch_Port rp0 type=router \
> + options:router-port=lrp0 \
> + addresses='"${router_mac} ${router_ip}"'
Nit: Please prefix all the ovn-nbctl commands with "check ".
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
