| Message ID | 20221210021620.2699347-1-i.maximets@ovn.org |
|---|---|
| Headers | show |
| Series | AF_XDP build fixes and enhancements. | expand |
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: - I still believe AF_XDP support in OVS has not had many field trials, so I would still classify this as an “experimental” feature. - Reversing build option logic might cause problems for distributions (or at least require people to think, as now they have to explicitly disable it). - I can remember there were some (memory management) issues/limitations, which William was supposed to send patches for, but never did. 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
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 >
On Mon, Dec 12, 2022 at 12:20 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > 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. fwiw; we have interest in AF_XDP too and I'm currently evaluating if we have the bits aligned to enable this by default in Debian too. One challenge is that in Debian libbpf 0.8 is a compatibility package and the development package is only available as a 1.x, and at the same time xdp-tools is linked with libbpf 0.8. So when building OVS it will try to link with both libbpf 0.8 via libxdp and libbpf 1.x, which I think may be preventing this from working right now. That is our problem to sort though, and we have no issue with having this important feature be enabled by default in the upstream build system.
On Mon, Dec 12, 2022 at 2:55 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Mon, Dec 12, 2022 at 12:20 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > 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. > > fwiw; we have interest in AF_XDP too and I'm currently evaluating if > we have the bits aligned to enable this by default in Debian too. > > One challenge is that in Debian libbpf 0.8 is a compatibility package > and the development package is only available as a 1.x, and at the > same time xdp-tools is linked with libbpf 0.8. So when building OVS it > will try to link with both libbpf 0.8 via libxdp and libbpf 1.x, which > I think may be preventing this from working right now. > > That is our problem to sort though, and we have no issue with having > this important feature be enabled by default in the upstream build > system. ftr; This was already sorted in Debian, I was using an out of date system. With that fixed, it does appear that OVS or xdp-tools may need an update to work with libbpf 1.x: 2022-12-12T16:01:32.800Z|00067|netdev_afxdp|WARN|libbpf: prog 'xdp_dispatcher': missing BPF prog type, check ELF section name 'xdp/dispatcher' 2022-12-12T16:01:32.800Z|00068|netdev_afxdp|WARN|libbpf: prog 'xdp_dispatcher': failed to load: -22
On 12/12/22 17:17, Frode Nordahl wrote: > On Mon, Dec 12, 2022 at 2:55 PM Frode Nordahl > <frode.nordahl@canonical.com> wrote: >> >> On Mon, Dec 12, 2022 at 12:20 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> >>> 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. >> >> fwiw; we have interest in AF_XDP too and I'm currently evaluating if >> we have the bits aligned to enable this by default in Debian too. >> >> One challenge is that in Debian libbpf 0.8 is a compatibility package >> and the development package is only available as a 1.x, and at the >> same time xdp-tools is linked with libbpf 0.8. So when building OVS it >> will try to link with both libbpf 0.8 via libxdp and libbpf 1.x, which >> I think may be preventing this from working right now. >> >> That is our problem to sort though, and we have no issue with having >> this important feature be enabled by default in the upstream build >> system. > > ftr; This was already sorted in Debian, I was using an out of date system. > > With that fixed, it does appear that OVS or xdp-tools may need an > update to work with libbpf 1.x: > 2022-12-12T16:01:32.800Z|00067|netdev_afxdp|WARN|libbpf: prog > 'xdp_dispatcher': missing BPF prog type, check ELF section name > 'xdp/dispatcher' > 2022-12-12T16:01:32.800Z|00068|netdev_afxdp|WARN|libbpf: prog > 'xdp_dispatcher': failed to load: -22 > Hmm. OVS doesn't load xdp programs on its own, it relies on libbpf to load a default program. BTW, are you testing with the second patch of this patch set applied? Otherwise, OVS will use some deprecated APIs that might be broken. Best regards, Ilya Maximets.
On Mon, Dec 12, 2022 at 5:25 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 12/12/22 17:17, Frode Nordahl wrote: > > On Mon, Dec 12, 2022 at 2:55 PM Frode Nordahl > > <frode.nordahl@canonical.com> wrote: > >> > >> On Mon, Dec 12, 2022 at 12:20 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >>> > >>> 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. > >> > >> fwiw; we have interest in AF_XDP too and I'm currently evaluating if > >> we have the bits aligned to enable this by default in Debian too. > >> > >> One challenge is that in Debian libbpf 0.8 is a compatibility package > >> and the development package is only available as a 1.x, and at the > >> same time xdp-tools is linked with libbpf 0.8. So when building OVS it > >> will try to link with both libbpf 0.8 via libxdp and libbpf 1.x, which > >> I think may be preventing this from working right now. > >> > >> That is our problem to sort though, and we have no issue with having > >> this important feature be enabled by default in the upstream build > >> system. > > > > ftr; This was already sorted in Debian, I was using an out of date system. > > > > With that fixed, it does appear that OVS or xdp-tools may need an > > update to work with libbpf 1.x: > > 2022-12-12T16:01:32.800Z|00067|netdev_afxdp|WARN|libbpf: prog > > 'xdp_dispatcher': missing BPF prog type, check ELF section name > > 'xdp/dispatcher' > > 2022-12-12T16:01:32.800Z|00068|netdev_afxdp|WARN|libbpf: prog > > 'xdp_dispatcher': failed to load: -22 > > > > Hmm. OVS doesn't load xdp programs on its own, it relies on libbpf > to load a default program. Yes, I believe libxdp is at fault here and not this series nor OVS. > BTW, are you testing with the second patch of this patch set applied? > Otherwise, OVS will use some deprecated APIs that might be broken. WIth the whole set, It works fine on a pristine Fedora 37 system with libbpf 0.8, but on for example Debian with libbpf 1.0.1. I've confirmed it works also on Debian with libbpf 0.8. I suspect xdp-tools needs to be updated along the lines of [0] after libbpf was updated in [1]. 0: https://lore.kernel.org/lkml/tencent_7DD02046A8398BE3324F85E0F56ED41EB105@qq.com/T/ 1: https://github.com/torvalds/linux/commit/450b167fb9be91a8164d3f3d734674f5fe95b22d
On Tue, Dec 13, 2022 at 12:02 AM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Mon, Dec 12, 2022 at 5:25 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > On 12/12/22 17:17, Frode Nordahl wrote: > > > On Mon, Dec 12, 2022 at 2:55 PM Frode Nordahl > > > <frode.nordahl@canonical.com> wrote: > > >> > > >> On Mon, Dec 12, 2022 at 12:20 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > >>> > > >>> 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. > > >> > > >> fwiw; we have interest in AF_XDP too and I'm currently evaluating if > > >> we have the bits aligned to enable this by default in Debian too. > > >> > > >> One challenge is that in Debian libbpf 0.8 is a compatibility package > > >> and the development package is only available as a 1.x, and at the > > >> same time xdp-tools is linked with libbpf 0.8. So when building OVS it > > >> will try to link with both libbpf 0.8 via libxdp and libbpf 1.x, which > > >> I think may be preventing this from working right now. > > >> > > >> That is our problem to sort though, and we have no issue with having > > >> this important feature be enabled by default in the upstream build > > >> system. > > > > > > ftr; This was already sorted in Debian, I was using an out of date system. > > > > > > With that fixed, it does appear that OVS or xdp-tools may need an > > > update to work with libbpf 1.x: > > > 2022-12-12T16:01:32.800Z|00067|netdev_afxdp|WARN|libbpf: prog > > > 'xdp_dispatcher': missing BPF prog type, check ELF section name > > > 'xdp/dispatcher' > > > 2022-12-12T16:01:32.800Z|00068|netdev_afxdp|WARN|libbpf: prog > > > 'xdp_dispatcher': failed to load: -22 > > > > > > > Hmm. OVS doesn't load xdp programs on its own, it relies on libbpf > > to load a default program. > > Yes, I believe libxdp is at fault here and not this series nor OVS. > > > BTW, are you testing with the second patch of this patch set applied? > > Otherwise, OVS will use some deprecated APIs that might be broken. > > WIth the whole set, It works fine on a pristine Fedora 37 system with > libbpf 0.8, but on for example Debian with libbpf 1.0.1. I've > confirmed it works also on Debian with libbpf 0.8. > > I suspect xdp-tools needs to be updated along the lines of [0] after > libbpf was updated in [1]. > > 0: https://lore.kernel.org/lkml/tencent_7DD02046A8398BE3324F85E0F56ED41EB105@qq.com/T/ > 1: https://github.com/torvalds/linux/commit/450b167fb9be91a8164d3f3d734674f5fe95b22d An update on this, with the new v1.2.9 release of xdp-tools [2] this is now resolved and we just put this series into a Open vSwitch 3.1.0 git snapshot in Debian experimental [3]. Cheers! 2: https://github.com/xdp-project/xdp-tools/issues/254 3: https://buildd.debian.org/status/package.php?p=openvswitch&suite=experimental