On 12/12/22 09:40, Eelco Chaudron wrote: > > > On 10 Dec 2022, at 3:16, Ilya Maximets wrote: > >> This patch set allows OVS to build with libxdp and newer libbpf. >> It also enables AF_XDP support by default as long as all the >> dependencies are available. > > Hi Ilya, > > I did not yet review the patch, but I do not like enabling AF_XPD support by > default for the following reasons:
Hi, Eelco. Thanks for taking a look! > > - I still believe AF_XDP support in OVS has not had many field trials, > so I would still classify this as an “experimental” feature. This patch set doesn't make the feature non-experimental. There is still a warning about that in the documentation. See the patch 3/6. In general, we do build a lot of experimental features by default and users can enable these features. For example, some PMD management features, userspace TSO or AVX512 support. Enabling the build may increase the number of field trials performed by enthusiastic users, so should have some positive impact on development in general. I've seen an increased interest in trying out this feature recently. Also, the feature doesn't impact existing users, they still need to explicitly create ports of afxdp type in order to use them. The code is pretty much independent from other parts of OVS. > - Reversing build option logic might cause problems for distributions> (or > at least require people to think, as now they have to explicitly disable it). This is true for every other feature. And I added an explicit disabling for Debian and Fedora packaging in the patch 3/6. In general, distributions should not blindly take every new release without checking the NEWS anyway. > - I can remember there were some (memory management) issues/limitations> > which William was supposed to send patches for, but never did. I know that we're lacking support for shared umem, which is a nice feature, but not essential. But I don't remember any major memory management issues at the moment. In any case, only patches 3 and 6 are about enabling by default. The rest of the set is about documentation and fixes for the buid process with new versions of libbpf and libxdp, so can be reviewed separately. Patches 4 and 5 might need a slight adjustment if we decide to not enable build by default though. Best regards, Ilya Maximets. > > Cheers, > > Eelco > >> Ilya Maximets (6): >> ci: Fix overriding OPTS provided from the yml. >> netdev-afxdp: Allow building with libxdp and newer libbpf. >> acinclude.m4: Build with AF_XDP support by default if possible. >> github: Test AF_XDP build using libbpf instead of kernel sources. >> Documentation/afxdp: Use packaged libbpf/libxdp for the build. >> rhel: Enable AF_XDP by default in Fedora builds. >> >> .ci/linux-build.sh | 24 +------ >> .github/workflows/build-and-test.yml | 9 +-- >> Documentation/intro/install/afxdp.rst | 43 ++++-------- >> NEWS | 5 ++ >> acinclude.m4 | 94 ++++++++++++++++++--------- >> debian/rules | 8 ++- >> lib/automake.mk | 1 - >> lib/libopenvswitch.pc.in | 2 +- >> lib/netdev-afxdp-pool.c | 2 + >> lib/netdev-afxdp-pool.h | 5 -- >> lib/netdev-afxdp.c | 38 ++++++++--- >> rhel/openvswitch-fedora.spec.in | 8 ++- >> 12 files changed, 128 insertions(+), 111 deletions(-) >> >> -- >> 2.38.1 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
