On Tue, Sep 03, 2024 at 06:12:03PM +0200, Paolo Valerio wrote: > Simon Horman <ho...@ovn.org> writes: > > > On Mon, Sep 02, 2024 at 01:28:48PM +0200, Paolo Valerio wrote: > >> Paolo Valerio <pvale...@redhat.com> writes: > >> > >> > Simon Horman <ho...@ovn.org> writes: > >> > > >> >> On Wed, Aug 28, 2024 at 07:14:06PM +0200, Paolo Valerio wrote: > >> >>> As Long reported, kernels built without CONFIG_NETFILTER_CONNCOUNT > >> >>> result in the unexpected failure of the following tests: > >> >>> > >> >>> conntrack - multiple zones, local > >> >>> conntrack - multi-stage pipeline, local > >> >>> conntrack - can match and clear ct_state from outside OVS > >> >>> > >> >>> this happens because the nf_conncount turns on connection tracking and > >> >>> the above tests rely on this side effect. However, this behavior may > >> >>> be corrected in the kernel, which could, in turn, cause the tests to > >> >>> fail. > >> >>> > >> >>> The patch removes the assumption by adding explicit iptables rules to > >> >>> attach an nf_conn template to the skb resulting tracked once hit the > >> >>> OvS pipeline. > >> >>> > >> >>> While at it, introduce $HAVE_IPTABLES and skip tests if iptables > >> >>> binary is not present. > >> >>> > >> >>> Reported-by: Xin Long <lucien....@gmail.com> > >> >>> Reported-at: https://issues.redhat.com/browse/FDP-708 > >> >>> Signed-off-by: Paolo Valerio <pvale...@redhat.com> > >> >> > >> >> Hi Paolo, > >> >> > >> >> I exercised this using vng with net-next compiled using > >> >> tools/testing/selftests/net/config from the upstream kernel tree [1]. > >> >> > >> >> [1] > >> >> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style > >> >> > >> >> The resulting config does not have CONFIG_NETFILTER_CONNCOUNT set. > >> >> > >> > > >> > Hi Simon, > >> > > >> > I used vng with net-next as well, but with a different config. In my > >> > case I simply reused a previously used config which is essentially my > >> > local one updated with olddefconfig and manually (to remove/add things). > >> > > >> >> Some observations: > >> >> > >> >> * CONFIG_NETFILTER_XT_TARGET_CT is required for -j CT > >> >> > >> >> I don't think this is a problem (other than my own problem > >> >> of it taking me a long time to figure that out). But it seems > >> >> worth noting (see parentheses in previous sentence:). > >> >> > >> > > >> > It's something I was thinking about right after the patch submission. > >> > Although extensions are fairly common in the main distros, maybe we may > >> > consider checking they are present. > >> > > >> > The only ways that comes to mind to reliably check whether the > >> > extensions (both kernel and user space) are present is simply by > >> > applying a rule. > >> > > >> > I guess we can (added a diff at the bottom), given the nft discussion > >> > and the potential follow-up, change things a bit in order to better > >> > accomodate a potential migration/follow-up. > >> > > >> > WDYT? > > > > Yes, I think this is a good idea. > > > > As an aside: While looking over this I noticed CHECK_CONNTRACK_TIMEOUT. It > > seems this relies on NF_CONNTRACK_TIMEOUT being a module (not a builtin), > > and /boot/config-$(uname -r) corresponding to the running kernel. I guess > > that is ok most of the time, but it produced a false-skip when running > > tests under vng. > > > > Another aside: I'm unsure how to create an equivalent of the following with > > nft. Do you happen to know? Perhaps we can use notrack as a feature > > test instead? > > > > iptables -t raw -I OUTPUT 1 -o $1 -j CT > > This should work: > > nft -f - <<EOF > table ip raw { > chain output { > type filter hook output priority raw; > oifname "..." ct state new > } > } > EOF > > also it seems that adding -c is enough to reliably check > that the requirement are in place.
Nice! Thanks! > >> >> * Of the tests that are updated by this patch, > >> >> I only observed that the last one, > >> >> "conntrack - can match and clear ct_state from outside OVS", > >> >> fails without this patch applied. > >> >> > >> >> I am unsure if that is something that warrants updating this > >> >> patch or not. Or if, rather, there is an error in my testing. > >> > > >> > Thanks for testing it. > >> > > >> > Interesting. > >> > I tried the following config against net-next and it seems I can't > >> > reproduce that behaviour: > >> > > >> > vng --build --config tools/testing/selftests/net/config \ > >> > --configitem CONFIG_NF_CONNTRACK_ZONES=y \ > >> > --configitem CONFIG_NETFILTER_XT_TARGET_CT=m -v > > > > Sorry about this. I spent some time on this again. > > > > I have verified that all the conntrack tests modified > > by this patch fail without it, as expected. > > > > no problem, thanks for double checking > > > And that I misunderstood the changes to the datapath tests. > > As they are separate from the conncount update I suggest > > breaking them out into a separate patch. > > > > do you mean to keep the change below separate but still part of the > series? Maybe I misunderstand things, but what I was getting at is: 1. Patch to update tests that depend on iptables (but not external CT) 2. Patch to update tests that depend on external CT If that is nonsensical then feel free to ignore it :) ... _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
