| Message ID | 20201116093452.7541-1-marekx.majtyka@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | New netdev feature flags for XDP | expand |
alardam@gmail.com writes: > From: Marek Majtyka <marekx.majtyka@intel.com> > > Implement support for checking if a netdev has native XDP and AF_XDP zero > copy support. Previously, there was no way to do this other than to try > to create an AF_XDP socket on the interface or load an XDP program and > see if it worked. This commit changes this by extending existing > netdev_features in the following way: > * xdp - full XDP support (XDP_{TX, PASS, DROP, ABORT, REDIRECT}) > * af-xdp-zc - AF_XDP zero copy support > NICs supporting these features are updated by turning the corresponding > netdev feature flags on. Thank you for working on this! The lack of a way to discover whether an interface supports XDP is really annoying. However, I don't think just having two separate netdev feature flags for XDP and AF_XDP is going to cut it. Whatever mechanism we end up will need to be able to express at least the following, in addition to your two flags: - Which return codes does it support (with DROP/PASS, TX and REDIRECT as separate options)? - Does this interface be used as a target for XDP_REDIRECT (supported/supported but not enabled)? - Does the interface support offloaded XDP? That's already five or six more flags, and we can't rule out that we'll need more; so I'm not sure if just defining feature bits for all of them is a good idea. In addition, we should be able to check this in a way so we can reject XDP programs that use features that are not supported. E.g., program uses REDIRECT return code (or helper), but the interface doesn't support it? Reject at attach/load time! Or the user attempts to insert an interface into a redirect map, but that interface doesn't implement ndo_xdp_xmit()? Reject the insert! Etc. That last bit can be added later, of course, but we need to make sure we design the support in a way that it is possible to do so... -Toke
On Mon, Nov 16, 2020 at 2:25 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > alardam@gmail.com writes: > > > From: Marek Majtyka <marekx.majtyka@intel.com> > > > > Implement support for checking if a netdev has native XDP and AF_XDP zero > > copy support. Previously, there was no way to do this other than to try > > to create an AF_XDP socket on the interface or load an XDP program and > > see if it worked. This commit changes this by extending existing > > netdev_features in the following way: > > * xdp - full XDP support (XDP_{TX, PASS, DROP, ABORT, REDIRECT}) > > * af-xdp-zc - AF_XDP zero copy support > > NICs supporting these features are updated by turning the corresponding > > netdev feature flags on. > > Thank you for working on this! The lack of a way to discover whether an > interface supports XDP is really annoying. > > However, I don't think just having two separate netdev feature flags for > XDP and AF_XDP is going to cut it. Whatever mechanism we end up will > need to be able to express at least the following, in addition to your > two flags: > > - Which return codes does it support (with DROP/PASS, TX and REDIRECT as > separate options)? > - Does this interface be used as a target for XDP_REDIRECT > (supported/supported but not enabled)? > - Does the interface support offloaded XDP? If we want feature discovery on this level, which seems to be a good idea and goal to have, then it is a dead end to bunch all XDP features into one. But fortunately, this can easily be addressed. > That's already five or six more flags, and we can't rule out that we'll > need more; so I'm not sure if just defining feature bits for all of them > is a good idea. I think this is an important question. Is extending the netdev features flags the right way to go? If not, is there some other interface in the kernel that could be used/extended for this? If none of these are possible, then we (unfortunately) need a new interface and in that case, what should it look like? Thanks for taking a look at this Toke. > In addition, we should be able to check this in a way so we can reject > XDP programs that use features that are not supported. E.g., program > uses REDIRECT return code (or helper), but the interface doesn't support > it? Reject at attach/load time! Or the user attempts to insert an > interface into a redirect map, but that interface doesn't implement > ndo_xdp_xmit()? Reject the insert! Etc. > > That last bit can be added later, of course, but we need to make sure we > design the support in a way that it is possible to do so... > > -Toke > > _______________________________________________ > Intel-wired-lan mailing list > Intel-wired-lan@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
On Tue, Nov 17, 2020 at 8:37 AM Magnus Karlsson <magnus.karlsson@gmail.com> wrote: Thank you for your quick answers and comments. > > On Mon, Nov 16, 2020 at 2:25 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > > > alardam@gmail.com writes: > > > > > From: Marek Majtyka <marekx.majtyka@intel.com> > > > > > > Implement support for checking if a netdev has native XDP and AF_XDP zero > > > copy support. Previously, there was no way to do this other than to try > > > to create an AF_XDP socket on the interface or load an XDP program and > > > see if it worked. This commit changes this by extending existing > > > netdev_features in the following way: > > > * xdp - full XDP support (XDP_{TX, PASS, DROP, ABORT, REDIRECT}) > > > * af-xdp-zc - AF_XDP zero copy support > > > NICs supporting these features are updated by turning the corresponding > > > netdev feature flags on. > > > > Thank you for working on this! The lack of a way to discover whether an > > interface supports XDP is really annoying. > > > > However, I don't think just having two separate netdev feature flags for > > XDP and AF_XDP is going to cut it. Whatever mechanism we end up will > > need to be able to express at least the following, in addition to your > > two flags: > > > > - Which return codes does it support (with DROP/PASS, TX and REDIRECT as > > separate options)? > > - Does this interface be used as a target for XDP_REDIRECT > > (supported/supported but not enabled)? > > - Does the interface support offloaded XDP? > > If we want feature discovery on this level, which seems to be a good > idea and goal to have, then it is a dead end to bunch all XDP features > into one. But fortunately, this can easily be addressed. Do you think that is it still considerable to have a single netdev flag that means "some" XDP feature support which would activate new further functionalities? > > > That's already five or six more flags, and we can't rule out that we'll > > need more; so I'm not sure if just defining feature bits for all of them > > is a good idea. > > I think this is an important question. Is extending the netdev > features flags the right way to go? If not, is there some other > interface in the kernel that could be used/extended for this? If none > of these are possible, then we (unfortunately) need a new interface > and in that case, what should it look like? Toke, are you thinking about any particular existing interface or a new specific one? > > Thanks for taking a look at this Toke. > > > In addition, we should be able to check this in a way so we can reject > > XDP programs that use features that are not supported. E.g., program > > uses REDIRECT return code (or helper), but the interface doesn't support > > it? Reject at attach/load time! Or the user attempts to insert an > > interface into a redirect map, but that interface doesn't implement > > ndo_xdp_xmit()? Reject the insert! Etc. > > > > That last bit can be added later, of course, but we need to make sure we > > design the support in a way that it is possible to do so... > > > > -Toke > > > > _______________________________________________ > > Intel-wired-lan mailing list > > Intel-wired-lan@osuosl.org > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Marek Majtyka <alardam@gmail.com> writes: > On Tue, Nov 17, 2020 at 8:37 AM Magnus Karlsson > <magnus.karlsson@gmail.com> wrote: > > Thank you for your quick answers and comments. > >> >> On Mon, Nov 16, 2020 at 2:25 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> > >> > alardam@gmail.com writes: >> > >> > > From: Marek Majtyka <marekx.majtyka@intel.com> >> > > >> > > Implement support for checking if a netdev has native XDP and AF_XDP zero >> > > copy support. Previously, there was no way to do this other than to try >> > > to create an AF_XDP socket on the interface or load an XDP program and >> > > see if it worked. This commit changes this by extending existing >> > > netdev_features in the following way: >> > > * xdp - full XDP support (XDP_{TX, PASS, DROP, ABORT, REDIRECT}) >> > > * af-xdp-zc - AF_XDP zero copy support >> > > NICs supporting these features are updated by turning the corresponding >> > > netdev feature flags on. >> > >> > Thank you for working on this! The lack of a way to discover whether an >> > interface supports XDP is really annoying. >> > >> > However, I don't think just having two separate netdev feature flags for >> > XDP and AF_XDP is going to cut it. Whatever mechanism we end up will >> > need to be able to express at least the following, in addition to your >> > two flags: >> > >> > - Which return codes does it support (with DROP/PASS, TX and REDIRECT as >> > separate options)? >> > - Does this interface be used as a target for XDP_REDIRECT >> > (supported/supported but not enabled)? >> > - Does the interface support offloaded XDP? >> >> If we want feature discovery on this level, which seems to be a good >> idea and goal to have, then it is a dead end to bunch all XDP features >> into one. But fortunately, this can easily be addressed. > > Do you think that is it still considerable to have a single netdev > flag that means "some" XDP feature support which would activate new > further functionalities? Why bother? The presence of any XDP-specific feature bits would imply the support for XDP :) >> > That's already five or six more flags, and we can't rule out that we'll >> > need more; so I'm not sure if just defining feature bits for all of them >> > is a good idea. >> >> I think this is an important question. Is extending the netdev >> features flags the right way to go? If not, is there some other >> interface in the kernel that could be used/extended for this? If none >> of these are possible, then we (unfortunately) need a new interface >> and in that case, what should it look like? > > Toke, are you thinking about any particular existing interface or a > new specific one? I have mostly been thinking about the internal kernel interface. The simple thing would just be to define a whole new bitmap of XDP-specific feature bits that the rest of the kernel can consume. That would also mean we don't have to do pointer chasing to see if the ndos are implemented, which Jesper pointed out the other day actually shows up on his profiling traces. The downside to having them be feature flags is that they can get out of sync, of course. But if we block the support from working unless the right flags are set, that should at least make driver developers pay attention. Although we'd have to change all the drivers in one go, but I suppose that's not too onerous seeing as you just did that for this series :) So what that boils down to is basically what you're doing in this series, but more fine grained, via a new netdev->xdp_features instead of burning bits in netdev->features. As for UAPI, i dunno? Ethtool is netlink now, right? So it should be fairly easy to just extend with a new attribute for XDP? I believe there was originally some resistance to explicitly exposing XDP capabilities to userspace because we wanted all drivers to implement all features. Clearly that has not panned out, though, so as far as I'm concerned we can just expose it and be done with it :) But I'll let others weigh in here; the original discussions predate my involvement. -Toke
From: Marek Majtyka <marekx.majtyka@intel.com> Implement support for checking if a netdev has native XDP and AF_XDP zero copy support. Previously, there was no way to do this other than to try to create an AF_XDP socket on the interface or load an XDP program and see if it worked. This commit changes this by extending existing netdev_features in the following way: * xdp - full XDP support (XDP_{TX, PASS, DROP, ABORT, REDIRECT}) * af-xdp-zc - AF_XDP zero copy support NICs supporting these features are updated by turning the corresponding netdev feature flags on. NOTE: Only the compilation check was performed for: - ice, - igb, - mlx5, - bnxt, - dpaa2, - mvmeta, - mvpp2, - qede, - sfc, - netsec, - cpsw, - xen, - virtio_net. Libbpf library is extended in order to provide a simple API for gathering information about XDP supported capabilities of a netdev. This API utilizes netlink interface towards kernel. With this function it is possible to get xsk supported options for netdev beforehand. The new API is used in core xsk code as well as in the xdpsock sample. These new flags also solve the problem with strict recognition of zero copy support. The problem is that there are drivers out there that only support XDP partially, so it is possible to successfully load the XDP program in native mode, but it will still not be able to support zero-copy as it does not have XDP_REDIRECT support. With af-xdp-zc flag the check is possible and trivial. Marek Majtyka (8): net: ethtool: extend netdev_features flag set drivers/net: turn XDP flags on xsk: add usage of xdp netdev_features flags xsk: add check for full support of XDP in bind libbpf: extend netlink attribute API libbpf: add functions to get XSK modes libbpf: add API to get XSK/XDP caps samples/bpf/xdp: apply netdev XDP/XSK modes info drivers/net/ethernet/broadcom/bnxt/bnxt.c | 1 + .../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 1 + drivers/net/ethernet/intel/i40e/i40e_main.c | 2 + drivers/net/ethernet/intel/ice/ice_main.c | 4 + drivers/net/ethernet/intel/igb/igb_main.c | 2 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 3 + drivers/net/ethernet/marvell/mvneta.c | 1 + .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 1 + .../net/ethernet/mellanox/mlx5/core/en_main.c | 2 + drivers/net/ethernet/qlogic/qede/qede_main.c | 1 + drivers/net/ethernet/sfc/efx.c | 1 + drivers/net/ethernet/socionext/netsec.c | 1 + drivers/net/ethernet/ti/cpsw.c | 2 + drivers/net/ethernet/ti/cpsw_new.c | 2 + drivers/net/tun.c | 4 + drivers/net/veth.c | 1 + drivers/net/virtio_net.c | 1 + drivers/net/xen-netfront.c | 1 + include/linux/netdev_features.h | 6 + include/net/xdp.h | 13 + include/net/xdp_sock_drv.h | 11 + include/uapi/linux/if_xdp.h | 1 + net/ethtool/common.c | 2 + net/xdp/xsk.c | 4 +- net/xdp/xsk_buff_pool.c | 21 +- samples/bpf/xdpsock_user.c | 117 +++++- tools/include/uapi/linux/ethtool.h | 44 ++ tools/include/uapi/linux/if_xdp.h | 1 + tools/lib/bpf/ethtool.h | 49 +++ tools/lib/bpf/libbpf.h | 1 + tools/lib/bpf/libbpf.map | 1 + tools/lib/bpf/netlink.c | 379 +++++++++++++++++- tools/lib/bpf/nlattr.c | 105 +++++ tools/lib/bpf/nlattr.h | 22 + tools/lib/bpf/xsk.c | 54 ++- tools/lib/bpf/xsk.h | 3 +- 36 files changed, 845 insertions(+), 20 deletions(-) create mode 100644 tools/lib/bpf/ethtool.h