On 4/12/23 03:29, 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]> > ---
Hi, Ihar, Thanks for the patch! For some reason the 0-day robot didn't reply back the CI status with this patch applied. There are some failures: https://github.com/ovsrobot/ovn/actions/runs/4673670032 One is because the tests fail to run scapy: Traceback (most recent call last): File "<stdin>", line 1, in <module> ModuleNotFoundError: No module named 'scapy' "netdev-dummy/receive" command requires at least 2 arguments ovs-appctl: ovs-vswitchd: server returned an error We do install python3-scapy in GitHub CI so I'm not sure what's going on there, it's a bit weird. The other failures are a couple of tests that need to be adjusted to match or ignore the new flows too. Regards, Dumitru > NEWS | 2 + > northd/northd.c | 20 ++++--- > northd/ovn-northd.8.xml | 18 ++++-- > tests/ovn.at | 127 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 153 insertions(+), 14 deletions(-) > > diff --git a/NEWS b/NEWS > index f71f408d0..60467581a 100644 > --- a/NEWS > +++ b/NEWS > @@ -12,6 +12,8 @@ Post v23.03.0 > target mac address matches. Arp requests matching no Logical Router will > only be forwarded to non-router ports. Default is true which keeps the > existing behaviour of flooding these arp requests to all attached Ports. > + - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast > + Listener Discovery protocols, regardless of ACLs defined. > > OVN v23.03.0 - 03 Mar 2023 > -------------------------- > diff --git a/northd/northd.c b/northd/northd.c > index da3c8cf77..830932bc6 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -6943,20 +6943,24 @@ build_acls(struct ovn_datapath *od, const struct > chassis_features *features, > ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, > ds_cstr(&match), ct_out_acl_action); > > - /* Ingress and Egress ACL Table (Priority 65532). > - * > - * Not to do conntrack on ND packets. */ > - ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3, > - "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;"); > - ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, > - "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;"); > - > /* Reply and related traffic matched by an "allow-related" ACL > * should be allowed in the ls_in_acl_after_lb stage too. */ > ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_AFTER_LB, UINT16_MAX - 3, > REGBIT_ACL_HINT_ALLOW_REL" == 1", "next;"); > } > > + /* Ingress and Egress ACL Table (Priority 65532). > + * > + * Always allow service IPv6 protocols regardless of other ACLs defined. > + * > + * Also, don't send them to conntrack because session tracking > + * for these protocols is not working properly: > + * https://bugzilla.kernel.org/show_bug.cgi?id=11797. */ > + ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3, > + "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;"); > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, > + "nd || nd_ra || nd_rs || mldv1 || mldv2", "next;"); > + > /* Ingress or Egress ACL Table (Various priorities). */ > for (size_t i = 0; i < od->nbs->n_acls; i++) { > struct nbrec_acl *acl = od->nbs->acls[i]; > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 98aa060b9..2d1b60307 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -748,6 +748,12 @@ > drop behavior. > </p> > > + <p> > + A priority-65532 flow is added to allow IPv6 Neighbor solicitation, > + Neighbor discover, Router solicitation, Router advertisement and MLD > + packets regardless of other ACLs defined. > + </p> > + > <p> > If the logical datapath has a stateful ACL or a load balancer with VIP > configured, the following flows will also be added: > @@ -824,12 +830,6 @@ > in the request direction are skipped here to let a newly created > ACL re-allow this connection. > </li> > - > - <li> > - A priority-65532 flow that allows IPv6 Neighbor solicitation, > - Neighbor discover, Router solicitation, Router advertisement and MLD > - packets. > - </li> > </ul> > > <p> > @@ -2123,6 +2123,12 @@ output; > <code>to-lport</code> ACLs. > </p> > > + <p> > + Similar to ingress table, a priority-65532 flow is added to allow IPv6 > + Neighbor solicitation, Neighbor discover, Router solicitation, Router > + advertisement and MLD packets regardless of other ACLs defined. > + </p> > + > <p> > In addition, the following flows are added. > </p> > diff --git a/tests/ovn.at b/tests/ovn.at > index 3246ac852..214c961e2 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -13898,6 +13898,133 @@ 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 > + > + 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}"' > + > +wait_for_ports_up > + > +test_ns_na() { > + local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5 > + > + packet=$(fmt_pkt "\ > + Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') /\ > + IPv6(src='${src_ip}', dst='ff02::1:ff00:2') /\ > + ICMPv6ND_NS(tgt='${dst_ip}') \ > + ") > + as hv1 ovs-appctl netdev-dummy/receive vif${inport} $packet > + > + expected_packet=$(fmt_pkt "\ > + Ether(dst='${src_mac}', src='${dst_mac}') /\ > + IPv6(src='${dst_ip}', dst='${src_ip}') /\ > + ICMPv6ND_NA(tgt='${dst_ip}', R=0, S=1) /\ > + ICMPv6NDOptDstLLAddr(lladdr='${dst_mac}') \ > + ") > + echo $expected_packet >> $inport.expected > +} > + > +test_rs_ra() { > + local inport=$1 src_mac=$2 src_ip=$3 > + local router_mac=$4 router_prefix=$5 router_ip=$6 > + > + packet=$(fmt_pkt "\ > + Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') /\ > + IPv6(src='${src_ip}', dst='ff02::2') /\ > + ICMPv6ND_RS() \ > + ") > + as hv1 ovs-appctl netdev-dummy/receive vif${inport} $packet > + > + expected_packet=$(fmt_pkt "\ > + Ether(dst='${src_mac}', src='${router_mac}') /\ > + IPv6(src='${router_ip}', dst='${src_ip}') /\ > + ICMPv6ND_RA(chlim=255, prf=0, routerlifetime=65535) /\ > + ICMPv6NDOptSrcLLAddr(lladdr='${router_mac}') /\ > + ICMPv6NDOptPrefixInfo(prefix='${router_prefix}') \ > + ") > + echo $expected_packet >> $inport.expected > +} > + > +test_mldv2() { > + local inport=$1 outport=$2 src_mac=$3 src_ip=$4 > + > + packet=$(fmt_pkt "\ > + Ether(dst='ff:ff:ff:ff:ff:ff', src='${src_mac}') /\ > + IPv6(src='${src_ip}', dst='ff02::2') /\ > + ICMPv6MLQuery2() \ > + ") > + as hv1 ovs-appctl netdev-dummy/receive vif${inport} $packet > + > + expected_packet=$packet > + echo $expected_packet >> $outport.expected > +} > + > +src_mac=${lsp_mac_prefix}1 > +dst_mac=${lsp_mac_prefix}2 > +src_ip=${lsp_ip6_prefix}1 > +dst_ip=${lsp_ip6_prefix}2 > + > +as hv1 > +test_ns_na 1 $src_mac $dst_mac $src_ip $dst_ip > + > +as hv1 > +router_local_ip=fe80::f816:3eff:fe00:1 > +test_rs_ra 1 $src_mac $src_ip $router_mac $router_prefix $router_local_ip > + > +as hv1 > +src_ip=fe80::1 > +test_mldv2 1 2 $src_mac $src_ip > + > +ovs-ofctl dump-flows br-int > +ovn-sbctl list Logical_Flow > + > +OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [1.expected]) > +OVN_CHECK_PACKETS([hv1/vif2-tx.pcap], [2.expected]) > + > +OVN_CLEANUP([hv1]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([IPv6 Neighbor Solicitation for unknown MAC]) > AT_KEYWORDS([ovn-nd_ns for unknown mac]) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
